* [PATCH 1/3] revert: rename --reset option to --quit
From: Jonathan Nieder @ 2011-11-20 9:48 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: git, Christian Couder, Martin von Zweigbergk, Phil Hord
In-Reply-To: <20111120094650.GB2278@elie.hsd1.il.comcast.net>
The option to "git cherry-pick" and "git revert" to discard the
sequencer state introduced by v1.7.8-rc0~141^2~6 (revert: Introduce
--reset to remove sequencer state, 2011-08-04) has a confusing name.
Change it now, while we still have the time.
Mechanics:
This commit removes the "git cherry-pick --reset" option. Hopefully
nobody was using it. If somebody was, we can add it back again as a
synonym.
The new name for "cherry-pick, please get out of my way, since I've
long forgotten about the sequence of commits I was cherry-picking when
you wrote that old .git/sequencer directory" is --quit. Mnemonic:
this is analagous to quiting a program the user is no longer using ---
we just want to get out of the multiple-command cherry-pick procedure
and not to reset HEAD or restore any other old state.
Adjust documentation and tests to match. While at it, let's clarify
the short descriptions of these operations in "-h" output.
Before:
--reset forget the current operation
--continue continue the current operation
After:
--quit end revert or cherry-pick sequence
--continue resume revert or cherry-pick sequence
Noticed-by: Phil Hord <phil.hord@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Documentation/git-cherry-pick.txt | 2 +-
Documentation/git-revert.txt | 2 +-
Documentation/sequencer.txt | 10 +++++-----
builtin/revert.c | 22 +++++++++++-----------
sequencer.h | 2 +-
t/t3510-cherry-pick-sequence.sh | 10 +++++-----
t/t7106-reset-sequence.sh | 2 +-
7 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 2660a842..21998b82 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -9,8 +9,8 @@ SYNOPSIS
--------
[verse]
'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>...
-'git cherry-pick' --reset
'git cherry-pick' --continue
+'git cherry-pick' --quit
DESCRIPTION
-----------
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index f3519413..b0fcabc8 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -9,8 +9,8 @@ SYNOPSIS
--------
[verse]
'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] <commit>...
-'git revert' --reset
'git revert' --continue
+'git revert' --quit
DESCRIPTION
-----------
diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt
index 3e6df338..75f8e869 100644
--- a/Documentation/sequencer.txt
+++ b/Documentation/sequencer.txt
@@ -1,9 +1,9 @@
---reset::
- Forget about the current operation in progress. Can be used
- to clear the sequencer state after a failed cherry-pick or
- revert.
-
--continue::
Continue the operation in progress using the information in
'.git/sequencer'. Can be used to continue after resolving
conflicts in a failed cherry-pick or revert.
+
+--quit::
+ Forget about the current operation in progress. Can be used
+ to clear the sequencer state after a failed cherry-pick or
+ revert.
diff --git a/builtin/revert.c b/builtin/revert.c
index 544e8c30..b59dd68c 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -40,7 +40,7 @@ static const char * const cherry_pick_usage[] = {
};
enum replay_action { REVERT, CHERRY_PICK };
-enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
+enum replay_subcommand { REPLAY_NONE, REPLAY_REMOVE_STATE, REPLAY_CONTINUE };
struct replay_opts {
enum replay_action action;
@@ -133,11 +133,11 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
{
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
const char *me = action_name(opts);
- int reset = 0;
+ int remove_state = 0;
int contin = 0;
struct option options[] = {
- OPT_BOOLEAN(0, "reset", &reset, "forget the current operation"),
- OPT_BOOLEAN(0, "continue", &contin, "continue the current operation"),
+ OPT_BOOLEAN(0, "quit", &remove_state, "end revert or cherry-pick sequence"),
+ OPT_BOOLEAN(0, "continue", &contin, "resume revert or cherry-pick sequence"),
OPT_BOOLEAN('n', "no-commit", &opts->no_commit, "don't automatically commit"),
OPT_BOOLEAN('e', "edit", &opts->edit, "edit the commit message"),
OPT_NOOP_NOARG('r', NULL),
@@ -168,13 +168,13 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
/* Check for incompatible subcommands */
verify_opt_mutually_compatible(me,
- "--reset", reset,
+ "--quit", remove_state,
"--continue", contin,
NULL);
/* Set the subcommand */
- if (reset)
- opts->subcommand = REPLAY_RESET;
+ if (remove_state)
+ opts->subcommand = REPLAY_REMOVE_STATE;
else if (contin)
opts->subcommand = REPLAY_CONTINUE;
else
@@ -183,8 +183,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
/* Check for incompatible command line arguments */
if (opts->subcommand != REPLAY_NONE) {
char *this_operation;
- if (opts->subcommand == REPLAY_RESET)
- this_operation = "--reset";
+ if (opts->subcommand == REPLAY_REMOVE_STATE)
+ this_operation = "--quit";
else
this_operation = "--continue";
@@ -965,7 +965,7 @@ static int pick_revisions(struct replay_opts *opts)
* cherry-pick should be handled differently from an existing
* one that is being continued
*/
- if (opts->subcommand == REPLAY_RESET) {
+ if (opts->subcommand == REPLAY_REMOVE_STATE) {
remove_sequencer_state(1);
return 0;
} else if (opts->subcommand == REPLAY_CONTINUE) {
@@ -988,7 +988,7 @@ static int pick_revisions(struct replay_opts *opts)
if (create_seq_dir() < 0) {
error(_("A cherry-pick or revert is in progress."));
advise(_("Use --continue to continue the operation"));
- advise(_("or --reset to forget about it"));
+ advise(_("or --quit to forget about it"));
return -1;
}
if (get_sha1("HEAD", sha1)) {
diff --git a/sequencer.h b/sequencer.h
index 905d2950..f435fdb4 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -13,7 +13,7 @@
*
* With the aggressive flag, it additionally removes SEQ_OLD_DIR,
* ignoring any errors. Inteded to be used by the sequencer's
- * '--reset' subcommand.
+ * '--quit' subcommand.
*/
void remove_sequencer_state(int aggressive);
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 3bca2b3d..913435e2 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -13,7 +13,7 @@ test_description='Test cherry-pick continuation features
. ./test-lib.sh
pristine_detach () {
- git cherry-pick --reset &&
+ git cherry-pick --quit &&
git checkout -f "$1^0" &&
git read-tree -u --reset HEAD &&
git clean -d -f -f -q -x
@@ -70,15 +70,15 @@ test_expect_success 'cherry-pick cleans up sequencer state upon success' '
test_path_is_missing .git/sequencer
'
-test_expect_success '--reset does not complain when no cherry-pick is in progress' '
+test_expect_success '--quit does not complain when no cherry-pick is in progress' '
pristine_detach initial &&
- git cherry-pick --reset
+ git cherry-pick --quit
'
-test_expect_success '--reset cleans up sequencer state' '
+test_expect_success '--quit cleans up sequencer state' '
pristine_detach initial &&
test_must_fail git cherry-pick base..picked &&
- git cherry-pick --reset &&
+ git cherry-pick --quit &&
test_path_is_missing .git/sequencer
'
diff --git a/t/t7106-reset-sequence.sh b/t/t7106-reset-sequence.sh
index 4956caaf..3f86e8c5 100755
--- a/t/t7106-reset-sequence.sh
+++ b/t/t7106-reset-sequence.sh
@@ -12,7 +12,7 @@ test_description='Test interaction of reset --hard with sequencer
. ./test-lib.sh
pristine_detach () {
- git cherry-pick --reset &&
+ git cherry-pick --quit &&
git checkout -f "$1^0" &&
git read-tree -u --reset HEAD &&
git clean -d -f -f -q -x
--
1.7.8.rc3
^ permalink raw reply related
* [RFC/PATCH 0/3] Re: cherry-pick/revert error messages
From: Jonathan Nieder @ 2011-11-20 9:46 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: git, Christian Couder, Martin von Zweigbergk, Phil Hord
In-Reply-To: <CALkWK0=45OwcBoH2TorsgwTbaXjnffVuh0mGxh2+ShN9cuF-=A@mail.gmail.com>
Ramkumar Ramachandra wrote:
> Looks much better! Yes, a series pretty'fying error messages would be
> really nice.
Good to hear. Here's a rough one.
I realize patch 1/3 might not be conservative enough, even though
there hasn't been much time for people to get used to --reset yet. So
I might be sending a second version that treats --reset as a synonym.
But please forget I said that when judging this version (i.e., if it
looks like a problem, I do want to know).
Thoughts?
Jonathan Nieder (3):
revert: rename --reset option to --quit
revert: restructure pick_revisions() for clarity
revert: improve error message for cherry-pick during cherry-pick
Documentation/git-cherry-pick.txt | 2 +-
Documentation/git-revert.txt | 2 +-
Documentation/sequencer.txt | 10 +++---
builtin/revert.c | 71 ++++++++++++++++++-------------------
sequencer.h | 2 +-
t/t3510-cherry-pick-sequence.sh | 10 +++---
t/t7106-reset-sequence.sh | 2 +-
7 files changed, 49 insertions(+), 50 deletions(-)
^ permalink raw reply
* Re: [msysGit] Re: [PATCH 1/2] MSVC: Do not close stdout to prevent a crash
From: Johannes Sixt @ 2011-11-20 9:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: Andreas Schwab, Vincent van Ravesteijn, git, msysgit, kusmabite,
Johannes.Schindelin
In-Reply-To: <7vpqgniid5.fsf@alter.siamese.dyndns.org>
Am 20.11.2011 04:27, schrieb Junio C Hamano:
> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> We have relied on fstat(-1, &st) to correctly error out, and if MSVC build
>>> crashes, it is a bug in its fstat() emulation, I would think.
>>
>> fileno(stdout) is alread wrong if stdout was closed.
>
> The "-1" in my message comes from here:
>
> DESCRIPTION
>
> The fileno() function shall return the integer file descriptor
> associated with the stream pointed to by stream.
>
> RETURN VALUE
>
> Upon successful completion, fileno() shall return the integer value of
> the file descriptor associated with stream. Otherwise, the value -1
> shall be returned and errno set to indicate the error.
But in the description of fclose() there is also:
After the call to fclose(), any use of stream results in undefined
behavior.
And we do call fclose(stdout) in cmd_format_patch.
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/2] MSVC: Do not close stdout to prevent a crash
From: Andreas Schwab @ 2011-11-20 9:05 UTC (permalink / raw)
To: Junio C Hamano
Cc: Vincent van Ravesteijn, git, msysgit, kusmabite,
Johannes.Schindelin
In-Reply-To: <7vpqgniid5.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> We have relied on fstat(-1, &st) to correctly error out, and if MSVC build
>>> crashes, it is a bug in its fstat() emulation, I would think.
>>
>> fileno(stdout) is alread wrong if stdout was closed.
>
> The "-1" in my message comes from here:
>
> DESCRIPTION
>
> The fileno() function shall return the integer file descriptor
> associated with the stream pointed to by stream.
After fclose(stdout) the stream does not exist any more. Thus the use
of fileno(stdout) is undefined, in fact *any* use of stdout is undefined
afterwards.
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: cherry-pick/revert error messages
From: Ramkumar Ramachandra @ 2011-11-20 8:02 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Christian Couder, Martin von Zweigbergk
In-Reply-To: <20111120073059.GA2278@elie.hsd1.il.comcast.net>
Hi Jonathan,
Jonathan Nieder wrote:
> Today I encountered the following error message:
> [...]
> I guess I would have expected something
> like the following instead:
>
> $ git cherry-pick foo..bar
> fatal: a cherry-pick or revert is already in progress
> hint: try "git cherry-pick (--continue | --quit)"
> $
>
> What do you think?
Looks much better! Yes, a series pretty'fying error messages would be
really nice. I'm busy with exams this week, and "New sequencer
workflow! v2" has been pending for some time -- would you like to
handle this bit?
Thanks.
-- Ram
^ permalink raw reply
* cherry-pick/revert error messages
From: Jonathan Nieder @ 2011-11-20 7:30 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git, Christian Couder, Martin von Zweigbergk
Hi Ram,
Today I encountered the following error message:
$ git cherry-pick foo..bar
error: .git/sequencer already exists.
error: A cherry-pick or revert is in progress.
hint: Use --continue to continue the operation
hint: or --reset to forget about it
fatal: cherry-pick failed
$
The double "error" seemed a little weird. Also, the capital letters
and periods feel out of place, when compared with other messages
emited by git with an "error:" prefix. The final "fatal: cherry-pick
failed" seems redundant, too. I guess I would have expected something
like the following instead:
$ git cherry-pick foo..bar
fatal: a cherry-pick or revert is already in progress
hint: try "git cherry-pick (--continue | --quit)"
$
What do you think?
^ permalink raw reply
* Re: [PATCH 1/2] MSVC: Do not close stdout to prevent a crash
From: Junio C Hamano @ 2011-11-20 3:27 UTC (permalink / raw)
To: Andreas Schwab
Cc: Vincent van Ravesteijn, git, msysgit, kusmabite,
Johannes.Schindelin
In-Reply-To: <m2obw7dg1e.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> We have relied on fstat(-1, &st) to correctly error out, and if MSVC build
>> crashes, it is a bug in its fstat() emulation, I would think.
>
> fileno(stdout) is alread wrong if stdout was closed.
The "-1" in my message comes from here:
DESCRIPTION
The fileno() function shall return the integer file descriptor
associated with the stream pointed to by stream.
RETURN VALUE
Upon successful completion, fileno() shall return the integer value of
the file descriptor associated with stream. Otherwise, the value -1
shall be returned and errno set to indicate the error.
^ permalink raw reply
* Re: Stack overflow at write_one()
From: Shawn Pearce @ 2011-11-20 2:00 UTC (permalink / raw)
To: Cesar Eduardo Barros; +Cc: Junio C Hamano, git
In-Reply-To: <4EC8437C.6000808@cesarb.net>
On Sat, Nov 19, 2011 at 16:02, Cesar Eduardo Barros <cesarb@cesarb.net> wrote:
> Em 19-11-2011 21:30, Shawn Pearce escreveu:
>>
>> On Sat, Nov 19, 2011 at 13:46, Cesar Eduardo Barros<cesarb@cesarb.net>
>> wrote:
>>>
>>> Em 19-11-2011 19:08, Junio C Hamano escreveu:
>>>>
>>>> Already found the real cause (jGit bug) and workaround posted, I think.
>>>
>>> I presume the cause then is what was fixed by
>>>
>>> http://egit.eclipse.org/w/?p=jgit.git;a=commit;h=2fbf296fda205446eac11a13abd4fcdb182f28d9
>>> ?
>>
>> Yes. The AOSP servers were all updated with the above JGit patch, so
>> the servers are no longer sending duplicate objects. But yes, for a
>> period of time there were duplicates in the kernel repositories,
>> particularly kernal/omap.
>
> So, would an alternative workaround in my situation be to delete
> kernel/omap.git and let repo sync recreate it? It seems repo does not have
> extra metadata anywhere else, so just removing the directory should be
> enough for it to clone again from scratch, hopefully getting a corrected
> pack from the server.
Yes. repo does not have extra state, so just removing the directory
and running `repo sync` again to clone the repository would correct
the local repository.
^ permalink raw reply
* Re: Stack overflow at write_one()
From: Cesar Eduardo Barros @ 2011-11-20 0:02 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <CAJo=hJv2aGEFcMjTPxJsyLerqUn3w3hc3hWnc1ScaDrSGihzyQ@mail.gmail.com>
Em 19-11-2011 21:30, Shawn Pearce escreveu:
> On Sat, Nov 19, 2011 at 13:46, Cesar Eduardo Barros<cesarb@cesarb.net> wrote:
>> Em 19-11-2011 19:08, Junio C Hamano escreveu:
>>>
>>> Already found the real cause (jGit bug) and workaround posted, I think.
>>
>> I presume the cause then is what was fixed by
>> http://egit.eclipse.org/w/?p=jgit.git;a=commit;h=2fbf296fda205446eac11a13abd4fcdb182f28d9
>> ?
>
> Yes. The AOSP servers were all updated with the above JGit patch, so
> the servers are no longer sending duplicate objects. But yes, for a
> period of time there were duplicates in the kernel repositories,
> particularly kernal/omap.
So, would an alternative workaround in my situation be to delete
kernel/omap.git and let repo sync recreate it? It seems repo does not
have extra metadata anywhere else, so just removing the directory should
be enough for it to clone again from scratch, hopefully getting a
corrected pack from the server.
--
Cesar Eduardo Barros
cesarb@cesarb.net
cesar.barros@gmail.com
^ permalink raw reply
* Re: Stack overflow at write_one()
From: Shawn Pearce @ 2011-11-19 23:30 UTC (permalink / raw)
To: Cesar Eduardo Barros; +Cc: Junio C Hamano, git
In-Reply-To: <4EC823A0.3010603@cesarb.net>
On Sat, Nov 19, 2011 at 13:46, Cesar Eduardo Barros <cesarb@cesarb.net> wrote:
> Em 19-11-2011 19:08, Junio C Hamano escreveu:
>>
>> Already found the real cause (jGit bug) and workaround posted, I think.
>
> I presume the cause then is what was fixed by
> http://egit.eclipse.org/w/?p=jgit.git;a=commit;h=2fbf296fda205446eac11a13abd4fcdb182f28d9
> ?
Yes. The AOSP servers were all updated with the above JGit patch, so
the servers are no longer sending duplicate objects. But yes, for a
period of time there were duplicates in the kernel repositories,
particularly kernal/omap.
^ permalink raw reply
* Re: Stack overflow at write_one()
From: Cesar Eduardo Barros @ 2011-11-19 21:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vty5zizwn.fsf@alter.siamese.dyndns.org>
Em 19-11-2011 19:08, Junio C Hamano escreveu:
> Already found the real cause (jGit bug) and workaround posted, I think.
I presume the cause then is what was fixed by
http://egit.eclipse.org/w/?p=jgit.git;a=commit;h=2fbf296fda205446eac11a13abd4fcdb182f28d9
?
> See $gmane/185573
That did it, thanks! The patch had an offset, a fuzz, and a reject, but
it was easy to fix by hand.
$ ../git/git gc
Counting objects: 30254, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (6614/6614), done.
warning: recursive delta detected for object
fac71dfa0fe8c70cc852099e061c334e2a548eab
warning: recursive delta detected for object
1b730f5b2e0bdb2a2206af8ed30170509e75a2f5
warning: recursive delta detected for object
2f25a87e67fa3a226e367b9e080f11aa90c9f953
warning: recursive delta detected for object
d5e5eefac91788da9a94efe9a15e0b928a77489e
Writing objects: 100% (30254/30254), done.
Total 30254 (delta 24008), reused 28803 (delta 23266)
And after that the repack does not break anymore:
$ ../git/git gc
Counting objects: 30254, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (5876/5876), done.
Writing objects: 100% (30254/30254), done.
Total 30254 (delta 24008), reused 30254 (delta 24008)
--
Cesar Eduardo Barros
cesarb@cesarb.net
cesar.barros@gmail.com
^ permalink raw reply
* Re: Stack overflow at write_one()
From: Junio C Hamano @ 2011-11-19 21:08 UTC (permalink / raw)
To: Cesar Eduardo Barros; +Cc: git
In-Reply-To: <4EC81131.2010704@cesarb.net>
Already found the real cause (jGit bug) and workaround posted, I think.
See $gmane/185573
^ permalink raw reply
* Re: [PATCH 1/2] MSVC: Do not close stdout to prevent a crash
From: Vincent van Ravesteijn @ 2011-11-19 20:52 UTC (permalink / raw)
To: Junio C Hamano
Cc: Andreas Schwab, git, msysgit, kusmabite, Johannes.Schindelin
In-Reply-To: <7v39dkj5ad.fsf@alter.siamese.dyndns.org>
Op 19-11-2011 20:11, Junio C Hamano schreef:
> ... This happens for 'format-patch' which closes stdout after a call to
> freopen which directs stdout to the format patch file.
>> It shouldn't do that in the first place. This is an error on any
>> platform.
> Correct. The clean-up codepath is for built-in command implementations
> that write out their result and return 0 to signal success. If we let the
> crt0 to run its usual clean-ups like closing the standard output stream,
> we wouldn't be able to catch errors from there.
>
> For built-ins that perform their own clean-ups, it is their responsibility
> to be careful, hence we skip this part of the code.
>
> We have relied on fstat(-1,&st) to correctly error out, and if MSVC build
> crashes, it is a bug in its fstat() emulation, I would think.
This doesn't work because fileno(stdout) returns 1 no matter whether we
closed stdout or not.
My reasoning for the proposed patch is that we call freopen(..., stdout)
for each patch file to write, but we only call fclose(stdout) once. Sp,
I concluded that it is not necessary to close stdout and that we just
need to redirect any new output to stdout.
Vincent
^ permalink raw reply
* Stack overflow at write_one()
From: Cesar Eduardo Barros @ 2011-11-19 20:27 UTC (permalink / raw)
To: git
I have found a stack overflow at builtin/pack-objects.c:write_one(),
where it calls itself endlessly. This is caused by the object_entry e
and e->delta->delta being the same. But I have no idea how that happened.
First, the full story:
I used Google's repo tool to mirror AOSP to my machine. This mirrors
several kernel trees (six last time I counted), without sharing objects
one with another. To save space, I decided to point their
objects/info/alternates to my mirror of the Linus kernel tree (which
should be safe, since Linus makes it always fast-forward), and run "git
gc" on them to create a smaller pack. This worked for all trees except
one, where it core dumped (abrt report at
https://bugzilla.redhat.com/show_bug.cgi?id=755132).
I compiled the latest git (v1.7.8-rc3-17-gf56ef11) to see if it still
happened, and here is what I could get from gdb. I attached to the
pack-objects process before it crashed (full command line "git
pack-objects --keep-true-parents --honor-pack-keep --non-empty --all
--reflog --unpack-unreachable --local --delta-base-offset
/home/cesarb/src/bug755132/omap.git/objects/pack/.tmp-5171-pack"),
continued, and let it crash:
(gdb) cont
Continuing.
[New Thread 0x7f3f2bad3700 (LWP 5205)]
[New Thread 0x7f3f2b2d2700 (LWP 5206)]
[New Thread 0x7f3f2aad1700 (LWP 5207)]
[New Thread 0x7f3f2a2d0700 (LWP 5208)]
[Thread 0x7f3f2b2d2700 (LWP 5206) exited]
[Thread 0x7f3f2bad3700 (LWP 5205) exited]
[Thread 0x7f3f2aad1700 (LWP 5207) exited]
[Thread 0x7f3f2a2d0700 (LWP 5208) exited]
Program received signal SIGSEGV, Segmentation fault.
0x00000000004472b9 in write_one (f=0x6a97db0, e=0x7f3f30233490,
offset=0x7fff79b53908) at builtin/pack-objects.c:415
415 {
Unlike on Fedora's git binary, where it happened on a call instruction,
this time it happened on a push instruction:
(gdb) disassemble
Dump of assembler code for function write_one:
0x00000000004472b0 <+0>: push %r15
0x00000000004472b2 <+2>: push %r14
0x00000000004472b4 <+4>: push %r13
0x00000000004472b6 <+6>: mov %rdx,%r13
=> 0x00000000004472b9 <+9>: push %r12
0x00000000004472bb <+11>: mov %rdi,%r12
The last few frames on the stack show the endless recursion:
(gdb) where
#0 0x00000000004472b9 in write_one (f=0x6a97db0, e=0x7f3f30233490,
offset=0x7fff79b53908) at builtin/pack-objects.c:415
#1 0x00000000004472ed in write_one (f=0x6a97db0, e=0x7f3f30277390,
offset=0x7fff79b53908) at builtin/pack-objects.c:423
#2 0x00000000004472ed in write_one (f=0x6a97db0, e=0x7f3f30233490,
offset=0x7fff79b53908) at builtin/pack-objects.c:423
#3 0x00000000004472ed in write_one (f=0x6a97db0, e=0x7f3f30277390,
offset=0x7fff79b53908) at builtin/pack-objects.c:423
#4 0x00000000004472ed in write_one (f=0x6a97db0, e=0x7f3f30233490,
offset=0x7fff79b53908) at builtin/pack-objects.c:423
And here is the loop in the data structures:
(gdb) p e
$1 = (struct object_entry *) 0x7f3f30233490
(gdb) p e->delta
$2 = (struct object_entry *) 0x7f3f30277390
(gdb) p e->delta->delta
$3 = (struct object_entry *) 0x7f3f30233490
Unfortunately, I do not know enough of git's internals to debug further.
In case it helps, here is the contents of a few of these structures:
(gdb) p *e
$4 = {idx = {
sha1 = "\257>J\241)\266\023\064\a\342J\320\375ӆ\262M\245",
<incomplete sequence \356>, crc32 = 0, offset = 0}, size = 20, in_pack =
0x259b610,
in_pack_offset = 231061238, delta = 0x7f3f30277390,
delta_child = 0x7f3f30277390, delta_sibling = 0x7f3f30413b10,
delta_data = 0x0, delta_size = 20, z_delta_size = 0, hash = 2099915708,
type = OBJ_OFS_DELTA, in_pack_type = OBJ_OFS_DELTA,
in_pack_header_size = 5 '\005', preferred_base = 0 '\000',
no_try_delta = 0 '\000', tagged = 0 '\000', filled = 1 '\001'}
(gdb) p *(e->delta)
$5 = {idx = {
sha1 =
"\372\307\035\372\017\350\307\f\310R\t\236\006\034\063N*T\216\253",
crc32 = 0, offset = 0}, size = 14, in_pack = 0x259b610,
in_pack_offset = 39990, delta = 0x7f3f30233490,
delta_child = 0x7f3f30233490, delta_sibling = 0x0, delta_data = 0x0,
delta_size = 14, z_delta_size = 0, hash = 2099915708, type =
OBJ_REF_DELTA,
in_pack_type = OBJ_REF_DELTA, in_pack_header_size = 21 '\025',
preferred_base = 0 '\000', no_try_delta = 0 '\000', tagged = 0 '\000',
filled = 1 '\001'}
(gdb) p *(e->in_pack)
$6 = {next = 0x25a53c0, windows = 0x259bc40, pack_size = 449155894,
index_data = 0x7f3f4f0a9000, index_size = 58351420, num_objects =
2083941,
num_bad_objects = 0, bad_object_sha1 = 0x0, index_version = 2,
mtime = 1321387261, pack_fd = -1, pack_local = 1, pack_keep = 0,
do_not_close = 0, sha1 =
"\371Q4\177.ȳv\364\246\332Z\234\025?\352ݠP\210",
pack_name = 0x259b671
"/home/cesarb/src/bug755132/omap.git/objects/pack/pack-f951347f2ec8b376f4a6da5a9c153feadda05088.pack"}
I tried using "git fsck" to see if it could find anything strange, but
it seems to get stuck (using 100% CPU) after these lines:
[...]
Checking commit fb630b9fc902e24209166b1659a8b375bf38099c
Checking tree fc32c012c750084eb1d82782cee7c80a45a78289
Checking blob fc7bbba585cee2c2b0d5282c42fb986bfb032a0a
Checking commit fdcb23634c9b6649bb02c681033d4973491b0e35
Checking tree fe773cf73ff553249be2f24ddf770f5dc43a41f1
Checking blob fe67b5c79f0ff33d92ebe7469a89c5a5d044fc0a
Checking blob fe73276e026bf263f494a917c84c6a3fcaeaaeda
Checking tree fe30eda9d92d074816f9c3a47fd3ffb9b89ca835
Checking tree fe9c75396e6d433b289d0e40c7e47921b91cad3a
Checking blob ff3ed6086ce1c6b6b4b5111c034d14a208c0d045
Checking blob ff66638ff54d5ad7067e4f246d392059eef1a7bf
Checking tree ff126d2bc67017199049ddba761979f3bda57eb9
Unfortunately, the reproducer I have (a copy of both trees with
objects/info/alternates modified) is 1.8G in size, and I do not know how
to create a smaller reproducer. If you know of a command which would get
more relevant information from them, just ask; I plan on keeping them
around for a while.
--
Cesar Eduardo Barros
cesarb@cesarb.net
cesar.barros@gmail.com
^ permalink raw reply
* Re: t/t1304: avoid -d option to setfacl (db82657)
From: Scott Parrish @ 2011-11-19 20:28 UTC (permalink / raw)
To: Brandon Casey; +Cc: wsp, Git Mailing List
In-Reply-To: <CA+sFfMc6BCPvk1FCjZwyY_pWqXBut3D--OfrFEcfK5p97D-KHQ@mail.gmail.com>
Hi Brandon,
Thanks for posting my question. I realized shortly after my post to github that I should have posted to the mailing list instead.
On Nov 18, 2011, at 12:37 AM, Brandon Casey wrote:
> [cc git@vger.kernel.org since that is the appropriate place for this discusion]
>
> On Thu, Nov 17, 2011 at 8:59 PM, wsp
> <reply+c-729354-3460aca0fa61e627f9d1a271cf70a99d5c1e7e4e-921167@reply.github.com>
> wrote:
>> Could this test case be reviewed again? It fails on FreeBSD where the appropriate way to specify default ACL's is with the "-d" option. The "d[efault]:" syntax is invalid on FreeBSD.
>
> Well, I'm not sure there is a right answer here.
>
> Linux accepts -d or "d[efault]"
> Solaris only accepts "d[efault]"
> FreeBSD only accepts -d
> a quick search shows z/OS only accepts "d[efault]"
> other?
>
> I think most everything else rolls their own implementation into chmod
> or chacl or something like that.
OS X does indeed roll ACL management into chmod. The test passes there because the prereq isn't met (i.e. the setfacl command does not exist) and thus the tests are skipped.
>
> The abandoned POSIX draft does actually specify the FreeBSD behavior.
>
> So I think it's kind of a toss-up. Which option we choose should
> probably depend on whether we get more test coverage by using the
> "d[efault]" notation or by using the -d option. That depends on
> whether there are more Solaris users compiling git or whether there
> are more FreeBSD users. I don't know the answer to that either. I
> tend to think there are very few of either.
Is there a reason conditional logic can't be used (perhaps keying off of `uname -s` or the like) so that we have coverage in all cases?
-S
>
> -Brandon
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Possible bug with branch names and case sensitivity
From: Gerd Knops @ 2011-11-19 20:08 UTC (permalink / raw)
To: git
Hi All,
On Mac OS X with a case-insensitive file system (not sure if that matters) git get's confused with branch names that differ only in case. Here is what happened as far as I can reconstruct:
While in a branch named "foundry" I accidentally
git checkout Crucible
instead of "crucible". This appears to have made a local branch "Crucible" of the remote tracking "crucible" branch (had I done this on purpose, I would have expected "Crucible" to be a branch of "foundry" instead of a branch of "crucible").
I made some changes, committed, and pushed, only to be puzzled that no changes were pushed upstream.
At this point "git branch -a" showed:
* Crucible
foundry
master
remotes/origin/DAExceptions
remotes/origin/HEAD -> origin/master
remotes/origin/centerSectionOptimizer
remotes/origin/crucible
remotes/origin/foundry
remotes/origin/ipad
remotes/origin/master
So naturally I proceeded with
git checkout crucible
git merge Crucible
only to see "Already up-to-date."
Not sure if any of this is expected behavior, but to me it didn't feel like it.
Thanks
Gerd
PS: here is how I "fixed" this:
git checkout Crucible
git reset --soft HEAD^
git stash
git stash apply
added, committed, pushed. BTW now "git branch -a" shows:
* crucible
foundry
master
remotes/origin/DAExceptions
remotes/origin/HEAD -> origin/master
remotes/origin/centerSectionOptimizer
remotes/origin/crucible
remotes/origin/foundry
remotes/origin/ipad
remotes/origin/master
No trace of the "Crucible" branch.
^ permalink raw reply
* Re: [PATCH 1/2] MSVC: Do not close stdout to prevent a crash
From: Andreas Schwab @ 2011-11-19 20:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Vincent van Ravesteijn, git, msysgit, kusmabite,
Johannes.Schindelin
In-Reply-To: <7v39dkj5ad.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> We have relied on fstat(-1, &st) to correctly error out, and if MSVC build
> crashes, it is a bug in its fstat() emulation, I would think.
fileno(stdout) is alread wrong if stdout was closed.
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: [msysGit] [PATCH 1/2] MSVC: Do not close stdout to prevent a crash
From: Johannes Sixt @ 2011-11-19 20:11 UTC (permalink / raw)
To: Vincent van Ravesteijn
Cc: git, msysgit, gitster, kusmabite, Johannes.Schindelin
In-Reply-To: <1321710345-2299-1-git-send-email-vfr@lyx.org>
Am 19.11.2011 14:45, schrieb Vincent van Ravesteijn:
> When compiled with MSVC, git crashes on Windows when calling
> fstat(stdout) when stdout is closed. fstat is being called at the end of
> run_builtin and this will thus be a problem for builtin command that close
> stdout. This happens for 'format-patch' which closes stdout after a call to
> freopen which directs stdout to the format patch file.
This crash happens because of the some drainbramage in the MS's newer C
runtime: Its functions never return EINVAL (like fstat should in this
case), but it is assumed that cases where EINVAL should be returned are
programming errors, and the application is redirected, via an "invalid
agument handler" to abort() instead. (It is advertized as a security
feature.)
> To prevent the crash and to prevent git from writing cruft into the patch
> file, we do not close stdout, but redirect it to "nul" instead.
A more robust solution is to add invalidcontinue.obj to the linker
command line. This installs an invalid argument handler that does not
abort, and restores a sane behavior.
-- Hannes
^ permalink raw reply
* [PATCH/RFC 3/3] git-submodule.sh: Don't use $path variable in eval_gettext string
From: Ramsay Jones @ 2011-11-19 19:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list, avarab, Jens.Lehmann, Johannes Sixt
The eval_gettext (and eval_gettextln) i18n shell functions call
git-sh-i18n--envsubst to process the variable references in the
string parameter. Unfortunately, environment variables are case
insensitive on windows, which leads to failure when eval_gettext
exports $path.
Commit df599e9 (Windows: teach getenv to do a case-sensitive search,
06-06-2011) solved this problem on MinGW by overriding the system
getenv() function to allow git-sh-i18n--envsubst to read $path
rather than $PATH from the environment.
As an alternative, we finesse the problem by renaming the $path
variable to $sm_path (submodule path). We note that the foreach
subcommand provides $path to user scripts, so we can't simply
rename it to $sm_path.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Note that, on cygwin, this fixes t7400-*.sh, t7407-*.sh and t7407-*.sh
(and maybe t7408-*.sh; I didn't get that far ;-).
Note that the first hunk has an independent fix which could be split
out into a separate patch.
This is marked RFC because the approach taken by commit df599e9 may
be the preferred solution; I just wanted to try a different approach
since t7400-submodue-basic.sh fails for me on MinGW (in a way that
seems to imply that $PATH has been corrupted; *but* I haven't spent
any time investigating it).
ATB,
Ramsay Jones
git-submodule.sh | 161 +++++++++++++++++++++++++++--------------------------
1 files changed, 82 insertions(+), 79 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 3adab93..1acd626 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -101,11 +101,12 @@ module_list()
module_name()
{
# Do we have "submodule.<something>.path = $1" defined in .gitmodules file?
+ sm_path="$1"
re=$(printf '%s\n' "$1" | sed -e 's/[].[^$\\*]/\\&/g')
name=$( git config -f .gitmodules --get-regexp '^submodule\..*\.path$' |
sed -n -e 's|^submodule\.\(.*\)\.path '"$re"'$|\1|p' )
test -z "$name" &&
- die "$(eval_gettext "No submodule mapping found in .gitmodules for path '\$path'")"
+ die "$(eval_gettext "No submodule mapping found in .gitmodules for path '\$sm_path'")"
echo "$name"
}
@@ -119,7 +120,7 @@ module_name()
#
module_clone()
{
- path=$1
+ sm_path=$1
url=$2
reference="$3"
quiet=
@@ -130,12 +131,12 @@ module_clone()
gitdir=
gitdir_base=
- name=$(module_name "$path" 2>/dev/null)
- base_path=$(dirname "$path")
+ name=$(module_name "$sm_path" 2>/dev/null)
+ base_path=$(dirname "$sm_path")
gitdir=$(git rev-parse --git-dir)
gitdir_base="$gitdir/modules/$base_path"
- gitdir="$gitdir/modules/$path"
+ gitdir="$gitdir/modules/$sm_path"
case $gitdir in
/*)
@@ -158,18 +159,18 @@ module_clone()
if test -d "$gitdir"
then
- mkdir -p "$path"
- echo "gitdir: $rel_gitdir" >"$path/.git"
+ mkdir -p "$sm_path"
+ echo "gitdir: $rel_gitdir" >"$sm_path/.git"
rm -f "$gitdir/index"
else
mkdir -p "$gitdir_base"
if test -n "$reference"
then
- git-clone $quiet "$reference" -n "$url" "$path" --separate-git-dir "$gitdir"
+ git-clone $quiet "$reference" -n "$url" "$sm_path" --separate-git-dir "$gitdir"
else
- git-clone $quiet -n "$url" "$path" --separate-git-dir "$gitdir"
+ git-clone $quiet -n "$url" "$sm_path" --separate-git-dir "$gitdir"
fi ||
- die "$(eval_gettext "Clone of '\$url' into submodule path '\$path' failed")"
+ die "$(eval_gettext "Clone of '\$url' into submodule path '\$sm_path' failed")"
fi
}
@@ -221,14 +222,14 @@ cmd_add()
done
repo=$1
- path=$2
+ sm_path=$2
- if test -z "$path"; then
- path=$(echo "$repo" |
+ if test -z "$sm_path"; then
+ sm_path=$(echo "$repo" |
sed -e 's|/$||' -e 's|:*/*\.git$||' -e 's|.*[/:]||g')
fi
- if test -z "$repo" -o -z "$path"; then
+ if test -z "$repo" -o -z "$sm_path"; then
usage
fi
@@ -249,7 +250,7 @@ cmd_add()
# normalize path:
# multiple //; leading ./; /./; /../; trailing /
- path=$(printf '%s/\n' "$path" |
+ sm_path=$(printf '%s/\n' "$sm_path" |
sed -e '
s|//*|/|g
s|^\(\./\)*||
@@ -259,49 +260,49 @@ cmd_add()
tstart
s|/*$||
')
- git ls-files --error-unmatch "$path" > /dev/null 2>&1 &&
- die "$(eval_gettext "'\$path' already exists in the index")"
+ git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 &&
+ die "$(eval_gettext "'\$sm_path' already exists in the index")"
- if test -z "$force" && ! git add --dry-run --ignore-missing "$path" > /dev/null 2>&1
+ if test -z "$force" && ! git add --dry-run --ignore-missing "$sm_path" > /dev/null 2>&1
then
eval_gettextln "The following path is ignored by one of your .gitignore files:
-\$path
+\$sm_path
Use -f if you really want to add it." >&2
exit 1
fi
# perhaps the path exists and is already a git repo, else clone it
- if test -e "$path"
+ if test -e "$sm_path"
then
- if test -d "$path"/.git -o -f "$path"/.git
+ if test -d "$sm_path"/.git -o -f "$sm_path"/.git
then
- eval_gettextln "Adding existing repo at '\$path' to the index"
+ eval_gettextln "Adding existing repo at '\$sm_path' to the index"
else
- die "$(eval_gettext "'\$path' already exists and is not a valid git repo")"
+ die "$(eval_gettext "'\$sm_path' already exists and is not a valid git repo")"
fi
else
- module_clone "$path" "$realrepo" "$reference" || exit
+ module_clone "$sm_path" "$realrepo" "$reference" || exit
(
clear_local_git_env
- cd "$path" &&
+ cd "$sm_path" &&
# ash fails to wordsplit ${branch:+-b "$branch"...}
case "$branch" in
'') git checkout -f -q ;;
?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
esac
- ) || die "$(eval_gettext "Unable to checkout submodule '\$path'")"
+ ) || die "$(eval_gettext "Unable to checkout submodule '\$sm_path'")"
fi
- git config submodule."$path".url "$realrepo"
+ git config submodule."$sm_path".url "$realrepo"
- git add $force "$path" ||
- die "$(eval_gettext "Failed to add submodule '\$path'")"
+ git add $force "$sm_path" ||
+ die "$(eval_gettext "Failed to add submodule '\$sm_path'")"
- git config -f .gitmodules submodule."$path".path "$path" &&
- git config -f .gitmodules submodule."$path".url "$repo" &&
+ git config -f .gitmodules submodule."$sm_path".path "$sm_path" &&
+ git config -f .gitmodules submodule."$sm_path".url "$repo" &&
git add --force .gitmodules ||
- die "$(eval_gettext "Failed to register submodule '\$path'")"
+ die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
}
#
@@ -339,23 +340,25 @@ cmd_foreach()
exec 3<&0
module_list |
- while read mode sha1 stage path
+ while read mode sha1 stage sm_path
do
- if test -e "$path"/.git
+ if test -e "$sm_path"/.git
then
- say "$(eval_gettext "Entering '\$prefix\$path'")"
- name=$(module_name "$path")
+ say "$(eval_gettext "Entering '\$prefix\$sm_path'")"
+ name=$(module_name "$sm_path")
(
- prefix="$prefix$path/"
+ prefix="$prefix$sm_path/"
clear_local_git_env
- cd "$path" &&
+ # we make $path available to scripts ...
+ path=$sm_path
+ cd "$sm_path" &&
eval "$@" &&
if test -n "$recursive"
then
cmd_foreach "--recursive" "$@"
fi
) <&3 3<&- ||
- die "$(eval_gettext "Stopping at '\$path'; script returned non-zero status.")"
+ die "$(eval_gettext "Stopping at '\$sm_path'; script returned non-zero status.")"
fi
done
}
@@ -389,15 +392,15 @@ cmd_init()
done
module_list "$@" |
- while read mode sha1 stage path
+ while read mode sha1 stage sm_path
do
# Skip already registered paths
- name=$(module_name "$path") || exit
+ name=$(module_name "$sm_path") || exit
if test -z "$(git config "submodule.$name.url")"
then
url=$(git config -f .gitmodules submodule."$name".url)
test -z "$url" &&
- die "$(eval_gettext "No url found for submodule path '\$path' in .gitmodules")"
+ die "$(eval_gettext "No url found for submodule path '\$sm_path' in .gitmodules")"
# Possibly a url relative to parent
case "$url" in
@@ -406,7 +409,7 @@ cmd_init()
;;
esac
git config submodule."$name".url "$url" ||
- die "$(eval_gettext "Failed to register url for submodule path '\$path'")"
+ die "$(eval_gettext "Failed to register url for submodule path '\$sm_path'")"
fi
# Copy "update" setting when it is not set yet
@@ -414,9 +417,9 @@ cmd_init()
test -z "$upd" ||
test -n "$(git config submodule."$name".update)" ||
git config submodule."$name".update "$upd" ||
- die "$(eval_gettext "Failed to register update mode for submodule path '\$path'")"
+ die "$(eval_gettext "Failed to register update mode for submodule path '\$sm_path'")"
- say "$(eval_gettext "Submodule '\$name' (\$url) registered for path '\$path'")"
+ say "$(eval_gettext "Submodule '\$name' (\$url) registered for path '\$sm_path'")"
done
}
@@ -488,14 +491,14 @@ cmd_update()
cloned_modules=
module_list "$@" | {
err=
- while read mode sha1 stage path
+ while read mode sha1 stage sm_path
do
if test "$stage" = U
then
- echo >&2 "Skipping unmerged submodule $path"
+ echo >&2 "Skipping unmerged submodule $sm_path"
continue
fi
- name=$(module_name "$path") || exit
+ name=$(module_name "$sm_path") || exit
url=$(git config submodule."$name".url)
if ! test -z "$update"
then
@@ -506,7 +509,7 @@ cmd_update()
if test "$update_module" = "none"
then
- echo "Skipping submodule '$path'"
+ echo "Skipping submodule '$sm_path'"
continue
fi
@@ -515,20 +518,20 @@ cmd_update()
# Only mention uninitialized submodules when its
# path have been specified
test "$#" != "0" &&
- say "$(eval_gettext "Submodule path '\$path' not initialized
+ say "$(eval_gettext "Submodule path '\$sm_path' not initialized
Maybe you want to use 'update --init'?")"
continue
fi
- if ! test -d "$path"/.git -o -f "$path"/.git
+ if ! test -d "$sm_path"/.git -o -f "$sm_path"/.git
then
- module_clone "$path" "$url" "$reference"|| exit
+ module_clone "$sm_path" "$url" "$reference"|| exit
cloned_modules="$cloned_modules;$name"
subsha1=
else
- subsha1=$(clear_local_git_env; cd "$path" &&
+ subsha1=$(clear_local_git_env; cd "$sm_path" &&
git rev-parse --verify HEAD) ||
- die "$(eval_gettext "Unable to find current revision in submodule path '\$path'")"
+ die "$(eval_gettext "Unable to find current revision in submodule path '\$sm_path'")"
fi
if test "$subsha1" != "$sha1"
@@ -544,10 +547,10 @@ Maybe you want to use 'update --init'?")"
then
# Run fetch only if $sha1 isn't present or it
# is not reachable from a ref.
- (clear_local_git_env; cd "$path" &&
+ (clear_local_git_env; cd "$sm_path" &&
( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
test -z "$rev") || git-fetch)) ||
- die "$(eval_gettext "Unable to fetch in submodule path '\$path'")"
+ die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
fi
# Is this something we just cloned?
@@ -561,24 +564,24 @@ Maybe you want to use 'update --init'?")"
case "$update_module" in
rebase)
command="git rebase"
- die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$path'")"
- say_msg="$(eval_gettext "Submodule path '\$path': rebased into '\$sha1'")"
+ die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")"
+ say_msg="$(eval_gettext "Submodule path '\$sm_path': rebased into '\$sha1'")"
must_die_on_failure=yes
;;
merge)
command="git merge"
- die_msg="$(eval_gettext "Unable to merge '\$sha1' in submodule path '\$path'")"
- say_msg="$(eval_gettext "Submodule path '\$path': merged in '\$sha1'")"
+ die_msg="$(eval_gettext "Unable to merge '\$sha1' in submodule path '\$sm_path'")"
+ say_msg="$(eval_gettext "Submodule path '\$sm_path': merged in '\$sha1'")"
must_die_on_failure=yes
;;
*)
command="git checkout $subforce -q"
- die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$path'")"
- say_msg="$(eval_gettext "Submodule path '\$path': checked out '\$sha1'")"
+ die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$sm_path'")"
+ say_msg="$(eval_gettext "Submodule path '\$sm_path': checked out '\$sha1'")"
;;
esac
- if (clear_local_git_env; cd "$path" && $command "$sha1")
+ if (clear_local_git_env; cd "$sm_path" && $command "$sha1")
then
say "$say_msg"
elif test -n "$must_die_on_failure"
@@ -592,11 +595,11 @@ Maybe you want to use 'update --init'?")"
if test -n "$recursive"
then
- (clear_local_git_env; cd "$path" && eval cmd_update "$orig_flags")
+ (clear_local_git_env; cd "$sm_path" && eval cmd_update "$orig_flags")
res=$?
if test $res -gt 0
then
- die_msg="$(eval_gettext "Failed to recurse into submodule path '\$path'")"
+ die_msg="$(eval_gettext "Failed to recurse into submodule path '\$sm_path'")"
if test $res -eq 1
then
err="${err};$die_msg"
@@ -883,30 +886,30 @@ cmd_status()
done
module_list "$@" |
- while read mode sha1 stage path
+ while read mode sha1 stage sm_path
do
- name=$(module_name "$path") || exit
+ name=$(module_name "$sm_path") || exit
url=$(git config submodule."$name".url)
- displaypath="$prefix$path"
+ displaypath="$prefix$sm_path"
if test "$stage" = U
then
say "U$sha1 $displaypath"
continue
fi
- if test -z "$url" || ! test -d "$path"/.git -o -f "$path"/.git
+ if test -z "$url" || ! test -d "$sm_path"/.git -o -f "$sm_path"/.git
then
say "-$sha1 $displaypath"
continue;
fi
- set_name_rev "$path" "$sha1"
- if git diff-files --ignore-submodules=dirty --quiet -- "$path"
+ set_name_rev "$sm_path" "$sha1"
+ if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path"
then
say " $sha1 $displaypath$revname"
else
if test -z "$cached"
then
- sha1=$(clear_local_git_env; cd "$path" && git rev-parse --verify HEAD)
- set_name_rev "$path" "$sha1"
+ sha1=$(clear_local_git_env; cd "$sm_path" && git rev-parse --verify HEAD)
+ set_name_rev "$sm_path" "$sha1"
fi
say "+$sha1 $displaypath$revname"
fi
@@ -916,10 +919,10 @@ cmd_status()
(
prefix="$displaypath/"
clear_local_git_env
- cd "$path" &&
+ cd "$sm_path" &&
eval cmd_status "$orig_args"
) ||
- die "$(eval_gettext "Failed to recurse into submodule path '\$path'")"
+ die "$(eval_gettext "Failed to recurse into submodule path '\$sm_path'")"
fi
done
}
@@ -951,9 +954,9 @@ cmd_sync()
done
cd_to_toplevel
module_list "$@" |
- while read mode sha1 stage path
+ while read mode sha1 stage sm_path
do
- name=$(module_name "$path")
+ name=$(module_name "$sm_path")
url=$(git config -f .gitmodules --get submodule."$name".url)
# Possibly a url relative to parent
@@ -968,11 +971,11 @@ cmd_sync()
say "$(eval_gettext "Synchronizing submodule url for '\$name'")"
git config submodule."$name".url "$url"
- if test -e "$path"/.git
+ if test -e "$sm_path"/.git
then
(
clear_local_git_env
- cd "$path"
+ cd "$sm_path"
remote=$(get_default_remote)
git config remote."$remote".url "$url"
)
--
1.7.7
^ permalink raw reply related
* [PATCH 2/3] config.c: Fix a static buffer overwrite bug by avoiding mkpath()
From: Ramsay Jones @ 2011-11-19 19:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list
On cygwin, test number 21 of t3200-branch.sh (git branch -m q q2
without config should succeed) fails. The failure involves the
functions from path.c which parcel out internal static buffers
from the git_path() and mkpath() functions.
In particular, the rename_ref() function calls safe_create_leading\
_directories() with a filename returned by git_path("logs/%s", ref).
safe_create_leading_directories(), in turn, calls stat() on each
element of the path it is given. On cygwin, this leads to a call
to git_config() for each component of the path, since this test
explicitly removes the config file. git_config() calls mkpath(), so
on the fourth component of the path, the original buffer passed
into the function is overwritten with the config filename.
Note that this bug is specific to cygwin and it's schizophrenic
stat() functions (see commits adbc0b6, 7faee6b and 7974843). The
lack of a config file and a path with at least four elements is
also important to trigger the bug.
In order to fix the problem, we replace the call to mkpath() with
a call to mksnpath() and provide our own buffer.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
config.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index edf9914..5a9ca84 100644
--- a/config.c
+++ b/config.c
@@ -851,6 +851,7 @@ int git_config_system(void)
int git_config_early(config_fn_t fn, void *data, const char *repo_config)
{
+ char buf[4096];
int ret = 0, found = 0;
const char *home = NULL;
@@ -865,12 +866,11 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
home = getenv("HOME");
if (home) {
- char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
+ char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
if (!access(user_config, R_OK)) {
ret += git_config_from_file(fn, user_config, data);
found += 1;
}
- free(user_config);
}
if (repo_config && !access(repo_config, R_OK)) {
--
1.7.7
^ permalink raw reply related
* [PATCH 1/3] t5501-*.sh: Fix url passed to clone in setup test
From: Ramsay Jones @ 2011-11-19 19:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list
In particular, the url passed to git-clone has an extra '/' given
after the 'file://' schema prefix, thus:
git clone --reference=original "file:///$(pwd)/original one
Once the prefix is removed, the remainder of the url looks something
like "//home/ramsay/git/t/...", which is then interpreted as an
network path. This then results in a "Permission denied" error, like
so:
ramsay $ ls //home
ls: cannot access //home: No such host or network path
ramsay $ ls //home/ramsay
ls: cannot access //home/ramsay: Permission denied
ramsay $
In order to fix the problem, we simply remove the extraneous '/'
character from the url.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
t/t5501-fetch-push-alternates.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t5501-fetch-push-alternates.sh b/t/t5501-fetch-push-alternates.sh
index b5ced84..1bc57ac 100755
--- a/t/t5501-fetch-push-alternates.sh
+++ b/t/t5501-fetch-push-alternates.sh
@@ -28,7 +28,7 @@ test_expect_success setup '
done
) &&
(
- git clone --reference=original "file:///$(pwd)/original" one &&
+ git clone --reference=original "file://$(pwd)/original" one &&
cd one &&
echo Z >count &&
git add count &&
--
1.7.7
^ permalink raw reply related
* [PATCH 0/3] test failures on v1.7.8-rc2
From: Ramsay Jones @ 2011-11-19 19:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list
Hi Junio,
The test-suite on cygwin has a number of failures. These patches fix
up most of the problems. I know v1.7.8-rc3 is out already, but it takes
2 hours 15 minutes to run the tests on cygwin (and that's without the svn
tests!) so I didn't want to rebase and re-test the patches tonight ... ;-)
[PATCH 1/3] t5501-*.sh: Fix url passed to clone in setup test
[PATCH 2/3] config.c: Fix a static buffer overwrite bug by avoiding mkpath()
[PATCH 3/3] git-submodule.sh: Don't use $path variable in eval_gettext string
Note that patch #3 is an RFC; see patch for more.
[with these patches applied, I have GIT_SKIP_TEST='t0061.3 t0070.3']
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
From: Ramsay Jones @ 2011-11-19 19:25 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Jonathan Nieder, Ramkumar Ramachandra, Junio C Hamano, Git List
In-Reply-To: <CACsJy8CYj_s92zG-LnBKtHxV2uaG8-rq-VNJiQYwNJXGKbFeDw@mail.gmail.com>
Nguyen Thai Ngoc Duy wrote:
> On Wed, Nov 16, 2011 at 3:59 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Nguyen Thai Ngoc Duy wrote:
>>
>>> Or perhaps
>> [...]
>>> - git_path(const char *path) maintains a small hash table to keep
>>> track of all returned strings based with "path" as key.
>>>
>>> Out of 142 git_path() calls in my tree, 97 of them are in form
>>> git_path("some static string").
>> The main bit I dislike about patch 3/3 is that constructs like
>> 'unlink(git_path("MERGE_HEAD"));' are not actually unsafe
>
> Well, we can create wrappers (e.g. repo_unlink(const char *) that
> calls git_path internally). According to grep/sed these functions are
> used in form xxx(git_path(xxx))
>
> 16 unlink
> 8 file_exists
> 7 stat
> 6 fopen
> 5 rename
> 5 open
> 4 unlink_or_warn
> 3 safe_create_dir
> 3 adjust_shared_perm
> 3 access
> 2 xstrdup
> 2 safe_create_leading_directories
This one at least, maybe others, is unsafe on cygwin. Indeed it causes
a test failure in t3200-branch.sh; patch is on it's way ...
ATB,
Ramsay Jones
^ permalink raw reply
* Re: clean bug on ignored subdirectories with no tracked files?
From: Junio C Hamano @ 2011-11-19 19:23 UTC (permalink / raw)
To: Jay Soffian; +Cc: git
In-Reply-To: <CAG+J_Dxw00e_cr7i3R9DAbTrqZvJHYk2yeUa=xGKh+Zqqmp-SA@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> git init test_repo &&
> cd test_repo &&
> mkdir -p foo/bar &&
> echo baz > foo/bar/baz &&
> echo /foo/bar > .gitignore &&
> git add .gitignore &&
> git clean -n -d
>
> Initialized empty Git repository in .../test_repo/.git/
> Would remove foo/
>
> Seems surprising.
You said "everythingthing in foo/bar is uninteresting and can be cleaned",
you have one untracked file in "foo/bar" hierarchy, and you have nothing
else in "foo/" hierarchy.
Removing the uninteresting cruft as your .gitignore instructs Git makes
the entire "foo/" hierarchy devoid of any contents. I would *expect* Git
to clean "foo" in this case.
I've seen some "surprising" behaviour in "git clean" (which I do not use
myself, I do not consider part of "my code", and I am not surprised if it
has many bugs), but I fail to see what is surprising in your transcript.
It would be a different issue if you had ">foo/other" before your "clean".
Then "foo/" has "foo/clean" that is not declared to be uninteresting.
^ permalink raw reply
* Re: [PATCH 1/2] MSVC: Do not close stdout to prevent a crash
From: Junio C Hamano @ 2011-11-19 19:11 UTC (permalink / raw)
To: Andreas Schwab
Cc: Vincent van Ravesteijn, git, msysgit, gitster, kusmabite,
Johannes.Schindelin
In-Reply-To: <m2sjlkcgyl.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Vincent van Ravesteijn <vfr@lyx.org> writes:
>
>> When compiled with MSVC, git crashes on Windows when calling
>> fstat(stdout) when stdout is closed. fstat is being called at the end of
>
> ITYM fileno(stdout).
>
>> run_builtin and this will thus be a problem for builtin command that close
>> stdout. This happens for 'format-patch' which closes stdout after a call to
>> freopen which directs stdout to the format patch file.
>
> It shouldn't do that in the first place. This is an error on any
> platform.
Correct. The clean-up codepath is for built-in command implementations
that write out their result and return 0 to signal success. If we let the
crt0 to run its usual clean-ups like closing the standard output stream,
we wouldn't be able to catch errors from there.
For built-ins that perform their own clean-ups, it is their responsibility
to be careful, hence we skip this part of the code.
We have relied on fstat(-1, &st) to correctly error out, and if MSVC build
crashes, it is a bug in its fstat() emulation, I would think.
We could do something like the following patch to be extra defensive,
though.
git.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git.c b/git.c
index 8e34903..64c28e4 100644
--- a/git.c
+++ b/git.c
@@ -309,8 +309,8 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
if (status)
return status;
- /* Somebody closed stdout? */
- if (fstat(fileno(stdout), &st))
+ if (fileno(stdout) < 0 || /* Somebody closed stdout? */
+ fstat(fileno(stdout), &st))
return 0;
/* Ignore write errors for pipes and sockets.. */
if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode))
^ 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