git: ensure exec option is propagated to child git clis #5096
Conversation
This file contains 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
|
We should make sure test coverage catches this case now. |
tonistiigi
reviewed
Jun 27, 2024
| @@ -119,20 +119,13 @@ func NewGitCLI(opts ...Option) *GitCLI { | |||
| // New returns a new git client with the same config as the current one, but | |||
| // with the given options applied on top. | |||
| func (cli *GitCLI) New(opts ...Option) *GitCLI { | |||
There was a problem hiding this comment.
Maybe this should be renamed to WithOptions() or smth. to make it clearer that this is not a constructor but a way to make a new variant of the existing instance with additional config options added.
|
FWIW, I have now also tested this change and verified it fixes the bug (for completeness, not because I didn't believe it would) ❤️ Happy to see someone who's familiar with this codebase working on it instead of me! 😂 ❤️ |
This was referenced Jun 27, 2024
|
I've seen this, just haven't had a moment to write the test - will look at this as soon as I can. |
Signed-off-by: Justin Chadwell <me@jedevc.com>
tonistiigi
force-pushed
the
git-checkout-umask
branch
from
35c359b
to
66859e5
Compare
|
Pushed a test update |
tonistiigi
force-pushed
the
git-checkout-umask
branch
from
66859e5
to
11c9682
Compare
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
tonistiigi
force-pushed
the
git-checkout-umask
branch
from
11c9682
to
899cac1
Compare
jedevc
commented
Jul 3, 2024
tonistiigi
approved these changes
Jul 3, 2024
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Alternative to #5092, fixes #5066.
This option is already correctly specified in
gitCLI, the call chain looks like:gitutil.NewCLI(..., gitutil.WithExec(runWithStandardUmask))called ingitCLIgitCLIcalled inmountRemotegit.Newcalled inmountRemoteThe
execfield specified byWithExecfrom the firstNewCLIcall should propagate down togit.New- but this wasn't being done, I clearly missed this somehow.