* Re: [PATCH] Respect core.autocrlf when preparing temporary files for external diff
From: Junio C Hamano @ 2009-03-22 0:03 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Sebastian Schuberth
In-Reply-To: <alpine.DEB.1.00.0903220053260.10279@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Sat, 21 Mar 2009, Junio C Hamano wrote:
>
>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>>
>> > When preparing temporary files for an external diff, the files should
>> > be handled as if they were worktree files.
>>
>> I do not think so. convert_to_working_tree() aka "smudge" means you
>> would be feeding crap like $Id$ expansion to the external diff, which we
>> chose not to do quite on purpose.
>
> You might have missed me mentioning that we often can do without temporary
> files, taking the working directory copies right away?
>
> And if you think about it, it makes complete sense.
Not "complete".
What is at issue is how consistent the codepath that calls out to an
external diff should be with the rest of git that solely work with the
"clean" version of blob contents. If you value consistency, I would say
that it is valid to argue that letting it borrow from a work tree is a
bug. It should be always feeding a clean version.
But if you think that we do not care really about the correctness of the
external diff codepath, because it is a mechanism used mostly for giving
output to humans (as opposed to feeding the output of the external diff
program back to programs to be processed further) , I can understand why
you think it is easier to the external programs if "smudged" version is
fed to them.
Not just I can _understand_, but I think I could be pursuaded to agree
with that position, iff you try harder.
But the assumption/rationale behind this change needs to be spelled out.
In essence, we are sacrificing purity and correctness because we consider
ease of building external diff filter is more important.
Or something like that.
^ permalink raw reply
* Re: [PATCH] builtin-fast-export.c: add default case to avoid crash on corrupt repo
From: Junio C Hamano @ 2009-03-21 23:54 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, Johannes Schindelin, Shawn O. Pearce
In-Reply-To: <1237675051-6688-1-git-send-email-kusmabite@gmail.com>
Erik Faye-Lund <kusmabite@gmail.com> writes:
> This small issue was discovered by Benjamin Kramers Clang-runs on the
> git code-base. If a tag object points to an object that is not a commit
> or a blob, an invalid pointer is dereferenced in the code that followed.
A tag can point at anything, so this is not an issue about "crash on a
_corrupt_ repository".
I am not very familiar with this program, but the codepath involved should
be prepared to _see_ any type of object instead of dying.
What to do after _seeing_ a type of object is a different matter. It
appears that there is no way to feed a tree object to fast-import, but I
think the fast-import language can represent a tag that points at another
tag just fine. So the best you can do is perhaps to issue a warning
"skipping a tag that points at a tree object" and impoement a proper
handling of a tag that points at a tag.
^ permalink raw reply
* Re: Google Summer of Code 2009: GIT
From: Caleb Cushing @ 2009-03-21 23:53 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: saurabh gupta, david, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0903191118210.10279@pacific.mpi-cbg.de>
On Thu, Mar 19, 2009 at 6:19 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Sorry, by "data type" I tried to refer to the nature of the file. I
> should have said "file type".
My concern is that xml will be focused on and html which is not xml
and yet similar and common will be left out.
--
Caleb Cushing
http://xenoterracide.blogspot.com
^ permalink raw reply
* Re: [PATCH] Respect core.autocrlf when preparing temporary files for external diff
From: Johannes Schindelin @ 2009-03-21 23:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Sebastian Schuberth
In-Reply-To: <7vocvuekjb.fsf@gitster.siamese.dyndns.org>
Hi,
On Sat, 21 Mar 2009, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > When preparing temporary files for an external diff, the files should
> > be handled as if they were worktree files.
>
> I do not think so. convert_to_working_tree() aka "smudge" means you
> would be feeding crap like $Id$ expansion to the external diff, which we
> chose not to do quite on purpose.
You might have missed me mentioning that we often can do without temporary
files, taking the working directory copies right away?
And if you think about it, it makes complete sense.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] builtin-fast-export.c: add default case to avoid crash on corrupt repo
From: Erik Faye-Lund @ 2009-03-21 22:37 UTC (permalink / raw)
To: git; +Cc: Erik Faye-Lund
This small issue was discovered by Benjamin Kramers Clang-runs on the git
code-base. If a tag object points to an object that is not a commit or a blob,
an invalid pointer is dereferenced in the code that followed.
This patch fixes this issue, by giving an error instead. This should also be
more useful when debugging corrupted repos.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
builtin-fast-export.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index fdf4ae9..26b2a93 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -375,6 +375,9 @@ static void get_tags_and_duplicates(struct object_array *pending,
case OBJ_BLOB:
handle_object(tag->object.sha1);
continue;
+ default:
+ die ("Unexpected object of type %s",
+ typename(tag->object.type));
}
break;
default:
--
1.6.2.1.215.gf786.dirty
^ permalink raw reply related
* Re: [PATCH] Invoke "git gc --auto" when a commit is successful.
From: David J. Mellor @ 2009-03-21 23:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7viqm2bgou.fsf@gitster.siamese.dyndns.org>
On 03/21/2009 04:26 PM, Junio C Hamano wrote:
> I do not think situation has changed since the previous round:
>
> http://thread.gmane.org/gmane.comp.version-control.git/78524/focus=82130
>
>
Thanks for the pointer. I had searched the list before coding this up,
but somehow had missed this thread.
^ permalink raw reply
* [PATCH] Add --staged to bash completion for git diff
From: Kevin McConnell @ 2009-03-21 23:29 UTC (permalink / raw)
To: git
The --staged option (synonym for --cached) isn't listed in the
completion choices for git diff. This tiny patch adds it.
---
contrib/completion/git-completion.bash | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index ed235f7..6bc32df 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -899,7 +899,7 @@ _git_diff ()
local cur="${COMP_WORDS[COMP_CWORD]}"
case "$cur" in
--*)
- __gitcomp "--cached --pickaxe-all --pickaxe-regex
+ __gitcomp "--cached --staged --pickaxe-all --pickaxe-regex
--base --ours --theirs
$__git_diff_common_options
"
--
1.6.2.1
^ permalink raw reply related
* Re: [PATCH] Invoke "git gc --auto" when a commit is successful.
From: Junio C Hamano @ 2009-03-21 23:26 UTC (permalink / raw)
To: David J. Mellor; +Cc: git
In-Reply-To: <1237674894-8151-1-git-send-email-dmellor@whistlingcat.com>
I do not think situation has changed since the previous round:
http://thread.gmane.org/gmane.comp.version-control.git/78524/focus=82130
^ permalink raw reply
* Re: [PATCH v2 1/2] t7700: demonstrate repack flaw which may loosen objects unnecessarily
From: Brandon Casey @ 2009-03-21 23:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <ee63ef30903211525q4a95a27eoa4e95c7954e5cc93@mail.gmail.com>
Man I just can't seem to get a patch off without issues. I meant to
put these two comments in the patch emails.
1) Junio, these two patches are on top of 4d6acb70 which was the tip
of your jc/maint-1.6.0-keep-pack branch.
2) Also, note that this issue affects --keep-unreachable too, but the
fix is different since add_objects_in_unpacked_packs() just calls
add_object_entry() to add the unreachable objects to the list of
objects to pack and it (add_object_entry) already iterates on the pack
list. One way to fix it, which feels very kludgy to me, is to
temporarily set 'local' to one before calling add_object_entry, so
that it will do nothing if the object is found in any non-local pack.
Unless I'm convinced it is worth it, I don't plan on fixing it. I
doubt anyone uses --keep-unreachable manually. It was invented to
make git-gc safe and has now been replaced by --unpack-unreachable.
So, --keep-unreachable may pack objects that it doesn't really need to
pack. I can live with it.
-brandon
On Sat, Mar 21, 2009 at 5:25 PM, Brandon Casey <drafnel@gmail.com> wrote:
> If an unreferenced object exists in both a local pack and in either a pack
> residing in an alternate object database or a local kept pack, then the
> pack-objects call made by repack will loosen that object only to have it
> immediately pruned by repack's call to prune-packed.
>
> Signed-off-by: Brandon Casey <drafnel@gmail.com>
> ---
^ permalink raw reply
* Re: [PATCH 05/16] test-lib: Infrastructure to test and check for prerequisites
From: Junio C Hamano @ 2009-03-21 23:16 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <a7bb394037e1c32d47d0b04da025bdbe2eb78d66.1237667830.git.j6t@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> Some tests can be run only if a particular prerequisite is available. For
> example, some tests require that an UTF-8 locale is available. Here we
> introduce functions that are used in this way:
>
> 1. Insert code that checks whether the prerequisite is available. If it is,
> call test_set_prereq with an arbitrary tag name that subsequently can be
> used to check for the prerequisite:
>
> case $LANG in
> *.utf-8)
> test_set_prereq UTF8
> ;;
> esac
>
> 2. In the calls to test_expect_success pass the tag name:
>
> test_expect_success UTF8 '...description...' '...tests...'
>
> 3. There is an auxiliary predicate that can be used anywhere to test for
> a prerequisite explicitly:
>
> if test_have_prereq UTF8
> then
> ...code to be skipped if prerequisite is not available...
> fi
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Very nicely done. Thanks.
^ permalink raw reply
* Re: [PATCH 06/16] t3600: Use test prerequisite tags
From: Junio C Hamano @ 2009-03-21 23:15 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <70f789fecee6955c362391b33272223c7ba74cc6.1237667830.git.j6t@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> The are two prerequisites.
Typo?
>
> - The filesystem support names with tabs or new-lines.
>
> - Files cannot be removed if their containing directory is read-only.
>
> Previously, whether these preconditions are satisified was tested inside
> test_expect_success. We move these tests outside because, strictly
> speaking, they are not part of the tests.
Otherwise looks good; thanks.
^ permalink raw reply
* Re: [PATCH 1/7] check_ref_format(): tighten refname rules
From: Junio C Hamano @ 2009-03-21 23:15 UTC (permalink / raw)
To: git
In-Reply-To: <1237673619-12608-2-git-send-email-gitster@pobox.com>
Junio C Hamano <gitster@pobox.com> writes:
> diff --git a/refs.c b/refs.c
> index 8d3c502..abd5623 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -717,19 +717,23 @@ int check_ref_format(const char *ref)
> return CHECK_REF_FORMAT_ERROR;
> }
>
> + last = ch;
> /* scan the rest of the path component */
> while ((ch = *cp++) != 0) {
> bad_type = bad_ref_char(ch);
> - if (bad_type) {
> + if (bad_type)
> return CHECK_REF_FORMAT_ERROR;
> - }
> if (ch == '/')
> break;
> - if (ch == '.' && *cp == '.')
> + if (last == '.' && ch == '.')
> + return CHECK_REF_FORMAT_ERROR;
> + if (last == '@' && ch == '{')
> return CHECK_REF_FORMAT_ERROR;
Oops; here I need:
+ last = ch;
here.
> }
^ permalink raw reply
* Re: [PATCH 13/16] t3700: Skip a test with backslashes in pathspec
From: Junio C Hamano @ 2009-03-21 23:14 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <6acd113f60d1b0e7926386f3aebe5d72ad362034.1237667830.git.j6t@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> t/t3700-add.sh | 2 +-
> t/test-lib.sh | 2 ++
> 2 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
> index dc17d9f..050de42 100755
> --- a/t/t3700-add.sh
> +++ b/t/t3700-add.sh
> @@ -222,7 +222,7 @@ test_expect_success POSIXPERM 'git add (add.ignore-errors = false)' '
> ! ( git ls-files foo1 | grep foo1 )
> '
>
> -test_expect_success 'git add '\''fo\[ou\]bar'\'' ignores foobar' '
> +test_expect_success BSLASHPSPEC "git add 'fo\\[ou\\]bar' ignores foobar" '
> git reset --hard &&
> touch fo\[ou\]bar foobar &&
> git add '\''fo\[ou\]bar'\'' &&
I do not think the justification for this change is explained well enough.
The test prepares a file whose name consists of "ef, oh, bra, oh, you,
ket, bee, ei and are" (no backslashes), and passes a filespec that quotes
bra and ket with backslash so glob won't misinterpret as if we are asking
to add "ef oh followed by either oh or you followed by bee ei are". There
is no path that has a backslash in it involved.
If this does not work on Windows, there is something else going on. Is it
that the shell eats one level of backslash too much or something?
^ permalink raw reply
* Re: Disallow amending published commits?
From: James Pickens @ 2009-03-21 22:49 UTC (permalink / raw)
To: Peter Harris; +Cc: Git ML
In-Reply-To: <eaa105840903211146s4ff398e3qa8b570a8d29a83f4@mail.gmail.com>
[Resend since I forgot to cc the list]
On Sat, Mar 21, 2009, Peter Harris <git@peter.is-a-geek.org> wrote:
> An amended commit will have a new SHA1, and therefore git will treat
> it as an entirely different commit. Trying to push an amended history
> is 'non fast forward' in git terminology, since it involves a rewind
> of existing history.
>
> Set receive.denyNonFastForwards if you don't want people to be able to
> amend (or otherwise rewind) published history.
Thanks, but unfortunately that won't work in our workflow. Users never
push their changes; instead, they do a turnin to a continuous integration
server. The server clones the central repo, pulls their changes into the
clone, builds and tests it, then pushes to the central repo if it passes
the tests. So integration happens via 'pull' instead of 'push'.
We can't force the pulls to be fast forward only, because we need to allow
turnins from multiple users to be built and tested in parallel, without
requiring users to pull from each other or otherwise coordinate their
turnins.
James
^ permalink raw reply
* [PATCH] Invoke "git gc --auto" when a commit is successful.
From: David J. Mellor @ 2009-03-21 22:34 UTC (permalink / raw)
To: gitster; +Cc: git
This functionality was added to the previous git-commit.sh script in d4bb43e,
but omitted when the script was ported to C in f5bbc32. This patch reinstates
the functionality by copying the equivalent code that was introduced in
builtin-merge.c in 1c7b76b.
Signed-off-by: David J. Mellor <dmellor@whistlingcat.com>
---
builtin-commit.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 46e649c..780c142 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -1029,5 +1029,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
if (!quiet)
print_summary(prefix, commit_sha1);
+ /*
+ * Perform an automatic garbage collection if the commit was successful.
+ * We ignore errors in 'gc --auto', since the user should see them.
+ */
+ const char *argv_gc_auto[] = { "gc", "--auto", NULL };
+ run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+
return 0;
}
--
1.6.2.1
^ permalink raw reply related
* [PATCH v2 2/2] pack-objects: don't loosen objects available in alternate or kept packs
From: Brandon Casey @ 2009-03-21 22:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
If pack-objects is called with the --unpack-unreachable option then it will
unpack (i.e. loosen) all unreferenced objects from local not-kept packs,
including those that also exist in packs residing in an alternate object
database or a local kept pack. The primary(sole?) user of this option is
git-repack. In this case, repack will follow the call to pack-objects with
a call to prune-packed which will delete these newly loosened objects,
making the act of loosening a waste of time. The unnecessary loosening can
be avoided by checking whether an object exists in a non-local pack or a
local kept pack before loosening it.
This fixes the 'local packed unreachable obs that exist in alternate ODB
are not loosened' test in t7700.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
Signed version of the previous version.
builtin-pack-objects.c | 26 +++++++++++++++++++++++++-
t/t7700-repack.sh | 2 +-
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 6222f19..3f477c5 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1944,6 +1944,29 @@ static void
add_objects_in_unpacked_packs(struct rev_info *revs)
free(in_pack.array);
}
+static int has_sha1_pack_kept_or_nonlocal(const unsigned char *sha1)
+{
+ static struct packed_git *last_found = (void *)1;
+ struct packed_git *p;
+
+ p = (last_found == (void *)1) ? packed_git : last_found;
+
+ while (p) {
+ if ((!p->pack_local || p->pack_keep) &&
+ find_pack_entry_one(sha1, p)) {
+ last_found = p;
+ return 1;
+ }
+ if (p == last_found)
+ p = packed_git;
+ else
+ p = p->next;
+ if (p == last_found)
+ p = p->next;
+ }
+ return 0;
+}
+
static void loosen_unused_packed_objects(struct rev_info *revs)
{
struct packed_git *p;
@@ -1959,7 +1982,8 @@ static void loosen_unused_packed_objects(struct
rev_info *revs)
for (i = 0; i < p->num_objects; i++) {
sha1 = nth_packed_object_sha1(p, i);
- if (!locate_object_entry(sha1))
+ if (!locate_object_entry(sha1) &&
+ !has_sha1_pack_kept_or_nonlocal(sha1))
if (force_object_loose(sha1, p->mtime))
die("unable to force loose object");
}
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 013e488..9ce546e 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -113,7 +113,7 @@ test_expect_success 'packed unreachable obs in
alternate ODB are not loosened' '
test_must_fail git show $csha1
'
-test_expect_failure 'local packed unreachable obs that exist in
alternate ODB are not loosened' '
+test_expect_success 'local packed unreachable obs that exist in
alternate ODB are not loosened' '
echo `pwd`/alt_objects > .git/objects/info/alternates &&
echo "$csha1" | git pack-objects --non-empty --all --reflog pack &&
rm -f .git/objects/pack/* &&
--
1.6.2.12.g83676
^ permalink raw reply related
* [PATCH v2 1/2] t7700: demonstrate repack flaw which may loosen objects unnecessarily
From: Brandon Casey @ 2009-03-21 22:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
If an unreferenced object exists in both a local pack and in either a pack
residing in an alternate object database or a local kept pack, then the
pack-objects call made by repack will loosen that object only to have it
immediately pruned by repack's call to prune-packed.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
Signed version of the previous version.
t/t7700-repack.sh | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 1ef3892..013e488 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -113,5 +113,22 @@ test_expect_success 'packed unreachable obs in
alternate ODB are not loosened' '
test_must_fail git show $csha1
'
+test_expect_failure 'local packed unreachable obs that exist in
alternate ODB are not loosened' '
+ echo `pwd`/alt_objects > .git/objects/info/alternates &&
+ echo "$csha1" | git pack-objects --non-empty --all --reflog pack &&
+ rm -f .git/objects/pack/* &&
+ mv pack-* .git/objects/pack/ &&
+ # The pack-objects call on the next line is equivalent to
+ # git repack -A -d without the call to prune-packed
+ git pack-objects --honor-pack-keep --non-empty --all --reflog \
+ --unpack-unreachable </dev/null pack &&
+ rm -f .git/objects/pack/* &&
+ mv pack-* .git/objects/pack/ &&
+ test 0 = $(git verify-pack -v -- .git/objects/pack/*.idx |
+ egrep "^$csha1 " | sort | uniq | wc -l) &&
+ echo > .git/objects/info/alternates &&
+ test_must_fail git show $csha1
+'
+
test_done
--
1.6.2.12.g83676
^ permalink raw reply related
* [PATCH 2/2] pack-objects: don't loosen objects available in alternate or kept packs
From: Brandon Casey @ 2009-03-21 22:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
If pack-objects is called with the --unpack-unreachable option then it will
unpack (i.e. loosen) all unreferenced objects from local not-kept packs,
including those that also exist in packs residing in an alternate object
database or a local kept pack. The primary(sole?) user of this option is
git-repack. In this case, repack will follow the call to pack-objects with
a call to prune-packed which will delete these newly loosened objects,
making the act of loosening a waste of time. The unnecessary loosening can
be avoided by checking whether an object exists in a non-local pack or a
local kept pack before loosening it.
This fixes the 'local packed unreachable obs that exist in alternate ODB
are not loosened' test in t7700.
---
builtin-pack-objects.c | 26 +++++++++++++++++++++++++-
t/t7700-repack.sh | 2 +-
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 6222f19..3f477c5 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1944,6 +1944,29 @@ static void
add_objects_in_unpacked_packs(struct rev_info *revs)
free(in_pack.array);
}
+static int has_sha1_pack_kept_or_nonlocal(const unsigned char *sha1)
+{
+ static struct packed_git *last_found = (void *)1;
+ struct packed_git *p;
+
+ p = (last_found == (void *)1) ? packed_git : last_found;
+
+ while (p) {
+ if ((!p->pack_local || p->pack_keep) &&
+ find_pack_entry_one(sha1, p)) {
+ last_found = p;
+ return 1;
+ }
+ if (p == last_found)
+ p = packed_git;
+ else
+ p = p->next;
+ if (p == last_found)
+ p = p->next;
+ }
+ return 0;
+}
+
static void loosen_unused_packed_objects(struct rev_info *revs)
{
struct packed_git *p;
@@ -1959,7 +1982,8 @@ static void loosen_unused_packed_objects(struct
rev_info *revs)
for (i = 0; i < p->num_objects; i++) {
sha1 = nth_packed_object_sha1(p, i);
- if (!locate_object_entry(sha1))
+ if (!locate_object_entry(sha1) &&
+ !has_sha1_pack_kept_or_nonlocal(sha1))
if (force_object_loose(sha1, p->mtime))
die("unable to force loose object");
}
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 013e488..9ce546e 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -113,7 +113,7 @@ test_expect_success 'packed unreachable obs in
alternate ODB are not loosened' '
test_must_fail git show $csha1
'
-test_expect_failure 'local packed unreachable obs that exist in
alternate ODB are not loosened' '
+test_expect_success 'local packed unreachable obs that exist in
alternate ODB are not loosened' '
echo `pwd`/alt_objects > .git/objects/info/alternates &&
echo "$csha1" | git pack-objects --non-empty --all --reflog pack &&
rm -f .git/objects/pack/* &&
--
1.6.2.12.g83676
^ permalink raw reply related
* [PATCH 1/2] t7700: demonstrate repack flaw which may loosen objects unnecessarily
From: Brandon Casey @ 2009-03-21 22:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
If an unreferenced object exists in both a local pack and in either a pack
residing in an alternate object database or a local kept pack, then the
pack-objects call made by repack will loosen that object only to have it
immediately pruned by repack's call to prune-packed.
---
t/t7700-repack.sh | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 1ef3892..013e488 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -113,5 +113,22 @@ test_expect_success 'packed unreachable obs in
alternate ODB are not loosened' '
test_must_fail git show $csha1
'
+test_expect_failure 'local packed unreachable obs that exist in
alternate ODB are not loosened' '
+ echo `pwd`/alt_objects > .git/objects/info/alternates &&
+ echo "$csha1" | git pack-objects --non-empty --all --reflog pack &&
+ rm -f .git/objects/pack/* &&
+ mv pack-* .git/objects/pack/ &&
+ # The pack-objects call on the next line is equivalent to
+ # git repack -A -d without the call to prune-packed
+ git pack-objects --honor-pack-keep --non-empty --all --reflog \
+ --unpack-unreachable </dev/null pack &&
+ rm -f .git/objects/pack/* &&
+ mv pack-* .git/objects/pack/ &&
+ test 0 = $(git verify-pack -v -- .git/objects/pack/*.idx |
+ egrep "^$csha1 " | sort | uniq | wc -l) &&
+ echo > .git/objects/info/alternates &&
+ test_must_fail git show $csha1
+'
+
test_done
--
1.6.2.12.g83676
^ permalink raw reply related
* Re: What's cooking in git.git (Mar 2009, #06; Sat, 21)
From: Johannes Sixt @ 2009-03-21 22:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk56jfgt2.fsf@gitster.siamese.dyndns.org>
On Samstag, 21. März 2009, Junio C Hamano wrote:
> * js/maint-1.6.0-exec-path-env (Wed Mar 18 08:42:53 2009 +0100) 1 commit
> - export GIT_EXEC_PATH when git is run with --exec-path
Here is the patch again with a commit message that explains what's going on.
But notice that this is probably not a complete solution: As you can see
(below) from the analysis, a git-repack that was taken from $prefix would
invoke a git that is not from $prefix, but from the initial command's
--exec-path. I think this can be solved by installing the git executable
in $prefix/libexec/git-core as well.
-- Hannes
-- 8< --
Subject: [PATCH] Propagate --exec-path setting to external commands via GIT_EXEC_PATH
Let PATH0=$PATH that was set before the invocation.
Let /foo be a build directory.
Let /pfx be the installation prefix.
Let pfxexecpath=/pfx/libexec/git-core.
The following is going on when 'git --exec-path=/foo gc' is invoked:
1. git sets PATH=/foo:$PATH0 using the path from --exec-path
2. gc execs 'git repack' (note: no dash).
3. Since there is a git in /foo (it's a build directory), /foo/git is
taken.
4. No explicit exec-path is set this time, hence, this secondary git sets
PATH=$pfxexecpath:/foo:$PATH
5. Since 'repack' is not a built-in, execv_dashed_external execs
'git-repack' (note: dash).
6. There is a $pfxexecpath/git-repack, and it is taken.
7. This git-repack runs 'git pack-objects' (note: no dash).
8. There is no git in $pfxexecpath, but there is one in /foo. Hence,
/foo/git is run.
9. pack-objects is a builtin, hence, in effect /foo/git-pack-objects
is run.
As you can see, the way in which we previously set the PATH allowed to
mix gits of different vintage. By setting GIT_EXEC_PATH when --exec-path
was given on the command line, we reduce the confusion.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
exec_cmd.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/exec_cmd.c b/exec_cmd.c
index 217c125..408e4e5 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -61,6 +61,10 @@ const char *git_extract_argv0_path(const char *argv0)
void git_set_argv_exec_path(const char *exec_path)
{
argv_exec_path = exec_path;
+ /*
+ * Propagate this setting to external programs.
+ */
+ setenv(EXEC_PATH_ENVIRONMENT, exec_path, 1);
}
--
1.6.2.1.224.g2225f
^ permalink raw reply related
* [PATCH 7/7] checkout -: make "-" to mean "previous branch" everywhere
From: Junio C Hamano @ 2009-03-21 22:13 UTC (permalink / raw)
To: git
In-Reply-To: <1237673619-12608-7-git-send-email-gitster@pobox.com>
This is iffy, in that it teaches the very low level machinery to interpret
it as "the tip of the previous branch" when "-" is fed to it, and may have
a high risk of unintended side effects.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-checkout.c | 8 +++++---
sha1_name.c | 21 +++++++++++++--------
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 66df0c0..6b3b450 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -666,9 +666,11 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
arg = argv[0];
has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
- if (!strcmp(arg, "-"))
- arg = "@{-1}";
-
+ {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_branchname(&sb, arg);
+ arg = strbuf_detach(&sb, NULL);
+ }
if (get_sha1(arg, rev)) {
if (has_dash_dash) /* case (1) */
die("invalid reference: %s", arg);
diff --git a/sha1_name.c b/sha1_name.c
index 904bcd9..3972f4c 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -758,14 +758,19 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
const char *brace;
char *num_end;
- if (name[0] != '@' || name[1] != '{' || name[2] != '-')
- return -1;
- brace = strchr(name, '}');
- if (!brace)
- return -1;
- nth = strtol(name+3, &num_end, 10);
- if (num_end != brace)
- return -1;
+ if (name[0] == '-' && !name[1]) {
+ nth = 1;
+ brace = name; /* "end of branch name expression" */
+ } else {
+ if (name[0] != '@' || name[1] != '{' || name[2] != '-')
+ return -1;
+ brace = strchr(name, '}');
+ if (!brace)
+ return -1;
+ nth = strtol(name+3, &num_end, 10);
+ if (num_end != brace)
+ return -1;
+ }
if (nth <= 0)
return -1;
cb.alloc = nth;
--
1.6.2.1.299.gda643a
^ permalink raw reply related
* [PATCH 6/7] strbuf_check_branch_ref(): a helper to check a refname for a branch
From: Junio C Hamano @ 2009-03-21 22:13 UTC (permalink / raw)
To: git
In-Reply-To: <1237673619-12608-6-git-send-email-gitster@pobox.com>
This allows a common calling sequence
strbuf_branchname(&ref, name);
strbuf_splice(&ref, 0, 0, "refs/heads/", 11);
if (check_ref_format(ref.buf))
die(...);
to be refactored into
if (strbuf_check_branch_ref(&ref, name))
die(...);
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
branch.c | 5 +----
builtin-branch.c | 8 ++------
builtin-check-ref-format.c | 5 ++---
builtin-checkout.c | 7 +++----
strbuf.c | 8 ++++++++
strbuf.h | 1 +
6 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/branch.c b/branch.c
index 558f092..62030af 100644
--- a/branch.c
+++ b/branch.c
@@ -135,10 +135,7 @@ void create_branch(const char *head,
struct strbuf ref = STRBUF_INIT;
int forcing = 0;
- strbuf_branchname(&ref, name);
- strbuf_splice(&ref, 0, 0, "refs/heads/", 11);
-
- if (check_ref_format(ref.buf))
+ if (strbuf_check_branch_ref(&ref, name))
die("'%s' is not a valid branch name.", name);
if (resolve_ref(ref.buf, sha1, 1, NULL)) {
diff --git a/builtin-branch.c b/builtin-branch.c
index e15440f..cd8f42d 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -468,14 +468,10 @@ static void rename_branch(const char *oldname, const char *newname, int force)
if (!oldname)
die("cannot rename the current branch while not on any.");
- strbuf_branchname(&oldref, oldname);
- strbuf_splice(&oldref, 0, 0, "refs/heads/", 11);
- if (check_ref_format(oldref.buf))
+ if (strbuf_check_branch_ref(&oldref, oldname))
die("Invalid branch name: '%s'", oldname);
- strbuf_branchname(&newref, newname);
- strbuf_splice(&newref, 0, 0, "refs/heads/", 11);
- if (check_ref_format(newref.buf))
+ if (strbuf_check_branch_ref(&newref, newname))
die("Invalid branch name: '%s'", newname);
if (resolve_ref(newref.buf, sha1, 1, NULL) && !force)
diff --git a/builtin-check-ref-format.c b/builtin-check-ref-format.c
index 39db6cb..f9381e0 100644
--- a/builtin-check-ref-format.c
+++ b/builtin-check-ref-format.c
@@ -11,9 +11,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
if (argc == 3 && !strcmp(argv[1], "--branch")) {
struct strbuf sb = STRBUF_INIT;
- strbuf_branchname(&sb, argv[2]);
- strbuf_splice(&sb, 0, 0, "refs/heads/", 11);
- if (check_ref_format(sb.buf))
+
+ if (strbuf_check_branch_ref(&sb, argv[2]))
die("'%s' is not a valid branch name", argv[2]);
printf("%s\n", sb.buf + 11);
exit(0);
diff --git a/builtin-checkout.c b/builtin-checkout.c
index b268046..66df0c0 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -733,12 +733,11 @@ no_reference:
if (opts.new_branch) {
struct strbuf buf = STRBUF_INIT;
- strbuf_addstr(&buf, "refs/heads/");
- strbuf_addstr(&buf, opts.new_branch);
+ if (strbuf_check_branch_ref(&buf, opts.new_branch))
+ die("git checkout: we do not like '%s' as a branch name.",
+ opts.new_branch);
if (!get_sha1(buf.buf, rev))
die("git checkout: branch %s already exists", opts.new_branch);
- if (check_ref_format(buf.buf))
- die("git checkout: we do not like '%s' as a branch name.", opts.new_branch);
strbuf_release(&buf);
}
diff --git a/strbuf.c b/strbuf.c
index a60b0ad..a884960 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,4 +1,5 @@
#include "cache.h"
+#include "refs.h"
int prefixcmp(const char *str, const char *prefix)
{
@@ -366,3 +367,10 @@ int strbuf_branchname(struct strbuf *sb, const char *name)
strbuf_add(sb, name, len);
return len;
}
+
+int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
+{
+ strbuf_branchname(sb, name);
+ strbuf_splice(sb, 0, 0, "refs/heads/", 11);
+ return check_ref_format(sb->buf);
+}
diff --git a/strbuf.h b/strbuf.h
index 68923e1..9ee908a 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -132,5 +132,6 @@ extern void stripspace(struct strbuf *buf, int skip_comments);
extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);
extern int strbuf_branchname(struct strbuf *sb, const char *name);
+extern int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
#endif /* STRBUF_H */
--
1.6.2.1.299.gda643a
^ permalink raw reply related
* [PATCH 5/7] Fix "branch -m @{-1} newname"
From: Junio C Hamano @ 2009-03-21 22:13 UTC (permalink / raw)
To: git
In-Reply-To: <1237673619-12608-5-git-send-email-gitster@pobox.com>
The command is supposed to rename the branch we were on before switched
from to a new name, but the codepath involved in it was not aware of the
short-hand notation we added recently.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-branch.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin-branch.c b/builtin-branch.c
index 7452db1..e15440f 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -468,18 +468,18 @@ static void rename_branch(const char *oldname, const char *newname, int force)
if (!oldname)
die("cannot rename the current branch while not on any.");
- strbuf_addf(&oldref, "refs/heads/%s", oldname);
-
+ strbuf_branchname(&oldref, oldname);
+ strbuf_splice(&oldref, 0, 0, "refs/heads/", 11);
if (check_ref_format(oldref.buf))
- die("Invalid branch name: %s", oldref.buf);
-
- strbuf_addf(&newref, "refs/heads/%s", newname);
+ die("Invalid branch name: '%s'", oldname);
+ strbuf_branchname(&newref, newname);
+ strbuf_splice(&newref, 0, 0, "refs/heads/", 11);
if (check_ref_format(newref.buf))
- die("Invalid branch name: %s", newref.buf);
+ die("Invalid branch name: '%s'", newname);
if (resolve_ref(newref.buf, sha1, 1, NULL) && !force)
- die("A branch named '%s' already exists.", newname);
+ die("A branch named '%s' already exists.", newref.buf+11);
strbuf_addf(&logmsg, "Branch: renamed %s to %s",
oldref.buf, newref.buf);
--
1.6.2.1.299.gda643a
^ permalink raw reply related
* [PATCH 4/7] strbuf_branchname(): a wrapper for branch name shorthands
From: Junio C Hamano @ 2009-03-21 22:13 UTC (permalink / raw)
To: git
In-Reply-To: <1237673619-12608-4-git-send-email-gitster@pobox.com>
The function takes a user-supplied string that is supposed to be a branch
name, and puts it in a strbuf after expanding possible shorthand notation.
This removes repeated lines to do so in existing code.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
branch.c | 7 +------
builtin-branch.c | 6 +-----
builtin-checkout.c | 11 +++--------
builtin-merge.c | 5 ++---
strbuf.c | 9 +++++++++
strbuf.h | 2 ++
6 files changed, 18 insertions(+), 22 deletions(-)
diff --git a/branch.c b/branch.c
index 313bcf1..558f092 100644
--- a/branch.c
+++ b/branch.c
@@ -134,13 +134,8 @@ void create_branch(const char *head,
char *real_ref, msg[PATH_MAX + 20];
struct strbuf ref = STRBUF_INIT;
int forcing = 0;
- int len;
- len = strlen(name);
- if (interpret_branch_name(name, &ref) != len) {
- strbuf_reset(&ref);
- strbuf_add(&ref, name, len);
- }
+ strbuf_branchname(&ref, name);
strbuf_splice(&ref, 0, 0, "refs/heads/", 11);
if (check_ref_format(ref.buf))
diff --git a/builtin-branch.c b/builtin-branch.c
index cacd7da..7452db1 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -121,11 +121,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
die("Couldn't look up commit object for HEAD");
}
for (i = 0; i < argc; i++, strbuf_release(&bname)) {
- int len = strlen(argv[i]);
-
- if (interpret_branch_name(argv[i], &bname) != len)
- strbuf_add(&bname, argv[i], len);
-
+ strbuf_branchname(&bname, argv[i]);
if (kinds == REF_LOCAL_BRANCH && !strcmp(head, bname.buf)) {
error("Cannot delete the branch '%s' "
"which you are currently on.", bname.buf);
diff --git a/builtin-checkout.c b/builtin-checkout.c
index a8d9d97..b268046 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -353,16 +353,11 @@ struct branch_info {
static void setup_branch_path(struct branch_info *branch)
{
struct strbuf buf = STRBUF_INIT;
- int ret;
- if ((ret = interpret_branch_name(branch->name, &buf))
- && ret == strlen(branch->name)) {
+ strbuf_branchname(&buf, branch->name);
+ if (strcmp(buf.buf, branch->name))
branch->name = xstrdup(buf.buf);
- strbuf_splice(&buf, 0, 0, "refs/heads/", 11);
- } else {
- strbuf_addstr(&buf, "refs/heads/");
- strbuf_addstr(&buf, branch->name);
- }
+ strbuf_splice(&buf, 0, 0, "refs/heads/", 11);
branch->path = strbuf_detach(&buf, NULL);
}
diff --git a/builtin-merge.c b/builtin-merge.c
index e94ea7c..6a51823 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -360,9 +360,8 @@ static void merge_name(const char *remote, struct strbuf *msg)
const char *ptr;
int len, early;
- len = strlen(remote);
- if (interpret_branch_name(remote, &bname) == len)
- remote = bname.buf;
+ strbuf_branchname(&bname, remote);
+ remote = bname.buf;
memset(branch_head, 0, sizeof(branch_head));
remote_head = peel_to_type(remote, 0, NULL, OBJ_COMMIT);
diff --git a/strbuf.c b/strbuf.c
index bfbd816..a60b0ad 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -357,3 +357,12 @@ int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
return len;
}
+
+int strbuf_branchname(struct strbuf *sb, const char *name)
+{
+ int len = strlen(name);
+ if (interpret_branch_name(name, sb) == len)
+ return 0;
+ strbuf_add(sb, name, len);
+ return len;
+}
diff --git a/strbuf.h b/strbuf.h
index 89bd36e..68923e1 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -131,4 +131,6 @@ extern int strbuf_getline(struct strbuf *, FILE *, int);
extern void stripspace(struct strbuf *buf, int skip_comments);
extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);
+extern int strbuf_branchname(struct strbuf *sb, const char *name);
+
#endif /* STRBUF_H */
--
1.6.2.1.299.gda643a
^ permalink raw reply related
* [PATCH 3/7] check-ref-format --branch: give Porcelain a way to grok branch shorthand
From: Junio C Hamano @ 2009-03-21 22:13 UTC (permalink / raw)
To: git
In-Reply-To: <1237673619-12608-3-git-send-email-gitster@pobox.com>
The command may not be the best place to add this new feature, but
$ git check-ref-format --branch "@{-1}"
allows Porcelains to figure out what branch you were on before the last
branch switching.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-check-ref-format.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/builtin-check-ref-format.c b/builtin-check-ref-format.c
index 701de43..39db6cb 100644
--- a/builtin-check-ref-format.c
+++ b/builtin-check-ref-format.c
@@ -5,9 +5,19 @@
#include "cache.h"
#include "refs.h"
#include "builtin.h"
+#include "strbuf.h"
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
+ if (argc == 3 && !strcmp(argv[1], "--branch")) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_branchname(&sb, argv[2]);
+ strbuf_splice(&sb, 0, 0, "refs/heads/", 11);
+ if (check_ref_format(sb.buf))
+ die("'%s' is not a valid branch name", argv[2]);
+ printf("%s\n", sb.buf + 11);
+ exit(0);
+ }
if (argc != 2)
usage("git check-ref-format refname");
return !!check_ref_format(argv[1]);
--
1.6.2.1.299.gda643a
^ 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