Skip to content

Make init more proactive in reporting errors #55

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 18, 2023
Merged

Conversation

vdye
Copy link
Collaborator

@vdye vdye commented May 17, 2023

Closes #35

This pull request fixes the remaining item in #35 (git-bundle-server init behaves incorrectly when the specified route already exists). The update to fix this revealed a different bug affecting usage of ReadRepositoryStorage() (repair and, as of this PR, init) when the bundle server directory structure hasn't been created yet.

  • Commit 1 updates init to exit if it receives an error from CloneBareRepo(), rather than trying to continue with creating a base bundle (as it does now).
  • Commit 2 fixes the issue with ReadRepositoryStorage() by ensuring ReadDirRecursive() catches "missing entry" errors and returns an empty result rather than an error.
  • Commit 3 adds an explicit check for the specified route before CreateRepository(). If the route is found in the results of ReadRepositoryStorage(), exit with an error.

If 'GitHelper.CloneBareRepo()' fails, it returns an error. However, the
'init' command ignores this error and attempts to create a base bundle then
fail. Update 'init' to instead exit with the error it receives from
'CloneBareRepo()'.

Signed-off-by: Victoria Dye <[email protected]>
@vdye vdye added this to the v1.0 milestone May 17, 2023
@vdye vdye requested a review from derrickstolee as a code owner May 17, 2023 21:45
@vdye vdye self-assigned this May 17, 2023
@vdye vdye requested a review from ldennington as a code owner May 17, 2023 21:45
vdye added 2 commits May 17, 2023 15:20
If 'ReadDirRecursive()' receives a non-existent root path, the initial
'os.ReadDir()' throws a ENOENT error. Handle this case more gracefully
(i.e., without error) by returning an empty entry list with no error if the
we should exit gracefully if the error from 'os.ReadDir()' is
'os.ErrNotExist' (corresponding to ENOENT).

Signed-off-by: Victoria Dye <[email protected]>
If 'init' tries to initialize a new route, but that route already exists on
disk (enabled or not), exit with an error. Without this explicit check, the
initialization will fail anyway (specifically, the bare repo clone), but the
call to 'RepositoryProvider.CreateRepository()' will enable the existing
route even if it was disabled. To avoid that unintended behavior and provide
clearer information to users, explicitly check for existing routes before
'CreateRepository()' and exit if the route already exists.

Signed-off-by: Victoria Dye <[email protected]>
Copy link
Collaborator

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look like excellent, small fixes. Testing them properly would involve integration-level tests for the most part, but they are also so small we can move forward without them for now.

@vdye vdye merged commit ed48647 into git-ecosystem:main May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Miscellaneous CLI bugs
2 participants