Re: [PATCH v15 07/13] ref-filter: add support for %(contents:lines=X)

[Matthieu Moy]

Karthik Nayak  writes:

> On Thu, Sep 3, 2015 at 7:04 PM, Karthik Nayak  wrote:
>>> struct strbuf s = STRBUF_INIT;
>>> if (strtoul_ui(valp, 10, >u.contents.lines))
>>> die(_("positive width expected 
>>> contents:lines=%s"), valp);
>>> -   append_lines(, subpos, sublen + bodylen - siglen, 
>>> v->u.contents.lines);
>>> +   append_lines(, subpos, bodypos + bodylen - 
>>> subpos, v->u.contents.lines);
>>> v->s = strbuf_detach(, NULL);
>>> }
>>> }
>
> append_lines(, subpos, bodylen + bodypos - subpos - siglen,
> v->u.contents.lines);
>
> We need to eliminate the signature if existing also.

Indeed. I thought body did not include the signature.

I'd write it as

  bodylen + bodypos - siglen - subpos

or even

  char *contents_end = bodylen + bodypos - siglen;
  ...
  append_lines(, subpos, contents_end - subpos, ...);

to make it self-explanatory.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 v15 07/13] ref-filter: add support for %(contents:lines=X)

[Eric Sunshine]

On Thu, Sep 3, 2015 at 10:39 AM, Eric Sunshine  wrote:
> On Tue, Sep 1, 2015 at 2:26 PM, Karthik Nayak  wrote:
>> +static void append_lines(struct strbuf *out, const char *buf, unsigned long 
>> size, int lines)
>> +{
>> +   if ((sp = strstr(buf, "\n\n")) && (sp <= buf + size))
>> +   size += 2;
>
> Aside from the +2 which Matthieu already questioned, this code has a
> much more serious problem. strstr() assumes that 'buf' is
> NUL-terminated, however, the fact that buf's size is also being passed
> to the function, implies that it may not be NUL-terminated.
> Consequently, strstr() could zip well past the end of 'buf', trying to
> consult memory not part of 'buf', which is a violation of the C
> standard. Even with the band-aid (sp <= buf + size) which tries to
> detect this situation, it's still a violation, and could crash if
> strstr() attempts to consult memory which hasn't even been allocated
> to the process or is otherwise protected in some fashion.
>
> It's possible that 'buf' may actually always be NUL-terminated, but
> this code does not convey that fact. If it is known to be
> NUL-terminated, then it is safe to use strstr(), however, that should
> be shown more clearly either by revising the code or adding an
> appropriate comment.

Also, if 'buf' is indeed unconditionally NUL-terminated, then the (sp
<= buf + size) check is wasted code since the result of strstr() will
always be either NULL or pointing somewhere within the NUL-terminated
string.
--
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 v1] t9821: use test_config

[Lars Schneider]

On 03 Sep 2015, at 15:04, Eric Sunshine  wrote:

> On Thu, Sep 3, 2015 at 5:34 AM,   wrote:
>> From: Lars Schneider 
>> 
>> Signed-off-by: Lars Schneider 
>> ---
>> diff --git a/t/t9821-git-p4-path-variations.sh 
>> b/t/t9821-git-p4-path-variations.sh
>> index 81e46ac..5a26fec 100755
>> --- a/t/t9821-git-p4-path-variations.sh
>> +++ b/t/t9821-git-p4-path-variations.sh
>> @@ -45,7 +45,7 @@ test_expect_success 'Clone root' '
>>(
>>cd "$git" &&
>>git init . &&
>> -   git config core.ignorecase false &&
>> +   test_config core.ignorecase false &&
> 
> test_config ensures that the config setting gets "unset" at the end of
> the test, whether the test succeeds or not, so that subsequent tests
> are not affected by the setting. However, in this case, since the $git
> repository gets recreated from scratch for each test anyhow, use of
> test_config is superfluous. In fact, it may be slightly
> contraindicated since it could mislead the reader into thinking that
> state is carried over from test to test. (Not a big objections, but
> something to take into consideration.)
OK. Do I need to do anything to take the PATCH suggestion back?
I thanks for the explanation!

Cheers,
Lars


--
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 v15 07/13] ref-filter: add support for %(contents:lines=X)

[Matthieu Moy]

Eric Sunshine  writes:

> Also, if 'buf' is indeed unconditionally NUL-terminated, then the (sp
> <= buf + size) check is wasted code since the result of strstr() will
> always be either NULL or pointing somewhere within the NUL-terminated
> string.

The null character is there, but after the signature. buf + size points
before the signature.

Anyway, all this should go away if Karthik applies my suggestion, which
I like even more after this discussion ;-).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] git-p4: add config git-p4.pathEncoding

[Lars Schneider]

On 03 Sep 2015, at 10:26, Remi Galan Alfonso 
 wrote:

> Hi Lars,
> 
> Lars Schneider  writes:
> 
>> +test_expect_success 'Clone repo containing iso8859-1 encoded paths with 
>> git-p4.pathEncoding' '
>> +
>> +test_when_finished cleanup_git &&
>> +(
>> +cd "$git" &&
>> +git init . &&
>> +git config git-p4.pathEncoding iso8859-1 &&
> 
> Wouldn't 'test_config' be better here?
> 
If I got Eric’s explanation right in "[PATCH v1] t9821: use test_config” then 
we should not use “test_config” here?

Thanks,
Lars--
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 v15 07/13] ref-filter: add support for %(contents:lines=X)

[Eric Sunshine]

On Tue, Sep 1, 2015 at 2:26 PM, Karthik Nayak  wrote:
> In 'tag.c' we can print N lines from the annotation of the tag using
> the '-n' option. Copy code from 'tag.c' to 'ref-filter' and
> modify it to support appending of N lines from the annotation of tags
> to the given strbuf.
>
> Implement %(contents:lines=X) where X lines of the given object are
> obtained.
>
> Add documentation and test for the same.
>
> Signed-off-by: Karthik Nayak 
> ---
>  struct atom_value {
> const char *s;
> -   struct align align;
> +   union {
> +   struct align align;
> +   struct contents contents;
> +   } u;
> void (*handler)(struct atom_value *atomv, struct ref_formatting_state 
> *state);
> unsigned long ul; /* used for sorting when not FIELD_STR */
>  };
> @@ -226,7 +235,7 @@ static void align_atom_handler(struct atom_value *atomv, 
> struct ref_formatting_s
> push_stack_element(>stack);
> new = state->stack;
> new->at_end = align_handler;
> -   new->cb_data = >align;
> +   new->cb_data = >u.align;

You could declare atom_value with the union from the start, even if it
has only a single member ('align', at first). Doing so would make this
patch less noisy and wouldn't distract the reader with these seemingly
unrelated changes.

>  }
>
>  static void end_atom_handler(struct atom_value *atomv, struct 
> ref_formatting_state *state)
> @@ -624,6 +633,33 @@ static void find_subpos(const char *buf, unsigned long 
> sz,
> *nonsiglen = *sig - buf;
>  }
>
> +/*
> + * If 'lines' is greater than 0, append that many lines from the given
> + * 'buf' of length 'size' to the given strbuf.
> + */
> +static void append_lines(struct strbuf *out, const char *buf, unsigned long 
> size, int lines)
> +{
> +   int i;
> +   const char *sp, *eol;
> +   size_t len;
> +
> +   if ((sp = strstr(buf, "\n\n")) && (sp <= buf + size))
> +   size += 2;

Aside from the +2 which Matthieu already questioned, this code has a
much more serious problem. strstr() assumes that 'buf' is
NUL-terminated, however, the fact that buf's size is also being passed
to the function, implies that it may not be NUL-terminated.
Consequently, strstr() could zip well past the end of 'buf', trying to
consult memory not part of 'buf', which is a violation of the C
standard. Even with the band-aid (sp <= buf + size) which tries to
detect this situation, it's still a violation, and could crash if
strstr() attempts to consult memory which hasn't even been allocated
to the process or is otherwise protected in some fashion.

It's possible that 'buf' may actually always be NUL-terminated, but
this code does not convey that fact. If it is known to be
NUL-terminated, then it is safe to use strstr(), however, that should
be shown more clearly either by revising the code or adding an
appropriate comment.

> +   sp = buf;
> +
> +   for (i = 0; i < lines && sp < buf + size; i++) {
> +   if (i)
> +   strbuf_addstr(out, "\n");
> +   eol = memchr(sp, '\n', size - (sp - buf));
> +   len = eol ? eol - sp : size - (sp - buf);
> +   strbuf_add(out, sp, len);
> +   if (!eol)
> +   break;
> +   sp = eol + 1;
> +   }
> +}
> @@ -663,6 +701,13 @@ static void grab_sub_body_contents(struct atom_value 
> *val, int deref, struct obj
> v->s = xmemdupz(sigpos, siglen);
> else if (!strcmp(name, "contents"))
> v->s = xstrdup(subpos);
> +   else if (skip_prefix(name, "contents:lines=", )) {
> +   struct strbuf s = STRBUF_INIT;
> +   if (strtoul_ui(valp, 10, >u.contents.lines))
> +   die(_("positive width expected 
> contents:lines=%s"), valp);

This error message is still too tightly coupled to %(align:), from
which it was copied. "width"?

> +   append_lines(, subpos, sublen + bodylen - siglen, 
> v->u.contents.lines);
> +   v->s = strbuf_detach(, NULL);
> +   }
> }
>  }
--
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 v15 07/13] ref-filter: add support for %(contents:lines=X)

[Matthieu Moy]

Eric Sunshine  writes:

>> @@ -624,6 +633,33 @@ static void find_subpos(const char *buf, unsigned long 
>> sz,
>> *nonsiglen = *sig - buf;
>>  }
>>
>> +/*
>> + * If 'lines' is greater than 0, append that many lines from the given
>> + * 'buf' of length 'size' to the given strbuf.
>> + */
>> +static void append_lines(struct strbuf *out, const char *buf, unsigned long 
>> size, int lines)
>> +{
>> +   int i;
>> +   const char *sp, *eol;
>> +   size_t len;
>> +
>> +   if ((sp = strstr(buf, "\n\n")) && (sp <= buf + size))
>> +   size += 2;
>
> Aside from the +2 which Matthieu already questioned, this code has a
> much more serious problem. strstr() assumes that 'buf' is
> NUL-terminated, however, the fact that buf's size is also being passed
> to the function, implies that it may not be NUL-terminated.

If Karthik applies my suggestion, then the strstr would go away. I think
the code would be correct even on non-null-terminated strings.

Actually, we're already making the assumption that the buffer for the
whole tag object is null-terminated (and contains no '\0') for
%(contents):

else if (!strcmp(name, "contents"))
v->s = xstrdup(subpos);

(But I agree that even if the assumption is correct, it should be made
explicit if it remains a precondition of append_lines).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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

Default ordering of git log output

[Kirill Likhodedov]

Is it intended behavior that the default git log output (without ordering 
parameters) can show parents before children? 

The man says:
Commit Ordering
   By default, the commits are shown in reverse chronological order.
so it tells nothing about parent-to-child relationship.

I‘ve just faced an example in an open source repository, where I reproduced 
this behavior: a parent is shown before its child. (so if this behavior is not 
intended I can give the project URL for debugging)

So could you please clarify:
1. Is it true that the rule "no parents before all of its children are shown” 
is not guaranteed for default ordering?

2. What does "reverse chronological order” mean? How commits are actually 
sorted in git log? 

// I was always under impression that git log lists commit in the order in 
which they are “recorded" to the repository (e.g. pulled), since this seems to 
be both the fastest way, and also pretty acceptable from the ordering point of 
view.  

Thanks a lot!
-- Kirill.

--
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 v15 07/13] ref-filter: add support for %(contents:lines=X)

[Karthik Nayak]

On Thu, Sep 3, 2015 at 7:04 PM, Karthik Nayak  wrote:
>> struct strbuf s = STRBUF_INIT;
>> if (strtoul_ui(valp, 10, >u.contents.lines))
>> die(_("positive width expected 
>> contents:lines=%s"), valp);
>> -   append_lines(, subpos, sublen + bodylen - siglen, 
>> v->u.contents.lines);
>> +   append_lines(, subpos, bodypos + bodylen - subpos, 
>> v->u.contents.lines);
>> v->s = strbuf_detach(, NULL);
>> }
>> }

append_lines(, subpos, bodylen + bodypos - subpos - siglen,
v->u.contents.lines);

We need to eliminate the signature if existing also.

-- 
Regards,
Karthik Nayak
--
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 v15 05/13] ref-filter: implement an `align` atom

[Eric Sunshine]

On Tue, Sep 1, 2015 at 2:26 PM, Karthik Nayak  wrote:
> Implement an `align` atom which left-, middle-, or right-aligns the
> content between %(align:..) and %(end).

Spell this either %(align:) or %(align:...) with three dots, not two.
I, personally, think %(align:) is sufficient.

> It is followed by `:,`, where the `` is
> either left, right or middle and `` is the size of the area
> into which the content will be placed. If the content between
> %(align:) and %(end) is more than the width then no alignment is
> performed. e.g. to align a refname atom to the middle with a total
> width of 40 we can do: --format="%(align:middle,40)%(refname)%(end)".
>
> We have an `at_end` function for each element of the stack which is to
> be called when the `end` atom is encountered. Using this we implement
> the aling_handler() for the `align` atom, this aligns the final strbuf
> by calling `strbuf_utf8_align()` from utf8.c.
>
> Ensure that quote formatting is performed on the whole of
> %(align)...%(end) rather than individual atoms inside.  We skip quote

Add colon: %(align:)

> formatting for individual atoms when the current stack element is
> handling an %(align) atom and perform quote formatting at the end when

%(align:)

> we encounter the %(end) atom.
>
> Add documentation and tests for the same.
>
> Signed-off-by: Karthik Nayak 
> ---
> diff --git a/Documentation/git-for-each-ref.txt 
> b/Documentation/git-for-each-ref.txt
> index e49d578..cac3128 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -127,6 +127,15 @@ color::
> Change output color.  Followed by `:`, where names
> are described in `color.branch.*`.
>
> +align::
> +   Left-, middle-, or right-align the content between %(align:..)

%(align:)

> +   and %(end). Followed by `:,`, where the
> +   `` is either left, right or middle and `` is
> +   the total length of the content with alignment. If the
> +   contents length is more than the width then no alignment is
> +   performed. If used with '--quote' everything in between
> +   %(align:..)  and %(end) is quoted.

%(align:)
Also drop the extra space before "and": s/\s+and/ and/

>  In addition to the above, for commit and tag objects, the header
>  field names (`tree`, `parent`, `object`, `type`, and `tag`) can
>  be used to specify the value in the header field.
--
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 v15 05/13] ref-filter: implement an `align` atom

[Junio C Hamano]

Eric Sunshine  writes:

> On Tue, Sep 1, 2015 at 2:26 PM, Karthik Nayak  wrote:
>> Implement an `align` atom which left-, middle-, or right-aligns the
>> content between %(align:..) and %(end).
>
> Spell this either %(align:) or %(align:...) with three dots, not two.
> I, personally, think %(align:) is sufficient.

I agree with you that double-dot does not signal "some things are
ellided here" to a usual reader.

I actually think consistent use of %(align:...) is needed, simply
because my knee-jerk reaction to "%(align:)" was "huh?  where does
the need for the trailing colon come from?", not "ah, you try to
imply that there must be something more by having just a colon
there".

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

Feature Request: enhance Git-GUI's Checkout Branch screen

[John Medema]

Git gurus, throw this one on your to-do list:

This is a feature request to enhance the Git GUI to make it easier to
checkout non-existing branches that exist upstream. Apologies if this
is not the correct place for feature requests.

Scenario: Upstream repo has 4 branches - master, develop, maint, test.
Local repo has only a master branch. In the command line, to switch to
a local copy of the test branch, it is a simple "git checkout test".
The git command automatically realizes your requested branch doesn't
exist but origin does have a branch named test, so it a) creates a
local branch off of origin/develop, b) sets the appropriate pull link,
and c) sets the appropriate push link. In effect, the git command line
hides the fact that the user doesn't know the branch doesn't exist and
creates it as the user was expecting it to exist as. The Git GUI has
no shortcut like this.

For reference, from the man page for git-checkout:
"If  is not found but there does exist a tracking branch in
exactly one remote (call it ) with a matching name, treat as
equivalent to "git checkout -b  --track /".

Currently, in order to checkout a non-existing branch in the GUI you
must go to the Branch Menu, click Create, select the "Tracking Branch"
radio, select the branch, then go back up and name the branch the
exact same name (to ease new user confusion). For a new user who just
wants a copy of the remote branch, it is very unintuitive to create a
new branch.

Fortunately, you already have some explicit warning messages after the
Checkout Branch screen if the user intuitively tries to checkout the
tracking branch, but even then a new user rarely realizes what they
have gotten themselves into. At best, they know that they must find
help (just for trying to checkout a branch).

In order to implement this feature effectively, I suggest that the
Checkout Branch screen be modified in one of two ways (exclusive):

Option A:
Merge the Local and Tracking Branch lists into one box keeping their
entries separate by their full name ("master" and "origin/master"). If
a user selects a remote branch, ask the user whether to create the
local branch or move to the detached HEAD state (current
functionality).

Option B (preferred):
Keep the Local and Tracking Branch lists separate (as they are now),
and keep the Tracking Branch list as-is. However, on the Local Branch
screen, select include the existing branches in normal font but also
include potential local branches based off of the remote in italics
(or greyed-out, or asterisked, etc). Selecting an italicized entry
creates the new branch from the tracking branch, without user
interaction.


Thanks,

John Medema
Systems Administrator
United Drugs, a Subsidiary of AAP (American Associated Pharmacies)
john.med...@uniteddrugs.com
7243 N 16th Street, Phoenix, AZ 85020
Office:  602-678-1179 x126
Fax:  602-639-4631

-- 
HIPAA NOTICE:  It is against United Drugs’ policy to receive or send 
un-encrypted or non-secured email correspondence containing Protected 
Health Information (PHI) as defined by HIPAA law.
 
Please use fax or phone for correspondence containing PHI.

-- 
This email message is for the sole use of the intended recipient(s) and may 
contain confidential and privileged information. Any unauthorized review, 
use, disclosure or distribution is prohibited. If you are not the intended 
recipient, contact the sender by reply email, and destroy all copies of the 
original message. 
--
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 v6] git-p4: add config git-p4.pathEncoding

[Lars Schneider]

On 03 Sep 2015, at 19:03, Junio C Hamano  wrote:

> larsxschnei...@gmail.com writes:
> 
>> +test_expect_success 'Clone repo containing iso8859-1 encoded paths without 
>> git-p4.pathEncoding' '
>> +git p4 clone --destination="$git" //depot &&
>> +test_when_finished cleanup_git &&
>> +(
>> +cd "$git" &&
>> +UTF8="$(printf "$UTF8_ESCAPED")" &&
>> +echo $UTF8 >expect &&
>> +git -c core.quotepath=false ls-files >actual &&
>> +test_must_fail test_cmp expect actual
> 
> I am not sure what this test wants to do.  It is not inconceivable
> that future versions of "git p4 clone" becomes more intelligent to
> detect the need for git-p4.pathEncoding and set it, so the only
> effect to insist the comparison fails is to block future advance in
> that direction.
> 
> Besides, "test_must_fail test_cmp" looks like a strange thing to
> say.  "! test_cmp expect actual" perhaps.
> 
> Even better, expect that "expect" and "actual" becomes the same, but
> mark the test itself to expect failure, to say "it ought to work
> this way in the ideal world, but we know the system currently does
> not pass this test".
> 
> I'm tempted to suggest squashing the following in.  Thoughts?
OK. The diff looks good to me. For some reason I can’t apply the patch though. 
git patch gives me "fatal: corrupt patch at line 10”. Any idea? (I might do 
something stupid because I am not used to patches…)

Thanks,
Lars

> 
> 
> t/t9822-git-p4-path-encoding.sh | 17 -
> 1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
> index e507ad7..2d652d89 100755
> --- a/t/t9822-git-p4-path-encoding.sh
> +++ b/t/t9822-git-p4-path-encoding.sh
> @@ -21,15 +21,15 @@ test_expect_success 'Create a repo containing iso8859-1 
> encoded paths' '
>   )
> '
> 
> -test_expect_success 'Clone repo containing iso8859-1 encoded paths without 
> git-p4.pathEncoding' '
> +test_expect_failure 'Clone auto-detects depot with iso8859-1 paths' '
>   git p4 clone --destination="$git" //depot &&
>   test_when_finished cleanup_git &&
>   (
>   cd "$git" &&
>   UTF8="$(printf "$UTF8_ESCAPED")" &&
> - echo $UTF8 >expect &&
> + echo "$UTF8" >expect &&
>   git -c core.quotepath=false ls-files >actual &&
> - test_must_fail test_cmp expect actual
> + test_cmp expect actual
>   )
> '
> 
> @@ -39,16 +39,15 @@ test_expect_success 'Clone repo containing iso8859-1 
> encoded paths with git-p4.p
>   (
>   cd "$git" &&
>   git init . &&
> - test_config git-p4.pathEncoding iso8859-1 &&
> + git config git-p4.pathEncoding iso8859-1 &&
>   git p4 clone --use-client-spec --destination="$git" //depot &&
>   UTF8="$(printf "$UTF8_ESCAPED")" &&
> - echo $UTF8 >expect &&
> + echo "$UTF8" >expect &&
>   git -c core.quotepath=false ls-files >actual &&
>   test_cmp expect actual &&
> - cat >expect <<-\EOF &&
> - content123
> - EOF
> - cat $UTF8 >actual &&
> +
> + echo content123 >expect &&
> + cat "$UTF8" >actual &&
>   test_cmp expect actual
>   )
> '

--
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: git dockerfile.

[Junio C Hamano]

Atul Sowani  writes:

> Hello!
>
> Any help/pointers/advise regarding my request about dockerfile?

[jc: "Gawade P" Cc'ed.]

Perhaps you can learn to ask the list archive ;-)

Going to http://news.gmane.org/gmane.comp.version-control.git/
and then asking for "dockerfile" in the search box at the bottom
finds

http://article.gmane.org/gmane.comp.version-control.git/276504/match=dockerfile

Clicking on the "Subject:" line there would show the article in
context.

--
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 2/2] date: make "local" orthogonal to date format

[Junio C Hamano]

John Keeping  writes:

> On Wed, Sep 02, 2015 at 05:30:14PM -0400, Jeff King wrote:
>> I think the error message would be a lot nicer if we indicate that "-"
>> is syntactically interesting, and say:
>> 
>>   fatal: unknown date-mode modifier: locale
>
> I wonder if we'd be better just saying:
>
>   fatal: unknown date format: short-locale
>
> I'm not sure users will consider "local" to be a modifier, there is
> simply a list of formats that happens to include pairs of matching
> "-local" and "not -local" variants.

Either is acceptable and better than the 'e' output.  I think yours
is better because it would equally work well for those who think in
terms of "modifier" and those who think in terms of "flat list of
possibilities".

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

Re: glibc mutex deadlock in signal handler

[Junio C Hamano]

Takashi Iwai  writes:

> we've got a bug report of git-log stall in below:
>   https://bugzilla.opensuse.org/show_bug.cgi?id=942297
>
> In short, git-log is left unterminated sometimes when pager is
> aborted.  When this happens, git process can't be killed by SIGTERM,
> but only by SIGKILL.  And, further investigation revealed the possible
> mutex deadlock used in glibc *alloc()/free().
>
> I tried to reproduce this by adding a fault injection in glibc code,
> and actually got the similar stack trace as reported.  The problem is
> that glibc malloc (in this case realloc() and free()) takes a private
> mutex.  Thus calling any function does *alloc() or free() in a signal
> handler may deadlock.  In the case of git, it was free() calls in
> various cleanup codes and the printf() / strerror() invocations.
>
> Also, basically it's not safe to call fflush() or raise(), either.
> But they seem to work practically on the current systems.
>
> Below is a band-aid patch I tested and confirmed to work in the
> fault-injection case.  But, some unsafe calls mentioned in the above
> are left.  If we want to be in a safer side, we should really limit
> the things to do in a signal handler, e.g. only calling close() and
> doing waitpid(), I suppose.
>
> Any better ideas?

Sorry, but I am not with better ideas (at least offhand right now).

Essentially the idea seems to be to avoid calling allocation-related
functions in the signal handler codepath that is used for cleaning
things up.  Not calling free() in the codepaths is perfectly fine as
we know we will be soon exiting anyway.  Not being able to call
error() and strerror() to report funnies (other than the fact that
we were interrupted) is somewhat sad, though.




> diff --git a/pager.c b/pager.c
> index 27d4c8a17aa1..57dda0142fa9 100644
> --- a/pager.c
> +++ b/pager.c
> @@ -14,19 +14,25 @@
>  static const char *pager_argv[] = { NULL, NULL };
>  static struct child_process pager_process = CHILD_PROCESS_INIT;
>  
> -static void wait_for_pager(void)
> +static void flush_pager(void)
>  {
>   fflush(stdout);
>   fflush(stderr);
>   /* signal EOF to pager */
>   close(1);
>   close(2);
> +}
> +
> +static void wait_for_pager(void)
> +{
> + flush_pager();
>   finish_command(_process);
>  }
>  
>  static void wait_for_pager_signal(int signo)
>  {
> - wait_for_pager();
> + flush_pager();
> + finish_command_in_signal(_process);
>   sigchain_pop(signo);
>   raise(signo);
>  }
> diff --git a/run-command.c b/run-command.c
> index 3277cf797ed4..a8cdc8f32944 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -18,26 +18,27 @@ struct child_to_clean {
>  static struct child_to_clean *children_to_clean;
>  static int installed_child_cleanup_handler;
>  
> -static void cleanup_children(int sig)
> +static void cleanup_children(int sig, int in_signal)
>  {
>   while (children_to_clean) {
>   struct child_to_clean *p = children_to_clean;
>   children_to_clean = p->next;
>   kill(p->pid, sig);
> - free(p);
> + if (!in_signal)
> + free(p);
>   }
>  }
>  
>  static void cleanup_children_on_signal(int sig)
>  {
> - cleanup_children(sig);
> + cleanup_children(sig, 1);
>   sigchain_pop(sig);
>   raise(sig);
>  }
>  
>  static void cleanup_children_on_exit(void)
>  {
> - cleanup_children(SIGTERM);
> + cleanup_children(SIGTERM, 0);
>  }
>  
>  static void mark_child_for_cleanup(pid_t pid)
> @@ -220,7 +221,7 @@ static inline void set_cloexec(int fd)
>   fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
>  }
>  
> -static int wait_or_whine(pid_t pid, const char *argv0)
> +static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
>  {
>   int status, code = -1;
>   pid_t waiting;
> @@ -231,13 +232,17 @@ static int wait_or_whine(pid_t pid, const char *argv0)
>  
>   if (waiting < 0) {
>   failed_errno = errno;
> - error("waitpid for %s failed: %s", argv0, strerror(errno));
> + if (!in_signal)
> + error("waitpid for %s failed: %s", argv0,
> +   strerror(errno));
>   } else if (waiting != pid) {
> - error("waitpid is confused (%s)", argv0);
> + if (!in_signal)
> + error("waitpid is confused (%s)", argv0);
>   } else if (WIFSIGNALED(status)) {
>   code = WTERMSIG(status);
>   if (code != SIGINT && code != SIGQUIT)
> - error("%s died of signal %d", argv0, code);
> + if (!in_signal)
> + error("%s died of signal %d", argv0, code);
>   /*
>* This return value is chosen so that code & 0xff
>* mimics the exit code that a POSIX shell would report for
> @@ -254,10 +259,12 @@ static int wait_or_whine(pid_t pid, const char *argv0)
> 

Re: [PATCH v6] git-p4: add config git-p4.pathEncoding

[Junio C Hamano]

Lars Schneider  writes:

> On 03 Sep 2015, at 19:03, Junio C Hamano  wrote:
>
>> I'm tempted to suggest squashing the following in.  Thoughts?
>
> OK. The diff looks good to me. For some reason I can’t apply the
> patch though. git patch gives me "fatal: corrupt patch at line
> 10”. Any idea? (I might do something stupid because I am not used to
> patches…)

Sorry, but no idea.  I just saved the message I see on the list to a
file and did "git am" myself and didn't find anything.

Line 9 of the patch is a removal of your overlong test_expect_success
line, so if that were somehow line-wrapped by something between the
list and your "git apply" (e.g. with your MUA), it is very probable
that "git apply" would see there is something funny on line 10, but
line 5 (a hunk header "@@ -21,15 ...") is also rather long, 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: [PATCHv5 1/3] submodule: Reimplement `module_list` shell function in C

[Junio C Hamano]

Stefan Beller  writes:

> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> new file mode 100644
> index 000..e97403e
> --- /dev/null
> +++ b/builtin/submodule--helper.c
> @@ -0,0 +1,124 @@
> +#include "builtin.h"
> +#include "cache.h"
> +#include "parse-options.h"
> +#include "quote.h"
> +#include "pathspec.h"
> +#include "dir.h"
> +#include "utf8.h"
> +
> +static const struct cache_entry **ce_entries;
> +static int ce_alloc, ce_used;

It is customary to use X_alloc, X_nr for an array X_something that
is managed by ALLOC_GROW(), I think.  I'd also suggest wrapping
these in a struct and passing it between module_list_compute() and
its callers.

> + for (i = 0; i < active_nr; i++) {
> + const struct cache_entry *ce = active_cache[i];
> +
> + if (!S_ISGITLINK(ce->ce_mode) ||
> + !match_pathspec(pathspec, ce->name, ce_namelen(ce),
> + max_prefix_len, ps_matched,
> + S_ISGITLINK(ce->ce_mode) | 
> S_ISDIR(ce->ce_mode)))
> + continue;

I may have said this already, but unlike tree entries, the index
entries will never be a directory.  S_ISDIR() check here is
meaningless [*1*].

More importantly when you make this call, you already know that
S_ISGITLINK(ce->ce_mode) is true.  Otherwise you wouldn't be calling
match_pathspec() in the first place.

So this should be:

if (!S_ISGITLINK(ce->ce_mode) ||
!match_pathspec(pathspec, ce->name, ce_namelen(ce),
max_prefix_len, ps_matched, 1))

I think.

I'll attach a suggested changes on top to be squashed in.

> +struct cmd_struct {
> + const char *cmd;
> + int (*fn)(int, const char **, const char *);
> +};
> +
> +static struct cmd_struct commands[] = {
> + {"list", module_list},
> +};
> +
> +int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
> +{
> + int i;
> + if (argc < 2)
> + die(_("fatal: submodule--helper subcommand must be "
> +   "called with a subcommand"));
> +
> + for (i = 0; i < ARRAY_SIZE(commands); i++)
> + if (!strcmp(argv[1], commands[i].cmd))
> + return commands[i].fn(argc - 1, argv + 1, prefix);
> +
> + die(_("fatal: '%s' is not a valid submodule--helper "
> +   "subcommand"), argv[1]);
> +}

Nice and clean code structure.  I like it ;-).

[Footnote]

*1* It is a benign bug, as a ce that is S_ISGITLINK() always
satisfies S_ISDIR() by chance.  But it is a bug nevertheless.

builtin/ls-files.c and dir.h, the former of which you copied
this code from, may want be corrected.  These mistakes were
introduced by ae8d0824 (pathspec: pass directory indicator to
match_pathspec_item(), 2014-01-24).  Further, I suspect these
mistakes were copied from ce_to_dtype() in cache.h, whose mistake
was introduced by d6b8fc30 (gitignore(5): Allow "foo/" in ignore
list to match directory "foo", 2008-01-31).

That commit mistakenly copied S_ISDIR() check from create_ce_mode(),
but create_ce_mode() is fed st.st_mode and checking for S_ISDIR() is
a correct thing to use there.

In short, there are three hits from "git grep 'S_ISDIR(ce'" that
checks if ce->ce_mode is S_ISDIR() || S_ISGITLINK(); they all should
check only for S_ISGITLINK().


 builtin/submodule--helper.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e97403e..10db4e6 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -6,12 +6,16 @@
 #include "dir.h"
 #include "utf8.h"
 
-static const struct cache_entry **ce_entries;
-static int ce_alloc, ce_used;
+struct module_list {
+   const struct cache_entry **entries;
+   int alloc, nr;
+};
+#define MODULE_LIST_INIT { NULL, 0, 0 }
 
 static int module_list_compute(int argc, const char **argv,
-   const char *prefix,
-   struct pathspec *pathspec)
+  const char *prefix,
+  struct pathspec *pathspec,
+  struct module_list *list)
 {
int i, result = 0;
char *max_prefix, *ps_matched = NULL;
@@ -36,12 +40,11 @@ static int module_list_compute(int argc, const char **argv,
 
if (!S_ISGITLINK(ce->ce_mode) ||
!match_pathspec(pathspec, ce->name, ce_namelen(ce),
-   max_prefix_len, ps_matched,
-   S_ISGITLINK(ce->ce_mode) | 
S_ISDIR(ce->ce_mode)))
+   max_prefix_len, ps_matched, 1))
continue;
 
-   ALLOC_GROW(ce_entries, ce_used + 1, ce_alloc);
-   ce_entries[ce_used++] = ce;
+   ALLOC_GROW(list->entries, list->nr + 1, list->alloc);
+  

Re: [PATCH] Remove perl dependant commands if NO_PERL is set

[Junio C Hamano]

ga...@freebsd.org writes:

> From: Renato Botelho 
>
> git-submodule and git-request-pull are written in sh but use perl
> internally. Add them to be replaced by unimplemented.sh when NO_PERL
> flag is set
> ---

Missing sign-off.

You also may want to hold off the "git-submodule" bit, as I expect
http://thread.gmane.org/gmane.comp.version-control.git/277128/focus=277129
would be one of the first changes to be in 'master' after the
upcoming release 2.6 (that is, it is likely that perl dependency
would be removed in 2.7).

>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index e326fa0..4dae0ec 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1795,7 +1795,7 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
>   chmod +x $@+ && \
>   mv $@+ $@
>  else # NO_PERL
> -$(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
> +$(SCRIPT_PERL_GEN) git-instaweb git-submodule git-request-pull: % : 
> unimplemented.sh
>   $(QUIET_GEN)$(RM) $@ $@+ && \
>   sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
>   -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
--
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: [PATCHv5 1/3] submodule: Reimplement `module_list` shell function in C

[Stefan Beller]

On Thu, Sep 3, 2015 at 11:57 AM, Junio C Hamano  wrote:
>
> It is customary to use X_alloc, X_nr for an array X_something that
> is managed by ALLOC_GROW(), I think.  I'd also suggest wrapping
> these in a struct and passing it between module_list_compute() and
> its callers.

I did not take the suggestion as a strong suggestion at the time, but the
looking at resulting squash proposal it looks way better.

> I may have said this already, but unlike tree entries, the index
> entries will never be a directory.  S_ISDIR() check here is
> meaningless [*1*].

Right. I was too focused on the other bug, of checking S_ISGITLINK after
the pathspec matching, that I overlooked the ISDIR again. :(

>> +int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
>> +{
>> + int i;
>> + if (argc < 2)
>> + die(_("fatal: submodule--helper subcommand must be "
>> +   "called with a subcommand"));
>> +
>> + for (i = 0; i < ARRAY_SIZE(commands); i++)
>> + if (!strcmp(argv[1], commands[i].cmd))
>> + return commands[i].fn(argc - 1, argv + 1, prefix);
>> +
>> + die(_("fatal: '%s' is not a valid submodule--helper "
>> +   "subcommand"), argv[1]);
>> +}
>
> Nice and clean code structure.  I like it ;-).

It took a good while of discussion and reviews to arrive at
that structure eventually.

The squash proposal looks good to me.
--
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: Do you plan to release 2.5.2 any time soon?

[Junio C Hamano]

Johannes Schindelin  writes:

> I have a couple of fixes lined up for bugs in Git for Windows 2.5.1. Do
> you plan to release 2.5.2 any time soon? If yes, I would hold off,
> otherwise I will just do a Git for Windows 2.5.1 (Rel 2).

Among the topics marked for possible later merging to 'maint' in the
draft release notes to 2.6, candidates for future maintenance
snapshots are:

* mh/get-remote-group-fix # 4 (2 weeks ago) 
* cb/open-noatime-clear-errno # 1 (9 days ago) 
* jk/long-error-messages # 2 (9 days ago) 
* jh/strbuf-read-use-read-in-full # 1 (9 days ago) 
* ps/t1509-chroot-test-fixup # 2 (2 weeks ago) 
* sg/help-group # 1 (8 days ago) 
* nd/dwim-wildcards-as-pathspecs # 1 (9 days ago) 
* jk/fix-alias-pager-config-key-warnings # 1 (3 days ago) 
* jk/rev-list-has-no-notes # 1 (3 days ago) 
* nd/fixup-linked-gitdir # 1 (2 days ago) 
* as/docfix-reflog-expire-unreachable # 1 (8 days ago) 
* sg/t3020-typofix # 1 (8 days ago) 
* po/po-readme # 1 (8 days ago) 
* sg/wt-status-header-inclusion # 1 (8 days ago) 
* ss/fix-config-fd-leak # 1 (8 days ago) 
* jc/calloc-pathspec # 1 (8 days ago) 
* rs/archive-zip-many # 3 (2 days ago) 
* dt/commit-preserve-base-index-upon-opportunistic-cache-tree-update # 1 (2 
days ago) 
* cc/trailers-corner-case-fix # 3 (22 hours ago) 
* jk/log-missing-default-HEAD # 1 (22 hours ago) 
* ee/clean-test-fixes # 1 (22 hours ago) 

These I deem are safe (not just the changes themselves are trivially
correct but it is unlikely to make things gravely worse if it turns
out there were ramifications no reviewers thought about with them)
and can go in 2.5.2.

As to other topics that are in 'master' but not in the above list, I
do not have doubt about their value (otherwise they would not be in
'master' in the first place), but either they are still too young in
'master' or are with sufficiently big impact on the callchain that
it is not entirely implausible that they have risks of unforeseen
ramifications and fallouts.

Let's aim to tag 2.5.2 soonish, before mid next-week at the latest.
--
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: glibc mutex deadlock in signal handler

[Takashi Iwai]

On Thu, 03 Sep 2015 20:12:38 +0200,
Junio C Hamano wrote:
> 
> Takashi Iwai  writes:
> 
> > we've got a bug report of git-log stall in below:
> >   https://bugzilla.opensuse.org/show_bug.cgi?id=942297
> >
> > In short, git-log is left unterminated sometimes when pager is
> > aborted.  When this happens, git process can't be killed by SIGTERM,
> > but only by SIGKILL.  And, further investigation revealed the possible
> > mutex deadlock used in glibc *alloc()/free().
> >
> > I tried to reproduce this by adding a fault injection in glibc code,
> > and actually got the similar stack trace as reported.  The problem is
> > that glibc malloc (in this case realloc() and free()) takes a private
> > mutex.  Thus calling any function does *alloc() or free() in a signal
> > handler may deadlock.  In the case of git, it was free() calls in
> > various cleanup codes and the printf() / strerror() invocations.
> >
> > Also, basically it's not safe to call fflush() or raise(), either.
> > But they seem to work practically on the current systems.
> >
> > Below is a band-aid patch I tested and confirmed to work in the
> > fault-injection case.  But, some unsafe calls mentioned in the above
> > are left.  If we want to be in a safer side, we should really limit
> > the things to do in a signal handler, e.g. only calling close() and
> > doing waitpid(), I suppose.
> >
> > Any better ideas?
> 
> Sorry, but I am not with better ideas (at least offhand right now).
> 
> Essentially the idea seems to be to avoid calling allocation-related
> functions in the signal handler codepath that is used for cleaning
> things up.  Not calling free() in the codepaths is perfectly fine as
> we know we will be soon exiting anyway.  Not being able to call
> error() and strerror() to report funnies (other than the fact that
> we were interrupted) is somewhat sad, though.

Reading signal(7) again, I correct some of my statement: raise() is
safe to use.  But fflush() still isn't.  Maybe fflush() should be
avoided but just call only close().

Regarding the error message: write() is still safe to use, so it is
possible in some level.  strerror() seems invoking malloc(), judging
from the stack trace in bugzilla, so this needs to be avoided.
printf() is a blackbox, so it's hard to tell.  That is, in the worst
case, we can just call write() directly for serious errors, if any.


Takashi

> 
> 
> 
> 
> > diff --git a/pager.c b/pager.c
> > index 27d4c8a17aa1..57dda0142fa9 100644
> > --- a/pager.c
> > +++ b/pager.c
> > @@ -14,19 +14,25 @@
> >  static const char *pager_argv[] = { NULL, NULL };
> >  static struct child_process pager_process = CHILD_PROCESS_INIT;
> >  
> > -static void wait_for_pager(void)
> > +static void flush_pager(void)
> >  {
> > fflush(stdout);
> > fflush(stderr);
> > /* signal EOF to pager */
> > close(1);
> > close(2);
> > +}
> > +
> > +static void wait_for_pager(void)
> > +{
> > +   flush_pager();
> > finish_command(_process);
> >  }
> >  
> >  static void wait_for_pager_signal(int signo)
> >  {
> > -   wait_for_pager();
> > +   flush_pager();
> > +   finish_command_in_signal(_process);
> > sigchain_pop(signo);
> > raise(signo);
> >  }
> > diff --git a/run-command.c b/run-command.c
> > index 3277cf797ed4..a8cdc8f32944 100644
> > --- a/run-command.c
> > +++ b/run-command.c
> > @@ -18,26 +18,27 @@ struct child_to_clean {
> >  static struct child_to_clean *children_to_clean;
> >  static int installed_child_cleanup_handler;
> >  
> > -static void cleanup_children(int sig)
> > +static void cleanup_children(int sig, int in_signal)
> >  {
> > while (children_to_clean) {
> > struct child_to_clean *p = children_to_clean;
> > children_to_clean = p->next;
> > kill(p->pid, sig);
> > -   free(p);
> > +   if (!in_signal)
> > +   free(p);
> > }
> >  }
> >  
> >  static void cleanup_children_on_signal(int sig)
> >  {
> > -   cleanup_children(sig);
> > +   cleanup_children(sig, 1);
> > sigchain_pop(sig);
> > raise(sig);
> >  }
> >  
> >  static void cleanup_children_on_exit(void)
> >  {
> > -   cleanup_children(SIGTERM);
> > +   cleanup_children(SIGTERM, 0);
> >  }
> >  
> >  static void mark_child_for_cleanup(pid_t pid)
> > @@ -220,7 +221,7 @@ static inline void set_cloexec(int fd)
> > fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
> >  }
> >  
> > -static int wait_or_whine(pid_t pid, const char *argv0)
> > +static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
> >  {
> > int status, code = -1;
> > pid_t waiting;
> > @@ -231,13 +232,17 @@ static int wait_or_whine(pid_t pid, const char *argv0)
> >  
> > if (waiting < 0) {
> > failed_errno = errno;
> > -   error("waitpid for %s failed: %s", argv0, strerror(errno));
> > +   if (!in_signal)
> > +   error("waitpid for %s failed: %s", argv0,
> > + strerror(errno));
> > } 

Re: [PATCH] Remove perl dependant commands if NO_PERL is set

[Renato Botelho]

> On Sep 3, 2015, at 16:11, Junio C Hamano  wrote:
> 
> ga...@freebsd.org writes:
> 
>> From: Renato Botelho 
>> 
>> git-submodule and git-request-pull are written in sh but use perl
>> internally. Add them to be replaced by unimplemented.sh when NO_PERL
>> flag is set
>> ---
> 
> Missing sign-off.
> 
> You also may want to hold off the "git-submodule" bit, as I expect
> http://thread.gmane.org/gmane.comp.version-control.git/277128/focus=277129
> would be one of the first changes to be in 'master' after the
> upcoming release 2.6 (that is, it is likely that perl dependency
> would be removed in 2.7).

Can’t we add git-submodule to the list now, for 2.6, and then remove it when 
this change is pushed? This would help to have a sane tarball for 2.6. I’m 
considering this from a package maintainer perspective.

I’ll re-send it with sign-off.
--
Renato Botelho

--
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] Remove perl dependant commands if NO_PERL is set

[garga]

From: Renato Botelho 

git-submodule and git-request-pull are written in sh but use perl
internally. Add them to be replaced by unimplemented.sh when NO_PERL
flag is set

Signed-off-by: Renato Botelho 
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index e326fa0..4dae0ec 100644
--- a/Makefile
+++ b/Makefile
@@ -1795,7 +1795,7 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
chmod +x $@+ && \
mv $@+ $@
 else # NO_PERL
-$(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
+$(SCRIPT_PERL_GEN) git-instaweb git-submodule git-request-pull: % : 
unimplemented.sh
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
-- 
2.5.1

--
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 v15 07/13] ref-filter: add support for %(contents:lines=X)

[Karthik Nayak]

On Thu, Sep 3, 2015 at 8:17 PM, Matthieu Moy
 wrote:
> Karthik Nayak  writes:
>
>> On Thu, Sep 3, 2015 at 7:04 PM, Karthik Nayak  wrote:
 struct strbuf s = STRBUF_INIT;
 if (strtoul_ui(valp, 10, >u.contents.lines))
 die(_("positive width expected 
 contents:lines=%s"), valp);
 -   append_lines(, subpos, sublen + bodylen - 
 siglen, v->u.contents.lines);
 +   append_lines(, subpos, bodypos + bodylen - 
 subpos, v->u.contents.lines);
 v->s = strbuf_detach(, NULL);
 }
 }
>>
>> append_lines(, subpos, bodylen + bodypos - subpos - siglen,
>> v->u.contents.lines);
>>
>> We need to eliminate the signature if existing also.
>
> Indeed. I thought body did not include the signature.
>
> I'd write it as
>
>   bodylen + bodypos - siglen - subpos
>
> or even
>
>   char *contents_end = bodylen + bodypos - siglen;
>   ...
>   append_lines(, subpos, contents_end - subpos, ...);
>
> to make it self-explanatory.
>

I was thinking of adding a comment but a self explanatory comment
seems like a good idea. Thanks.

-- 
Regards,
Karthik Nayak
--
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 v2] git-p4: add support for large file systems

[larsxschneider]

From: Lars Schneider 

Diff to v1:

Fixed:
* simpler makedirs
* remove temp file if git-lfs command failed
* fix typos
* remove uneccessary sub shell for (git lfs help)
* make TC not dependable on hashes

Thanks Luke for the feedback!

Added:
* new gitConfigInt function
* support for generic large file systems based on gitattributes
* check compressed file size

Cheers,
Lars

Lars Schneider (4):
  git-p4: add optional type specifier to gitConfig reader
  git-p4: add gitConfigInt reader
  git-p4: return an empty list if a list config has no values
  git-p4: add support for large file systems

 Documentation/git-p4.txt |  21 
 git-p4.py| 147 +++---
 t/t9823-git-p4-lfs.sh| 263 +++
 3 files changed, 418 insertions(+), 13 deletions(-)
 create mode 100755 t/t9823-git-p4-lfs.sh

--
1.9.5 (Apple Git-50.3)

--
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: Default ordering of git log output

[Junio C Hamano]

Kirill Likhodedov  writes:

> Is it intended behavior that the default git log output (without
> ordering parameters) can show parents before children?
>
> The man says:
> Commit Ordering
>By default, the commits are shown in reverse chronological order.
> so it tells nothing about parent-to-child relationship.

When you do not give order and when your traversal is not "limited"
(i.e. you do not specify A in "git log A..B" that tells us where the
traversal ends in topological sense), the traversal "git log" goes:

 - We put HEAD to a queue that holds commits that are further to be
   processed.

 - We pick up the youngest commit from the queue; we show it, and
   push its parents that haven't been shown to the queue.  We repeat
   this step until the queue runs out items.

Your history, when a project participant uses a broken clock to
record the committer timestamp, could look like this (topology flows
from left to right):

   1---5---6
  /   /
 2---3---4

where the labels in the illustration depict the relative order of
their committer timestamps.  Imagine your HEAD is at '6'.

So "git log" would do

Queue   Action
6   show 6, push 5 to the queue
5   show 6, push 1 and 4 to the queue
4 1 show 4, push 3 to the queue
3 1 show 3, push 2 to the queue
2 1 show 2; nothing pushed (as it is root)
1   show 1; nothing is pushed (as its parent 3 has
already been shown)
-empty-
--
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 v15 07/13] ref-filter: add support for %(contents:lines=X)

[Karthik Nayak]

On Thu, Sep 3, 2015 at 8:35 PM, Matthieu Moy
 wrote:
> Eric Sunshine  writes:
>
>> Also, if 'buf' is indeed unconditionally NUL-terminated, then the (sp
>> <= buf + size) check is wasted code since the result of strstr() will
>> always be either NULL or pointing somewhere within the NUL-terminated
>> string.
>
> The null character is there, but after the signature. buf + size points
> before the signature.
>
> Anyway, all this should go away if Karthik applies my suggestion, which
> I like even more after this discussion ;-).

Yes, your suggestion seems like a good way to go.

-- 
Regards,
Karthik Nayak
--
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 v2 1/4] git-p4: add optional type specifier to gitConfig reader

[larsxschneider]

From: Lars Schneider 

Signed-off-by: Lars Schneider 
---
 git-p4.py | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 073f87b..c139cab 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -604,9 +604,12 @@ def gitBranchExists(branch):
 
 _gitConfig = {}
 
-def gitConfig(key):
+def gitConfig(key, typeSpecifier=None):
 if not _gitConfig.has_key(key):
-cmd = [ "git", "config", key ]
+cmd = [ "git", "config" ]
+if typeSpecifier:
+cmd += [ typeSpecifier ]
+cmd += [ key ]
 s = read_pipe(cmd, ignore_error=True)
 _gitConfig[key] = s.strip()
 return _gitConfig[key]
@@ -617,10 +620,7 @@ def gitConfigBool(key):
in the config."""
 
 if not _gitConfig.has_key(key):
-cmd = [ "git", "config", "--bool", key ]
-s = read_pipe(cmd, ignore_error=True)
-v = s.strip()
-_gitConfig[key] = v == "true"
+_gitConfig[key] = gitConfig(key, '--bool') == "true"
 return _gitConfig[key]
 
 def gitConfigList(key):
-- 
1.9.5 (Apple Git-50.3)

--
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 v2 3/4] git-p4: return an empty list if a list config has no values

[larsxschneider]

From: Lars Schneider 

Signed-off-by: Lars Schneider 
---
 git-p4.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/git-p4.py b/git-p4.py
index ae1a4d3..4d78e1c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -638,6 +638,8 @@ def gitConfigList(key):
 if not _gitConfig.has_key(key):
 s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
 _gitConfig[key] = s.strip().split(os.linesep)
+if _gitConfig[key] == ['']:
+_gitConfig[key] = []
 return _gitConfig[key]
 
 def p4BranchesInGit(branchesAreInRemotes=True):
-- 
1.9.5 (Apple Git-50.3)

--
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 v2 2/4] git-p4: add gitConfigInt reader

[larsxschneider]

From: Lars Schneider 

Signed-off-by: Lars Schneider 
---
 git-p4.py | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/git-p4.py b/git-p4.py
index c139cab..ae1a4d3 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -623,6 +623,17 @@ def gitConfigBool(key):
 _gitConfig[key] = gitConfig(key, '--bool') == "true"
 return _gitConfig[key]
 
+def gitConfigInt(key):
+if not _gitConfig.has_key(key):
+cmd = [ "git", "config", "--int", key ]
+s = read_pipe(cmd, ignore_error=True)
+v = s.strip()
+try:
+_gitConfig[key] = int(gitConfig(key, '--int'))
+except Exception, e:
+_gitConfig[key] = None
+return _gitConfig[key]
+
 def gitConfigList(key):
 if not _gitConfig.has_key(key):
 s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
-- 
1.9.5 (Apple Git-50.3)

--
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 Report: git gui clones non-master default branch as master

[John Medema]

Git gurus, your assistance is needed.

Environment #1:
git version 2.5.0.windows.1
git-gui version 0.20.GITGUI
Windows 7 Pro 64-bit

Environment #2:
git version 1.7.9.msysgit.0
git-gui version 0.16.GITGUI
Windows Server 2003 R2 32-bit

Environment #3:
Linux (check with dscho, https://github.com/git-for-windows/git/issues/345)

Issue:
Cloning from Git GUI when the upstream repo has the default branch set
to something other than master (say "develop") yields a local
repository with a single branch named "master". This local master
branch has no pull link, but it has a push link pointing to the
origin/master, leading to a situation where a commit meant for
origin/develop is instead pushed to origin/master. Note that this only
happens when cloning from the Git GUI - using Git Bash creates a local
develop branch pulling & pushing to origin/develop.

To reproduce (example uses fake repo):
1) Open Git GUI
2) Click "Clone Existing Repository"
Source Location: g...@github.com:PrivateCo/mytestrepo.git
Target Directory: c:\temp\mytestrepo
Clone
3) Open Git Bash
4) $ cd /c/temp/mytestrepo
5) $ git remote show origin
remote origin Fetch URL: g...@github.com:PrivateCo/mytestrepo.git
Push URL: g...@github.com:PrivateCo/mytestrepo.git
HEAD branch: develop
Remote branches:
develop tracked
master tracked
Local ref configured for 'git push':
master pushes to master (fast-forwardable)
6) $ git branch -avv
master 846504a 2nd test file, just for develop branch
remotes/origin/develop 846504a 2nd test file, just for develop branch
remotes/origin/master c2b577c initial commit with test file

Workaround (creates new develop branch, wipes out bad master, and
recreates master):
1) Open Git Bash
2) $ cd /c/temp/mytestrepo
3) $ git checkout -b develop --track origin/develop
4) $ git branch -d master
5) $ git checkout -b master --track origin/master
6) $ git remote show origin
remote origin Fetch URL: g...@github.com:PrivateCo/mytestrepo.git
Push URL: g...@github.com:PrivateCo/mytestrepo.git
HEAD branch: develop
Remote branches:
develop tracked
master tracked
Local branches configured for 'git pull':
develop merges with remote develop
master merges with remote master
Local refs configured for 'git push': develop pushes to develop (up to date)
master pushes to master (up to date)
7) $ git branch -avv
develop 846504a [origin/develop] 2nd test file, just for develop branch
master c2b577c [origin/master] initial commit with test file
remotes/origin/develop 846504a 2nd test file, just for develop branch
remotes/origin/master c2b577c initial commit with test file


Let me know if you need any more information.

John Medema
Systems Administrator
United Drugs, a Subsidiary of AAP (American Associated Pharmacies)
john.med...@uniteddrugs.com
7243 N 16th Street, Phoenix, AZ 85020
Office:  602-678-1179 x126
Fax:  602-639-4631

-- 
HIPAA NOTICE:  It is against United Drugs’ policy to receive or send 
un-encrypted or non-secured email correspondence containing Protected 
Health Information (PHI) as defined by HIPAA law.
 
Please use fax or phone for correspondence containing PHI.

-- 
This email message is for the sole use of the intended recipient(s) and may 
contain confidential and privileged information. Any unauthorized review, 
use, disclosure or distribution is prohibited. If you are not the intended 
recipient, contact the sender by reply email, and destroy all copies of the 
original message. 
--
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 v15 07/13] ref-filter: add support for %(contents:lines=X)

[Karthik Nayak]

On Thu, Sep 3, 2015 at 8:09 PM, Eric Sunshine  wrote:
> On Tue, Sep 1, 2015 at 2:26 PM, Karthik Nayak  wrote:
>> In 'tag.c' we can print N lines from the annotation of the tag using
>> the '-n' option. Copy code from 'tag.c' to 'ref-filter' and
>> modify it to support appending of N lines from the annotation of tags
>> to the given strbuf.
>>
>> Implement %(contents:lines=X) where X lines of the given object are
>> obtained.
>>
>> Add documentation and test for the same.
>>
>> Signed-off-by: Karthik Nayak 
>> ---
>>  struct atom_value {
>> const char *s;
>> -   struct align align;
>> +   union {
>> +   struct align align;
>> +   struct contents contents;
>> +   } u;
>> void (*handler)(struct atom_value *atomv, struct 
>> ref_formatting_state *state);
>> unsigned long ul; /* used for sorting when not FIELD_STR */
>>  };
>> @@ -226,7 +235,7 @@ static void align_atom_handler(struct atom_value *atomv, 
>> struct ref_formatting_s
>> push_stack_element(>stack);
>> new = state->stack;
>> new->at_end = align_handler;
>> -   new->cb_data = >align;
>> +   new->cb_data = >u.align;
>
> You could declare atom_value with the union from the start, even if it
> has only a single member ('align', at first). Doing so would make this
> patch less noisy and wouldn't distract the reader with these seemingly
> unrelated changes.
>

Will do.

>>  }
>>
>>  static void end_atom_handler(struct atom_value *atomv, struct 
>> ref_formatting_state *state)
>> @@ -624,6 +633,33 @@ static void find_subpos(const char *buf, unsigned long 
>> sz,
>> *nonsiglen = *sig - buf;
>>  }
>>
>> +/*
>> + * If 'lines' is greater than 0, append that many lines from the given
>> + * 'buf' of length 'size' to the given strbuf.
>> + */
>> +static void append_lines(struct strbuf *out, const char *buf, unsigned long 
>> size, int lines)
>> +{
>> +   int i;
>> +   const char *sp, *eol;
>> +   size_t len;
>> +
>> +   if ((sp = strstr(buf, "\n\n")) && (sp <= buf + size))
>> +   size += 2;
>
> Aside from the +2 which Matthieu already questioned, this code has a
> much more serious problem. strstr() assumes that 'buf' is
> NUL-terminated, however, the fact that buf's size is also being passed
> to the function, implies that it may not be NUL-terminated.
> Consequently, strstr() could zip well past the end of 'buf', trying to
> consult memory not part of 'buf', which is a violation of the C
> standard. Even with the band-aid (sp <= buf + size) which tries to
> detect this situation, it's still a violation, and could crash if
> strstr() attempts to consult memory which hasn't even been allocated
> to the process or is otherwise protected in some fashion.
>
> It's possible that 'buf' may actually always be NUL-terminated, but
> this code does not convey that fact. If it is known to be
> NUL-terminated, then it is safe to use strstr(), however, that should
> be shown more clearly either by revising the code or adding an
> appropriate comment.
>

Although it is NUL terminated, you do have a point, I hope you checked out
Matthieu's suggestion of removing that snippet of code and calculating size
in the function call itself.

>> +   sp = buf;
>> +
>> +   for (i = 0; i < lines && sp < buf + size; i++) {
>> +   if (i)
>> +   strbuf_addstr(out, "\n");
>> +   eol = memchr(sp, '\n', size - (sp - buf));
>> +   len = eol ? eol - sp : size - (sp - buf);
>> +   strbuf_add(out, sp, len);
>> +   if (!eol)
>> +   break;
>> +   sp = eol + 1;
>> +   }
>> +}
>> @@ -663,6 +701,13 @@ static void grab_sub_body_contents(struct atom_value 
>> *val, int deref, struct obj
>> v->s = xmemdupz(sigpos, siglen);
>> else if (!strcmp(name, "contents"))
>> v->s = xstrdup(subpos);
>> +   else if (skip_prefix(name, "contents:lines=", )) {
>> +   struct strbuf s = STRBUF_INIT;
>> +   if (strtoul_ui(valp, 10, >u.contents.lines))
>> +   die(_("positive width expected 
>> contents:lines=%s"), valp);
>
> This error message is still too tightly coupled to %(align:), from
> which it was copied. "width"?
>

Will change it, thanks.

-- 
Regards,
Karthik Nayak
--
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 v2 4/4] git-p4: add support for large file systems

[larsxschneider]

From: Lars Schneider 

Perforce repositories can contain large (binary) files. Migrating these
repositories to Git generates very large local clones. External storage
systems such as LFS [1] or git-annex [2] try to address this problem.

Add a generic mechanism to detect large files based on extension,
uncompressed size, and/or compressed size. Add LFS as example
implementation.

[1] https://git-lfs.github.com/
[2] http://www.git-annex.org/

Signed-off-by: Lars Schneider 
---
 Documentation/git-p4.txt |  21 
 git-p4.py| 126 +--
 t/t9823-git-p4-lfs.sh| 263 +++
 3 files changed, 401 insertions(+), 9 deletions(-)
 create mode 100755 t/t9823-git-p4-lfs.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 82aa5d6..eac5bad 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -510,6 +510,27 @@ git-p4.useClientSpec::
option '--use-client-spec'.  See the "CLIENT SPEC" section above.
This variable is a boolean, not the name of a p4 client.
 
+git-p4.largeFileSystem::
+   Specify the system that is used used for large (binary) files. Only
+   "LFS" [1] is supported right now. Download and install the Git LFS
+   command line extension to use this option.
+   [1] https://git-lfs.github.com/
+
+git-p4.largeFileExtensions::
+   All files matching a file extension in the list will be processed
+   by the large file system. Do not prefix the extensions with '.'.
+
+git-p4.largeFileThreshold::
+   All files with an uncompressed size exceeding the threshold will be
+   processed by the large file system. By default the threshold is
+   defined in bytes. Add the suffix k, m, or g to change the unit.
+
+git-p4.largeFileCompressedThreshold::
+   All files with an compressed size exceeding the threshold will be
+   processed by the large file system. This option might significantly
+   slow down your clone/sync process. By default the threshold is
+   defined in bytes. Add the suffix k, m, or g to change the unit.
+
 Submit variables
 
 git-p4.detectRenames::
diff --git a/git-p4.py b/git-p4.py
index 4d78e1c..cde75a5 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -22,6 +22,8 @@ import platform
 import re
 import shutil
 import stat
+import zipfile
+import zlib
 
 try:
 from subprocess import CalledProcessError
@@ -922,6 +924,51 @@ def wildcard_present(path):
 m = re.search("[*#@%]", path)
 return m is not None
 
+def largeFileSystem():
+try:
+return getattr(sys.modules[__name__], 
gitConfig('git-p4.largeFileSystem'))
+except AttributeError as e:
+die('Large file system not supported: %s' % 
gitConfig('git-p4.largeFileSystem'))
+
+class LFS:
+@staticmethod
+def description():
+return 'LFS (see https://git-lfs.github.com/)'
+
+@staticmethod
+def attributeFilter():
+return 'lfs'
+
+@staticmethod
+def generatePointer(cloneDestination, relPath, contents):
+# Write P4 content to temp file
+p4ContentTempFile = 
tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
+for d in contents:
+p4ContentTempFile.write(d)
+p4ContentTempFile.flush()
+
+# Generate LFS pointer file based on P4 content
+lfsProcess = subprocess.Popen(
+['git', 'lfs', 'pointer', '--file=' + p4ContentTempFile.name],
+stdout=subprocess.PIPE
+)
+lfsPointerFile = lfsProcess.stdout.read()
+if lfsProcess.wait():
+os.remove(p4ContentTempFile.name)
+die('git-lfs command failed. Did you install the extension?')
+contents = [i+'\n' for i in lfsPointerFile.split('\n')[2:][:-1]]
+
+# Write P4 content to LFS
+oid = contents[1].split(' ')[1].split(':')[1][:-1]
+oidPath = os.path.join(cloneDestination, '.git', 'lfs', 'objects', 
oid[:2], oid[2:4])
+if not os.path.isdir(oidPath):
+os.makedirs(oidPath)
+shutil.move(p4ContentTempFile.name, os.path.join(oidPath, oid))
+
+# LFS Spec states that pointer files should not have the executable 
bit set.
+gitMode = '100644'
+return (gitMode, contents)
+
 class Command:
 def __init__(self):
 self.usage = "usage: %prog [options]"
@@ -2038,6 +2085,7 @@ class P4Sync(Command, P4UserMap):
 self.clientSpecDirs = None
 self.tempBranches = []
 self.tempBranchLocation = "git-p4-tmp"
+self.largeFiles = []
 
 if gitConfig("git-p4.syncFromOrigin") == "false":
 self.syncWithOrigin = False
@@ -2158,6 +2206,59 @@ class P4Sync(Command, P4UserMap):
 
 return branches
 
+def writeToGitStream(self, gitMode, relPath, contents):
+self.gitStream.write('M %s inline %s\n' % (gitMode, relPath))
+self.gitStream.write('data 

[PATCH] Remove perl dependant commands if NO_PERL is set

[garga]

From: Renato Botelho 

git-submodule and git-request-pull are written in sh but use perl
internally. Add them to be replaced by unimplemented.sh when NO_PERL
flag is set
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index e326fa0..4dae0ec 100644
--- a/Makefile
+++ b/Makefile
@@ -1795,7 +1795,7 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
chmod +x $@+ && \
mv $@+ $@
 else # NO_PERL
-$(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
+$(SCRIPT_PERL_GEN) git-instaweb git-submodule git-request-pull: % : 
unimplemented.sh
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
-- 
2.5.1

--
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 v1] t9821: use test_config

[Junio C Hamano]

Lars Schneider  writes:

> On 03 Sep 2015, at 15:04, Eric Sunshine  wrote:
>
>> On Thu, Sep 3, 2015 at 5:34 AM,   wrote:
>>> From: Lars Schneider 
>>> 
>>> Signed-off-by: Lars Schneider 
>>> ---
>>> diff --git a/t/t9821-git-p4-path-variations.sh 
>>> b/t/t9821-git-p4-path-variations.sh
>>> index 81e46ac..5a26fec 100755
>>> --- a/t/t9821-git-p4-path-variations.sh
>>> +++ b/t/t9821-git-p4-path-variations.sh
>>> @@ -45,7 +45,7 @@ test_expect_success 'Clone root' '
>>>(
>>>cd "$git" &&
>>>git init . &&
>>> -   git config core.ignorecase false &&
>>> +   test_config core.ignorecase false &&
>> 
>> test_config ensures that the config setting gets "unset" at the end of
>> the test, whether the test succeeds or not, so that subsequent tests
>> are not affected by the setting. However, in this case, since the $git
>> repository gets recreated from scratch for each test anyhow, use of
>> test_config is superfluous. In fact, it may be slightly
>> contraindicated since it could mislead the reader into thinking that
>> state is carried over from test to test. (Not a big objections, but
>> something to take into consideration.)
> OK. Do I need to do anything to take the PATCH suggestion back?

You can just say "I retract this one because...", which you just
did.

For the path-encoding patch, I think the following is all that is
necessary to be squashed in, but please double check (unless you
have some other improvements you want to make on top of v6 of that
patch, no need to reroll only for the following).

Thanks.


 t/t9822-git-p4-path-encoding.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
index e507ad7..3646580 100755
--- a/t/t9822-git-p4-path-encoding.sh
+++ b/t/t9822-git-p4-path-encoding.sh
@@ -39,7 +39,7 @@ test_expect_success 'Clone repo containing iso8859-1 encoded 
paths with git-p4.p
(
cd "$git" &&
git init . &&
-   test_config git-p4.pathEncoding iso8859-1 &&
+   git config git-p4.pathEncoding iso8859-1 &&
git p4 clone --use-client-spec --destination="$git" //depot &&
UTF8="$(printf "$UTF8_ESCAPED")" &&
echo $UTF8 >expect &&

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

git bash bug: ipython doesn't work

[Sergey Chipiga]

I downloaded git for windows v2.5.1. And in git bash I can't launch ipython.
I suppose it forwards stdout somethere.

--
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] graph.c: visual difference on subsequent series

[Junio C Hamano]

Michael J Gruber  writes:

>> Is the design of your independent implementation the same except
>> that 'o' is used instead of 'x'?  Independent implementation does
>> not make the same design magically better, if that is the case ;-)
>
> Interestingly, the patch to the tests lists * to o changes only, no < or
>> to o.

Well, in that case, then the opposite but an equivalent problem
exists in the design, no?  It promises to make roots stand out by
painting them as 'o', but it sometimes fails to do so.  In other
words, ...

> The reason is simply that the patch doesn't change anything for left nor
> right commits. I would say that is the best compromise since it does not
> change the overall layout, provides more information by default and does
> not override information that is requested specifically.

... it fails your last criteria.

> If we want to put more information into log --graph simultaneously we
> should really go beyond ASCII and look at how tig does it, e.g. using
> unicode characters.

That's another way to do so, but shifting columns to show where the
history is not connected also does not change the overall layout,
provides more information by default, etc., and a big plus is that
it would be an approach to do so without having to go beyond ASCII.

--
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 v15 05/13] ref-filter: implement an `align` atom

[Karthik Nayak]

On Thu, Sep 3, 2015 at 7:42 PM, Eric Sunshine  wrote:
> On Tue, Sep 1, 2015 at 2:26 PM, Karthik Nayak  wrote:
>> Implement an `align` atom which left-, middle-, or right-aligns the
>> content between %(align:..) and %(end).
>
> Spell this either %(align:) or %(align:...) with three dots, not two.
> I, personally, think %(align:) is sufficient.
>

Will change.

>> It is followed by `:,`, where the `` is
>> either left, right or middle and `` is the size of the area
>> into which the content will be placed. If the content between
>> %(align:) and %(end) is more than the width then no alignment is
>> performed. e.g. to align a refname atom to the middle with a total
>> width of 40 we can do: --format="%(align:middle,40)%(refname)%(end)".
>>
>> We have an `at_end` function for each element of the stack which is to
>> be called when the `end` atom is encountered. Using this we implement
>> the aling_handler() for the `align` atom, this aligns the final strbuf
>> by calling `strbuf_utf8_align()` from utf8.c.
>>
>> Ensure that quote formatting is performed on the whole of
>> %(align)...%(end) rather than individual atoms inside.  We skip quote
>
> Add colon: %(align:)
>

Okay.

>> formatting for individual atoms when the current stack element is
>> handling an %(align) atom and perform quote formatting at the end when
>
> %(align:)
>

will do.

>> we encounter the %(end) atom.
>>
>> Add documentation and tests for the same.
>>
>> Signed-off-by: Karthik Nayak 
>> ---
>> diff --git a/Documentation/git-for-each-ref.txt 
>> b/Documentation/git-for-each-ref.txt
>> index e49d578..cac3128 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -127,6 +127,15 @@ color::
>> Change output color.  Followed by `:`, where names
>> are described in `color.branch.*`.
>>
>> +align::
>> +   Left-, middle-, or right-align the content between %(align:..)
>
> %(align:)
>

noted.

>> +   and %(end). Followed by `:,`, where the
>> +   `` is either left, right or middle and `` is
>> +   the total length of the content with alignment. If the
>> +   contents length is more than the width then no alignment is
>> +   performed. If used with '--quote' everything in between
>> +   %(align:..)  and %(end) is quoted.
>
> %(align:)
> Also drop the extra space before "and": s/\s+and/ and/
>

Noted, thanks for the review.

-- 
Regards,
Karthik Nayak
--
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

test

[abgadvertising]

hi

--
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 v6] git-p4: add config git-p4.pathEncoding

[Junio C Hamano]

larsxschnei...@gmail.com writes:

> +test_expect_success 'Clone repo containing iso8859-1 encoded paths without 
> git-p4.pathEncoding' '
> + git p4 clone --destination="$git" //depot &&
> + test_when_finished cleanup_git &&
> + (
> + cd "$git" &&
> + UTF8="$(printf "$UTF8_ESCAPED")" &&
> + echo $UTF8 >expect &&
> + git -c core.quotepath=false ls-files >actual &&
> + test_must_fail test_cmp expect actual

I am not sure what this test wants to do.  It is not inconceivable
that future versions of "git p4 clone" becomes more intelligent to
detect the need for git-p4.pathEncoding and set it, so the only
effect to insist the comparison fails is to block future advance in
that direction.

Besides, "test_must_fail test_cmp" looks like a strange thing to
say.  "! test_cmp expect actual" perhaps.

Even better, expect that "expect" and "actual" becomes the same, but
mark the test itself to expect failure, to say "it ought to work
this way in the ideal world, but we know the system currently does
not pass this test".

I'm tempted to suggest squashing the following in.  Thoughts?


 t/t9822-git-p4-path-encoding.sh | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
index e507ad7..2d652d89 100755
--- a/t/t9822-git-p4-path-encoding.sh
+++ b/t/t9822-git-p4-path-encoding.sh
@@ -21,15 +21,15 @@ test_expect_success 'Create a repo containing iso8859-1 
encoded paths' '
)
 '
 
-test_expect_success 'Clone repo containing iso8859-1 encoded paths without 
git-p4.pathEncoding' '
+test_expect_failure 'Clone auto-detects depot with iso8859-1 paths' '
git p4 clone --destination="$git" //depot &&
test_when_finished cleanup_git &&
(
cd "$git" &&
UTF8="$(printf "$UTF8_ESCAPED")" &&
-   echo $UTF8 >expect &&
+   echo "$UTF8" >expect &&
git -c core.quotepath=false ls-files >actual &&
-   test_must_fail test_cmp expect actual
+   test_cmp expect actual
)
 '
 
@@ -39,16 +39,15 @@ test_expect_success 'Clone repo containing iso8859-1 
encoded paths with git-p4.p
(
cd "$git" &&
git init . &&
-   test_config git-p4.pathEncoding iso8859-1 &&
+   git config git-p4.pathEncoding iso8859-1 &&
git p4 clone --use-client-spec --destination="$git" //depot &&
UTF8="$(printf "$UTF8_ESCAPED")" &&
-   echo $UTF8 >expect &&
+   echo "$UTF8" >expect &&
git -c core.quotepath=false ls-files >actual &&
test_cmp expect actual &&
-   cat >expect <<-\EOF &&
-   content123
-   EOF
-   cat $UTF8 >actual &&
+
+   echo content123 >expect &&
+   cat "$UTF8" >actual &&
test_cmp expect actual
)
 '
--
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 v15 07/13] ref-filter: add support for %(contents:lines=X)

[Junio C Hamano]

Eric Sunshine  writes:

> Also, if 'buf' is indeed unconditionally NUL-terminated, then the (sp
> <= buf + size) check is wasted code since the result of strstr() will
> always be either NULL or pointing somewhere within the NUL-terminated
> string.

A caller can give a buf that is NUL terminated but specify that the
only early part of the buffer to be used by giving you a shorter
size, no?  In such a case, strstr() is safe in the sense that it is
guaranteed not to go on forever, but you need to verify the location
of the string it found is within the bounds.
--
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 2/4] git-p4: add gitConfigInt reader

[Lars Schneider]

On 03 Sep 2015, at 21:57, Luke Diamand  wrote:

> On 03/09/15 17:35, larsxschnei...@gmail.com wrote:
>> From: Lars Schneider 
>> 
> 
> Explanation?
Add a git config reader for integer variables. Please note that the git config 
implementation automatically supports k, m, and g suffixes.

OK?
 
> 
>> Signed-off-by: Lars Schneider 
>> ---
>>  git-p4.py | 11 +++
>>  1 file changed, 11 insertions(+)
>> 
>> diff --git a/git-p4.py b/git-p4.py
>> index c139cab..ae1a4d3 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -623,6 +623,17 @@ def gitConfigBool(key):
>>  _gitConfig[key] = gitConfig(key, '--bool') == "true"
>>  return _gitConfig[key]
>> 
>> +def gitConfigInt(key):
>> +if not _gitConfig.has_key(key):
>> +cmd = [ "git", "config", "--int", key ]
>> +s = read_pipe(cmd, ignore_error=True)
>> +v = s.strip()
>> +try:
>> +_gitConfig[key] = int(gitConfig(key, '--int'))
>> +except Exception, e:
> 
> Could do with specifying the actual type of the exception here (ValueError).
Will do!

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

Re: [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader

[Lars Schneider]

On 03 Sep 2015, at 22:18, Junio C Hamano  wrote:

> Lars Schneider  writes:
> 
>> On 03 Sep 2015, at 21:55, Luke Diamand  wrote:
>> 
>>> On 03/09/15 17:35, larsxschnei...@gmail.com wrote:
 From: Lars Schneider 
 
>>> 
>>> I think this commit may need some explanation!
>> 
>> The functions “gitConfig” and “gitConfigBool” are almost
>> identical. Make “gitConfig” more generic by adding an optional type
>> specifier. Use the type specifier “—bool” with “gitConfig” to
>> implement “gitConfigBool. This prepares the implementation of other
>> type specifiers such as “—int”.
> 
> OK.
> 
>> OK?
> 
> Not really ;-).  The point of Luke's message is that all of the
> above belong to the log message, I think.
Right, it is my intention to add this as commit message. I just wanted to check 
upfront if the message is what he expects.

That leads me to a general question:
In case I agree with a reviewer. What is the more appropriate action? A 
response like the one above or a new role that includes the change right away? 
I don’t want to spam the list with lots of tiny changes…

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

[PATCH v3 11/11] t6300: add tests for "-local" date formats

[John Keeping]

Signed-off-by: John Keeping 
---
Changes since v2:
- "relative-local" and "raw-local" are now allowed

 t/t6300-for-each-ref.sh | 36 
 1 file changed, 36 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 899251e..03873b0 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -180,6 +180,10 @@ test_expect_success 'Check format "default" formatted date 
fields output' '
"Tue Jul 4 01:18:45 2006 +0200"
 '
 
+test_expect_success 'Check format "default-local" date fields output' '
+   test_date default-local "Mon Jul 3 23:18:43 2006" "Mon Jul 3 23:18:44 
2006" "Mon Jul 3 23:18:45 2006"
+'
+
 # Don't know how to do relative check because I can't know when this script
 # is going to be run and can't fake the current time to git, and hence can't
 # provide expected output.  Instead, I'll just make sure that "relative"
@@ -190,10 +194,22 @@ test_expect_success 'Check format "relative" date fields 
output' '
git for-each-ref --shell --format="%(refname) %(taggerdate:$f)" 
refs/tags) >actual
 '
 
+# We just check that this is the same as "relative" for now.
+test_expect_success 'Check format "relative-local" date fields output' '
+   test_date relative-local \
+   "$(git for-each-ref --format="%(committerdate:relative)" 
refs/heads)" \
+   "$(git for-each-ref --format="%(authordate:relative)" 
refs/heads)" \
+   "$(git for-each-ref --format="%(taggerdate:relative)" 
refs/tags)"
+'
+
 test_expect_success 'Check format "short" date fields output' '
test_date short 2006-07-04 2006-07-04 2006-07-04
 '
 
+test_expect_success 'Check format "short-local" date fields output' '
+   test_date short-local 2006-07-03 2006-07-03 2006-07-03
+'
+
 test_expect_success 'Check format "local" date fields output' '
test_date local \
"Mon Jul 3 23:18:43 2006" \
@@ -208,6 +224,10 @@ test_expect_success 'Check format "iso8601" date fields 
output' '
"2006-07-04 01:18:45 +0200"
 '
 
+test_expect_success 'Check format "iso8601-local" date fields output' '
+   test_date iso8601-local "2006-07-03 23:18:43 +" "2006-07-03 
23:18:44 +" "2006-07-03 23:18:45 +"
+'
+
 test_expect_success 'Check format "rfc2822" date fields output' '
test_date rfc2822 \
"Tue, 4 Jul 2006 01:18:43 +0200" \
@@ -215,10 +235,18 @@ test_expect_success 'Check format "rfc2822" date fields 
output' '
"Tue, 4 Jul 2006 01:18:45 +0200"
 '
 
+test_expect_success 'Check format "rfc2822-local" date fields output' '
+   test_date rfc2822-local "Mon, 3 Jul 2006 23:18:43 +" "Mon, 3 Jul 
2006 23:18:44 +" "Mon, 3 Jul 2006 23:18:45 +"
+'
+
 test_expect_success 'Check format "raw" date fields output' '
test_date raw "1151968723 +0200" "1151968724 +0200" "1151968725 +0200"
 '
 
+test_expect_success 'Check format "raw-local" date fields output' '
+   test_date raw-local "1151968723 +" "1151968724 +" "1151968725 
+"
+'
+
 test_expect_success 'Check format of strftime date fields' '
echo "my date is 2006-07-04" >expected &&
git for-each-ref \
@@ -227,6 +255,14 @@ test_expect_success 'Check format of strftime date fields' 
'
test_cmp expected actual
 '
 
+test_expect_success 'Check format of strftime-local date fields' '
+   echo "my date is 2006-07-03" >expected &&
+   git for-each-ref \
+ --format="%(authordate:format-local:my date is %Y-%m-%d)" \
+ refs/heads >actual &&
+   test_cmp expected actual
+'
+
 test_expect_success 'exercise strftime with odd fields' '
echo >expected &&
git for-each-ref --format="%(authordate:format:)" refs/heads >actual &&
-- 
2.5.0.466.g9af26fa

--
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 v3 10/11] t6300: make UTC and local dates different

[John Keeping]

By setting the UTC time to 23:18:43 the date in +0200 is the following
day, 2006-07-04.  This will ensure that the test for "short-local" to be
added in the following patch tests for different output from the "short"
format.

Signed-off-by: John Keeping 
---
 t/t6300-for-each-ref.sh | 70 -
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 6afcff6..899251e 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -8,8 +8,8 @@ test_description='for-each-ref test'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-gpg.sh
 
-# Mon Jul 3 15:18:43 2006 +
-datestamp=1151939923
+# Mon Jul 3 23:18:43 2006 +
+datestamp=1151968723
 setdate_and_increment () {
 GIT_COMMITTER_DATE="$datestamp +0200"
 datestamp=$(expr "$datestamp" + 1)
@@ -61,21 +61,21 @@ test_atom head object ''
 test_atom head type ''
 test_atom head '*objectname' ''
 test_atom head '*objecttype' ''
-test_atom head author 'A U Thor  1151939924 +0200'
+test_atom head author 'A U Thor  1151968724 +0200'
 test_atom head authorname 'A U Thor'
 test_atom head authoremail ''
-test_atom head authordate 'Mon Jul 3 17:18:44 2006 +0200'
-test_atom head committer 'C O Mitter  1151939923 +0200'
+test_atom head authordate 'Tue Jul 4 01:18:44 2006 +0200'
+test_atom head committer 'C O Mitter  1151968723 +0200'
 test_atom head committername 'C O Mitter'
 test_atom head committeremail ''
-test_atom head committerdate 'Mon Jul 3 17:18:43 2006 +0200'
+test_atom head committerdate 'Tue Jul 4 01:18:43 2006 +0200'
 test_atom head tag ''
 test_atom head tagger ''
 test_atom head taggername ''
 test_atom head taggeremail ''
 test_atom head taggerdate ''
-test_atom head creator 'C O Mitter  1151939923 +0200'
-test_atom head creatordate 'Mon Jul 3 17:18:43 2006 +0200'
+test_atom head creator 'C O Mitter  1151968723 +0200'
+test_atom head creatordate 'Tue Jul 4 01:18:43 2006 +0200'
 test_atom head subject 'Initial'
 test_atom head contents:subject 'Initial'
 test_atom head body ''
@@ -96,7 +96,7 @@ test_atom tag parent ''
 test_atom tag numparent ''
 test_atom tag object $(git rev-parse refs/tags/testtag^0)
 test_atom tag type 'commit'
-test_atom tag '*objectname' '67a36f10722846e891fbada1ba48ed035de75581'
+test_atom tag '*objectname' 'ea122842f48be4afb2d1fc6a4b96c05885ab7463'
 test_atom tag '*objecttype' 'commit'
 test_atom tag author ''
 test_atom tag authorname ''
@@ -107,18 +107,18 @@ test_atom tag committername ''
 test_atom tag committeremail ''
 test_atom tag committerdate ''
 test_atom tag tag 'testtag'
-test_atom tag tagger 'C O Mitter  1151939925 +0200'
+test_atom tag tagger 'C O Mitter  1151968725 +0200'
 test_atom tag taggername 'C O Mitter'
 test_atom tag taggeremail ''
-test_atom tag taggerdate 'Mon Jul 3 17:18:45 2006 +0200'
-test_atom tag creator 'C O Mitter  1151939925 +0200'
-test_atom tag creatordate 'Mon Jul 3 17:18:45 2006 +0200'
-test_atom tag subject 'Tagging at 1151939927'
-test_atom tag contents:subject 'Tagging at 1151939927'
+test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'
+test_atom tag creator 'C O Mitter  1151968725 +0200'
+test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'
+test_atom tag subject 'Tagging at 1151968727'
+test_atom tag contents:subject 'Tagging at 1151968727'
 test_atom tag body ''
 test_atom tag contents:body ''
 test_atom tag contents:signature ''
-test_atom tag contents 'Tagging at 1151939927
+test_atom tag contents 'Tagging at 1151968727
 '
 test_atom tag HEAD ' '
 
@@ -168,16 +168,16 @@ test_date () {
 
 test_expect_success 'Check unformatted date fields output' '
test_date "" \
-   "Mon Jul 3 17:18:43 2006 +0200" \
-   "Mon Jul 3 17:18:44 2006 +0200" \
-   "Mon Jul 3 17:18:45 2006 +0200"
+   "Tue Jul 4 01:18:43 2006 +0200" \
+   "Tue Jul 4 01:18:44 2006 +0200" \
+   "Tue Jul 4 01:18:45 2006 +0200"
 '
 
 test_expect_success 'Check format "default" formatted date fields output' '
test_date default \
-   "Mon Jul 3 17:18:43 2006 +0200" \
-   "Mon Jul 3 17:18:44 2006 +0200" \
-   "Mon Jul 3 17:18:45 2006 +0200"
+   "Tue Jul 4 01:18:43 2006 +0200" \
+   "Tue Jul 4 01:18:44 2006 +0200" \
+   "Tue Jul 4 01:18:45 2006 +0200"
 '
 
 # Don't know how to do relative check because I can't know when this script
@@ -191,36 +191,36 @@ test_expect_success 'Check format "relative" date fields 
output' '
 '
 
 test_expect_success 'Check format "short" date fields output' '
-   test_date short 2006-07-03 2006-07-03 2006-07-03
+   

Re: [PATCH] Remove perl dependant commands if NO_PERL is set

[Junio C Hamano]

Renato Botelho  writes:

>> You also may want to hold off the "git-submodule" bit, as I expect
>> http://thread.gmane.org/gmane.comp.version-control.git/277128/focus=277129
>> would be one of the first changes to be in 'master' after the
>> upcoming release 2.6 (that is, it is likely that perl dependency
>> would be removed in 2.7).
>
> Can’t we add git-submodule to the list now, for 2.6, and then remove
> it when this change is pushed? This would help to have a sane tarball
> for 2.6. I’m considering this from a package maintainer perspective.

Not really.  Anything brand-new that comes this late in the cycle
will not be in 2.6, so the earliest release this NO_PERL change can
appear in is one after the upcoming release.

--
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 1/4] git-p4: add optional type specifier to gitConfig reader

[Luke Diamand]

On 03/09/15 17:35, larsxschnei...@gmail.com wrote:

From: Lars Schneider 



I think this commit may need some explanation!


Signed-off-by: Lars Schneider 
---
  git-p4.py | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 073f87b..c139cab 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -604,9 +604,12 @@ def gitBranchExists(branch):

  _gitConfig = {}

-def gitConfig(key):
+def gitConfig(key, typeSpecifier=None):
  if not _gitConfig.has_key(key):
-cmd = [ "git", "config", key ]
+cmd = [ "git", "config" ]
+if typeSpecifier:
+cmd += [ typeSpecifier ]
+cmd += [ key ]
  s = read_pipe(cmd, ignore_error=True)
  _gitConfig[key] = s.strip()
  return _gitConfig[key]
@@ -617,10 +620,7 @@ def gitConfigBool(key):
 in the config."""

  if not _gitConfig.has_key(key):
-cmd = [ "git", "config", "--bool", key ]
-s = read_pipe(cmd, ignore_error=True)
-v = s.strip()
-_gitConfig[key] = v == "true"
+_gitConfig[key] = gitConfig(key, '--bool') == "true"
  return _gitConfig[key]

  def gitConfigList(key):



--
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 1/4] git-p4: add optional type specifier to gitConfig reader

[Junio C Hamano]

Lars Schneider  writes:

> On 03 Sep 2015, at 21:55, Luke Diamand  wrote:
>
>> On 03/09/15 17:35, larsxschnei...@gmail.com wrote:
>>> From: Lars Schneider 
>>> 
>> 
>> I think this commit may need some explanation!
>
> The functions “gitConfig” and “gitConfigBool” are almost
> identical. Make “gitConfig” more generic by adding an optional type
> specifier. Use the type specifier “—bool” with “gitConfig” to
> implement “gitConfigBool. This prepares the implementation of other
> type specifiers such as “—int”.

OK.

> OK?

Not really ;-).  The point of Luke's message is that all of the
above belong to the log message, I think.
--
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 1/4] git-p4: add optional type specifier to gitConfig reader

[Junio C Hamano]

Lars Schneider  writes:

> In case I agree with a reviewer. What is the more appropriate action?
> A response like the one above or a new role that includes the change
> right away? I don’t want to spam the list with lots of tiny changes…

Responding to review comment like you did with "will do" is
perfectly fine.

When you do think you will (eventually) want to send an updated
patch, there are things other than "will do" that you can say.  "I
understand what you said, but I am not sure how exactly to make this
better.  Perhaps lend me a help?" is good, too.

An explanation, followed by "ok?", in response to "it is unclear
what you are doing here" (commenting on code) or to "I cannot
understand what this is trying to say" (commenting on log message),
is problematic because your intention becomes ambiguous.

The reviewers are rarely saying "I do not understand; educate me."
but "I do not understand, and it is likely many others don't, too.
Make it more easily understandable." is what they mean.

An explanation with "ok?" can be taken as a sign that you mistook
the review comment as "educate me".

What I meant was   Do you think commenting the code here
with the above description is good enough?  Or do you think
of a way to restructure the code itself to be more self
evident?

or something like that may be a way to avoid the ambiguity.

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

Re: git bash bug: ipython doesn't work

[Johannes Schindelin]

Hi Sergey,

On 2015-09-03 18:56, Sergey Chipiga wrote:
> I downloaded git for windows v2.5.1. And in git bash I can't launch ipython.
> I suppose it forwards stdout somethere.

I replied in the ticket you opened on GitHub: 
https://github.com/git-for-windows/git/issues/352

Ciao,
Johannes
--
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 4/4] git-p4: add support for large file systems

[Luke Diamand]

On 03/09/15 17:35, larsxschnei...@gmail.com wrote:

From: Lars Schneider 

Perforce repositories can contain large (binary) files. Migrating these
repositories to Git generates very large local clones. External storage
systems such as LFS [1] or git-annex [2] try to address this problem.

Add a generic mechanism to detect large files based on extension,
uncompressed size, and/or compressed size. Add LFS as example
implementation.


Can you split this into "add mechanism for large file support" and then 
"add LFS implementation".


It will make it easier to review if nothing else.

Some other comments inline.

Thanks!
Luke



[1] https://git-lfs.github.com/
[2] http://www.git-annex.org/

Signed-off-by: Lars Schneider 
---
  Documentation/git-p4.txt |  21 
  git-p4.py| 126 +--
  t/t9823-git-p4-lfs.sh| 263 +++
  3 files changed, 401 insertions(+), 9 deletions(-)
  create mode 100755 t/t9823-git-p4-lfs.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 82aa5d6..eac5bad 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -510,6 +510,27 @@ git-p4.useClientSpec::
option '--use-client-spec'.  See the "CLIENT SPEC" section above.
This variable is a boolean, not the name of a p4 client.

+git-p4.largeFileSystem::
+   Specify the system that is used used for large (binary) files. Only
+   "LFS" [1] is supported right now. Download and install the Git LFS
+   command line extension to use this option.
+   [1] https://git-lfs.github.com/
+
+git-p4.largeFileExtensions::
+   All files matching a file extension in the list will be processed
+   by the large file system. Do not prefix the extensions with '.'.
+
+git-p4.largeFileThreshold::
+   All files with an uncompressed size exceeding the threshold will be
+   processed by the large file system. By default the threshold is
+   defined in bytes. Add the suffix k, m, or g to change the unit.
+
+git-p4.largeFileCompressedThreshold::
+   All files with an compressed size exceeding the threshold will be


s/an compressed/a compressed/


+   processed by the large file system. This option might significantly
+   slow down your clone/sync process. By default the threshold is
+   defined in bytes. Add the suffix k, m, or g to change the unit.
+
  Submit variables
  
  git-p4.detectRenames::
diff --git a/git-p4.py b/git-p4.py
index 4d78e1c..cde75a5 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -22,6 +22,8 @@ import platform
  import re
  import shutil
  import stat
+import zipfile
+import zlib

  try:
  from subprocess import CalledProcessError
@@ -922,6 +924,51 @@ def wildcard_present(path):
  m = re.search("[*#@%]", path)
  return m is not None

+def largeFileSystem():
+try:
+return getattr(sys.modules[__name__], 
gitConfig('git-p4.largeFileSystem'))
+except AttributeError as e:
+die('Large file system not supported: %s' % 
gitConfig('git-p4.largeFileSystem'))
+
+class LFS:


Docstrings?



+@staticmethod
+def description():
+return 'LFS (see https://git-lfs.github.com/)'
+
+@staticmethod
+def attributeFilter():
+return 'lfs'
+
+@staticmethod
+def generatePointer(cloneDestination, relPath, contents):
+# Write P4 content to temp file
+p4ContentTempFile = 
tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)


delete=False, doesn't that mean that if anything goes wrong, we leave 
large files lying around? Perhaps that doesn't matter, I don't know.



+for d in contents:
+p4ContentTempFile.write(d)
+p4ContentTempFile.flush()


These seems like behaviour that could live in the base class or the main 
git-p4 code. It doesn't look LFS-specific.



+
+# Generate LFS pointer file based on P4 content
+lfsProcess = subprocess.Popen(
+['git', 'lfs', 'pointer', '--file=' + p4ContentTempFile.name],
+stdout=subprocess.PIPE
+)
+lfsPointerFile = lfsProcess.stdout.read()
+if lfsProcess.wait():
+os.remove(p4ContentTempFile.name)
+die('git-lfs command failed. Did you install the extension?')
+contents = [i+'\n' for i in lfsPointerFile.split('\n')[2:][:-1]]
+
+# Write P4 content to LFS
+oid = contents[1].split(' ')[1].split(':')[1][:-1]
+oidPath = os.path.join(cloneDestination, '.git', 'lfs', 'objects', 
oid[:2], oid[2:4])
+if not os.path.isdir(oidPath):
+os.makedirs(oidPath)
+shutil.move(p4ContentTempFile.name, os.path.join(oidPath, oid))


This also does not look LFS-specific.


+
+# LFS Spec states that pointer files should not have the executable 
bit set.
+gitMode = '100644'
+return (gitMode, contents)
+
  class Command:
  

Re: [PATCH] Remove perl dependant commands if NO_PERL is set

[Renato Botelho]

> On Sep 3, 2015, at 16:56, Junio C Hamano  wrote:
> 
> Renato Botelho  writes:
> 
>>> You also may want to hold off the "git-submodule" bit, as I expect
>>> http://thread.gmane.org/gmane.comp.version-control.git/277128/focus=277129
>>> would be one of the first changes to be in 'master' after the
>>> upcoming release 2.6 (that is, it is likely that perl dependency
>>> would be removed in 2.7).
>> 
>> Can’t we add git-submodule to the list now, for 2.6, and then remove
>> it when this change is pushed? This would help to have a sane tarball
>> for 2.6. I’m considering this from a package maintainer perspective.
> 
> Not really.  Anything brand-new that comes this late in the cycle
> will not be in 2.6, so the earliest release this NO_PERL change can
> appear in is one after the upcoming release.

Even if it’s a bug that produce a broken installation?

Nevermind, I’ll add the patch into FreeBSD ports so FreeBSD users can have a 
sane package installed.

--
Renato Botelho

--
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 v3 02/11] Documentation/config: don't list date formats

[John Keeping]

This list is already incomplete (missing "raw") and we're about to add
new formats.  Since this option sets a default for git-log's --date
option, just refer to git-log(1).

Signed-off-by: John Keeping 
---
 Documentation/config.txt | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f5d15ff..49f9fa8 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1829,9 +1829,7 @@ log.abbrevCommit::
 log.date::
Set the default date-time mode for the 'log' command.
Setting a value for log.date is similar to using 'git log''s
-   `--date` option.  Possible values are `relative`, `local`,
-   `default`, `iso`, `rfc`, and `short`; see linkgit:git-log[1]
-   for details.
+   `--date` option.  See linkgit:git-log[1] for details.
 
 log.decorate::
Print out the ref names of any commits that are shown by the log
-- 
2.5.0.466.g9af26fa

--
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 v3 03/11] Documentation/git-for-each-ref: don't list date formats

[John Keeping]

We are about to add a new set of supported date formats and do not want
to have to maintain the same list in several different bits of
documentation.  Refer to git-rev-list(1) which contains the full list of
supported formats.

Signed-off-by: John Keeping 
---
 Documentation/git-for-each-ref.txt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 7f8d9a5..d062639 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -134,9 +134,8 @@ the object referred by the ref does not cause an error.  It
 returns an empty string instead.
 
 As a special case for the date-type fields, you may specify a format for
-the date by adding one of `:default`, `:relative`, `:short`, `:local`,
-`:iso8601`, `:rfc2822` or `:raw` to the end of the fieldname; e.g.
-`%(taggerdate:relative)`.
+the date by adding `:` followed by date format name (see the
+values the `--date` option to linkgit::git-rev-list[1] takes).
 
 
 EXAMPLES
-- 
2.5.0.466.g9af26fa

--
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 v3 01/11] Documentation/blame-options: don't list date formats

[John Keeping]

This list is already incomplete (missing "raw") and we're about to add
new formats.  Remove it and refer to the canonical documentation in
git-log(1).

Signed-off-by: John Keeping 
---
 Documentation/blame-options.txt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index a09969b..760eab7 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -63,11 +63,10 @@ include::line-range-format.txt[]
`-` to make the command read from the standard input).
 
 --date ::
-   The value is one of the following alternatives:
-   {relative,local,default,iso,rfc,short}. If --date is not
+   Specifies the format used to output dates. If --date is not
provided, the value of the blame.date config variable is
used. If the blame.date config variable is also not set, the
-   iso format is used. For more information, See the discussion
+   iso format is used. For supported values, see the discussion
of the --date option at linkgit:git-log[1].
 
 -M||::
-- 
2.5.0.466.g9af26fa

--
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 v3 00/11] Make "local" orthogonal to date format

[John Keeping]

Since version 2 there are four new preparatory patches which remove
lists of date formats from documentation in favour of referring to the
detailed list in git-rev-list(1) or git-log(1) (both generated from
Documentation/rev-list-options.txt) depending on whether the page in
question is plumbing/porcelain.

I've also reordered the test cleanup patches earlier so that the test
for "--date=raw" is added before the new patch that moves "local"
processing before the "raw" case.  The tests also now wrap long lines
and a missing "&&" has been added.

In patch 7 (date: check for "local" before anything else), we no longer
reject "relative-local" and "raw-local" now prints the user's local
timezone offset.  The error message for invalid formats that are
prefixed with a valid format name is now the same as that if there is no
valid prefix.

Jeff King (2):
  fast-import: switch crash-report date to iso8601
  date: make "local" orthogonal to date format

John Keeping (9):
  Documentation/blame-options: don't list date formats
  Documentation/config: don't list date formats
  Documentation/git-for-each-ref: don't list date formats
  Documentation/rev-list: don't list date formats
  t6300: introduce test_date() helper
  t6300: add test for "raw" date format
  date: check for "local" before anything else
  t6300: make UTC and local dates different
  t6300: add tests for "-local" date formats

 Documentation/blame-options.txt|   5 +-
 Documentation/config.txt   |   4 +-
 Documentation/git-for-each-ref.txt |   5 +-
 Documentation/git-rev-list.txt |   2 +-
 Documentation/rev-list-options.txt |  23 --
 builtin/blame.c|   1 -
 cache.h|   2 +-
 date.c |  74 ++---
 fast-import.c  |   2 +-
 t/t6300-for-each-ref.sh| 162 ++---
 10 files changed, 166 insertions(+), 114 deletions(-)

-- 
2.5.0.466.g9af26fa

--
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: glibc mutex deadlock in signal handler

[Andreas Schwab]

See

for the complete list of functions you may safely call from a signal
handler.

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: [PATCH v2 2/4] git-p4: add gitConfigInt reader

[Luke Diamand]

On 03/09/15 17:35, larsxschnei...@gmail.com wrote:

From: Lars Schneider 



Explanation?


Signed-off-by: Lars Schneider 
---
  git-p4.py | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/git-p4.py b/git-p4.py
index c139cab..ae1a4d3 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -623,6 +623,17 @@ def gitConfigBool(key):
  _gitConfig[key] = gitConfig(key, '--bool') == "true"
  return _gitConfig[key]

+def gitConfigInt(key):
+if not _gitConfig.has_key(key):
+cmd = [ "git", "config", "--int", key ]
+s = read_pipe(cmd, ignore_error=True)
+v = s.strip()
+try:
+_gitConfig[key] = int(gitConfig(key, '--int'))
+except Exception, e:


Could do with specifying the actual type of the exception here (ValueError).




+_gitConfig[key] = None
+return _gitConfig[key]
+
  def gitConfigList(key):
  if not _gitConfig.has_key(key):
  s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)



--
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] Remove perl dependant commands if NO_PERL is set

[Junio C Hamano]

ga...@freebsd.org writes:

> From: Renato Botelho 
>
> git-submodule and git-request-pull are written in sh but use perl
> internally. Add them to be replaced by unimplemented.sh when NO_PERL
> flag is set
>
> Signed-off-by: Renato Botelho 
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index e326fa0..4dae0ec 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1795,7 +1795,7 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
>   chmod +x $@+ && \
>   mv $@+ $@
>  else # NO_PERL
> -$(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
> +$(SCRIPT_PERL_GEN) git-instaweb git-submodule git-request-pull: % : 
> unimplemented.sh
>   $(QUIET_GEN)$(RM) $@ $@+ && \
>   sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
>   -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \

Thanks, but this I suspect is insufficient.  In the pre-context of
your patch, you can see we have a rule to create git-instaweb when
NO_PERL is not in effect, so either way, we only have one rule to
create git-instaweb.

You are not disabling the rule to create the real git-submodule
and git-request-pull when NO_PERL is in effect with this patch,
without it, 'make' cannot tell which variant of git-submodule and
git-request-pull needs to be built.

I agree replacing commands with unimplemented may be a good thing; I
do not object to the goal of the patch.  But hopefully you now see
why it is too late to start discussing it with 2.6 as the goal.
--
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 1/4] git-p4: add optional type specifier to gitConfig reader

[Lars Schneider]

On 03 Sep 2015, at 21:55, Luke Diamand  wrote:

> On 03/09/15 17:35, larsxschnei...@gmail.com wrote:
>> From: Lars Schneider 
>> 
> 
> I think this commit may need some explanation!

The functions “gitConfig” and “gitConfigBool” are almost identical. Make 
“gitConfig” more generic by adding an optional type specifier. Use the type 
specifier “—bool” with “gitConfig” to implement “gitConfigBool. This prepares 
the implementation of other type specifiers such as “—int”.

OK?

Thank you,
Lars

> 
>> Signed-off-by: Lars Schneider 
>> ---
>>  git-p4.py | 12 ++--
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>> 
>> diff --git a/git-p4.py b/git-p4.py
>> index 073f87b..c139cab 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -604,9 +604,12 @@ def gitBranchExists(branch):
>> 
>>  _gitConfig = {}
>> 
>> -def gitConfig(key):
>> +def gitConfig(key, typeSpecifier=None):
>>  if not _gitConfig.has_key(key):
>> -cmd = [ "git", "config", key ]
>> +cmd = [ "git", "config" ]
>> +if typeSpecifier:
>> +cmd += [ typeSpecifier ]
>> +cmd += [ key ]
>>  s = read_pipe(cmd, ignore_error=True)
>>  _gitConfig[key] = s.strip()
>>  return _gitConfig[key]
>> @@ -617,10 +620,7 @@ def gitConfigBool(key):
>> in the config."""
>> 
>>  if not _gitConfig.has_key(key):
>> -cmd = [ "git", "config", "--bool", key ]
>> -s = read_pipe(cmd, ignore_error=True)
>> -v = s.strip()
>> -_gitConfig[key] = v == "true"
>> +_gitConfig[key] = gitConfig(key, '--bool') == "true"
>>  return _gitConfig[key]
>> 
>>  def gitConfigList(key):
>> 
> 

--
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 2/4] git-p4: add gitConfigInt reader

[Luke Diamand]

On 03/09/15 21:17, Lars Schneider wrote:


On 03 Sep 2015, at 21:57, Luke Diamand  wrote:


On 03/09/15 17:35, larsxschnei...@gmail.com wrote:

From: Lars Schneider 



Explanation?

Add a git config reader for integer variables. Please note that the git config 
implementation automatically supports k, m, and g suffixes.


Sorry, I should have been clearer. The commit needs a comment to say 
both what it's doing ("adding a git config int reader") and why.


c.f. Documentation/SubmittingChanges.

Luke

--
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 v3 05/11] fast-import: switch crash-report date to iso8601

[John Keeping]

From: Jeff King 

When fast-import emits a crash report, it does so in the
user's local timezone. But because we omit the timezone
completely for DATE_LOCAL, a reader of the report does not
immediately know which time zone was used. Let's switch this
to ISO8601 instead, which includes the time zone.

This does mean we will show the time in UTC, but that's not
a big deal. A crash report like this will either be looked
at immediately (in which case nobody even looks at the
timestamp), or it will be passed along to a developer to
debug, in which case the original timezone is less likely to
be of interest.

Signed-off-by: Jeff King 
Signed-off-by: John Keeping 
---
 fast-import.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fast-import.c b/fast-import.c
index 6c7c3c9..adcbfc6 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -424,7 +424,7 @@ static void write_crash_report(const char *err)
fprintf(rpt, "fast-import crash report:\n");
fprintf(rpt, "fast-import process: %"PRIuMAX"\n", (uintmax_t) 
getpid());
fprintf(rpt, "parent process : %"PRIuMAX"\n", (uintmax_t) 
getppid());
-   fprintf(rpt, "at %s\n", show_date(time(NULL), 0, DATE_MODE(LOCAL)));
+   fprintf(rpt, "at %s\n", show_date(time(NULL), 0, 
DATE_MODE(ISO8601)));
fputc('\n', rpt);
 
fputs("fatal: ", rpt);
-- 
2.5.0.466.g9af26fa

--
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 v3 04/11] Documentation/rev-list: don't list date formats

[John Keeping]

We are about to add several new date formats which will make this list
too long to display in a single line.

Signed-off-by: John Keeping 
---
 Documentation/git-rev-list.txt | 2 +-
 Documentation/rev-list-options.txt | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 7b49c85..ef22f17 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -45,7 +45,7 @@ SYNOPSIS
 [ --regexp-ignore-case | -i ]
 [ --extended-regexp | -E ]
 [ --fixed-strings | -F ]
-[ --date=(local|relative|default|iso|iso-strict|rfc|short) ]
+[ --date=]
 [ [ --objects | --objects-edge | --objects-edge-aggressive ]
   [ --unpacked ] ]
 [ --pretty | --header ]
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index a9b808f..14c4cce 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -699,7 +699,7 @@ include::pretty-options.txt[]
 --relative-date::
Synonym for `--date=relative`.
 
---date=(relative|local|default|iso|iso-strict|rfc|short|raw)::
+--date=::
Only takes effect for dates shown in human-readable format, such
as when using `--pretty`. `log.date` config variable sets a default
value for the log command's `--date` option.
-- 
2.5.0.466.g9af26fa

--
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 v3 07/11] t6300: add test for "raw" date format

[John Keeping]

Signed-off-by: John Keeping 
---
 t/t6300-for-each-ref.sh | 4 
 1 file changed, 4 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 0bf709b..6afcff6 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -215,6 +215,10 @@ test_expect_success 'Check format "rfc2822" date fields 
output' '
"Mon, 3 Jul 2006 17:18:45 +0200"
 '
 
+test_expect_success 'Check format "raw" date fields output' '
+   test_date raw "1151939923 +0200" "1151939924 +0200" "1151939925 +0200"
+'
+
 test_expect_success 'Check format of strftime date fields' '
echo "my date is 2006-07-03" >expected &&
git for-each-ref \
-- 
2.5.0.466.g9af26fa

--
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 v3 06/11] t6300: introduce test_date() helper

[John Keeping]

This moves the setup of the "expected" file inside the test case.  The
helper function has the advantage that we can use SQ in the file content
without needing to escape the quotes.

Signed-off-by: John Keeping 
---
Changes since v2:
- add missing "&&" after "f=$1"
- wrap long lines

 t/t6300-for-each-ref.sh | 92 +
 1 file changed, 40 insertions(+), 52 deletions(-)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7c9bec7..0bf709b 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -146,85 +146,73 @@ test_expect_success 'Check invalid format specifiers are 
errors' '
test_must_fail git for-each-ref --format="%(authordate:INVALID)" 
refs/heads
 '
 
-cat >expected <<\EOF
-'refs/heads/master' 'Mon Jul 3 17:18:43 2006 +0200' 'Mon Jul 3 17:18:44 2006 
+0200'
-'refs/tags/testtag' 'Mon Jul 3 17:18:45 2006 +0200'
-EOF
+test_date () {
+   f=$1 &&
+   committer_date=$2 &&
+   author_date=$3 &&
+   tagger_date=$4 &&
+   cat >expected <<-EOF &&
+   'refs/heads/master' '$committer_date' '$author_date'
+   'refs/tags/testtag' '$tagger_date'
+   EOF
+   (
+   git for-each-ref --shell \
+   --format="%(refname) %(committerdate${f:+:$f}) 
%(authordate${f:+:$f})" \
+   refs/heads &&
+   git for-each-ref --shell \
+   --format="%(refname) %(taggerdate${f:+:$f})" \
+   refs/tags
+   ) >actual &&
+   test_cmp expected actual
+}
 
 test_expect_success 'Check unformatted date fields output' '
-   (git for-each-ref --shell --format="%(refname) %(committerdate) 
%(authordate)" refs/heads &&
-   git for-each-ref --shell --format="%(refname) %(taggerdate)" refs/tags) 
>actual &&
-   test_cmp expected actual
+   test_date "" \
+   "Mon Jul 3 17:18:43 2006 +0200" \
+   "Mon Jul 3 17:18:44 2006 +0200" \
+   "Mon Jul 3 17:18:45 2006 +0200"
 '
 
 test_expect_success 'Check format "default" formatted date fields output' '
-   f=default &&
-   (git for-each-ref --shell --format="%(refname) %(committerdate:$f) 
%(authordate:$f)" refs/heads &&
-   git for-each-ref --shell --format="%(refname) %(taggerdate:$f)" 
refs/tags) >actual &&
-   test_cmp expected actual
+   test_date default \
+   "Mon Jul 3 17:18:43 2006 +0200" \
+   "Mon Jul 3 17:18:44 2006 +0200" \
+   "Mon Jul 3 17:18:45 2006 +0200"
 '
 
 # Don't know how to do relative check because I can't know when this script
 # is going to be run and can't fake the current time to git, and hence can't
 # provide expected output.  Instead, I'll just make sure that "relative"
 # doesn't exit in error
-#
-#cat >expected <<\EOF
-#
-#EOF
-#
 test_expect_success 'Check format "relative" date fields output' '
f=relative &&
(git for-each-ref --shell --format="%(refname) %(committerdate:$f) 
%(authordate:$f)" refs/heads &&
git for-each-ref --shell --format="%(refname) %(taggerdate:$f)" 
refs/tags) >actual
 '
 
-cat >expected <<\EOF
-'refs/heads/master' '2006-07-03' '2006-07-03'
-'refs/tags/testtag' '2006-07-03'
-EOF
-
 test_expect_success 'Check format "short" date fields output' '
-   f=short &&
-   (git for-each-ref --shell --format="%(refname) %(committerdate:$f) 
%(authordate:$f)" refs/heads &&
-   git for-each-ref --shell --format="%(refname) %(taggerdate:$f)" 
refs/tags) >actual &&
-   test_cmp expected actual
+   test_date short 2006-07-03 2006-07-03 2006-07-03
 '
 
-cat >expected <<\EOF
-'refs/heads/master' 'Mon Jul 3 15:18:43 2006' 'Mon Jul 3 15:18:44 2006'
-'refs/tags/testtag' 'Mon Jul 3 15:18:45 2006'
-EOF
-
 test_expect_success 'Check format "local" date fields output' '
-   f=local &&
-   (git for-each-ref --shell --format="%(refname) %(committerdate:$f) 
%(authordate:$f)" refs/heads &&
-   git for-each-ref --shell --format="%(refname) %(taggerdate:$f)" 
refs/tags) >actual &&
-   test_cmp expected actual
+   test_date local \
+   "Mon Jul 3 15:18:43 2006" \
+   "Mon Jul 3 15:18:44 2006" \
+   "Mon Jul 3 15:18:45 2006"
 '
 
-cat >expected <<\EOF
-'refs/heads/master' '2006-07-03 17:18:43 +0200' '2006-07-03 17:18:44 +0200'
-'refs/tags/testtag' '2006-07-03 17:18:45 +0200'
-EOF
-
 test_expect_success 'Check format "iso8601" date fields output' '
-   f=iso8601 &&
-   (git for-each-ref --shell --format="%(refname) %(committerdate:$f) 
%(authordate:$f)" refs/heads &&
-   git for-each-ref --shell --format="%(refname) %(taggerdate:$f)" 
refs/tags) >actual &&
-   test_cmp expected actual
+   test_date iso8601 \
+   "2006-07-03 17:18:43 +0200" \
+   "2006-07-03 17:18:44 +0200" \
+   "2006-07-03 17:18:45 +0200"
 '
 
-cat >expected <<\EOF
-'refs/heads/master' 'Mon, 3 Jul 2006 17:18:43 

[PATCH v3 09/11] date: make "local" orthogonal to date format

[John Keeping]

From: Jeff King 

Most of our "--date" modes are about the format of the date:
which items we show and in what order. But "--date=local" is
a bit of an oddball. It means "show the date in the normal
format, but using the local timezone". The timezone we use
is orthogonal to the actual format, and there is no reason
we could not have "localized iso8601", etc.

This patch adds a "local" boolean field to "struct
date_mode", and drops the DATE_LOCAL element from the
date_mode_type enum (it's now just DATE_NORMAL plus
local=1). The new feature is accessible to users by adding
"-local" to any date mode (e.g., "iso-local"), and we retain
"local" as an alias for "default-local" for backwards
compatibility.

Signed-off-by: Jeff King 
Signed-off-by: John Keeping 
---
Changes since v2:
- "local" check has moved above DATE_RAW processing as a result of an
  earlier patch
- "relative-local" and "raw-local" are now allowed
- the error message if the format starts with a valid sequence but is
  invalid as a whole is now consistent with the case where there is no
  valid prefix

 Documentation/rev-list-options.txt | 21 
 builtin/blame.c|  1 -
 cache.h|  2 +-
 date.c | 70 --
 4 files changed, 61 insertions(+), 33 deletions(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 14c4cce..359587c 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -702,12 +702,16 @@ include::pretty-options.txt[]
 --date=::
Only takes effect for dates shown in human-readable format, such
as when using `--pretty`. `log.date` config variable sets a default
-   value for the log command's `--date` option.
+   value for the log command's `--date` option. By default, dates
+   are shown in the original time zone (either committer's or
+   author's). If `-local` is appended to the format (e.g.,
+   `iso-local`), the user's local time zone is used instead.
 +
 `--date=relative` shows dates relative to the current time,
-e.g. ``2 hours ago''.
+e.g. ``2 hours ago''. The `-local` option cannot be used with
+`--raw` or `--relative`.
 +
-`--date=local` shows timestamps in user's local time zone.
+`--date=local` is an alias for `--date=default-local`.
 +
 `--date=iso` (or `--date=iso8601`) shows timestamps in a ISO 8601-like format.
 The differences to the strict ISO 8601 format are:
@@ -730,10 +734,15 @@ format, often found in email messages.
 `--date=format:...` feeds the format `...` to your system `strftime`.
 Use `--date=format:%c` to show the date in your system locale's
 preferred format.  See the `strftime` manual for a complete list of
-format placeholders.
+format placeholders. When using `-local`, the correct syntax is
+`--date=format-local:...`.
 +
-`--date=default` shows timestamps in the original time zone
-(either committer's or author's).
+`--date=default` is the default format, and is similar to
+`--date=rfc2822`, with a few exceptions:
+
+   - there is no comma after the day-of-week
+
+   - the time zone is omitted when the local time zone is used
 
 ifdef::git-rev-list[]
 --header::
diff --git a/builtin/blame.c b/builtin/blame.c
index 4db01c1..6fd1a63 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2600,7 +2600,6 @@ parse_done:
   fewer display columns. */
blame_date_width = utf8_strwidth(_("4 years, 11 months ago")) + 
1; /* add the null */
break;
-   case DATE_LOCAL:
case DATE_NORMAL:
blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
break;
diff --git a/cache.h b/cache.h
index 4e25271..9a91b1d 100644
--- a/cache.h
+++ b/cache.h
@@ -1091,7 +1091,6 @@ struct date_mode {
DATE_NORMAL = 0,
DATE_RELATIVE,
DATE_SHORT,
-   DATE_LOCAL,
DATE_ISO8601,
DATE_ISO8601_STRICT,
DATE_RFC2822,
@@ -1099,6 +1098,7 @@ struct date_mode {
DATE_RAW
} type;
const char *strftime_fmt;
+   int local;
 };
 
 /*
diff --git a/date.c b/date.c
index 9f0a5dd..7c9f769 100644
--- a/date.c
+++ b/date.c
@@ -166,6 +166,7 @@ struct date_mode *date_mode_from_type(enum date_mode_type 
type)
if (type == DATE_STRFTIME)
die("BUG: cannot create anonymous strftime date_mode struct");
mode.type = type;
+   mode.local = 0;
return 
 }
 
@@ -174,7 +175,7 @@ const char *show_date(unsigned long time, int tz, const 
struct date_mode *mode)
struct tm *tm;
static struct strbuf timebuf = STRBUF_INIT;
 
-   if (mode->type == DATE_LOCAL)
+   if (mode->local)
tz = local_tzoffset(time);
 
if (mode->type == DATE_RAW) {
@@ -232,7 +233,7 @@ const char 

[PATCH v3 08/11] date: check for "local" before anything else

[John Keeping]

In a following commit we will make "local" orthogonal to the format.
Although this will not apply to "relative", which does not use the
timezone, it applies to all other formats so move the timezone
conversion to the start of the function.

Signed-off-by: John Keeping 
---
 date.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/date.c b/date.c
index 8f91569..9f0a5dd 100644
--- a/date.c
+++ b/date.c
@@ -174,6 +174,9 @@ const char *show_date(unsigned long time, int tz, const 
struct date_mode *mode)
struct tm *tm;
static struct strbuf timebuf = STRBUF_INIT;
 
+   if (mode->type == DATE_LOCAL)
+   tz = local_tzoffset(time);
+
if (mode->type == DATE_RAW) {
strbuf_reset();
strbuf_addf(, "%lu %+05d", time, tz);
@@ -189,9 +192,6 @@ const char *show_date(unsigned long time, int tz, const 
struct date_mode *mode)
return timebuf.buf;
}
 
-   if (mode->type == DATE_LOCAL)
-   tz = local_tzoffset(time);
-
tm = time_to_tm(time, tz);
if (!tm) {
tm = time_to_tm(0, 0);
-- 
2.5.0.466.g9af26fa

--
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: [PATCHv5 3/3] submodule: Reimplement `module_clone` shell function in C

[Junio C Hamano]

Stefan Beller  writes:

> @@ -119,6 +120,140 @@ static int module_name(int argc, const char **argv, 
> const char *prefix)
>  
>   return 0;
>  }
> +static int clone_submodule(const char *path, const char *gitdir, const char 
> *url,
> +const char *depth, const char *reference, int quiet)
> +{
> + struct child_process cp;
> + child_process_init();
> +
> + argv_array_push(, "clone");
> + argv_array_push(, "--no-checkout");
> + if (quiet)
> + argv_array_push(, "--quiet");
> + if (depth && *depth)
> + argv_array_pushl(, "--depth", depth, NULL);
> + if (reference && *reference)
> + argv_array_pushl(, "--reference", reference, NULL);
> + if (gitdir && *gitdir)
> + argv_array_pushl(, "--separate-git-dir", gitdir, NULL);
> +
> + argv_array_push(, url);
> + argv_array_push(, path);
> +
> + cp.git_cmd = 1;
> + cp.env = local_repo_env;
> +
> + cp.no_stdin = 1;
> + cp.no_stdout = 1;
> + cp.no_stderr = 1;

Output from "git clone" is not shown, regardless of --quiet option?

> + return run_command();
> +}
> +
> +static int module_clone(int argc, const char **argv, const char *prefix)
> +{
> + const char *path = NULL, *name = NULL, *url = NULL;
> + const char *reference = NULL, *depth = NULL;
> + int quiet = 0;
> + FILE *submodule_dot_git;
> + char *sm_gitdir, *cwd, *p;
> + struct strbuf rel_path = STRBUF_INIT;
> + struct strbuf sb = STRBUF_INIT;
> +
> + struct option module_clone_options[] = {
> + OPT_STRING(0, "prefix", ,
> +N_("path"),
> +N_("alternative anchor for relative paths")),
> + OPT_STRING(0, "path", ,
> +N_("path"),
> +N_("where the new submodule will be cloned to")),
> + OPT_STRING(0, "name", ,
> +N_("string"),
> +N_("name of the new submodule")),
> + OPT_STRING(0, "url", ,
> +N_("string"),
> +N_("url where to clone the submodule from")),
> + OPT_STRING(0, "reference", ,
> +N_("string"),
> +N_("reference repository")),
> + OPT_STRING(0, "depth", ,
> +N_("string"),
> +N_("depth for shallow clones")),
> + OPT__QUIET(, "Suppress output for cloning a submodule"),
> + OPT_END()
> + };
> +
> + const char *const git_submodule_helper_usage[] = {
> + N_("git submodule--helper clone [--prefix=] [--quiet] "
> +"[--reference ] [--name ] [--url ]"
> +"[--depth ] [--] [...]"),
> + NULL
> + };
> +
> + argc = parse_options(argc, argv, prefix, module_clone_options,
> +  git_submodule_helper_usage, 0);
> +
> + strbuf_addf(, "%s/modules/%s", get_git_dir(), name);

The original says

base_name=$(dirname "$name")

before doing this %s/modules/%s concatenation.  I do not think we
intended to allow a slash in submodule name, so this difference may
be showing that the original was doing an unnecessary thing.

> + sm_gitdir = strbuf_detach(, NULL);
> +
> + if (!file_exists(sm_gitdir)) {
> + if (safe_create_leading_directories_const(sm_gitdir) < 0)
> + die(_("could not create directory '%s'"), sm_gitdir);
> + if (clone_submodule(path, sm_gitdir, url, depth, reference, 
> quiet))
> + die(_("clone of '%s' into submodule path '%s' failed"),
> + url, path);
> + } else {
> + if (safe_create_leading_directories_const(path) < 0)
> + die(_("could not create directory '%s'"), path);
> + strbuf_addf(, "%s/index", sm_gitdir);
> + if (unlink(sb.buf) < 0)
> + die_errno(_("failed to delete '%s'"), sm_gitdir);

The original says "we do not care if it failed" with

rm -f "$gitdir/index"

I think the intention of the original is "we do not care if it
failed because it did not exist." in which case unconditional
die_errno() here may be something we do not want?

> + strbuf_reset();
> + }
> +
> + /* Write a .git file in the submodule to redirect to the superproject. 
> */
> + if (safe_create_leading_directories_const(path) < 0)
> + die(_("could not create directory '%s'"), path);
> +
> + if (path && *path)
> + strbuf_addf(, "%s/.git", path);
> + else
> + strbuf_addf(, ".git");
> +
> + if (safe_create_leading_directories_const(sb.buf) < 0)
> + die(_("could not create leading directories of '%s'"), sb.buf);
> + submodule_dot_git = fopen(sb.buf, "w");
> + if (!submodule_dot_git)
> + 

Re: [PATCH] graph.c: visual difference on subsequent series

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 27.07.2015 22:17:
> Antoine Beaupré  writes:
> 
>> Any reason why this patch wasn't included / reviewed?
>> ...
>>> This patch is similar than the one provided by Milton Soares Filho in
>>> 1382734287.31768.1.git.send.email.milton.soares.fi...@gmail.com but was
>>> implemented independently and uses the 'o' character instead of 'x'.
> 
> The reason why Milton's patch was not taken after discussion [*1*]
> was not because its implementation was poor, but its design was not
> good.  By overriding '*' '<' or '>' with x, it made it impossible to
> distinguish a root on the left side branch and a root on the right
> side branch.
> 
> Is the design of your independent implementation the same except
> that 'o' is used instead of 'x'?  Independent implementation does
> not make the same design magically better, if that is the case ;-)

Interestingly, the patch to the tests lists * to o changes only, no < or
> to o.

The reason is simply that the patch doesn't change anything for left nor
right commits. I would say that is the best compromise since it does not
change the overall layout, provides more information by default and does
not override information that is requested specifically.

If we want to put more information into log --graph simultaneously we
should really go beyond ASCII and look at how tig does it, e.g. using
unicode characters.

> If the design is different, please explain how your patch solves the
> issue with Milton's design in your log message.
> 
> For example, you could use the column arrangement to solve it, e.g.
> 
> History sequence A: a1 -- a2 -- a3 (root-commit)
> History sequence B: b1 -- b2 -- b3 (root-commit)
> 
> $ git log --graph --oneline A B
> * a1
> * a2
> * a3
>   * b1
>   * b2
>   * b3
> 
> $ git log --graph --oneline --left-right A...B
> < a1
> < a2
> < a3
>   > b1
>   > b2
>   > b3
> 
> I am not saying that the above would be the only way to do so; there
> may be other ways to solve the issue.
> 
> [Reference]
> 
> *1* http://thread.gmane.org/gmane.comp.version-control.git/236708/focus=236843
> 

--
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 v6] git-p4: add config git-p4.pathEncoding

[larsxschneider]

From: Lars Schneider 

Diff to v5:
* use "test_config" (Thanks Remi! I am still learning all the tools...)
* removed whitespaces (Thanks Luke! I added this to my "generate patch" script. 
Won't happen again :-)
* added ACK from Luke (I interpreted "Looks good to me" that way. I hope this 
is OK.)

Thanks,
Lars

Lars Schneider (1):
  git-p4: add config git-p4.pathEncoding

 Documentation/git-p4.txt|  7 +
 git-p4.py   | 11 
 t/t9822-git-p4-path-encoding.sh | 60 +
 3 files changed, 78 insertions(+)
 create mode 100755 t/t9822-git-p4-path-encoding.sh

--
1.9.5 (Apple Git-50.3)

--
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: Git Deployment Multiple Existing Environments

[Thomas Koch]

On Thursday, September 03, 2015 09:11:10 AM Sukhwinder Singh wrote:
> Hello,
> 
> I have posted this question on various forums as well but I have not
> received any guidance yet. I was requesting some steps which I can follow.
> Our Servers are windows based.

Don't use windows. Get rid of people who think windows is a server operating 
system.
--
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] pack-protocol: clarify LF-handling in PKT-LINE()

[Jeff King]

On Thu, Sep 03, 2015 at 04:24:09AM -0400, Jeff King wrote:

> diff --git a/Documentation/technical/protocol-common.txt 
> b/Documentation/technical/protocol-common.txt
> index 889985f..bf30167 100644
> --- a/Documentation/technical/protocol-common.txt
> +++ b/Documentation/technical/protocol-common.txt
> @@ -62,7 +62,10 @@ A pkt-line MAY contain binary data, so implementors MUST 
> ensure
>  pkt-line parsing/formatting routines are 8-bit clean.
>  
>  A non-binary line SHOULD BE terminated by an LF, which if present
> -MUST be included in the total length.
> +MUST be included in the total length. Receivers MUST treat pkt-lines
> +with non-binary data the same whether or not they contain the trailing
> +LF (stripping the LF if present, and not complaining when it is
> +missing).

I noticed that this section leaves vague the question of "what is a
binary pkt-line". All of the PKT-LINE calls I tweaked in the grammar are
for non-binary instances. The only binary ones are described in the
sideband discussion of the "Packfile data" section, and there we do not
have a grammar.

So I think the result is reasonably clear, but I have far too much
intimate knowledge of git's protocol to be a good judge.

-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 v5] git-p4: add config git-p4.pathEncoding

[Remi Galan Alfonso]

Hi Lars,

Lars Schneider  writes:

> +test_expect_success 'Clone repo containing iso8859-1 encoded paths with 
> git-p4.pathEncoding' '
> +
> +test_when_finished cleanup_git &&
> +(
> +cd "$git" &&
> +git init . &&
> +git config git-p4.pathEncoding iso8859-1 &&

Wouldn't 'test_config' be better here?

Thanks,
Rémi
--
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] pack-protocol: document newline behavior in push commands

[Jeff King]

Our pack-protocol spec indicates that a pushing client
should send ref update commands like:

  $old_sha1 $new_sha1 $ref\n

with each ref update in its own pktline, with a trailing
newline. However, git itself does not follow this behavior;
it omits the trailing newline.

For the most part the distinction is harmless. The
`receive-pack` on the other end will call
`packet_read_line`, which strips off the newline if it is
there, and we are fine either way.

Things are more complicated for the initial ref, which also
has capabilities. The spec says to send:

  $old_sha1 $new_sha1 $ref\0 $caps\n

which is also OK by the current `receive-pack code (the
newline is at the end of the packet, so we still strip it).

As an aside, it would _not_ be OK to send:

  $old_sha1 $new_sha1 $ref\n\0 $caps\n

The spec does not allow that and receive-pack would reject
it, but it is perhaps a mistake that a naive client
implementation might make.

So what is in the current spec is quite reasonable, and
simply follows the normal rules for pkt-line framing (we say
LF, but it really is optional). But it does not document
how git actually behaves.

Signed-off-by: Jeff King 
---
+cc Junio, Dave, and Shawn, as this is somewhat related to the
discussion in
http://thread.gmane.org/gmane.comp.version-control.git/273175/focus=273444

I happened to be looking into some protocol stuff recently and noticed
that we do not follow the spec here (but interestingly, libgit2 does!).

Frankly, this feels a bit like a step backwards to me. I am tempted to
suggest instead that git start sending the newlines, but I'm not sure
it's worth any potential fallout.

I'm also tempted to scrap this and say it just falls under the rule
that every PKT-LINE is "sender SHOULD include LF" and "receiver MUST NOT
complain about missing LF" (which does appear earlier in the document,
though in a different context). Or maybe we should write out that rule
explicitly and drop the "LF" from all parts of the grammar (which is
what the thread above advocates).

 Documentation/technical/pack-protocol.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index 4064fc7..9ce53b4 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -469,8 +469,8 @@ references.
 
   shallow   =  PKT-LINE("shallow" SP obj-id LF)
 
-  command-list  =  PKT-LINE(command NUL capability-list LF)
-  *PKT-LINE(command LF)
+  command-list  =  PKT-LINE(command NUL capability-list)
+  *PKT-LINE(command)
   flush-pkt
 
   command   =  create / delete / update
@@ -586,8 +586,8 @@ An example client/server communication might look like this:
S: 003f74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/team\n
S: 
 
-   C: 003e7d1665144a3a975c05f1f43902ddaf084e784dbe 
74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/debug\n
-   C: 003e74730d410fcb6603ace96f1dc55ea6196122532d 
5a3f6be755bbb7deae50065988cbfa1ffa9ab68a refs/heads/master\n
+   C: 003e7d1665144a3a975c05f1f43902ddaf084e784dbe 
74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/debug
+   C: 003e74730d410fcb6603ace96f1dc55ea6196122532d 
5a3f6be755bbb7deae50065988cbfa1ffa9ab68a refs/heads/master
C: 
C: [PACKDATA]
 
-- 
2.5.1.812.ge796bff
--
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] pack-protocol: clarify LF-handling in PKT-LINE()

[Jeff King]

On Thu, Sep 03, 2015 at 04:02:05AM -0400, Jeff King wrote:

> I'm also tempted to scrap this and say it just falls under the rule
> that every PKT-LINE is "sender SHOULD include LF" and "receiver MUST NOT
> complain about missing LF" (which does appear earlier in the document,
> though in a different context). Or maybe we should write out that rule
> explicitly and drop the "LF" from all parts of the grammar (which is
> what the thread above advocates).

Hmph, I just noticed that patch 2/7 from Dave's original series is
essentially the same as what I just sent.

Since the discussion seemed to end up in this "let's make PKT-LINE more
clear" direction, here is an attempt to do that.

-- >8 --
Subject: pack-protocol: clarify LF-handling in PKT-LINE()

The spec is very inconsistent about which PKT-LINE() parts
of the grammar include a LF. On top of that, the code is not
consistent, either (e.g., send-pack does not put newlines
into the ref-update commands it sends).

Let's make explicit the long-standing expectation that we
generally expect pkt-lines to end in a newline, but that
receivers should be lenient. This makes the spec consistent,
and matches what git already does (though it does not always
fulfill the SHOULD).

We do make an exception for the push-cert, where the
receiving code is currently a bit pickier. This is a
reasonable way to be, as the data needs to be byte-for-byte
compatible with what was signed. We _could_ make up some
rules about signing a canonicalized version including
newlines, but that would require a code change, and is out
of scope for this patch.

Signed-off-by: Jeff King 
---
 Documentation/technical/pack-protocol.txt   | 46 -
 Documentation/technical/protocol-common.txt |  5 +++-
 2 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index 4064fc7..c6977bb 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -14,6 +14,14 @@ data.  The protocol functions to have a server tell a client 
what is
 currently on the server, then for the two to negotiate the smallest amount
 of data to send in order to fully update one or the other.
 
+pkt-line Format
+---
+
+The descriptions below build on the pkt-line format described in
+protocol-common.txt. When the grammar indicate `PKT-LINE(...)`, unless
+otherwise noted the usual pkt-line LF rules apply: the sender SHOULD
+include a LF, but the receiver MUST NOT complain if it is not present.
+
 Transports
 --
 There are three transports over which the packfile protocol is
@@ -143,9 +151,6 @@ with the object name that each reference currently points 
to.
003fe92df48743b7bc7d26bcaabfddde0a1e20cae47c refs/tags/v1.0^{}

 
-Server SHOULD terminate each non-flush line using LF ("\n") terminator;
-client MUST NOT complain if there is no terminator.
-
 The returned response is a pkt-line stream describing each ref and
 its current value.  The stream MUST be sorted by name according to
 the C locale ordering.
@@ -165,15 +170,15 @@ MUST peel the ref if it's an annotated tag.
  flush-pkt
 
   no-refs  =  PKT-LINE(zero-id SP "capabilities^{}"
- NUL capability-list LF)
+ NUL capability-list)
 
   list-of-refs =  first-ref *other-ref
   first-ref=  PKT-LINE(obj-id SP refname
- NUL capability-list LF)
+ NUL capability-list)
 
   other-ref=  PKT-LINE(other-tip / other-peeled)
-  other-tip=  obj-id SP refname LF
-  other-peeled =  obj-id SP refname "^{}" LF
+  other-tip=  obj-id SP refname
+  other-peeled =  obj-id SP refname "^{}"
 
   shallow  =  PKT-LINE("shallow" SP obj-id)
 
@@ -216,8 +221,8 @@ out of what the server said it could do with the first 
'want' line.
 
   depth-request =  PKT-LINE("deepen" SP depth)
 
-  first-want=  PKT-LINE("want" SP obj-id SP capability-list LF)
-  additional-want   =  PKT-LINE("want" SP obj-id LF)
+  first-want=  PKT-LINE("want" SP obj-id SP capability-list)
+  additional-want   =  PKT-LINE("want" SP obj-id)
 
   depth =  1*DIGIT
 
@@ -284,7 +289,7 @@ so that there is always a block of 32 "in-flight on the 
wire" at a time.
   compute-end
 
   have-list =  *have-line
-  have-line =  PKT-LINE("have" SP obj-id LF)
+  have-line =  PKT-LINE("have" SP obj-id)
   compute-end   =  flush-pkt / PKT-LINE("done")
 
 
@@ -348,10 +353,10 @@ Then the server will start sending its packfile data.
 
 
   server-response = *ack_multi ack / nak
-  ack_multi   = PKT-LINE("ACK" SP obj-id ack_status LF)
+  ack_multi   = PKT-LINE("ACK" SP obj-id ack_status)
   ack_status  = "continue" / "common" / "ready"
-  ack = PKT-LINE("ACK SP obj-id LF)
-  nak   

[PATCH v1] t9821: use test_config

[larsxschneider]

From: Lars Schneider 

Signed-off-by: Lars Schneider 
---
 t/t9821-git-p4-path-variations.sh | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/t/t9821-git-p4-path-variations.sh 
b/t/t9821-git-p4-path-variations.sh
index 81e46ac..5a26fec 100755
--- a/t/t9821-git-p4-path-variations.sh
+++ b/t/t9821-git-p4-path-variations.sh
@@ -45,7 +45,7 @@ test_expect_success 'Clone root' '
(
cd "$git" &&
git init . &&
-   git config core.ignorecase false &&
+   test_config core.ignorecase false &&
git p4 clone --use-client-spec --destination="$git" //depot &&
# This method is used instead of "test -f" to ensure the case is
# checked even if the test is executed on case-insensitive file 
systems.
@@ -67,7 +67,7 @@ test_expect_success 'Clone root (ignorecase)' '
(
cd "$git" &&
git init . &&
-   git config core.ignorecase true &&
+   test_config core.ignorecase true &&
git p4 clone --use-client-spec --destination="$git" //depot &&
# This method is used instead of "test -f" to ensure the case is
# checked even if the test is executed on case-insensitive file 
systems.
@@ -91,7 +91,7 @@ test_expect_success 'Clone root and ignore one file' '
(
cd "$git" &&
git init . &&
-   git config core.ignorecase false &&
+   test_config core.ignorecase false &&
git p4 clone --use-client-spec --destination="$git" //depot &&
# We ignore one file in the client spec and all path cases 
change from
# "TO" to "to"!
@@ -113,7 +113,7 @@ test_expect_success 'Clone root and ignore one file 
(ignorecase)' '
(
cd "$git" &&
git init . &&
-   git config core.ignorecase true &&
+   test_config core.ignorecase true &&
git p4 clone --use-client-spec --destination="$git" //depot &&
# We ignore one file in the client spec and all path cases 
change from
# "TO" to "to"!
@@ -133,7 +133,7 @@ test_expect_success 'Clone path' '
(
cd "$git" &&
git init . &&
-   git config core.ignorecase false &&
+   test_config core.ignorecase false &&
git p4 clone --use-client-spec --destination="$git" //depot &&
cat >expect <<-\EOF &&
to/File2.txt
@@ -149,7 +149,7 @@ test_expect_success 'Clone path (ignorecase)' '
(
cd "$git" &&
git init . &&
-   git config core.ignorecase true &&
+   test_config core.ignorecase true &&
git p4 clone --use-client-spec --destination="$git" //depot &&
cat >expect <<-\EOF &&
TO/File2.txt
@@ -180,7 +180,7 @@ test_expect_success 'Add a new file and clone path with new 
file (ignorecase)' '
(
cd "$git" &&
git init . &&
-   git config core.ignorecase true &&
+   test_config core.ignorecase true &&
git p4 clone --use-client-spec --destination="$git" //depot &&
cat >expect <<-\EOF &&
to/File0.txt
--
1.9.5 (Apple Git-50.3)

--
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 v1] t9821: use test_config

[larsxschneider]

From: Lars Schneider 

Thanks Remi for pointing out test_config to me!

Cheers,
Lars

Lars Schneider (1):
  t9821: use test_config

 t/t9821-git-p4-path-variations.sh | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

--
1.9.5 (Apple Git-50.3)

--
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: git dockerfile.

[Atul Sowani]

Hello!

Any help/pointers/advise regarding my request about dockerfile?

Thanks,
Atul.

On Tue, Sep 1, 2015 at 3:13 PM, Atul Sowani  wrote:
> Hi,
>
> Greetings everybody!
>
> I am looking for a Dockerfile for git which will _build_ git from source on
> ppc64le platform. I want to build git with different versions (say
> top-of-the-tree, latest-stable etc.) and it would be good if there is a
> dockerfile present alongwith the code which can build the source
> code with minimal changes (or better if there is a dockerfile
> available for each released version). This will avoid many incoherent
> copies of dockerfile floating around the Net.
>
> I tried to search on Internet but haven't come across any dockerfile
> which will build git source. I would highly appreciate if somebody
> could point me to a source where such dockerfile is available.
>
> I saw some communication on git mailing list, but so far haven't found
> any docker file. If there is no dockerfile available, I am willing to create
> one for ppc64le and commit it to git source. Please advise.
>
> Thanks,
> Atul.
--
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 v6] git-p4: add config git-p4.pathEncoding

[larsxschneider]

From: Lars Schneider 

Perforce keeps the encoding of a path as given by the originating OS.
Git expects paths encoded as UTF-8. Add a config to tell git-p4 what
encoding Perforce had used for the paths. This encoding is used to
transcode the paths to UTF-8. As an example, Perforce on Windows often
uses “cp1252” to encode path names.

Signed-off-by: Lars Schneider 
Acked-by: Luke Diamand 
Signed-off-by: Lars Schneider 
---
 Documentation/git-p4.txt|  7 +
 git-p4.py   | 11 
 t/t9822-git-p4-path-encoding.sh | 60 +
 3 files changed, 78 insertions(+)
 create mode 100755 t/t9822-git-p4-path-encoding.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 82aa5d6..12a57d4 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -510,6 +510,13 @@ git-p4.useClientSpec::
option '--use-client-spec'.  See the "CLIENT SPEC" section above.
This variable is a boolean, not the name of a p4 client.
 
+git-p4.pathEncoding::
+   Perforce keeps the encoding of a path as given by the originating OS.
+   Git expects paths encoded as UTF-8. Use this config to tell git-p4
+   what encoding Perforce had used for the paths. This encoding is used
+   to transcode the paths to UTF-8. As an example, Perforce on Windows
+   often uses “cp1252” to encode path names.
+
 Submit variables
 
 git-p4.detectRenames::
diff --git a/git-p4.py b/git-p4.py
index 073f87b..b1ad86d 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2213,6 +2213,17 @@ class P4Sync(Command, P4UserMap):
 text = regexp.sub(r'$\1$', text)
 contents = [ text ]
 
+if gitConfig("git-p4.pathEncoding"):
+relPath = 
relPath.decode(gitConfig("git-p4.pathEncoding")).encode('utf8', 'replace')
+elif self.verbose:
+try:
+relPath.decode('ascii')
+except:
+print (
+"Path with Non-ASCII characters detected and no path 
encoding defined. "
+"Please check the encoding: %s" % relPath
+)
+
 self.gitStream.write("M %s inline %s\n" % (git_mode, relPath))
 
 # total length...
diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
new file mode 100755
index 000..e507ad7
--- /dev/null
+++ b/t/t9822-git-p4-path-encoding.sh
@@ -0,0 +1,60 @@
+#!/bin/sh
+
+test_description='Clone repositories with non ASCII paths'
+
+. ./lib-git-p4.sh
+
+UTF8_ESCAPED="a-\303\244_o-\303\266_u-\303\274.txt"
+ISO8859_ESCAPED="a-\344_o-\366_u-\374.txt"
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+test_expect_success 'Create a repo containing iso8859-1 encoded paths' '
+   (
+   cd "$cli" &&
+   ISO8859="$(printf "$ISO8859_ESCAPED")" &&
+   echo content123 >"$ISO8859" &&
+   p4 add "$ISO8859" &&
+   p4 submit -d "test commit"
+   )
+'
+
+test_expect_success 'Clone repo containing iso8859-1 encoded paths without 
git-p4.pathEncoding' '
+   git p4 clone --destination="$git" //depot &&
+   test_when_finished cleanup_git &&
+   (
+   cd "$git" &&
+   UTF8="$(printf "$UTF8_ESCAPED")" &&
+   echo $UTF8 >expect &&
+   git -c core.quotepath=false ls-files >actual &&
+   test_must_fail test_cmp expect actual
+   )
+'
+
+test_expect_success 'Clone repo containing iso8859-1 encoded paths with 
git-p4.pathEncoding' '
+
+   test_when_finished cleanup_git &&
+   (
+   cd "$git" &&
+   git init . &&
+   test_config git-p4.pathEncoding iso8859-1 &&
+   git p4 clone --use-client-spec --destination="$git" //depot &&
+   UTF8="$(printf "$UTF8_ESCAPED")" &&
+   echo $UTF8 >expect &&
+   git -c core.quotepath=false ls-files >actual &&
+   test_cmp expect actual &&
+   cat >expect <<-\EOF &&
+   content123
+   EOF
+   cat $UTF8 >actual &&
+   test_cmp expect actual
+   )
+'
+
+test_expect_success 'kill p4d' '
+   kill_p4d
+'
+
+test_done
-- 
1.9.5 (Apple Git-50.3)

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

On a personal note

[Johannes Schindelin]

Hey all,

yes, it is true: since mid-August I am working for Microsoft. Over a
year ago, I got into contact with the Visual Studio Online group at
Microsoft, of which I am now a happy member. A large part of my mission
is to improve the experience of Git for Windows. This is very exciting
to me: I finally can focus pretty much full time on something that I
could only address in my spare time previously.

Of course, Git for Windows will stay exactly the same project as before.
It will not suddenly turn into a Microsoft product. Obviously, it will
stay Open Source, its goal remains to bring the awesome Git SCM to
Windows, and I will continue to participate actively in the Git
community, if anything even more actively. After the release of Git for
Windows 2.5.0 and 2.5.1, there are quite a few loose ends to tie up; All
the better that I can now dedicate enough time to address them, and that
I now also have time to work properly with contributors on improving Git
for Windows.

I am really excited to join the club of Git developers who get paid to
work on Git as part of their day-jobs.

In short: for users and contributors of Git for Windows, nothing
changes. Except that Pull Requests and issues may be dealt with more
swiftly, and that I will actively work on keeping Git for Windows more
closely in sync with Git proper.

Good times!
Johannes
--
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: GSoC 2015 is over

[Christian Couder]

Hi,

On Thu, Sep 3, 2015 at 12:46 AM, Johannes Schindelin
 wrote:
> Hi,
>
> On Wed, 2 Sep 2015, Paul Tan wrote:
>
>> On Wed, Sep 2, 2015 at 12:55 AM, Matthieu Moy
>>  wrote:
>> > I consider this GSoC as a great success and a pleasant experience.
>> > Congratulation to Paul and Karthik, and a warm "thank you" to everybody
>> > who contributed: administrators, mentors, reviewers, and obviously
>> > Junio! (not to mention Google, who made all this possible)
>> >
>> > Thanks all!
>>
>> Thanks!
>
> Well, with so many thankyous going on, who am I to hide behind a rock?

Yeah, not to hide behind a rock myself either, I will say that I agree
Karthik's GSoC has been a great success, thanks to everyone involved
especially Karthik who has been very responsive and very persistant,
Matthieu who has been a great co-mentor, a great reviewer and a great
admin, and Eric, Michael and Junio who have been great reviewers of
Karthik's work.

I also feel very lucky to be the Git project mentor who will go to the
GSoC mentor summit on November 6 & 7. Thanks!

By the way while in the Bay Area from October 31 to November 14 it
would be nice for me to meet some Git developers living there. As Peff
suggested we could even have a mini GitTogether if enough developers
are interested. It doesn't need to be big. It could be just a lunch or
diner. Tell me if you are interested. We will see depending on the
number of people interested what we can do.

Thanks,
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

Re: [RFC PATCH] git-p4: add option to store files in Git LFS on import

[Luke Diamand]

>>>
>>> Regarding Python 3:
>>> Would you drop Python 2 support or do you want to support Python 2/3 in 
>>> parallel? I would prefer the former…
>>
>> For quite some time we would need to support both; we can't just have
>> a release of git that one day breaks git-p4 for people stuck on Python
>> 2. But it might not be that hard to support both (though converting
>> all those print statements could be quite tiresome).
> Agreed. However supporting both versions increases code complexity as well as 
> testing effort. Would a compromise like the following work? We fork 
> “git-p4.py” to “git-p4-python2.py” and just apply important bug fixes to that 
> file. All new development happens on a Python 3 only git-p4.py.

I'm not a python expert, but I think we're quite a way from that point
anyway. I think we'd want to run 2to3 on it and make it work - at that
point it should work on both python 2.7 (and earlier? I don't know)
and python 3.x. By the time that's done, we may well find that we
_can_ just drop python2 support, or fork, as you suggest.

Running 2to3 also includes adding test cases for all the code that is
in there that's not currently covered so that end-users don't find out
the hard way that we've missed bits. That's why I think it's a fairly
long-term goal.

Regardless, I think we'd want to have a wider discussion about the
best way forward, and there doesn't seem much point having that
discussion now when there's no actual code!
--
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

glibc mutex deadlock in signal handler

[Takashi Iwai]

Hi,

we've got a bug report of git-log stall in below:
  https://bugzilla.opensuse.org/show_bug.cgi?id=942297

In short, git-log is left unterminated sometimes when pager is
aborted.  When this happens, git process can't be killed by SIGTERM,
but only by SIGKILL.  And, further investigation revealed the possible
mutex deadlock used in glibc *alloc()/free().

I tried to reproduce this by adding a fault injection in glibc code,
and actually got the similar stack trace as reported.  The problem is
that glibc malloc (in this case realloc() and free()) takes a private
mutex.  Thus calling any function does *alloc() or free() in a signal
handler may deadlock.  In the case of git, it was free() calls in
various cleanup codes and the printf() / strerror() invocations.

Also, basically it's not safe to call fflush() or raise(), either.
But they seem to work practically on the current systems.

Below is a band-aid patch I tested and confirmed to work in the
fault-injection case.  But, some unsafe calls mentioned in the above
are left.  If we want to be in a safer side, we should really limit
the things to do in a signal handler, e.g. only calling close() and
doing waitpid(), I suppose.

Any better ideas?


thanks,

Takashi

---
diff --git a/pager.c b/pager.c
index 27d4c8a17aa1..57dda0142fa9 100644
--- a/pager.c
+++ b/pager.c
@@ -14,19 +14,25 @@
 static const char *pager_argv[] = { NULL, NULL };
 static struct child_process pager_process = CHILD_PROCESS_INIT;
 
-static void wait_for_pager(void)
+static void flush_pager(void)
 {
fflush(stdout);
fflush(stderr);
/* signal EOF to pager */
close(1);
close(2);
+}
+
+static void wait_for_pager(void)
+{
+   flush_pager();
finish_command(_process);
 }
 
 static void wait_for_pager_signal(int signo)
 {
-   wait_for_pager();
+   flush_pager();
+   finish_command_in_signal(_process);
sigchain_pop(signo);
raise(signo);
 }
diff --git a/run-command.c b/run-command.c
index 3277cf797ed4..a8cdc8f32944 100644
--- a/run-command.c
+++ b/run-command.c
@@ -18,26 +18,27 @@ struct child_to_clean {
 static struct child_to_clean *children_to_clean;
 static int installed_child_cleanup_handler;
 
-static void cleanup_children(int sig)
+static void cleanup_children(int sig, int in_signal)
 {
while (children_to_clean) {
struct child_to_clean *p = children_to_clean;
children_to_clean = p->next;
kill(p->pid, sig);
-   free(p);
+   if (!in_signal)
+   free(p);
}
 }
 
 static void cleanup_children_on_signal(int sig)
 {
-   cleanup_children(sig);
+   cleanup_children(sig, 1);
sigchain_pop(sig);
raise(sig);
 }
 
 static void cleanup_children_on_exit(void)
 {
-   cleanup_children(SIGTERM);
+   cleanup_children(SIGTERM, 0);
 }
 
 static void mark_child_for_cleanup(pid_t pid)
@@ -220,7 +221,7 @@ static inline void set_cloexec(int fd)
fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
 }
 
-static int wait_or_whine(pid_t pid, const char *argv0)
+static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
 {
int status, code = -1;
pid_t waiting;
@@ -231,13 +232,17 @@ static int wait_or_whine(pid_t pid, const char *argv0)
 
if (waiting < 0) {
failed_errno = errno;
-   error("waitpid for %s failed: %s", argv0, strerror(errno));
+   if (!in_signal)
+   error("waitpid for %s failed: %s", argv0,
+ strerror(errno));
} else if (waiting != pid) {
-   error("waitpid is confused (%s)", argv0);
+   if (!in_signal)
+   error("waitpid is confused (%s)", argv0);
} else if (WIFSIGNALED(status)) {
code = WTERMSIG(status);
if (code != SIGINT && code != SIGQUIT)
-   error("%s died of signal %d", argv0, code);
+   if (!in_signal)
+   error("%s died of signal %d", argv0, code);
/*
 * This return value is chosen so that code & 0xff
 * mimics the exit code that a POSIX shell would report for
@@ -254,10 +259,12 @@ static int wait_or_whine(pid_t pid, const char *argv0)
failed_errno = ENOENT;
}
} else {
-   error("waitpid is confused (%s)", argv0);
+   if (!in_signal)
+   error("waitpid is confused (%s)", argv0);
}
 
-   clear_child_for_cleanup(pid);
+   if (!in_signal)
+   clear_child_for_cleanup(pid);
 
errno = failed_errno;
return code;
@@ -437,7 +444,7 @@ fail_pipe:
 * At this point we know that fork() succeeded, but execvp()
 * failed. Errors have been reported to our stderr.
 */
-   

Re: [RFC PATCH] git-p4: add option to store files in Git LFS on import

[Lars Schneider]

On 30 Aug 2015, at 18:36, Luke Diamand  wrote:

> On 30 August 2015 at 11:18, Lars Schneider  wrote:
>> Thanks for your feedback!
>> 
>> I like the “handle big files” plugin kind of idea. However, I wonder if it 
>> makes sense to put more and more stuff into git-p4.py (>3000 LOC already). 
>> What do you think about splitting git-p4 into multiple files?
> 
> I was wondering about that. I think for now, the simplicity of keeping
> everything in one file is worth the slight extra pain. I don't imagine
> that the big-file-handler code would be very large.
OK.

> 
>> 
>> Regarding Python 3:
>> Would you drop Python 2 support or do you want to support Python 2/3 in 
>> parallel? I would prefer the former…
> 
> For quite some time we would need to support both; we can't just have
> a release of git that one day breaks git-p4 for people stuck on Python
> 2. But it might not be that hard to support both (though converting
> all those print statements could be quite tiresome).
Agreed. However supporting both versions increases code complexity as well as 
testing effort. Would a compromise like the following work? We fork “git-p4.py” 
to “git-p4-python2.py” and just apply important bug fixes to that file. All new 
development happens on a Python 3 only git-p4.py. 

Cheers,
Lars--
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] git-p4: add option to store files in Git LFS on import

[John Keeping]

On Thu, Sep 03, 2015 at 11:40:20AM +0200, Lars Schneider wrote:
> 
> On 30 Aug 2015, at 18:36, Luke Diamand  wrote:
> 
> > On 30 August 2015 at 11:18, Lars Schneider  wrote:
> >> Thanks for your feedback!
> >> 
> >> I like the “handle big files” plugin kind of idea. However, I
> >> wonder if it makes sense to put more and more stuff into git-p4.py
> >> (>3000 LOC already). What do you think about splitting git-p4 into
> >> multiple files?
> > 
> > I was wondering about that. I think for now, the simplicity of keeping
> > everything in one file is worth the slight extra pain. I don't imagine
> > that the big-file-handler code would be very large.
> OK.
> 
> > 
> >> 
> >> Regarding Python 3:
> >> Would you drop Python 2 support or do you want to support Python
> >> 2/3 in parallel? I would prefer the former…
> > 
> > For quite some time we would need to support both; we can't just have
> > a release of git that one day breaks git-p4 for people stuck on Python
> > 2. But it might not be that hard to support both (though converting
> > all those print statements could be quite tiresome).
> Agreed. However supporting both versions increases code complexity as
> well as testing effort. Would a compromise like the following work? We
> fork “git-p4.py” to “git-p4-python2.py” and just apply important bug
> fixes to that file. All new development happens on a Python 3 only
> git-p4.py. 

Documentation/CodingGuidelines currently says:

 - As a minimum, we aim to be compatible with Python 2.6 and 2.7.

 - Where required libraries do not restrict us to Python 2, we try to
   also be compatible with Python 3.1 and later.

That was added in commit 9ef43dd (CodingGuidelines: add Python coding
guidelines, 2013-01-30), which gives the following rationale in the
commit message:

 - Advocating Python 3 support in all scripts is currently unrealistic
   because:

 - 'p4 -G' provides output in a format that is very hard to use with
   Python 3 (and its documentation claims Python 3 is unsupported).

Has that changed?

I also found a message describing why the output is hard to use with
Python 3:

http://permalink.gmane.org/gmane.comp.version-control.git/213316

If that problem can be solved, I don't think it would be difficult to
support 2.6+ and 3.x with a single file.
--
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: git svn clone fails

[Jörg Schaible]

Hi Eric,

Eric Wong wrote:

> Jörg Schaible  wrote:
>> Is there really no other place for a bug report? This will simply vanish
>> in the list's noise ...
> 
> These messages do get seen and read.  (And I would not have seen this
> message if it were posted anywhere else).

As first time poster for this list without any response in days I was simply 
not sure, if it was the correct place to report problems.

> But I don't have much time or motivation to work on git svn since it's
> mostly made itself obsolete for me.

That's our goal also, to make svn obsolete ;-)
All we need is a one time conversion.
 
> --preserve-empty-dirs probably hasn't seen much real-world use and
> (IIRC) not something that could always be 100% reliable.
> 
> Regardless of options, git svn does log empty directories, so there's
> also an obscure, probably equally-unused "git svn mkdirs" command which
> processes the unhandled.log files inside $GIT_DIR to recreate empty
> directories.  You could give that a try.

I am not sure, what you actually try to tell me here. How can I interfere in 
the complete "svn clone" process?

Note, that the problem seems really to be caused by some obscure condition. 
We can actually clone some svn trees that contain empty directories and they 
are properly handled with a .gitignore file. However, some fail for unknown 
reason.

I tried to recreate the situation with a separte non-critical svn 
playground, but was never able reproduce the problem. svn clone succeeded 
every time. Not very useful for an error report, I know. :-/

Cheers,
Jörg


--
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 00/43] refs lmdb backend

[Junio C Hamano]

David Turner  writes:

> I think I've broken about all of the standalone stuff out, so here's
> the main enchilada.
>
> This series depends on at least the following topics in pu:
> dt/refs-bisection
> dt/refs-pseudo
> dt/reflog-tests
> kn/for-each-tag (patch 21 and corresponding bits of 42 depend on this;
> we could skip them, but I wanted this to apply on top of pu)
>
> As before, I tested by hacking the test suite to run under the lmdb
> backend and changing a few dozen tests.  The remaiing failures are
> documented in Documentation/technical/refs-be-lmdb.txt, except for one
> in t1404 where this version gives a different error message (but still
> an error).
>
> As Jeff King suggested last time I sent this around, I've made the
> reflog format slightly more efficient.  Now it stores shas in a binary
> format, and only uses a header entry if there are no real entries.
>
> Also, now per-worktree refs live in the filesystem.
>
> I've also made a number of fixes to memory leaks, formatting,
> factoring, etc.
>
> As Michael Haggerty suggested, I'm now using struct ref_transaction as
> a base struct for the ref transaction structs.
>
> Looking forward to reviews.

[03/43] seems to be missing, but without the list of changes in the
cover letter it is hard to tell what got dropped.
--
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 01/43] refs.c: create a public version of verify_refname_available

[Junio C Hamano]

David Turner  writes:

> @@ -2827,7 +2831,7 @@ int pack_refs(unsigned int flags)
>   *
>   * The refs in 'refnames' needn't be sorted. `err` must not be NULL.
>   */
> -static int repack_without_refs(struct string_list *refnames, struct strbuf 
> *err)
> +int repack_without_refs(struct string_list *refnames, struct strbuf *err)
>  {
>   struct ref_dir *packed;
>   struct string_list_item *refname;

An unrelated change (belongs to 2/43) snuk in here?
--
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 42/43] refs: add LMDB refs backend

[Junio C Hamano]

David Turner  writes:

> Add a database backend for refs using LMDB.  This backend runs git
> for-each-ref about 30% faster than the files backend with fully-packed
> refs on a repo with ~120k refs.

Nice ;-)

--
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 08/11] date: check for "local" before anything else

[Junio C Hamano]

John Keeping  writes:

> In a following commit we will make "local" orthogonal to the format.
> Although this will not apply to "relative", which does not use the
> timezone, it applies to all other formats so move the timezone
> conversion to the start of the function.

"local" is a request to show the timestamp in the output in our
local timezone regardless of the format, so even though "relative"
is not affected by the timezone (because "relative" does not show a
timestamp at all---it only shows the duration), this change
conceptually makes sense.

Thanks.


>
> Signed-off-by: John Keeping 
> ---
>  date.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/date.c b/date.c
> index 8f91569..9f0a5dd 100644
> --- a/date.c
> +++ b/date.c
> @@ -174,6 +174,9 @@ const char *show_date(unsigned long time, int tz, const 
> struct date_mode *mode)
>   struct tm *tm;
>   static struct strbuf timebuf = STRBUF_INIT;
>  
> + if (mode->type == DATE_LOCAL)
> + tz = local_tzoffset(time);
> +
>   if (mode->type == DATE_RAW) {
>   strbuf_reset();
>   strbuf_addf(, "%lu %+05d", time, tz);
> @@ -189,9 +192,6 @@ const char *show_date(unsigned long time, int tz, const 
> struct date_mode *mode)
>   return timebuf.buf;
>   }
>  
> - if (mode->type == DATE_LOCAL)
> - tz = local_tzoffset(time);
> -
>   tm = time_to_tm(time, tz);
>   if (!tm) {
>   tm = time_to_tm(0, 0);
--
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] pack-protocol: document newline behavior in push commands

[Junio C Hamano]

Jeff King  writes:

> Frankly, this feels a bit like a step backwards to me. I am tempted to
> suggest instead that git start sending the newlines, but I'm not sure
> it's worth any potential fallout.

I actually think we should do both in the longer term.

If we say sender "SHOULD" and we know no existing receiver violates
the "MUST NOT reject", our sender should follow that "SHOULD".

This documentation update is good in that it makes the examples
easier to read (by the way, the first pre-context line ends with
'\n', which we would eventually also address) by making the reader
understand that the convention used in this S:/C: exchange
illustration the optional LF is not shown.

> I'm also tempted to scrap this and say it just falls under the rule
> that every PKT-LINE is "sender SHOULD include LF" and "receiver MUST NOT
> complain about missing LF" (which does appear earlier in the document,
> though in a different context). Or maybe we should write out that rule
> explicitly and drop the "LF" from all parts of the grammar (which is
> what the thread above advocates).

Hmm, I view this patch as a first step in that direction.

>
>  Documentation/technical/pack-protocol.txt | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/technical/pack-protocol.txt 
> b/Documentation/technical/pack-protocol.txt
> index 4064fc7..9ce53b4 100644
> --- a/Documentation/technical/pack-protocol.txt
> +++ b/Documentation/technical/pack-protocol.txt
> @@ -469,8 +469,8 @@ references.
>  
>shallow   =  PKT-LINE("shallow" SP obj-id LF)
>  
> -  command-list  =  PKT-LINE(command NUL capability-list LF)
> -*PKT-LINE(command LF)
> +  command-list  =  PKT-LINE(command NUL capability-list)
> +*PKT-LINE(command)
>  flush-pkt
>  
>command   =  create / delete / update
> @@ -586,8 +586,8 @@ An example client/server communication might look like 
> this:
> S: 003f74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/team\n
> S: 
>  
> -   C: 003e7d1665144a3a975c05f1f43902ddaf084e784dbe 
> 74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/debug\n
> -   C: 003e74730d410fcb6603ace96f1dc55ea6196122532d 
> 5a3f6be755bbb7deae50065988cbfa1ffa9ab68a refs/heads/master\n
> +   C: 003e7d1665144a3a975c05f1f43902ddaf084e784dbe 
> 74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/debug
> +   C: 003e74730d410fcb6603ace96f1dc55ea6196122532d 
> 5a3f6be755bbb7deae50065988cbfa1ffa9ab68a refs/heads/master
> C: 
> C: [PACKDATA]
--
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 04/11] Documentation/rev-list: don't list date formats

[Junio C Hamano]

John Keeping  writes:

> We are about to add several new date formats which will make this list
> too long to display in a single line.
>
> Signed-off-by: John Keeping 
> ---
>  Documentation/git-rev-list.txt | 2 +-
>  Documentation/rev-list-options.txt | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

After seeing 1-3/11 this makes me wonder if we still have a list
somewhere in the documentation.  I'll read on first but I may have
to come back to this later.

Thanks.

>
> diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
> index 7b49c85..ef22f17 100644
> --- a/Documentation/git-rev-list.txt
> +++ b/Documentation/git-rev-list.txt
> @@ -45,7 +45,7 @@ SYNOPSIS
>[ --regexp-ignore-case | -i ]
>[ --extended-regexp | -E ]
>[ --fixed-strings | -F ]
> -  [ --date=(local|relative|default|iso|iso-strict|rfc|short) ]
> +  [ --date=]
>[ [ --objects | --objects-edge | --objects-edge-aggressive ]
>  [ --unpacked ] ]
>[ --pretty | --header ]
> diff --git a/Documentation/rev-list-options.txt 
> b/Documentation/rev-list-options.txt
> index a9b808f..14c4cce 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -699,7 +699,7 @@ include::pretty-options.txt[]
>  --relative-date::
>   Synonym for `--date=relative`.
>  
> ---date=(relative|local|default|iso|iso-strict|rfc|short|raw)::
> +--date=::
>   Only takes effect for dates shown in human-readable format, such
>   as when using `--pretty`. `log.date` config variable sets a default
>   value for the log command's `--date` option.
--
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: [PATCHv5 3/3] submodule: Reimplement `module_clone` shell function in C

[Eric Sunshine]

On Wed, Sep 2, 2015 at 5:42 PM, Stefan Beller  wrote:
> This reimplements the helper function `module_clone` in shell
> in C as `clone`. This functionality is needed for converting
> `git submodule update` later on, which we want to add threading
> to.
>
> Signed-off-by: Stefan Beller 
> ---
> +static int module_clone(int argc, const char **argv, const char *prefix)
> +{
> +   /* Write a .git file in the submodule to redirect to the 
> superproject. */
> +   if (safe_create_leading_directories_const(path) < 0)
> +   die(_("could not create directory '%s'"), path);
> +
> +   if (path && *path)
> +   strbuf_addf(, "%s/.git", path);
> +   else
> +   strbuf_addf(, ".git");

Minor: strbuf_addstr(...);

> +   if (safe_create_leading_directories_const(sb.buf) < 0)
> +   die(_("could not create leading directories of '%s'"), 
> sb.buf);
> +   submodule_dot_git = fopen(sb.buf, "w");
> +   if (!submodule_dot_git)
> +   die_errno(_("cannot open file '%s'"), sb.buf);
> +}
--
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

git grep broken in Fedora 21 update?

[Rustad, Mark D]

I just found a case where grep and git grep yield different results. Inside the 
ixgbe directory of the Linux kernel I did:

$ grep enter_lplu *.[ch]

And got the following:

ixgbe_main.c:   if (adapter->hw.phy.ops.enter_lplu) {
ixgbe_main.c:   adapter->hw.phy.ops.enter_lplu(>hw);
ixgbe_type.h:   s32 (*enter_lplu)(struct ixgbe_hw *);
ixgbe_x550.c:/** ixgbe_enter_lplu_x550em - Transition to low power states
ixgbe_x550.c:static s32 ixgbe_enter_lplu_t_x550em(struct ixgbe_hw *hw)
ixgbe_x550.c:   phy->ops.enter_lplu = ixgbe_enter_lplu_t_x550em;

But when I did:

$ git grep enter_lplu

I got:

ixgbe_main.c:   if (adapter->hw.phy.ops.enter_lplu) {
ixgbe_main.c:   adapter->hw.phy.ops.enter_lplu(>hw);
ixgbe_type.h:   s32 (*enter_lplu)(struct ixgbe_hw *);
ixgbe_x550.c:/** ixgbe_enter_lplu_x550em - Transition to low power states
ixgbe_x550.c:static s32 ixgbe_enter_lplu_t_x550em(struct ixgbe_hw *hw)

You can see that git grep missed the line in ixgbe_x550.c that had two hits on 
the string.

This was with git 2.1.0 in Fedora 21. I use git grep a lot and never noticed a 
problem before. I just updated my Fedora 21 system yesterday, so I have to 
figure that has something to do with it. I checked and git didn't get updated, 
so I figure it must be a library that is really broken.

I see in my update log that pcre was updated to:

pcre.x86_64 0:8.35-12.fc21
pcre-devel.x86_64 0:8.35-12.fc21

Yet the grep command is unaffected.

--
Mark Rustad, Networking Division, Intel Corporation



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH] pack-protocol: clarify LF-handling in PKT-LINE()

[Junio C Hamano]

Jeff King  writes:

> Since the discussion seemed to end up in this "let's make PKT-LINE more
> clear" direction, here is an attempt to do that.

Ah, I was confused while reading the other one ;-)

Will queue this one instead.  Thanks.

> -- >8 --
> Subject: pack-protocol: clarify LF-handling in PKT-LINE()
>
> The spec is very inconsistent about which PKT-LINE() parts
> of the grammar include a LF. On top of that, the code is not
> consistent, either (e.g., send-pack does not put newlines
> into the ref-update commands it sends).
>
> Let's make explicit the long-standing expectation that we
> generally expect pkt-lines to end in a newline, but that
> receivers should be lenient. This makes the spec consistent,
> and matches what git already does (though it does not always
> fulfill the SHOULD).
>
> We do make an exception for the push-cert, where the
> receiving code is currently a bit pickier. This is a
> reasonable way to be, as the data needs to be byte-for-byte
> compatible with what was signed. We _could_ make up some
> rules about signing a canonicalized version including
> newlines, but that would require a code change, and is out
> of scope for this patch.
>
> Signed-off-by: Jeff King 
> ---
>  Documentation/technical/pack-protocol.txt   | 46 
> -
>  Documentation/technical/protocol-common.txt |  5 +++-
>  2 files changed, 30 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/technical/pack-protocol.txt 
> b/Documentation/technical/pack-protocol.txt
> index 4064fc7..c6977bb 100644
> --- a/Documentation/technical/pack-protocol.txt
> +++ b/Documentation/technical/pack-protocol.txt
> @@ -14,6 +14,14 @@ data.  The protocol functions to have a server tell a 
> client what is
>  currently on the server, then for the two to negotiate the smallest amount
>  of data to send in order to fully update one or the other.
>  
> +pkt-line Format
> +---
> +
> +The descriptions below build on the pkt-line format described in
> +protocol-common.txt. When the grammar indicate `PKT-LINE(...)`, unless
> +otherwise noted the usual pkt-line LF rules apply: the sender SHOULD
> +include a LF, but the receiver MUST NOT complain if it is not present.
> +
>  Transports
>  --
>  There are three transports over which the packfile protocol is
> @@ -143,9 +151,6 @@ with the object name that each reference currently points 
> to.
> 003fe92df48743b7bc7d26bcaabfddde0a1e20cae47c refs/tags/v1.0^{}
> 
>  
> -Server SHOULD terminate each non-flush line using LF ("\n") terminator;
> -client MUST NOT complain if there is no terminator.
> -
>  The returned response is a pkt-line stream describing each ref and
>  its current value.  The stream MUST be sorted by name according to
>  the C locale ordering.
> @@ -165,15 +170,15 @@ MUST peel the ref if it's an annotated tag.
> flush-pkt
>  
>no-refs  =  PKT-LINE(zero-id SP "capabilities^{}"
> -   NUL capability-list LF)
> +   NUL capability-list)
>  
>list-of-refs =  first-ref *other-ref
>first-ref=  PKT-LINE(obj-id SP refname
> -   NUL capability-list LF)
> +   NUL capability-list)
>  
>other-ref=  PKT-LINE(other-tip / other-peeled)
> -  other-tip=  obj-id SP refname LF
> -  other-peeled =  obj-id SP refname "^{}" LF
> +  other-tip=  obj-id SP refname
> +  other-peeled =  obj-id SP refname "^{}"
>  
>shallow  =  PKT-LINE("shallow" SP obj-id)
>  
> @@ -216,8 +221,8 @@ out of what the server said it could do with the first 
> 'want' line.
>  
>depth-request =  PKT-LINE("deepen" SP depth)
>  
> -  first-want=  PKT-LINE("want" SP obj-id SP capability-list LF)
> -  additional-want   =  PKT-LINE("want" SP obj-id LF)
> +  first-want=  PKT-LINE("want" SP obj-id SP capability-list)
> +  additional-want   =  PKT-LINE("want" SP obj-id)
>  
>depth =  1*DIGIT
>  
> @@ -284,7 +289,7 @@ so that there is always a block of 32 "in-flight on the 
> wire" at a time.
>  compute-end
>  
>have-list =  *have-line
> -  have-line =  PKT-LINE("have" SP obj-id LF)
> +  have-line =  PKT-LINE("have" SP obj-id)
>compute-end   =  flush-pkt / PKT-LINE("done")
>  
>  
> @@ -348,10 +353,10 @@ Then the server will start sending its packfile data.
>  
>  
>server-response = *ack_multi ack / nak
> -  ack_multi   = PKT-LINE("ACK" SP obj-id ack_status LF)
> +  ack_multi   = PKT-LINE("ACK" SP obj-id ack_status)
>ack_status  = "continue" / "common" / "ready"
> -  ack = PKT-LINE("ACK SP obj-id LF)
> -  nak = PKT-LINE("NAK" LF)
> +  ack = PKT-LINE("ACK" SP obj-id)
> +  nak = PKT-LINE("NAK")
>  
>  
>  A simple clone may look like this (with no 'have' lines):
> @@ -467,10 +472,10 @@ references.
>  

Re: Feature Request: enhance Git-GUI's Checkout Branch screen

[David Aguilar]

On Thu, Sep 03, 2015 at 10:08:24AM -0700, John Medema wrote:
> Git gurus, throw this one on your to-do list:
> 
> This is a feature request to enhance the Git GUI to make it easier to
> checkout non-existing branches that exist upstream. Apologies if this
> is not the correct place for feature requests.
> 
> Scenario: Upstream repo has 4 branches - master, develop, maint, test.
> Local repo has only a master branch. In the command line, to switch to
> a local copy of the test branch, it is a simple "git checkout test".
> The git command automatically realizes your requested branch doesn't
> exist but origin does have a branch named test, so it a) creates a
> local branch off of origin/develop, b) sets the appropriate pull link,
> and c) sets the appropriate push link. In effect, the git command line
> hides the fact that the user doesn't know the branch doesn't exist and
> creates it as the user was expecting it to exist as. The Git GUI has
> no shortcut like this.
> 
> For reference, from the man page for git-checkout:
> "If  is not found but there does exist a tracking branch in
> exactly one remote (call it ) with a matching name, treat as
> equivalent to "git checkout -b  --track /".
> 
> Currently, in order to checkout a non-existing branch in the GUI you
> must go to the Branch Menu, click Create, select the "Tracking Branch"
> radio, select the branch, then go back up and name the branch the
> exact same name (to ease new user confusion). For a new user who just
> wants a copy of the remote branch, it is very unintuitive to create a
> new branch.
> 
> Fortunately, you already have some explicit warning messages after the
> Checkout Branch screen if the user intuitively tries to checkout the
> tracking branch, but even then a new user rarely realizes what they
> have gotten themselves into. At best, they know that they must find
> help (just for trying to checkout a branch).
> 
> In order to implement this feature effectively, I suggest that the
> Checkout Branch screen be modified in one of two ways (exclusive):
> 
> Option A:
> Merge the Local and Tracking Branch lists into one box keeping their
> entries separate by their full name ("master" and "origin/master"). If
> a user selects a remote branch, ask the user whether to create the
> local branch or move to the detached HEAD state (current
> functionality).
> 
> Option B (preferred):
> Keep the Local and Tracking Branch lists separate (as they are now),
> and keep the Tracking Branch list as-is. However, on the Local Branch
> screen, select include the existing branches in normal font but also
> include potential local branches based off of the remote in italics
> (or greyed-out, or asterisked, etc). Selecting an italicized entry
> creates the new branch from the tracking branch, without user
> interaction.

How about Option C?

What git-cola[1] has done for the longest time is when the
remote branch is selected it strips of the remote part and
automatically fills in the name field with that.  e.g. it fills
in "todo" when "origin/todo" is selected.

Mixing remote branches with pseudo-branches in the UI would
complicate it, so that seems like it could be made simpler.
Italics, asterisks, those are all signs that they shouldn't
be displayed next to each other.

I think the UI is fine as-is.  If it were just tweaked to
automatically fill in that field, like git-cola does, then it
can stay simple and make this use case easier.

If you're looking for other interesting usability things that
could be improved about git-gui, take a look at the
keyboard-driven user interface features in git-cola.  It's
optimized for keyboard-centric use.

[1] https://github.com/git-cola/git-cola
-- 
David
--
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: git grep broken in Fedora 21 update?

[Jacob Keller]

On Thu, Sep 3, 2015 at 5:04 PM, Rustad, Mark D  wrote:
> I just found a case where grep and git grep yield different results. Inside 
> the ixgbe directory of the Linux kernel I did:
>
> $ grep enter_lplu *.[ch]
>
> And got the following:
>
> ixgbe_main.c:   if (adapter->hw.phy.ops.enter_lplu) {
> ixgbe_main.c:   adapter->hw.phy.ops.enter_lplu(>hw);
> ixgbe_type.h:   s32 (*enter_lplu)(struct ixgbe_hw *);
> ixgbe_x550.c:/** ixgbe_enter_lplu_x550em - Transition to low power states
> ixgbe_x550.c:static s32 ixgbe_enter_lplu_t_x550em(struct ixgbe_hw *hw)
> ixgbe_x550.c:   phy->ops.enter_lplu = 
> ixgbe_enter_lplu_t_x550em;
>
> But when I did:
>
> $ git grep enter_lplu
>
> I got:
>
> ixgbe_main.c:   if (adapter->hw.phy.ops.enter_lplu) {
> ixgbe_main.c:   adapter->hw.phy.ops.enter_lplu(>hw);
> ixgbe_type.h:   s32 (*enter_lplu)(struct ixgbe_hw *);
> ixgbe_x550.c:/** ixgbe_enter_lplu_x550em - Transition to low power states
> ixgbe_x550.c:static s32 ixgbe_enter_lplu_t_x550em(struct ixgbe_hw *hw)
>
> You can see that git grep missed the line in ixgbe_x550.c that had two hits 
> on the string.
>
> This was with git 2.1.0 in Fedora 21. I use git grep a lot and never noticed 
> a problem before. I just updated my Fedora 21 system yesterday, so I have to 
> figure that has something to do with it. I checked and git didn't get 
> updated, so I figure it must be a library that is really broken.
>
> I see in my update log that pcre was updated to:
>
> pcre.x86_64 0:8.35-12.fc21
> pcre-devel.x86_64 0:8.35-12.fc21
>
> Yet the grep command is unaffected.
>
> --
> Mark Rustad, Networking Division, Intel Corporation
>

I don't know if command line grep uses pcre?

Does git-grep by default ignore changes not yet committed? That was my
first thought but I am not very familiar with the tool.

Regards,
Jake
--
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: determine name of tag used for checkout when multiple tags exist?

[Jacob Keller]

On Thu, Sep 3, 2015 at 7:53 PM, Jesse Hopkins  wrote:

>
> It seems to me that when checking out to a tag, Git does not record the
> *name* of the tag anywhere, but rather sets HEAD to the de-referenced
> commit SHA-1.  As far as I can tell, it is not possible to recover the
> original name of the tag in the case of multiple tags on the same commit.
> Is my conclusion correct?
>

I believe you are correct unless you check out to a branch named after
the tag? I don't believe it will actually store the tag as part of its
checkout.

Ie:

git checkout 

may show "checked out at " via status, but that is only because
status uses describe to find that out..

I don't believe we store the information. You could recover it if you
know something about the format so as to distinquish them, ie: if you
only cared about annotated tags vs lightweight tags, for example.

Regards,
Jake
--
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] Pinning of submodules

[Eric Sunshine]

On Wed, Sep 2, 2015 at 7:34 PM, Anders Ro  wrote:
> Patch to make it possible to pin submodules so that they are not
> affected by the --remote option in "git submodule".

Thanks for the patches. I don't use submodules, so I can't comment
specifically on this change, however, I can offer some general
comments on the patches. But first, a piece of advice...

Use git-send-email to post the patches as proper emails, one email
per patch, rather than as attachments. Reviewers are going to want to
write inline comments on the patches, and they can't do so when the
patches are attachments, so attaching patches discourages reviewers
from responding.

> git-submodule.sh: pin submodule when branch name is '@'
>
> Setting branch name to '@' for a submodule will disable 'git submodule
> update --remote' calls for that specific submodule. I.e. instead of
> follow the unspecified default choice of master, nothing is being
> updated. This is useful when multiple submodules exist but not all
> should follow the remote branch head.

With the disclaimer that I'm not a submodule user (to which the
answer might be obvious): What benefit is there in using a magic
value like this ("@") over, say, an explicit configuration setting?

> Signed-off-by: Anders Ro 
> ---
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 25b1ddf..1bb2bb1 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -843,7 +843,8 @@ Maybe you want to use 'update --init'?")"
>   die "$(eval_gettext "Unable to find current revision in submodule path 
> '\$displaypath'")"
>   fi
>
> - if test -n "$remote"
> + # Fetch latest in remote unless branch name in config is '@'
> + if test -n "$remote" -a "$branch" != "@"

The -a option to 'test' is not portable[1] and is considered obsolete
by POSIX[2]. Use "test foo && test bar" instead.

[1]: 
http://www.gnu.org/software/autoconf/manual/autoconf.html#index-g_t_0040command_007btest_007d-1793
[2]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html

>   then
>   if test -z "$nofetch"
>   then
> @@ -857,6 +858,12 @@ Maybe you want to use 'update --init'?")"
>   die "$(eval_gettext "Unable to find current ${remote_name}/${branch} 
> revision in submodule path '\$sm_path'")"
>   fi
>
> + # Inform that the current sm is pinned and use of '--remote' ignored
> + if test -n "$remote" -a "$branch" = "@"

Ditto.

> + then
> + say "$(eval_gettext "Submodule path '\$displaypath' pinned, remote update 
> ignored")"
> + fi
> +

> Subject: [PATCH 2/2] t7412: add test case for pinned submodules
>
> Signed-off-by: Anders Ro 
> ---
> diff --git a/t/t7412-submodule-pinning.sh b/t/t7412-submodule-pinning.sh
> new file mode 100755
> index 000..2844b1e
> --- /dev/null
> +++ b/t/t7412-submodule-pinning.sh
> @@ -0,0 +1,73 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2015 Anders Ronnbrant
> +#
> +
> +test_description="Branch name '@' disables submodule update --remote calls"
> +
> +. ./test-lib.sh
> +
> +get_sha() {
> +  cd $1 && git rev-list --max-count=1 HEAD
> +}

A few issues:

Indent with tabs (width 8), not spaces. (This comment applies to the
entire patch).

Style for shell scripts on this project is to have a space before
"()".

Taking a hint from t/test-lib-functions.sh:test_cmp_rev(), use "git
rev-parse --verify" instead.

It's a bit ugly that this does "cd $1" without ever balancing it with
a return to the original directory. If someone later comes along and
writes:

get_sha1 fiddle >fiddle-id

in a new test, then that person will be surprised to find that the
current working directory changed out from under him. The current
patch doesn't experience this problem since it always invokes it as
$(get_sha1 fiddle), but it could be made more robust by either
wrapping it in a subshell so that the 'cd' is undone when the
subshell exits, or by using "-C $1".

Taking the above comments into account:

get_sha () {
git -C "$1" rev-parse --verify HEAD
}

> +equal_sha() {
> +  test "$(get_sha $1)" = "$(get_sha $2)"
> +}
> +
> +not_equal_sha() {
> +  test "$(get_sha $1)" != "$(get_sha $2)"
> +}

Style: space before "()" on these two function declarations

> +test_expect_success 'setup submodule tree structure' '
> +for i in 1 2 3; do echo file$i > file$i; git add file$i; git commit -m 
> file$i; done &&

Style: write each command of the 'for' loop on its own line,
including 'do', don't use semi-colons

Style: no space after redirection operator: >file$i

Keep the &&-chain intact, and end the chain with "|| return $?" so
that the 'for' loop correctly exits if any contained command fails.

for i in 1 2 3
do
echo file$i >file$i &&
git add file$i &&
git commit -m file$i || return $?
done

> +test_tick &&

Why is this needed? There doesn't seem to be any obvious reason for
its presence, and the test still passes without it.

> +git clone . super &&
> +git clone . follow &&
> +git