Cope with safe.bare repository=explicit
#116
Conversation
As of Git v2.38.0, there is an option to prevent Git from accessing bare repositories unless asked for explicitly (via `--git-dir` or `GIT_DIR`): `safe.bareRepository`. The tests of `git sizer`, however, assume that Git will access a bare repository when the current directory points inside that repository. This only works if `safe.bareRepository` indicates that this is safe. Let's just override whatever value the system or user-wide Git config has for that config setting, to ensure that the tests can pass. Signed-off-by: Johannes Schindelin <[email protected]>
When a user has configured `safe.bareRepository=explicit` in their user-wide Git config, any Git call will fail when the current directory points inside a Git directory unless `GIT_DIR` is set (either explicitly or via `--git-dir`). This extends to the Git directories of non-bare repositories, too, because Git does not really have any way to discern between them and bare repositories. Therefore, we need to make sure that that `GIT_DIR` is set when calling Git from inside a Git directory. Happily, there is only one Git invocation that needs to be fixed. Signed-off-by: Johannes Schindelin <[email protected]>
There was a problem hiding this comment.
Do I understand correctly that the first git invocation in NewRepository, i.e.
git -C $path rev-parse --git-dir
would still be expected to fail if
- the user has
safe.bareRepository=explicitset, pathis within a bare repository, and- the user hasn't set
GIT_PATHbefore runninggit-sizer
? If so, I think that correctly reflects what the user would expect given that they made that config setting.
In other words, the change only has an effect when that first command would succeed but the second command would have previously failed, namely when
- the user has
safe.bareRepository=explicitset, pathis within a non-bare repository, and- the user hasn't set
GIT_PATHbefore runninggit-sizer
, and that's specifically because git rev-parse --git-path shallow is treated differently than typical git commands.
If that's all correct, then the change seems good to me aside from the problem that I pointed out in a line comment, which I think needs to be addressed.
| @@ -64,8 +64,7 @@ func NewRepository(path string) (*Repository, error) { | |||
| gitDir := smartJoin(path, string(bytes.TrimSpace(out))) | |||
|
|
|||
| //nolint:gosec // `gitBin` is chosen carefully. | |||
| cmd = exec.Command(gitBin, "rev-parse", "--git-path", "shallow") | |||
| cmd.Dir = gitDir | |||
| cmd = exec.Command(gitBin, "--git-dir", gitDir, "rev-parse", "--git-path", "shallow") | |||
There was a problem hiding this comment.
With this change, it is no longer correct to call shallow := smartJoin(...) below, because git rev-parse --git-path is documented to return the path to the file relative to the current directory. Therefore, I think that the line below should be changed to shallow := string(bytes.TrimSpace(out)).
There was a problem hiding this comment.
True. But since you offer a much improved solution with #117 that basically supersedes this here PR, I will simply close this PR instead ;-)
|
When reviewing this PR I found a bunch of things about repository initialization that were gratuitously confusing. What would you think of #117, which does some cleanup and then fixes this issue, instead? |
This matches my understanding. |
Since Git v2.38.0,
safe.bareRepositoryis a thing. When a user configures this with the valueexplicit, Git invocations will fail on bare repositories unlessGIT_DIRis set (either explicitly or via--git-dir).The tests of
git sizerwill currently fail in such a scenario because they invoke Git inside bare repositories without settingGIT_DIR.A more subtle issue arises when calling
git sizerin a non-bare repository whilesafe.bareRepository=explicitis in effect: The call togit rev-parse --git-path shallowis not performed in the worktree but in the Git directory. For reasons, Git treats that situation identically to being called in a bare repository, and refuses to perform the operation.Let's fix both of these issues so that the tests pass and so that
git sizerworks as expected in user setups that havesafe.bareRepositoryset toexplicit.