Skip to content

verify locks against each ref being pushed#2706

Merged
technoweenie merged 11 commits intomasterfrom
api-add-ref
Nov 3, 2017
Merged

verify locks against each ref being pushed#2706
technoweenie merged 11 commits intomasterfrom
api-add-ref

Conversation

@technoweenie
Copy link
Contributor

@technoweenie technoweenie commented Oct 31, 2017

Implements 2 aspects of #2712:

  • basic push.default support, which governs how Git chooses an implicit remote ref based on the remote and local ref being pushed.
  • Verify locks on each ref being pushed.

Bonus: locks are not verified for git lfs push --object-id anymore. Since this push doesn't affect any Git code, there's no need to verify anything.

This also extracts a *lockVerifier and *refUpdate. There is still too much coupling, but it's better than everything being in *uploadContext. Baby steps....

@technoweenie technoweenie added this to the v2.4.0 milestone Oct 31, 2017
@technoweenie technoweenie mentioned this pull request Oct 31, 2017
2 tasks
@technoweenie
Copy link
Contributor Author

I ran into a snag supporting the locks/verify endpoint. Pushes can either specify a remote ref, or you can push all the refs.

$ git push origin local-branch:remote-branch

# calls the pre-push hook by passing this to STDIN:

echo "refs/heads/local-branch AFTER-SHA refs/heads/remote-branch BEFORE-SHA" | git lfs pre-push

That leaves us with some options:

  1. Call the locks/verify endpoint all the refs as a json array. This leads to some small inconsistencies with the API, with some endpoints sending a ref string, and others sending a refs array.
  2. Call the locks/verify endpoint for each ref. It adds more locks calls, but also keeps the API consistent and gives the server implementation the ability to return filtered locked files per ref, if desired.

I'm currently leaning towards option 2.

@technoweenie
Copy link
Contributor Author

#2700 implements this portion of the git push docs:

When the command line does not specify where to push with the <repository> argument, branch.*.remote configuration for the current branch is consulted to determine where to push. If the configuration is missing, it defaults to origin.

Now, it needs to implement the following:

When the command line does not specify what to push with <refspec>... arguments or --all, --mirror, --tags options, the command finds the default <refspec> by consulting remote.*.push configuration, and if it is not found, honors push.default configuration to decide what to push (See git-config for the meaning of push.default).

When neither the command-line nor the configuration specify what to push, the default behavior is used, which corresponds to the simple value for push.default: the current branch is pushed to the corresponding upstream branch, but as a safety measure, the push is aborted if the upstream branch does not have the same name as the local one.

@technoweenie technoweenie mentioned this pull request Nov 2, 2017
9 tasks
@technoweenie technoweenie changed the title Send remote ref with api calls verify locks against each ref being pushed Nov 2, 2017
@technoweenie technoweenie merged commit 5904e06 into master Nov 3, 2017
@technoweenie technoweenie deleted the api-add-ref branch November 3, 2017 16:32
config/config.go Outdated
merge, _ := c.Git.Get(fmt.Sprintf("branch.%s.merge", r.Name))
if strings.HasPrefix(merge, "refs/heads/") {
c.remoteRef = &git.Ref{
Name: merge[11:],
Copy link

Choose a reason for hiding this comment

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

Caspernaing

chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Feb 7, 2022
Since at least PR git-lfs#2706, and indeed earlier, the local and remote
refs which are part of the RefUpdate structure have been referred
to as the "left" and "right" refs, terminology which stems from
the origin of this structure in the "git lfs pre-push" hook command,
where each line of input contains commit information in the form:

  <local ref> <local sha1> <remote ref> <remote sha1>

However, outside of this context, "left" and "right" are ambiguous
terms, and may potentially be confused with the left and right
refs specified in a Git-style two-dot range notation.  We therefore
replace these terms with "local" and "remote", which should help
clarify their purpose throughout the codebase.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Feb 7, 2022
Since at least PR git-lfs#2706, and indeed earlier, the local and remote
refs which are part of the RefUpdate structure have been referred
to as the "left" and "right" refs, terminology which stems from
the origin of this structure in the "git lfs pre-push" hook command,
where each line of input contains commit information in the form:

  <local ref> <local sha1> <remote ref> <remote sha1>

However, outside of this context, "left" and "right" are ambiguous
terms, and may potentially be confused with the left and right
refs specified in a Git-style two-dot range notation.  We therefore
replace these terms with "local" and "remote", which should help
clarify their purpose throughout the codebase.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Feb 7, 2022
Since at least PR git-lfs#2706, and indeed earlier, the local and remote
refs which are part of the RefUpdate structure have been referred
to as the "left" and "right" refs, terminology which stems from
the origin of this structure in the "git lfs pre-push" hook command,
where each line of input contains commit information in the form:

  <local ref> <local sha1> <remote ref> <remote sha1>

However, outside of this context, "left" and "right" are ambiguous
terms, and may potentially be confused with the left and right
refs specified in a Git-style two-dot range notation.  We therefore
replace these terms with "local" and "remote", which should help
clarify their purpose throughout the codebase.
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.

1 participant