Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2 0/2] user-manual: new "getting started" section
From: Nanako Shiraishi @ 2009-11-17 12:06 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Michael J Gruber, Jonathan Nieder, Junio C Hamano, git,
	J. Bruce Fields, Hannu Koivisto, Jeff King, Wincent Colaiuta,
	Matthias Lederhofer
In-Reply-To: <94a0d4530911161452xe82858el322a1985341bf13c@mail.gmail.com>

Quoting Felipe Contreras <felipe.contreras@gmail.com>

> On Fri, Nov 13, 2009 at 11:06 PM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
>> ... I don't
>> think Felipe seriously wants to change them to --gogo vs --dance, but
>> if he made a more constructive proposal, instead of making such a
>> comment whose intended effect is only to annoy people, we may see
>> an improved UI at the end.  Proposing "--index-only" vs "--index-too"
>> or even "--stage-only" vs "--stage-too" would have helped him appear
>> to be more serious and constructive and I think your expression
>> "mismatching participants" was a great way to say this.
>
> Right, your explanation is more clear:

You have a funny way of saying "I'm sorry, I wasn't constructive, 
and my attitude repelled many participants from the discussion".

> the fact that we need both
> doesn't mean we cannot use the term "stage". As to "constructive
> proposal" I deliberately tried to avoid them in case somebody tried to
> disregard it as bike-shedding, and move on.

If the only constructive proposal you could make is to replace 
words used in two operations without clarifying concepts any 
better to newbies, then what you are doing is bike-shedding. 
I don't think trying to hide that by not making any proposal 
changes that.

> What I'm trying to do is
> bring up the issue that the stage is not user friendly.

I thought you were the one who wanted to use "stage" everywhere?

For what it's worth, "stage" isn't very user friendly to me; 
maybe it is because I'm not a native English speaker. I'm not 
saying that when I hear "index" or "cached" I'll understand 
what they mean even if I didn't have any prior knowledge of 
git, but I am saying "stage" isn't any better than these two 
words in that respect. Of course the user needs to understand 
what it is and how it is used, no matter what word you use.

I think a proposal to replace the word "index" with "stage" 
will sound nothing but bike-shedding to anybody, especially 
after getting familiar with "index" and seeing it taught on 
many web pages and books.

> I like David Kågedal's suggestion more:
> http://kerneltrap.org/mailarchive/git/2008/10/29/3857134

For people who like a usable threaded interface to read 
the message in context here is its URL.

http://thread.gmane.org/gmane.comp.version-control.git/99332/focus=99401

Yes, I had David's proposal in mind when I wrote my response. 
Even though the fundamental idea is the same, I used --X-vs-Y 
option to avoid the problems David's proposal has in a slightly 
nicer way.

David's proposal introduced two magic tokens STAGE and WORKTREE.

  git diff STAGE WORKTREE   (like "git diff" today)
  git diff HEAD WORKTREE    (like "git diff HEAD" today)
  git diff WORKTREE HEAD    (like "git diff -R HEAD" today)
  git diff HEAD STAGE       (like "git diff --cached" today)
  git diff commit STAGE     (like "git diff --cached commit" today)

This looks nice on surface, but I think the apparent niceness 
is shallow. If of course has a small problem of introducing an 
obvious backward incompatibility. You can't use a branch whose 
name is STAGE anymore, but a deeper problem is that these two 
magic tokens pretend to be refs. But they do so only to the diff 
command. I don't see how you can make them sanely be usable to 
other commands like "git log v1.0.0..WORKTREE".

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: [ANNOUNCE] GIT 1.6.5.3
From: Jakub Narebski @ 2009-11-17 10:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4ootltzm.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> The RPM binary packages for a few architectures are found in:
> 
>   RPMS/$arch/git-*-1.6.5.3-1.fc9.$arch.rpm	(RPM)

It's RPMS/$arch/git-*-1.6.5.3-1.fc11.$arch.rpm	(RPM)
 
> Git v1.6.5.3 Release Notes
> ==========================
> 
> Fixes since v1.6.5.2
> --------------------

>  * Packages generated on newer FC were unreadable by older versions of
>    RPM as the new default is to use stronger hash.

Thanks.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: bash completion on 4.0 broken?
From: Michael J Gruber @ 2009-11-17  9:54 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: git, Shawn O. Pearce
In-Reply-To: <4B0246C7.6020401@gmail.com>

Stephen Boyd venit, vidit, dixit 17.11.2009 07:46:
> When I try
> 
>     git show --pretty=<TAB><TAB>
> 
> I get a list of filenames and not the list of pretty formats.
> 
> I've debugged a little and see that the cur variable in _git_show () is 
> set to '=' when it should be '--pretty='. So it looks like something is 
> causing the command line to be split weirdly. Looking at the bash 
> NEWS[1] for 4.0 I see
> 
> i.  The programmable completion code now uses the same set of characters as
>     readline when breaking the command line into a list of words.
> 
> 
> which causes me to believe this is why it's broken now. I've tried 
> removing '=' from COMP_WORDBREAKS and that shows the list of formats 
> correctly, but then causes the entire '--pretty=' to be replaced with 
> the selected format.
> 
> Anyone else seeing the same problem or is my system just b0rked?
> 
> $ bash --version
> GNU bash, version 4.0.35(2)-release (x86_64-pc-linux-gnu)
> 
> References:
> [1] http://tiswww.case.edu/php/chet/bash/NEWS

Ouch, just when I decided to use completion rather than a bunch of
aliases it stops working. B0rked here 0ls0 on Fedora 12 (with git.git's
next):

GNU bash, Version 4.0.33(1)-release (x86_64-redhat-linux-gnu)

Command name completion and prompt magic do work.

Michael

^ permalink raw reply

* Re: [PATCH 1/2] replace: use a GIT_NO_REPLACE_OBJECTS env variable
From: Michael J Gruber @ 2009-11-17  9:33 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, git, Jakub Narebski, Johannes Sixt, bill lam,
	Andreas Schwab, Paul Mackerras
In-Reply-To: <20091117051125.3588.91072.chriscool@tuxfamily.org>

Christian Couder venit, vidit, dixit 17.11.2009 06:11:
> This environment variable is set when the --no-replace-objects
> flag is passed to git, and it is read when other environment
> variables are read.
> 
> It is useful for example for scripts, as the git commands used in
> them can now be aware that they must not read replace refs.
> 
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>

Tested-by: Michael J Gruber <git@drmicha.warpmail.net>

:) This works, thanks, as well as the gitk patch 2/2, which is difficult
to cover by test scripts. Some OSes (or rather certain setenv/putenv
variants) have problems distinguishing an unset variable from an empty
one. I think we've worked around this, but avoiding it is safer, as J6t
pointed out.

Thanks!
Michael

^ permalink raw reply

* Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template
From: Jakub Narebski @ 2009-11-17  9:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matthieu Moy, git, Karl Chen
In-Reply-To: <7viqd9isbm.fsf@alter.siamese.dyndns.org>

Dnia wtorek 17. listopada 2009 07:22, Junio C Hamano napisał:
> Jakub Narebski <jnareb@gmail.com> writes:

> >
> > It would be nice to have an option to git-config which would do such
> > expansion, as a separate type similar to --int and --bool, e.g.:
> >
> >   git config --path section.key
> >
> > so that not only core.excludesfile could use this new feature, but for
> > example also core.worktree, commit.template, gitcvs.logfile,
> > mailmap.file, and perhaps also *.receivepack and *.uploadpack
> 
> What should "git config -l" do for these (and core.excludesfile)?

Nothing (i.e. show unexpanded / not converted), just like for boolean
variables "git config -l" doesn't convert 1/on/yes to true, and 0/off/no
to false, just like for integer variables "git config -l" doesn't convert
to simple decimal number taking into account optional 'k', 'm' or 'g'
suffix.


BTW. the suffix part of integer conversion should really be described
in the paragraph about --int and --bool options (which should be made
into proper description list, and not a prose paragraph).

P.S. I am a bit missing --local / --repository option to git-config to
complement --global (which should probably be named --user) and --system
options.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template
From: Matthieu Moy @ 2009-11-17  8:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Karl Chen
In-Reply-To: <7vy6m5hci0.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> I'd like to squash this to your patch, based on the earlier review
> comments.

Sure. And apologize for the style issues, I knew it, but one hardly
changes his (bad) habits ;-).

I'll resend soon.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template
From: Matthieu Moy @ 2009-11-17  8:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, git, Karl Chen
In-Reply-To: <7viqd9isbm.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Jakub Narebski <jnareb@gmail.com> writes:
>
>> It would be nice to have an option to git-config which would do such
>> expansion, as a separate type similar to --int and --bool, e.g.:
>>
>>   git config --path section.key
>>
>> so that not only core.excludesfile could use this new feature, but for
>> example also core.worktree, commit.template, gitcvs.logfile,
>> mailmap.file, and perhaps also *.receivepack and *.uploadpack
>
> What should "git config -l" do for these (and core.excludesfile)?

I don't know what it "should", but it "does" not do the expansion. I
had the same questionning when testing the patch, I'd have liked to be
able to write a simple test-case like

$ git config core.excludesfile '~/foo'
$ git config --i-dont-know-what core.excludesfile

to go through this codepath. Maybe we can just say

$ git config --default core.excludesfile

to say "call git_default_config(...) on this before printing it". My
understanding is that this is what the C code is doing, we should
allow the shell scripts to do the same.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template
From: Jeff King @ 2009-11-17  8:56 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, Karl Chen
In-Reply-To: <vpqpr7hpm5b.fsf@bauges.imag.fr>

On Tue, Nov 17, 2009 at 09:53:52AM +0100, Matthieu Moy wrote:

> Yes. "user" in the sentence is either the user running Git or the same
> string as "user" in "~user". I'm not against your proposal, but I'm
> afraid we're making the sentence uselessly heavy, since, as you say,
> this ~ and ~user convention is widely spread, and I hardly imagine
> someone interpreting the sentence as "if you say ~foo, it will expand
> to the home directory of bar".

I didn't think it would expand ~foo to the home directory of bar. I
thought that it might _only_ accept ~bar, and not ~foo.

If I'm the only one confused, then we can drop it. But if not, I don't
think it is much more work to be precise.

-Peff

^ permalink raw reply

* Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template
From: Matthieu Moy @ 2009-11-17  8:53 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Karl Chen
In-Reply-To: <20091117073426.GB4007@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Nov 16, 2009 at 11:07:40AM +0100, Matthieu Moy wrote:
>
>> +	of files which are not meant to be tracked.  "~" and "~user"
>> +	are expanded to the user's home directory.  See
>>  	linkgit:gitignore[5].
>
> Reading this, it is not clear to me if:
>
>   1. "~" and "~user" are expanded to the home directory of "user", where
>      "user" is the user running git
>
> or
>
>   2. "~" is expanded to the home directory of the user running git, and
>      an arbitrary "~user" is expanded to that user's home directory.
>
> I would expect (2), since that is how everything else works.

Yes. "user" in the sentence is either the user running Git or the same
string as "user" in "~user". I'm not against your proposal, but I'm
afraid we're making the sentence uselessly heavy, since, as you say,
this ~ and ~user convention is widely spread, and I hardly imagine
someone interpreting the sentence as "if you say ~foo, it will expand
to the home directory of bar".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* [PATCH] Make sure $PERL_PATH is defined when the test suite is run.
From: Philippe Bruhat (BooK) @ 2009-11-17  8:42 UTC (permalink / raw)
  To: git; +Cc: Philippe Bruhat (BooK)
In-Reply-To: <4B025F19.1050809@viscovery.net>

Some test scripts run Perl scripts as if they were git-* scripts, and
thus need to use the same perl that will be put in the shebang line of
git*.perl commands. $PERL_PATH therefore needs to be used instead of
a bare "perl".

The tests can fail if another perl is found in $PATH before the one
defined in $PERL_PATH.

Example test failure caused by this: the perl defined in $PERL_PATH has
Error.pm installed, and therefore the Git.pm's Makefile.PL doesn't install
the private copy. The perl from $PATH doesn't have Error.pm installed, and
all git*.perl scripts invoked during the test will fail loading Error.pm.

Makefile patch by Jeff King <peff@peff.net>.

Signed-off-by: Philippe Bruhat (BooK) <book@cpan.org>
---
 Makefile                        |    1 +
 t/t9400-git-cvsserver-server.sh |    2 +-
 t/t9401-git-cvsserver-crlf.sh   |    2 +-
 t/t9700-perl-git.sh             |    4 ++--
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 35f5294..287c7fc 100644
--- a/Makefile
+++ b/Makefile
@@ -1633,6 +1633,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
 # and the first level quoting from the shell that runs "echo".
 GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
 	@echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' >$@
+	@echo PERL_PATH=\''$(subst ','\'',$(PERL_PATH_SQ))'\' >>$@
 	@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@
 	@echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' >>$@
 	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 64f947d..c2ec3cb 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -20,7 +20,7 @@ then
     say 'skipping git-cvsserver tests, cvs not found'
     test_done
 fi
-perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+"$PERL_PATH" -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
 }
diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh
index aca40c1..40637d6 100755
--- a/t/t9401-git-cvsserver-crlf.sh
+++ b/t/t9401-git-cvsserver-crlf.sh
@@ -57,7 +57,7 @@ then
     say 'skipping git-cvsserver tests, perl not available'
     test_done
 fi
-perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+"$PERL_PATH" -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
 }
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 4eb7d3f..8686086 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -11,7 +11,7 @@ if ! test_have_prereq PERL; then
 	test_done
 fi
 
-perl -MTest::More -e 0 2>/dev/null || {
+"$PERL_PATH" -MTest::More -e 0 2>/dev/null || {
 	say "Perl Test::More unavailable, skipping test"
 	test_done
 }
@@ -48,6 +48,6 @@ test_expect_success \
 
 test_external_without_stderr \
     'Perl API' \
-    perl "$TEST_DIRECTORY"/t9700/test.pl
+    "$PERL_PATH" "$TEST_DIRECTORY"/t9700/test.pl
 
 test_done
-- 
1.6.0.3.517.g759a

^ permalink raw reply related

* Re: [PATCH] Make sure $PERL_PATH is defined when the test suite is run.
From: Philippe Bruhat (BooK) @ 2009-11-17  8:35 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <4B025F19.1050809@viscovery.net>

On Tue, Nov 17, 2009 at 09:30:17AM +0100, Johannes Sixt wrote:
> Philippe Bruhat (BooK) schrieb:
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1633,6 +1633,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
> >  # and the first level quoting from the shell that runs "echo".
> >  GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
> >  	@echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' >$@
> > +	@echo PERL_PATH=\''$(subst ','\'',$(PERL_PATH_SQ))'\' >$@
> 
> Make it: ... >>$@

This proves late commits needs many extra pair of eyes. :-)

> >  	@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@
> >  	@echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' >>$@
> >  	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@
> 
> >  test_external_without_stderr \
> >      'Perl API' \
> > -    perl "$TEST_DIRECTORY"/t9700/test.pl
> > +    $PERL_PATH "$TEST_DIRECTORY"/t9700/test.pl
> 
> This one needs the double-quotes as well.

Thanks. Sending again. (sorry for the noise)

-- 
 Philippe Bruhat (BooK)

 "Did I err?"      (Groo, in too many issues to count - ...and *YES* he did!)

^ permalink raw reply

* Re: [PATCH] Make sure $PERL_PATH is defined when the test suite is run.
From: Johannes Sixt @ 2009-11-17  8:30 UTC (permalink / raw)
  To: Philippe Bruhat (BooK); +Cc: git
In-Reply-To: <1258417206-5406-1-git-send-email-book@cpan.org>

Philippe Bruhat (BooK) schrieb:
> --- a/Makefile
> +++ b/Makefile
> @@ -1633,6 +1633,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
>  # and the first level quoting from the shell that runs "echo".
>  GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
>  	@echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' >$@
> +	@echo PERL_PATH=\''$(subst ','\'',$(PERL_PATH_SQ))'\' >$@

Make it: ... >>$@

>  	@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@
>  	@echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' >>$@
>  	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@

>  test_external_without_stderr \
>      'Perl API' \
> -    perl "$TEST_DIRECTORY"/t9700/test.pl
> +    $PERL_PATH "$TEST_DIRECTORY"/t9700/test.pl

This one needs the double-quotes as well.

-- Hannes

^ permalink raw reply

* Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template
From: Mike Hommey @ 2009-11-17  7:49 UTC (permalink / raw)
  To: Jeff King; +Cc: Matthieu Moy, git, Karl Chen
In-Reply-To: <20091117073426.GB4007@coredump.intra.peff.net>

On Tue, Nov 17, 2009 at 02:34:26AM -0500, Jeff King wrote:
> Maybe:
> 
>   A leading path component of "~user" is expanded to the home directory
>   of "user"; "~" is expanded to the home directory of the user running
>   git.
> 
> would be more clear?

Add "real" before "user running git" and you have my vote. Or maybe
using the effective user would be better, and the patch should use
geteuid() instead of getuid(), I don't know. ident.c uses getuid(), but
I'm wondering if that's what it should use (although it doesn't seem to
have been a problem to anyone)

Mike

^ permalink raw reply

* Re: [PATCH] Doc: mention the crlf attribute in config autocrlf section
From: Matthew Ogilvie @ 2009-11-17  7:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7v7htpirmy.fsf@alter.siamese.dyndns.org>

On Mon, Nov 16, 2009 at 10:37:09PM -0800, Junio C Hamano wrote:
> How about this?  I didn't touch the patch text (other than dropping
> trailing whitespaces).
> 
> commit ff68668695486b72b5f06146eddf85b70841088a
> Author: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
> Date:   Sat Nov 14 11:35:00 2009 -0700
> 
>     core.autocrlf documentation: mention the crlf attribute
>     
>     The description of the configuration variable is obsolete and
>     wrong (saying only file content is used), not just incomplete.
>     It has used the attribute mechanism for a long time.
>     
>     The documentation of gitattributes mentions the core.autocrlf
>     configuration variable in its description of crlf attribute.
>     Refer to the gitattributes documentation from here as well.
>     
>     Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>

Looks good.  Sorry about the extra space in the patch text;
stripping it sounds good as well.

--
Matthew Ogilvie   [mmogilvi_git@miniinfo.net]

^ permalink raw reply

* Re: [PATCH 1/2] replace: use a GIT_NO_REPLACE_OBJECTS env variable
From: Johannes Sixt @ 2009-11-17  7:42 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, git, Michael J Gruber, Jakub Narebski, bill lam,
	Andreas Schwab, Paul Mackerras
In-Reply-To: <20091117051125.3588.91072.chriscool@tuxfamily.org>

Christian Couder schrieb:
> diff --git a/git.c b/git.c
> index bd2c5fe..7f7d73d 100644
> --- a/git.c
> +++ b/git.c
> @@ -89,6 +89,9 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
>  				*envchanged = 1;
>  		} else if (!strcmp(cmd, "--no-replace-objects")) {
>  			read_replace_refs = 0;
> +			setenv(NO_REPLACE_OBJECTS_ENVIRONMENT, "", 1);

It is safer to set to a non-empty string, e.g., "1".

I think this variable should be added to the list in connect.c around line
630 so that it does not propagate to the remote end.

-- Hannes

^ permalink raw reply

* Re: [PATCH] Document git-svn's first-parent rule
From: Eric Wong @ 2009-11-17  7:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, git
In-Reply-To: <7vd43his6n.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > Thomas Rast <trast@student.ethz.ch> wrote:
> >> git-svn has the following rule to detect the SVN base for its
> >> operations: find the first git-svn-id line reachable through
> >> first-parent ancestry.  IOW,
> >> 
> >>   git log --grep=^git-svn-id: --first-parent -1
> >> 
> >> Document this, as it is very important when using merges with git-svn.
> >> 
> >> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> >
> > Thanks Thomas,
> >
> > Acked-by: Eric Wong <normalperson@yhbt.net>
> 
> Thanks; is it a good time to pull from your bogomips repository to get
> accumulated changes?

Now is, I just pushed to it:

Eric Wong (3):
      git svn: read global+system config for clone+init
      git svn: add authorsfile test case for ~/.gitconfig
      git svn: attempt to create empty dirs on clone+rebase

Thomas Rast (1):
      Document git-svn's first-parent rule

Toby Allsopp (1):
      git svn: handle SVN merges from revisions past the tip of the branch

HEAD=ce45a45f24cc7b3ccc7f6ebcd0025559b4421bda

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings
From: Jeff King @ 2009-11-17  7:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Brian Collins, Nanako Shiraishi, git
In-Reply-To: <7v639am2uq.fsf@alter.siamese.dyndns.org>

On Mon, Nov 16, 2009 at 04:06:37PM -0800, Junio C Hamano wrote:

> By the way, I would suggest updating the test like the attached.
> 
> By looking for rEtUrN, you will catch a bug that breaks "-i"-ness
> of the grep, but your test does not catch breakages in "-F"-ness.

Your change looks good to me.

-Peff

^ permalink raw reply

* Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template
From: Jeff King @ 2009-11-17  7:34 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, Karl Chen
In-Reply-To: <1258366060-27966-1-git-send-email-Matthieu.Moy@imag.fr>

On Mon, Nov 16, 2009 at 11:07:40AM +0100, Matthieu Moy wrote:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index d1e2120..c37b51d 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -380,7 +380,8 @@ Common unit suffixes of 'k', 'm', or 'g' are supported.
>  core.excludesfile::
>  	In addition to '.gitignore' (per-directory) and
>  	'.git/info/exclude', git looks into this file for patterns
> -	of files which are not meant to be tracked.  See
> +	of files which are not meant to be tracked.  "~" and "~user"
> +	are expanded to the user's home directory.  See
>  	linkgit:gitignore[5].

Reading this, it is not clear to me if:

  1. "~" and "~user" are expanded to the home directory of "user", where
     "user" is the user running git

or

  2. "~" is expanded to the home directory of the user running git, and
     an arbitrary "~user" is expanded to that user's home directory.

I would expect (2), since that is how everything else works. And it
seems from the code that is what you do. But something about the way it
is written makes me think of (1). I also recall having this same
question during the last round, so at the very least it is not me just
mis-reading it once. ;)

Maybe:

  A leading path component of "~user" is expanded to the home directory
  of "user"; "~" is expanded to the home directory of the user running
  git.

would be more clear?

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] ls-tree: migrate to parse-options
From: Stephen Boyd @ 2009-11-17  7:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpr7hisc7.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
>
> @@ -24,7 +24,7 @@ static int chomp_prefix;
>  static const char *ls_tree_prefix;
>  
>  static const  char * const ls_tree_usage[] = {
> -	"git ls-tree [-d] [-r] [-t] [-l | --long] [-z] [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]] <tree-ish> [path...]",
> +	"git ls-tree <options> <tree-ish> [path...]",
>  	NULL
>  };
>  

Is it [<options>] or [<options>...] or <options> or even <options>... 
though?

^ permalink raw reply

* Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template
From: Junio C Hamano @ 2009-11-17  6:49 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, Karl Chen
In-Reply-To: <7v3a4enkzt.fsf@alter.siamese.dyndns.org>

I'd like to squash this to your patch, based on the earlier review
comments.

I didn't mention about the first hunk; it is just a style.  An opening
brace that begins a function body comes at column 1.

Also first_slash and to_copy are made const pointers, as they do not have
to touch the region of memory they point to (otherwise you cannot assign
path to to_copy without getting warned).

 config.c |    3 ++-
 path.c   |   32 ++++++++++++++------------------
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/config.c b/config.c
index 0fcc4ce..b3d1ff4 100644
--- a/config.c
+++ b/config.c
@@ -351,7 +351,8 @@ int git_config_string(const char **dest, const char *var, const char *value)
 	return 0;
 }
 
-int git_config_pathname(const char **dest, const char *var, const char *value) {
+int git_config_pathname(const char **dest, const char *var, const char *value)
+{
 	if (!value)
 		return config_error_nonbool(var);
 	*dest = expand_user_path(value);
diff --git a/path.c b/path.c
index 009c8e0..2470f78 100644
--- a/path.c
+++ b/path.c
@@ -208,11 +208,8 @@ int validate_headref(const char *path)
 	return -1;
 }
 
-static inline struct passwd *getpw_str(const char *username, size_t len)
+static struct passwd *getpw_str(const char *username, size_t len)
 {
-	if (len == 0)
-		return getpwuid(getuid());
-
 	struct passwd *pw;
 	char *username_z = xmalloc(len + 1);
 	memcpy(username_z, username, len);
@@ -223,18 +220,18 @@ static inline struct passwd *getpw_str(const char *username, size_t len)
 }
 
 /*
- * Return a string with ~ and ~user expanded via getpw*.  If buf != NULL, then
- * it is a newly allocated string. Returns NULL on getpw failure or if
- * path is NULL.
+ * Return a string with ~ and ~user expanded via getpw*.  If buf != NULL,
+ * then it is a newly allocated string. Returns NULL on getpw failure or
+ * if path is NULL.
  */
 char *expand_user_path(const char *path)
 {
 	struct strbuf user_path = STRBUF_INIT;
-	char * first_slash = strchrnul(path, '/');
-	char * to_copy;
+	const char *first_slash = strchrnul(path, '/');
+	const char *to_copy = path;
+
 	if (path == NULL)
 		goto return_null;
-
 	if (path[0] == '~') {
 		const char *username = path + 1;
 		size_t username_len = first_slash - username;
@@ -243,8 +240,6 @@ char *expand_user_path(const char *path)
 			goto return_null;
 		strbuf_add(&user_path, pw->pw_dir, strlen(pw->pw_dir));
 		to_copy = first_slash;
-	} else if (path[0] != '/') {
-		to_copy = path;
 	}
 	strbuf_add(&user_path, to_copy, strlen(to_copy));
 	return strbuf_detach(&user_path, NULL);
@@ -300,14 +295,15 @@ char *enter_repo(char *path, int strict)
 		if (path[0] == '~') {
 			char *newpath = expand_user_path(path);
 			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
-				if (path != newpath)
-					free(newpath);
+				free(newpath);
 				return NULL;
 			}
-			/* Copy back into the static buffer. A pity
-			   since newpath was not bounded, but other
-			   branches of the if are limited by PATH_MAX
-			   anyway. */
+			/*
+			 * Copy back into the static buffer. A pity
+			 * since newpath was not bounded, but other
+			 * branches of the if are limited by PATH_MAX
+			 * anyway.
+			 */
 			strcpy(used_path, newpath); free(newpath);
 			strcpy(validated_path, path);
 			path = used_path;
-- 
1.6.5.3.283.g4b054

^ permalink raw reply related

* bash completion on 4.0 broken?
From: Stephen Boyd @ 2009-11-17  6:46 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce

When I try

    git show --pretty=<TAB><TAB>

I get a list of filenames and not the list of pretty formats.

I've debugged a little and see that the cur variable in _git_show () is 
set to '=' when it should be '--pretty='. So it looks like something is 
causing the command line to be split weirdly. Looking at the bash 
NEWS[1] for 4.0 I see

i.  The programmable completion code now uses the same set of characters as
    readline when breaking the command line into a list of words.


which causes me to believe this is why it's broken now. I've tried 
removing '=' from COMP_WORDBREAKS and that shows the list of formats 
correctly, but then causes the entire '--pretty=' to be replaced with 
the selected format.

Anyone else seeing the same problem or is my system just b0rked?

$ bash --version
GNU bash, version 4.0.35(2)-release (x86_64-pc-linux-gnu)

References:
[1] http://tiswww.case.edu/php/chet/bash/NEWS

^ permalink raw reply

* Re: [PATCH] Doc: mention the crlf attribute in config autocrlf section
From: Junio C Hamano @ 2009-11-17  6:37 UTC (permalink / raw)
  To: Matthew Ogilvie; +Cc: Nanako Shiraishi, git, gitster
In-Reply-To: <20091117035945.GA1728@comcast.net>

Matthew Ogilvie <mmogilvi_git@miniinfo.net> writes:

> On Mon, Nov 16, 2009 at 07:50:48PM +0900, Nanako Shiraishi wrote:
>> Quoting Matthew Ogilvie <mmogilvi_git@miniinfo.net>
>> 
>> > The reverse reference has long existed, and the autocrlf description
>> > was actually obsolete and wrong (saying only file content is used),
>> > not just incomplete.
>> 
>> What do you mean by "reverse reference"?
>
> I'm refering to the fact that the "crlf" section of
> Documentation/gitattributes.txt mentions core.autocrlf,
> which is in the opposite (reverse) direction as this new reference
> I'm adding.
> ...
> Do I need to resubmit the patch, in order to rephrase the commit
> message?

Thanks; I heard you, as your response was Cc'ed to me as well ;-)

How about this?  I didn't touch the patch text (other than dropping
trailing whitespaces).

commit ff68668695486b72b5f06146eddf85b70841088a
Author: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
Date:   Sat Nov 14 11:35:00 2009 -0700

    core.autocrlf documentation: mention the crlf attribute
    
    The description of the configuration variable is obsolete and
    wrong (saying only file content is used), not just incomplete.
    It has used the attribute mechanism for a long time.
    
    The documentation of gitattributes mentions the core.autocrlf
    configuration variable in its description of crlf attribute.
    Refer to the gitattributes documentation from here as well.
    
    Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

^ permalink raw reply

* Re: [PATCH 2/2] ls-tree: migrate to parse-options
From: Stephen Boyd @ 2009-11-17  6:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpr7hisc7.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Thanks.
>
> As parse-options infrastructure gives much better per-option help text,
> there is not much point to keep the list of options that can go stale
> in the usage text.
>
> I'd squash this to yours.  Ok?
>

I like this style too. Less usage update patches are better. Thanks.

^ permalink raw reply

* Re: [PATCH 1/2] t3101: test more ls-tree options
From: Stephen Boyd @ 2009-11-17  6:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vws1piscw.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Thanks.  I'd squash this patch in for the following reasons; Ok?

Looks good. Thanks.

I figured reusing the test_output function would be a bad idea.

^ permalink raw reply

* Re: [PATCH] Document git-svn's first-parent rule
From: Junio C Hamano @ 2009-11-17  6:25 UTC (permalink / raw)
  To: Eric Wong; +Cc: Thomas Rast, git
In-Reply-To: <20091116231455.GA13460@dcvr.yhbt.net>

Eric Wong <normalperson@yhbt.net> writes:

> Thomas Rast <trast@student.ethz.ch> wrote:
>> git-svn has the following rule to detect the SVN base for its
>> operations: find the first git-svn-id line reachable through
>> first-parent ancestry.  IOW,
>> 
>>   git log --grep=^git-svn-id: --first-parent -1
>> 
>> Document this, as it is very important when using merges with git-svn.
>> 
>> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
>
> Thanks Thomas,
>
> Acked-by: Eric Wong <normalperson@yhbt.net>

Thanks; is it a good time to pull from your bogomips repository to get
accumulated changes?

^ 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