Fix --remote option when a git push remote is set - #6228
Conversation
|
Hey, thanks very much for identifying this issue and providing a potential fix! Looking back through our history, it appears that our locking commands were not updated back in PR #2715 when support for the My only suggestion for this PR is that it would be ideal if we could add some tests which exercise the For example, here's a test we could add to our begin_test "list a single lock (--remote overrides push default)"
(
set -e
reponame="locks-list-remote"
setup_remote_repo_with_file "$reponame" "f.dat"
clone_repo "$reponame" "$reponame"
git lfs lock --json "f.dat" | tee lock.log
id=$(assert_lock lock.log f.dat)
assert_server_lock "$reponame" "$id" "refs/heads/main"
git remote add bad-remote "invalid-url"
git config remote.pushDefault bad-remote
git lfs locks 2>&1 | tee locks.log
if [ "0" -eq "${PIPESTATUS[0]}" ]; then
echo >&2 "fatal: expected 'git lfs locks' to fail ..."
exit 1
fi
git lfs locks --remote origin --path "f.dat" | tee locks.log
[ 1 -eq "$(wc -l <locks.log)" ]
grep "f.dat" locks.log
grep "Git LFS Tests" locks.log
)
end_testWould you be willing to add a test similar to this to each of the Thank you again very much for taking the time to help improve our project! |
chrisd8088
left a comment
There was a problem hiding this comment.
Thanks for this PR! I'm going to approve it, as the changes to the Git LFS client itself look good to me.
If we can add some tests to it to demonstrate that the changes are effective, that would be even better, and then we can kick off the CI suite and make sure it passes.
In a prior commit in this PR we updated the Git LFS locking commands so that they correctly prioritize the Git remote specified with the command-line "--remote" option when a Git "remote.pushDefault" or "branch.<name>.pushRemote" configuration option is also defined and identifies a different Git remote. Previously, our "git lfs lock", "git lfs locks", and "git lfs unlock" commands would incorrectly prioritize a remote specified by the "remote.pushDefault" or "branch.<name>.pushRemote" configuration options over a remote provided as the argument of the "--remote" command-line option. This oversight was first introduced when we added support for the "remote.pushDefault" and "branch.<name>.pushRemote" configuration options in PR git-lfs#2715. The "--remote" option was already supported by our locking commands at that time, as it was part of the original implementation of the commands in PR git-lfs#1256. However, our test suite has apparently never contained any tests which exercise the "--remote" command-line option of our locking commands. We therefore now add tests which check that each of the three locking commands succeeds when an invalid Git remote is specified in either the "remote.pushDefault" or "branch.<name>.pushRemote" configuration options and a valid remote is also provided as the argument of the "--remote" command-line option. Note that all of these new tests would fail without the changes to the locking commands that we made in this PR.
371a942 to
0c2b6d8
Compare
|
I hope you don't mind; I went ahead and added four new tests I wrote to this PR so that our test suite now exercises the |
When running
git lfscommands in a local branch that has a push remote set that's different from the fetch remote the--remoteoption gets ignored and the push remote is always queried. For example if you have a repository which is configured like this:Then the command
git lfs locks --remote originwill actually query thepreviewremote for the locks rather than the requestedoriginremote.This PR fixes that bug so that the
--remoteoption is honoured for thelock,locksandunlockcommands.