Re: Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-03-13 Thread Junio C Hamano
Eric Sunshine sunsh...@sunshineco.com writes:

 +Prepare a request to your upstream project to pull your changes to
 +their tree to the standard output, by summarizing your changes and
 +showing where your changes can be pulled from.

 Perhaps splitting this into two sentence (and using fewer to's) would
 make it a bit easier to grok? Something like:

 Generate a request asking your upstream project to pull changes
 into their tree. The request, printed to standard output,
 summarizes the changes and indicates from where they can be
 pulled.

Thanks.

 +When the repository named by `url` has the commit at a tip of a
 +ref that is different from the ref you have it locally, you can use

 Did you want to drop it from this sentence? Or did you mean to say
 the ref as you have it locally?

Thanks for your careful reading.  Will drop it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-03-12 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Sorry for back-burnering this topic so long.

 I think the following does what you suggested in the message I am
 responding to.

 Now, hopefully the only thing we need is a documentation update and
 the series should be ready to go.

... and here it is, to be sanity checked before I queue the whole
thing in 'next'.

-- 8 --
Subject: [PATCH] request-pull: documentation updates

The original description talked only about what it does.  Instead,
start it with the purpose of the command, i.e. what it is used for,
and then mention what it does to achieve that goal.

Clarify what start, url and end means in the context of the
overall purpose of the command.

Describe the extended syntax of end parameter that is used when
the local branch name is different from the branch name at the
repository the changes are published.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/git-request-pull.txt | 55 +-
 1 file changed, 49 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-request-pull.txt 
b/Documentation/git-request-pull.txt
index b99681c..57bc1f5 100644
--- a/Documentation/git-request-pull.txt
+++ b/Documentation/git-request-pull.txt
@@ -13,22 +13,65 @@ SYNOPSIS
 DESCRIPTION
 ---
 
-Summarizes the changes between two commits to the standard output, and includes
-the given URL in the generated summary.
+Prepare a request to your upstream project to pull your changes to
+their tree to the standard output, by summarizing your changes and
+showing where your changes can be pulled from.
+
+The upstream project is expected to have the commit named by
+`start` and the output asks it to integrate the changes you made
+since that commit, up to the commit named by `end`, by visiting
+the repository named by `url`.
+
 
 OPTIONS
 ---
 -p::
-   Show patch text
+   Include patch text in the output.
 
 start::
-   Commit to start at.
+   Commit to start at.  This names a commit that is already in
+   the upstream history.
 
 url::
-   URL to include in the summary.
+   The repository URL to be pulled from.
 
 end::
-   Commit to end at; defaults to HEAD.
+   Commit to end at (defaults to HEAD).  This names the commit
+   at the tip of the history you are asking to be pulled.
++
+When the repository named by `url` has the commit at a tip of a
+ref that is different from the ref you have it locally, you can use
+the `local:remote` syntax, to have its local name, a colon `:`,
+and its remote name.
+
+
+EXAMPLE
+---
+
+Imagine that you built your work on your `master` branch on top of
+the `v1.0` release, and want it to be integrated to the project.
+First you push that change to your public repository for others to
+see:
+
+   git push https://git.ko.xz/project master
+
+Then, you run this command:
+
+   git request-pull v1.0 https://git.ko.xz/project master
+
+which will produce a request to the upstream, summarizing the
+changes between the `v1.0` release and your `master`, to pull it
+from your public repository.
+
+If you pushed your change to a branch whose name is different from
+the one you have locally, e.g.
+
+   git push https://git.ko.xz/project master:for-linus
+
+then you can ask that to be pulled with
+
+   git request-pull v1.0 https://git.ko.xz/project master:for-linus
+
 
 GIT
 ---
-- 
1.9.0-293-gd838d6f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-03-12 Thread Eric Sunshine
On Wed, Mar 12, 2014 at 2:04 PM, Junio C Hamano gits...@pobox.com wrote:
 Subject: [PATCH] request-pull: documentation updates

 The original description talked only about what it does.  Instead,
 start it with the purpose of the command, i.e. what it is used for,
 and then mention what it does to achieve that goal.

 Clarify what start, url and end means in the context of the
 overall purpose of the command.

 Describe the extended syntax of end parameter that is used when
 the local branch name is different from the branch name at the
 repository the changes are published.

 Signed-off-by: Junio C Hamano gits...@pobox.com
 ---
  Documentation/git-request-pull.txt | 55 
 +-
  1 file changed, 49 insertions(+), 6 deletions(-)

 diff --git a/Documentation/git-request-pull.txt 
 b/Documentation/git-request-pull.txt
 index b99681c..57bc1f5 100644
 --- a/Documentation/git-request-pull.txt
 +++ b/Documentation/git-request-pull.txt
 @@ -13,22 +13,65 @@ SYNOPSIS
  DESCRIPTION
  ---

 -Summarizes the changes between two commits to the standard output, and 
 includes
 -the given URL in the generated summary.
 +Prepare a request to your upstream project to pull your changes to
 +their tree to the standard output, by summarizing your changes and
 +showing where your changes can be pulled from.

Perhaps splitting this into two sentence (and using fewer to's) would
make it a bit easier to grok? Something like:

Generate a request asking your upstream project to pull changes
into their tree. The request, printed to standard output,
summarizes the changes and indicates from where they can be
pulled.

 +The upstream project is expected to have the commit named by
 +`start` and the output asks it to integrate the changes you made
 +since that commit, up to the commit named by `end`, by visiting
 +the repository named by `url`.
 +

  OPTIONS
  ---
  -p::
 -   Show patch text
 +   Include patch text in the output.

  start::
 -   Commit to start at.
 +   Commit to start at.  This names a commit that is already in
 +   the upstream history.

  url::
 -   URL to include in the summary.
 +   The repository URL to be pulled from.

  end::
 -   Commit to end at; defaults to HEAD.
 +   Commit to end at (defaults to HEAD).  This names the commit
 +   at the tip of the history you are asking to be pulled.
 ++
 +When the repository named by `url` has the commit at a tip of a
 +ref that is different from the ref you have it locally, you can use

Did you want to drop it from this sentence? Or did you mean to say
the ref as you have it locally?

 +the `local:remote` syntax, to have its local name, a colon `:`,
 +and its remote name.
 +
 +
 +EXAMPLE
 +---
 +
 +Imagine that you built your work on your `master` branch on top of
 +the `v1.0` release, and want it to be integrated to the project.
 +First you push that change to your public repository for others to
 +see:
 +
 +   git push https://git.ko.xz/project master
 +
 +Then, you run this command:
 +
 +   git request-pull v1.0 https://git.ko.xz/project master
 +
 +which will produce a request to the upstream, summarizing the
 +changes between the `v1.0` release and your `master`, to pull it
 +from your public repository.
 +
 +If you pushed your change to a branch whose name is different from
 +the one you have locally, e.g.
 +
 +   git push https://git.ko.xz/project master:for-linus
 +
 +then you can ask that to be pulled with
 +
 +   git request-pull v1.0 https://git.ko.xz/project master:for-linus
 +

  GIT
  ---
 --
 1.9.0-293-gd838d6f
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-02-25 Thread Junio C Hamano
Linus Torvalds torva...@linux-foundation.org writes:

 Thinking some more about the tag_name issue, I realize that the other
 patch (Make request-pull able to take a refspec of form
 local:remote) broke another thing.

 The first patch pretty-printed the local branch-name, removing refs/
 and possibly heads/ from the local refname. So for a branch, it
 would ask people to just pull from the branch-name, and for a tag it
 would ask people to pull from tags/name, which is good policy. So if
 you had a tag called for-linus, it would say so (using
 tags/for-linus).

 But the local:remote syntax thing ends up breaking that nice feature.
 The old find_matching_refs would actually cause us to show the tags
 part if it existed on the remote, but that had become pointless and
 counter-productive with the first patch. But with the second patch,
 maybe we should reinstate that logic..

Sorry for back-burnering this topic so long.

I think the following does what you suggested in the message I am
responding to.

Now, hopefully the only thing we need is a documentation update and
the series should be ready to go.

-- 8 --
Subject: request-pull: resurrect pretty refname feature

When asking to fetch/pull a branch whose name is B or a tag whose
name is T, we used to show the command to run as:

git pull $URL B
git pull $URL tags/T

even when B and T were spelled in a more qualified way in order to
disambiguate, e.g. heads/B or refs/tags/T, but the recent update
lost this feature.  Resurrect it.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 git-request-pull.sh | 4 +++-
 t/t5150-request-pull.sh | 8 +++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 93b4135..b67513a 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -53,6 +53,8 @@ fi
 local=${3%:*}
 local=${local:-HEAD}
 remote=${3#*:}
+pretty_remote=${remote#refs/}
+pretty_remote=${pretty_remote#heads/}
 head=$(git symbolic-ref -q $local)
 head=${head:-$(git show-ref --heads --tags $local | cut -d' ' -f2)}
 head=${head:-$(git rev-parse --quiet --verify $local)}
@@ -124,7 +126,7 @@ git show -s --format='The following changes since commit %H:
 
 are available in the git repository at:
 ' $merge_base 
-echo   $url $remote 
+echo   $url $pretty_remote 
 git show -s --format='
 for you to fetch changes up to %H:
 
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 2622057..75d6b38 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -216,8 +216,14 @@ test_expect_success 'pull request format' '
git request-pull initial $downstream_url tags/full ../request
) 
request sed -nf fuzz.sed request.fuzzy 
-   test_i18ncmp expect request.fuzzy
+   test_i18ncmp expect request.fuzzy 
 
+   (
+   cd local 
+   git request-pull initial $downstream_url 
tags/full:refs/tags/full
+   ) request 
+   sed -nf fuzz.sed request request.fuzzy 
+   test_i18ncmp expect request.fuzzy
 '
 
 test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-01-29 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 So there are two remaining items, I think.

  - After creating a tags/for-linus signed tag and pushing it to
tags/for-linus, asking request-pull to request that tag to be
pulled seems to lose the tag message from the output.

  - Docs.

 [Footnote]

 *1* Not that it is always acceptable to break the existing users as
 long as they are clueful ones and they are given an escape hatch.
 But this time I know I won't be in the middle of firestorm like
 the one we had immediately after 1.6.0, as long as I keep the
 URL of the message I am responding to in the list archive ;-)

I am not yet doing the docs, but here is a minimal (and I think is
the most sensible) fix to the If I asked a tag to be pulled, I used
to get the message from the tag in the output---the updated code no
longer does so problem.

With this fix, the updates to t5150 I queued on top of the two
patches can lose a test_expect_failure.

I would not be surprised if there are other regressions, though
[*1*].  I am worried about regressions when the user explicitly asks
a ref to be pulled---e.g the command refuses to produce output and
instead fails (perhaps because the ambiguity check is overly
stricter than it should be), or the command produces output that is
different from what we used to produce (this patch is a fix to the
problem in that latter category, but there may be other differences
the existing tests are not covering).

[Footnote]

*1* No, I do not count I used to be able to ask 'master' (or
implicitly 'HEAD' that I happen to be sitting on) to be pulled and
rely on that the command figures out that I have that commit on
'for-linus' in my publish repository, but that feature was removed
as a regression.  Removing that cleverness is the point of this
series.

-- 8 --
Subject: [PATCH] request-pull: pick up tag message as before

The previous two steps were meant to stop promoting the explicit
refname the user gave to the command to a different ref that points
at it.  Most notably, we no longer substitute a branch name the user
used with a name of the tqag that points at the commit at the tip of
the branch.

However, they also lost the code that included the message in a
tag when the user _did_ ask the tag to be pulled.  Resurrect it.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 git-request-pull.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index c8ab0e9..93b4135 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -132,6 +132,14 @@ for you to fetch changes up to %H:
 
 ' $headrev 
 
+if test $(git cat-file -t $head) = tag
+then
+   git cat-file tag $head |
+   sed -n -e '1,/^$/d' -e '/^-BEGIN PGP /q' -e p
+   echo
+   echo 
+fi 
+
 if test -n $branch_name
 then
echo (from the branch description for $branch_name local branch)
-- 
1.9-rc1-183-g614c158

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-01-29 Thread brian m. carlson
On Wed, Jan 29, 2014 at 03:34:32PM -0800, Junio C Hamano wrote:
 The previous two steps were meant to stop promoting the explicit
 refname the user gave to the command to a different ref that points
 at it.  Most notably, we no longer substitute a branch name the user
 used with a name of the tqag that points at the commit at the tip of

s/tqag/tag/


-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-01-29 Thread Linus Torvalds
On Wed, Jan 29, 2014 at 3:34 PM, Junio C Hamano gits...@pobox.com wrote:

 I am not yet doing the docs, but here is a minimal (and I think is
 the most sensible) fix to the If I asked a tag to be pulled, I used
 to get the message from the tag in the output---the updated code no
 longer does so problem.

That was a complete oversight/bug on my part, due to just removing the
tag_name special cases, not thinking about the tag message.

Thinking some more about the tag_name issue, I realize that the other
patch (Make request-pull able to take a refspec of form
local:remote) broke another thing.

The first patch pretty-printed the local branch-name, removing refs/
and possibly heads/ from the local refname. So for a branch, it
would ask people to just pull from the branch-name, and for a tag it
would ask people to pull from tags/name, which is good policy. So if
you had a tag called for-linus, it would say so (using
tags/for-linus).

But the local:remote syntax thing ends up breaking that nice feature.
The old find_matching_refs would actually cause us to show the tags
part if it existed on the remote, but that had become pointless and
counter-productive with the first patch. But with the second patch,
maybe we should reinstate that logic..

Linus
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-01-24 Thread Junio C Hamano
Linus Torvalds torva...@linux-foundation.org writes:

 So I don't actually think anybody should need to be retrained, or
 always use the local:remote syntax. The local:remote syntax exists
 only for that special insane case where you used (the same)
 local:remote syntax to push out a branch under a different name.

 [ And yeah, maybe that behavior is more common than I think, but even
 if it is, such behavior would always be among people who are *very*
 aware of the whole local branch vs remote branch name is different
 situation. ]

As the new default for git push would push to the same name, I
agree that people who are now forced to use local:remote syntax
would be the ones who know what they are doing [*1*].

So there are two remaining items, I think.

 - After creating a tags/for-linus signed tag and pushing it to
   tags/for-linus, asking request-pull to request that tag to be
   pulled seems to lose the tag message from the output.

 - Docs.


[Footnote]

*1* Not that it is always acceptable to break the existing users as
long as they are clueful ones and they are given an escape hatch.
But this time I know I won't be in the middle of firestorm like
the one we had immediately after 1.6.0, as long as I keep the
URL of the message I am responding to in the list archive ;-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-01-23 Thread Junio C Hamano
Linus Torvalds torva...@linux-foundation.org writes:

 So this relaxes the remote matching, and allows using the local:remote 
 syntax to say that the local branch is differently named from the remote 
 one.

 It is probably worth folding it into the previous patch if you think this 
 whole approach is workable.

Haven't thought enough to decide on that part, yet.

  # $3 must be a symbolic ref, a unique ref, or
 -# a SHA object expression
 +# a SHA object expression. It can also be of
 +# the format 'local-name:remote-name'.
  #
 -head=$(git symbolic-ref -q ${3-HEAD})
 -head=${head:-$(git show-ref ${3-HEAD} | cut -d' ' -f2)}
 -head=${head:-$(git rev-parse --quiet --verify $3)}
 +local=${3%:*}
 +local=${local:-HEAD}
 +remote=${3#*:}
 +head=$(git symbolic-ref -q $local)
 +head=${head:-$(git show-ref --heads --tags $local | cut -d' ' -f2)}
 +head=${head:-$(git rev-parse --quiet --verify $local)}
  
  # None of the above? Bad.
 -test -z $head  die fatal: Not a valid revision: $3
 +test -z $head  die fatal: Not a valid revision: $local
  
  # This also verifies that the resulting head is unique:

I am not sure if it is a good idea to hand-craft resulting head is
unique constraint here.  We already have disambiguation rules (and
warning mechanism) we use in other places---this part should use the
same rule, I think.

A short experiment tells me that we are almost there:

  $ git init  git commit --allow-empty -m initial
  $ git branch other  git tag master
  $ git -c core.warnambiguousrefs=true \
  rev-parse --symbolic-full-name other
  refs/heads/other
  $ git -c core.warnambiguousrefs=true \
  rev-parse --symbolic-full-name master; echo $?
  warning: refname 'master' is ambiguous.
  error: refname 'master' is ambiguous
  0

We say error but we do not actually error out, but that shouldn't
be too hard to fix.

  # git show-ref could have shown multiple matching refs..
  headrev=$(git rev-parse --verify --quiet $head^0)
 -test -z $headrev  die fatal: Ambiguous revision: $3
 +test -z $headrev  die fatal: Ambiguous revision: $local
  
  # Was it a branch with a description?
  branch_name=${head#refs/heads/}
 @@ -69,9 +73,6 @@ then
   branch_name=
  fi
  
 -prettyhead=${head#refs/}
 -prettyhead=${prettyhead#heads/}
 -
  merge_base=$(git merge-base $baserev $headrev) ||
  die fatal: No commits in common between $base and $head
  
 @@ -81,30 +82,37 @@ die fatal: No commits in common between $base and $head
  #
  # Otherwise find a random ref that matches $headrev.
  find_matching_ref='
 + my ($head,$headrev) = (@ARGV);
 + my ($found);
 +
   while (STDIN) {
 + chomp;
   my ($sha1, $ref, $deref) = /^(\S+)\s+([^^]+)(\S*)$/;
 + my ($pattern);
 + next unless ($sha1 eq $headrev);
 +
 + $pattern=/$head\$;

I think $head is constant inside the loop, so lift it outside?

 + if ($ref eq $head) {
 + $found = $ref;
 + }
 + if ($ref =~ /$pattern/) {
 + $found = $ref;
   }
 + if ($sha1 eq $head) {

I think this is $headrev ($head may be $remote or HEAD), but then
anything that does not point at $headrev has already been rejected
at the beginning of this loop, so...?

   $found = $sha1;
   }
   }
 + if ($found) {
   print $found\n;
   }
  '

I somehow feel that this is inadequate to catch the delayed
propagation error in the opposite direction.  The publish
repository may have an unrelated ref pointing at the $headrev and we
may guess that is the ref to be fetched by the integrator based on
that, but by the time the integrator fetches from the repository,
the ref may have been updated to its new value that does not match
$headrev.  But I do not think of a way to solve that one.

In any case, shouldn't we be catching duplicate matches here, if the
real objective is to make it less likely for the users to make
mistakes?  Otherwise, if there are two 'master' over there
(e.g. refs/heads/master and refs/remotes/origin/master), it seems to
me that $ref =~ /$pattern/ will trigger twice in your loop and ends
up reporting the last match.

In other words,

my (@found) = ();
my (@guessed) = ();
while (STDIN) {
next unless ($sha1 eq $headrev);
...
if ($ref =~ /$pattern/) {
push @found, $ref;
} else {
push @guessed, $ref;
}
}
if (@found == 1) {
print $found[0]\n;
} else if (@guessed == 1) {
print $guessed[0]\n;
}

or something like that?

 -ref=$(git ls-remote $url | @@PERL@@ -e $find_matching_ref $head 
 $headrev)
 +ref=$(git ls-remote $url | @@PERL@@ -e $find_matching_ref 
 ${remote:-HEAD} $headrev)

There are three cases as far as ${remote:-HEAD} aka $head in the
script is concerned:

 1. The 

Re: [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-01-23 Thread Linus Torvalds
On Thu, Jan 23, 2014 at 11:43 AM, Junio C Hamano gits...@pobox.com wrote:

 I am not sure if it is a good idea to hand-craft resulting head is
 unique constraint here.  We already have disambiguation rules (and
 warning mechanism) we use in other places---this part should use the
 same rule, I think.

If you can fix that, then yes, that would be lovely. As it is, I
couldn't find any easily scriptable way to do that.

  #
  # Otherwise find a random ref that matches $headrev.
  find_matching_ref='
 + my ($head,$headrev) = (@ARGV);
 + my ($found);
 +
   while (STDIN) {
 + chomp;
   my ($sha1, $ref, $deref) = /^(\S+)\s+([^^]+)(\S*)$/;
 + my ($pattern);
 + next unless ($sha1 eq $headrev);
 +
 + $pattern=/$head\$;

 I think $head is constant inside the loop, so lift it outside?

Yes. I'm not really a perl person, and this came from me trying to
make the code more readable (and it used to do that magic quoting
thing inside the loop, I just used a helper pattern variable).

 + if ($sha1 eq $head) {

 I think this is $headrev ($head may be $remote or HEAD), but then
 anything that does not point at $headrev has already been rejected
 at the beginning of this loop, so...?

No, this is for when head ends up not being a ref, but a SHA1 expression.

IOW, for when you do something odd like

git request-pull HEAD^^ origin HEAD^

when hacking things together. It doesn't actually generate the right
request-pull message (because there's no valid branch name), but it
*works* in the sense that you can get the diffstat etc and edit things
manually.

It's not a big deal - it has never really worked, and I actually
broke that when I then used $remote that doesn't actually have the
SHA1 any more.

 + if ($found) {
   print $found\n;
   }
  '

 I somehow feel that this is inadequate to catch the delayed
 propagation error in the opposite direction.  The publish
 repository may have an unrelated ref pointing at the $headrev and we
 may guess that is the ref to be fetched by the integrator based on
 that, but by the time the integrator fetches from the repository,
 the ref may have been updated to its new value that does not match
 $headrev.  But I do not think of a way to solve that one.

Yes, so you'll get a warning (or, if you get a partial match, maybe
not even that), but the important part about all these changes is that
it DOESN'T MATTER.

Why? Because it no longer re-writes the target branch name based on
that match or non-match. So the pull request will be fine.

In other words, the really fundamental change here i that the oops, I
couldn't find things on the remote no longer affects the output. It
only affects the warning. And I think that's important.

It used to be that the remote matching actually changed the output of
the request-pull, and *THAT* was the fundamental problem.

 In any case, shouldn't we be catching duplicate matches here, if the
 real objective is to make it less likely for the users to make
 mistakes?

It would be good, yes. But my perl-fu is weak, and I really didn't
want to worry about it. Also, as above: my primary issue was to not
screw up the output, so the remote matching actually has become much
less important, and now the warning about it is purely about being
helpful, it no longer fundamentally alters any semantics.

So I agree that there is room for improvement, but that's kind of
separate from the immediate problem I was trying to solve.

  Linus
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-01-23 Thread Junio C Hamano
Linus Torvalds torva...@linux-foundation.org writes:

 Yes, so you'll get a warning (or, if you get a partial match, maybe
 not even that), but the important part about all these changes is that
 it DOESN'T MATTER.

 Why? Because it no longer re-writes the target branch name based on
 that match or non-match. So the pull request will be fine.

Will be fine, provided if they always use local:remote syntax, I'd
agree.

 In other words, the really fundamental change here i that the oops, I
 couldn't find things on the remote no longer affects the output. It
 only affects the warning. And I think that's important.

 It used to be that the remote matching actually changed the output of
 the request-pull, and *THAT* was the fundamental problem.

The fingers of users can be retrained to use the local:remote syntax
after we apologize for this change in the Release Notes, I see only
one issue (we seem to lose the message from the annotated/signed tag
when asking to pull it) remaining, after looking at what updates are
needed for t5150.

Thanks.

-- 8 --
Subject: [PATCH] pull-request: test updates

This illustrates behaviour changes that result from the recent
change by Linus.  Most shows good changes, but there may be
usability regressions:

 - The command continues to fail when the user forgot to push out
   before running the command, but the wording of the message has
   been slightly changed.

 - The command no longer guesses when asked to request the commit at
   the HEAD be pulled after pushing it to a branch 'for-upstream',
   even when that branch points at the correct commit.  The user
   must ask the command with the new master:for-upstream syntax.

 - The command no longer favours a tag that peels to the requested
   commit over a branch that points at the same commit.  This needs
   to be asked explicitly by specifying the tag object, not the
   commit.  But somehow this does not see to work (yet); somewhere
   the tag-ness of the requested ref seems to be lost.

The new behaviour needs to be documented in any case, but we need to
agree what the new behaviour should be before doing so, so...

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 t/t5150-request-pull.sh | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 1afa0d5..412ee4f 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -86,7 +86,7 @@ test_expect_success 'setup: two scripts for reading pull 
requests' '
s/[-0-9]\{10\} [:0-9]\{8\} [-+][0-9]\{4\}/DATE/g
s/[^ ].*/SUBJECT/g
s/  [^ ].* (DATE)/  SUBJECT (DATE)/g
-   s/for-upstream/BRANCH/g
+   s|tags/full|BRANCH|g
s/mnemonic.txt/FILENAME/g
s/^version [0-9]/VERSION/
/^ FILENAME | *[0-9]* [-+]*\$/ b diffstat
@@ -127,7 +127,7 @@ test_expect_success 'pull request when forgot to push' '
test_must_fail git request-pull initial $downstream_url \
2../err
) 
-   grep No branch of.*is at:\$ err 
+   grep No match for commit .* err 
grep Are you sure you pushed err
 
 '
@@ -141,7 +141,7 @@ test_expect_success 'pull request after push' '
git checkout initial 
git merge --ff-only master 
git push origin master:for-upstream 
-   git request-pull initial origin ../request
+   git request-pull initial origin master:for-upstream ../request
) 
sed -nf read-request.sed request digest 
cat digest 
@@ -160,7 +160,7 @@ test_expect_success 'pull request after push' '
 
 '
 
-test_expect_success 'request names an appropriate branch' '
+test_expect_success 'request asks HEAD to be pulled' '
 
rm -fr downstream.git 
git init --bare downstream.git 
@@ -179,11 +179,11 @@ test_expect_success 'request names an appropriate branch' 
'
read repository 
read branch
} digest 
-   test $branch = tags/full
+   test -z $branch
 
 '
 
-test_expect_success 'pull request format' '
+test_expect_failure 'pull request format' '
 
rm -fr downstream.git 
git init --bare downstream.git 
@@ -212,8 +212,8 @@ test_expect_success 'pull request format' '
cd local 
git checkout initial 
git merge --ff-only master 
-   git push origin master:for-upstream 
-   git request-pull initial $downstream_url ../request
+   git push origin tags/full 
+   git request-pull initial $downstream_url tags/full ../request
) 
request sed -nf fuzz.sed request.fuzzy 
test_i18ncmp expect request.fuzzy
@@ -229,7 +229,7 @@ test_expect_success 'request-pull ignores 
OPTIONS_KEEPDASHDASH poison' '
git checkout initial 
git merge --ff-only master 
git push origin master:for-upstream 
-

Re: [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-01-23 Thread Linus Torvalds
On Thu, Jan 23, 2014 at 2:58 PM, Junio C Hamano gits...@pobox.com wrote:

 Will be fine, provided if they always use local:remote syntax, I'd
 agree.

Why? No sane user should actually need to use the local:remote syntax.

The normal situation should be that you create the correctly named
branch or tag locally, and then push it out under that name.

So I don't actually think anybody should need to be retrained, or
always use the local:remote syntax. The local:remote syntax exists
only for that special insane case where you used (the same)
local:remote syntax to push out a branch under a different name.

[ And yeah, maybe that behavior is more common than I think, but even
if it is, such behavior would always be among people who are *very*
aware of the whole local branch vs remote branch name is different
situation. ]

   Linus
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html