[PATCH] de.po: Fixup one translation

[Stefan Beller]

English grammar with German words doesn't make it a German translation. ;)
We also need to fix the

Signed-off-by: Stefan Beller 
---
 po/de.po | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/po/de.po b/po/de.po
index e5d2b25..bae3723 100644
--- a/po/de.po
+++ b/po/de.po
@@ -3463,13 +3463,13 @@ msgid_plural ""
 "\n"
 "%s\n"
 msgstr[0] ""
-"Warnung: Sie sind um %d Commit hinterher, nicht verbunden zu\n"
-"einem Ihrer Branches:\n"
+"Warnung: Sie sind um %d Commit hinterher, folgende Commits sind in\n"
+"keinem Ihrer Branches enthalten: \n"
 "\n"
 "%s\n"
 msgstr[1] ""
-"Warnung: Sie sind um %d Commits hinterher, nicht verbunden zu\n"
-"einem Ihrer Branches:\n"
+"Warnung: Sie sind um %d Commits hinterher, folgende Commits sind in\n"
+"keinem Ihrer Branches enthalten: \n"
 "\n"
 "%s\n"
 
-- 
2.1.0.238.gce1d3a9

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] git-am: support any number of signatures

[Christian Couder]

Hi Michael,

On Mon, Sep 22, 2014 at 4:01 PM, Michael S. Tsirkin  wrote:
>
> Hi Junio, Christian,
> it's been a while.
> I see that the work on trailers is going on.
> I tried going over the documentation but I could not figure
> out how would one implement multiple signatures using the
> trailers mechanism.
>
> As a reminder, this old patchset (that I replied to) enhanced git am -s
> with an option to add different signatures depending on
> the option passed to the -s flag.
> E.g. I have
> [am "a"]
> signoff = "Acked-by: Michael S. Tsirkin "
>
> [am "r"]
> signoff = "Reviewed-by: Michael S. Tsirkin "
>
> [am "t"]
> signoff = "Tested-by: Michael S. Tsirkin "
>
> and now:
> git am -s art
> adds all 3 signatures when applying the patch.

This is probably not as simple as you would like but it works with
something like:

$ git interpret-trailers --trailer "Acked-by: Michael S. Tsirkin
" --trailer "Reviewed-by: Michael S. Tsirkin
"  --trailer "Tested-by: Michael S. Tsirkin
" 0001-foo.patch >to_apply/0001-foo.patch

and then:

$ git am to_apply/*.patch

Also by using something like:

$ git config trailer.a.key Acked-by
$ git config trailer.r.key Reviewed-by
$ git config trailer.t.key Tested-by

the first command could be simplified to:

$ git interpret-trailers --trailer "a: Michael S. Tsirkin
" --trailer "r: Michael S. Tsirkin "
--trailer "t: Michael S. Tsirkin " 0001-foo.patch
>to_apply/0001-foo.patch

And if you use an env variable:

$ ME="Michael S. Tsirkin "
$ git interpret-trailers --trailer "a: $ME" --trailer "r: $ME"
--trailer "t: $ME" 0001-foo.patch >to_apply/0001-foo.patch

Maybe later we will integrate git interpret-trailers with git commit,
git am and other commands, so that you can do directly:

git am --trailer "a: $ME" --trailer "r: $ME"  --trailer "t: $ME" 0001-foo.patch

Maybe we wil also assign a one letter shortcut to --trailer, for
example "z", so that could be:

git am -z "a: $ME" -z "r: $ME"  -z "t: $ME" 0001-foo.patch

We could also allow many separators in the same -z argument as long as
they are separated by say "~", so you could have:

git am -z "a: $ME~r: $ME~t: $ME" 0001-foo.patch

And then we could also allow people to define default values for
trailers with something like:

$ git config trailer.a.defaultvalue "Michael S. Tsirkin "
$ git config trailer.r.defaultvalue "Michael S. Tsirkin "
$ git config trailer.t.defaultvalue "Michael S. Tsirkin "

So that in the end you could have:

git am -z a~r~t 0001-foo.patch

which is very close to "git am -s art".

Best,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Bug in git 2.1.0 when cloning to directory with same name as repository

[Chris Salzberg]

I've found what looks like a bug wherein if you are using an ssh alias
for a git remote, and that remote has a dash in its name, and you
specify the target path as the name of the url itself, git complains
about refs not being valid packed references.

To reproduce, in git 2.1.0 and with a repository using ssh config and
which has a dash in the name, e.g.:

> git clone github:nixme/pry-nav "github:nixme/pry-nav"
Cloning into 'github:nixme/pry-nav'...
done.
  ror: internal error: refs/remotes/origin/master is not a valid
packed reference!
error: internal error: refs/tags/v0.0.1 is not a valid packed reference!
error: internal error: refs/tags/v0.0.2 is not a valid packed reference!
error: internal error: refs/tags/v0.0.3 is not a valid packed reference!
error: internal error: refs/tags/v0.0.4 is not a valid packed reference!
error: internal error: refs/tags/v0.1.0 is not a valid packed reference!
error: internal error: refs/tags/v0.2.0 is not a valid packed reference!
error: internal error: refs/tags/v0.2.1 is not a valid packed reference!
error: internal error: refs/tags/v0.2.2 is not a valid packed reference!
error: internal error: refs/tags/v0.2.3 is not a valid packed reference!
error: internal error: refs/tags/v0.2.4 is not a valid packed reference!
error: Trying to write ref refs/heads/master with nonexistent object
f0e17451f0bd508f408d4fdda97e3a131d11f696
fatal: Cannot update the ref 'HEAD'

The ssh config for github (not that it matters, but for completeness) is:

Host github
user git
hostname github.com

I have confirmed that if the repository url does not have a dash, this
works as expected. I have also downgraded to 2.0.4 and found that
again, this works as expected. Beyond this I have not narrowed the
scope.

For now, I've downgraded my version of git and so have not tested beyond this.

Chris Salzberg
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mailinfo: resolve -Wstring-plus-int warning

[Jeff King]

On Mon, Sep 22, 2014 at 11:26:14PM -0700, Junio C Hamano wrote:

> On Mon, Sep 22, 2014 at 11:04 PM, Jeff King  wrote:
> >
> > I don't mind silencing this one warning (even though I find it a little
> > ridiculous). I'm slightly concerned that more brain-damage may be coming
> > our way, but we can deal with that if it ever does.
> >
> > Like Junio, I prefer keeping strlen() rather than switching to sizeof,
> > as it is less error-prone (no need for extra "-1" dance, and it won't
> > silently do the wrong thing if the array is ever converted to a
> > pointer).
> 
> I actually do not mind losing the sample[] array too much.
> 
> The early 45 bytes or so of that array (or a string constant) is not used
> by the code at all; I didn't want to count "From " (that's 5), 40-hex and
> then a SP -- ah, see, it is 46 bytes and I didn't want such miscounting.
> The only real contents that matter in that sample[] array is the tail part
> that is meant as the magic(5) cue. I'd be OK if the code checked the
> length of the line against a hardcoded constant and then did strcmp()
> starting from a hardcoded offset of the string and the magic cue string,
> and that would also avoid the warning from Eric's compiler.
> 
> But personally, I think the way it is coded is much easier to read,
> and is much harder to get it wrong while maintaining it.  So...

I agree. I was going to suggest switching to a static const array
instead of a string literal, but retaining strlen()...but I see you
already queued that in pu. So if what is there works for Eric (I do not
have the compiler in question to test with), that seems reasonable.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mailinfo: resolve -Wstring-plus-int warning

[Eric Sunshine]

On Tue, Sep 23, 2014 at 2:04 AM, Jeff King  wrote:
> On Mon, Sep 22, 2014 at 05:10:08PM -0400, Eric Sunshine wrote:
>
>> On Mon, Sep 22, 2014 at 1:41 PM, Junio C Hamano  wrote:
>> > Eric Sunshine  writes:
>> >
>> >> The just-released Apple Xcode 6.0.1 has -Wstring-plus-int enabled by
>> >> default which complains about pointer arithmetic applied to a string
>> >> literal:
>> >>
>> >> builtin/mailinfo.c:303:24: warning:
>> >> adding 'long' to a string does not append to the string
>> >> return !memcmp(SAMPLE + (cp - line), cp, strlen(SAMPLE) ...
>> >>~~~^
>> >
>> > And why is that a warning-worthy violation?
>>
>> Not being privy to Apple's decision making process, I can only guess
>> that it is in response to their new Swift programming language which
>> they are pushing heavily on iOS (and soon Mac OS X), in which '+' is
>> the string concatenation operator. For projects written in Swift and
>> incorporating legacy or portable components in C, C++, or Objective-C,
>> the warning may help programmer's avoid the pitfall of thinking that
>> '+' is also concatenation in the C-based languages.
>
> That is my reading from the warning text, too, but I have to wonder:
> wouldn't that mean they should be warning about pointer + pointer, not
> pointer + int?

'pointer + pointer' is not legal C, is it? What would the result
represent? The compiler correctly diagnoses that case as an error
(without special command-line switches):

error: invalid operands to binary expression
  ('char *' and 'char *')
  return "a" + "b";
 ~~~ ^ ~~~

> Also, wouldn't the same advice apply to adding to _any_ char pointer,
> not just a string literal?

Not really. Indexing into a char array via pointer arithmetic is a
perfectly reasonable and common idiom in C (indeed, git is peppered
with it), so such a warning would be pure noise. The more restricted
case of 'stringliteral + something' is probably sufficiently rare that
diagnosing it may catch genuine errors by programmers coming from
languages in which '+' does string concatenation (which may be the
original motivation for the warning; see below).

> I know you don't have answers to those questions, but the whole thing
> seems rather silly to me.

I did some more research and discovered that -Wstring-plus-int was
added in response to 'string + int' cases being found in Chromium's C
code (perhaps by programmers living in both JavaScript and C worlds).
The proposed option was discussed [1] and committed [2]. Later a
-Wstring-plus-char option was also discussed [3] and committed [4].

Regarding Apple: The Swift language supports 'string + string' and
'string + char', but not 'string + int'. However, in C, the char would
be promoted to int, so there is some sense in warning about 'string +
int' if they want to help safeguard programmers coming to C from Swift
(but, again, I'm just speculating).

[1]: http://thread.gmane.org/gmane.comp.compilers.clang.scm/47203/
[2]: 
http://llvm.org/klaus/clang/commit/1cb2d742eb6635aeab6132ee5f0b5781d39487d7/
[3]: http://thread.gmane.org/gmane.comp.compilers.clang.scm/82462/
[4]: 
http://llvm.org/klaus/clang/commit/02debf605cd904edac8dceb196e5f142ce3d14eb/

>> > Can we have them fix their compiler instead?
>>
>> If the above supposition is correct, then it's likely that Apple
>> considers this a feature, not a bug which needs to be fixed.
>
> Like Junio, I prefer keeping strlen() rather than switching to sizeof,
> as it is less error-prone (no need for extra "-1" dance, and it won't
> silently do the wrong thing if the array is ever converted to a
> pointer).

Sounds reasonable.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mailinfo: resolve -Wstring-plus-int warning

[Eric Sunshine]

On Tue, Sep 23, 2014 at 2:26 AM, Junio C Hamano  wrote:
> On Mon, Sep 22, 2014 at 11:04 PM, Jeff King  wrote:
>>
>> I don't mind silencing this one warning (even though I find it a little
>> ridiculous). I'm slightly concerned that more brain-damage may be coming
>> our way, but we can deal with that if it ever does.
>>
>> Like Junio, I prefer keeping strlen() rather than switching to sizeof,
>> as it is less error-prone (no need for extra "-1" dance, and it won't
>> silently do the wrong thing if the array is ever converted to a
>> pointer).
>
> I actually do not mind losing the sample[] array too much.
>
> The early 45 bytes or so of that array (or a string constant) is not used
> by the code at all; I didn't want to count "From " (that's 5), 40-hex and
> then a SP -- ah, see, it is 46 bytes and I didn't want such miscounting.
> The only real contents that matter in that sample[] array is the tail part
> that is meant as the magic(5) cue. I'd be OK if the code checked the
> length of the line against a hardcoded constant and then did strcmp()
> starting from a hardcoded offset of the string and the magic cue string,
> and that would also avoid the warning from Eric's compiler.

The -Wstring-plus-int option is "smart" enough to suppress the warning
if the hardcoded offset falls within the bounds of the string literal,
so this could work (but it feels a bit fragile compared to the current
code).

> But personally, I think the way it is coded is much easier to read,
> and is much harder to get it wrong while maintaining it.  So...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] git-am: support any number of signatures

[Michael S. Tsirkin]

On Tue, Sep 23, 2014 at 09:45:50AM +0200, Christian Couder wrote:
> Hi Michael,
> 
> On Mon, Sep 22, 2014 at 4:01 PM, Michael S. Tsirkin  wrote:
> >
> > Hi Junio, Christian,
> > it's been a while.
> > I see that the work on trailers is going on.
> > I tried going over the documentation but I could not figure
> > out how would one implement multiple signatures using the
> > trailers mechanism.
> >
> > As a reminder, this old patchset (that I replied to) enhanced git am -s
> > with an option to add different signatures depending on
> > the option passed to the -s flag.
> > E.g. I have
> > [am "a"]
> > signoff = "Acked-by: Michael S. Tsirkin "
> >
> > [am "r"]
> > signoff = "Reviewed-by: Michael S. Tsirkin "
> >
> > [am "t"]
> > signoff = "Tested-by: Michael S. Tsirkin "
> >
> > and now:
> > git am -s art
> > adds all 3 signatures when applying the patch.
> 
> This is probably not as simple as you would like but it works with
> something like:
> 
> $ git interpret-trailers --trailer "Acked-by: Michael S. Tsirkin
> " --trailer "Reviewed-by: Michael S. Tsirkin
> "  --trailer "Tested-by: Michael S. Tsirkin
> " 0001-foo.patch >to_apply/0001-foo.patch
> 
> and then:
> 
> $ git am to_apply/*.patch
> 
> Also by using something like:
> 
> $ git config trailer.a.key Acked-by
> $ git config trailer.r.key Reviewed-by
> $ git config trailer.t.key Tested-by

I would like multiple keys to match a specific
letter, e.g. as a maintainer I need
both reviewed by and signed off by when I
apply a patch, I like applying them with
a single "-s m".

> the first command could be simplified to:
> 
> $ git interpret-trailers --trailer "a: Michael S. Tsirkin
> " --trailer "r: Michael S. Tsirkin "
> --trailer "t: Michael S. Tsirkin " 0001-foo.patch
> >to_apply/0001-foo.patch
> 
> And if you use an env variable:
> 
> $ ME="Michael S. Tsirkin "
> $ git interpret-trailers --trailer "a: $ME" --trailer "r: $ME"
> --trailer "t: $ME" 0001-foo.patch >to_apply/0001-foo.patch
> 
> Maybe later we will integrate git interpret-trailers with git commit,
> git am and other commands, so that you can do directly:
> 
> git am --trailer "a: $ME" --trailer "r: $ME"  --trailer "t: $ME" 
> 0001-foo.patch
> 
> Maybe we wil also assign a one letter shortcut to --trailer, for
> example "z", so that could be:
> 
> git am -z "a: $ME" -z "r: $ME"  -z "t: $ME" 0001-foo.patch

-s could apply here, right?
It doesn't have a parameter at the moment.

> We could also allow many separators in the same -z argument as long as
> they are separated by say "~",

I think -z a -z r -z t is enough.

> so you could have:
> 
> git am -z "a: $ME~r: $ME~t: $ME" 0001-foo.patch
> 
> And then we could also allow people to define default values for
> trailers with something like:
> 
> $ git config trailer.a.defaultvalue "Michael S. Tsirkin "
> $ git config trailer.r.defaultvalue "Michael S. Tsirkin "
> $ git config trailer.t.defaultvalue "Michael S. Tsirkin "

I'm kind of confused by the key/value concept.
Can't I define the whole 'Acked-by: Michael S. Tsirkin '
string as the key?


> So that in the end you could have:
> 
> git am -z a~r~t 0001-foo.patch
> 
> which is very close to "git am -s art".
> 
> Best,
> Christian.

If I figure out the defaultvalue thing, I might
find the time to work on git am integration.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mailinfo: resolve -Wstring-plus-int warning

[Eric Sunshine]

On Tue, Sep 23, 2014 at 3:51 AM, Jeff King  wrote:
> On Mon, Sep 22, 2014 at 11:26:14PM -0700, Junio C Hamano wrote:
>
>> On Mon, Sep 22, 2014 at 11:04 PM, Jeff King  wrote:
>> >
>> > I don't mind silencing this one warning (even though I find it a little
>> > ridiculous). I'm slightly concerned that more brain-damage may be coming
>> > our way, but we can deal with that if it ever does.
>> >
>> > Like Junio, I prefer keeping strlen() rather than switching to sizeof,
>> > as it is less error-prone (no need for extra "-1" dance, and it won't
>> > silently do the wrong thing if the array is ever converted to a
>> > pointer).
>>
>> I actually do not mind losing the sample[] array too much.
>>
>> But personally, I think the way it is coded is much easier to read,
>> and is much harder to get it wrong while maintaining it.  So...
>
> I agree. I was going to suggest switching to a static const array
> instead of a string literal, but retaining strlen()...but I see you
> already queued that in pu. So if what is there works for Eric (I do not
> have the compiler in question to test with), that seems reasonable.

What is queued in 'pu' is the same as my patch [1] minus the
superfluous strlen() => sizeof() change, so it works fine for me.
Thanks.

[1]: http://article.gmane.org/gmane.comp.version-control.git/257345
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] de.po: Fixup one translation

[Eric Sunshine]

On Tue, Sep 23, 2014 at 3:37 AM, Stefan Beller  wrote:
> English grammar with German words doesn't make it a German translation. ;)
> We also need to fix the

Sentence fragment.

> Signed-off-by: Stefan Beller 
> ---
>  po/de.po | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/po/de.po b/po/de.po
> index e5d2b25..bae3723 100644
> --- a/po/de.po
> +++ b/po/de.po
> @@ -3463,13 +3463,13 @@ msgid_plural ""
>  "\n"
>  "%s\n"
>  msgstr[0] ""
> -"Warnung: Sie sind um %d Commit hinterher, nicht verbunden zu\n"
> -"einem Ihrer Branches:\n"
> +"Warnung: Sie sind um %d Commit hinterher, folgende Commits sind in\n"
> +"keinem Ihrer Branches enthalten: \n"
>  "\n"
>  "%s\n"
>  msgstr[1] ""
> -"Warnung: Sie sind um %d Commits hinterher, nicht verbunden zu\n"
> -"einem Ihrer Branches:\n"
> +"Warnung: Sie sind um %d Commits hinterher, folgende Commits sind in\n"
> +"keinem Ihrer Branches enthalten: \n"
>  "\n"
>  "%s\n"
>
> --
> 2.1.0.238.gce1d3a9
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mailinfo: resolve -Wstring-plus-int warning

[Jeff King]

On Tue, Sep 23, 2014 at 03:52:21AM -0400, Eric Sunshine wrote:

> > That is my reading from the warning text, too, but I have to wonder:
> > wouldn't that mean they should be warning about pointer + pointer, not
> > pointer + int?
> 
> 'pointer + pointer' is not legal C, is it? What would the result
> represent? The compiler correctly diagnoses that case as an error
> (without special command-line switches):
> 
> error: invalid operands to binary expression
>   ('char *' and 'char *')
>   return "a" + "b";
>  ~~~ ^ ~~~

You're correct that it's not legal. My point was more that "pointer +
int" is already clearly not string concatenation, because the operands
are not two strings.

I think the answer here (from the threads you linked below) is that
people expect it to not just concatenate, but also auto-convert integers
to strings. Yeesh.

> > Also, wouldn't the same advice apply to adding to _any_ char pointer,
> > not just a string literal?
> 
> Not really. Indexing into a char array via pointer arithmetic is a
> perfectly reasonable and common idiom in C (indeed, git is peppered
> with it), so such a warning would be pure noise.

That is a good reason not to do the warning in these cases (and why I
hope that we will not have to deal with this further). But IMHO it is
good evidence that the warning is not well thought-out. It seems silly
that:

  x = "foo" + 1;

is bad, but:

  y = "foo";
  x = y + 1;

is not[1]. Saying the first one is rare does not seem like a good
excuse; rather the existence of the second should tip you off that the
idiom is valid and not to be complained about.

Anyway, there is not much point in me complaining further about it here.
You are not the one who introduced it. :)

Thanks for digging in the history. It was interesting at least.

-Peff

[1] Actually, from reading the patch thread, I think the "1" needs to be
a non-constant integer here. But that just furthers the point: if
you have to neuter the warning to prevent a ton of false positives,
that is a good indication that you should not be warning. :)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Bug in git 2.1.0 when cloning to directory with same name as repository

[Jeff King]

On Tue, Sep 23, 2014 at 04:49:55PM +0900, Chris Salzberg wrote:

> I've found what looks like a bug wherein if you are using an ssh alias
> for a git remote, and that remote has a dash in its name, and you
> specify the target path as the name of the url itself, git complains
> about refs not being valid packed references.
> 
> To reproduce, in git 2.1.0 and with a repository using ssh config and
> which has a dash in the name, e.g.:

Thanks for a reproduction recipe. I think we can make it simpler,
though. The problem seems to come when your destination directory is
identical to the remote URL. I saw similar failures with:

  git clone g...@github.com:git/git g...@github.com:git/git

and even:

  git clone https://github.com/git/git https://github.com/git/git

It bisects to f38aa83 (use local cloning if insteadOf makes a local URL,
2014-07-17) by Michael Barabanov (cc'd). That commit moved a call to
get_repo_path() much further down cmd_clone, after we have already
created the repo directory. As a result, we try to fetch objects from
the newly created directory, which of course has none (and because it's
local, we try to use "cp" instead of the git protocol. We don't notice
the error at transfer time, but rather later when trying to write out
the references).

I'm not sure of the simplest solution. I guess along the lines of one
of:

  1. Move the code back before the directory is created. Rather than
 using `remote_get` to apply insteadOf substitution as a side
 effect, make it possible to call into the insteadOf code directly.

  2. It might work to chdir() into the repo after creating it, though
 that probably creates its own problems with other relative paths
 (and I am not sure it would save us if you had a remote URL that
 looked like an absolute path).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] de.po: Fixup one translation

[Stefan Beller]

On 23.09.2014 10:06, Eric Sunshine wrote:
> On Tue, Sep 23, 2014 at 3:37 AM, Stefan Beller  wrote:
>> English grammar with German words doesn't make it a German translation. ;)
>> We also need to fix the
> 
> Sentence fragment.

Yeah, originally I intended to just fix the singular form and let the
plural form slip. But then I did both singular and plural form and
forgot about the already started commit message.

Anyways I'd assume Ralf will incorporate this patch in a larger patch
for the German translation, so the commit message for this particular
patch is of minor relevance.

Thanks,
Stefan

> 
>> Signed-off-by: Stefan Beller 
>> ---
>>  po/de.po | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/po/de.po b/po/de.po
>> index e5d2b25..bae3723 100644
>> --- a/po/de.po
>> +++ b/po/de.po
>> @@ -3463,13 +3463,13 @@ msgid_plural ""
>>  "\n"
>>  "%s\n"
>>  msgstr[0] ""
>> -"Warnung: Sie sind um %d Commit hinterher, nicht verbunden zu\n"
>> -"einem Ihrer Branches:\n"
>> +"Warnung: Sie sind um %d Commit hinterher, folgende Commits sind in\n"
>> +"keinem Ihrer Branches enthalten: \n"
>>  "\n"
>>  "%s\n"
>>  msgstr[1] ""
>> -"Warnung: Sie sind um %d Commits hinterher, nicht verbunden zu\n"
>> -"einem Ihrer Branches:\n"
>> +"Warnung: Sie sind um %d Commits hinterher, folgende Commits sind in\n"
>> +"keinem Ihrer Branches enthalten: \n"
>>  "\n"
>>  "%s\n"
>>
>> --
>> 2.1.0.238.gce1d3a9

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation/git-rebase.txt: discuss --fork-point assumption of vanilla "git rebase" in DESCRIPTION.

[Sergey Organov]

Junio C Hamano  writes:

> Sergey Organov  writes:
>
>> Vanilla "git rebase" defaults to --fork-point that in some cases
>> makes behavior very different from "git rebase ",
>> where --no-fork-point is assumed. This fact was not mentioned in
>> the DESCRIPTION section of the manual page, even though the case of
>> omitted  was otherwise discussed. That in turn made actual
>> behavior of vanilla "git rebase" hardly discoverable.
>>
>> While we are at it, clarify the --fork-point description itself as well.
>>
>> Signed-off-by: Sergey Organov 
>> ---
>>  Documentation/git-rebase.txt | 18 +-
>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
>> index 4138554..73e1e1c 100644
>> --- a/Documentation/git-rebase.txt
>> +++ b/Documentation/git-rebase.txt
>> @@ -21,15 +21,16 @@ If  is specified, 'git rebase' will perform an 
>> automatic
>>  it remains on the current branch.
>>  
>>  If  is not specified, the upstream configured in
>> -branch..remote and branch..merge options will be used; see
>> -linkgit:git-config[1] for details.  If you are currently not on any
>> -branch or if the current branch does not have a configured upstream,
>> -the rebase will abort.
>> +branch..remote and branch..merge options will be used (see
>> +linkgit:git-config[1] for details) and the `--fork-point` option is
>> +assumed.  If you are currently not on any branch or if the current
>> +branch does not have a configured upstream, the rebase will abort.
>
> OK.  When you do not tell rebase with respect to what exact _commit_
> the operation is to be done, then we will enable --fork-point, which
> makes perfect sense because it is clear that the user is rebasing
> with respect to a _branch_, for which we may find a place better
> than its current tip to rebase onto if we look at its reflog.

I think you meant to say that we may find a better source to calculate
the exact set of commits to rebase, as we still rebase onto the current
tip. I.e., with this we select what to rebase, not where (the latter
being handled by the --onto switch.)

> It is debatable if we should do the same when the user tells us to
> rebase with respect to a specific _branch_ by giving the 'upstream'
> argument, but that is an entirely separate issue.  We might want to
> do a similar command line heuristics to tell between the branch
> switching "git checkout master" (which is an operation about a
> branch) and head detaching "git checkout refs/heads/master^0" (which
> is an operation about a commit) if we want to help the users by
> auto-enabling fork-point mode.

Well, IMHO "git rebase" and "git rebase @{u}" must do exactly the same
thing. In its current state, when they have different default for
fork-point, it's too surprising. From this POV I do like suggested
heuristics to activate --fork-point when  (either specified or
figured from configuration) is a branch. However, it seems that this
would be functionally equivalent to just making the --fork-point the
default, unconditionally, as trying to find better fork-point in a
reflog for a non-reference will bring nothing anyway. The heuristics
could be considered an optimization, but it would optimize very rare
case.

On the other hand, I'm afraid different defaults were chosen for
backward compatibility?

>>  All changes made by commits in the current branch but that are not
>>  in  are saved to a temporary area.  This is the same set
>> -of commits that would be shown by `git log ..HEAD` (or
>> -`git log HEAD`, if --root is specified).
>> +of commits that would be shown by `git log ..HEAD`; or by
>> +`git log ..HEAD`, if --fork-point is either specified or
>> +assumed; or by `git log HEAD`, if --root is specified.
>
> OK.   is a new term this patch introduces to this
> document.  Do we define what it is anywhere in this document, or
> would it help the readers to add something like "... where 
> is computed in such and such way (see ... for details)"?

Yes, it's new and is not defined, but first I didn't want to overload
the DESCRIPTION with details, and second I don't know how it's actually
done, as it seems that "fork_point=$(git merge-base 
)" sometimes returns nothing, in which case fork_point is set
back to ? If we are going to describe it, I think it should go
to --fork-point option description.

Could you please suggest the wording?

>
>> @@ -331,9 +332,8 @@ 
>> link:howto/revert-a-faulty-merge.html[revert-a-faulty-merge How-To] for 
>> details)
>>  between `upstream` and `branch` when calculating which commits have
>>  have been introduced by `branch` (see linkgit:git-merge-base[1]).
>>  +
>> -If no non-option arguments are given on the command line, then the default 
>> is
>> -`--fork-point @{u}` otherwise the `upstream` argument is interpreted 
>> literally
>> -unless the `--fork-point` option is specified.
>> +If either  or --root is given on the command line, then the
>> +default is `--no-fork-po

Re: [Bug] Query string not being phrased correctly when question marks present in config URL.

[Jeff King]

On Tue, Sep 23, 2014 at 04:20:55AM +1000, Steven Lawler wrote:

> Cause:
> [remote "repo"]
>   url = http://example.com/git/example.com?foo=bar
> There is a question mark in the URL of the repo URL.

Is the question mark there because it is separating query parameters
from the path, or is "?foo=bar" part of the literal path? If the latter,
it needs to be percent-escaped.

> Effect (Taken from Apache logs):
> [22/Sep/2014:14:12:07 -0400] "GET
> /git/example.com?foo=bar/info/refs&service=git-receive-pack HTTP/1.1"
> 403 207 "-" "git/1.9.4.msysgit.1"
> 
> Git attempts to correct the issue by making the query string continue
> using ampersands where it would have started the original query
> string.

It has to, because:

  http://example.com/git/example.com?foo=bar?service=git-receive-pack

would not mark the "service" parameter as a separate parameter of the
query-string.  However, from your expected output below, I do not think
that part is the thing you are complaining about:

> Expected outcome:
> The git client should move the ?foo=bar onto the beginning (or end) of
> the query string that it is creating. For example:
> [22/Sep/2014:14:12:07 -0400] "GET
> /git/example.com/info/refs?foo=bar&service=git-receive-pack HTTP/1.1"
> 403 207 "-" "git/1.9.4.msysgit.1"

The difference here is not the ampersand but the placement of
"info/refs".  Let's forget about the "service" parameter for a moment.
Git needs to access the "info/refs" directory of the repo specified by
the URL it is passed. Given:

  http://host/path?key=value

which part of the URL specifies the path to the repository (and that we
could munge to access info/refs)? I do not think there is a well-defined
answer. If "path" is the path to the repo and "key=value" is an
unrelated parameter, you would want:

  http://host/path/info/refs?key=value

which matches your example. But if the key is specifying the repository
path in its value, you would want to append it directly there. For
example,

  http://host/git?path=my-repo.git

should become:

  http://host/git?path=my-repo.git/info/refs

So I think you would need a config option or other mechanism to specify
which form your URL requires.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] part3: l10n: de.po: use imperative form for command options

[Phillip Sz]

Hi,

>  #: builtin/describe.c:395
>  msgid "find the tag that comes after the commit"
> -msgstr "findet das Tag, das nach Commit kommt"
> +msgstr "das Tag finden, das nach Commit kommt"
>  

"das Tag finden, das nach dem Commit kommt"

>  #: builtin/fast-export.c:718
>  msgid "Use the done feature to terminate the stream"
> -msgstr "Benutzt die \"done\"-Funktion um den Strom abzuschließen"
> +msgstr "die \"done\"-Funktion benutzen um den Strom abzuschließen"
>  

"die \"done\"-Funktion benutzen, um den Strom abzuschließen"

Maybe its better to use »Stream» instead of »Strom« in general?

Phillip


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] de.po: Fixup one translation

[Ralf Thielow]

2014-09-23 10:43 GMT+02:00 Stefan Beller :
> On 23.09.2014 10:06, Eric Sunshine wrote:
>> On Tue, Sep 23, 2014 at 3:37 AM, Stefan Beller  
>> wrote:
>>> English grammar with German words doesn't make it a German translation. ;)
>>> We also need to fix the
>>
>> Sentence fragment.
>
> Yeah, originally I intended to just fix the singular form and let the
> plural form slip. But then I did both singular and plural form and
> forgot about the already started commit message.
>
> Anyways I'd assume Ralf will incorporate this patch in a larger patch
> for the German translation, so the commit message for this particular
> patch is of minor relevance.
>

I'd prefer to pick the patch, but with no or another commit message. ;)

Thanks

> Thanks,
> Stefan
>
>>
>>> Signed-off-by: Stefan Beller 
>>> ---
>>>  po/de.po | 8 
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/po/de.po b/po/de.po
>>> index e5d2b25..bae3723 100644
>>> --- a/po/de.po
>>> +++ b/po/de.po
>>> @@ -3463,13 +3463,13 @@ msgid_plural ""
>>>  "\n"
>>>  "%s\n"
>>>  msgstr[0] ""
>>> -"Warnung: Sie sind um %d Commit hinterher, nicht verbunden zu\n"
>>> -"einem Ihrer Branches:\n"
>>> +"Warnung: Sie sind um %d Commit hinterher, folgende Commits sind in\n"
>>> +"keinem Ihrer Branches enthalten: \n"
>>>  "\n"
>>>  "%s\n"
>>>  msgstr[1] ""
>>> -"Warnung: Sie sind um %d Commits hinterher, nicht verbunden zu\n"
>>> -"einem Ihrer Branches:\n"
>>> +"Warnung: Sie sind um %d Commits hinterher, folgende Commits sind in\n"
>>> +"keinem Ihrer Branches enthalten: \n"
>>>  "\n"
>>>  "%s\n"
>>>
>>> --
>>> 2.1.0.238.gce1d3a9
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 18/35] commit_lock_file(): if close fails, roll back

[Michael Haggerty]

On 09/17/2014 12:19 AM, Jonathan Nieder wrote:
> Michael Haggerty wrote:
> 
>> If closing an open lockfile fails, then we cannot be sure of the
>> contents of the lockfile
> 
> Is that true?  It seems more like a bug in close_lock_file: if it
> fails, perhaps it should either set lk->fd back to fd or unlink the
> lockfile itself.

>From close(2) (note especially the last sentence):

> Not checking the return value of close() is a common but nevertheless serious 
>  programming
> error.   It  is  quite  possible  that  errors  on a previous write(2) 
> operation are first
> reported at the final close().  Not checking the return value when closing  
> the  file  may
> lead  to  silent  loss  of  data.   This can especially be observed with NFS 
> and with disk
> quota.  Note that the return value should only be used  for  diagnostics.   
> In  particular
> close() should not be retried after an EINTR since this may cause a reused 
> descriptor from
> another thread to be closed.

>From that I conclude that if close() fails, pretty much all bets are off
about the contents of the lockfile. So we wouldn't want to set lk->fd
back to fd.

But finishing the rollback itself might be an alternative...

> What do other callers do on close_lock_file failure?

>From what I can see, the only callers that don't die() immediately are
the following (which call close_lock_file() directly or indirectly via
write_locked_index()):

try_merge_strategy(): returns an error. It looks like this could end up
reusing the same lock_file object before it had been rolled back ->
would be improved if close_lock_file() would rollback on failure.

write_cache_as_tree(): does a rollback -> wouldn't mind an automatic
rollback.

merge_recursive_generic(): returns an error, and caller exits
immediately -> wouldn't mind an automatic rollback.

So, I will change close_lock_file() to roll back on errors.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 19/35] commit_lock_file(): rollback lock file on failure to rename

[Michael Haggerty]

On 09/17/2014 12:22 AM, Jonathan Nieder wrote:
> Michael Haggerty wrote:
> 
>> If rename() fails, call rollback_lock_file() to delete the lock file
>> (in case it is still present) and reset the filename field to the
>> empty string so that the lockfile object is left in a valid state.
> 
> Can you spell out more what the goal is?  Is the idea to keep the
> .lock file for a shorter period of time, so other processes can lock
> that file before the current process exits?

I doubt that there are situations where releasing the .lock file
immediately vs. holding it until the end of the process makes any
practical difference. Most callers die() right away if the commit fails.

This change is more about cleaning up the API by making the state
diagram for lock_file objects well-defined. commit_lock_file() can fail
in multiple ways that callers cannot easily distinguish. So all of the
failure paths have to leave the lock_file object in the same final
state, otherwise the callers cannot know what state it is in. Since
lock_file objects are reusable, this is an API design bug.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] l10n:de.po: use comma before "um"

[Phillip Sz]

Hi,
this patch added a comma before the "um". See:

http://www.duden.de/sprachwissen/rechtschreibregeln/komma#K117

Phillip 

Signed-off-by: Phillip Sz 
---
 po/de.po | 86 
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/po/de.po b/po/de.po
index c6aa69f..c807967 100644
--- a/po/de.po
+++ b/po/de.po
@@ -29,7 +29,7 @@ msgid ""
 "'git commit -a'."
 msgstr ""
 "Korrigieren Sie dies im Arbeitsverzeichnis, und benutzen Sie\n"
-"dann 'git add/rm ' um die Auflösung entsprechend zu markieren\n"
+"dann 'git add/rm ', um die Auflösung entsprechend zu markieren\n"
 "und zu committen, oder benutzen Sie 'git commit -a'."
 
 #: archive.c:10
@@ -619,7 +619,7 @@ msgstr "Fehler beim Erstellen des Pfades '%s'%s"
 #: merge-recursive.c:703
 #, c-format
 msgid "Removing %s to make room for subdirectory\n"
-msgstr "Entferne %s um Platz für Unterverzeichnis zu schaffen\n"
+msgstr "Entferne %s, um Platz für Unterverzeichnis zu schaffen\n"
 
 #: merge-recursive.c:717 merge-recursive.c:738
 msgid ": perhaps a D/F conflict?"
@@ -1037,7 +1037,7 @@ msgstr[1] "Ihr Branch ist vor '%s' um %d Commits.\n"
 
 #: remote.c:1960
 msgid "  (use \"git push\" to publish your local commits)\n"
-msgstr "  (benutzen Sie \"git push\" um lokale Commits zu publizieren)\n"
+msgstr "  (benutzen Sie \"git push\", um lokale Commits zu publizieren)\n"
 
 #: remote.c:1963
 #, c-format
@@ -1052,7 +1052,7 @@ msgstr[1] ""
 #: remote.c:1971
 msgid "  (use \"git pull\" to update your local branch)\n"
 msgstr ""
-"  (benutzen Sie \"git pull\" um Ihren lokalen Branch zu aktualisieren)\n"
+"  (benutzen Sie \"git pull\", um Ihren lokalen Branch zu aktualisieren)\n"
 
 #: remote.c:1974
 #, c-format
@@ -1072,7 +1072,7 @@ msgstr[1] ""
 #: remote.c:1984
 msgid "  (use \"git pull\" to merge the remote branch into yours)\n"
 msgstr ""
-"  (benutzen Sie \"git pull\" um Ihren Branch mit dem Remote-Branch "
+"  (benutzen Sie \"git pull\", um Ihren Branch mit dem Remote-Branch "
 "zusammenzuführen)\n"
 
 #: run-command.c:80
@@ -1136,7 +1136,7 @@ msgstr "Ihre lokalen Änderungen würden von \"revert\" 
überschrieben werden."
 #: sequencer.c:233
 msgid "Commit your changes or stash them to proceed."
 msgstr ""
-"Tragen Sie Ihre Änderungen ein oder benutzen Sie \"stash\" um fortzufahren."
+"Tragen Sie Ihre Änderungen ein oder benutzen Sie \"stash\", um fortzufahren."
 
 #: sequencer.c:250
 msgid "Failed to lock HEAD during fast_forward_to"
@@ -1488,18 +1488,18 @@ msgstr ""
 #: wt-status.c:183
 msgid "  (use \"git add ...\" to mark resolution)"
 msgstr ""
-"  (benutzen Sie \"git add/rm ...\" um die Auflösung zu markieren)"
+"  (benutzen Sie \"git add/rm ...\", um die Auflösung zu markieren)"
 
 #: wt-status.c:185 wt-status.c:189
 msgid "  (use \"git add/rm ...\" as appropriate to mark resolution)"
 msgstr ""
-"  (benutzen Sie \"git add/rm ...\" um die Auflösung entsprechend zu "
+"  (benutzen Sie \"git add/rm ...\", um die Auflösung entsprechend zu "
 "markieren)"
 
 #: wt-status.c:187
 msgid "  (use \"git rm ...\" to mark resolution)"
 msgstr ""
-"  (benutzen Sie \"git add/rm ...\" um die Auflösung zu markieren)"
+"  (benutzen Sie \"git add/rm ...\", um die Auflösung zu markieren)"
 
 #: wt-status.c:198
 msgid "Changes to be committed:"
@@ -1512,20 +1512,20 @@ msgstr "Änderungen, die nicht zum Commit vorgemerkt 
sind:"
 #: wt-status.c:220
 msgid "  (use \"git add ...\" to update what will be committed)"
 msgstr ""
-"  (benutzen Sie \"git add ...\" um die Änderungen zum Commit "
+"  (benutzen Sie \"git add ...\", um die Änderungen zum Commit "
 "vorzumerken)"
 
 #: wt-status.c:222
 msgid "  (use \"git add/rm ...\" to update what will be committed)"
 msgstr ""
-"  (benutzen Sie \"git add/rm ...\" um die Änderungen zum Commit "
+"  (benutzen Sie \"git add/rm ...\", um die Änderungen zum Commit "
 "vorzumerken)"
 
 #: wt-status.c:223
 msgid ""
 "  (use \"git checkout -- ...\" to discard changes in working directory)"
 msgstr ""
-"  (benutzen Sie \"git checkout -- ...\" um die Änderungen im "
+"  (benutzen Sie \"git checkout -- ...\", um die Änderungen im "
 "Arbeitsverzeichnis zu verwerfen)"
 
 #: wt-status.c:225
@@ -1538,7 +1538,7 @@ msgstr ""
 #, c-format
 msgid "  (use \"git %s ...\" to include in what will be committed)"
 msgstr ""
-"  (benutzen Sie \"git %s ...\" um die Änderungen zum Commit "
+"  (benutzen Sie \"git %s ...\", um die Änderungen zum Commit "
 "vorzumerken)"
 
 #: wt-status.c:252
@@ -1653,7 +1653,7 @@ msgstr "Alle Konflikte sind behoben, aber Sie sind immer 
noch beim Merge."
 
 #: wt-status.c:945
 msgid "  (use \"git commit\" to conclude merge)"
-msgstr "  (benutzen Sie \"git commit\" um den Merge abzuschließen)"
+msgstr "  (benutzen Sie \"git commit\", um den Merge abzuschließen)"
 
 #: wt-status.c:955
 msgid "You are in the middle of an am session."
@@ -1670,12 +1670,12 @@ msgstr ""
 
 #: wt-status.c:964
 msgid "  (use \"git am --skip\" to skip this patch)"
-msgstr "  (benutzen Sie \

Re: How to compile Git with NDK?

[Robert Dailey]

But if I type just 'make', I don't see how it will know where my ARM
toolchain is. I'll read the INSTALL file in the meantime.

On Mon, Sep 22, 2014 at 4:22 PM, Stefan Beller  wrote:
> On 22.09.2014 21:04, Robert Dailey wrote:
>> I run the following on Ubuntu:
>>
>> fe@BLD01:~/code/git$ autoconf
>> fe@BLD01:~/code/git$ ./configure --prefix=/home/fe/git-arm
>> --build=x86_64-linux-gnu --host=arm-linux-androideabi
>> configure: Setting lib to 'lib' (the default)
>> configure: Will try -pthread then -lpthread to enable POSIX Threads.
>> configure: CHECKS for site configuration
>> checking for arm-linux-androideabi-gcc... arm-linux-androideabi-gcc
>> checking whether the C compiler works... no
>> configure: error: in `/home/fe/code/git':
>> configure: error: C compiler cannot create executables
>> See `config.log' for more details
>>
>>
>> I have my NDK's prebuilt GCC 4.8 toolchain on PATH (the bin folder).
>> Can someone help me cross compile Git using Android NDK toolchain?
>>
>> Thanks.
>
> You don't need autoconf, just type 'make' and you'll be fine.
>
> Please read the file INSTALL for installation instructions.
>
> Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to compile Git with NDK?

[Robert Dailey]

On Tue, Sep 23, 2014 at 7:47 AM, Robert Dailey  wrote:
> But if I type just 'make', I don't see how it will know where my ARM
> toolchain is. I'll read the INSTALL file in the meantime.

Sorry for top post earlier, my mistake.

I reviewed the INSTALL file. It doesn't mention anything regarding how
to setup the toolchain for the Git build. Also looks like I'll have to
build the library dependencies with ARM toolchain as well. Looks like
zlib is the only absolute required one. Is that correct?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] de.po: Fixup one translation

[Stefan Beller]

English grammar with German words doesn't make it a German translation. ;)

Signed-off-by: Stefan Beller 
---
Here we go again without sentence fragments.

Thanks,
Stefan

 po/de.po | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/po/de.po b/po/de.po
index e5d2b25..bae3723 100644
--- a/po/de.po
+++ b/po/de.po
@@ -3463,13 +3463,13 @@ msgid_plural ""
 "\n"
 "%s\n"
 msgstr[0] ""
-"Warnung: Sie sind um %d Commit hinterher, nicht verbunden zu\n"
-"einem Ihrer Branches:\n"
+"Warnung: Sie sind um %d Commit hinterher, folgende Commits sind in\n"
+"keinem Ihrer Branches enthalten: \n"
 "\n"
 "%s\n"
 msgstr[1] ""
-"Warnung: Sie sind um %d Commits hinterher, nicht verbunden zu\n"
-"einem Ihrer Branches:\n"
+"Warnung: Sie sind um %d Commits hinterher, folgende Commits sind in\n"
+"keinem Ihrer Branches enthalten: \n"
 "\n"
 "%s\n"
 
-- 
2.1.0.238.gce1d3a9

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 20/35] api-lockfile: document edge cases

[Michael Haggerty]

On 09/17/2014 12:23 AM, Jonathan Nieder wrote:
> Michael Haggerty wrote:
> 
>> * Document the behavior of commit_lock_file() when it fails, namely
>>   that it rolls back the lock_file object and sets errno
>>   appropriately.
>>
>> * Document the behavior of rollback_lock_file() when called for a
>>   lock_file object that has already been committed or rolled back,
>>   namely that it is a NOOP.
> 
> I think this would be easier to read in a separate error handling
> section.  That way, when writing new code using the lockfile API,
> I can quickly find what functions to use and quickly find out what
> the error handling conventions are --- each use of the documentation
> wouldn't interfere with the other.

I added a little blurb in the error handling section explaining how
commit_lock_file() and close_lock_file() behave on failure, but I left
the detailed information in the sections on those functions because I
couldn't find a graceful way to put all of the information in the error
handling sections.

If you have a concrete suggestion, as they say, "patches are welcome" :-)

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 22/35] git_config_set_multivar_in_file(): avoid call to rollback_lock_file()

[Michael Haggerty]

On 09/17/2014 12:28 AM, Jonathan Nieder wrote:
> Michael Haggerty wrote:
> 
>> After commit_lock_file() is called, then the lock_file object is
>> necessarily either committed or rolled back.  So there is no need to
>> call rollback_lock_file() again in either of these cases.
>>
>> Signed-off-by: Michael Haggerty 
> 
> This seems to involve an unadvertised behavior change: before
> it wouldn't call git_config_clear() on failure, and after the
> patch it would.  Intended?
> 
> I think it would be clearer with the goto for exception handling
> maintained (and just a second 'lock = NULL' assignment).

Good catch; will fix.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 23/35] lockfile: avoid transitory invalid states

[Michael Haggerty]

On 09/17/2014 12:45 AM, Jonathan Nieder wrote:
> Michael Haggerty wrote:
> 
>> We could probably continue to use the filename field to encode the
>> state by being careful to write characters 1..N-1 of the filename
>> first, and then overwrite the NUL at filename[0] with the first
>> character of the filename, but that would be awkward and error-prone.
>>
>> So, instead of using the filename field to determine whether the
>> lock_file object is active, add a new field "lock_file::active" for
>> this purpose.
> 
> Nice.
> 
> [...]
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -576,6 +576,7 @@ extern int refresh_index(struct index_state *, unsigned 
>> int flags, const struct
>>  
>>  struct lock_file {
>>  struct lock_file *next;
>> +volatile sig_atomic_t active;
>>  int fd;
>>  pid_t owner;
>>  char on_list;
> [...]
>> +++ b/lockfile.c
>> @@ -27,16 +27,19 @@
> [...]
>> @@ -189,9 +198,14 @@ static int lock_file(struct lock_file *lk, const char 
>> *path, int flags)
>>  atexit(remove_lock_file);
>>  }
>>  
>> +if (lk->active)
>> +die("BUG: lock_file(\"%s\") called with an active lock_file 
>> object",
>> +path);
> 
> The error message doesn't make it entirely obvious to me what went
> wrong.
> 
> Maybe something like
> 
>   die("BUG: cannot lock_file(\"%s\") on active struct lock_file",
>   path);

This is an internal sanity check that users should never see, and if
they do the first thing a developer will do is grep the source code for
the error message in the bug report and then he/she will see exactly
what went wrong. So I don't think it is very important that this error
message be super self-explanatory.

But it doesn't hurt, so I'll make the change you suggest.

> lock_file already assumed on_list was initialized to zero but it
> wasn't particularly obvious since everything else is blindly
> scribbled over.  Probably worth mentioning in the API docs that
> the lock_file should be zeroed before calling hold_lock_file_...

Good point. I will document this better.

> [...]
>> @@ -326,6 +341,7 @@ int commit_lock_file(struct lock_file *lk)
>>  if (rename(lk->filename, result_file))
>>  goto rollback;
>>  
>> +lk->active = 0;
>>  lk->filename[0] = 0;
> 
> Is it useful to set filename[0] any more?
> 
> It seems potentially fragile to set both, since new code can appear
> that only sets or checks one or the other.  Would it make sense to
> rename the filename field to make sure no new callers relying on the
> filename[0] convention sneak in (with the new convention being that
> the filename doesn't get cleared, to avoid problems)?

I admit that nobody should be relying on filename being cleared anymore.
And I can see your point that somebody might come to rely on this
implementation detail. But I don't like leaving valid filenames around
where a bug might cause them to be accessed accidentally. I would rather
set the filename to the empty string so that any attempt to use it
causes an immediate error message from the OS rather than accessing the
wrong file.

I will note in the lock_file docstring that client code should not rely
on the filename being empty when in the 'unlocked' state.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] merge-recursive: Remove miss leading comment

[Stefan Beller]

Commented code, which doesn't even compile, is of no use any more?

Signed-off-by: Stefan Beller 
---

To be applied on top of sb/merge-recursive-copy-paste-fix


 merge-recursive.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index d63524c..8ad4be8 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1686,10 +1686,6 @@ static int merge_content(struct merge_options *o,
 static int process_entry(struct merge_options *o,
 const char *path, struct stage_data *entry)
 {
-   /*
-   printf("processing entry, clean cache: %s\n", index_only ? "yes": "no");
-   print_index_entry("\tpath: ", entry);
-   */
int clean_merge = 1;
int normalize = o->renormalize;
unsigned o_mode = entry->stages[1].mode;
-- 
2.1.0.238.gce1d3a9

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] l10n:de.po: use comma before "um"

[Ralf Thielow]

Am 23. September 2014 14:00 schrieb Phillip Sz :
> Hi,
> this patch added a comma before the "um". See:
>
> http://www.duden.de/sprachwissen/rechtschreibregeln/komma#K117
>

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH] fsck: do not canonicalize modes in trees we are checking

[Jeff King]

Doing so means that we do not actually get to see bogus
modes; they are converted into one of our known-good modes
by decode_tree_entry. We want to see the raw data so that we
can complain about it.

Signed-off-by: Jeff King 
---
As far as I can tell, fsck's mode-checking has been totally broken
basically forever. Which makes me a little nervous to fix it. :)
linux.git does have some bogus modes, but they are 100664, which is
specifically ignored here unless "fsck --strict" is in effect.

The implementation feels a bit hacky. The global is ugly, but it avoids
having to pass options all through the call chain of init_tree_entry,
update_tree_entry, etc.

Another option would be to have decode_tree_entry leave the raw mode in
the tree_desc, and only canonicalize it during tree_entry_extract (and
teach fsck to look at the raw data, not _extract). That would mean
reverting 7146e66 (tree-walk: finally switch over tree descriptors to
contain a pre-parsed entry, 2014-02-06). That commit says there's no
real benefit at the time, but that future code might benefit. I don't
know if that future code ever materialized (Kirill cc'd).

I thought at first that commit might have been responsible for this
breakage, but I don't think so. The canonicalization has happened in
tree_entry_extract since at least 2006, and we have always called that
function in fsck.

 fsck.c  |  2 ++
 t/t1450-fsck.sh | 21 +
 tree-walk.c |  4 +++-
 tree-walk.h |  3 +++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/fsck.c b/fsck.c
index 56156ff..ef79396 100644
--- a/fsck.c
+++ b/fsck.c
@@ -153,6 +153,7 @@ static int fsck_tree(struct tree *item, int strict, 
fsck_error error_func)
unsigned o_mode;
const char *o_name;
 
+   decode_tree_raw = 1;
init_tree_desc(&desc, item->buffer, item->size);
 
o_mode = 0;
@@ -213,6 +214,7 @@ static int fsck_tree(struct tree *item, int strict, 
fsck_error error_func)
o_name = name;
}
 
+   decode_tree_raw = 0;
retval = 0;
if (has_null_sha1)
retval += error_func(&item->object, FSCK_WARN, "contains 
entries pointing to null sha1");
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index b52397a..ba40c6f 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -176,6 +176,27 @@ test_expect_success 'malformatted tree object' '
grep "error in tree .*contains duplicate file entries" out
 '
 
+# Basically an 8-bit clean version of sed 's/$1/$2/g'.
+munge_tree_bytes () {
+   perl -e '
+   binmode(STDIN); binmode(STDOUT);
+   $_ = do { local $/;  };
+   s/$ARGV[0]/$ARGV[1]/g;
+   print
+   ' "$@"
+}
+
+test_expect_success 'bogus mode in tree' '
+   test_when_finished "remove_object \$T" &&
+   T=$(
+   git cat-file tree HEAD >good &&
+   munge_tree_bytes 100644 123456 bad &&
+   git hash-object -w -t tree bad
+   ) &&
+   git fsck 2>out &&
+   grep "warning.*contains bad file modes" out
+'
+
 test_expect_success 'tag pointing to nonexistent' '
cat >invalid-tag <<-\EOF &&
object 
diff --git a/tree-walk.c b/tree-walk.c
index 5dd9a71..baca766 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -5,6 +5,8 @@
 #include "tree.h"
 #include "pathspec.h"
 
+int decode_tree_raw;
+
 static const char *get_mode(const char *str, unsigned int *modep)
 {
unsigned char c;
@@ -37,7 +39,7 @@ static void decode_tree_entry(struct tree_desc *desc, const 
char *buf, unsigned
 
/* Initialize the descriptor entry */
desc->entry.path = path;
-   desc->entry.mode = canon_mode(mode);
+   desc->entry.mode = decode_tree_raw ? mode : canon_mode(mode);
desc->entry.sha1 = (const unsigned char *)(path + len);
 }
 
diff --git a/tree-walk.h b/tree-walk.h
index ae7fb3a..9f78e85 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -25,6 +25,9 @@ static inline int tree_entry_len(const struct name_entry *ne)
return (const char *)ne->sha1 - ne->path - 1;
 }
 
+/* If set, do not canonicalize modes in tree entries. */
+extern int decode_tree_raw;
+
 void update_tree_entry(struct tree_desc *);
 void init_tree_desc(struct tree_desc *desc, const void *buf, unsigned long 
size);
 
-- 
2.1.1.496.g1ba8081
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] git tag --contains: avoid stack overflow

[Jeff King]

On Sat, Sep 20, 2014 at 08:18:59PM +0200, Andreas Schwab wrote:

> Stepan Kasal  writes:
> 
> > diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> > index 143a8ea..a911df0 100755
> > --- a/t/t7004-tag.sh
> > +++ b/t/t7004-tag.sh
> > @@ -1423,4 +1423,30 @@ EOF
> > test_cmp expect actual
> >  '
> >  
> > +run_with_limited_stack () {
> > +   (ulimit -s 64 && "$@")
> > +}
> 
> That is way too small.
> 
> https://build.opensuse.org/package/live_build_log/openSUSE:Factory:PowerPC/git/standard/ppc64le

Thanks for the report. I'd be OK with just giving up and dropping this
test as too flaky and system-specific to be worth the trouble.

But if we do want to keep it, does bumping it to 128 (and bumping the
4000 to 8000 in the test below it) work?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to compile Git with NDK?

[Jeff King]

On Tue, Sep 23, 2014 at 07:47:11AM -0500, Robert Dailey wrote:

> But if I type just 'make', I don't see how it will know where my ARM
> toolchain is. I'll read the INSTALL file in the meantime.

It won't. If you are cross-compiling you'll have to specify CC and LD
manually, plus a host of other settings. We usually pick pretty sane
defaults (which is why you can get away without running autoconf), but
they're not going to be reasonable for cross-compiling. If you do go the
non-autoconf route, you'd probably want to try building with "make
uname_S=whatever" to override our defaults (see config.mak.uname for an
idea of which uname variables we look at).

In your original report:

> >> fe@BLD01:~/code/git$ autoconf
> >> fe@BLD01:~/code/git$ ./configure --prefix=/home/fe/git-arm
> >> --build=x86_64-linux-gnu --host=arm-linux-androideabi
> >> configure: Setting lib to 'lib' (the default)
> >> configure: Will try -pthread then -lpthread to enable POSIX Threads.
> >> configure: CHECKS for site configuration
> >> checking for arm-linux-androideabi-gcc... arm-linux-androideabi-gcc
> >> checking whether the C compiler works... no
> >> configure: error: in `/home/fe/code/git':
> >> configure: error: C compiler cannot create executables
> >> See `config.log' for more details

Autoconf couldn't even build a simple hello-world with the compiler you
specified. So the first step would probably be to figure that out. What
does config.log say?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to compile Git with NDK?

[Jeff King]

On Tue, Sep 23, 2014 at 07:52:32AM -0500, Robert Dailey wrote:

> On Tue, Sep 23, 2014 at 7:47 AM, Robert Dailey  
> wrote:
> > But if I type just 'make', I don't see how it will know where my ARM
> > toolchain is. I'll read the INSTALL file in the meantime.
> 
> Sorry for top post earlier, my mistake.
> 
> I reviewed the INSTALL file. It doesn't mention anything regarding how
> to setup the toolchain for the Git build. Also looks like I'll have to
> build the library dependencies with ARM toolchain as well. Looks like
> zlib is the only absolute required one. Is that correct?

Yes, zlib is the only absolute dependency. However, libcurl is useful if
you want to clone http repositories.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] fsck: do not canonicalize modes in trees we are checking

[Edward Thomson]

On Tue, Sep 23, 2014 at 11:47:51AM -0400, Jeff King wrote:
> As far as I can tell, fsck's mode-checking has been totally broken
> basically forever. Which makes me a little nervous to fix it. :)
> linux.git does have some bogus modes, but they are 100664, which is
> specifically ignored here unless "fsck --strict" is in effect.

I'm in favor of checking the mode in fsck, at least when --strict.  
But I would suggest we be lax (by default) about other likely-to-exist
but strictly invalid modes to prevent peoples previously workable
repositories from being now broken.

I have, for example, encountered 100775 in the wild, and would argue that
like 100644, it should probably not fail unless we are in --strict mode.

Thanks-
-ed
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] fsck: do not canonicalize modes in trees we are checking

[Jeff King]

[-cc Kirill, as his address seem out-of-date]

On Tue, Sep 23, 2014 at 04:23:43PM +, Edward Thomson wrote:

> On Tue, Sep 23, 2014 at 11:47:51AM -0400, Jeff King wrote:
> > As far as I can tell, fsck's mode-checking has been totally broken
> > basically forever. Which makes me a little nervous to fix it. :)
> > linux.git does have some bogus modes, but they are 100664, which is
> > specifically ignored here unless "fsck --strict" is in effect.
> 
> I'm in favor of checking the mode in fsck, at least when --strict.  
> But I would suggest we be lax (by default) about other likely-to-exist
> but strictly invalid modes to prevent peoples previously workable
> repositories from being now broken.
> 
> I have, for example, encountered 100775 in the wild, and would argue that
> like 100644, it should probably not fail unless we are in --strict mode.

Yeah, I'd agree with that. The big question is: what breakage have we
seen in the wild? :)

I think treating 100775 the same as 100664 makes sense (want to do a
patch?). Do we know of any others? I guess we can collect them as time
goes on and reports come in. That's not the nicest thing for people with
such repos, but then again, their repos _are_ broken (and it's only
really a showstopper if they are trying to push to somebody with
receive.fsckObjects turned on).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] archive: support filtering paths with glob

[Junio C Hamano]

Duy Nguyen  writes:

> On Tue, Sep 23, 2014 at 2:15 AM, Junio C Hamano  wrote:
>> When we have a/b/c and a/d/e to be written, the first round would
>> write a/ and then a/b/ with the above, and presumably elsewhere
>> somebody will write a/b/c; next time around we do need to write a/d/
>> but we wouldn't want to write a/ itself.  How is this code
>> preventing the recursion going all the way up every time to avoid
>> repeating a/?
>>
>> Puzzled...
>
> We never traverse 'a' (or any directory) twice and we only push a
> directory to the stack when we examine it. After a/b and a are written
> down and we examine 'd', 'a/d' is pushed to the stack. When we hit
> 'a/d/e', we only have 'a/d' in the stack, not 'a'.

So the traverser will feed you

a/
a/b/
a/b/c
a/d/
a/d/e

in that order, and you keep a/ and a/b/ until you see a/b/c at which
time you flush the two queued ones and likewise for a/d/ and a/d/e.
When you queue a/d/ you do not decompose that to a/ and a/d/ because
you know that the caller of the callback would have made sure all
the leading path components have been given you by the time it gave
you a/d/, and it all works out fine.

Thanks for clarifying.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] git-am: support any number of signatures

[Junio C Hamano]

Christian Couder  writes:

> On Mon, Sep 22, 2014 at 4:01 PM, Michael S. Tsirkin  wrote:
>> ...
>> As a reminder, this old patchset (that I replied to) enhanced git am -s
>> with an option to add different signatures depending on
>> the option passed to the -s flag.
>> E.g. I have
>> [am "a"]
>> signoff = "Acked-by: Michael S. Tsirkin "
>>
>> [am "r"]
>> signoff = "Reviewed-by: Michael S. Tsirkin "
>>
>> [am "t"]
>> signoff = "Tested-by: Michael S. Tsirkin "
>>
>> and now:
>> git am -s art
>> adds all 3 signatures when applying the patch.
>
> This is probably not as simple as you would like but it works with
> something like:
>
> $ git interpret-trailers --trailer "Acked-by: Michael S. Tsirkin
> " --trailer "Reviewed-by: Michael S. Tsirkin
> "  --trailer "Tested-by: Michael S. Tsirkin
> " 0001-foo.patch >to_apply/0001-foo.patch
>
> and then:
>
> $ git am to_apply/*.patch

If I understand it correctly, Michael is envisioning to implement
his "git am -s art" (I would recommend against reusing -s for this,
though.  "git am --trailer art" is fine) and doing so by using
interpret-trailers as an internal implementation detail, so I would
say the above is a perfectly fine way to do so.  An equivalent of
that command line is synthesized and run internally in his version
of "git am" when his "git am" sees "--trailer art" option using
those am.{"a","r","t"}.trailer configuration variables.

> Also by using something like:
>
> $ git config trailer.a.key Acked-by
> $ git config trailer.r.key Reviewed-by
> $ git config trailer.t.key Tested-by
>
> the first command could be simplified to:

So I think this mechanism buys Michael's use case very little, if
any.  It might be useful in other contexts, though.

What would be more interesting is if the primitives you have,
e.g. "replace", "append", etc. are sufficient to express his use
case and similar ones.  For example, when working on multiple
trailers (e.g. "am --trailer art" would muck with three kinds), how
should "do this if exists at the end and do that otherwise" work?
To an existing message ends with Michael's Signed-off-by:, if his
"git am --trailer arts" is called to add these three and then a
Signed-off-by: from him, should it add an extra S-o-b (because his
existing S-o-b will no longer be the last one after adding Acked and
others), or should it refrain from doing so?  Can you express both
preferences?

Another thing that got me wondered this morning while I was thinking
about this topic was if "replace" is flexible enough.  We may want
to have "if an entry exists (not necessarily at the end), remove it
and then append a new one with this value at the end" to implement
"Last-tested-by: me@my.domain", for example.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] merge-recursive: Remove miss leading comment

[Junio C Hamano]

Stefan Beller  writes:

> Commented code, which doesn't even compile, is of no use any more?

Apparently that is meant to help debugging the code.  An alternative
would be to make it usable again, but removal is fine by me as well.

>
> Signed-off-by: Stefan Beller 
> ---
>
> To be applied on top of sb/merge-recursive-copy-paste-fix
>
>
>  merge-recursive.c | 4 
>  1 file changed, 4 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index d63524c..8ad4be8 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1686,10 +1686,6 @@ static int merge_content(struct merge_options *o,
>  static int process_entry(struct merge_options *o,
>const char *path, struct stage_data *entry)
>  {
> - /*
> - printf("processing entry, clean cache: %s\n", index_only ? "yes": "no");
> - print_index_entry("\tpath: ", entry);
> - */
>   int clean_merge = 1;
>   int normalize = o->renormalize;
>   unsigned o_mode = entry->stages[1].mode;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

[Junio C Hamano]

Laszlo Ersek  writes:

>   git format-patch master..branch1

The output from this has these (excerpt from "od -xc" output):

360   f   2  \n  \n   d   i   f   f   -   -   g   i   t
   66200a32640a666920662d2d69672074
400   a   /   f   2   b   /   f   2  \n   n   e   w   f   i
   2f6132666220662f0a32656e20776966
420   l   e   m   o   d   e   1   0   0   6   4   4  \n   i
   656c6d20646f2065303136303434690a
440   n   d   e   x   0   0   0   0   0   0   0   .   .   f   3
   646e786530203030303030302e2e3366
460   5   d   3   e   6  \n   -   -   -   /   d   e   v   /   n
   643565330a362d2d202d642f76656e2f
500   u   l   l  \n   +   +   +   b   /   f   2  \n   @   @
   6c750a6c2b2b202b2f623266400a2040
520   -   0   ,   0   +   1   @   @  \n   +   h   e   l   l
   302d302c2b20203140402b0a65686c6c
540   o   w   o   r   l   d  \r  \n   -   -  \n   2   .   1
   206f6f776c720d642d0a202d320a312e

The structural parts of the diff, including "--- /dev/null" line,
are all terminated by "\n" (as they should be), and the only CR
appears in the message is at the end of "+hello world" line.

So I do not think apply should need to loosen its sanity check and
take a random whitespace after the "/dev/null" as a valid "this is a
creation event for the path" marker (e.g. "--- /dev/null whoa"?).

is_dev_null() is used to in the fallback code path that parses
traditional patch output (e.g. GNU diff) which throws random cruft
(e.g. timestamp) after the /dev/null marker, e.g.

$ diff -u /dev/null f2
--- /dev/null   2014-09-17 18:22:57.995111003 -0700
+++ f2  2014-09-23 11:37:09.0 -0700
@@ -0,0 +1 @@
+hello world

and we'd be hesitant to allow that kind of looseness for Git patches
where we know we end the line after the "/dev/null" marker.

> 3. In the reviewer / tester / maintainer role, save the patch from your
> email client to a local file. Assume that your email client does not
> corrupt the patch when saving it.

Perhaps compare this saved file with the output from the above
format-patch to see where things got broken?

SMTP transport may be CRLF-unsafe, so I have a suspicion that it may
turn out that what you are trying to do might be an equilvalent of

git format-patch ... |
# first lose all \r\n
dos2unix | 
# then make everything \r\n
unix2dos |
# and apply
git am

which is not workable in the first place.  I dunno.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Passing tar(1) options via git-archive(1)

[Daniel Brockman]

Some background from the git-archive(1) man page:

git-archive behaves differently when given a tree ID versus when
given a commit ID or tag ID.  In the first case the current time is
used as the modification time of each file in the archive.  In the
latter case the commit time as recorded in the referenced commit
object is used instead.

Would it make sense to add an --mtime option to git-archive(1) to enable
explicitly setting the mtime for all files in the archive?  It could
just pass through to the tar(1) --mtime option.

My use case is `git archive HEAD | docker build -`, in which the Docker
cache is prevented from working because the mtime keeps getting bumped
on all files.  I would like to have the mtime always be the same.

See, e.g., .

Otherwise, how about a generic way to pass options to tar(1)?

Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

BUG?: git-filter-branch removes mergetags

[Uwe Kleine-König]

Hello,

I just noticed that git-filter-branch doesn't preserve mergetag
annotations in situations where it could. Here is an example (with
linux.git):

$ git checkout -b test 6cd2f85413eef8fe7bcd7c25bf55e7b055fa257c
$ git cat-file commit HEAD | grep mergetag
mergetag object 954263938706bf62d36e81b6b49f313390f2ed35
$ git filter-branch -f --msg-filter 'sed s/foo/bar/' HEAD^!
Rewrite 6cd2f85413eef8fe7bcd7c25bf55e7b055fa257c (1/1)
Ref 'refs/heads/test' was rewritten
$ diff -u <(git cat-file commit refs/original/refs/heads/test) <(git 
cat-file commit HEAD)
--- /dev/fd/63  2014-09-23 21:07:49.987065017 +0200
+++ /dev/fd/62  2014-09-23 21:07:49.991064988 +0200
@@ -3,32 +3,6 @@
 parent 954263938706bf62d36e81b6b49f313390f2ed35
 author Linus Torvalds  1411488823 -0700
 committer Linus Torvalds  1411488823 
-0700
-mergetag object 954263938706bf62d36e81b6b49f313390f2ed35
- type commit
- tag for-linus
- tagger Paolo Bonzini  1411478481 +0200
- 
- Another fix for 3.17 arrived at just the wrong time, after I had sent
- yesterday's pull request.  Normally I would have waited for
- some other patches to pile up, but since 3.17 might be short
- here it is.
- -BEGIN PGP SIGNATURE-
- Version: GnuPG v2.0.22 (GNU/Linux)
- 
- iQIcBAABAgAGBQJUIXPRAAoJEBvWZb6bTYbyySEP/AkPjfNGYqwBbM8GUJ4tt4gR
- C+xbiO+xPr2qCwfi36DQtL0UPwJHWSq7SXaDMvSqMo22FjnFcVaGuQcGAPno/8ZA
- tLBe1km9HIPlEIV3vpoe8PPpj9cuZ86+YOCuPIqK5fC7l6Ops0dhCOjf88tmPVQ4
- yhodpJ1Lt/sPBUWb6pNfk0iWD+qSbfUWtwzv89oudEvLcLiAcPSBdbvnxVS3bmGm
- qbL8pvCOozK5GJbl0+cYWCoEPBP5ekqGvwvGdEBTx+4qv2S2htzUX30UA2VYy5IR
- jMXVrJbvSW9FXQdBgr0Q4ql6evOVjL+5LpwgRUC6tuC6r1rMP+nXyHKS35HC1i8W
- FYr62B/LZTm4vyDHsmsiEl43VLAcF7kmXufQT62vJg+ifeA1MAMIJra7ZDx8WbsD
- HDqM+CeaZrF3p4okRrktbecQdeFFyg4wOasHRTO7SETkbP7i1cS0Mp8rRbX3CnJq
- 0UM8STe+hViXR7uYZEbjlbGKkszDS49fstJIaNm9JPJm+S/V5/MZFelNWgPp/+kF
- xpQUxtoSaFnqgBXpRZ7t2Y2zGeZkMWn/P84S23/7K1TfRPCsUpgFbiY26rGW9l4v
- r8gz7v+V1gCzWYVRuEzolFrea6A1ik2sspzeDuZOrf+QYwMyyUdEQ/NfCm032wba
- CYL0V2M/dJNmZnZRPP9/
- =ZkSE
- -END PGP SIGNATURE-
 
 Merge tag 'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm
 
I expected the signature to not disappear such that in the example above
no change is introduced.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

[Laszlo Ersek]

On 09/23/14 20:54, Junio C Hamano wrote:
> Laszlo Ersek  writes:
> 
>>   git format-patch master..branch1
> 
> The output from this has these (excerpt from "od -xc" output):
> 
> 360   f   2  \n  \n   d   i   f   f   -   -   g   i   t
>66200a32640a666920662d2d69672074
> 400   a   /   f   2   b   /   f   2  \n   n   e   w   f   i
>2f6132666220662f0a32656e20776966
> 420   l   e   m   o   d   e   1   0   0   6   4   4  \n   i
>656c6d20646f2065303136303434690a
> 440   n   d   e   x   0   0   0   0   0   0   0   .   .   f   3
>646e786530203030303030302e2e3366
> 460   5   d   3   e   6  \n   -   -   -   /   d   e   v   /   n
>643565330a362d2d202d642f76656e2f
> 500   u   l   l  \n   +   +   +   b   /   f   2  \n   @   @
>6c750a6c2b2b202b2f623266400a2040
> 520   -   0   ,   0   +   1   @   @  \n   +   h   e   l   l
>302d302c2b20203140402b0a65686c6c
> 540   o   w   o   r   l   d  \r  \n   -   -  \n   2   .   1
>206f6f776c720d642d0a202d320a312e
> 
> The structural parts of the diff, including "--- /dev/null" line,
> are all terminated by "\n" (as they should be), and the only CR
> appears in the message is at the end of "+hello world" line.

That's right -- until the patch email goes through an MTA that turns all
line endings into CRLF. (Did you email the patch to yourself as
requested in the reproducer?)

Such CRLFs are normally transparent because git-am strips them. The
keepcr=true setting preserves them, but not only for the source code
lines (where it's the right thing to do): it also preserves them in the
git diff header lines. Which is not a problem in general, *except* when
said header line includes /dev/null.

> So I do not think apply should need to loosen its sanity check and
> take a random whitespace after the "/dev/null" as a valid "this is a
> creation event for the path" marker (e.g. "--- /dev/null whoa"?).
> 
> is_dev_null() is used to in the fallback code path that parses
> traditional patch output (e.g. GNU diff) which throws random cruft
> (e.g. timestamp) after the /dev/null marker, e.g.
> 
> $ diff -u /dev/null f2
> --- /dev/null   2014-09-17 18:22:57.995111003 -0700
> +++ f2  2014-09-23 11:37:09.0 -0700
> @@ -0,0 +1 @@
> +hello world
> 
> and we'd be hesitant to allow that kind of looseness for Git patches
> where we know we end the line after the "/dev/null" marker.

Okay, let's say I only relax the check in question to accept "\r\n" in
addition to the current "\n". Will you take that?

>> 3. In the reviewer / tester / maintainer role, save the patch from your
>> email client to a local file. Assume that your email client does not
>> corrupt the patch when saving it.
> 
> Perhaps compare this saved file with the output from the above
> format-patch to see where things got broken?
> 
> SMTP transport may be CRLF-unsafe, so I have a suspicion that it may
> turn out that what you are trying to do might be an equilvalent of
> 
>   git format-patch ... |
> # first lose all \r\n
> dos2unix | 
>   # then make everything \r\n
> unix2dos |
> # and apply
> git am
> 
> which is not workable in the first place.  I dunno.

I agree with your analysis. It is indeed the MTA (or the MUA, not sure)
that turns all line endings into uniform CRLFs -- it is a requirement in
RFC 2822 / 822bis.

http://cr.yp.to/docs/smtplf.html
http://www.rfc-editor.org/rfc/rfc2822.txt

> 2.3. Body
>
>The body of a message is simply lines of US-ASCII characters.  The
>only two limitations on the body are as follows:
>
>- CR and LF MUST only occur together as CRLF; they MUST NOT appear
>  independently in the body.

But why is this situation "not workable"? The same happens with *all*
patches that people mail around, it's just not visible to them, because
git-am strips all CRs indiscriminately.

People who are forced to work with CRLF repositories don't have this
luxury with git-am, and bump into the /dev/null problem all the time.

What do you think about accepting only "/dev/null\n" and "/dev/null\r\n"?

Another question I had about gitdiff_verify_name() -- what ensures there
that the memcmp(), with the fixed size of 9 bytes, won't fall off the
end of the line? Some of the outer caller functions verify the line
length before their comparisons, but I don't see any length checks in
gitdiff_verify_name() for /dev/null specifically.

Thank you,
Laszlo
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

[Junio C Hamano]

Laszlo Ersek  writes:

> What do you think about accepting only "/dev/null\n" and "/dev/null\r\n"?

I thought we agreed that what you are doing is not workable in the
first place, no?

I suspect one way to handle "In this project, the files that are
checked out must be with CRLF line endings no matter what the
platform is" might be to use the line ending attributes to force
that while keeping the in-repository data with LF line endings.  The
diff output (format-patch output is just one of them) comes from
comparing the in-repository representation, so you won't have \r\n
that will be stripped via MTA in it, "apply" and "am" will apply the
patch without having to worry about \r\n, _and_ the line ending
attributes would end the lines in your in-working-tree files with
CRLF that way.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

[Junio C Hamano]

Laszlo Ersek  writes:

> On 09/23/14 20:54, Junio C Hamano wrote:
> ...
>> SMTP transport may be CRLF-unsafe, so I have a suspicion that it may
>> turn out that what you are trying to do might be an equilvalent of
>> 
>>  git format-patch ... |
>> # first lose all \r\n
>> dos2unix | 
>>  # then make everything \r\n
>> unix2dos |
>> # and apply
>> git am
>> 
>> which is not workable in the first place.  I dunno.
>
> I agree with your analysis. It is indeed the MTA...
>>- CR and LF MUST only occur together as CRLF; they MUST NOT appear
>>  independently in the body.
>
> But why is this situation "not workable"? The same happens with *all*
> patches that people mail around, it's just not visible to them, because
> git-am strips all CRs indiscriminately.

It is not "git am" or "git apply" that "strips all CRs
indiscriminately".  I just tried to apply 0001-add-f2 without
letting your MTA/MUA corrupt it on "master" branch in the repository
you prepared that patch from, i.e.

git checkout master^0 ;# go back
git am 0001-add-f2* ;# apply that "+hello world\r\n" patch
git diff branch ;# nothing

> Another question I had about gitdiff_verify_name() -- what ensures there
> that the memcmp(), with the fixed size of 9 bytes,...

That may be worth fixing.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

[Junio C Hamano]

Junio C Hamano  writes:

> SMTP transport may be CRLF-unsafe, so I have a suspicion that it may
> turn out that what you are trying to do might be an equilvalent of
>
>   git format-patch ... |
> # first lose all \r\n
> dos2unix | 
>   # then make everything \r\n
> unix2dos |
> # and apply
> git am
>
> which is not workable in the first place.  I dunno.

This is a tangent, but if the problem were slightly different, I
would be more sympathetic.  For example

A popular MUA, when asked to write out a message in the mbox
format, ends _all_ the lines in its output with CRLF, whether
the original was sent as LF-only or CRLF, and there is no way to
convince it to use LF-only.  "git apply" fails to grok such an
input.

could be, if the use of such an MUA is very prevalent, a common
problem worth working around.

But then I would suspect that the workaround for such a case may not
be "accept /dev/null\n and /dev/null\r\n equally".  It is likely
that the right workaround for such a case would be to "turn all \r\n
into \n before processing".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: BUG?: git-filter-branch removes mergetags

[Junio C Hamano]

Uwe Kleine-König   writes:

>   --- /dev/fd/63  2014-09-23 21:07:49.987065017 +0200
>   +++ /dev/fd/62  2014-09-23 21:07:49.991064988 +0200
>   @@ -3,32 +3,6 @@
>parent 954263938706bf62d36e81b6b49f313390f2ed35
>author Linus Torvalds  1411488823 -0700
>committer Linus Torvalds  1411488823 
> -0700
>   -mergetag object 954263938706bf62d36e81b6b49f313390f2ed35
>   - type commit
> ...
>
> I expected the signature to not disappear such that in the example above
> no change is introduced.

Yeah, I agree that the above shows that in the rewritten merge
commit, the side parent 95426393... has not changed from the
original and the mergetag can be taken as a still valid one.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

[Junio C Hamano]

Laszlo Ersek  writes:

> On 09/23/14 22:02, Junio C Hamano wrote:
>> Laszlo Ersek  writes:
>> 
>>> On 09/23/14 20:54, Junio C Hamano wrote:
>>> ...
 SMTP transport may be CRLF-unsafe, so I have a suspicion that it may
 turn out that what you are trying to do might be an equilvalent of

git format-patch ... |
 # first lose all \r\n
 dos2unix | 
# then make everything \r\n
 unix2dos |
 # and apply
 git am

 which is not workable in the first place.  I dunno.
>>>
>>> I agree with your analysis. It is indeed the MTA...
- CR and LF MUST only occur together as CRLF; they MUST NOT appear
  independently in the body.
>>>
>>> But why is this situation "not workable"? The same happens with *all*
>>> patches that people mail around, it's just not visible to them, because
>>> git-am strips all CRs indiscriminately.
>> 
>> It is not "git am" or "git apply" that "strips all CRs
>> indiscriminately".  I just tried to apply 0001-add-f2 without
>> letting your MTA/MUA corrupt it on "master" branch in the repository
>> you prepared that patch from, i.e.
>> 
>>  git checkout master^0 ;# go back
>> git am 0001-add-f2* ;# apply that "+hello world\r\n" patch
>> git diff branch ;# nothing
>
> When you did this, was am.keepcr=true in effect?

I actually briefly scratched my head but realized when I saw it work
"as expected" without me passing "--keep-cr" to "am" myself.

But I did that experiment in the repository created by following
your reproduction recipe, in which it had these:

  git config core.whitespace cr-at-eol
  git config am.keepcr true

so yes I had keepcr set.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

[Laszlo Ersek]

On 09/23/14 22:02, Junio C Hamano wrote:
> Laszlo Ersek  writes:
> 
>> On 09/23/14 20:54, Junio C Hamano wrote:
>> ...
>>> SMTP transport may be CRLF-unsafe, so I have a suspicion that it may
>>> turn out that what you are trying to do might be an equilvalent of
>>>
>>> git format-patch ... |
>>> # first lose all \r\n
>>> dos2unix | 
>>> # then make everything \r\n
>>> unix2dos |
>>> # and apply
>>> git am
>>>
>>> which is not workable in the first place.  I dunno.
>>
>> I agree with your analysis. It is indeed the MTA...
>>>- CR and LF MUST only occur together as CRLF; they MUST NOT appear
>>>  independently in the body.
>>
>> But why is this situation "not workable"? The same happens with *all*
>> patches that people mail around, it's just not visible to them, because
>> git-am strips all CRs indiscriminately.
> 
> It is not "git am" or "git apply" that "strips all CRs
> indiscriminately".  I just tried to apply 0001-add-f2 without
> letting your MTA/MUA corrupt it on "master" branch in the repository
> you prepared that patch from, i.e.
> 
>   git checkout master^0 ;# go back
> git am 0001-add-f2* ;# apply that "+hello world\r\n" patch
> git diff branch ;# nothing

When you did this, was am.keepcr=true in effect?

Thanks
Laszlo
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

[Laszlo Ersek]

On 09/23/14 21:56, Junio C Hamano wrote:
> Laszlo Ersek  writes:
> 
>> What do you think about accepting only "/dev/null\n" and "/dev/null\r\n"?
> 
> I thought we agreed that what you are doing is not workable in the
> first place, no?
> 
> I suspect one way to handle "In this project, the files that are
> checked out must be with CRLF line endings no matter what the
> platform is" might be to use the line ending attributes to force
> that while keeping the in-repository data with LF line endings.  The
> diff output (format-patch output is just one of them) comes from
> comparing the in-repository representation, so you won't have \r\n
> that will be stripped via MTA in it, "apply" and "am" will apply the
> patch without having to worry about \r\n, _and_ the line ending
> attributes would end the lines in your in-working-tree files with
> CRLF that way.

This would be a perfect solution if the git repository was not a mirror
of a Subversion repository that contains all files with embedded CRLFs.

Anyway I accept defeat, thanks for your time.

Laszlo

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

[Junio C Hamano]

Laszlo Ersek  writes:

> On 09/23/14 21:56, Junio C Hamano wrote:
>> Laszlo Ersek  writes:
>> 
>>> What do you think about accepting only "/dev/null\n" and "/dev/null\r\n"?
>> 
>> I thought we agreed that what you are doing is not workable in the
>> first place, no?
>> 
>> I suspect one way to handle "In this project, the files that are
>> checked out must be with CRLF line endings no matter what the
>> platform is" might be to use the line ending attributes to force
>> that while keeping the in-repository data with LF line endings.  The
>> diff output (format-patch output is just one of them) comes from
>> comparing the in-repository representation, so you won't have \r\n
>> that will be stripped via MTA in it, "apply" and "am" will apply the
>> patch without having to worry about \r\n, _and_ the line ending
>> attributes would end the lines in your in-working-tree files with
>> CRLF that way.
>
> This would be a perfect solution if the git repository was not a mirror
> of a Subversion repository that contains all files with embedded CRLFs.

Yikes.

> Anyway I accept defeat, thanks for your time.

I do not consider that a "defeat".  It is just I do not think it is
the right solution for the problem you are having to butcher "apply".
You'd need to find some other way to solve it, and other people on
the list may be able to offer a solution neither of us thought of in
this thread.

Perhaps those who are on Windows have more experience in situations
like yours?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

[Laszlo Ersek]

On 09/23/14 22:35, Junio C Hamano wrote:
> Laszlo Ersek  writes:
> 
>> On 09/23/14 22:02, Junio C Hamano wrote:
>>> Laszlo Ersek  writes:
>>>
 On 09/23/14 20:54, Junio C Hamano wrote:
 ...
> SMTP transport may be CRLF-unsafe, so I have a suspicion that it may
> turn out that what you are trying to do might be an equilvalent of
>
>   git format-patch ... |
> # first lose all \r\n
> dos2unix | 
>   # then make everything \r\n
> unix2dos |
> # and apply
> git am
>
> which is not workable in the first place.  I dunno.

 I agree with your analysis. It is indeed the MTA...
>- CR and LF MUST only occur together as CRLF; they MUST NOT appear
>  independently in the body.

 But why is this situation "not workable"? The same happens with *all*
 patches that people mail around, it's just not visible to them, because
 git-am strips all CRs indiscriminately.
>>>
>>> It is not "git am" or "git apply" that "strips all CRs
>>> indiscriminately".  I just tried to apply 0001-add-f2 without
>>> letting your MTA/MUA corrupt it on "master" branch in the repository
>>> you prepared that patch from, i.e.
>>>
>>> git checkout master^0 ;# go back
>>> git am 0001-add-f2* ;# apply that "+hello world\r\n" patch
>>> git diff branch ;# nothing
>>
>> When you did this, was am.keepcr=true in effect?
> 
> I actually briefly scratched my head but realized when I saw it work
> "as expected" without me passing "--keep-cr" to "am" myself.
> 
> But I did that experiment in the repository created by following
> your reproduction recipe, in which it had these:
> 
>   git config core.whitespace cr-at-eol
>   git config am.keepcr true
> 
> so yes I had keepcr set.

Thank you for confirming, I expected so.

Because in this case the test doesn't refute my claim that "git-am
strips all CRs indiscriminately".

Git-am *does* strip all CRs indiscriminately, undoing the CRs that the
email servers / clients introduce. Your above test worked out because
you prevented git-am from stripping the CRs, with the keepcr=true
setting. If you turn that off, then either your git am command won't
succeed (because it will run into context conflicts due to different
line endings -- although not in this example), or the final git-diff
will report differences.

In summary:
- the email infrastructure turns all line terminators into CRLFs
- git-am strips these by default, from source code lines and from git
  diff header lines alike,
- this is fine for repos that store files with \n terminators,
- not fine for repos with embedded \r\n terminators -- the default
  stripping behavior of git-am breaks the source code in that case
  (runs into conflicts with existing files, and creates new files with
  wrong line endings)
- if you set am.keepcr=true, then the source code remains intact (no
  conflicts for existing files), but new files cannot be created,
  because the /dev/null\r header lines are rejected.

Thanks
Laszlo

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

[Laszlo Ersek]

On 09/23/14 22:40, Junio C Hamano wrote:
> Laszlo Ersek  writes:
> 
>> On 09/23/14 21:56, Junio C Hamano wrote:
>>> Laszlo Ersek  writes:
>>>
 What do you think about accepting only "/dev/null\n" and "/dev/null\r\n"?
>>>
>>> I thought we agreed that what you are doing is not workable in the
>>> first place, no?
>>>
>>> I suspect one way to handle "In this project, the files that are
>>> checked out must be with CRLF line endings no matter what the
>>> platform is" might be to use the line ending attributes to force
>>> that while keeping the in-repository data with LF line endings.  The
>>> diff output (format-patch output is just one of them) comes from
>>> comparing the in-repository representation, so you won't have \r\n
>>> that will be stripped via MTA in it, "apply" and "am" will apply the
>>> patch without having to worry about \r\n, _and_ the line ending
>>> attributes would end the lines in your in-working-tree files with
>>> CRLF that way.
>>
>> This would be a perfect solution if the git repository was not a mirror
>> of a Subversion repository that contains all files with embedded CRLFs.
> 
> Yikes.
> 
>> Anyway I accept defeat, thanks for your time.
> 
> I do not consider that a "defeat".  It is just I do not think it is
> the right solution for the problem you are having to butcher "apply".
> You'd need to find some other way to solve it, and other people on
> the list may be able to offer a solution neither of us thought of in
> this thread.
> 
> Perhaps those who are on Windows have more experience in situations
> like yours?

I'm not on Windows, "obviously". :)

The overwhelming majority of the EDK II developers use windows, and
connect directly to subversion. They work with CRLF line endings "natively".

Jordan (CC'd) operates a robot that mirrors SVN commits to the git repo
on github, with "git svn". "git svn rebase --use-log-author" fetches the
new SVN commits to the robots local git clone, and then "git push" (as
usual) pushes them to github. This is being done so that people knowing
git don't lose their sanity, trying to work with SVN.

The process works very well, up to a point (git-loving people clone the
github mirror, and submit patches with git-format-patch / git send-email
to edk2-devel). The problem is when you want to apply patches with
git-am from the list -- the CRLFs are a mess. Hence my thread starter here.

We can get around this by maintaining personal forks on github, pushing
our patches there too in parallel with the email postings. People can
then fetch directly, and avoid git-am altogether. But this is very
cumbersome -- you need a github account, you need an edk2 fork on
github, others need to add your repo as a remote, etc etc, while the
review occurs anyway in our MUAs.

Thanks
Laszlo
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

[Junio C Hamano]

Laszlo Ersek  writes:

> In summary:

This is not entirely correct, though.  But it suggests an avenue for
a possible enhancement.

> - the email infrastructure turns all line terminators into CRLFs

Yes, but that is within MTAs and is expected to be invisible at MUA
level.  Typically mailbox files are stored in platform's native
format (e.g. I see LF endings in my mailbox), be it CRLF of LF.

The important thing to note here is that use of text/plain for
patches, if you want to have distinction between CRLF and LF in your
payload, is not designed to work over e-mails.

> - git-am strips these by default, from source code lines and from git
>   diff header lines alike,

On a platform whose native line endings are LF, the stripping may
not even happen (i.e. "am/apply" may not see CRLF in the mailbox in
the first place).

On a platform whose mbox has CRLF, or if you "unix2dos" a mbox on a
platform whose mbox has LF to manufacture a mbox with CRLF line
endings, you would have an opposite issue.  Information is lost in
MTAs while your e-mail is in transit and you cannot tell patch to
which paths had CRLF line endings and patch to which paths had LF
line endings.  If all files you are tracking uniformly use CRLF, you
can assume that everything had CRLF but it is still an assumption
and that is a special case that is not particularly interesting.

Applying such a CRLF patch with your "/dev/null\r\n" patch applied
to "am" will _add_ unwanted CR before LF to files that are meant to
use LF line endings.

Now if we accept that this issue is coming from lossy nature of
transporting patches via e-mails, we would realize that the right
place to solve this is not in "apply"'s parsing of structural part
of the "diff" output (e.g. "diff --git ...", "rename from ..." or
"--- filename"), but the payload part (i.e. " " followed by context,
"-" followed by removed and "+" followed by added).  Removal of CR
by "am -> mailsplit -> mailinfo -> apply" callchain is not losing
any information, as the input does not have useful information to
let us answer "are the lines in this path supposed to end with
CRLF?" in the first place; "/dev/null\r" patch is barking up a wrong
tree.

Our line-endings infrastructure (e.g. core.autocrlf configuration
variable, `eol` attribute) is all geared toward supporting cross
platform projects in that the BCP is to use LF line endings as the
canonical line endings for in-repository data and convert it to CRLF
for working tree files when necessary.  We do not have direct
support (other than declaring contents for paths as "binary" aka "no
conversion") to use CRLF in in-repository data (and that is partly
deliberate).

But it is conceivable to enhance the attribute system to allow us to
mark certain paths (or "all paths", which is a trivial special case)
as using CRLF line endings in in-repository and in-working-tree.  It
could be setting `eol` attribute to `both-crlf` or something.

Then "am -> mailsplit -> mailinfo -> apply" chain could:

 * "mailsplit" and "mailinfo" does not have to do anything special,
   other than stripping CR and make sure "apply" only sees LF
   endings;

 * "apply" is taught a new option "--fix-mta-corruption" which
   causes it to pay attention to the `eol` attribute set to
   `both-crlf`, and when it reads a patch

diff --git a/one b/one
--- one
+++ one
@@ -4,6 +4,6 @@
 common1
 common2
-old1
-old2
+new1
+new2
 common3
 common4

   and notices that path "one" is marked as such, it pretends as if
   the input were

diff --git a/one b/one
--- one
+++ one
@@ -4,6 +4,6 @@
 common1^M
 common2^M
-old1^M
-old2^M
+new1^M
+new2^M
 common3^M
 common4^M

   to compensate for possible breakage during the mail transit.

 * "am" is taught to pass "--fix-mta-corruption" to "apply" perhaps
   by default.

Because code paths that internally run "git am", e.g. "git rebase",
work on data that is *not* corrupt by mta (we generate diff and
apply ourselves), these places need to tell "am" not to use the
"--fix-mta-corruption" when running "apply".

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] git tag --contains: avoid stack overflow

[Andreas Schwab]

Jeff King  writes:

> But if we do want to keep it, does bumping it to 128 (and bumping the
> 4000 to 8000 in the test below it) work?

It works for all architectures supported by the openSUSE build service.

https://build.opensuse.org/package/show/home:AndreasSchwab:f/git

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Passing tar(1) options via git-archive(1)

[Thomas Braun]

Am 23.09.2014 um 20:57 schrieb Daniel Brockman:
> Some background from the git-archive(1) man page:
> 
> git-archive behaves differently when given a tree ID versus when
> given a commit ID or tag ID.  In the first case the current time is
> used as the modification time of each file in the archive.  In the
> latter case the commit time as recorded in the referenced commit
> object is used instead.
> 
> Would it make sense to add an --mtime option to git-archive(1) to enable
> explicitly setting the mtime for all files in the archive?  It could
> just pass through to the tar(1) --mtime option.
> 
> My use case is `git archive HEAD | docker build -`, in which the Docker
> cache is prevented from working because the mtime keeps getting bumped
> on all files.  I would like to have the mtime always be the same.
> 
> See, e.g., .
> 
> Otherwise, how about a generic way to pass options to tar(1)?

Actually I wanted to just hint to TAR_OPTIONS
as in
TAR_OPTIONS="--mtime 2014-09-23\ 00:00" git archive -o ausg.tar HEAD
but that does not work here (on windows).

The questions is should it be supported?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] git tag --contains: avoid stack overflow

[Junio C Hamano]

Andreas Schwab  writes:

> Jeff King  writes:
>
>> But if we do want to keep it, does bumping it to 128 (and bumping the
>> 4000 to 8000 in the test below it) work?
>
> It works for all architectures supported by the openSUSE build service.
>
> https://build.opensuse.org/package/show/home:AndreasSchwab:f/git
>
> Andreas.

So this on top of cbc60b67 (git tag --contains: avoid stack
overflow, 2014-04-24) and merge it to maint and upwards?

 t/t7004-tag.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index e4ab0f5..c564197 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1424,7 +1424,7 @@ EOF
 '
 
 run_with_limited_stack () {
-   (ulimit -s 64 && "$@")
+   (ulimit -s 128 && "$@")
 }
 
 test_lazy_prereq ULIMIT 'run_with_limited_stack true'
@@ -1433,7 +1433,7 @@ test_lazy_prereq ULIMIT 'run_with_limited_stack true'
 test_expect_success ULIMIT '--contains works in a deep repo' '
>expect &&
i=1 &&
-   while test $i -lt 4000
+   while test $i -lt 8000
do
echo "commit refs/heads/master
 committer A U Thor  $((10 + $i * 100)) +0200
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Passing tar(1) options via git-archive(1)

[brian m. carlson]

On Tue, Sep 23, 2014 at 11:49:24PM +0200, Thomas Braun wrote:
> Am 23.09.2014 um 20:57 schrieb Daniel Brockman:
> > Would it make sense to add an --mtime option to git-archive(1) to enable
> > explicitly setting the mtime for all files in the archive?  It could
> > just pass through to the tar(1) --mtime option.
> > 
> > My use case is `git archive HEAD | docker build -`, in which the Docker
> > cache is prevented from working because the mtime keeps getting bumped
> > on all files.  I would like to have the mtime always be the same.
> > 
> > See, e.g., .
> > 
> > Otherwise, how about a generic way to pass options to tar(1)?
> 
> Actually I wanted to just hint to TAR_OPTIONS
> as in
> TAR_OPTIONS="--mtime 2014-09-23\ 00:00" git archive -o ausg.tar HEAD
> but that does not work here (on windows).

Git does not invoke tar(1).  It has its own tar (actually, pax)
implementation, so any options would have to be implemented in Git.
We'd probably want to make such a change effective in the zip format as
well.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature

Re: Passing tar(1) options via git-archive(1)

[Daniel Brockman]

"brian m. carlson"  writes:

> Git does not invoke tar(1).  It has its own tar (actually, pax)
> implementation, so any options would have to be implemented in Git.
> We'd probably want to make such a change effective in the zip format
> as well.

Ah, I see...

Well, I guess in the meantime I'll just do this:

   git commit -m dummy --allow-empty --date=1970-01-01T00:00:00Z
   git archive HEAD | docker build -
   git reset HEAD~

Or even (slightly more atomically) this:

   git archive HEAD | { tmpdir=`mktemp -d` && tar x -C"$tmpdir" &&
 tar c -C"$tmpdir" --mtime=1970-01-01T00:00:00Z . &&
 rm -r "$tmpdir"; } | docker build -
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Passing tar(1) options via git-archive(1)

[Jeff King]

On Wed, Sep 24, 2014 at 03:26:22AM +0200, Daniel Brockman wrote:

> "brian m. carlson"  writes:
> 
> > Git does not invoke tar(1).  It has its own tar (actually, pax)
> > implementation, so any options would have to be implemented in Git.
> > We'd probably want to make such a change effective in the zip format
> > as well.
> 
> Ah, I see...
> 
> Well, I guess in the meantime I'll just do this:
> 
>git commit -m dummy --allow-empty --date=1970-01-01T00:00:00Z
>git archive HEAD | docker build -
>git reset HEAD~

I don't think that will work. The `--date` parameter sets the author
date, but archive uses the committer date. You'd have to set
GIT_COMMITTER_DATE in the environment to override that.

Also, you can avoid writing to the HEAD ref entirely by using the
commit-tree plumbing. Like:

  commit=$(
GIT_COMMITTER_DATE=1970-01-01T00:00:00Z \
git commit-tree HEAD^{tree} tree = tree;
ar_args->commit_sha1 = commit_sha1;
ar_args->commit = commit;
-   ar_args->time = archive_time;
+   if (!ar_args->time)
+   ar_args->time = archive_time;
 }
 
 #define OPT__COMPR(s, v, h, p) \
@@ -323,6 +324,7 @@ static int parse_archive_args(int argc, const char **argv,
int i;
int list = 0;
int worktree_attributes = 0;
+   unsigned long mtime = 0;
struct option opts[] = {
OPT_GROUP(""),
OPT_STRING(0, "format", &format, N_("fmt"), N_("archive 
format")),
@@ -332,6 +334,7 @@ static int parse_archive_args(int argc, const char **argv,
N_("write the archive to this file")),
OPT_BOOL(0, "worktree-attributes", &worktree_attributes,
N_("read .gitattributes in working directory")),
+   OPT_DATE(0, "mtime", &mtime, N_("mtime of files in archive")),
OPT__VERBOSE(&verbose, N_("report archived files on stderr")),
OPT__COMPR('0', &compression_level, N_("store only"), 0),
OPT__COMPR('1', &compression_level, N_("compress faster"), 1),
@@ -398,6 +401,7 @@ static int parse_archive_args(int argc, const char **argv,
args->base = base;
args->baselen = strlen(base);
args->worktree_attributes = worktree_attributes;
+   args->time = mtime;
 
return argc;
 }

which allows:

  git archive --mtime='yesterday at 3pm' HEAD

For inclusion in git, it would need someone to wrap it up with a commit
message, and add a basic test (see the existing mtime test in
t/t5000-tar-tree) and documentation in Documentation/git-archive.txt.
That someone could be you. :)

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html