* [PATCH 1/3] add tests for merge message headings
From: Jeff King @ 2009-08-09 10:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20090809100045.GA25197@coredump.intra.peff.net>
When calling "git merge $X", we automatically generate a
commit message containing something like "Merge branch
'$X'". This test script checks that those messages say what
they should, and exposes a failure when merging a refname
that is ambiguous between a tag and a branch.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t7608-merge-messages.sh | 50 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 50 insertions(+), 0 deletions(-)
create mode 100755 t/t7608-merge-messages.sh
diff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh
new file mode 100755
index 0000000..9d10583
--- /dev/null
+++ b/t/t7608-merge-messages.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='test auto-generated merge messages'
+. ./test-lib.sh
+
+check_oneline() {
+ echo "$1" | sed "s/Q/'/g" >expect &&
+ git log -1 --pretty=tformat:%s >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success 'merge local branch' '
+ test_commit master-1 &&
+ git checkout -b local-branch &&
+ test_commit branch-1 &&
+ git checkout master &&
+ test_commit master-2 &&
+ git merge local-branch &&
+ check_oneline "Merge branch Qlocal-branchQ"
+'
+
+test_expect_success 'merge octopus branches' '
+ git checkout -b octopus-a master &&
+ test_commit octopus-1 &&
+ git checkout -b octopus-b master &&
+ test_commit octopus-2 &&
+ git checkout master &&
+ git merge octopus-a octopus-b &&
+ check_oneline "Merge branches Qoctopus-aQ and Qoctopus-bQ"
+'
+
+test_expect_success 'merge tag' '
+ git checkout -b tag-branch master &&
+ test_commit tag-1 &&
+ git checkout master &&
+ test_commit master-3 &&
+ git merge tag-1 &&
+ check_oneline "Merge commit Qtag-1Q"
+'
+
+test_expect_failure 'ambiguous tag' '
+ git checkout -b ambiguous master &&
+ test_commit ambiguous &&
+ git checkout master &&
+ test_commit master-4 &&
+ git merge ambiguous &&
+ check_oneline "Merge commit QambiguousQ"
+'
+
+test_done
--
1.6.4.178.g7a987
^ permalink raw reply related
* Re: [PATCH] merge: indicate remote tracking branches in merge message
From: Jeff King @ 2009-08-09 10:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20090809091443.GA676@coredump.intra.peff.net>
On Sun, Aug 09, 2009 at 05:14:43AM -0400, Jeff King wrote:
> In t4202, we have a branch name and a tag name that are the same
> (octopus-a), and we "git merge octopus-a". This actually merges the tag,
> but because the branch name existed, we write "Merge branch 'octopus-a'"
> in the log, which is not true. With your patch, it does the right thing
> and says "Merge commit 'octopus-a'".
>
> The simple thing is to just update the "expect" text. Though the current
> behavior does show off the ability to collape the two branches and say
>
> Merge branches 'octopus-a' and 'octopus-b'
>
> instead of
>
> Merge commit 'octopus-a'; commit 'octopus-b'
Thinking about it for a few seconds, it's silly to try to test something
that happens to occur in a totally unrelated test. The right thing to do
is to write actual tests for this area, fix the bug, and then add the
new feature. So how about this series:
[1/3] add tests for merge message headings
[2/3] merge: fix incorrect merge message for ambiguous tag/branch
[3/3] merge: indicate remote tracking branches in merge message
-Peff
^ permalink raw reply
* Re: [PATCH] Document 'stash clear' recovery via unreachable commits
From: Sitaram Chamarty @ 2009-08-09 9:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, git, Björn Steinbrink
In-Reply-To: <7veirlbwws.fsf@alter.siamese.dyndns.org>
2009/8/9 Junio C Hamano <gitster@pobox.com>
>
> Thomas Rast <trast@student.ethz.ch> writes:
>
> > Unless you have merges of branch names containing WIP, or edit your
> > merge messages to say WIP, there will be no false positives.
>
> That may be true, but I suspect that people's stash entries that are worth
> saving are given their own messages with "git stash save 'message'" and do
> not necessarily say WIP. I wish if there were a better way to identify
> them, but I do not think of any offhand.
gitk $(git fsck | grep commit | cut -f3 -d' ') --since='1 week ago'
The --since clause limits the display to stashes lost recently; adjust to taste.
A "stash" has a very recognisable, triangular, shape in the commit DAG
-- with gitk you can visually find stashes really fast even if your
time limit is too broad or you have too many other kinds of
unreachable commits perhaps due to too many rebase etc.
^ permalink raw reply
* Re: [PATCH v4 0/5] {checkout,reset,stash} --patch
From: Thomas Rast @ 2009-08-09 9:17 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Sverre Rabbelier, Nanako Shiraishi
In-Reply-To: <20090809065207.GA14093@coredump.intra.peff.net>
[-- Attachment #1: Type: Text/Plain, Size: 1833 bytes --]
Jeff King wrote:
> I finally got a few minutes to look at this. I tried "checkout --patch"
> first, which was very confusing:
>
> $ echo old content >file && git add file && git commit -m old
> $ echo new content >>file
> $ git checkout --patch file
> diff --git a/file b/file
> index 33194a0..805c3b0 100644
> --- a/file
> +++ b/file
> @@ -1 +1,2 @@
> old content
> +new content
> Check out this hunk [y,n,q,a,d,/,e,?]?
>
> Shouldn't the diff be reversed? That is, I think what users would like
> to see is "bring this hunk over from the index to the working tree". But
> we have the opposite (a hunk that is in the working tree that we would
> like to undo).
Well, my thinking for the initial (restricted; you couldn't say 'git
checkout -p HEAD~14') version went something like this: 'reset -p'
should be the opposite of 'add -p', so it offers the same hunks with
the question "Reset?". Then 'checkout -p' should somehow follow suit,
but asked "Discard?" (IIRC I even had it in all caps).
I'm not 100% happy with your suggested forward patches strategy
because I think (particularly for people with colors enabled, and I
suspect we all have), it's less confusing "my" way if they go through
'add -p' and suddenly think "oops, mistake, I need to reset that": it
is much easier for the (at least for my) eye to find the same hunk
again if it is really 100% the same. Probably we would have to change
the verb to "discard" again, though.
OTOH this does become rather weird once you specify anything other
than HEAD. And while we could of course switch the approach if the
user does that, and hope that he'll understand on the grounds that
it's an advanced usage, I'm not sure switching is a good idea in
general.
--
Thomas Rast
trast@{inf,student}.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] merge: indicate remote tracking branches in merge message
From: Jeff King @ 2009-08-09 9:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20090809074035.GA4778@coredump.intra.peff.net>
On Sun, Aug 09, 2009 at 03:40:35AM -0400, Jeff King wrote:
> > Shouldn't it instead feed what it got from the end user to the dwim
> > machinery, and make sure it dwims into refs/remotes/ hierarchy?
>
> I'm not sure that is all that different in practice than what is
> happening now. Mainly I did it the way I did so that I didn't touch the
> code path for detecting local branches.
>
> But assuming they are functionally identical, I think your patch is much
> more readable.
I tested your patch; the two methods aren't identical. In fact, yours
fixes a bug. :)
In t4202, we have a branch name and a tag name that are the same
(octopus-a), and we "git merge octopus-a". This actually merges the tag,
but because the branch name existed, we write "Merge branch 'octopus-a'"
in the log, which is not true. With your patch, it does the right thing
and says "Merge commit 'octopus-a'".
The simple thing is to just update the "expect" text. Though the current
behavior does show off the ability to collape the two branches and say
Merge branches 'octopus-a' and 'octopus-b'
instead of
Merge commit 'octopus-a'; commit 'octopus-b'
which we could preserve that by renaming the tags, as below.
---
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 48e0088..9d0db2a 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -306,13 +306,13 @@ test_expect_success 'set up more tangled history' '
git checkout master &&
git merge tangle &&
git checkout -b reach &&
- test_commit reach &&
+ test_commit reach-1 &&
git checkout master &&
git checkout -b octopus-a &&
- test_commit octopus-a &&
+ test_commit octopus-a-1 &&
git checkout master &&
git checkout -b octopus-b &&
- test_commit octopus-b &&
+ test_commit octopus-b-1 &&
git checkout master &&
test_commit seventh &&
git merge octopus-a octopus-b
@@ -327,12 +327,12 @@ cat > expect <<\EOF
*-. \ Merge branches 'octopus-a' and 'octopus-b'
|\ \ \
* | | | seventh
-| | * | octopus-b
+| | * | octopus-b-1
| |/ /
|/| |
-| * | octopus-a
+| * | octopus-a-1
|/ /
-| * reach
+| * reach-1
|/
* Merge branch 'tangle'
|\
^ permalink raw reply related
* [PATCH 4/3] t/Makefile: include config.mak
From: Jeff King @ 2009-08-09 8:50 UTC (permalink / raw)
To: git
In-Reply-To: <20090809083518.GA8147@coredump.intra.peff.net>
This is useful if you want to specify GIT_TEST_OPTS that you
always use.
Signed-off-by: Jeff King <peff@peff.net>
---
On Sun, Aug 09, 2009 at 04:35:19AM -0400, Jeff King wrote:
> [1/3] tests: use "$TEST_DIRECTORY" instead of ".."
> [2/3] tests: provide $TRASH_DIRECTORY variable
> [3/3] tests: allow user to specify trash directory location
I think a 4/3 like this is nice to let you use --root without extra
typing each time. And when you do "cd t && make t$WHATEVER.sh
GIT_TEST_OPTS='-v -i'", it will automatically be overridden, which is
probably what you want, so you can inspect the failed tests in t/trash*.
t/Makefile | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/t/Makefile b/t/Makefile
index bf816fc..bd09390 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -3,6 +3,8 @@
# Copyright (c) 2005 Junio C Hamano
#
+-include ../config.mak
+
#GIT_TEST_OPTS=--verbose --debug
SHELL_PATH ?= $(SHELL)
TAR ?= $(TAR)
--
1.6.4.178.g7a987
^ permalink raw reply related
* [PATCH 3/3] tests: allow user to specify trash directory location
From: Jeff King @ 2009-08-09 8:39 UTC (permalink / raw)
To: git
In-Reply-To: <20090809083518.GA8147@coredump.intra.peff.net>
The tests generate a large amount of I/O activity creating
and destroying repositories and files. We can improve the
time it takes to run the test suite by creating trash
directories on filesystems with better performance
characteristic, even though we may not want the rest of the
git repository on those filesystems (e.g., because they are
not network connected, or because they are temporary
ramdisks).
For example, on a dual processor system:
$ cd t && time make -j32
real 1m51.562s
user 0m59.260s
sys 1m20.933s
# /dev/shm is tmpfs
$ cd t && time make -j32 GIT_TEST_OPTS="--root=/dev/shm"
real 1m1.484s
user 0m53.555s
sys 1m5.264s
We almost halve the wall clock time, and we utilize the
dual processors much better.
Signed-off-by: Jeff King <peff@peff.net>
---
There is a test below for absolute versus relative path in the root
provided. Do we need some extra magic to make it work on non-Unix
platforms?
t/test-lib.sh | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e6a7bb1..a5b8d03 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -114,6 +114,9 @@ do
valgrind=t; verbose=t; shift ;;
--tee)
shift ;; # was handled already
+ --root=*)
+ root=$(expr "z$1" : 'z[^=]*=\(.*\)')
+ shift ;;
*)
echo "error: unknown test option '$1'" >&2; exit 1 ;;
esac
@@ -645,7 +648,11 @@ fi
# Test repository
test="trash directory.$(basename "$0" .sh)"
-TRASH_DIRECTORY="$TEST_DIRECTORY/$test"
+test -n "$root" && test="$root/$test"
+case "$test" in
+/*) TRASH_DIRECTORY="$test" ;;
+ *) TRASH_DIRECTORY="$TEST_DIRECTORY/$test" ;;
+esac
test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY
rm -fr "$test" || {
GIT_EXIT_OK=t
--
1.6.4.178.g7a987
^ permalink raw reply related
* [PATCH 2/3] tests: provide $TRASH_DIRECTORY variable
From: Jeff King @ 2009-08-09 8:38 UTC (permalink / raw)
To: git
In-Reply-To: <20090809083518.GA8147@coredump.intra.peff.net>
Most scripts don't care about the absolute path to the trash
directory. The one exception was t4014 script, which pieced
together $TEST_DIRECTORY and $test itself to get an absolute
directory.
Instead, let's provide a $TRASH_DIRECTORY which specifies
the same thing. This keeps the $test variable internal to
test-lib.sh and paves the way for trash directories in other
locations.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t4014-format-patch.sh | 3 +--
t/test-lib.sh | 3 ++-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 922a894..531f5b7 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -493,13 +493,12 @@ test_expect_success 'format-patch from a subdirectory (2)' '
'
test_expect_success 'format-patch from a subdirectory (3)' '
- here="$TEST_DIRECTORY/$test" &&
rm -f 0* &&
filename=$(
rm -rf sub &&
mkdir -p sub/dir &&
cd sub/dir &&
- git format-patch -1 -o "$here"
+ git format-patch -1 -o "$TRASH_DIRECTORY"
) &&
basename=$(expr "$filename" : ".*/\(.*\)") &&
test -f "$basename"
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 5fdc5d9..e6a7bb1 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -645,7 +645,8 @@ fi
# Test repository
test="trash directory.$(basename "$0" .sh)"
-test ! -z "$debug" || remove_trash="$TEST_DIRECTORY/$test"
+TRASH_DIRECTORY="$TEST_DIRECTORY/$test"
+test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY
rm -fr "$test" || {
GIT_EXIT_OK=t
echo >&5 "FATAL: Cannot prepare test area"
--
1.6.4.178.g7a987
^ permalink raw reply related
* [PATCH 1/3] tests: use "$TEST_DIRECTORY" instead of ".."
From: Jeff King @ 2009-08-09 8:37 UTC (permalink / raw)
To: git
In-Reply-To: <20090809083518.GA8147@coredump.intra.peff.net>
The $TEST_DIRECTORY variable allows tests to find the
top-level test directory regardless of the current working
directory.
In the past, this has been used to accomodate tests which
change directories, but it is also the first step to being
able to move trash directories outside of the
$TEST_DIRECTORY hierarchy.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t3404-rebase-interactive.sh | 2 +-
t/t3411-rebase-preserve-around-merges.sh | 2 +-
t/t3414-rebase-preserve-onto.sh | 2 +-
t/t4020-diff-external.sh | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index a973628..4cae019 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -10,7 +10,7 @@ that the result still makes sense.
'
. ./test-lib.sh
-. ../lib-rebase.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
set_fake_editor
diff --git a/t/t3411-rebase-preserve-around-merges.sh b/t/t3411-rebase-preserve-around-merges.sh
index 6533505..14a23cd 100755
--- a/t/t3411-rebase-preserve-around-merges.sh
+++ b/t/t3411-rebase-preserve-around-merges.sh
@@ -10,7 +10,7 @@ a merge to before the merge.
'
. ./test-lib.sh
-. ../lib-rebase.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
set_fake_editor
diff --git a/t/t3414-rebase-preserve-onto.sh b/t/t3414-rebase-preserve-onto.sh
index 80019ee..ee0a6cc 100755
--- a/t/t3414-rebase-preserve-onto.sh
+++ b/t/t3414-rebase-preserve-onto.sh
@@ -10,7 +10,7 @@ aren'"'"'t on top of $ONTO, even if they are on top of $UPSTREAM.
'
. ./test-lib.sh
-. ../lib-rebase.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
# Set up branches like this:
# A1---B1---E1---F1---G1
diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index 4ea42e0..a7602cf 100755
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -166,7 +166,7 @@ test_expect_success 'diff --cached' '
git update-index --assume-unchanged file &&
echo second >file &&
git diff --cached >actual &&
- test_cmp ../t4020/diff.NUL actual
+ test_cmp "$TEST_DIRECTORY"/t4020/diff.NUL actual
'
test_done
--
1.6.4.178.g7a987
^ permalink raw reply related
* [PATCH 0/3] moving test trash directories
From: Jeff King @ 2009-08-09 8:35 UTC (permalink / raw)
To: git
This series adds support for making trash directories for the test
scripts outside of the usual hierarchy. The main benefit is improved
performance. By putting the trash directories onto a tmpfs (rather than
SATA disks), my "make -j32 test" time went from around 1m51s to around
1m01s. I can only imagine what improvement it would have for people on
NFS. :)
It also has the nice side effect that running the test suite doesn't
thrash your disk as much, in case you wanted to do something else while
it was running.
[1/3] tests: use "$TEST_DIRECTORY" instead of ".."
[2/3] tests: provide $TRASH_DIRECTORY variable
[3/3] tests: allow user to specify trash directory location
-Peff
^ permalink raw reply
* Re: git gc expanding packed data?
From: Andreas Schwab @ 2009-08-09 7:43 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Hin-Tak Leung, git
In-Reply-To: <alpine.LFD.2.00.0908082246020.440@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
> If you do a clone using the git:// protocol and the server sends you
> only the ref for the trunk branch,
A clone will fetch all branches from refs/heads/*.
> then it should send you only objects reachable from that branch.
Apparantly this does not work. I'd guess the extra objects are needed
due to the delta compression.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH] merge: indicate remote tracking branches in merge message
From: Jeff King @ 2009-08-09 7:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vab29a1fr.fsf@alter.siamese.dyndns.org>
On Sun, Aug 09, 2009 at 12:31:04AM -0700, Junio C Hamano wrote:
> I somewhat suspect that the patch was not applied because it also lacked
> necessary adjustments to tests. With this patch, I think the tests would
> fail.
Yeah, see my follow-up patch.
> Nevertheless, I think it is a good thing to do. But I am unsure about the
> implementation.
>
> Shouldn't it instead feed what it got from the end user to the dwim
> machinery, and make sure it dwims into refs/remotes/ hierarchy?
I'm not sure that is all that different in practice than what is
happening now. Mainly I did it the way I did so that I didn't touch the
code path for detecting local branches.
But assuming they are functionally identical, I think your patch is much
more readable.
> In other words, like this. Note that it would be much clearer to see
> what's needed, if you want to extend it to refs/tags hierarchy ;-)
I'm not sure adding "tag foo" will actually work, as it still has to
make it through the bit where we parse FETCH_HEAD. I'm not sure if it
would get mutilated to "commit foo" by that code or not.
-Peff
^ permalink raw reply
* Re: [PATCH] fix potential infinite loop given large unsigned integer
From: Junio C Hamano @ 2009-08-09 7:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ryan Flynn, git
In-Reply-To: <7vy6pta4rd.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Ryan Flynn <parseerror@gmail.com> writes:
>
>> given n, tried to find i greater than n via i=1, iterate i *= 10.
>> given n sufficiently close to UINT_MAX this will overflow; which can
>> produce i==0, which results in an infinite loop. iteratively dividing
>> n /= 10 does not have this problem, and though division is slower than
>> multiplication this only runs once per `git format-patch
>> --cover-letter`
>>
>> Signed-off-by: pizza <parseerror@gmail.com>
>
> Pizza?
>
> This is somewhat amusing.
>
> - digits_in_number() is called only with opt->total that is "int";
>
> - opt->total is the total number of patches.
>
> - the return value is used like this:
>
> sprintf(buf, "%0*d", digits_in_number(opt->total), opt->nr);
>
> and opt->nr runs from 1 to opt->total; the use of "d" would be already
> wrong anyway even if you computed digits_in_number() correctly.
>
> Perhaps we should get rid of this function altogether?
Or perhaps something stupid like this...
builtin-log.c | 6 +++++-
log-tree.c | 12 +-----------
revision.h | 2 +-
3 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 3817bf1..321e8f5 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1096,8 +1096,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
total = nr;
if (!keep_subject && auto_number && total > 1)
numbered = 1;
- if (numbered)
+ if (numbered) {
+ static char num_buf[64];
rev.total = total + start_number - 1;
+ sprintf(num_buf, "%d", rev.total);
+ rev.num_width = strlen(num_buf);
+ }
if (in_reply_to || thread || cover_letter)
rev.ref_message_ids = xcalloc(1, sizeof(struct string_list));
if (in_reply_to) {
diff --git a/log-tree.c b/log-tree.c
index 6f73c17..0d32a6c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -158,16 +158,6 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
strbuf_addch(sb, '\n');
}
-static unsigned int digits_in_number(unsigned int number)
-{
- unsigned int i = 10, result = 1;
- while (i <= number) {
- i *= 10;
- result++;
- }
- return result;
-}
-
static int has_non_ascii(const char *s)
{
int ch;
@@ -212,7 +202,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
snprintf(buffer, sizeof(buffer),
"Subject: [%s %0*d/%d] ",
opt->subject_prefix,
- digits_in_number(opt->total),
+ opt->num_width,
opt->nr, opt->total);
subject = buffer;
} else if (opt->total == 0 && opt->subject_prefix && *opt->subject_prefix) {
diff --git a/revision.h b/revision.h
index fb74492..21e4d9d 100644
--- a/revision.h
+++ b/revision.h
@@ -84,7 +84,7 @@ struct rev_info {
unsigned int abbrev;
enum cmit_fmt commit_format;
struct log_info *loginfo;
- int nr, total;
+ int nr, total, num_width;
const char *mime_boundary;
const char *patch_suffix;
int numbered_files;
^ permalink raw reply related
* Re: [PATCH] merge: indicate remote tracking branches in merge message
From: Junio C Hamano @ 2009-08-09 7:31 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20090809065936.GA24112@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Previously when merging directly from a local tracking
> branch like:
>
> git merge origin/master
>
> The merge message said:
>
> Merge commit 'origin/master'
>
> * commit 'origin/master':
> ...
>
> Instead, let's be more explicit about what we are merging:
>
> Merge remote branch 'origin/master'
>
> * origin/master:
> ...
>
> We accomplish this by recognizing remote tracking branches
> in git-merge when we build the simulated FETCH_HEAD output
> that we feed to fmt-merge-msg.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This is a repost of
>
> http://article.gmane.org/gmane.comp.version-control.git/119909
>
> which got no response from you. I think it is a good idea, but I am not
> deeply committed to it. I mainly want a yes or no so I can clean it out
> of my patch queue.
I somewhat suspect that the patch was not applied because it also lacked
necessary adjustments to tests. With this patch, I think the tests would
fail.
Nevertheless, I think it is a good thing to do. But I am unsure about the
implementation.
Shouldn't it instead feed what it got from the end user to the dwim
machinery, and make sure it dwims into refs/remotes/ hierarchy?
In other words, like this. Note that it would be much clearer to see
what's needed, if you want to extend it to refs/tags hierarchy ;-)
builtin-merge.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index 82b5466..f4de73f 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -358,6 +358,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
struct strbuf buf = STRBUF_INIT;
struct strbuf bname = STRBUF_INIT;
const char *ptr;
+ char *found_ref;
int len, early;
strbuf_branchname(&bname, remote);
@@ -368,14 +369,17 @@ static void merge_name(const char *remote, struct strbuf *msg)
if (!remote_head)
die("'%s' does not point to a commit", remote);
- strbuf_addstr(&buf, "refs/heads/");
- strbuf_addstr(&buf, remote);
- resolve_ref(buf.buf, branch_head, 0, NULL);
-
- if (!hashcmp(remote_head->sha1, branch_head)) {
- strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
- sha1_to_hex(branch_head), remote);
- goto cleanup;
+ if (dwim_ref(remote, strlen(remote), branch_head, &found_ref) > 0) {
+ if (!prefixcmp(found_ref, "refs/heads/")) {
+ strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
+ sha1_to_hex(branch_head), remote);
+ goto cleanup;
+ }
+ if (!prefixcmp(found_ref, "refs/remotes/")) {
+ strbuf_addf(msg, "%s\t\tremote branch '%s' of .\n",
+ sha1_to_hex(branch_head), remote);
+ goto cleanup;
+ }
}
/* See if remote matches <name>^^^.. or <name>~<number> */
^ permalink raw reply related
* Re: [PATCH] merge: indicate remote tracking branches in merge message
From: Jeff King @ 2009-08-09 7:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20090809065936.GA24112@coredump.intra.peff.net>
On Sun, Aug 09, 2009 at 02:59:36AM -0400, Jeff King wrote:
> This is a repost of
>
> http://article.gmane.org/gmane.comp.version-control.git/119909
>
> which got no response from you. I think it is a good idea, but I am not
> deeply committed to it. I mainly want a yes or no so I can clean it out
> of my patch queue.
Ah, it may have had to do with the fact that it doesn't actually pass
the tests. There is a trivial text update needed in t3409, which calls
"git merge origin/master" during its setup phase.
Updated, passing-all-tests patch is below. Sorry for the confusion.
-- >8 --
Subject: [PATCH] merge: indicate remote tracking branches in merge message
Previously when merging directly from a local tracking
branch like:
git merge origin/master
The merge message said:
Merge commit 'origin/master'
* commit 'origin/master':
...
Instead, let's be more explicit about what we are merging:
Merge remote branch 'origin/master'
* origin/master:
...
We accomplish this by recognizing remote tracking branches
in git-merge when we build the simulated FETCH_HEAD output
that we feed to fmt-merge-msg.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-merge.c | 11 +++++++++++
t/t3409-rebase-preserve-merges.sh | 2 +-
2 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index 82b5466..1f59567 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -378,6 +378,17 @@ static void merge_name(const char *remote, struct strbuf *msg)
goto cleanup;
}
+ strbuf_setlen(&buf, 0);
+ strbuf_addstr(&buf, "refs/remotes/");
+ strbuf_addstr(&buf, remote);
+ resolve_ref(buf.buf, branch_head, 0, 0);
+
+ if (!hashcmp(remote_head->sha1, branch_head)) {
+ strbuf_addf(msg, "%s\t\tremote branch '%s' of .\n",
+ sha1_to_hex(branch_head), remote);
+ goto cleanup;
+ }
+
/* See if remote matches <name>^^^.. or <name>~<number> */
for (len = 0, ptr = remote + strlen(remote);
remote < ptr && ptr[-1] == '^';
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index e6c8327..2ce0236 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -71,7 +71,7 @@ test_expect_success 'rebase -p fakes interactive rebase' '
git fetch &&
git rebase -p origin/topic &&
test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
- test 1 = $(git rev-list --all --pretty=oneline | grep "Merge commit" | wc -l)
+ test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote branch" | wc -l)
)
'
--
1.6.4.178.g7a987
^ permalink raw reply related
* [PATCH] merge: indicate remote tracking branches in merge message
From: Jeff King @ 2009-08-09 6:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Previously when merging directly from a local tracking
branch like:
git merge origin/master
The merge message said:
Merge commit 'origin/master'
* commit 'origin/master':
...
Instead, let's be more explicit about what we are merging:
Merge remote branch 'origin/master'
* origin/master:
...
We accomplish this by recognizing remote tracking branches
in git-merge when we build the simulated FETCH_HEAD output
that we feed to fmt-merge-msg.
Signed-off-by: Jeff King <peff@peff.net>
---
This is a repost of
http://article.gmane.org/gmane.comp.version-control.git/119909
which got no response from you. I think it is a good idea, but I am not
deeply committed to it. I mainly want a yes or no so I can clean it out
of my patch queue.
builtin-merge.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index 0b12fb3..c5688e1 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -378,6 +378,17 @@ static void merge_name(const char *remote, struct strbuf *msg)
goto cleanup;
}
+ strbuf_setlen(&buf, 0);
+ strbuf_addstr(&buf, "refs/remotes/");
+ strbuf_addstr(&buf, remote);
+ resolve_ref(buf.buf, branch_head, 0, 0);
+
+ if (!hashcmp(remote_head->sha1, branch_head)) {
+ strbuf_addf(msg, "%s\t\tremote branch '%s' of .\n",
+ sha1_to_hex(branch_head), remote);
+ goto cleanup;
+ }
+
/* See if remote matches <name>^^^.. or <name>~<number> */
for (len = 0, ptr = remote + strlen(remote);
remote < ptr && ptr[-1] == '^';
--
1.6.4.72.g50730
^ permalink raw reply related
* Re: [PATCH v4 0/5] {checkout,reset,stash} --patch
From: Jeff King @ 2009-08-09 6:52 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Junio C Hamano, Sverre Rabbelier, Nanako Shiraishi
In-Reply-To: <cover.1248815580.git.trast@student.ethz.ch>
On Tue, Jul 28, 2009 at 11:20:06PM +0200, Thomas Rast wrote:
> Thomas Rast (6):
> git-apply--interactive: Refactor patch mode code
> builtin-add: refactor the meat of interactive_add()
> Implement 'git reset --patch'
> Implement 'git checkout --patch'
> Implement 'git stash save --patch'
> DWIM 'git stash save -p' for 'git stash -p'
I finally got a few minutes to look at this. I tried "checkout --patch"
first, which was very confusing:
$ echo old content >file && git add file && git commit -m old
$ echo new content >>file
$ git checkout --patch file
diff --git a/file b/file
index 33194a0..805c3b0 100644
--- a/file
+++ b/file
@@ -1 +1,2 @@
old content
+new content
Check out this hunk [y,n,q,a,d,/,e,?]?
Shouldn't the diff be reversed? That is, I think what users would like
to see is "bring this hunk over from the index to the working tree". But
we have the opposite (a hunk that is in the working tree that we would
like to undo).
Maybe it is just the use of the verb "check out". I think of it as
bringing _this_ hunk out. But it is really about erasing this hunk. "git
reset --patch" has the same behavior, but I don't find it as bothersome,
since it says "Reset this hunk".
And "stash --patch" doesn't have the issue because it goes from the
working tree to the stash already.
-Peff
^ permalink raw reply
* [ANN] git# (GitSharp) project status
From: Meinrad Recheis @ 2009-08-09 6:41 UTC (permalink / raw)
To: git
Hi,
Thanks to massive contributions from various people the project
GitSharp has progressed very fast.
GitSharp now counts about 17753 lines of code, the jgit.lib snapshot
we are porting from counts 24588 lines. This means that roughly 72% of
the porting work are done! About two months ago, when I announced the
project on this list only about 25% had been done.
There is also still much testing work to do (426 tests of approx. 1000
tests are ported) and a lot of bugs to fix (28 tests are failing) till
all the ported code may be considered as correct.
I want to say thanks to the git community, to the jgit folks and to
all who contributed to GitSharp!
For more info visit http://www.eqqon.com/index.php/GitSharp
Cheers,
-- henon
^ permalink raw reply
* Re: [PATCH] fix potential infinite loop given large unsigned integer
From: Junio C Hamano @ 2009-08-09 6:19 UTC (permalink / raw)
To: Ryan Flynn; +Cc: git
In-Reply-To: <a3f15ee60908082141l7b2134cg5ddcef17c45fc888@mail.gmail.com>
Ryan Flynn <parseerror@gmail.com> writes:
> given n, tried to find i greater than n via i=1, iterate i *= 10.
> given n sufficiently close to UINT_MAX this will overflow; which can
> produce i==0, which results in an infinite loop. iteratively dividing
> n /= 10 does not have this problem, and though division is slower than
> multiplication this only runs once per `git format-patch
> --cover-letter`
>
> Signed-off-by: pizza <parseerror@gmail.com>
Pizza?
This is somewhat amusing.
- digits_in_number() is called only with opt->total that is "int";
- opt->total is the total number of patches.
- the return value is used like this:
sprintf(buf, "%0*d", digits_in_number(opt->total), opt->nr);
and opt->nr runs from 1 to opt->total; the use of "d" would be already
wrong anyway even if you computed digits_in_number() correctly.
Perhaps we should get rid of this function altogether?
^ permalink raw reply
* [PATCH] fix potential infinite loop given large unsigned integer
From: Ryan Flynn @ 2009-08-09 4:41 UTC (permalink / raw)
To: git
given n, tried to find i greater than n via i=1, iterate i *= 10.
given n sufficiently close to UINT_MAX this will overflow; which can
produce i==0, which results in an infinite loop. iteratively dividing
n /= 10 does not have this problem, and though division is slower than
multiplication this only runs once per `git format-patch
--cover-letter`
Signed-off-by: pizza <parseerror@gmail.com>
---
log-tree.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 6f73c17..53a1bd2 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -160,9 +160,9 @@ static void append_signoff(struct strbuf *sb,
const char *signoff)
static unsigned int digits_in_number(unsigned int number)
{
- unsigned int i = 10, result = 1;
- while (i <= number) {
- i *= 10;
+ unsigned int result = 1;
+ while (number > 9) {
+ number /= 10;
result++;
}
return result;
--
1.6.0.4
^ permalink raw reply related
* Re: git gc expanding packed data?
From: Nicolas Pitre @ 2009-08-09 2:56 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Hin-Tak Leung, git
In-Reply-To: <m2tz0j154o.fsf@igel.home>
On Sat, 8 Aug 2009, Andreas Schwab wrote:
> Nicolas Pitre <nico@cam.org> writes:
>
> > It appears that the git installation serving clone requests for
> > git://gcc.gnu.org/git/gcc.git generates lots of unreferenced objects. I
> > just cloned it and the pack I was sent contains 1383356 objects (can be
> > determined with 'git show-index < .git/objects/pack/*.idx | wc -l').
> > However, there are only 978501 actually referenced objects in that
> > cloned repository ( 'git rev-list --all --objects | wc -l'). That makes
> > for 404855 useless objects in the cloned repository.
>
> Those objects are not useless. They are referenced by the remote refs
> on the remote side, which are not fetched by default. If you clone a
> mirror of the repository you'll see no unreferenced objects.
If you do a clone using the git:// protocol and the server sends you
only the ref for the trunk branch, then it should send you only objects
reachable from that branch. Any extra objects sent by the server are
useless to me and wastes my and everyone else's bandwidth, and on my
next repack those objects are pruned anyway. The point of the git
protocol is _not_ necessarily to send a copy of the remote pack file
over, even during a clone.
Nicolas
^ permalink raw reply
* Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking
From: Nicolas Pitre @ 2009-08-09 2:37 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Sam Vilain, Nick Edelen, Michael J Gruber, Junio C Hamano,
Jeff King, Shawn O. Pearce, Andreas Ericsson, Christian Couder,
git@vger.kernel.org
In-Reply-To: <alpine.DEB.1.00.0908081709380.8306@pacific.mpi-cbg.de>
On Sat, 8 Aug 2009, Johannes Schindelin wrote:
> Hi,
>
> On Fri, 7 Aug 2009, Nicolas Pitre wrote:
>
> > On Fri, 7 Aug 2009, Johannes Schindelin wrote:
> >
> > > Hi,
> > >
> > > On Fri, 7 Aug 2009, Nicolas Pitre wrote:
> > >
> > > > On Fri, 7 Aug 2009, Sam Vilain wrote:
> > > >
> > > > > Johannes Schindelin wrote:
> > > > > >> the short answer is that cache slices are totally independant of
> > > > > >> pack files.
> > > > > >>
> > > > > >
> > > > > > My idea with that was that you already have a SHA-1 map in the pack
> > > > > > index, and if all you want to be able to accelerate the revision
> > > > > > walker, you'd probably need something that adds yet another mapping,
> > > > > > from commit to parents and tree, and from tree to sub-tree and blob
> > > > > > (so you can avoid unpacking commit and tree objects).
> > > > > >
> > > > >
> > > > > Tying indexes together like that is not a good idea in the database
> > > > > world. Especially as in this case as Nick mentions, the domain is
> > > > > subtly different (ie pack vs dag). Unfortunately you just can't try to
> > > > > pretend that they will always be the same; you can't force a full
> > > > > repack on every ref change!
> > > >
> > > > Right. And the rev cache must work even if the repository is not
> > > > packed.
> > >
> > > Umm, why? AFAICT the principal purpose of the rev cache is to help work
> > > loads on, say, www.kernel.org.
> >
> > So what?
> >
> > Speeding up rev-list with a rev cache is completely orthogonal to
> > whether the repository is packed or not.
>
> No, it is not.
>
> For both technical and practical reasons, caching revision walker data is
> very closely related to packing.
No it is not.
> You are _very_ unlikely helped by speeding up revision walking in the
> general case, _especially_ when you do stuff like blame or -S that needs
> to unpack tons of objects _anyway_.
I completely agree. And this is why I wish for the rev cache to be
enabled with a config variable. Why? Because "client" repositories are
unlikely to benefit as much as "server" repositories from this cache.
> The one big kicker argument for speeding up revision walking _is_ to
> relieve the loads on big ass servers, and they _should_ be as packed as
> possible (as I will patiently explain over and over again).
Please do a shortlog on builtin-pack-objects.c and realize who spent a
lot of energy making git repository packing what it is now.
Then re-read what I said on this list for more than a year now about the
remaining latency on large git clone operations. After explaining it a
couple times already I will need some of your patience to make people
understand that fully packing your repo is _not_ going to help it as
we're always talking about fully packed repos to start with.
> > It is like having a "git diff" result cache: no one would think of
> > stuffing that in the pack index.
>
> Do you want to try to kid me? You'll have to try harder. Caching "git
> diff" results... no, really!
Why not? You worked on git diff yourself, so you certainly are well
positionned to appreciate my suggestion, no?
> > If we want to improve on the repository packing format, that must be
> > doable without bothering with an independent concept such as a rev
> > cache.
>
> I would love to tell you that you're right, but the single fact that
> pack v4 is startig to compete with Duke Nukem Forever just prevents me
> from doing that.
Because of the current economy, I was waiting to be laid off so to have
the time to make pack v4 a reality. Unfortunately for git they didn't
fire me yet. Oh well...
> > > I am unlikely to notice the improvements in my regular "git log" calls
> > > that only show a couple of pages before I quit the pager.
> >
> > Indeed. But what is your point again?
>
> Oh? My point? Being that the rev cache has a certain target audience,
> and that the regular user is not part of that audience, and that it just
> so happens that the _technical_ similarities with the pack index can be
> exploited in those scenarios?
I don't see a similarity with the pack index at all, certainly not a
technical one.
> IOW we can be pretty certain that a heavy-load server has a fully (or
> next-to-fully) packed object database. The pack indices already contain a
> SHA-1 table that we can simply reuse. And it should not be hard (or
> fragile) at all to put the "cached" information about parents,
> referenced tree and blob objects into that file, into a different section.
And then someone does a few pushes. So most of the time your repository
contains a few packs and not only a single one. So in which pack index
files should you put the rev cache? What do yo do with that cache if it
happens to be split across multiple pack index files when a repack is
performed? Can't you see all the disadvantages to tie rev cache data
which happens to share no issue with repacking into the same file?
So what do you do to keep the code simple and maintainable? Yes, you
abstract things and use separate files, thank you very much. After all,
the packed refs are not stored in the pack index file either even if the
packed-refs file contains a list of SHA1's.
Nicolas
^ permalink raw reply
* Re: [PATCH] Document 'stash clear' recovery via unreachable commits
From: Björn Steinbrink @ 2009-08-09 2:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, git
In-Reply-To: <7veirlbwws.fsf@alter.siamese.dyndns.org>
On 2009.08.08 18:25:55 -0700, Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
>
> > Add an example to the stash documentation that shows how to quickly
> > find candidate commits among the 'git fsck --unreachable' output.
>
> Thanks.
>
> > Unless you have merges of branch names containing WIP, or edit your
> > merge messages to say WIP, there will be no false positives.
>
> That may be true, but I suspect that people's stash entries that are worth
> saving are given their own messages with "git stash save 'message'" and do
> not necessarily say WIP. I wish if there were a better way to identify
> them, but I do not think of any offhand.
Here's what I have now:
git fsck --unreachable | grep commit | cut -d\ -f3 |
xargs git log --no-walk --merges \
--grep='^\(WIP on \|On \)\((no branch)\|[^ ]\+\):'
Should catch stashes with and without user-supplied messages and avoid
most false-positives. Drop the "WIP on " option from the grep to capture
just those that have a user-supplied message.
To try really really hard to get stash stuff only, one could add some
extra stuff to verify that the second parent's commit message is
"index on \2:" (\2 being the second group from the above --grep). But
that's probably total overkill...
Björn
^ permalink raw reply
* Re: [PATCH] Document 'stash clear' recovery via unreachable commits
From: Junio C Hamano @ 2009-08-09 1:25 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Björn Steinbrink
In-Reply-To: <9e8ddf7c6c4d4df4150bff0467b461bfff92a401.1249778711.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Add an example to the stash documentation that shows how to quickly
> find candidate commits among the 'git fsck --unreachable' output.
Thanks.
> Unless you have merges of branch names containing WIP, or edit your
> merge messages to say WIP, there will be no false positives.
That may be true, but I suspect that people's stash entries that are worth
saving are given their own messages with "git stash save 'message'" and do
not necessarily say WIP. I wish if there were a better way to identify
them, but I do not think of any offhand.
^ permalink raw reply
* [PATCH] Document 'stash clear' recovery via unreachable commits
From: Thomas Rast @ 2009-08-09 0:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Björn Steinbrink
Add an example to the stash documentation that shows how to quickly
find candidate commits among the 'git fsck --unreachable' output.
Unless you have merges of branch names containing WIP, or edit your
merge messages to say WIP, there will be no false positives.
Snippet written by Björn "doener" Steinbrink and me after zepolen_
asked on IRC.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Documentation/git-stash.txt | 17 ++++++++++++++++-
1 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 1c64a02..2f5ca7b 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -114,7 +114,8 @@ no conflicts.
clear::
Remove all the stashed states. Note that those states will then
- be subject to pruning, and may be difficult or impossible to recover.
+ be subject to pruning, and may be impossible to recover (see
+ 'Examples' below for a possible strategy).
drop [-q|--quiet] [<stash>]::
@@ -217,6 +218,20 @@ $ edit/build/test remaining parts
$ git commit foo -m 'Remaining parts'
----------------------------------------------------------------
+Recovering stashes that were cleared/dropped erroneously::
+
+If you mistakenly drop or clear stashes, they cannot be recovered
+through the normal safety mechanisms. However, you can try the
+following incantation to get a list of stashes that are still in your
+repository, but not reachable any more:
++
+----------------------------------------------------------------
+git fsck --unreachable |
+grep commit | cut -d\ -f3 |
+xargs git log --merges --no-walk --grep=WIP
+----------------------------------------------------------------
+
+
SEE ALSO
--------
linkgit:git-checkout[1],
--
1.6.4.195.g3a1c2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox