This repository was archived by the owner on Sep 3, 2025. It is now read-only.
Conversation
The 'UpdateBareRepo()' method was added to the 'GitHelper' interface in 62f35ce (bundles: fetch changes before building incremental bundle, 2023-03-14), but the corresponding mock method was not added to 'MockGitHelper'. Add it so that 'MockGitHelper' can be used as a 'GitHelper' mock in later commits. Signed-off-by: Victoria Dye <vdye@github.com>
Add a 'GetRemoteUrl()' function to 'git.GitHelper', which calls 'git remote get-url origin' on a given repository path and returns the whitespace- trimmed remote URL. Neither stdout nor stderr are printed to the console, but the stderr text is included in the error message if Git exits with an error status. Although it is not used in this patch, 'GetRemoteUrl()' will be used in later commits to display information about the configuration of the bundle server. Signed-off-by: Victoria Dye <vdye@github.com>
Add a new 'git-bundle-server' command to list the routes that are currently configured and their remote Git URLs. Update manpage and 'README.md' with description and usage information. Due to how routes are currently stored in the bundle server, only "active" routes are listed (that is, routes that have not been stopped with 'git-bundle-server stop'). This is intended to be only a temporary issue, with future updates to how routes are stored allowing better tracking of "stopped" routes. Signed-off-by: Victoria Dye <vdye@github.com>
derrickstolee
approved these changes
Mar 28, 2023
Collaborator
derrickstolee
left a comment
There was a problem hiding this comment.
Good changes. Looking forward to integration tests that let us check these CLI changes on a real filesystem.
| for _, tt := range getRepositoriesTests { | ||
| t.Run(tt.title, func(t *testing.T) { | ||
| testFileSystem.On("UserHomeDir").Return("/my/test/dir", nil) | ||
| testFileSystem.On("ReadFileLines", |
Collaborator
There was a problem hiding this comment.
I suppose this means we are not checking that each mock is called at least once?
Collaborator
Author
There was a problem hiding this comment.
In that test, we weren't. There is a function for doing that (mock.AssertExpectationsForObjects()), so I've also added that to TestRepos_GetRepositories().
| } | ||
| } | ||
|
|
||
| return |
Collaborator
There was a problem hiding this comment.
Interesting that since you named the return values in the function prototype, you don't need to list them here.
Remove mock of 'UserHomeDir()' for the 'FileSystem' interface leftover from an earlier iteration of de58f24 (repo: add more GetRepositories() tests, fix bug, 2023-01-31). The function was removed by the time the commit was merged, so the mock can safely be deleted. Add a call to 'mock.AssertExpectationsForObjects()' to ensure all mocked functions are called. This function should be called in all mocked unit tests to ensure similar issues do not occur in the future. Signed-off-by: Victoria Dye <vdye@github.com>
Add the 'ReadDirRecursive()' function to 'FileSystem' to facilitate easy
lookup of filesystem entries at a fixed depth from the root. In later
commits, this will be used to look up routes in the bundle server under the
'reporoot' on the filesystem.
'ReadDirRecursive()' takes three arguments:
* 'path': the root directory into which we want to recurse
* 'depth': the number of times to recurse into 'path'. A depth of 0 will
return nothing, a depth of 1 will return all entries inside 'path', a
depth of 2 will return all entries inside the subdirectories of 'path',
etc.
* 'strictDepth': a flag indicating whether to return only entries that are
*exactly 'depth' levels beneath 'path'. If 'false', 'ReadDirRecursive()'
will also include files and empty directories at depths less than 'depth'.
For example, consider the following directory tree:
.
├── another-dir/
│ └── testfile
├── myfile
├── org/
│ └── repo/
│ ├── deeper/
│ └── repofile
└── too-shallow/
'ReadDirRecursive(".", 2, true)' will return:
- ./another-dir/testfile
- ./org/repo/
Whereas 'ReadDirRecursive(".", 2, false)' will return:
- ./another-dir/testfile
- ./myfile
- ./org/repo/
- ./too-shallow/
Signed-off-by: Victoria Dye <vdye@github.com>
Replace manual path joining via string concatenation with use of 'filepath.Join()'. The path normalization provided by the function helps avoid issues caused by missing (or extra) path separators and aligns paths to the to the appropriate filesystem's conventions. Signed-off-by: Victoria Dye <vdye@github.com>
Add a function to the 'core.RepositoryProvider' that reads the on-disk repository storage for the bundle server and returns the valid repos in a route -> 'core.Repository' map. 'ReadRepositoryStorage()' identifies repositories by finding the directories at a depth of 2 from the repo storage root, then checking that the repository has a remote 'origin'. The result includes the "active" routes in the bundle server (stored in the 'routes' file) as well as those that have been removed from the routes file with 'git-bundle-server stop'. In later commits, this information will be used to provide more complete information about the state of the bundle server and configure "repairs" to a corrupted server. Signed-off-by: Victoria Dye <vdye@github.com>
Rename the 'core.RepositoryProvider' function 'writeRouteFile()' to 'WriteAllRoutes()' to make it accessible from other packages. The ability to make "bulk" changes to the repository registry and commit all at once will be useful in future patches, such as rebuilding the route list based on the repositories on disk. Signed-off-by: Victoria Dye <vdye@github.com>
Create a new command 'repair' to fix common issues with the bundle server's internal storage. Start by implementing the 'routes' subcommand, which (by default) removes routes from the 'routes' file if they do not appear (or are not Git repositories) on disk. Include two additional options: - '--dry-run', which gathers repository information and constructs the list of repairs needed, but does not update anything - '--start-all', which adds any unregistered, valid repositories on disk to the 'routes' file (effectively running 'git-bundle-server start' on all) Signed-off-by: Victoria Dye <vdye@github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #35
Summary
This pull request introduces two new commands to the bundle server CLI:
listandrepair. Thelistcommand lists the route and remote URL pairs for each route active within the bundle server. It is implemented by the first three commits:GitHelperGitHelperto retrieve theoriginremote URLlistcommand, including documentationThe
repaircommand is meant to be a general-purpose tool for keeping the bundle server internally consistent. The first subcommand (implemented here) is theroutesoption, which checks the repo content in thereporootdirectory and compares it to theroutesfile. The remaining commits implement it:FileSystemhelper function for recursively listing directory contentsRepoProviderwithfilepath.JoinsRepoProviderfunction for getting the repo content that exists atreporoot(independent of theroutesfile)routesfile accessible to external packagesrepair routescommand, including documentation and two options (--dry-runand--start-all)Future work
There's lots of room for future improvement on these commands/things related to them, including:
listinclude the "inactive" repos not in theroutesfile--alloption tostartorstopto enable or disable all repos in the server at once--cleanupflag torepair routesto delete files that aren't part of a valid repository in thereporootrepair, such asbundle(to correct the bundle list(s))routesfile, each having anActiveboolean flag to indicate active status