Junio C Hamano <gitster@xxxxxxxxx> writes:
> Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
>
>> I think that would probably be a good idea, although I'd actually
>> prefer you to be more verbose, and more human-friendly, and actually
>> talk about the commit in a readable way. Get rid of the *horrible*
>> BRANCH-NOT-VERIFIED message...
>>
>> Top commit 1f51b001cccf: "Merge branches 'cns3xxx/fixes',
>> 'omap/fixes' and 'davinci/fixes' into fixes"
>>
>> and at *that* point you might have a "UNVERIFIED" notice for people
>> to check if they forgot to push.
>
> That UNVERIFIED thing was neither my favorite nor my idea, and I'd happily
> rip it out in any second ;-)
So this is the third round.
-- >8 --
The message gives a detailed explanation of the commit the requester based
the changes on, but lacks information that is necessary for the person who
performs a fetch & merge in order to verify that the correct branch was
fetched when responding to the pull request.
Add a few more lines to describe the commit at the tip expected to be
fetched to the same level of detail as the base commit.
Also update the warning message slightly when the script notices that the
commit may not have been pushed.
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
A UI wart that we cannot fix without breaking backward compatibility is
that the "end" parameter (which defaults to HEAD and is assigned to $head
variable in the script) the requestor uses from the command line names a
commit (often the name of a local branch), but for the purpose of telling
which ref to pull from the public repository, that is a _wrong_ thing to
give to the recipient.
Because the act of generating a request-pull message and the act of
pushing to the public repository are not linked in any way, the script
does not know _how_ the requestor caused (or intends to cause) the commit
to sit at the tip of which branch. There is no guarantee that a lazy "git
push" that relies on the configured refspec will be (or have been) used,
so even parsing the output from "git push -n --porcelain -v $there" would
not tell the script which branch the commit to be pulled is to be pushed
out to, or if the branch is consistent with the request message.
The use of "git ls-remote" in the script and picking one of the refs that
matches the commit object at random from its output is unsatisfactory, but
that is unfortunately the best this script could do without correcting the
design mistake and redefining what the "end" parameter means.
If we can break the backward compatibility and redefine that the "end"
parameter now means the name of the branch at the public repository, it
would make the operation a lot more robust. We could then:
- $branch is what is given by the end user (it is an error not to give
the "end" parameter);
- run "git ls-remote $url $head" to find $headrev;
- generate the message and shortlog using the information obtained from
$url; and
- get rid of "did you forget to push" message.
We could allow adding yet another argument which names a commit object
locally, and make sure if the $headrev observed by ls-remote does not
match it.
---
git-request-pull.sh | 34 +++++++++++++++++++---------------
t/t5150-request-pull.sh | 6 ++++++
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/git-request-pull.sh b/git-request-pull.sh
index afb75e8..438e7eb 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -35,7 +35,7 @@ do
shift
done
-base=$1 url=$2 head=${3-HEAD}
+base=$1 url=$2 head=${3-HEAD} status=0
test -n "$base" && test -n "$url" || usage
baserev=$(git rev-parse --verify "$base"^0) &&
@@ -51,25 +51,29 @@ find_matching_branch="/^$headrev "'refs\/heads\//{
}'
branch=$(git ls-remote "$url" | sed -n -e "$find_matching_branch")
url=$(git ls-remote --get-url "$url")
-if test -z "$branch"
-then
- echo "warn: No branch of $url is at:" >&2
- git log --max-count=1 --pretty='tformat:warn: %h: %s' $headrev >&2
- echo "warn: Are you sure you pushed $head there?" >&2
- echo >&2
- echo >&2
- branch=..BRANCH.NOT.VERIFIED..
- status=1
-fi
git show -s --format='The following changes since commit %H:
%s (%ci)
-are available in the git repository at:' $baserev &&
-echo " $url $branch" &&
-echo &&
+are available in the git repository at:
+' $baserev &&
+echo " $url${branch+ $branch}" &&
+git show -s --format='
+for you to fetch changes up to %H:
+
+ %s (%ci)
+
+----------------------------------------------------------------' $headrev &&
git shortlog ^$baserev $headrev &&
-git diff -M --stat --summary $patch $merge_base..$headrev || exit
+git diff -M --stat --summary $patch $merge_base..$headrev || status=1
+
+if test -z "$branch"
+then
+ echo "warn: No branch of $url is at:" >&2
+ git show -s --format='warn: %h: %s' $headrev >&2
+ echo "warn: Are you sure you pushed '$head' there?" >&2
+ status=1
+fi
exit $status
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 9cc0a42..5bd1682 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -193,8 +193,14 @@ test_expect_success 'pull request format' '
SUBJECT (DATE)
are available in the git repository at:
+
URL BRANCH
+ for you to fetch changes up to OBJECT_NAME:
+
+ SUBJECT (DATE)
+
+ ----------------------------------------------------------------
SHORTLOG
DIFFSTAT
--
1.7.7.rc1.3.g559357
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html