{"thread":{"id":"23376","subject":"[PATCH] request-pull: return the entered branch if more branches are at the same commit","startedAt":"2010-04-07T22:22:36Z","lastAt":"2010-04-08T20:37:17Z","messageCount":3,"participants":["Lars Jarnbo Pedersen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"138898","messageId":"1270678956.27317.34.camel@neo","threadId":"23376","inReplyTo":null,"subject":"[PATCH] request-pull: return the entered branch if more branches are at the same commit","fromName":"Lars Jarnbo Pedersen","fromEmail":"lars.jarnbo.pedersen@gmail.com","sentAt":"2010-04-07T22:22:36Z","receivedAt":"2010-04-07T22:22:36Z","isPatch":true,"sender":{"key":"lars.jarnbo.pedersen@gmail.com","avatar":"https://gravatar.com/avatar/47aad966277ba481ef51b433b9405befb2847e65acdcc7b7743733a1305a22ec?d=mp&s=160"},"body":"Currently request-pull identifies the branch to pull from by finding the commit\nusing rev-parse and then identifying the branch from the commit using ls-remote.\nIf more branches are pointing to the same commit the first will be chosen even\nif the one we entered in the command-line exist.\n\nThis change returns the same branch as entered at the commandline if it exists in refs/heads.\n\nSigned-off-by: Lars Jarnbo Pedersen <lars.jarnbo.pedersen@gmail.com>\n---\n git-request-pull.sh |   10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex 8fd15f6..787383f 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -49,11 +49,21 @@ merge_base=`git merge-base $baserev $headrev` ||\n die \"fatal: No commits in common between $base and $head\"\n \n branch=$(git ls-remote \"$url\" \\\n+\t| sed -n -e \"/^$headrev\trefs.heads.$head/{\n+\t\ts/^.*\trefs.heads.//\n+\t\tp\n+\t\tq\n+\t}\")\n+\n+if [ -z \"$branch\" ]; then\n+    branch=$(git ls-remote \"$url\" \\\n \t| sed -n -e \"/^$headrev\trefs.heads./{\n \t\ts/^.*\trefs.heads.//\n \t\tp\n \t\tq\n \t}\")\n+fi\n+\n url=$(get_remote_url \"$url\")\n if [ -z \"$branch\" ]; then\n \techo \"warn: No branch of $url is at:\" >&2\n-- \n1.7.1.rc0.1.ga751b\n"},{"id":"138907","messageId":"7vvdc290ol.fsf@alter.siamese.dyndns.org","threadId":"23376","inReplyTo":"1270678956.27317.34.camel@neo","subject":"Re: [PATCH] request-pull: return the entered branch if more branches are at the same commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-08T01:45:46Z","receivedAt":"2010-04-08T01:45:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Jarnbo Pedersen <lars.jarnbo.pedersen@gmail.com> writes:\n\n>  git-request-pull.sh |   10 ++++++++++\n>  1 files changed, 10 insertions(+), 0 deletions(-)\n>\n> diff --git a/git-request-pull.sh b/git-request-pull.sh\n> index 8fd15f6..787383f 100755\n> --- a/git-request-pull.sh\n> +++ b/git-request-pull.sh\n> @@ -49,11 +49,21 @@ merge_base=`git merge-base $baserev $headrev` ||\n>  die \"fatal: No commits in common between $base and $head\"\n>  \n>  branch=$(git ls-remote \"$url\" \\\n> +\t| sed -n -e \"/^$headrev\trefs.heads.$head/{\n\nIsn't $head often omitted, defaulting to HEAD?\n\nSince the original version of this logic was written, git has changed a\nlot, not in an incompatible way, but simply it got a lot richer.  Some\nassumptions the script made when it was written may need to be revisited,\nworking backwards from the command line to see what we can compute better\nand how.\n\n    $ git request-pull [options] start url [end]\n\nWhen \"end\" is specified, and if that is the name of a branch, we know what\nbranch you are talking about.  We can dereference HEAD with symbolic-ref\nif \"end\" was missing and we defaulted to HEAD.  Either way, in majority of\nthe cases, the user has pushed out the tip of a local branch and that is\nwhat \"end\" would be.\n\nBut that \"end\" branch may not necessarily be the name of the branch your\npublishing repository has.  By looking at configured refspec mapping and\nthe push.default configuration, we can tell which remote ref a push to the\nurl should have updated.  The script predates many configurations that\ncontrol this process, and that is the primary reason it currently guesses\nfrom ls-remote output.\n\nYou are introducing something better than a guess, but it is not quite\nthere, I suspect.  Who says that your branch 'my/topic' will push to your\npublished branch 'my/topic', not 'topic' with \"push = my/topic:topic\", or\n\"branch.my/topic.merge = topic\", for example?\n\nWe can take one step at a time, and your patch might be a good first step\nin the right direction, but I think overhauling this script to be more\naware of the ref mapping is worth discussing before moving forward.  After\nsuch a discussion, it may turn out that majority of people do:\n\n    $ git push $my_public_repo master~3:for-linus\n\nand say \"git request-pull origin master~3\", in which case the current\nprogram output is already correct and the new code may not be adding much\nvalue in practice.\n"},{"id":"138986","messageId":"1270759037.27317.104.camel@neo","threadId":"23376","inReplyTo":"7vvdc290ol.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] request-pull: return the entered branch if more branches are at the same commit","fromName":"Lars Jarnbo Pedersen","fromEmail":"lars.jarnbo.pedersen@gmail.com","sentAt":"2010-04-08T20:37:17Z","receivedAt":"2010-04-08T20:37:17Z","isPatch":true,"sender":{"key":"lars.jarnbo.pedersen@gmail.com","avatar":"https://gravatar.com/avatar/47aad966277ba481ef51b433b9405befb2847e65acdcc7b7743733a1305a22ec?d=mp&s=160"},"body":"Hi,\n\nI clearly did not understand the full usage of the request-pull command\nwhen I wrote this patch. \n\nThe development group I'm working with uses the request-pull command in\na pretty \"naive\" way. We all have one or more development repos all\ncloning from the same public repo. We have one \"Release Manager\" that\nputs the next release together by pulling from our development repos on\nrequest. When the next release is ready the public repo gets updated.\n\nWhen I want to notify the Release Manager that I have new commits I run\nthe following command:\n\ngit request-pull origin/master <my repo> my_dev_branch\n\nSo we dont use public repos for each developer (even if it may be best\npractice) and therefore I missed some of the points of the request-pull\ncommand.\n\nMy main problem with the fact that request-pull may pick the wrong\nbranch is that there is often a delay between sending of the\nrequest-pull command and the Release Manager actually pulling the\ncommits and therefore pulling from the wrong branch is potentially very\ndangerous. \n\nThat said I agree with all of your points below.\n\nRegards,\n\n- Lars\n\nOn Wed, 2010-04-07 at 18:45 -0700, Junio C Hamano wrote:\n> Lars Jarnbo Pedersen <lars.jarnbo.pedersen@gmail.com> writes:\n> \n> >  git-request-pull.sh |   10 ++++++++++\n> >  1 files changed, 10 insertions(+), 0 deletions(-)\n> >\n> > diff --git a/git-request-pull.sh b/git-request-pull.sh\n> > index 8fd15f6..787383f 100755\n> > --- a/git-request-pull.sh\n> > +++ b/git-request-pull.sh\n> > @@ -49,11 +49,21 @@ merge_base=`git merge-base $baserev $headrev` ||\n> >  die \"fatal: No commits in common between $base and $head\"\n> >  \n> >  branch=$(git ls-remote \"$url\" \\\n> > +\t| sed -n -e \"/^$headrev\trefs.heads.$head/{\n> \n> Isn't $head often omitted, defaulting to HEAD?\n> \n> Since the original version of this logic was written, git has changed a\n> lot, not in an incompatible way, but simply it got a lot richer.  Some\n> assumptions the script made when it was written may need to be revisited,\n> working backwards from the command line to see what we can compute better\n> and how.\n> \n>     $ git request-pull [options] start url [end]\n> \n> When \"end\" is specified, and if that is the name of a branch, we know what\n> branch you are talking about.  We can dereference HEAD with symbolic-ref\n> if \"end\" was missing and we defaulted to HEAD.  Either way, in majority of\n> the cases, the user has pushed out the tip of a local branch and that is\n> what \"end\" would be.\n> \n> But that \"end\" branch may not necessarily be the name of the branch your\n> publishing repository has.  By looking at configured refspec mapping and\n> the push.default configuration, we can tell which remote ref a push to the\n> url should have updated.  The script predates many configurations that\n> control this process, and that is the primary reason it currently guesses\n> from ls-remote output.\n> \n> You are introducing something better than a guess, but it is not quite\n> there, I suspect.  Who says that your branch 'my/topic' will push to your\n> published branch 'my/topic', not 'topic' with \"push = my/topic:topic\", or\n> \"branch.my/topic.merge = topic\", for example?\n> \n> We can take one step at a time, and your patch might be a good first step\n> in the right direction, but I think overhauling this script to be more\n> aware of the ref mapping is worth discussing before moving forward.  After\n> such a discussion, it may turn out that majority of people do:\n> \n>     $ git push $my_public_repo master~3:for-linus\n> \n> and say \"git request-pull origin master~3\", in which case the current\n> program output is already correct and the new code may not be adding much\n> value in practice.\n> \n"}]}