Git development
 help / color / mirror / Atom feed
* Re: [PATCH] git(1): remove a defunct link to "list of authors"
From: Junio C Hamano @ 2012-12-07 17:59 UTC (permalink / raw)
  To: git
In-Reply-To: <7vobi5hhn9.fsf@alter.siamese.dyndns.org>

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

> The linked page has not been showing the promised "more complete
> list" for more than 6 months by now, and nobody has resurrected
> the list there nor elsewhere since then.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  * If somebody has a working replacement URL, we could use that
>    instead, of course.  Takers?

A possible alternative could be https://www.ohloh.net/p/git/contributors/summary

^ permalink raw reply

* Re: Weird problem with git-submodule.sh
From: Marc Branchaud @ 2012-12-07 18:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vvccdhhod.fsf@alter.siamese.dyndns.org>

On 12-12-07 12:54 PM, Junio C Hamano wrote:
> Marc Branchaud <marcnarc@xiplink.com> writes:
> 
>> This is with git 1.8.0.1 on all the machines involved.
>>
>> One of our build machines is having trouble with "git submodule":
>> ...
>> Any ideas?
> 
> How and why is the IFS set differently only on one of your build
> machines?

It's not.  On all machines:
	$ set | grep IFS
	IFS=$' \t\n'

As I said, if I isolate the module_list() function into another script it
works fine, with the exact same environment that breaks in git-submodule.sh.

Also, note that at the top of git-submodule there's
	. git-sh-setup
which does
	unset IFS

		M.

^ permalink raw reply

* Re: [PATCH 0/6] Improve remote helper documentation
From: Junio C Hamano @ 2012-12-07 19:09 UTC (permalink / raw)
  To: Max Horn; +Cc: git, Sverre Rabbelier, Florian Achleitner, Felipe Contreras
In-Reply-To: <1354038279-76475-1-git-send-email-max@quendi.de>

Max Horn <max@quendi.de> writes:

> Various remote helper capabilities and commands were not
> documented, in particular 'export', or documented in a misleading
> way (e.g. 'for-push' was listed as a ref attribute understood by
> git, which is not the case). This patch series changes that, and
> also address some other things in the remote helper documentation
> that I found jarring when reading through it.
>
> Note that the description of export and (im|ex)port-marks probably can be
> improved, and I hope that somebody who knows more about them
> than me and/or is better at writing documentation will do just that.
> But I felt it was better to provide something than to do nothing
> and only complain, as I did previously on this subject ;-).

A second ping to people who have touched transport-helper.c,
remote-testsvn.c, git-remote-testgit, and contrib/remote-helpers/ in
the past 18 months for comments.  I've re-read the documentation
updates myself and didn't find anything majorly objectionable.

Except for a minor nit in 6/6; I think "defined options" should be
"defined attributes".

    --- a/Documentation/git-remote-helpers.txt
    +++ b/Documentation/git-remote-helpers.txt
    @@ -227,6 +227,8 @@ Support for this command is mandatory.
            the name; unrecognized attributes are ignored. The list ends
            with a blank line.
     +
    +See REF LIST ATTRIBUTES for a list of currently defined options.
    ++

^ permalink raw reply

* Re: Weird problem with git-submodule.sh
From: Junio C Hamano @ 2012-12-07 19:11 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: Git Mailing List
In-Reply-To: <50C22F72.6010701@xiplink.com>

Marc Branchaud <marcnarc@xiplink.com> writes:

> On 12-12-07 12:54 PM, Junio C Hamano wrote:
>> Marc Branchaud <marcnarc@xiplink.com> writes:
>> 
>>> This is with git 1.8.0.1 on all the machines involved.
>>>
>>> One of our build machines is having trouble with "git submodule":
>>> ...
>>> Any ideas?
>> 
>> How and why is the IFS set differently only on one of your build
>> machines?
>
> It's not.  On all machines:
> 	$ set | grep IFS
> 	IFS=$' \t\n'
>
> As I said, if I isolate the module_list() function into another script it
> works fine, with the exact same environment that breaks in git-submodule.sh.
>
> Also, note that at the top of git-submodule there's
> 	. git-sh-setup
> which does
> 	unset IFS

Yeah, now it makes sense why you wrote "Weird" on the subject line.
What difference, if any, does the problematic box have compared to
your other healthy boxes?  It uses a different /bin/sh?

Just taking a shot in the dark...

^ permalink raw reply

* Re: Weird problem with git-submodule.sh
From: Marc Branchaud @ 2012-12-07 20:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vwqwtfzis.fsf@alter.siamese.dyndns.org>

On 12-12-07 02:11 PM, Junio C Hamano wrote:
> Marc Branchaud <marcnarc@xiplink.com> writes:
> 
>> On 12-12-07 12:54 PM, Junio C Hamano wrote:
>>> Marc Branchaud <marcnarc@xiplink.com> writes:
>>>
>>>> This is with git 1.8.0.1 on all the machines involved.
>>>>
>>>> One of our build machines is having trouble with "git submodule":
>>>> ...
>>>> Any ideas?
>>>
>>> How and why is the IFS set differently only on one of your build
>>> machines?
>>
>> It's not.  On all machines:
>> 	$ set | grep IFS
>> 	IFS=$' \t\n'
>>
>> As I said, if I isolate the module_list() function into another script it
>> works fine, with the exact same environment that breaks in git-submodule.sh.
>>
>> Also, note that at the top of git-submodule there's
>> 	. git-sh-setup
>> which does
>> 	unset IFS
> 
> Yeah, now it makes sense why you wrote "Weird" on the subject line.
> What difference, if any, does the problematic box have compared to
> your other healthy boxes?  It uses a different /bin/sh?
> 
> Just taking a shot in the dark...

Bisected this down to exactly that "unset IFS" line in git-sh-setup.sh, from
your commit 785063e02bb249 (whaddya trying to do to me Junio?? :) ):

Author: Junio C Hamano <gitster@pobox.com>
Date:   Wed Aug 8 12:08:17 2012 -0700

    sh-setup: protect from exported IFS

    Many scripted Porcelains rely on being able to split words at the
    default $IFS characters, i.e. SP, HT and LF.  If the user exports a
    non-default IFS to the environment, what they read from plumbing
    commands such as ls-files that use HT to delimit fields may not be
    split in the way we expect.

    Protect ourselves by resetting it, just like we do so against CDPATH
    exported to the environment.

    Noticed by Andrew Dranse <adranse@oanda.com>.

    Signed-off-by: Junio C Hamano <gitster@pobox.com>

Perhaps IFS should be set to " \t\n" (which I believe is sh's default)
instead of just unsetting it altogether?  (Note that in my testing I had to
set IFS to a literal <space><tab><newline> string.)

		M.

^ permalink raw reply

* Re: Weird problem with git-submodule.sh
From: Junio C Hamano @ 2012-12-07 20:23 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: Git Mailing List
In-Reply-To: <50C24ED7.90000@xiplink.com>

Marc Branchaud <marcnarc@xiplink.com> writes:

>     sh-setup: protect from exported IFS
>
>     Many scripted Porcelains rely on being able to split words at the
>     default $IFS characters, i.e. SP, HT and LF.  If the user exports a
>     non-default IFS to the environment, what they read from plumbing
>     commands such as ls-files that use HT to delimit fields may not be
>     split in the way we expect.
>
>     Protect ourselves by resetting it, just like we do so against CDPATH
>     exported to the environment.
>
>     Noticed by Andrew Dranse <adranse@oanda.com>.
>
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> Perhaps IFS should be set to " \t\n" (which I believe is sh's default)
> instead of just unsetting it altogether?

POSIX.1 says this:

    IFS - A string treated as a list of characters that is used for
    field splitting and to split lines into fields with the read
    command.  If IFS is not set, it shall behave as normal for an
    unset variable, except that field splitting by the shell and
    line splitting by the read command shall be performed as if the
    value of IFS is <space> <tab> <newline>; see Field Splitting.

    Implementations may ignore the value of IFS in the environment, or
    the absence of IFS from the environment, at the time the shell is
    invoked, in which case the shell shall set IFS to <space> <tab>
    <newline> when it is invoked.

So setting it to SP HT LF should not make a difference, or your
shell is buggy.

This is exactly why I asked you about the /bin/sh on your
problematic box.

^ permalink raw reply

* Re: [PATCH 0/6] Improve remote helper documentation
From: Sverre Rabbelier @ 2012-12-07 20:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Max Horn, git, Florian Achleitner, Felipe Contreras
In-Reply-To: <7v1uf1he6m.fsf@alter.siamese.dyndns.org>

On Fri, Dec 7, 2012 at 11:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
> A second ping to people who have touched transport-helper.c,
> remote-testsvn.c, git-remote-testgit, and contrib/remote-helpers/ in
> the past 18 months for comments.  I've re-read the documentation
> updates myself and didn't find anything majorly objectionable.

FWIW:

Acked-by: Sverre Rabbelier <srabbelier@gmail.com>

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: Weird problem with git-submodule.sh
From: Marc Branchaud @ 2012-12-07 20:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vsj7hfw6q.fsf@alter.siamese.dyndns.org>

On 12-12-07 03:23 PM, Junio C Hamano wrote:
> Marc Branchaud <marcnarc@xiplink.com> writes:
> 
>>     sh-setup: protect from exported IFS
>>
>>     Many scripted Porcelains rely on being able to split words at the
>>     default $IFS characters, i.e. SP, HT and LF.  If the user exports a
>>     non-default IFS to the environment, what they read from plumbing
>>     commands such as ls-files that use HT to delimit fields may not be
>>     split in the way we expect.
>>
>>     Protect ourselves by resetting it, just like we do so against CDPATH
>>     exported to the environment.
>>
>>     Noticed by Andrew Dranse <adranse@oanda.com>.
>>
>>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
>>
>> Perhaps IFS should be set to " \t\n" (which I believe is sh's default)
>> instead of just unsetting it altogether?
> 
> POSIX.1 says this:
> 
>     IFS - A string treated as a list of characters that is used for
>     field splitting and to split lines into fields with the read
>     command.  If IFS is not set, it shall behave as normal for an
>     unset variable, except that field splitting by the shell and
>     line splitting by the read command shall be performed as if the
>     value of IFS is <space> <tab> <newline>; see Field Splitting.
> 
>     Implementations may ignore the value of IFS in the environment, or
>     the absence of IFS from the environment, at the time the shell is
>     invoked, in which case the shell shall set IFS to <space> <tab>
>     <newline> when it is invoked.
> 
> So setting it to SP HT LF should not make a difference, or your
> shell is buggy.
> 
> This is exactly why I asked you about the /bin/sh on your
> problematic box.

Fair 'nuf.

It's FreeBSD 7.2, which I know is an obsolete version but I'm not able to
upgrade the machine.  I believe FreeBSD's sh is, or is derived from, dash.

Anyway, given that there is at least one buggy version of sh, wouldn't it be
better for git to explicitly set IFS?

		M.

^ permalink raw reply

* Re: Weird problem with git-submodule.sh
From: Marc Branchaud @ 2012-12-07 21:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vsj7hfw6q.fsf@alter.siamese.dyndns.org>

On 12-12-07 03:23 PM, Junio C Hamano wrote:
> Marc Branchaud <marcnarc@xiplink.com> writes:
> 
>>     sh-setup: protect from exported IFS
>>
>>     Many scripted Porcelains rely on being able to split words at the
>>     default $IFS characters, i.e. SP, HT and LF.  If the user exports a
>>     non-default IFS to the environment, what they read from plumbing
>>     commands such as ls-files that use HT to delimit fields may not be
>>     split in the way we expect.
>>
>>     Protect ourselves by resetting it, just like we do so against CDPATH
>>     exported to the environment.
>>
>>     Noticed by Andrew Dranse <adranse@oanda.com>.
>>
>>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
>>
>> Perhaps IFS should be set to " \t\n" (which I believe is sh's default)
>> instead of just unsetting it altogether?
> 
> POSIX.1 says this:
> 
>     IFS - A string treated as a list of characters that is used for
>     field splitting and to split lines into fields with the read
>     command.  If IFS is not set, it shall behave as normal for an
>     unset variable, except that field splitting by the shell and
>     line splitting by the read command shall be performed as if the
>     value of IFS is <space> <tab> <newline>; see Field Splitting.
> 
>     Implementations may ignore the value of IFS in the environment, or
>     the absence of IFS from the environment, at the time the shell is
>     invoked, in which case the shell shall set IFS to <space> <tab>
>     <newline> when it is invoked.

Not to defend anyone, but I can understand how an implementer might think
they're complying with the above while still deciding that an explicit "unset
IFS" means IFS=''.

		M.

^ permalink raw reply

* Re: Weird problem with git-submodule.sh
From: Junio C Hamano @ 2012-12-07 21:08 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: Git Mailing List
In-Reply-To: <50C25539.9010206@xiplink.com>

Marc Branchaud <marcnarc@xiplink.com> writes:

> It's FreeBSD 7.2, which I know is an obsolete version but I'm not able to
> upgrade the machine.  I believe FreeBSD's sh is, or is derived from, dash.

Finally.  Yes, as you suspected, I am perfectly fine to explicitly
set IFS to the default values.

I wanted to have specific names to write in the commit log message,
in-code comments and possibly release notes.  That way, people can
decide if the issue affects them and they should upgrade once the
fix is made.

^ permalink raw reply

* Re: [PATCH] git(1): remove a defunct link to "list of authors"
From: Shawn Pearce @ 2012-12-07 21:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk3sthhfy.fsf@alter.siamese.dyndns.org>

On Fri, Dec 7, 2012 at 9:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> The linked page has not been showing the promised "more complete
>> list" for more than 6 months by now, and nobody has resurrected
>> the list there nor elsewhere since then.
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>  * If somebody has a working replacement URL, we could use that
>>    instead, of course.  Takers?
>
> A possible alternative could be https://www.ohloh.net/p/git/contributors/summary

Eh, I think just removing the link is sufficient.

^ permalink raw reply

* Re: [PATCH 0/6] Improve remote helper documentation
From: Max Horn @ 2012-12-07 21:41 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Sverre Rabbelier, Florian Achleitner, Felipe Contreras
In-Reply-To: <7v1uf1he6m.fsf@alter.siamese.dyndns.org>


On 07.12.2012, at 20:09, Junio C Hamano wrote:

> Max Horn <max@quendi.de> writes:
> 
>> Various remote helper capabilities and commands were not
>> documented, in particular 'export', or documented in a misleading
>> way (e.g. 'for-push' was listed as a ref attribute understood by
>> git, which is not the case). This patch series changes that, and
>> also address some other things in the remote helper documentation
>> that I found jarring when reading through it.
>> 
>> Note that the description of export and (im|ex)port-marks probably can be
>> improved, and I hope that somebody who knows more about them
>> than me and/or is better at writing documentation will do just that.
>> But I felt it was better to provide something than to do nothing
>> and only complain, as I did previously on this subject ;-).
> 
> A second ping to people who have touched transport-helper.c,
> remote-testsvn.c, git-remote-testgit, and contrib/remote-helpers/ in
> the past 18 months for comments.  I've re-read the documentation
> updates myself and didn't find anything majorly objectionable.
> 
> Except for a minor nit in 6/6; I think "defined options" should be
> "defined attributes".
> 
>    --- a/Documentation/git-remote-helpers.txt
>    +++ b/Documentation/git-remote-helpers.txt
>    @@ -227,6 +227,8 @@ Support for this command is mandatory.
>            the name; unrecognized attributes are ignored. The list ends
>            with a blank line.
>     +
>    +See REF LIST ATTRIBUTES for a list of currently defined options.
>    ++

Indeed, my mistake. Dumb question since I am still not completely familiar with the procedures here: Would you just fix this yourself as you apply this, or is it better (or even required) that I reroll for this? (Of course if I ned to re-roll for other reasons, I'd fix this one as well).


Thanks,
Max

^ permalink raw reply

* Re: [PATCH 0/6] Improve remote helper documentation
From: Junio C Hamano @ 2012-12-07 21:52 UTC (permalink / raw)
  To: Max Horn; +Cc: git, Sverre Rabbelier, Florian Achleitner, Felipe Contreras
In-Reply-To: <A6FF9929-74F8-4F2F-BA0B-1B1086D4E7F5@quendi.de>

Max Horn <max@quendi.de> writes:

> On 07.12.2012, at 20:09, Junio C Hamano wrote:
>
>> Except for a minor nit in 6/6; I think "defined options" should be
>> "defined attributes".
>> 
>>    --- a/Documentation/git-remote-helpers.txt
>>    +++ b/Documentation/git-remote-helpers.txt
>>    @@ -227,6 +227,8 @@ Support for this command is mandatory.
>>            the name; unrecognized attributes are ignored. The list ends
>>            with a blank line.
>>     +
>>    +See REF LIST ATTRIBUTES for a list of currently defined options.
>>    ++
>
> Indeed, my mistake. Dumb question since I am still not completely
> familiar with the procedures here: Would you just fix this
> yourself as you apply this, or is it better (or even required)
> that I reroll for this? (Of course if I ned to re-roll for other
> reasons, I'd fix this one as well).

I didn't see any other issues myself, and I didn't see anybody
raising issues on the series, either, so I could fix it up by
amending what is already queued, as long as you are OK with it.

Thanks.

^ permalink raw reply

* [PATCH] sh-setup: Explicitly set IFS to its default, instead of unsetting it.
From: marcnarc @ 2012-12-07 22:15 UTC (permalink / raw)
  To: git; +Cc: Marc Branchaud
In-Reply-To: <50C24ED7.90000@xiplink.com>

From: Marc Branchaud <marcnarc@xiplink.com>

Some sh implementations interpret "unset IFS" to mean IFS=''.  This was
seen in FreeBSD 7.2's sh.

We need to make sure IFS has its default value: <space><tab><newline>.

Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
---
 git-sh-setup.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index ee0e0bc..56e6498 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -12,8 +12,11 @@
 # But we protect ourselves from such a user mistake nevertheless.
 unset CDPATH
 
-# Similarly for IFS
-unset IFS
+# Similarly for IFS, except that some sh implementations interpret "unset IFS"
+# as IFS='', so we need to set IFS explicitly to its POSIX default using
+# literal <space><tab><newline> characters.
+IFS=' 	
+'
 
 git_broken_path_fix () {
 	case ":$PATH:" in
-- 
1.8.0.1

^ permalink raw reply related

* [PATCH] sh-setup: work around "unset IFS" bug in some shells
From: Junio C Hamano @ 2012-12-07 22:34 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: Git Mailing List
In-Reply-To: <7vobi5fu3c.fsf@alter.siamese.dyndns.org>

With an unset IFS, field splitting is supposed to act as if IFS is
set to the usual SP HT LF, but Marc Branchaud reports that the shell
on FreeBSD 7.2 gets this wrong.

It is easy to set it to the default value manually, so let's do so.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-sh-setup.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index ee0e0bc..107c144 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -12,8 +12,11 @@
 # But we protect ourselves from such a user mistake nevertheless.
 unset CDPATH
 
-# Similarly for IFS
-unset IFS
+# Similarly for IFS, but some shells (e.g. FreeBSD 7.2) are buggy and
+# do not equate an unset IFS with IFS with the default, so here is
+# an explicit SP HT LF.
+IFS=' 	
+'
 
 git_broken_path_fix () {
 	case ":$PATH:" in
-- 
1.8.1.rc0.125.g5274144

^ permalink raw reply related

* Re: [PATCH] sh-setup: work around "unset IFS" bug in some shells
From: Marc Branchaud @ 2012-12-07 22:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7va9tpfq46.fsf_-_@alter.siamese.dyndns.org>

I like your patch's subject line better than mine.

		M.


On 12-12-07 05:34 PM, Junio C Hamano wrote:
> With an unset IFS, field splitting is supposed to act as if IFS is
> set to the usual SP HT LF, but Marc Branchaud reports that the shell
> on FreeBSD 7.2 gets this wrong.
> 
> It is easy to set it to the default value manually, so let's do so.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  git-sh-setup.sh | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index ee0e0bc..107c144 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -12,8 +12,11 @@
>  # But we protect ourselves from such a user mistake nevertheless.
>  unset CDPATH
>  
> -# Similarly for IFS
> -unset IFS
> +# Similarly for IFS, but some shells (e.g. FreeBSD 7.2) are buggy and
> +# do not equate an unset IFS with IFS with the default, so here is
> +# an explicit SP HT LF.
> +IFS=' 	
> +'
>  
>  git_broken_path_fix () {
>  	case ":$PATH:" in
> 

^ permalink raw reply

* Re: [PATCH 0/6] Improve remote helper documentation
From: Max Horn @ 2012-12-07 22:38 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Sverre Rabbelier, Florian Achleitner, Felipe Contreras
In-Reply-To: <7vehj1fs3c.fsf@alter.siamese.dyndns.org>


On 07.12.2012, at 22:52, Junio C Hamano wrote:

> Max Horn <max@quendi.de> writes:
> 
>> On 07.12.2012, at 20:09, Junio C Hamano wrote:
>> 
>>> Except for a minor nit in 6/6; I think "defined options" should be
>>> "defined attributes".
>>> 
>>>   --- a/Documentation/git-remote-helpers.txt
>>>   +++ b/Documentation/git-remote-helpers.txt
>>>   @@ -227,6 +227,8 @@ Support for this command is mandatory.
>>>           the name; unrecognized attributes are ignored. The list ends
>>>           with a blank line.
>>>    +
>>>   +See REF LIST ATTRIBUTES for a list of currently defined options.
>>>   ++
>> 
>> Indeed, my mistake. Dumb question since I am still not completely
>> familiar with the procedures here: Would you just fix this
>> yourself as you apply this, or is it better (or even required)
>> that I reroll for this? (Of course if I ned to re-roll for other
>> reasons, I'd fix this one as well).
> 
> I didn't see any other issues myself, and I didn't see anybody
> raising issues on the series, either, so I could fix it up by
> amending what is already queued, as long as you are OK with it.

Sure, I'd appreciate if you could make that fix.

Thanks,
Max

^ permalink raw reply

* Re: [PATCH] sh-setup: work around "unset IFS" bug in some shells
From: Andreas Schwab @ 2012-12-07 22:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Marc Branchaud, Git Mailing List
In-Reply-To: <7va9tpfq46.fsf_-_@alter.siamese.dyndns.org>

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

> +# Similarly for IFS, but some shells (e.g. FreeBSD 7.2) are buggy and
> +# do not equate an unset IFS with IFS with the default, so here is
> +# an explicit SP HT LF.
> +IFS=' 	
> +'

Trailing whitespace can easily get lost, so it's probably better to
stick a '' in here.  The sequence <space><tab> is also easily being
mangled to <tab>.

IFS=' ''	''
'

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH] sh-setup: work around "unset IFS" bug in some shells
From: Junio C Hamano @ 2012-12-07 22:58 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Marc Branchaud, Git Mailing List
In-Reply-To: <m2fw3h8oja.fsf@igel.home>

Andreas Schwab <schwab@linux-m68k.org> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> +# Similarly for IFS, but some shells (e.g. FreeBSD 7.2) are buggy and
>> +# do not equate an unset IFS with IFS with the default, so here is
>> +# an explicit SP HT LF.
>> +IFS=' 	
>> +'
>
> Trailing whitespace can easily get lost

The comment above the assingment spell the character names out for
that exact reason.  Honestly, I think

> IFS=' ''	''
> '

is a lot harder to read.  If we were writing in bash, we would
probably use $' \t\n' but because we aren't, the best you can do
with your approach is what you wrote, but it would make the reader
wonder what these empty '' are there for and how many spaces are in
between the ''<string here>'' pair.

In other words, I agree with your concerns, but I do not think your
rewrite helps very much.

^ permalink raw reply

* [PATCH] Documentation/diff-config: work around AsciiDoc misfortune
From: Junio C Hamano @ 2012-12-07 23:34 UTC (permalink / raw)
  To: git

The line that happens to begin with indent followed by "3. " was
interpreted as if it were an enumerated list; just wrap the lines
differently to work it around for now.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * The last section of Documentation/technical/api-command.txt has a
   related/similar issue in that AsciiDoc wants to renumber them
   starting from 1 not from 0, as all humans do, but the result is
   more desirable.  I'll leave a patch to renumber them in the
   source to others, as I am busy cutting the 1.8.1-rc1 today.

   But "3" in this one is not the beginning of an enumerated item;
   it is cardinal (i.e. counting), not ordinal, and needs to be
   corrected before the final release.

 Documentation/diff-config.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 89dd634..4314ad0 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -57,8 +57,8 @@ diff.statGraphWidth::
 	to all commands generating --stat output except format-patch.
 
 diff.context::
-	Generate diffs with <n> lines of context instead of the default of
-	3. This value is overridden by the -U option.
+	Generate diffs with <n> lines of context instead of the default
+	of 3. This value is overridden by the -U option.
 
 diff.external::
 	If this config variable is set, diff generation is not
-- 
1.8.1.rc1.123.gb624e49

^ permalink raw reply related

* [ANNOUNCE] Git v1.8.1-rc1
From: Junio C Hamano @ 2012-12-08  0:20 UTC (permalink / raw)
  To: git; +Cc: Linux Kernel

A release candidate Git v1.8.1-rc1 is now available for testing
at the usual places.

The release tarballs are found at:

    http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

4b451bb5b7125349c35cf15118e8f1893569e48f  git-1.8.1.rc1.tar.gz
7416b28a0917fef26ca06f22bc493ebea371267f  git-htmldocs-1.8.1.rc1.tar.gz
b5758adf5814d64ee8e0d26bdfb919be1c605071  git-manpages-1.8.1.rc1.tar.gz

Also the following public repositories all have a copy of the v1.8.1-rc1
tag and the master branch that the tag points at:

  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

Git v1.8.1 Release Notes (draft)
========================

Backward compatibility notes
----------------------------

In the next major release (not *this* one), we will change the
behavior of the "git push" command.

When "git push [$there]" does not say what to push, we have used the
traditional "matching" semantics so far (all your branches were sent
to the remote as long as there already are branches of the same name
over there).  We will use the "simple" semantics that pushes the
current branch to the branch with the same name, only when the current
branch is set to integrate with that remote branch.  There is a user
preference configuration variable "push.default" to change this, and
"git push" will warn about the upcoming change until you set this
variable in this release.

"git branch --set-upstream" is deprecated and may be removed in a
relatively distant future.  "git branch [-u|--set-upstream-to]" has
been introduced with a saner order of arguments to replace it.


Updates since v1.8.0
--------------------

UI, Workflows & Features

 * Command-line completion scripts for tcsh and zsh have been added.

 * A new remote-helper interface for Mercurial has been added to
   contrib/remote-helpers.

 * We used to have a workaround for a bug in ancient "less" that
   causes it to exit without any output when the terminal is resized.
   The bug has been fixed in "less" version 406 (June 2007), and the
   workaround has been removed in this release.

 * Some documentation pages that used to ship only in the plain text
   format are now formatted in HTML as well.

 * "git-prompt" scriptlet (in contrib/completion) can be told to paint
   pieces of the hints in the prompt string in colors.

 * A new configuration variable "diff.context" can be used to
   give the default number of context lines in the patch output, to
   override the hardcoded default of 3 lines.

 * When "git checkout" checks out a branch, it tells the user how far
   behind (or ahead) the new branch is relative to the remote tracking
   branch it builds upon.  The message now also advises how to sync
   them up by pushing or pulling.  This can be disabled with the
   advice.statusHints configuration variable.

 * "git config --get" used to diagnose presence of multiple
   definitions of the same variable in the same configuration file as
   an error, but it now applies the "last one wins" rule used by the
   internal configuration logic.  Strictly speaking, this may be an
   API regression but it is expected that nobody will notice it in
   practice.

 * "git log -p -S<string>" now looks for the <string> after applying
   the textconv filter (if defined); earlier it inspected the contents
   of the blobs without filtering.

 * "git format-patch" learned the "--notes=<ref>" option to give
   notes for the commit after the three-dash lines in its output.

 * "git log --grep=<pcre>" learned to honor the "grep.patterntype"
   configuration set to "perl".

 * "git replace -d <object>" now interprets <object> as an extended
   SHA-1 (e.g. HEAD~4 is allowed), instead of only accepting full hex
   object name.

 * "git rm $submodule" used to punt on removing a submodule working
   tree to avoid losing the repository embedded in it.  Because
   recent git uses a mechanism to separate the submodule repository
   from the submodule working tree, "git rm" learned to detect this
   case and removes the submodule working tree when it is safe to do so.

 * "git send-email" used to prompt for the sender address, even when
   the committer identity is well specified (e.g. via user.name and
   user.email configuration variables).  The command no longer gives
   this prompt when not necessary.

 * "git send-email" did not allow non-address garbage strings to
   appear after addresses on Cc: lines in the patch files (and when
   told to pick them up to find more recipients), e.g.

     Cc: Stable Kernel <stable@k.org> # for v3.2 and up

   The command now strips " # for v3.2 and up" part before adding the
   remainder of this line to the list of recipients.

 * "git submodule add" learned to add a new submodule at the same
   path as the path where an unrelated submodule was bound to in an
   existing revision via the "--name" option.

 * "git submodule sync" learned the "--recursive" option.

 * "diff.submodule" configuration variable can be used to give custom
   default value to the "git diff --submodule" option.

 * "git symbolic-ref" learned the "-d $symref" option to delete the
   named symbolic ref, which is more intuitive way to spell it than
   "update-ref -d --no-deref $symref".


Foreign Interface

 * "git cvsimport" can be told to record timezones (other than GMT)
   per-author via its author info file.

 * The remote helper interface to interact with subversion
   repositories (one of the GSoC 2012 projects) has been merged.


Performance, Internal Implementation, etc.

 * Compilation on Cygwin with newer header files are supported now.

 * The logic to generate the initial advertisement from "upload-pack"
   (i.e. what is invoked by "git fetch" on the other side of the
   connection) to list what refs are available in the repository has
   been optimized.

 * The logic to find set of attributes that match a given path has
   been optimized.

 * Use preloadindex in "git diff-index" and "git update-index", which
   has a nice speedup on systems with slow stat calls (and even on
   Linux).


Also contains minor documentation updates and code clean-ups.


Fixes since v1.8.0
------------------

Unless otherwise noted, all the fixes since v1.8.0 in the maintenance
track are contained in this release (see release notes to them for
details).

 * The configuration parser had an unnecessary hardcoded limit on
   variable names that was not checked consistently.

 * The "say" function in the test scaffolding incorrectly allowed
   "echo" to interpret "\a" as if it were a C-string asking for a
   BEL output.

 * "git mergetool" feeds /dev/null as a common ancestor when dealing
   with an add/add conflict, but p4merge backend cannot handle
   it. Work it around by passing a temporary empty file.

 * "git log -F -E --grep='<ere>'" failed to use the given <ere>
   pattern as extended regular expression, and instead looked for the
   string literally.

 * "git grep -e pattern <tree>" asked the attribute system to read
   "<tree>:.gitattributes" file in the working tree, which was
   nonsense.

 * A symbolic ref refs/heads/SYM was not correctly removed with "git
   branch -d SYM"; the command removed the ref pointed by SYM
   instead.

 * Update "remote tracking branch" in the documentation to
   "remote-tracking branch".

 * "git pull --rebase" run while the HEAD is detached tried to find
   the upstream branch of the detached HEAD (which by definition
   does not exist) and emitted unnecessary error messages.

 * The refs/replace hierarchy was not mentioned in the
   repository-layout docs.

 * Various rfc2047 quoting issues around a non-ASCII name on the
   From: line in the output from format-patch have been corrected.

 * Sometimes curl_multi_timeout() function suggested a wrong timeout
   value when there is no file descriptor to wait on and the http
   transport ended up sleeping for minutes in select(2) system call.
   A workaround has been added for this.

 * For a fetch refspec (or the result of applying wildcard on one),
   we always want the RHS to map to something inside "refs/"
   hierarchy, but the logic to check it was not exactly right.
   (merge 5c08c1f jc/maint-fetch-tighten-refname-check later to maint).

 * "git diff -G<pattern>" did not honor textconv filter when looking
   for changes.

 * Some HTTP servers ask for auth only during the actual packing phase
   (not in ls-remote phase); this is not really a recommended
   configuration, but the clients used to fail to authenticate with
   such servers.
   (merge 2e736fd jk/maint-http-half-auth-fetch later to maint).

 * "git p4" used to try expanding malformed "$keyword$" that spans
   across multiple lines.

 * Syntax highlighting in "gitweb" was not quite working.

 * RSS feed from "gitweb" had a xss hole in its title output.

 * "git config --path $key" segfaulted on "[section] key" (a boolean
   "true" spelled without "=", not "[section] key = true").

 * "git checkout -b foo" while on an unborn branch did not say
   "Switched to a new branch 'foo'" like other cases.

 * Various codepaths have workaround for a common misconfiguration to
   spell "UTF-8" as "utf8", but it was not used uniformly.  Most
   notably, mailinfo (which is used by "git am") lacked this support.

 * We failed to mention a file without any content change but whose
   permission bit was modified, or (worse yet) a new file without any
   content in the "git diff --stat" output.

 * When "--stat-count" hides a diffstat for binary contents, the total
   number of added and removed lines at the bottom was computed
   incorrectly.

 * When "--stat-count" hides a diffstat for unmerged paths, the total
   number of affected files at the bottom of the "diff --stat" output
   was computed incorrectly.

 * "diff --shortstat" miscounted the total number of affected files
   when there were unmerged paths.

 * "update-ref -d --deref SYM" to delete a ref through a symbolic ref
   that points to it did not remove it correctly.

----------------------------------------------------------------

Changes since v1.8.1-rc0 are as follows:

Jiang Xin (1):
      l10n: Update git.pot (5 new, 1 removed messages)

Junio C Hamano (3):
      Update draft release notes to 1.8.0.2
      Documentation/diff-config: work around AsciiDoc misfortune
      Git 1.8.1-rc1

Matthieu Moy (1):
      document that statusHints affects git checkout

Peter Krefting (1):
      l10n: Update Swedish translation (1979t0f0u)

Ralf Thielow (2):
      l10n: de.po: translate 825 new messages
      l10n: de.po: translate 22 new messages

Ramkumar Ramachandra (4):
      t4041 (diff-submodule-option): don't hardcode SHA-1 in expected outputs
      t4041 (diff-submodule-option): parse digests sensibly
      t4041 (diff-submodule-option): rewrite add_file() routine
      t4041 (diff-submodule-option): modernize style

Tran Ngoc Quan (1):
      l10n: vi.po: update to git-v1.8.0.1-347-gf94c3

^ permalink raw reply

* [PATCH] shortlog: add --format to the usage
From: yannicklm @ 2012-12-08  0:46 UTC (permalink / raw)
  To: git; +Cc: yannicklm

---
 builtin/shortlog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index b316cf3..cb85ede 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -10,7 +10,7 @@
 #include "parse-options.h"
 
 static char const * const shortlog_usage[] = {
-	N_("git shortlog [-n] [-s] [-e] [-w] [rev-opts] [--] [<commit-id>... ]"),
+	N_("git shortlog [-n] [-s] [-e] [-w] [--format=...] [rev-opts] [--] [<commit-id>... ]"),
 	"",
 	N_("[rev-opts] are documented in git-rev-list(1)"),
 	NULL
-- 
1.8.0.1

^ permalink raw reply related

* Feature Request - Hide ignored files before checkout
From: Matthew Ciancio @ 2012-12-08  0:50 UTC (permalink / raw)
  To: git

To whom it may concern,

I am not sure if this is the right place to send this, but I couldn't find
anything on the web that seemed official, so here goes.

Imagine this scenario:

1) You have a Git repo with two branches (branchA and branchB), which are
currently identical.
2) Checkout to branch.
3) Create file foo.txt, stage it and commit it.
4) Create file ignore.txt and add it to the ".gitignore" file of branchB so
that it is successfully ignored by Git.
5) Checkout to branchA.

Problem: ignore.txt does not "disappear" like foo.txt does and is now just
sitting in branchA (and now any other branch I checkout into).

When I first started using Git, I genuinely thought this was a bug, because
it seems so logical to me that ignore files should hide/reappear just like
tracked files do, when switching branches.
I have been told ways of circumventing this (using commits and un-commits OR
using stash), but my reason for avoiding commits is: say you have binary/OS
specific files which really do not belong in the commit logs (even locally)
and hence should be ignored. What if you want those files in only one branch
and not all?
Stashing doesn't seem appropriate either, because it would get messy.

Do you think this warrants a feature request?

If so, I was thinking that maybe the .gitignore file could have a flag after
each entry, to indicate whether the file(s)/folder(s) should have this new
feature or not (that way it would cater for everyone, but I can't see why
you wouldn't want this behaviour). 

If you like this idea and submit this as a feature request, please respond
with a link of the official request so that I can follow it, or provide me
with a link to submit it myself.

P.S. Here is a forum post I made on StackOverflow about the issue:
http://stackoverflow.com/questions/13761682/gitignore-hide-ignored-files-dur
ing-checkout

Kind regards,
Matt

^ permalink raw reply

* [PATCH v2] cache-tree: invalidate i-t-a paths after generating trees
From: Nguyễn Thái Ngọc Duy @ 2012-12-08  4:10 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Junio C Hamano, Jonathon Mah,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1352459040-14452-1-git-send-email-pclouds@gmail.com>

Intent-to-add entries used to forbid writing trees so it was not a
problem. After commit 3f6d56d (commit: ignore intent-to-add entries
instead of refusing - 2012-02-07), we can generate trees from an index
with i-t-a entries.

However, the commit forgets to invalidate all paths leading to i-t-a
entries. With fully valid cache-tree (e.g. after commit or
write-tree), diff operations may prefer cache-tree to index and not
see i-t-a entries in the index, because cache-tree does not have them.

Reported-by: Jonathon Mah <me@JonathonMah.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Sorry for the late update, I have little time for git these days.
 It turns out that we don't need phony bit and another round for
 invalidation. We can do it in update_one.

 cache-tree.c          | 30 ++++++++++++++++++++++++++----
 t/t2203-add-intent.sh | 20 ++++++++++++++++++++
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 28ed657..989a7ff 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -248,6 +248,7 @@ static int update_one(struct cache_tree *it,
 	int missing_ok = flags & WRITE_TREE_MISSING_OK;
 	int dryrun = flags & WRITE_TREE_DRY_RUN;
 	int i;
+	int to_invalidate = 0;
 
 	if (0 <= it->entry_count && has_sha1_file(it->sha1))
 		return it->entry_count;
@@ -324,7 +325,13 @@ static int update_one(struct cache_tree *it,
 			if (!sub)
 				die("cache-tree.c: '%.*s' in '%s' not found",
 				    entlen, path + baselen, path);
-			i += sub->cache_tree->entry_count - 1;
+			i--; /* this entry is already counted in "sub" */
+			if (sub->cache_tree->entry_count < 0) {
+				i -= sub->cache_tree->entry_count;
+				to_invalidate = 1;
+			}
+			else
+				i += sub->cache_tree->entry_count;
 			sha1 = sub->cache_tree->sha1;
 			mode = S_IFDIR;
 		}
@@ -339,8 +346,23 @@ static int update_one(struct cache_tree *it,
 				mode, sha1_to_hex(sha1), entlen+baselen, path);
 		}
 
-		if (ce->ce_flags & (CE_REMOVE | CE_INTENT_TO_ADD))
-			continue; /* entry being removed or placeholder */
+		/*
+		 * CE_REMOVE entries are removed before the index is
+		 * written to disk. Skip them to remain consistent
+		 * with the future on-disk index.
+		 */
+		if (ce->ce_flags & CE_REMOVE)
+			continue;
+
+		/*
+		 * CE_INTENT_TO_ADD entries exist on on-disk index but
+		 * they are not part of generated trees. Invalidate up
+		 * to root to force cache-tree users to read elsewhere.
+		 */
+		if (ce->ce_flags & CE_INTENT_TO_ADD) {
+			to_invalidate = 1;
+			continue;
+		}
 
 		strbuf_grow(&buffer, entlen + 100);
 		strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
@@ -360,7 +382,7 @@ static int update_one(struct cache_tree *it,
 	}
 
 	strbuf_release(&buffer);
-	it->entry_count = i;
+	it->entry_count = to_invalidate ? -i : i;
 #if DEBUG
 	fprintf(stderr, "cache-tree update-one (%d ent, %d subtree) %s\n",
 		it->entry_count, it->subtree_nr,
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index ec35409..2a4a749 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -62,5 +62,25 @@ test_expect_success 'can "commit -a" with an i-t-a entry' '
 	git commit -a -m all
 '
 
+test_expect_success 'cache-tree invalidates i-t-a paths' '
+	git reset --hard &&
+	mkdir dir &&
+	: >dir/foo &&
+	git add dir/foo &&
+	git commit -m foo &&
+
+	: >dir/bar &&
+	git add -N dir/bar &&
+	git diff --cached --name-only >actual &&
+	echo dir/bar >expect &&
+	test_cmp expect actual &&
+
+	git write-tree >/dev/null &&
+
+	git diff --cached --name-only >actual &&
+	echo dir/bar >expect &&
+	test_cmp expect actual
+'
+
 test_done
 
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* Re: [PATCH] sh-setup: work around "unset IFS" bug in some shells
From: Andreas Schwab @ 2012-12-08  9:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Marc Branchaud, Git Mailing List
In-Reply-To: <7v624dfp1e.fsf@alter.siamese.dyndns.org>

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

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> +# Similarly for IFS, but some shells (e.g. FreeBSD 7.2) are buggy and
>>> +# do not equate an unset IFS with IFS with the default, so here is
>>> +# an explicit SP HT LF.
>>> +IFS=' 	
>>> +'
>>
>> Trailing whitespace can easily get lost
>
> The comment above the assingment spell the character names out for
> that exact reason.  Honestly, I think

That won't help if you don't look for it.  The loss can easily happen
while you are in a different part of the file.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply


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