* Re: git rebase --skip
From: Björn Steinbrink @ 2007-11-08 10:24 UTC (permalink / raw)
To: Jeff King; +Cc: Mike Hommey, git
In-Reply-To: <20071108032308.GA5638@sigill.intra.peff.net>
On 2007.11.07 22:23:10 -0500, Jeff King wrote:
> On Wed, Nov 07, 2007 at 11:21:05PM +0100, Mike Hommey wrote:
>
> > I use git-rebase quite regularly, and I haven't used git-rebase --skip
> > after a failed merge without first resetting the working tree. I was
> > wondering if it wouldn't make sense to automatically do the reset when
> > running git-rebase --skip.
>
> I have often been annoyed by this behavior, too, and I can't think of
> any situation where you _wouldn't_ want the reset to happen. But I
> would be more comfortable hearing confirmation from others that they
> can't think of such a situation.
Let's take this history:
C---D---E topic
/
A---B master
You then do:
git rebase master topic
Now D causes conflicts, because B did a similar change, but not quite
the same, maybe having a bug. So you want to keep parts of D, but it's
no longer the same commit semantically and the original commit message
would be bogus. So you resolve the conflicts and do:
git commit
git rebase --skip
Because you replaced D with a new different commit, instead of really
just rebasing it. With plain --continue, you'd have to go back and fix
the commit message once the rebase is complete. And --continue after the
commit is a no-go, too, because rebase will complain that there's
nothing left to produce the rebased D commit.
And now imagine that you forget to commit but instead just --skip.
Ouch, all the work is lost, time to restart the rebase. With the current
behaviour, rebase won't just throw away your stuff.
Björn
^ permalink raw reply
* Re: git add -i fails to heed user's exclude settings
From: Miles Bader @ 2007-11-08 10:17 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: git
In-Reply-To: <4732DAD8.8000702@op5.se>
Andreas Ericsson <ae@op5.se> writes:
> Which git version are you using?
1.5.3.5
Thanks,
-Miles
--
The secret to creativity is knowing how to hide your sources.
--Albert Einstein
^ permalink raw reply
* Re: [PATCH 3/3] git-fetch: avoid local fetching from alternate (again)
From: Junio C Hamano @ 2007-11-08 10:07 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071108100039.GM14735@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> I'm starting to suspect heap corruption again in builtin-fetch.
> This patch alters the malloc() calls we are doing and may be shifting
> something around just enough in memory to cause a data overwrite or
> something and that's why this tag just drops out of the linked list?
> But then why does that happen in the test suite but not outside.
> Maybe because the test suite is setting environment variables that
> I'm not and the impact of those combined with these additional
> mallocs is what is breaking it? *sigh*
Thanks for working hard on this one.
It is starting to look like today was "let's kill other people's
bugs" day. I'd go to bed before I completely miss sleep, which
I often end up doing when thinking too long about bugs.
^ permalink raw reply
* Re: [PATCH 3/3] git-fetch: avoid local fetching from alternate (again)
From: Shawn O. Pearce @ 2007-11-08 10:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20071108080058.GC16690@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Back in e3c6f240fd9c5bdeb33f2d47adc859f37935e2df Junio taught
> git-fetch to avoid copying objects when we are fetching from
> a repository that is already registered as an alternate object
> database. In such a case there is no reason to copy any objects
> as we can already obtain them through the alternate.
I haven't figured it out yet but this patch seriously breaks
t5515-fetch-merge-logic. For some reason the tag tag-three-file is
not being included in .git/FETCH_HEAD as a not-for-merge ref, but all
of the test vectors are expecting it to be present. Prior to this
patch it was included and I don't think the test vectors are wrong.
If I run git-fetch from outside the test library it does the right
thing and fetches this annotated tag pointing to a blob just fine.
But during the test vector it never even mentions that tag as part
of the status output, nor does it include it into .git/FETCH_HEAD.
Its almost like the tag ain't there.
I'm starting to suspect heap corruption again in builtin-fetch.
This patch alters the malloc() calls we are doing and may be shifting
something around just enough in memory to cause a data overwrite or
something and that's why this tag just drops out of the linked list?
But then why does that happen in the test suite but not outside.
Maybe because the test suite is setting environment variables that
I'm not and the impact of those combined with these additional
mallocs is what is breaking it? *sigh*
--
Shawn.
^ permalink raw reply
* Re: Strange git-show-branch behavior.
From: Sergei Organov @ 2007-11-08 9:58 UTC (permalink / raw)
To: Björn Steinbrink; +Cc: git
In-Reply-To: <20071103182224.GA16345@atjola.homenet>
Björn Steinbrink <B.Steinbrink@gmx.de> writes:
> On 2007.11.03 20:46:39 +0300, Sergei Organov wrote:
[...]
>> $ git-show-branch --more=9 master mybranch
>> * [master] Go to sleep
>> ! [mybranch] Some work.
>> --
>> * [master] Go to sleep
>> *+ [mybranch] Some work.
>> * [master~2] Some fun.
>> *+ [master~3] Commit message
>> *+ [master~4] Initial commit
>> $
>>
>> In this output, why git doesn't show the merge commit having "Merged
>> mybranch" commit message?
>
> Because you didn't pass --sparse.
Well, therefore, provided I have the following history:
.-F-. mybranch
/ \
A---B---C---D---E master
the 'D' merge commit is reachable only from 'master', so 'D' is not
shown unless I specify --sparse, right? Rather confusing, I'd say, and
the name 'sparse' for the option suggests that the output will have less
revisions in the output, not more. I mean I even didn't care to look at
the description of --sparse when I first read the manual page in order to
find some option to increase number of revs output, while I did look at
the --more.
What is the rationale for skipping such merge commits by default?
Anyway, courtesy to your explanation, I think I will be able to come
with a patch for the 'Documentation/core-tutorial.txt' that seems to
have wrong description for one of its examples.
>>
>> Yet another confusion:
>>
>> $ git-show-branch master mybranch
>> * [master] Go to sleep
>> ! [mybranch] Some work.
>> --
>> * [master] Go to sleep
>> *+ [mybranch] Some work.
>> $
>>
>> Why does it stop at "Some work." commit? The manual page says: "Usually
>> the command stops output upon showing the commit that is the common
>> ancestor of all the branches.", so I'd expect it should go down to
>> "Commit message" commit that is the fork point.
>
> Common ancestor means, that the commit is reachable through all refs.
> Let's take a look at your history:
>
> .-F-. mybranch
> / \
> A---B---C---D---E master
>
> There you can see that mybranch can of course reach F, and that master
> can reach it, too. E -> D -> F. So the output stops at F, not at B.
You are right, this particular confusion was due to my misunderstanding
of the term "common ancestor".
However, shouldn't "*the* common ancestor" in the manual be replaced by
"*a* common ancestor"? I mean that according to git-merge-base, there
could be multiple common ancestors even for 2 commits, so saying "*the*
common ancestor" implies use of particular algorithm to select
*the* common ancestor among all the possibilities, and therefore I'd
expect some explanation of the algorithm being used to get *the* common
ancestor.
--
Sergei.
^ permalink raw reply
* Re: [PATCH] contrib/hooks/post-receive-email: add a From: line to the email header
From: Junio C Hamano @ 2007-11-08 9:55 UTC (permalink / raw)
To: Gerrit Pape; +Cc: git, Andy Parkins
In-Reply-To: <20071108094809.22151.qmail@97f06c2e73713e.315fe32.mid.smarden.org>
Gerrit Pape <pape@smarden.org> writes:
> $committer is already extracted from the latest existing rev, so add the
> corresponding From: line to the email header.
You may fight this out with Andy if you want to, but I think I'd
side with the existing behaviour.
commit e6dc8d60fbd2c84900a26545c5d360b0e202d95b
Author: Andy Parkins <andyparkins@gmail.com>
Date: Fri Sep 28 15:24:26 2007 +0100
post-receive-hook: Remove the From field from the generated email header so that the pusher's name is used
Using the name of the committer of the revision at the tip of the
updated ref is not sensible. That information is available in the email
itself should it be wanted, and by supplying a "From", we were
effectively hiding the person who performed the push - which is useful
information in itself.
Signed-off-by: Andy Parkins <andyparkins@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 1f88099..cbbd02f 100644
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -177,7 +177,6 @@ generate_email_header()
# --- Email (all stdout will be the email)
# Generate header
cat <<-EOF
- From: $committer
To: $recipients
Subject: ${EMAILPREFIX}$projectdesc $refname_type, $short_refname, ${change_type}d. $describe
X-Git-Refname: $refname
^ permalink raw reply related
* Re: [PATCH v2] user-manual: add advanced topic "bisecting merges"
From: Johannes Sixt @ 2007-11-08 9:53 UTC (permalink / raw)
To: Andreas Ericsson, Steffen Prohaska; +Cc: gitster, Ralf.Wildenhues, tsuna, git
In-Reply-To: <4732D7F6.7040006@op5.se>
Andreas Ericsson schrieb:
> Johannes Sixt wrote:
>> Steffen Prohaska schrieb:
>>>
>>> On Nov 8, 2007, at 8:19 AM, Johannes Sixt wrote:
>>>
>>>> Steffen Prohaska schrieb:
>>>>> +If you linearize the history by rebasing the lower branch on
>>>>> +top of the upper, instead of merging, the bug becomes much easier to
>>>>> +find and understand. Your history would instead be:
>>>>
>>>> At this point I'm missing the words
>>>>
>>>> The solution is ...
>>>>
>>>> I.e.:
>>>>
>>>> The solution is to linearize the history by rebasing the lower
>>>> branch on
>>>> top of the upper, instead of merging. Now the bug becomes much
>>>> easier to
>>>> find and understand. Your history would instead be:
>>>
>>> Hmm. It might be a solution if you did not publish history.
>>
>> This is about finding the commit that introduced a bug. Once you found
>> it, better: you know how to fix the bug, you are expected to throw
>> away the rebased branch, not to publish it! Maybe a note along these
>> lines could be appended:
>>
>> Now that you know what caused the error (and how to fix it), throw
>> away the rebased branch, and commit a fix on top of D.
>>
>
> Well, if rebasing becomes the standard for normal development, it's hardly
> right to throw it away, is it? I like Steffen's suggestion better.
There is a big misunderstanding. The text that the patch amends is about
bisecting history that reveals that a merge commit breaks, which is not
helpful, and then how to find where and what and why the breakage really was
introduce.
And the answer to "how to find" is to rebase and bisect in the rebased history.
My initial complaint was that in the flow of reading the instructions the
pointer to "the solution" was missing. Rather, at the point where the reader
is supposed to think "ah, yes, that's how to do it", there is the
conditional statement "If you linearize history". My suggestion is to put a
big emphasis on the solution by using the words "The solution is".
Now, the user can *always* rebase one of the branches on top of the other,
even if both histories are already published. *But* if both were indeed
published, then the rebased history must be thrown away, and the only thing
you learnt from it was where and what and why the breakage really was
introduced.
Of course we could include a few "ifs" and "unlesses" (about published
histories), before suggesting to throw away rebased history. But once the
task is accomplished (find the bogus commit), throwing away the rebased
history (and continuing at commit D) is always correct, but keeping it (and
continuing at D*) is not.
-- Hannes
^ permalink raw reply
* [PATCH] contrib/hooks/post-receive-email: add a From: line to the email header
From: Gerrit Pape @ 2007-11-08 9:48 UTC (permalink / raw)
To: git, Junio C Hamano
$committer is already extracted from the latest existing rev, so add the
corresponding From: line to the email header.
Signed-off-by: Gerrit Pape <pape@smarden.org>
---
contrib/hooks/post-receive-email | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 3904c18..c73f2d5 100644
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -189,6 +189,7 @@ generate_email_header()
# --- Email (all stdout will be the email)
# Generate header
cat <<-EOF
+ From: $committer
To: $recipients
Subject: ${emailprefix}$projectdesc $refname_type, $short_refname, ${change_type}d. $describe
X-Git-Refname: $refname
--
1.5.3.5
^ permalink raw reply related
* [PATCH] hooks--update: decline deleting tags or branches by default, add config options
From: Gerrit Pape @ 2007-11-08 9:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v640d8skt.fsf@gitster.siamese.dyndns.org>
Decline deleting tags or branches through git push <remote> :<ref> by
default, support config options hooks.allowdeletetag, hooks.allowdeletebranch
to override this per repository.
Before this patch the update hook interpreted deleting a tag, no matter if
annotated or not, through git push <remote> :<tag> as unannotated tag, and
declined it by default, but with an unappropriate error message:
$ git push origin :atag
deleting 'refs/tags/atag'
*** The un-annotated tag, atag, is not allowed in this repository
*** Use 'git tag [ -a | -s ]' for tags you want to propagate.
ng refs/tags/atag hook declined
error: hooks/update exited with error code 1
error: hook declined to update refs/tags/atag
error: failed to push to 'monolith:/git/qm/test-repo'
Signed-off-by: Gerrit Pape <pape@smarden.org>
---
On Wed, Nov 07, 2007 at 04:54:42PM -0800, Junio C Hamano wrote:
> Since you are allowing deletion for anything, wouldn't this be
> much simpler, I wonder...
Yes, true. But to be a good axample hook, I suggest to differentiate
this in the implementation, and to deny deleting tags, branches by
default.
templates/hooks--update | 33 +++++++++++++++++++++++++++++++--
1 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/templates/hooks--update b/templates/hooks--update
index 65e8c32..7d7c251 100644
--- a/templates/hooks--update
+++ b/templates/hooks--update
@@ -10,6 +10,12 @@
# hooks.allowunannotated
# This boolean sets whether unannotated tags will be allowed into the
# repository. By default they won't be.
+# hooks.allowdeletetag
+# This boolean sets whether deleting tags will be allowed in the
+# repository. By default they won't be.
+# hooks.allowdeletebranch
+# This boolean sets whether deleting branches will be allowed in the
+# repository. By default they won't be.
#
# --- Command line
@@ -32,6 +38,8 @@ fi
# --- Config
allowunannotated=$(git-repo-config --bool hooks.allowunannotated)
+allowdeletebranch=$(git-repo-config --bool hooks.allowdeletebranch)
+allowdeletetag=$(git-repo-config --bool hooks.allowdeletetag)
# check for no description
projectdesc=$(sed -ne '1p' "$GIT_DIR/description")
@@ -41,9 +49,9 @@ if [ -z "$projectdesc" -o "$projectdesc" = "Unnamed repository; edit this file t
fi
# --- Check types
-# if $newrev is 0000...0000, it's a commit to delete a branch
+# if $newrev is 0000...0000, it's a commit to delete a ref.
if [ "$newrev" = "0000000000000000000000000000000000000000" ]; then
- newrev_type=commit
+ newrev_type=delete
else
newrev_type=$(git-cat-file -t $newrev)
fi
@@ -58,15 +66,36 @@ case "$refname","$newrev_type" in
exit 1
fi
;;
+ refs/tags/*,delete)
+ # delete tag
+ if [ "$allowdeletetag" != "true" ]; then
+ echo "*** Deleting a tag is not allowed in this repository" >&2
+ exit 1
+ fi
+ ;;
refs/tags/*,tag)
# annotated tag
;;
refs/heads/*,commit)
# branch
;;
+ refs/heads/*,delete)
+ # delete branch
+ if [ "$allowdeletebranch" != "true" ]; then
+ echo "*** Deleting a branch is not allowed in this repository" >&2
+ exit 1
+ fi
+ ;;
refs/remotes/*,commit)
# tracking branch
;;
+ refs/remotes/*,delete)
+ # delete tracking branch
+ if [ "$allowdeletebranch" != "true" ]; then
+ echo "*** Deleting a tracking branch is not allowed in this repository" >&2
+ exit 1
+ fi
+ ;;
*)
# Anything else (is there anything else?)
echo "*** Update hook: unknown type of update to ref $refname of type $newrev_type" >&2
--
1.5.3.5
^ permalink raw reply related
* Re: git add -i fails to heed user's exclude settings
From: Andreas Ericsson @ 2007-11-08 9:46 UTC (permalink / raw)
To: Miles Bader; +Cc: git
In-Reply-To: <buowsstmapt.fsf@dhapc248.dev.necel.com>
Miles Bader wrote:
> Inside git add -i, option 4 "add untracked", it doesn't seem to consider
> the user's personal ignore patterns, which is _really_ annoying.
>
> E.g., I have:
>
> $ git config core.excludesfile
> /home/miles/.gitignore
>
> $ cat /home/miles/.gitignore
> [+,#]*
> *~
>
> $ git status
> # On branch master
> nothing to commit (working directory clean)
>
> $ git add -i
> staged unstaged path
>
> *** Commands ***
> 1: status 2: update 3: revert 4: add untracked
> 5: patch 6: diff 7: quit 8: help
> What now> 4
> 1: ,l
> 2: ,l.~1~
> 3: AUTHORS.~1~
> 4: Makefile.am.~1~
> 5: config.h.in~
> 6: configure.ac.~1~
> ...tons of other random backup files etc...
>
> This makes it very hard to find files that actually need to be added!
>
Which git version are you using?
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] send-pack: segfault fix on forced push
From: Jeff King @ 2007-11-08 9:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsl3h2i2j.fsf@gitster.siamese.dyndns.org>
On Thu, Nov 08, 2007 at 01:38:12AM -0800, Junio C Hamano wrote:
> When pushing to overwrite a ref that points at a commit we do
> not even have, the recent "terse push" patch tried to get a
> unique abbreviation for the non-existent (from our point of
> view) object, which resulted in strcpy(buf, NULL) and
> segfaulted.
Good catch. The fix looks obviously correct.
-Peff
^ permalink raw reply
* [PATCH] send-pack: segfault fix on forced push
From: Junio C Hamano @ 2007-11-08 9:38 UTC (permalink / raw)
To: Jeff King; +Cc: git
When pushing to overwrite a ref that points at a commit we do
not even have, the recent "terse push" patch tried to get a
unique abbreviation for the non-existent (from our point of
view) object, which resulted in strcpy(buf, NULL) and
segfaulted.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-send-pack.c | 5 +++--
t/t5405-send-pack-rewind.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index c1fd3f5..5a0f5c6 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -365,8 +365,9 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
char quickref[83];
char type = ' ';
const char *msg = "";
-
- strcpy(quickref, find_unique_abbrev(ref->old_sha1, DEFAULT_ABBREV));
+ const char *old_abb;
+ old_abb = find_unique_abbrev(ref->old_sha1, DEFAULT_ABBREV);
+ strcpy(quickref, old_abb ? old_abb : old_hex);
if (ref_newer(ref->peer_ref->new_sha1, ref->old_sha1))
strcat(quickref, "..");
else {
diff --git a/t/t5405-send-pack-rewind.sh b/t/t5405-send-pack-rewind.sh
new file mode 100755
index 0000000..86abc62
--- /dev/null
+++ b/t/t5405-send-pack-rewind.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+test_description='forced push to replace commit we do not have'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+ >file1 && git add file1 && test_tick &&
+ git commit -m Initial &&
+
+ mkdir another && (
+ cd another &&
+ git init &&
+ git fetch .. master:master
+ ) &&
+
+ >file2 && git add file2 && test_tick &&
+ git commit -m Second
+
+'
+
+test_expect_success 'non forced push should die not segfault' '
+
+ (
+ cd another &&
+ git push .. master:master
+ test $? = 1
+ )
+
+'
+
+test_expect_success 'forced push should succeed' '
+
+ (
+ cd another &&
+ git push .. +master:master
+ )
+
+'
+
+test_done
^ permalink raw reply related
* Re: Problem with https and git-pull
From: Andreas Ericsson @ 2007-11-08 9:37 UTC (permalink / raw)
To: Luke Diamand; +Cc: git
In-Reply-To: <4732BEDD.3010703@vidanti.com>
Luke Diamand wrote:
> OK, I just read the FAQ.
>
... and found that "git update-server-info" needs to be run on the remote
side in order for dumb protocols (http, rsync, ftp...) to work.
Directed at any poor soul who finds this solution in the mailing list
archives before posting to the list. Hello there, poor soul ;-)
> Luke Diamand wrote:
>> I'm finding that git-pull using https does not work in the way I would
>> expect.
>>
>> I created a bare repository, test.git, available by https://
>>
>> I then cloned it:
>>
>> % git-clone https://host/git/test.git
>>
>> So far, so good.
>>
>> Then I made a change in a different clone and pushed it.
>>
>> When I next did git-pull it just said:
>>
>> % git-pull
>> Fetching refs/heads/master from https://host/git/test.git using https
>> Already up-to-date.
>>
>> But it *isn't* up-to-date! If I do the same exercise with git:// or
>> ssh:// on the same repo then it pulls down my changes as expected.
>>
>> Tried with:
>> git version 1.5.3.4 (debian testing)
>> git 1.5.3.5-dirty
>>
>> curl is 7.16.4
>>
>> The server access log shows the git-pull happening, and there are no
>> errors reported by the server.
>>
>> Is there something obvious I'm missing?
>>
>> Thanks
>> Luke
>>
>>
>
> -
> 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
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] git-sh-setup: fix parseopt `eval`.
From: Pierre Habouzit @ 2007-11-08 9:35 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <20071108091402.GA7391@artemis.corp>
[-- Attachment #1: Type: text/plain, Size: 2906 bytes --]
On Thu, Nov 08, 2007 at 09:14:02AM +0000, Pierre Habouzit wrote:
> On Thu, Nov 08, 2007 at 07:09:29AM +0000, Junio C Hamano wrote:
> > The 'automagic parseopt' support corrupted non option parameters
> > that had IFS characters in them. The worst case can be seen
> > when it has a non option parameter like this:
> >
> > $1=" * some string blech"
> >
> > Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> > - parseopt_extra=
> > - [ -n "$OPTIONS_KEEPDASHDASH" ] &&
> > - parseopt_extra="$parseopt_extra --keep-dashdash"
> > + [ -n "$OPTIONS_KEEPDASHDASH" ] && parseopt_extra="--keep-dashdash"
>
> oh and this part is wrong because you're affected by $parseopt_extra
> environment poisonning. And you have to fix git-clone.sh that uses
> git-rev-parse --parsopt directly with the same call too (as it doesn't
> use git-sh-setup).
Here is a patch that should fix all those issues at once, replace
yours. I tested it with this minimal test:
$ cat parseopt.sh
#!/bin/sh
OPTIONS_KEEPDASHDASH=
OPTIONS_SPEC="\
foo
--
"
. git-sh-setup
for i in "$@"; do echo "$i"; done
$ ./parseopt.sh " * hahahah bleh"
--
* hahahah bleh
$ ./parseopt.sh -asd " * hahahah bleh"
error: unknown switch `a'
usage: foo
$ echo $?
129
which fix your bug, and still behaves as advertised.
From 3c2095533094ff6d82272dc36d9f576b0e81d135 Mon Sep 17 00:00:00 2001
From: Pierre Habouzit <madcoder@debian.org>
Date: Thu, 8 Nov 2007 10:32:11 +0100
Subject: [PATCH] Prevent eval of $(git-rev-parse --parseopt) output to be shell-expansed.
Thanks to Junio for having spotted this.
Use the preferred $(...) form rather than ``
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
git-clone.sh | 2 +-
git-sh-setup.sh | 8 ++++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/git-clone.sh b/git-clone.sh
index f216f03..24ad179 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -36,7 +36,7 @@ usage() {
exec "$0" -h
}
-eval `echo "$OPTIONS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?`
+eval "$(echo "$OPTIONS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
get_repo_base() {
(
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index e1cf885..5aa62dd 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -23,9 +23,13 @@ if test -n "$OPTIONS_SPEC"; then
parseopt_extra=
[ -n "$OPTIONS_KEEPDASHDASH" ] &&
- parseopt_extra="$parseopt_extra --keep-dashdash"
+ parseopt_extra="--keep-dashdash"
- eval `echo "$OPTIONS_SPEC" | git rev-parse --parseopt $parseopt_extra -- "$@" || echo exit $?`
+ eval "$(
+ echo "$OPTIONS_SPEC" |
+ git rev-parse --parseopt $parseopt_extra -- "$@" ||
+ echo exit $?
+ )"
else
usage() {
die "Usage: $0 $USAGE"
--
1.5.3.5.1598.gdef4e-dirty
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related
* Re: [PATCH v2] user-manual: add advanced topic "bisecting merges"
From: Andreas Ericsson @ 2007-11-08 9:33 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Steffen Prohaska, gitster, Ralf.Wildenhues, tsuna, git
In-Reply-To: <4732D2CC.1010008@viscovery.net>
Johannes Sixt wrote:
> Steffen Prohaska schrieb:
>>
>> On Nov 8, 2007, at 8:19 AM, Johannes Sixt wrote:
>>
>>> Steffen Prohaska schrieb:
>>>> +If you linearize the history by rebasing the lower branch on
>>>> +top of the upper, instead of merging, the bug becomes much easier to
>>>> +find and understand. Your history would instead be:
>>>
>>> At this point I'm missing the words
>>>
>>> The solution is ...
>>>
>>> I.e.:
>>>
>>> The solution is to linearize the history by rebasing the lower branch on
>>> top of the upper, instead of merging. Now the bug becomes much easier to
>>> find and understand. Your history would instead be:
>>
>> Hmm. It might be a solution if you did not publish history.
>
> This is about finding the commit that introduced a bug. Once you found
> it, better: you know how to fix the bug, you are expected to throw away
> the rebased branch, not to publish it! Maybe a note along these lines
> could be appended:
>
> Now that you know what caused the error (and how to fix it), throw away
> the rebased branch, and commit a fix on top of D.
>
Well, if rebasing becomes the standard for normal development, it's hardly
right to throw it away, is it? I like Steffen's suggestion better.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: Inconsistencies with git log
From: Wincent Colaiuta @ 2007-11-08 9:24 UTC (permalink / raw)
To: Jon Smirl; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <9e4733910711071529m604f3b12v29b3a040074ea4e@mail.gmail.com>
El 8/11/2007, a las 0:29, Jon Smirl escribió:
> It's not consistent. git log with no parameters is relative to the
> project root, git log with a parameter is relative to the current
> directory.
A minor quibble: git log with no parameters isn't "relative" to
anything. It shows the history of the entire project. There is no
inconsistency.
Cheers,
Wincent
^ permalink raw reply
* Re: [REPLACEMENT PATCH] git-checkout: Add a test case for relative paths use.
From: Andreas Ericsson @ 2007-11-08 9:23 UTC (permalink / raw)
To: David Symonds; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <11945006082887-git-send-email-dsymonds@gmail.com>
David Symonds wrote:
> +'
> +
> +test_expect_failure 'checkout with relative path outside tree should fail (1)' \
> + 'git checkout HEAD - ../file0'
> +
> +test_expect_failure 'checkout with relative path outside tree should fail (2)' \
> + 'cd dir1 && git checkout HEAD - ./file0'
> +
> +test_expect_failure 'checkout with relative path outside tree should fail (2)' \
> + 'cd dir1 && git checkout HEAD - ../../file0'
Single-dashes on all of these?
Looks good otherwise.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: Inconsistencies with git log
From: Wincent Colaiuta @ 2007-11-08 9:19 UTC (permalink / raw)
To: Jon Smirl; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <9e4733910711071503va92a653s25fd978989d5917d@mail.gmail.com>
El 8/11/2007, a las 0:03, Jon Smirl escribió:
> On 11/7/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>>
>>
>> We also tend to take the approach of viewing the history as that of
>> the whole project.
>
> But if you type 'git log' while cd'd into a subdirectory the whole log
> is almost never what you want. It's this kind of thing that makes git
> harder to use.
At least in my case, that's completely untrue. Whole-project history
is basically *always* what I want even if I am cd'd into a
subdirectory. If I wanted to path-limit the project history I'd do
"git log ."
Cheers,
Wincent
^ permalink raw reply
* Re: stgit: cleaning up after using git branch delete commands
From: Catalin Marinas @ 2007-11-08 9:18 UTC (permalink / raw)
To: Karl Hasselström; +Cc: Jon Smirl, Git Mailing List
In-Reply-To: <20071108055302.GA11230@diana.vm.bytemark.co.uk>
On 08/11/2007, Karl Hasselström <kha@treskal.com> wrote:
> On 2007-11-07 11:11:42 -0500, Jon Smirl wrote:
>
> > how about a 'stg gc' command that gets rid of all the inaccessible
> > clutter?
>
> "stg assimilate" already has the job of fixing up stuff after the user
> has used git commands to move HEAD around. I think it would make sense
> to teach it to do this too -- and then rename it "stg repair" or
> something. That way, there's one command to fix every kind of "damage"
> that git can do to stgit.
"repair" sounds better than "gc" (which might also be confused with
the "git gc" command).
> Alternatively, "stg branch --create" and "stg init" and whoever else
> is bothered by the clutter could simply remove it themselves. That
> would be even more user-friendly, I guess.
I did some fixes for branch --delete but, since I use StGIT almost
exclusively, haven't thought that we need to relax the branch creation
as well.
--
Catalin
^ permalink raw reply
* Re: [PATCH] git-branch --with=commit
From: Andreas Ericsson @ 2007-11-08 9:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git
In-Reply-To: <7vejf140jd.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>
>> Junio C Hamano schrieb:
>
>>> With this patch, I could do this to find out which topic
>>> branches already contain the faulty commit:
>>>
>>> $ git branch --with=maint^ | grep /
>>> xx/maint-fix-foo
>> It'd be helpful if you could construct the example in this commit
>> message such that you don't need the "grep /" here; otherwise, the
>> reader doesn't know which part of the effect is hidden by the grep.
>
> Yeah, in the example sequence, I think only maint itself and
> xx/maint-fix-foo are shown, so there is no need for grep.
And "maint" could certainly be stripped by the code itself, since the
user can reasonably be expected to know that plain maint will have
everything maint^ has.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] git-sh-setup: fix parseopt `eval`.
From: Pierre Habouzit @ 2007-11-08 9:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr6j15i3a.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 941 bytes --]
On Thu, Nov 08, 2007 at 07:09:29AM +0000, Junio C Hamano wrote:
> The 'automagic parseopt' support corrupted non option parameters
> that had IFS characters in them. The worst case can be seen
> when it has a non option parameter like this:
>
> $1=" * some string blech"
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> - parseopt_extra=
> - [ -n "$OPTIONS_KEEPDASHDASH" ] &&
> - parseopt_extra="$parseopt_extra --keep-dashdash"
> + [ -n "$OPTIONS_KEEPDASHDASH" ] && parseopt_extra="--keep-dashdash"
oh and this part is wrong because you're affected by $parseopt_extra
environment poisonning. And you have to fix git-clone.sh that uses
git-rev-parse --parsopt directly with the same call too (as it doesn't
use git-sh-setup).
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH v2] user-manual: add advanced topic "bisecting merges"
From: Johannes Sixt @ 2007-11-08 9:11 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: gitster, Ralf.Wildenhues, tsuna, git
In-Reply-To: <6E62E205-0951-4CCB-A807-AC107E40ACE1@zib.de>
Steffen Prohaska schrieb:
>
> On Nov 8, 2007, at 8:19 AM, Johannes Sixt wrote:
>
>> Steffen Prohaska schrieb:
>>> +If you linearize the history by rebasing the lower branch on
>>> +top of the upper, instead of merging, the bug becomes much easier to
>>> +find and understand. Your history would instead be:
>>
>> At this point I'm missing the words
>>
>> The solution is ...
>>
>> I.e.:
>>
>> The solution is to linearize the history by rebasing the lower branch on
>> top of the upper, instead of merging. Now the bug becomes much easier to
>> find and understand. Your history would instead be:
>
> Hmm. It might be a solution if you did not publish history.
This is about finding the commit that introduced a bug. Once you found it,
better: you know how to fix the bug, you are expected to throw away the
rebased branch, not to publish it! Maybe a note along these lines could be
appended:
Now that you know what caused the error (and how to fix it), throw away the
rebased branch, and commit a fix on top of D.
-- Hannes
^ permalink raw reply
* Re: [PATCH v2] user-manual: add advanced topic "bisecting merges"
From: Steffen Prohaska @ 2007-11-08 8:59 UTC (permalink / raw)
To: Johannes Sixt; +Cc: gitster, Ralf.Wildenhues, tsuna, git
In-Reply-To: <4732B899.6000908@viscovery.net>
On Nov 8, 2007, at 8:19 AM, Johannes Sixt wrote:
> Steffen Prohaska schrieb:
>> +If you linearize the history by rebasing the lower branch on
>> +top of the upper, instead of merging, the bug becomes much easier to
>> +find and understand. Your history would instead be:
>
> At this point I'm missing the words
>
> The solution is ...
>
> I.e.:
>
> The solution is to linearize the history by rebasing the lower
> branch on
> top of the upper, instead of merging. Now the bug becomes much
> easier to
> find and understand. Your history would instead be:
Hmm. It might be a solution if you did not publish history.
How about leaving the text as is and adding an introductory
paragraph at the beginning of the section?
I.e:
This section discusses how gitlink:git-bisect[1] plays
with differently shaped histories. If you did not yet
publish a branch you can use either gitlink:git-merge[1] or
gitlink:git-rebase[1] to integrate changes from a second
branch. The two approaches create differently shaped
histories. So it might be interesting to know about the
implications on gitlink:git-bisect[1].
Steffen
^ permalink raw reply
* Re: [PATCH] Re: git-svn fetch doesn't like spaces in branch names
From: Benoit Sigoure @ 2007-11-08 8:49 UTC (permalink / raw)
To: Alex Riesen; +Cc: Michael J. Cohen, Git Mailing List
In-Reply-To: <20071108072918.GC3170@steel.home>
[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]
On Nov 8, 2007, at 8:29 AM, Alex Riesen wrote:
> Michael J. Cohen, Thu, Nov 08, 2007 01:53:07 +0100:
>>> mini:TextMateBundles mjc$ git-svn fetch
>>> Found possible branch point:
>>> http://macromates.com/svn/Bundles/trunk/Tools/Dialog PlugIn =>
>>> http://macromates.com/svn/Bundles/branches/Dialog PlugIn
>>> Completion Menu,
>>> 8089
>>> Initializing parent: Dialog PlugIn Completion Menu@8089
>>> Bad URL passed to RA layer: Malformed URL for repository at
>>> /opt/local/bin/git-svn line 1607
>>>
>>> looks like that might need to be %20 ?
>>
>>
>> Hacky, but it works.
>>
>> Signed-off-by: Michael J. Cohen <mjc@cruiseplanners.com>
>>
>> diff --git a/git-svn.perl b/git-svn.perl
>> index dd93e32..5dc3b9c 100755
>> --- a/git-svn.perl
>> +++ b/git-svn.perl
>> @@ -1976,6 +1976,7 @@ sub find_parent_branch {
>> my $r = $i->{copyfrom_rev};
>> my $repos_root = $self->ra->{repos_root};
>> my $url = $self->ra->{url};
>> + $branch_from =~ s@([\s])@sprintf("%%%02X", ord($1))@seg;
>
> You don't need "[" and "]".
You don't even need the "(" and ")"
$branch_from =~ s@\s@sprintf("%%%02X", ord($&))@seg;
But I think it'd be better to fix this properly. I guess some people
use branch names with accentuated characters such as é è ü whatever.
What about this instead (untested):
$branch_from =~ s@[^\w\d_]@sprintf("%%%02X", ord($&))@seg;
Otherwise there are various existing Perl modules such as http://
search.cpan.org/dist/URI/URI/Escape.pm but this seems overkill / not
portable (unless we distribute these files along with Git).
Cheers,
--
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]
^ permalink raw reply
* Re: [PATCH MISC 1/1] Make gcc warning about empty if body go away.
From: Andreas Ericsson @ 2007-11-08 8:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pierre Habouzit, git
In-Reply-To: <7vode57awg.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
>> diff --git a/builtin-diff.c b/builtin-diff.c
>> index f77352b..80392a8 100644
>> --- a/builtin-diff.c
>> +++ b/builtin-diff.c
>> @@ -204,7 +204,7 @@ static void refresh_index_quietly(void)
>> if (write_cache(fd, active_cache, active_nr) ||
>> close(fd) ||
>> commit_locked_index(lock_file))
>> - ; /*
>> + (void)0; /*
>> * silently ignore it -- we haven't mucked
>> * with the real index.
>> */
>
> Wouldn't this be much easier to read, by the way?
>
> The point is that if we touched the active_cache, we try to
> write it out and make it the index file for later users to use
> by calling "commit", but we do not really care about the failure
> from this sequence because it is done purely as an optimization.
>
> The original code called three functions primarily for their
> side effects, which is admittedly a bad style.
>
> builtin-diff.c | 12 +++---------
> 1 files changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/builtin-diff.c b/builtin-diff.c
> index f77352b..906c924 100644
> --- a/builtin-diff.c
> +++ b/builtin-diff.c
> @@ -200,15 +200,9 @@ static void refresh_index_quietly(void)
> discard_cache();
> read_cache();
> refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
> - if (active_cache_changed) {
> - if (write_cache(fd, active_cache, active_nr) ||
> - close(fd) ||
> - commit_locked_index(lock_file))
> - ; /*
> - * silently ignore it -- we haven't mucked
> - * with the real index.
> - */
> - }
> + if (active_cache_changed &&
> + !write_cache(fd, active_cache, active_nr) && !close(fd))
> + commit_locked_index(lock_file);
> rollback_lock_file(lock_file);
> }
>
Ack, obviously, as it no longer requires a comment to explain it, although
I'd prefer an empty line after commit_locked_index(lock_file); so as to not
confuse the rollback_lock_file() statement as being part of the conditional
path.
First I thought the rollback_lock_file() was the *only* statement to the
condition, and everyone who uses 4 for tabsize) will have double trouble
since commit_locked_index(lock_file) aligns with the second line of the
condition.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox