Re: [PATCH] request-pull: return the entered branch if more branches are at the same commit

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] request-pull: return the entered branch if more branches are at the same commit

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:48:34

Lars Jarnbo Pedersen [off-list ref] writes:
quoted hunk
 git-request-pull.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/git-request-pull.sh b/git-request-pull.sh
index 8fd15f6..787383f 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -49,11 +49,21 @@ merge_base=`git merge-base $baserev $headrev` ||
 die "fatal: No commits in common between $base and $head"
 
 branch=$(git ls-remote "$url" \
+	| sed -n -e "/^$headrev	refs.heads.$head/{
Isn't $head often omitted, defaulting to HEAD?

Since the original version of this logic was written, git has changed a
lot, not in an incompatible way, but simply it got a lot richer.  Some
assumptions the script made when it was written may need to be revisited,
working backwards from the command line to see what we can compute better
and how.

    $ git request-pull [options] start url [end]

When "end" is specified, and if that is the name of a branch, we know what
branch you are talking about.  We can dereference HEAD with symbolic-ref
if "end" was missing and we defaulted to HEAD.  Either way, in majority of
the cases, the user has pushed out the tip of a local branch and that is
what "end" would be.

But that "end" branch may not necessarily be the name of the branch your
publishing repository has.  By looking at configured refspec mapping and
the push.default configuration, we can tell which remote ref a push to the
url should have updated.  The script predates many configurations that
control this process, and that is the primary reason it currently guesses
from ls-remote output.

You are introducing something better than a guess, but it is not quite
there, I suspect.  Who says that your branch 'my/topic' will push to your
published branch 'my/topic', not 'topic' with "push = my/topic:topic", or
"branch.my/topic.merge = topic", for example?

We can take one step at a time, and your patch might be a good first step
in the right direction, but I think overhauling this script to be more
aware of the ref mapping is worth discussing before moving forward.  After
such a discussion, it may turn out that majority of people do:

    $ git push $my_public_repo master~3:for-linus

and say "git request-pull origin master~3", in which case the current
program output is already correct and the new code may not be adding much
value in practice.

Re: [PATCH] request-pull: return the entered branch if more branches are at the same commit

From: Lars Jarnbo Pedersen <hidden>
Date: 2016-06-15 22:48:35

Hi,

I clearly did not understand the full usage of the request-pull command
when I wrote this patch. 

The development group I'm working with uses the request-pull command in
a pretty "naive" way. We all have one or more development repos all
cloning from the same public repo. We have one "Release Manager" that
puts the next release together by pulling from our development repos on
request. When the next release is ready the public repo gets updated.

When I want to notify the Release Manager that I have new commits I run
the following command:

git request-pull origin/master <my repo> my_dev_branch

So we dont use public repos for each developer (even if it may be best
practice) and therefore I missed some of the points of the request-pull
command.

My main problem with the fact that request-pull may pick the wrong
branch is that there is often a delay between sending of the
request-pull command and the Release Manager actually pulling the
commits and therefore pulling from the wrong branch is potentially very
dangerous. 

That said I agree with all of your points below.

Regards,

- Lars

On Wed, 2010-04-07 at 18:45 -0700, Junio C Hamano wrote:
Lars Jarnbo Pedersen [off-list ref] writes:
quoted
 git-request-pull.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/git-request-pull.sh b/git-request-pull.sh
index 8fd15f6..787383f 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -49,11 +49,21 @@ merge_base=`git merge-base $baserev $headrev` ||
 die "fatal: No commits in common between $base and $head"
 
 branch=$(git ls-remote "$url" \
+	| sed -n -e "/^$headrev	refs.heads.$head/{
Isn't $head often omitted, defaulting to HEAD?

Since the original version of this logic was written, git has changed a
lot, not in an incompatible way, but simply it got a lot richer.  Some
assumptions the script made when it was written may need to be revisited,
working backwards from the command line to see what we can compute better
and how.

    $ git request-pull [options] start url [end]

When "end" is specified, and if that is the name of a branch, we know what
branch you are talking about.  We can dereference HEAD with symbolic-ref
if "end" was missing and we defaulted to HEAD.  Either way, in majority of
the cases, the user has pushed out the tip of a local branch and that is
what "end" would be.

But that "end" branch may not necessarily be the name of the branch your
publishing repository has.  By looking at configured refspec mapping and
the push.default configuration, we can tell which remote ref a push to the
url should have updated.  The script predates many configurations that
control this process, and that is the primary reason it currently guesses
from ls-remote output.

You are introducing something better than a guess, but it is not quite
there, I suspect.  Who says that your branch 'my/topic' will push to your
published branch 'my/topic', not 'topic' with "push = my/topic:topic", or
"branch.my/topic.merge = topic", for example?

We can take one step at a time, and your patch might be a good first step
in the right direction, but I think overhauling this script to be more
aware of the ref mapping is worth discussing before moving forward.  After
such a discussion, it may turn out that majority of people do:

    $ git push $my_public_repo master~3:for-linus

and say "git request-pull origin master~3", in which case the current
program output is already correct and the new code may not be adding much
value in practice.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help