* [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
* [PATCH 2/3] merge: fix incorrect merge message for ambiguous tag/branch
From: Jeff King @ 2009-08-09 10:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20090809100045.GA25197@coredump.intra.peff.net>
If we have both a tag and a branch named "foo", then calling
"git merge foo" will warn about the ambiguous ref, but merge
the tag.
When generating the commit message, though, we simply
checked whether "refs/heads/foo" existed, and if it did,
assumed it was a branch. This led to the statement "Merge
branch 'foo'" in the commit message, which is quite wrong.
Instead, we should use dwim_ref to find the actual ref used,
and describe it appropriately.
In addition to the test in t7608, we must also tweak the
expected output of t4202, which was accidentally triggering
this bug.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-merge.c | 15 +++++++--------
t/t4202-log.sh | 4 ++--
t/t7608-merge-messages.sh | 2 +-
3 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index 82b5466..f7db148 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,12 @@ 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;
+ }
}
/* See if remote matches <name>^^^.. or <name>~<number> */
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 48e0088..1e952ca 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -320,11 +320,11 @@ test_expect_success 'set up more tangled history' '
'
cat > expect <<\EOF
-* Merge branch 'reach'
+* Merge commit 'reach'
|\
| \
| \
-*-. \ Merge branches 'octopus-a' and 'octopus-b'
+*-. \ Merge commit 'octopus-a'; commit 'octopus-b'
|\ \ \
* | | | seventh
| | * | octopus-b
diff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh
index 9d10583..81ced8a 100755
--- a/t/t7608-merge-messages.sh
+++ b/t/t7608-merge-messages.sh
@@ -38,7 +38,7 @@ test_expect_success 'merge tag' '
check_oneline "Merge commit Qtag-1Q"
'
-test_expect_failure 'ambiguous tag' '
+test_expect_success 'ambiguous tag' '
git checkout -b ambiguous master &&
test_commit ambiguous &&
git checkout master &&
--
1.6.4.178.g7a987
^ permalink raw reply related
* [PATCH 3/3] merge: indicate remote tracking branches in merge message
From: Jeff King @ 2009-08-09 10:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20090809100045.GA25197@coredump.intra.peff.net>
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.
In addition to a new test in t7608, we have to tweak the
expected output of t3409, which does such a merge.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-merge.c | 5 +++++
t/t3409-rebase-preserve-merges.sh | 2 +-
t/t7608-merge-messages.sh | 10 ++++++++++
3 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index f7db148..f4de73f 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -375,6 +375,11 @@ static void merge_name(const char *remote, struct strbuf *msg)
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> */
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index e6c8327..297d165 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)
)
'
diff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh
index 81ced8a..28d5679 100755
--- a/t/t7608-merge-messages.sh
+++ b/t/t7608-merge-messages.sh
@@ -47,4 +47,14 @@ test_expect_success 'ambiguous tag' '
check_oneline "Merge commit QambiguousQ"
'
+test_expect_success 'remote branch' '
+ git checkout -b remote master &&
+ test_commit remote-1 &&
+ git update-ref refs/remotes/origin/master remote &&
+ git checkout master &&
+ test_commit master-5 &&
+ git merge origin/master &&
+ check_oneline "Merge remote branch Qorigin/masterQ"
+'
+
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:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20090809100045.GA25197@coredump.intra.peff.net>
On Sun, Aug 09, 2009 at 06:00:45AM -0400, Jeff King wrote:
> [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
And here is the 4/3 you mentioned earlier:
-- >8 --
Subject: [PATCH] merge: describe tags as such in merge message
Previously, merging a tag directly via "git merge tag" would
get you the message "Merge commit 'tag'". It is a little
more descriptive to note that it was actually a tag (i.e.,
"Merge tag 'tag'").
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-merge.c | 5 +++++
t/t7608-merge-messages.sh | 4 ++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index f4de73f..db74901 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -380,6 +380,11 @@ static void merge_name(const char *remote, struct strbuf *msg)
sha1_to_hex(branch_head), remote);
goto cleanup;
}
+ if (!prefixcmp(found_ref, "refs/tags/")) {
+ strbuf_addf(msg, "%s\t\ttag '%s' of .\n",
+ sha1_to_hex(branch_head), remote);
+ goto cleanup;
+ }
}
/* See if remote matches <name>^^^.. or <name>~<number> */
diff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh
index 28d5679..3ee0983 100755
--- a/t/t7608-merge-messages.sh
+++ b/t/t7608-merge-messages.sh
@@ -35,7 +35,7 @@ test_expect_success 'merge tag' '
git checkout master &&
test_commit master-3 &&
git merge tag-1 &&
- check_oneline "Merge commit Qtag-1Q"
+ check_oneline "Merge tag Qtag-1Q"
'
test_expect_success 'ambiguous tag' '
@@ -44,7 +44,7 @@ test_expect_success 'ambiguous tag' '
git checkout master &&
test_commit master-4 &&
git merge ambiguous &&
- check_oneline "Merge commit QambiguousQ"
+ check_oneline "Merge tag QambiguousQ"
'
test_expect_success 'remote branch' '
--
1.6.4.178.g7a987
^ permalink raw reply related
* Re: [PATCH] fix potential infinite loop given large unsigned integer
From: Erik Faye-Lund @ 2009-08-09 12:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ryan Flynn, git
In-Reply-To: <7v3a81a13z.fsf@alter.siamese.dyndns.org>
On Sun, Aug 9, 2009 at 9:38 AM, Junio C Hamano<gitster@pobox.com> wrote:
> + static char num_buf[64];
> rev.total = total + start_number - 1;
> + sprintf(num_buf, "%d", rev.total);
> + rev.num_width = strlen(num_buf);
how about
rev.num_width = (int)log10((double)rev.total) + 1;
hm?
log10() appears to be C99, but can be emulated on earlier C-versions by doing
#define log10(x) (log(x) / log(10.0))
--
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656
^ permalink raw reply
* Re: [PATCH 3/3] tests: allow user to specify trash directory location
From: Johannes Sixt @ 2009-08-09 13:33 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20090809083945.GC8250@coredump.intra.peff.net>
On Sonntag, 9. August 2009, Jeff King wrote:
> 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?
Not for Windows. Those devolpers who want to use an absolute path can use the
POSIX path notation:
GIT_TEST_OPTS=--root=/c/temp/gittests
This works in t4014.
-- Hannes
^ permalink raw reply
* Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking
From: Nick Edelen @ 2009-08-09 13:42 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Johannes Schindelin, Sam Vilain, Michael J Gruber, Junio C Hamano,
Jeff King, Shawn O. Pearce, Andreas Ericsson, Christian Couder,
git@vger.kernel.org
In-Reply-To: <alpine.LFD.2.00.0908082147480.32635@xanadu.home>
I can see the logic behind Johannes's ideas, but I'm still not sure
it'd be a great modification. If you wanted to associate revision
caching much more strongly with packs, then packs and slices could be
merged reasonably well. Say you just attached the actual slice data
at the end of the pack, then stored offsets of the slice payload in
the pack index. Since you'd (presumably) have to search the index for
the object anyway, you wouldn't have to deal with searching a
rev-cache index on top of that (although it's not exactly unoptimized
now).
However, that would sorta be preemptively limiting rev-cache to
pack-related optimizations. I mean at the moment that's the main
target, but it could be improved in the future to be more relavant to
other operations as well. Leaving the rev-cache as a seperate system
would keep both it and packing much more flexible, and open to
longer-term developments.
>I haven't read the side of the patch that _uses_ the information stored in
>the rev-cache to figure out what it optimizes and what its limitations are
>(e.g. how it interacts with pathspecs). Perhaps the rev-cache may turn
>out to be _only_ useful for pack-objects and nothing else, in which case
>we may not care about standalone version of rev-cache generator after all.
rev-cache's cache slice traversal basically emulates git's revision
walker, on a smaller scale. At the moment it only really handles date
limiting (and obviously slop stuff) so it's not used for any pruning.
That's not to say it couldn't be updated in the future though.
^ permalink raw reply
* Re: [PATCH 1/6 (v2)] revision caching documentation: man page and technical discussion
From: Nick Edelen @ 2009-08-09 14:01 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nicolas Pitre, Johannes Schindelin, Sam Vilain, Michael J Gruber,
Jeff King, Shawn O. Pearce, Andreas Ericsson, Christian Couder,
git@vger.kernel.org
In-Reply-To: <7vbpmqi2d4.fsf@alter.siamese.dyndns.org>
Wow, thank you very much for your commentary!
Unfortunately I don't have a stable internet connection this week
(although I can get on at minimum once a day), but I'll rework the
docs today and upload a new patch for them tomorrow or the next day.
I'll also fix the file permissions ;-)
As you said, I'll answer most of your questions in the revised docs,
but I have one question about the time field size. git uses unsigned
long internally for its commit date, so it'd be limited by the 32bits
a long is on 32bit archs. So, wouldn't the commit times effectively
be a uin32_t? I mean if not I can change it; I just figured I'd save
some space.
Other than that there shouldn't be an overflow problem, unless
someone's merging over 63 branches in one go. Maybe I'll expand that
a bit just in case though...
Thanks again,
- Nick
^ permalink raw reply
* [PATCH] git-blame.el: Add `git-blame-show-full-message'
From: Martin Nordholts @ 2009-08-09 14:27 UTC (permalink / raw)
To: git
Add an interactively callable function `git-blame-show-full-message'
that shows the full commit message with --pretty=full in the echo
area.
The purpose of the function is to allow a user of git-blame.el to
browse around in a file until an interesting commit oneliner is shown,
at which point the user can invoke `git-blame-show-full-message' to
get more verbose information.
Signed-off-by: Martin Nordholts <martinn@src.gnome.org>
---
contrib/emacs/git-blame.el | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/contrib/emacs/git-blame.el b/contrib/emacs/git-blame.el
index 4fa70c5..443ebce 100644
--- a/contrib/emacs/git-blame.el
+++ b/contrib/emacs/git-blame.el
@@ -40,7 +40,8 @@
;; turn it on while viewing a file, the editor buffer will be updated by
;; setting the background of individual lines to a color that reflects
;; which commit it comes from. And when you move around the buffer, a
-;; one-line summary will be shown in the echo area.
+;; one-line summary will be shown in the echo area. To get the complete
+;; commit message, invoke `git-blame-show-full-message'.
;;; Installation:
;;
@@ -340,7 +341,7 @@ See also function `git-blame-mode'."
(git-blame-random-pop git-blame-colors)
git-blame-ancient-color)))
(setq info (list hash src-line res-line num-lines
- (git-describe-commit hash)
+ (git-describe-commit hash git-blame-log-oneline-format)
(cons 'color color))))
(puthash hash info git-blame-cache))
(goto-line res-line)
@@ -375,11 +376,11 @@ See also function `git-blame-mode'."
(car info)
(error "No commit info"))))
-(defun git-describe-commit (hash)
+(defun git-describe-commit (hash pretty-format)
(with-temp-buffer
(call-process "git" nil t nil
"log" "-1"
- (concat "--pretty=" git-blame-log-oneline-format)
+ (concat "--pretty=" pretty-format)
hash)
(buffer-substring (point-min) (point-max))))
@@ -392,6 +393,11 @@ See also function `git-blame-mode'."
(message "%s" (nth 4 info))
(setq git-blame-last-identification info))))
+(defun git-blame-show-full-message ()
+ "Show full log message about current commit"
+ (interactive)
+ (message (git-describe-commit (git-blame-current-commit) "full")))
+
;; (defun git-blame-after-save ()
;; (when git-blame-mode
;; (git-blame-cleanup)
--
1.6.2.5
^ permalink raw reply related
* [PATCH jp/symlink-dirs] t6035-merge-dir-to-symlink depends on SYMLINKS prerequisite
From: Johannes Sixt @ 2009-08-09 15:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
t/t6035-merge-dir-to-symlink.sh | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh
index a0ddf1e..5b96fb0 100755
--- a/t/t6035-merge-dir-to-symlink.sh
+++ b/t/t6035-merge-dir-to-symlink.sh
@@ -3,6 +3,12 @@
test_description='merging when a directory was replaced with a symlink'
. ./test-lib.sh
+if ! test_have_prereq SYMLINKS
+then
+ say 'Symbolic links not supported, skipping tests.'
+ test_done
+fi
+
test_expect_success 'create a commit where dir a/b changed to symlink' '
mkdir -p a/b/c a/b-2/c &&
> a/b/c/d &&
--
1.6.4.1026.g2deb5
^ permalink raw reply related
* [PATCH 1/2] t0001-init: fix a file name
From: Johannes Sixt @ 2009-08-09 15:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
Without this change, grep fails because it does not find the file
instead of because it does not find the text in the file.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
t/t0001-init.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 49caa29..07e011d 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -251,7 +251,7 @@ test_expect_success 'init creates a new deep directory' '
git init --bare --shared=0660 newdir/a/b/c &&
test -d newdir/a/b/c/refs &&
ls -ld newdir/a newdir/a/b > lsab.out &&
- ! grep -v "^drwxrw[sx]r-x" ls.out &&
+ ! grep -v "^drwxrw[sx]r-x" lsab.out &&
ls -ld newdir/a/b/c > lsc.out &&
! grep -v "^drwxrw[sx]---" lsc.out
)
--
1.6.4.1186.g1d9a
^ permalink raw reply related
* [PATCH 2/2] t0001-init: split the existence test from the permission test
From: Johannes Sixt @ 2009-08-09 15:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <4A7EED5C.8050707@kdbg.org>
The test for correct permissions after init created a deep directory
must be guarded by POSIXPERM. But testing that the deep dirctory exists
is good even on platforms that do not provide the POSIXPERM prerequiste.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
t/t0001-init.sh | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 07e011d..e53b234 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -245,6 +245,14 @@ test_expect_success 'init recreates a new bare directory' '
test_expect_success 'init creates a new deep directory' '
rm -fr newdir &&
(
+ git init newdir/a/b/c &&
+ test -d newdir/a/b/c/.git/refs
+ )
+'
+
+test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. shared)' '
+ rm -fr newdir &&
+ (
# Leading directories should honor umask while
# the repository itself should follow "shared"
umask 002 &&
--
1.6.4.1186.g1d9a
^ permalink raw reply related
* [PATCH v2 2/2] t0001-init: split the existence test from the permission test
From: Johannes Sixt @ 2009-08-09 16:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <4A7EEDA7.30301@kdbg.org>
The test for correct permissions after init created a deep directory
must be guarded by POSIXPERM. But testing that the deep dirctory exists
is good even on platforms that do not provide the POSIXPERM prerequiste.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Sorry, the first version was the result of mindless copying. This version
removes the sub-shell (parentheses).
-- Hannes
t/t0001-init.sh | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 07e011d..5386504 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -244,6 +244,12 @@ test_expect_success 'init recreates a new bare directory' '
test_expect_success 'init creates a new deep directory' '
rm -fr newdir &&
+ git init newdir/a/b/c &&
+ test -d newdir/a/b/c/.git/refs
+'
+
+test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. shared)' '
+ rm -fr newdir &&
(
# Leading directories should honor umask while
# the repository itself should follow "shared"
--
1.6.4.1186.g1d9a
^ permalink raw reply related
* Hash algorithm choice
From: Jerome Baum @ 2009-08-09 16:17 UTC (permalink / raw)
To: git
In-Reply-To: <f448a46a0908090907v68542e4dw1f1c4f610cb46ca2@mail.gmail.com>
Hi all,
I just had an idea regarding git and hashing. Couldn't find any
previous discussion on the subject so here's what I'm thinking of:
How difficult would it be to allow users to choose a hash function
during git-init which is then globally used in the repo? Are there
many changes needed or are changes in git-hash-object and git-init
sufficient?
I'm not trying to undermine the decision to use SHA-1 or anything, but
I would guess it builds for the future and adds flexibility to the
system. So when SHA-1 is no longer sufficient, it would be easy to
switch to RIPEMD-160 with a simple "git-init --hash=ripemd160"
Would be happy for any comments on this.
Regards,
Jerome Baum
Hugo-Junkers-Str. 2
D-37083 Göttingen
Germany
Tel.: +49 551 2008782
Web: www.JeromeBaum.com
^ permalink raw reply
* [PATCH v4 0/5] Re: {checkout,reset,stash} --patch
From: Nicolas Sebrecht @ 2009-08-09 16:32 UTC (permalink / raw)
To: Thomas Rast
Cc: Jeff King, git, Junio C Hamano, Sverre Rabbelier,
Nanako Shiraishi
In-Reply-To: <200908091117.19167.trast@student.ethz.ch>
The 09/08/09, Thomas Rast wrote:
> Jeff King wrote:
> >
> > 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 agree this approach is fine. That said, I admit I've been confused at
the beginning. FMPOV, asking "Discard this hunk?" is a real improvement.
In the same way I'd change "Reset?" of 'git reset -p' to "Unstage?".
Otherwise, the end-user don't know if the command will discard the hunk
from the WT too.
Also, I'd expect to have 'git reset --hard -p' discarding hunks from
both the index and the WT (which is not possible for now unless I missed
something).
--
Nicolas Sebrecht
^ permalink raw reply
* Re: [PATCH v4 0/5] Re: {checkout,reset,stash} --patch
From: Thomas Rast @ 2009-08-09 16:44 UTC (permalink / raw)
To: Nicolas Sebrecht
Cc: Jeff King, git, Junio C Hamano, Sverre Rabbelier,
Nanako Shiraishi
In-Reply-To: <20090809163233.GA12911@vidovic>
Nicolas Sebrecht wrote:
>
> Also, I'd expect to have 'git reset --hard -p' discarding hunks from
> both the index and the WT (which is not possible for now unless I missed
> something).
Well, the unfortunate overlap between 'git reset --hard' and 'git
checkout HEAD -- .' strikes again :-)
Since you can't say 'git reset --hard -- file', you have to do 'git
checkout HEAD -- file' to achieve this effect. So this usage is
covered by 'git checkout -p HEAD'.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH 3/3] tests: allow user to specify trash directory location
From: Johannes Schindelin @ 2009-08-09 17:42 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jeff King, git
In-Reply-To: <200908091533.40227.j6t@kdbg.org>
Hi,
On Sun, 9 Aug 2009, Johannes Sixt wrote:
> On Sonntag, 9. August 2009, Jeff King wrote:
> > 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?
>
> Not for Windows. Those devolpers who want to use an absolute path can use the
> POSIX path notation:
>
> GIT_TEST_OPTS=--root=/c/temp/gittests
>
> This works in t4014.
But I am not aware of any reasonable tmpfs equivalent on Windows. Are
you? I would be _very_ interested.
Ciao,
Dscho
^ permalink raw reply
* Re: Hash algorithm choice
From: Linus Torvalds @ 2009-08-09 17:46 UTC (permalink / raw)
To: Jerome Baum; +Cc: git
In-Reply-To: <f448a46a0908090917s102b4c83pbad6f298a8e127cc@mail.gmail.com>
On Sun, 9 Aug 2009, Jerome Baum wrote:
>
> How difficult would it be to allow users to choose a hash function
> during git-init which is then globally used in the repo? Are there
> many changes needed or are changes in git-hash-object and git-init
> sufficient?
If youlimit the hash size to 20 bytes, there are almost no changes
necessary.
You'd need to hijack the 'SHA1_Init/SHA1_Update/SHA1_Final' functions, of
course, and you'd likely want to rename them (and eventually a lot of
other functions too), but that renaming is mechanical and isn't even
needed for proper working.
Now, if you would ever want to extend the _size_ of the hash, that's a
much much bigger problem, but if you're ok with just changing the hash and
then truncating the result to 20 bytes (ie kind of like sha-512-160), or
you're ok with limiting yourself to 20-byte hashes like REIPMD-160, the
size of the changes should be minimal.
Linus
^ permalink raw reply
* Re: Hash algorithm choice
From: Johannes Schindelin @ 2009-08-09 17:49 UTC (permalink / raw)
To: Jerome Baum; +Cc: git
In-Reply-To: <f448a46a0908090917s102b4c83pbad6f298a8e127cc@mail.gmail.com>
Hi,
On Sun, 9 Aug 2009, Jerome Baum wrote:
> I just had an idea regarding git and hashing. Couldn't find any previous
> discussion on the subject so here's what I'm thinking of:
>
> How difficult would it be to allow users to choose a hash function
> during git-init which is then globally used in the repo? Are there many
> changes needed or are changes in git-hash-object and git-init
> sufficient?
A quick search revealed this:
http://thread.gmane.org/gmane.comp.version-control.git/25632/focus=25735
Hth,
Dscho
^ permalink raw reply
* Re: Hash algorithm choice
From: Junio C Hamano @ 2009-08-09 18:03 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Jerome Baum, git
In-Reply-To: <alpine.LFD.2.01.0908091038300.3288@localhost.localdomain>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> If youlimit the hash size to 20 bytes, there are almost no changes
> necessary.
>
> You'd need to hijack the 'SHA1_Init/SHA1_Update/SHA1_Final' functions, of
> course, and you'd likely want to rename them (and eventually a lot of
> other functions too), but that renaming is mechanical and isn't even
> needed for proper working.
>
> Now, if you would ever want to extend the _size_ of the hash, that's a
> much much bigger problem, but if you're ok with just changing the hash and
> then truncating the result to 20 bytes (ie kind of like sha-512-160), or
> you're ok with limiting yourself to 20-byte hashes like REIPMD-160, the
> size of the changes should be minimal.
Just in case Jerome really wants to go further, "almost no changes" and
"minimal" refers to the fact that we have a few hard-coded hash values
known to the code, such as the object name for an empty blob and an empty
tree.
^ permalink raw reply
* Re: Hash algorithm choice
From: Sverre Rabbelier @ 2009-08-09 18:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, Jerome Baum, git
In-Reply-To: <7vljls986b.fsf@alter.siamese.dyndns.org>
Heya,
On Sun, Aug 9, 2009 at 11:03, Junio C Hamano<gitster@pobox.com> wrote:
> Just in case Jerome really wants to go further, "almost no changes" and
> "minimal" refers to the fact that we have a few hard-coded hash values
> known to the code, such as the object name for an empty blob and an empty
> tree.
Wouldn't the transport code also have to be modified? I assume git's
integrity checking would yell at if you gave it commits with
non-sha1-hashes and have no way to tell it that that hash was
calculated with a non-sha1-hash at clone time?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 1/6 (v2)] revision caching documentation: man page and technical discussion
From: Junio C Hamano @ 2009-08-09 18:16 UTC (permalink / raw)
To: Nick Edelen
Cc: Nicolas Pitre, Johannes Schindelin, Sam Vilain, Michael J Gruber,
Jeff King, Shawn O. Pearce, Andreas Ericsson, Christian Couder,
git@vger.kernel.org
In-Reply-To: <c77435a80908090701x3aa2584fq353df1886ed1b02@mail.gmail.com>
Nick Edelen <sirnot@gmail.com> writes:
> As you said, I'll answer most of your questions in the revised docs,
> but I have one question about the time field size. git uses unsigned
> long internally for its commit date, so it'd be limited by the 32bits
> a long is on 32bit archs. So, wouldn't the commit times effectively
> be a uin32_t?
The current implementations may be either 32 or 64 bit, but at the on-disk
permanent data structure level they are decimal integers of unlimited
size. As the rev-cache, similarly to the index, is a local "cache" whose
nature is transient (iow we can afford to say "this new version of git
updates the file format for the rev-cache; your existing rev-cache will be
converted automatically"), its data representation can have limitation
tied to the implementation. But the limitation should be documented.
As to defining the on-disk data format using C structure, I'd strongly
suggest looking at, learning from and mimicking how read-cache.c and
cache.h handles struct ondisk_cache_entry. I find writing out a structure
with bitfields as-is, like your patch does, quite iffy.
> Other than that there shouldn't be an overflow problem, unless
> someone's merging over 63 branches in one go.
The current Porcelains may not allow it, but at the data structure level
commit can have arbitrary number of parents, so that assumption is already
broken at the design level. You need an escape hatch there.
"Because such a merge is so rare, walking the rev-cache that contains such
a commit gives a wrong result without any diagnosis to the caller" is an
unacceptable escape hatch. It is perfectly fine if the escape hatch is
"because it is so rare, rev-cache marks commits involved in such a path,
and walking the path falls back to actually reading the objects, without
any performance benefit from the cache". It probably is also Ok if the
escape hatch is "because it is beyond the implementation limit, 'add'
command errors out and rev-cache won't kick in for such a history."
^ permalink raw reply
* Re: [PATCH 5/6 (v2)] full integration of rev-cache into git's revision walker, completed test suite
From: Junio C Hamano @ 2009-08-09 18:22 UTC (permalink / raw)
To: Nick Edelen
Cc: Nicolas Pitre, Johannes Schindelin, Sam Vilain, Michael J Gruber,
Jeff King, Shawn O. Pearce, Andreas Ericsson, Christian Couder,
git@vger.kernel.org
In-Reply-To: <op.uyb1vl2jtdk399@sirnot.private>
"Nick Edelen" <sirnot@gmail.com> writes:
> This patch provides a working integration of rev-cache into the revision
> walker, along with some touch-ups:
This message from me is not about the patch 5/6 at all. It is about your
earlier patches in the series.
> ...
> - printf("queue:\n");
> + fprintf(stderr, "queue:\n");
> ...
> - printf("work:\n");
> + fprintf(stderr, "work:\n");
> ...
> - printf("pending:\n");
> + fprintf(stderr, "pending:\n");
> ...
> - pptr = &item->parents;
> + pptr = &item->parents;
> ...
> -#define SLOP 5
> +#define SLOP 5
> ...
These are all "Shoot, the mistakes in earlier ones need to be fixed here"
changes.
Please fix them in the patch earlier in the series so that you do not have
to make, and more importantly, reviewers do not have to get distracted by,
these fix-up in a later patch like this.
^ permalink raw reply
* Re: Hash algorithm choice
From: Linus Torvalds @ 2009-08-09 18:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jerome Baum, git
In-Reply-To: <7vljls986b.fsf@alter.siamese.dyndns.org>
On Sun, 9 Aug 2009, Junio C Hamano wrote:
>
> Just in case Jerome really wants to go further, "almost no changes" and
> "minimal" refers to the fact that we have a few hard-coded hash values
> known to the code, such as the object name for an empty blob and an empty
> tree.
Once nice thing about git is also that in many ways git doesn't even
_care_ about the actual hash algorithm, because read-only git will
generally (always?) just use the hashes as pointers.
So you could actually create a very limited sort of git that doesn't have
any hash algorithm at all - and it would still be able to do a lot of
regular git operations.
Git strictly speaking needs to hash things only when
- validating the index (ie if the index and stat data do not match). Git
also checks the SHA1 hash at teh end of the index file every time it
loads it.
- creating new objects (ie commit)
- git-fsck
- probably some situation I didn't think about.
but during normal operations git doesn't strictly _need_ to hash anything.
For example, I literally just checked what happens when you break our hash
algorithm on purpose, and while any index operation is unhappy and
complains about index corruption:
[torvalds@nehalem git]$ ./git diff
error: bad index file sha1 signature
fatal: index file corrupt
that's really largely a sanity check. You can literally do things like
"git log -p" without ever generating a single hash at all - because all
git will do is to look up objects based on the hashes it finds.
Now, what's nice about this is that it means that
(a) Hash performance only really matters for "git add" and "git fsck"
(well, as long as it's not _totally_ sucky. As mentioned above, we do
check the integrity of the index file more often, but that could be a
different hash than the _object_ hashes - so even if you change the
object hashes to be something else than SHA1, you wouldn't
necessarily have to change the index checksum)
(b) You could actually afford to have git auto-detect the hashes from
existing objects
(c) it's not even entirely unreasonable to mix different hashes in the
same repository (ie "old objects use old hash, new objects use new
hash".
Aliasing (same object with different hashes) will hurt disk-space,
and will make some operations (like merges and 'git diff') much more
expensive when they hit a "hash boundary", but other than that you'd
never even notice.
Of course, the downside to the above is that git may not notice some kinds
of corruption until you actually do things like "git fsck" (or native git
transfers like "git pull", which always check the result very carefully).
For disk corruption issues, there are things like zlib Adler checksums
(and xdelta crc's) etc that we always check when unpacking objects, but
the hash itself only gets recomputed for fairly special events.
Linus
^ permalink raw reply
* Re: Hash algorithm choice
From: Linus Torvalds @ 2009-08-09 18:35 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Junio C Hamano, Jerome Baum, git
In-Reply-To: <fabb9a1e0908091116y6012caa7j6865d5fcd5d9888c@mail.gmail.com>
On Sun, 9 Aug 2009, Sverre Rabbelier wrote:
>
> Wouldn't the transport code also have to be modified? I assume git's
> integrity checking would yell at if you gave it commits with
> non-sha1-hashes and have no way to tell it that that hash was
> calculated with a non-sha1-hash at clone time?
Well, if you start introducing new hashes, the assumption is that all
git's that access it would have to be updated.
You certainly could never pull/push between git versions that don't know
about each others hashes. But you _can_ autodetect the hash mechanism
(simple: just try them all on the first object you encounter)
Linus
^ permalink raw reply
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