Git development
 help / color / mirror / Atom feed
* Re: [PATCH] git-branch --with=commit
From: Junio C Hamano @ 2007-11-08  8:13 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <4732BC6F.7070005@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Junio C Hamano schrieb:
>>      $ git checkout -b xx/maint-fix-foo
>>      $ git am -3 -s ,xx-maint-fix-foo.patch
>
> Is this comma a hidden feature?

No, just my personal convention to queue e-mails from my mailbox.

>> 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.

^ permalink raw reply

* Re: [PATCH] git-sh-setup: fix parseopt `eval`.
From: Pierre Habouzit @ 2007-11-08  8:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr6j15i3a.fsf@gitster.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 789 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:

hu sorry about that, I should have put "" around the ``. I knew it also
but it slipped my mind too.  I believe this works as well:

eval "$(echo "$OPTIONS_SPEC" | git rev-parse --parseopt $parseopt_extra -- "$@" || echo exit $?)"

I like it better because you will then exit with an exit 129 wich is
what we want (and what I documented would work too :P)

-- 
·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 DIFF-CLEANUP 1/2] Make the diff_options bitfields be an unsigned with explicit masks.
From: Pierre Habouzit @ 2007-11-08  8:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7d95ji1.fsf@gitster.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 2688 bytes --]

On Thu, Nov 08, 2007 at 06:39:02AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > reverse_diff was a bit-value in disguise, it's merged in the flags now.
> >
> > Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> 
> Just my first impression, as I am in the middle of unrelated
> bisect.  I haven't read beyond diff-lib.c changes.
> 
> > diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
> > index 0b591c8..e71841a 100644
> > --- a/builtin-diff-tree.c
> > +++ b/builtin-diff-tree.c
> > @@ -118,12 +118,12 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
> >  	}
> >  
> >  	if (!read_stdin)
> > -		return opt->diffopt.exit_with_status ?
> > -		    opt->diffopt.has_changes: 0;
> > +		return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
> > +			&& DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);
> 
> Had to think a bit about this, although it is correct.
> 
> >  	if (opt->diffopt.detect_rename)
> >  		opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
> > -				       DIFF_SETUP_USE_CACHE);
> > +							   DIFF_SETUP_USE_CACHE);
> 
> I wonder what this is about.

  err I code with tabs of size 4 and I believe my editor was
over-zealous when I asked to reindent some part that I changed :P

> > diff --git a/combine-diff.c b/combine-diff.c
> > index fe5a2a1..3cab04b 100644
> > --- a/combine-diff.c
> > +++ b/combine-diff.c
> > @@ -664,7 +664,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
> >  	int mode_differs = 0;
> >  	int i, show_hunks;
> >  	int working_tree_file = is_null_sha1(elem->sha1);
> > -	int abbrev = opt->full_index ? 40 : DEFAULT_ABBREV;
> > +        int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
> 
> Indent?

  will fix.

> > diff --git a/diff-lib.c b/diff-lib.c
> > index da55713..69b5dc9 100644
> > --- a/diff-lib.c
> > +++ b/diff-lib.c
> > @@ -188,8 +188,7 @@ static int handle_diff_files_args(struct rev_info *revs,
> >  		else if (!strcmp(argv[1], "-n") ||
> >  				!strcmp(argv[1], "--no-index")) {
> >  			revs->max_count = -2;
> > -			revs->diffopt.exit_with_status = 1;
> > -			revs->diffopt.no_index = 1;
> > +			revs->diffopt.flags |= DIFF_OPT_EXIT_WITH_STATUS | DIFF_OPT_NO_INDEX;
> >  		}
> 
> Now this looks harder to read that everybody else uses
> DIFF_OPT_SET() for this, without DIFF_OPT_ prefix for the
> bitmask names.

  that could be splitted in two DIFF_OPT_SET indeed. will do.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH 4/3] git-fetch: test avoiding unnecessary copying from alternates
From: Shawn O. Pearce @ 2007-11-08  8:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This test verifies my prior "avoid local fetching from alternate"
patch is functional and doesn't regress in the future during any
additional improvements made to git.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 t/t5502-quickfetch.sh |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/t/t5502-quickfetch.sh b/t/t5502-quickfetch.sh
index b4760f2..16eadd6 100755
--- a/t/t5502-quickfetch.sh
+++ b/t/t5502-quickfetch.sh
@@ -86,4 +86,37 @@ test_expect_success 'quickfetch should not leave a corrupted repository' '
 
 '
 
+test_expect_success 'quickfetch should not copy from alternate' '
+
+	(
+		mkdir quickclone &&
+		cd quickclone &&
+		git init-db &&
+		(cd ../.git/objects && pwd) >.git/objects/info/alternates &&
+		git remote add origin .. &&
+		git fetch -k -k
+	) &&
+	obj_cnt=$( (
+		cd quickclone &&
+		git count-objects | sed -e "s/ *objects,.*//"
+	) ) &&
+	pck_cnt=$( (
+		cd quickclone &&
+		git count-objects -v | sed -n -e "/packs:/{
+				s/packs://
+				p
+				q
+			}"
+	) ) &&
+	origin_master=$( (
+		cd quickclone &&
+		git rev-parse origin/master
+	) ) &&
+	echo "loose objects: $obj_cnt, packfiles: $pck_cnt" &&
+	test $obj_cnt -eq 0 &&
+	test $pck_cnt -eq 0 &&
+	test z$origin_master = z$(git rev-parse master)
+
+'
+
 test_done
-- 
1.5.3.5.1590.gfadfad

^ permalink raw reply related

* Re: Inconsistencies with git log
From: Andreas Ericsson @ 2007-11-08  0:11 UTC (permalink / raw)
  To: David Symonds
  Cc: Brian Gernhardt, Jon Smirl, Johannes Schindelin, Git Mailing List
In-Reply-To: <ee77f5c20711071531q5acc4d06u264f5daad7c04cc4@mail.gmail.com>

David Symonds wrote:
> On Nov 8, 2007 10:19 AM, Brian Gernhardt <benji@silverinsanity.com> wrote:
>> However, Dave's suggestion of altering git-status output to be
>> relative to (but not limited by) CWD has merit.  Too bad I don't have
>> time to work on it right now.
> 
> I am happy to hack on this if there's not widespread revolt against the concept.
> 

I'd definitely like that feature, but I wonder how many people will run
"git commit -a" in a subdir after seeing only what they want to see in the
output, and then accidentally committing junk somewhere else in the repo.

So perhaps git-commit -a should also be path-delimited, but where would we
end up then? It might be better to just let git-status accept a path
delimiter and let the path delimiter default to current work-dir.

-- 
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: Andreas Ericsson @ 2007-11-08  0:16 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <9e4733910711071609t3e5412f1mf02e501b2d820bb3@mail.gmail.com>

Jon Smirl wrote:
> On 11/7/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> It is consistent, when you realise that the path arguments are interpreted
>> relative to the project root.
> 
> Then why doesn't this work?
> 
> jonsmirl@terra:~/mpc5200b$ git log Documentation
> all the log for Documentation....
> jonsmirl@terra:~/mpc5200b$ cd Documentation
> jonsmirl@terra:~/mpc5200b/Documentation$ git log Documentation
> fatal: ambiguous argument 'Documentation': unknown revision or path
> not in the working tree.
> Use '--' to separate paths from revisions
> jonsmirl@terra:~/mpc5200b/Documentation$
> 

Because your current working directory, relative to the project root, is
prepended to the path you're in, so git sees "Documentation/Documentation".
I'm unsure why

	cd Documentation; git log -- /Documentation

doesn't do the trick though. I know that particular trick used to work for
some other command a while back anyways.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH 2/2] git status: show relative paths when run in a subdirectory
From: Junio C Hamano @ 2007-11-08  8:26 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: David Symonds, Brian Gernhardt, Jon Smirl, Git Mailing List,
	gitster
In-Reply-To: <Pine.LNX.4.64.0711080011170.4362@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> -static const char *quote_crlf(const char *in, char *buf, size_t sz)
> +static const char *quote_crlf(const char *in, int len, char *buf, size_t sz,
> +	const char *prefix)
>  {

This is not quote_*crlf* anymore.

> @@ -118,8 +150,8 @@ static void wt_status_print_filepair(struct wt_status *s,
>  	const char *one, *two;
>  	char onebuf[PATH_MAX], twobuf[PATH_MAX];
>  
> -	one = quote_crlf(p->one->path, onebuf, sizeof(onebuf));
> -	two = quote_crlf(p->two->path, twobuf, sizeof(twobuf));
> +	one = quote_crlf(p->one->path, -1, onebuf, sizeof(onebuf), s->prefix);
> +	two = quote_crlf(p->two->path, -1, twobuf, sizeof(twobuf), s->prefix);

I wonder if it makes more sense to use strbuf here...

^ permalink raw reply

* Re: Inconsistencies with git log
From: Peter Baumann @ 2007-11-08  8:29 UTC (permalink / raw)
  To: Andreas Ericsson
  Cc: David Symonds, Brian Gernhardt, Jon Smirl, Johannes Schindelin,
	Git Mailing List
In-Reply-To: <47325415.1070205@op5.se>

On Thu, Nov 08, 2007 at 01:11:01AM +0100, Andreas Ericsson wrote:
> David Symonds wrote:
>> On Nov 8, 2007 10:19 AM, Brian Gernhardt <benji@silverinsanity.com> wrote:
>>> However, Dave's suggestion of altering git-status output to be
>>> relative to (but not limited by) CWD has merit.  Too bad I don't have
>>> time to work on it right now.
>>
>> I am happy to hack on this if there's not widespread revolt against the concept.
>>
>
> I'd definitely like that feature, but I wonder how many people will run
> "git commit -a" in a subdir after seeing only what they want to see in the
> output, and then accidentally committing junk somewhere else in the repo.
>
> So perhaps git-commit -a should also be path-delimited, but where would we
> end up then? It might be better to just let git-status accept a path
> delimiter and let the path delimiter default to current work-dir.
>

I agree that 'git status' should show the *whole* tree and if it will work
in subdirectories with 'git status .' or 'git status Documentation', it
would be a nice UI improvement.

But please don't make it always show only the current subdir.

-Peter

^ permalink raw reply

* Re: [PATCH] git-checkout: Handle relative paths containing "..".
From: Junio C Hamano @ 2007-11-08  8:30 UTC (permalink / raw)
  To: David Symonds; +Cc: git, Johannes Schindelin
In-Reply-To: <1194489192-20021-1-git-send-email-dsymonds@gmail.com>

David Symonds <dsymonds@gmail.com> writes:

> diff --git a/git-checkout.sh b/git-checkout.sh
> index 8993920..b2c50aa 100755
> --- a/git-checkout.sh
> +++ b/git-checkout.sh
> @@ -134,9 +134,10 @@ Did you intend to checkout '$@' which can not be resolved as commit?"
>  	fi
>  
>  	# Make sure the request is about existing paths.
> -	git ls-files --error-unmatch -- "$@" >/dev/null || exit
> -	git ls-files -- "$@" |
> -	git checkout-index -f -u --stdin
> +	git ls-files --full-name --error-unmatch -- "$@" >/dev/null || exit
> +	git ls-files --full-name -- "$@" |
> +		(cd "$(git-rev-parse --show-cdup)" &&
> +		 git checkout-index -f -u --stdin)

Have you tested this patch from the toplevel of any tree, where
"git-rev-parse --show-cdup" would yield an empty string?

I also wonder how this patch (with an obvious fix to address the
above point) would interact with GIT_DIR and/or GIT_WORK_TREE in
the environment.

^ permalink raw reply

* Re: [PATCH 4/3] git-fetch: test avoiding unnecessary copying from alternates
From: Shawn O. Pearce @ 2007-11-08  8:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20071108082213.GA17054@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> This test verifies my prior "avoid local fetching from alternate"
> patch is functional and doesn't regress in the future during any
> additional improvements made to git.
...
> +test_expect_success 'quickfetch should not copy from alternate' '
> +
> +	(
> +		mkdir quickclone &&
> +		cd quickclone &&
> +		git init-db &&
> +		(cd ../.git/objects && pwd) >.git/objects/info/alternates &&
> +		git remote add origin .. &&
> +		git fetch -k -k

Hmmph.  On second thought I think this is a little sketchy for
a test.  Versions without my quickfetch patch fail this test and
versions with it pass.  But it depends on the implementation of
`-k -k` to always call index-pack over unpack-objects.

I'm using -k -k here to ensure we keep the pack fetched as we're only
fetching 6 objects and they are already reachable in the quickclone
repository thanks to the alternate ODB.  If we use unpack-objects
during this fetch we will still pass this test because the objects
won't be unpacked if they are already reachable locally.

Of course this test is for a performance optimization.  For 6
tiny objects it really doesn't matter if we copy them or not, or
if we copy them over a pipe only to discard them because they are
already reachable.  It does however matter when you are talking
about nearly 300MB worth of objects.  :-\

-- 
Shawn.

^ 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

* 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 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 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] 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] 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: 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: 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: [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: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: [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: [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: 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

* [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: [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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox