Re: [PATCH v4 05/12] ref-filter: introduce parsing functions for each valid atom

[Karthik Nayak]

On Sun, Feb 7, 2016 at 12:03 PM, Eric Sunshine  wrote:
> On Sat, Feb 6, 2016 at 10:15 AM, Karthik Nayak  wrote:
>> On Sun, Jan 31, 2016 at 11:12 PM, Karthik Nayak  
>> wrote:
>>> @@ -138,10 +140,9 @@ int parse_ref_filter_atom(const char *atom, const char 
>>> *ep)
>>>  * shouldn't be used for checking against the valid_atom
>>>  * table.
>>>  */
>>> -   const char *formatp = strchr(sp, ':');
>>> -   if (!formatp || ep < formatp)
>>> -   formatp = ep;
>>> -   if (len == formatp - sp && !memcmp(valid_atom[i].name, sp, 
>>> len))
>>> +   arg = memchr(sp, ':', ep - sp);
>>> +   if ((!arg || len == arg - sp) &&
>>> +   !memcmp(valid_atom[i].name, sp, len))
>>> break;
>>> }
>>
>> Also having a look at this, this breaks the previous error checking we
>> had at parse_ref_filter_atom().
>> e.g: git for-each-ref --format="%(refnameboo)" would not throw an error.
>>
>> I think the code needs to be changed to:
>>
>> -   if ((!arg || len == arg - sp) &&
>> +   if ((arg || len == ep - sp) &&
>> +   (!arg || len == arg - sp) &&
>
> For completeness, for people reading the mailing list archive, a
> couple alternate fixes were presented elsewhere[1], with a personal
> bias toward:
>
> arg = memchr(...);
> if (!arg)
> arg = ep;
> if (len == arg - sp && !memcmp(...))
> ...
>
> [1]: 
> http://git.661346.n2.nabble.com/PATCH-ref-filter-c-don-t-stomp-on-memory-tp7647432p7647433.html

There is a slight issue with this solution though, as you see 'arg'
gets modified
here, hence 'arg' passed to parser functions will never will null.

We could do something like
if (arg ==ep)
arg = NULL;
if (arg)
arg = used_atom[at].name + (arg - atom) + 1;
if (valid_atom[i].parser)
valid_atom[i].parser(_atom[at], arg);

Else we could avoid this assignment and re-assignment by letting 'arg'
hold the value it gets from memcmp(...) and use the solution provided
by me or Ramsay (preferably)

Ramsay's solution being

 ref-filter.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ref-filter.c b/ref-filter.c
index d48e2a3..c98065e 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -260,7 +260,8 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
 * table.
 */
arg = memchr(sp, ':', ep - sp);
-   if ((!arg || len == arg - sp) &&
+   if ((( arg && len == arg - sp)  ||
+(!arg && len == ep - sp )) &&
!memcmp(valid_atom[i].name, sp, len))
break;
}

-- 
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 7/8] index-pack: --append-pack implies --strict

[Eric Sunshine]

On Fri, Feb 5, 2016 at 3:57 AM, Nguyễn Thái Ngọc Duy  wrote:
> A frankenstein pack, generated by multiple pack-objects runs,
> certainly has higher risk of broken, especially when the server side

s/of/& being/

> could be some other implementation than pack-objects. Be safe and
> strict.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
--
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 5/8] fetch-pack.c: send "skip" line to pack-objects

[Eric Sunshine]

On Fri, Feb 5, 2016 at 3:57 AM, Nguyễn Thái Ngọc Duy  wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
> diff --git a/fetch-pack.c b/fetch-pack.c
> @@ -856,6 +864,8 @@ static struct ref *do_fetch_pack(struct fetch_pack_args 
> *args,
> fprintf(stderr, "Server supports ofs-delta\n");
> } else
> prefer_ofs_delta = 0;
> +   if (args->resume_path && !server_supports("partial"))
> +   die(_("Server does not support resume capability\n"));

Drop "\n" from die() message.

> if ((agent_feature = server_feature_value("agent", _len))) {
> agent_supported = 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 v4 06/12] ref-filter: introduce color_atom_parser()

[Karthik Nayak]

On Sun, Feb 7, 2016 at 1:13 PM, Eric Sunshine  wrote:
> On Sat, Feb 6, 2016 at 10:20 AM, Karthik Nayak  wrote:
>> On Fri, Feb 5, 2016 at 3:55 AM, Eric Sunshine  
>> wrote:
>>> On Sun, Jan 31, 2016 at 12:42 PM, Karthik Nayak  
>>> wrote:
 +static void color_atom_parser(struct used_atom *atom, const char 
 *color_value)
 +{
 +   if (!color_value)
 +   die(_("expected format: %%(color:)"));
 +   if (color_parse(color_value, atom->u.color) < 0)
 +   die(_("invalid color value: %s"), atom->u.color);
>>>
>>> Shouldn't this be:
>>>
>>> die(_("invalid color value: %s"), color_value);
>>>
>>> ?
>>
>> Oops. You're right, it should.
>> Also the error is reported already in color_parse(...), so seems duplicated.
>>
>> git for-each-ref  --format="%(color:sfadf)%(align:middle,30)%(refname)%(end)"
>> error: invalid color value: sfadf
>> fatal: invalid color value: sfadf
>>
>> What would be an ideal way around this?
>
> According to f6c5a29 (color_parse: do not mention variable name in
> error message, 2014-10-07), the doubled diagnostic messages are
> intentional, so I don't think you need to do anything about it in this
> series. If you want to make the "fatal" message a bit more helpful,
> you could add a %(color:) annotation, like this:
>
> die(_("unrecognized color: %%(color:%s)"), color_value);
>
> which would give you the slightly more helpful:
>
> error: invalid color value: sfadf
> fatal: unrecognized color: %(color:sfadf)

That seems to be a good way around the repetitive message.
will add 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

Re: [PATCH v4 05/12] ref-filter: introduce parsing functions for each valid atom

[Karthik Nayak]

On Sun, Feb 7, 2016 at 12:33 PM, Eric Sunshine  wrote:
> On Sat, Feb 6, 2016 at 9:36 AM, Karthik Nayak  wrote:
>> On Thu, Feb 4, 2016 at 3:49 AM, Eric Sunshine  
>> wrote:
>>> On Sun, Jan 31, 2016 at 12:42 PM, Karthik Nayak  
>>> wrote:
 -   const char *formatp = strchr(sp, ':');
 -   if (!formatp || ep < formatp)
 -   formatp = ep;
 -   if (len == formatp - sp && !memcmp(valid_atom[i].name, sp, 
 len))
 +   arg = memchr(sp, ':', ep - sp);
>>>
>>> Why this change from strchr() to memchr()? I understand that you're
>>> taking advantage of the fact that you know the extent of the string
>>> via 'sp' and 'ep', however, was the original strchr() doing extra
>>> work? Even if this change is desirable, it seems somewhat unrelated to
>>> the overall purpose of this patch, thus might deserves its own.
>>>
>>> Aside from that, although the "expensive" strchr() / memchr() resides
>>> within the loop, it will always return the same value since it doesn't
>>> depend upon any condition local to the loop. This implies that it
>>> ought to be hoisted out of the loop. (This problem is not new to this
>>> patch; it's already present in the existing code.)
>>
>> I'm thinking I'll make a patch for that separately. i.e remove strchr()
>> and introduce memchr() outside the loop.
>
> I'd almost suggest making it two patches: (1) change strchr() to
> memchr(), and (2) hoist it outside the loop. However, it would be nice
> to see this series land with v5, and adding more refactoring patches
> could delay its landing if problems are found with those new patches.
> Consequently, it might make sense to forego any additional refactoring
> for now and just keep the patch as is, except for fixing the
> relatively minor issues (and bug or two) raised in the v4 review. If
> you take that approach, then hoisting memchr() out of the loop can be
> done as a follow-up patch after this series lands.
>

That makes sense too, I'll keep it the way it is in v4 and send a patch
post this series. Thanks

 +   if ((!arg || len == arg - sp) &&
 +   !memcmp(valid_atom[i].name, sp, len))
 break;
 }

 @@ -154,6 +155,10 @@ int parse_ref_filter_atom(const char *atom, const 
 char *ep)
 REALLOC_ARRAY(used_atom, used_atom_cnt);
 used_atom[at].name = xmemdupz(atom, ep - atom);
 used_atom[at].type = valid_atom[i].cmp_type;
 +   if (arg)
 +   arg = used_atom[at].name + (arg - atom) + 1;
>>>
>>> This is a harder to understand than it ought to be because it's
>>> difficult to tell at first glance that you don't actually care about
>>> 'arg' in relation to the original incoming string, but instead use it
>>> only to compute an offset into the string which is ultimately stored
>>> in the newly allocated used_atom[]. Re-using 'arg' for a different
>>> purpose (in a manner of speaking) confuses the issue further.
>>>
>>> The intention might be easier to follow if you made it clear that you
>>> were interested in the *offset* of the argument in the string, rather
>>> than a pointer into the incoming string which you ultimately don't
>>> use. A variable named 'arg_offset' might go a long way toward
>>> clarifying this intention.
>>
>> I hope you mean something like this.
>>
>> if (arg) {
>> int arg_offset;
>>
>> arg_offset = (arg - atom) + 1;
>> arg = used_atom[at].name + arg_offset;
>> }
>
> That's one way, but I was actually thinking about computing arg_offset
> earlier in the "is it a valid atom?" loop, which would make it clear
> that you care only about the offset at that point, rather than the
> pointer to the ':' in the original string (since that pointer is never
> itself used other than to compute the offset). However, having tried
> it myself, the code ends up being nosier, thus not necessarily a win,
> so maybe just leave this as is for now.

True, letting it be seems to make sense.

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

Re: [PATCH v6 04/11] test-regex: expose full regcomp() to the command line

[Eric Sunshine]

On Fri, Feb 5, 2016 at 9:03 PM, Nguyễn Thái Ngọc Duy  wrote:
> diff --git a/test-regex.c b/test-regex.c
> @@ -21,8 +38,38 @@ static int test_regex_bug(void)
>  int main(int argc, char **argv)
>  {
> +   const char *pat;
> +   const char *str;
> +   int flags = 0;
> +   regex_t r;
> +   regmatch_t m[1];
> +
> if (argc == 2 && !strcmp(argv[1], "--bug"))
> return test_regex_bug();
> -   else
> -   die("must be used with --bug");
> +   else if (argc < 3)
> +   die("usage: test-regex --bug\n"
> +   "   test-regex   []");

This is just a test program, so it probably isn't that important, but
die() automatically prepends "fatal: " which means the alignment of
the second line will be wrong. Perhaps you want to use usage() instead
which automatically prepends "usage: " (and drop the literal "usage: "
from the above string).

> +   argv++;
> +   pat = *argv++;
> +   str = *argv++;
> +   while (*argv) {
> +   struct reg_flag *rf;
> +   for (rf = reg_flags; rf->name; rf++)
> +   if (!strcmp(*argv, rf->name)) {
> +   flags |= rf->flag;
> +   break;
> +   }
> +   if (!rf->name)
> +   die("do not recognize %s", *argv);
> +   argv++;
> +   }
> +   git_setup_gettext();
> +
> +   if (regcomp(, pat, flags))
> +   die("failed regcomp() for pattern '%s'", pat);
> +   if (regexec(, str, 1, m, 0))
> +   return 1;
> +
> +   return 0;
>  }

This version is much easier to read without the "bug" special case
spread throughout the code. 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 v4 05/12] ref-filter: introduce parsing functions for each valid atom

[Eric Sunshine]

On Sun, Feb 7, 2016 at 4:01 AM, Karthik Nayak  wrote:
> On Sun, Feb 7, 2016 at 12:03 PM, Eric Sunshine  
> wrote:
>> On Sat, Feb 6, 2016 at 10:15 AM, Karthik Nayak  wrote:
>>> I think the code needs to be changed to:
>>>
>>> -   if ((!arg || len == arg - sp) &&
>>> +   if ((arg || len == ep - sp) &&
>>> +   (!arg || len == arg - sp) &&
>>
>> For completeness, for people reading the mailing list archive, a
>> couple alternate fixes were presented elsewhere[1], with a personal
>> bias toward:
>>
>> arg = memchr(...);
>> if (!arg)
>> arg = ep;
>
> There is a slight issue with this solution though, as you see 'arg'
> gets modified
> here, hence 'arg' passed to parser functions will never will null.
> [...]
> Else we could avoid this assignment and re-assignment by letting 'arg'
> hold the value it gets from memcmp(...) and use the solution provided
> by me or Ramsay (preferably)
>
> Ramsay's solution being
>
> arg = memchr(sp, ':', ep - sp);
> -   if ((!arg || len == arg - sp) &&
> +   if ((( arg && len == arg - sp)  ||
> +(!arg && len == ep - sp )) &&
> !memcmp(valid_atom[i].name, sp, len))
> break;

Yep, Ramsey's fix is preferable.
--
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

update_linked_gitdir writes relative path to .git/worktrees//gitdir

[Matt McCutchen]

I noticed that when update_linked_gitdir chooses to update
.git/worktrees//gitdir, the path it writes is relative, at least
under some circumstances.  This contradicts the gitrepository-layout
man page, which says:

worktrees//gitdir::
A text file containing the absolute path back to the .git file
that points to here.

IIUC, this behavior defeats one of the three safeguards that is
supposed to prevent "git worktree prune" from pruning information for
worktrees that still exist.

A simple script to reproduce:

#!/bin/bash
set -e -x
rm -rf repo worktree2
git init repo
cd repo
touch foo
git add foo
git commit -m 'dummy commit'
git worktree add ../worktree2 -b branch2
cat .git/worktrees/worktree2/gitdir
touch -d '2 days ago' .git/worktrees/worktree2/gitdir
(cd ../worktree2 && git status)
cat .git/worktrees/worktree2/gitdir

Trying this on master as of earlier today (ff4ea60), I get:

[...]
/PATH/REDACTED/worktree2/.git
[...]
.git

Matt


--
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 00/11] Fix icase grep on non-ascii

[Eric Sunshine]

On Fri, Feb 5, 2016 at 9:02 PM, Nguyễn Thái Ngọc Duy  wrote:
> v6 fixes comments from Ramsay and Eric. Interdiff below. The only
> thing to add is, I decided not to replace !icase_non_ascii with
> icase_ascii_only. I went with spelling out "!icase || ascii_only". I
> think it expresses the intention better.

v6 appears to address all the comments raised in my v5 review. 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 show doesn't work on file names with square brackets

[Kirill Likhodedov]

Hi Johannes,

> On 06 Feb 2016, at 19:10 , Johannes Schindelin  
> wrote:
> 
>   git show 'HEAD:bra[ckets].txt' --
> 

Nice catch! It works for me even without quotes. Although this “--“ is 
mentioned in the error message, I didn’t even try since its meaning is totally 
unrelated with the problem ;) Anyway, thanks a lot for finding the workaround.--
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

Add Cc,Tested-by list while 'git commit'

[Jagan Teki]

Do we have any git config options to add Cc and Tested-by list like
Signed-off-by is fetched from git config.

example:

$ git commit -s

Cc: Arjun Ani 
Tested-by: Jagan Teki 
Signed-off-by: Jagan Teki 

Please share if we have any inputs to solve this.

-- 
Jagan.
--
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] setup.c: make check_filename() return 0 on ENAMETOOLONG

[Johannes Schindelin]

Hi Duy,

On Sun, 7 Feb 2016, Nguyễn Thái Ngọc Duy wrote:

> Noticed-by: Ole Tange 
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  On Sun, Feb 7, 2016 at 4:56 AM, Ole Tange  wrote:
>  > If file name too long it should just try to see if it is a reference
>  > to a revision.
> 
>  Looks easy enough to fix.

Maybe with a little bit more informative commit message? ;-)

Something like

Avoid interpreting too-long parameter as file name

Even if it is easier to write HEAD~2000, it is legal to write
HEAD^^^... (repeats "^" 2000 times in total). However, such a
string is too long to be a legal filename (and on Windows, by
default even much, much shorter strings are still illegal
because they exceed MAX_PATH).

Therefore, if the check_filename() function encounters too long
a command-line parameter, it should interpet the error code
ENAMETOOLONG as a strong hint that this is not a file name
instead of dying with an error message.

Noticed-by: ...

What do you think?
Dscho

Re: [PATCH v4 05/12] ref-filter: introduce parsing functions for each valid atom

[Andreas Schwab]

Karthik Nayak  writes:

> +   if ((( arg && len == arg - sp)  ||
> +(!arg && len == ep - sp )) &&

  len == (arg ? arg : ep) - sp &&

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: git submodule should honor "-c credential.helper" command line argument

[Marc Strapetz]

On 07.02.2016 05:41, Jacob Keller wrote:

On Wed, Feb 3, 2016 at 3:44 PM, Jacob Keller  wrote:

Ok so I am not sure we even really need to use "-c" option in
git-clone considering that we can just use the same flow we do for
setting core.worktree values. I'll propose a patch with you two Cc'ed,
which I think fixes the issue. There may actually be a set of
configuration we want to include though, and the main issue I see is
that it won't get updated correctly whenever the parent configuration
changes.

Thanks,
Jake


I tried adding the config as part of module_clone in
submodule--helper.c but it didn't pass the test I added. I haven't had
time to look at this in the last few days, but I am stuck as to why
submodule--helper.c appeared to not use module_clone as I thought.


I've tried to just comment out clearing of environment variables in 
git-sh-setup.sh, clear_local_git_env(). I've noticed that "-c 
credentials-helper ..." is stored in $GIT_CONFIG_PARAMETERS and with 
existing code is reset there. If not clearing the environment variables, 
at least "git submodule init" is working properly. I didn't try with 
other commands nor to run tests.


-Marc


--
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 show doesn't work on file names with square brackets

[Kirill Likhodedov]

Hi Duy,

> It's from 28fcc0b (pathspec: avoid the need of "--" when wildcard is
> used - 2015-05-02)

v2.5.0 is the first release which contains 28fcc0b.
I can confirm that older versions of Git work correctly without “--“:

# /opt/local/bin/git version
git version 1.7.1.1
# /opt/local/bin/git show HEAD:bra[ckets].txt 
asd

Looks like a regression?--
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] On the --depth argument when fetching with submodules

[Lars Schneider]

On 06 Feb 2016, at 01:05, Junio C Hamano  wrote:

> Stefan Beller  writes:
> 
>> Currently when cloning a project, including submodules, the --depth argument
>> is passed on recursively, i.e. when cloning with "--depth 2", both the
>> superproject as well as the submodule will have a depth of 2.  It is not
>> garantueed that the commits as specified by the superproject are included
>> in these 2 commits of the submodule.
>> 
>> Illustration:
>> (superproject with depth 2, so A would have more parents, not shown)
>> 
>> superproject/master: A <- B
>>/  \
>> submodule/master:  C <- D <- E <- F <- G
>> 
>> (Current behavior is to fetch G and F)
> 
> I think the issue is deeper than merely "--depth 2", and you would
> be better off stepping back and think about various use cases to
> make sure that we know what kind of behaviour we want to support
> before delving into one particular corner case.  We currently pass
> the depth recursively, and I do not think it makes much sense, but I
> view it as a secondary question "among the behaviours we want to
> support, which one should be the default?"  It may turn out that not
> passing it recursively at all, or even passing a different depth, is
> a better default, but we wouldn't know until we know what are the
> desirable behaviour in various workflows.
> 
> If you are actively working on the superproject plus some submodules
> but you are merely using the submodule you depicted above, not
> working on changing it, even when you want the full history of the
> superproject (i.e. no "--depth 2"), you may not want history of the
> submodule.  Even though we have a way to say "I am not interested in
> this submodule AT ALL" by not doing "submodule init", not having
> anything at all at the path submodule/ may not allow you to build
> the whole thing, and we currently lack a way to express "I am not
> interested in the history of this thing, but I need at least the
> tree that matches the commit referred to by the superproject".
> 
> If you are working on a single submodule, trying to fix a bug in the
> context of the whole project, you might want to have a single-depth
> clone of the superproject and all other submodules, plus the whole
> history of the single submodule.
> 
> In either of these examples, the top-level "--depth" does not have
> much to do with what depth the user wants to use when cloning or
> fetching the submodule repositories.
> 
> I have a feeling (but I would not be surprised if somebody who uses
> submodules heavily has a counter-example from real life) that
> regardless of "--depth" or full clone, fetching the tip of matching
> branch is not a good default behaviour.  In your picture, even when
> depth is not given at all, there isn't much point fetching F or G.

I really wonder in what cases people use the "--depth" option, too. 
For instance I have never used it in either one of the two cases you
described above. I don't worry about a long running "clone" as it 
usually is a one-time operation.

However, in case of a continuous integration system that starts with
a clean state in the beginning of every run (e.g. Travis CI) a
"clone" operation is no one-time operation anymore. In this case the
"--depth 1" option makes very much sense to me. This was the situation
where I realized the problem that Stefan wants to tackle here and I
tried to make it tangible with a test case [1]. 

On top of that I think Git's error message is really confusing if
you clone a repo with "--depth" that has submodules and Git is not
fetching the necessary submodule commits:

Unable to checkout '$SHA' in submodule path 'path/to/submodule'

I tried to tackle that with [2] which would detect this case and
print the following error instead (slightly changed from the patch):

Unable to checkout '$SHA' in submodule path '/path/to/commit'.
Try to remove the '--depth' argument on clone!

[1] https://www.mail-archive.com/git%40vger.kernel.org/msg82614.html
[2] https://www.mail-archive.com/git%40vger.kernel.org/msg82613.html


> 
>> So to fetch the correct submodule commits, we need to
>> * traverse the superproject and list all submodule commits.
>> * fetch these submodule commits (C and E) by sha1
> 
> I do not think requiring that C to be fetched when the superproject
> is cloned with --depth=2 (hence A and B are present in the result)
> is a good definition of "correct submodule commits".  The initial
> clone could be "superproject follows --depth, all submodules are
> cloned with --depth=1 at the commits referenced by the superproject
> tree"--by that definition, you need E but you do not want C.
> 
> As a specification of the behaviour, the above two might work, but I
> do not think that should be the implementation.  In other words,
> "The implementation should behave as if it did the above two" is OK,
> and it is also OK to qualify with further conditions to help the
> implementation.  For example, the 

Re: [PATCH v1] config: add '--sources' option to print the source of a config value

[Lars Schneider]

On 05 Feb 2016, at 14:58, Jeff King  wrote:

> On Fri, Feb 05, 2016 at 12:31:15PM +0100, Sebastian Schuberth wrote:
> 
>>> I'm not sure returning here is the best idea. We won't have a config
>>> filename if we are reading from "-c", but if we return early from this
>>> function, it parses differently than every other line. E.g., with your
>>> patch, if I do:
>>> 
>>>  git config -c foo.bar=true config --sources --list
>>> 
>>> I'll get:
>>> 
>>>  /home/peff/.gitconfig  user.name=Jeff King
>>>  /home/peff/.gitconfig  user.email=p...@peff.net
>>>  ...etc...
>>>  foo.bar=true
>>> 
>>> If somebody is parsing this as a tab-delimited list, then instead of the
>>> source field for that line being empty, it is missing (and it looks like
>>> "foo.bar=true" is the source file). I think it would be more friendly to
>>> consumers of the output to have a blank (i.e., set "fn" to the empty
>>> string and continue in the function).
>> 
>> Or to come up with a special string to denote config values specified on the
>> command line. Maybe somehting like
>> 
>>  foo.bar=true
>> 
>> I acknowledge that "" would be a valid filename on some
>> filesystems, but I think the risk is rather low that someone would actually
>> be using that name for a Git config file.
> 
> Yeah, I agree it's unlikely. And the output is already ambiguous, as the
> first field could be a blob (though I guess the caller knows if they
> passed "--blob" or not). If we really wanted an unambiguous output, we
> could have something like "file:...", "blob:...", etc. But that's a bit
> less readable for humans, and I don't think solves any real-world
> problems.
> 
> So I think it would be OK to use "" here, as long as the
> token is documented.
Sounds good to me. I'll add it!

> Are there any other reasons why current_config_filename() would return
> NULL, besides command-line config? I don't think so. It looks like we
> can read config from stdin, but we use the token "" there for the
> name field (another ambiguity!).
During my testing I haven't found any other case either. To be honest
I didn't know the "stdin" way to set the config! I added a test case for 
that, too!

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 v1] config: add '--sources' option to print the source of a config value

[Lars Schneider]

On 05 Feb 2016, at 12:20, Jeff King  wrote:

> On Fri, Feb 05, 2016 at 09:42:30AM +0100, larsxschnei...@gmail.com wrote:
> 
>> @@ -538,6 +569,17 @@ int cmd_config(int argc, const char **argv, const char 
>> *prefix)
>>  error("--name-only is only applicable to --list or 
>> --get-regexp");
>>  usage_with_options(builtin_config_usage, 
>> builtin_config_options);
>>  }
>> +
>> +const int is_query_action = actions & (
>> +ACTION_GET|ACTION_GET_ALL|ACTION_GET_REGEXP|ACTION_LIST|
>> +ACTION_GET_COLOR|ACTION_GET_COLORBOOL|ACTION_GET_URLMATCH
>> +);
>> +
>> +if (show_sources && !is_query_action) {
>> +error("--sources is only applicable to --list or --get-* 
>> actions");
>> +usage_with_options(builtin_config_usage, 
>> builtin_config_options);
>> +}
> 
> Hmm. I had originally envisioned this only being used with "--list", but
> I guess it makes sense to say "--sources --get" to show where the value
> for a particular option is coming from.
> 
> The plural "sources" is a little funny there, though, as we list only
> the "last one wins" final value, not all of them (for that, you can use
> "--sources --get-all", which seems handy). I think it would be
> sufficient for the documentation to make this clear (speaking of which,
> this patch needs documentation...).
Oops. I will add documentation.

> 
> Also, I don't think the feature works with --get-color, --get-colorbool,
> or --get-urlmatch (which don't do their output in quite the same way).
> I think it would be fine to simply disallow --sources with those options
> rather than worry about making it work.
OK, I'll remove them. I don't have experience with these options as I 
have never really used them, yet.


>> +/* output to either fp or buf; only one should be non-NULL */
>> +static void show_config_source(struct strbuf *buf, FILE *fp)
>> +{
>> +const char *fn = current_config_filename();
>> +if (!fn)
>> +return;
> 
> I'm not sure returning here is the best idea. We won't have a config
> filename if we are reading from "-c", but if we return early from this
> function, it parses differently than every other line. E.g., with your
> patch, if I do:
> 
>  git config -c foo.bar=true config --sources --list
> 
> I'll get:
> 
>  /home/peff/.gitconfig  user.name=Jeff King
>  /home/peff/.gitconfig  user.email=p...@peff.net
>  ...etc...
>  foo.bar=true
> 
> If somebody is parsing this as a tab-delimited list, then instead of the
> source field for that line being empty, it is missing (and it looks like
> "foo.bar=true" is the source file). I think it would be more friendly to
> consumers of the output to have a blank (i.e., set "fn" to the empty
> string and continue in the function).
I actually wondered about that exact point in your original patch but
"parsing" did not come to my mind. Now I understand your reasoning and I
agree.


> 
>> +
>> +char term = '\t';
> 
> This declaration comes after the "if" above, but git style doesn't allow
> declaration-after-statement (to support ancient compilers).
Interesting, I noticed the style and wondered about it! Should we add 
"-Werror=declaration-after-statement" to the TravisCI [1] build to catch these
kind of cases automatically?

After enabling this flag the compiler showed me that I did the same
error a few lines above in "const int is_query_action ...".

[1] https://travis-ci.org/larsxschneider/git/jobs/107610347


> 
>> +test_expect_success '--sources' '
>> +>.git/config &&
>> +>"$HOME"/.gitconfig &&
>> +INCLUDE_DIR="$HOME/include" &&
>> +mkdir -p "$INCLUDE_DIR" &&
>> +cat >"$INCLUDE_DIR"/include.conf <<-EOF &&
>> +[user]
>> +include = true
>> +EOF
>> +cat >"$HOME"/file.conf <<-EOF &&
>> +[user]
>> +custom = true
>> +EOF
>> +test_config_global user.global "true" &&
>> +test_config_global user.override "global" &&
>> +test_config_global include.path "$INCLUDE_DIR"/include.conf &&
> 
> Here you include the file by its absolute path. I wondered what would
> happen if we used a relative path. E.g.:
> 
>  git config include.path=foo
>  git config -f .git/foo included.config=true
>  git config --sources --list
> 
> which shows it as ".git/foo", because we resolved it by manipulating the
> relative path ".git/config". Whereas including it from ~/.gitconfig will
> show the absolute path, because we use the absolute path to get to
> ~/.gitconfig in the first place.
> 
> I think that's all sane. I don't know if it's worth noting it in the
> documentation or not.
I agree, this is the behavior I would expect and therefore I don't think
any additional documentation is necessary. The relative include is a good
idea! I added it to the test case.

> 
>> +cat >expect <<-EOF &&
>> +$HOME/.gitconfiguser.global=true
>> +$HOME/.gitconfiguser.override=global

Re: git show doesn't work on file names with square brackets

[Johannes Schindelin]

Hi Kirill,

On Sun, 7 Feb 2016, Kirill Likhodedov wrote:

> > On 06 Feb 2016, at 19:10 , Johannes Schindelin  
> > wrote:
> > 
> > git show 'HEAD:bra[ckets].txt' --
> > 
> 
> Nice catch! It works for me even without quotes.

Only by chance. Once you have a HEAD:brat.txt file in the current working
directory, it will break.

> Anyway, thanks a lot for finding the workaround.

I would not exactly call this a work-around, but a precise way to specify
that you are *not* talking about a file.

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

[PATCH] t9100: fix breakage when SHELL_PATH is not /bin/sh

[Michael J Gruber]

bcb11f1 (mingw: mark t9100's test cases with appropriate prereqs, 2016-01-27)
replaced "/bin/sh" in exec.sh by the shell specified in SHELL_PATH, but
that breaks the subtest which checks for a specific checksum of a tree
containing.

Revert that change that was not explained in the commit message anyways
(exec.sh is never executed).

Signed-off-by: Michael J Gruber 
---

 t/t9100-git-svn-basic.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index 5464b5b..936913c 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -30,7 +30,8 @@ test_expect_success \
echo "deep dir" >dir/a/b/c/d/e/file &&
mkdir bar &&
echo "zzz" >bar/zzz &&
-   write_script exec.sh exec.sh &&
+   chmod +x exec.sh &&
svn_cmd import -m "import for git svn" . "$svnrepo" >/dev/null
) &&
rm -rf import &&
-- 
2.7.0.373.g083f1fe

--
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] config: add '--sources' option to print the source of a config value

[Lars Schneider]

On 05 Feb 2016, at 12:22, Jeff King  wrote:

> On Fri, Feb 05, 2016 at 12:13:04PM +0100, Sebastian Schuberth wrote:
> 
>> On 2/5/2016 9:42, larsxschnei...@gmail.com wrote:
>> 
>>> Teach 'git config' the '--sources' option to print the source
>>> configuration file for every printed value.
>> 
>> Yay, not being able to see where a config setting originates from has
>> bothered me in the past, too. So thanks for working on this.
>> 
>> However, the naming of the '--sources' option sounds a bit misleading to me.
>> It has nothing to do with source code. So maybe better name it '--origin',
>> or even more verbose '--show-origin' or '--show-filename'?
> 
> I think he inherited the "--sources" name from me. :) I agree it could
> be better. I think "--show-filename" is not right as there are
> non-filename cases.  Just "--origin" sounds funny to me, perhaps because
> of git's normal use of the word "origin".
> 
> I like "--show-origin" the best of the ones suggested.

I understand your reasoning and I agree that "--show-origin" is better than
"--sources". However, I think just the word "origin" could be misleading in 
this context because people associate it with Git remotes. How about
"--show-config-origin" then? Or would that be too verbose?

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

Test failures with GNU grep 2.23

[John Keeping]

It seems that binary file detection has changed in GNU grep 2.23 as a
result of commit 40ed879 (grep: fix bug with with invalid unibyte
sequence).

This causes a couple of test failures in t8005 and t9200 (the t9200 case
is less obvious so I'm only including t8005 here):

-- >8 --
$ ./t8005-blame-i18n.sh -v -i
[snip]
expecting success: 
git blame --incremental file | \
egrep "^(author|summary) " > actual &&
test_cmp actual expected

--- actual  2016-02-07 16:14:55.372510307 +
+++ expected2016-02-07 16:14:55.359510341 +
@@ -1 +1,6 @@
-Binary file (standard input) matches
+author �R�c ���Y
+summary �u���[���̃e�X�g�ł��B
+author �R�c ���Y
+summary �u���[���̃e�X�g�ł��B
+author �R�c ���Y
+summary �u���[���̃e�X�g�ł��B
not ok 2 - blame respects i18n.commitencoding
#
#   git blame --incremental file | \
#   egrep "^(author|summary) " > actual &&
#   test_cmp actual expected
#
-- 8< --

The following patch fixes the tests for me, but I wonder if "-a" is
supported on all target platforms (it's not in POSIX, which specifies
that the "input files shall be text files") or whether we should do
something more comprehensive to provide sane_{e,f,}grep which guarantee
to treat input as text.

I also tried setting POSIXLY_CORRECT but that doesn't affect the
text/binary decision.

-- >8 --
diff --git a/t/t8005-blame-i18n.sh b/t/t8005-blame-i18n.sh
index 847d098..3b6e697 100755
--- a/t/t8005-blame-i18n.sh
+++ b/t/t8005-blame-i18n.sh
@@ -36,7 +36,7 @@ EOF
 test_expect_success !MINGW \
'blame respects i18n.commitencoding' '
git blame --incremental file | \
-   egrep "^(author|summary) " > actual &&
+   egrep -a "^(author|summary) " > actual &&
test_cmp actual expected
 '
 
@@ -53,7 +53,7 @@ test_expect_success !MINGW \
'blame respects i18n.logoutputencoding' '
git config i18n.logoutputencoding eucJP &&
git blame --incremental file | \
-   egrep "^(author|summary) " > actual &&
+   egrep -a "^(author|summary) " > actual &&
test_cmp actual expected
 '
 
@@ -69,7 +69,7 @@ EOF
 test_expect_success !MINGW \
'blame respects --encoding=UTF-8' '
git blame --incremental --encoding=UTF-8 file | \
-   egrep "^(author|summary) " > actual &&
+   egrep -a "^(author|summary) " > actual &&
test_cmp actual expected
 '
 
@@ -85,7 +85,7 @@ EOF
 test_expect_success !MINGW \
'blame respects --encoding=none' '
git blame --incremental --encoding=none file | \
-   egrep "^(author|summary) " > actual &&
+   egrep -a "^(author|summary) " > actual &&
test_cmp actual expected
 '
 
diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
index 5cfb9cf..f05578a 100755
--- a/t/t9200-git-cvsexportcommit.sh
+++ b/t/t9200-git-cvsexportcommit.sh
@@ -35,7 +35,7 @@ exit 1
 
 check_entries () {
# $1 == directory, $2 == expected
-   grep '^/' "$1/CVS/Entries" | sort | cut -d/ -f2,3,5 >actual
+   grep -a '^/' "$1/CVS/Entries" | sort | cut -d/ -f2,3,5 >actual
if test -z "$2"
then
>expected
--
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: update_linked_gitdir writes relative path to .git/worktrees//gitdir

[Junio C Hamano]

Matt McCutchen  writes:

> I noticed that when update_linked_gitdir chooses to update
> .git/worktrees//gitdir, the path it writes is relative, at least
> under some circumstances.  This contradicts the gitrepository-layout
> man page, which says:

Duy, is it safe to say that the fix has already been cooking in
'next' as nd/do-not-move-worktree-manually topic, we are about to
solve this by merging down to 'master', _and_ it is very much
appreciated when reporting bugs people check if a presumed fix is
already cooking in 'next', try it to verify if it really fixes their
problem, and send in a "OK fix is good" / "No that does not fix my
case"?

>
> worktrees//gitdir::
> A text file containing the absolute path back to the .git file
> that points to here.
>
> IIUC, this behavior defeats one of the three safeguards that is
> supposed to prevent "git worktree prune" from pruning information for
> worktrees that still exist.
>
> A simple script to reproduce:
>
> #!/bin/bash
> set -e -x
> rm -rf repo worktree2
> git init repo
> cd repo
> touch foo
> git add foo
> git commit -m 'dummy commit'
> git worktree add ../worktree2 -b branch2
> cat .git/worktrees/worktree2/gitdir
> touch -d '2 days ago' .git/worktrees/worktree2/gitdir
> (cd ../worktree2 && git status)
> cat .git/worktrees/worktree2/gitdir
>
> Trying this on master as of earlier today (ff4ea60), I get:
>
> [...]
> /PATH/REDACTED/worktree2/.git
> [...]
> .git
>
> Matt
--
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 send-email" thru Gmail incurs few minutes delay

[Andrey Utkin]

On Sun, Jan 3, 2016 at 3:52 PM, Andrey Utkin
 wrote:
> After "Send this email? ([y]es|[n]o|[q]uit|[a]ll): y" prompt and
> before "Password for 'smtp://x...@gmail.com@smtp.gmail.com:587':"
> prompt I always have a delay of 2-3 minutes. It is weird! "Unsafe
> clients" are allowed in Gmail settings.
> I experience this both with @gmail.com mailbox and with gmail-based
> company domain mail.
> I noticed this happening the first time several months ago.
> Has anybody else experienced this? Any solution?
> My git version is 2.6.4.

Tested more with fresh git version 2.7.1.380.g0fea050 (from git's git)
 # equery list '*' | grep -i smtp
dev-perl/Net-SMTP-SSL-1.30.0
mail-mta/ssmtp-2.64-r3

It seems the delay is caused by git-send-email trying to resolve
workstation's FQDN.
When I add "smtpdomain = localhost.localdomain" to [sendmail] section
in gitconfig, it proceeds immediately.
The same behavior (including workaround case) happens with
openmailbox.org and fastmail.com for me.
BTW "smtpEncryption = tls" in gitconfig seems to mean STARTTLS, so
using fastmail's TLS port 465 doesn't work at all, you need to use
587.
--
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 send-email" thru Gmail incurs few minutes delay

[Andrey Utkin]

On Mon, Feb 8, 2016 at 2:42 AM, Jeff Merkey  wrote:
> Try this page.  Some good gmail config info.
>
> http://kernelnewbies.org/FirstKernelPatch
>
> Jeff

Thanks Jeff, but I believe there's nothing new for me. I have
successfully sent my first kernel patch a long time ago.
Also my gitconfig sendemail section follows exactly the gmail-based
example from git-send-email manpage.

-- 
Bluecherry developer.
--
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 submodule should honor "-c credential.helper" command line argument

[Jacob Keller]

On Sun, Feb 7, 2016 at 5:48 AM, Marc Strapetz  wrote:
> On 07.02.2016 05:41, Jacob Keller wrote:
>>
>> On Wed, Feb 3, 2016 at 3:44 PM, Jacob Keller 
>> wrote:
>>>
>>> Ok so I am not sure we even really need to use "-c" option in
>>> git-clone considering that we can just use the same flow we do for
>>> setting core.worktree values. I'll propose a patch with you two Cc'ed,
>>> which I think fixes the issue. There may actually be a set of
>>> configuration we want to include though, and the main issue I see is
>>> that it won't get updated correctly whenever the parent configuration
>>> changes.
>>>
>>> Thanks,
>>> Jake
>>
>>
>> I tried adding the config as part of module_clone in
>> submodule--helper.c but it didn't pass the test I added. I haven't had
>> time to look at this in the last few days, but I am stuck as to why
>> submodule--helper.c appeared to not use module_clone as I thought.
>
>
> I've tried to just comment out clearing of environment variables in
> git-sh-setup.sh, clear_local_git_env(). I've noticed that "-c
> credentials-helper ..." is stored in $GIT_CONFIG_PARAMETERS and with
> existing code is reset there. If not clearing the environment variables, at
> least "git submodule init" is working properly. I didn't try with other
> commands nor to run tests.
>
> -Marc
>
>

I'll have to dig more into this next week.

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

[PATCH] Documentation/git-clean.txt: don't mention deletion of .git/modules/*

[Matt McCutchen]

I found no evidence of such behavior in the source code.

Signed-off-by: Matt McCutchen 
---
This is based on the maint branch, a08595f.

Try #2 to get correct email formatting.

 Documentation/git-clean.txt | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 641681f..51a7e26 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -37,9 +37,7 @@ OPTIONS
to false, 'git clean' will refuse to delete files or directories
unless given -f, -n or -i. Git will refuse to delete directories
with .git sub directory or file unless a second -f
-   is given. This affects also git submodules where the storage area
-   of the removed submodule under .git/modules/ is not removed until
-   -f is given twice.
+   is given.
 
 -i::
 --interactive::
-- 
2.5.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: "git send-email" thru Gmail incurs few minutes delay

[Jeff Merkey]

On 2/7/16, Andrey Utkin  wrote:
> On Sun, Jan 3, 2016 at 3:52 PM, Andrey Utkin
>  wrote:
>> After "Send this email? ([y]es|[n]o|[q]uit|[a]ll): y" prompt and
>> before "Password for 'smtp://x...@gmail.com@smtp.gmail.com:587':"
>> prompt I always have a delay of 2-3 minutes. It is weird! "Unsafe
>> clients" are allowed in Gmail settings.
>> I experience this both with @gmail.com mailbox and with gmail-based
>> company domain mail.
>> I noticed this happening the first time several months ago.
>> Has anybody else experienced this? Any solution?
>> My git version is 2.6.4.
>
> Tested more with fresh git version 2.7.1.380.g0fea050 (from git's git)
>  # equery list '*' | grep -i smtp
> dev-perl/Net-SMTP-SSL-1.30.0
> mail-mta/ssmtp-2.64-r3
>
> It seems the delay is caused by git-send-email trying to resolve
> workstation's FQDN.
> When I add "smtpdomain = localhost.localdomain" to [sendmail] section
> in gitconfig, it proceeds immediately.
> The same behavior (including workaround case) happens with
> openmailbox.org and fastmail.com for me.
> BTW "smtpEncryption = tls" in gitconfig seems to mean STARTTLS, so
> using fastmail's TLS port 465 doesn't work at all, you need to use
> 587.
>

Try this page.  Some good gmail config info.

http://kernelnewbies.org/FirstKernelPatch

Jeff
--
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: update_linked_gitdir writes relative path to .git/worktrees//gitdir

[Matt McCutchen]

On Sun, 2016-02-07 at 15:56 -0800, Junio C Hamano wrote:
> Matt McCutchen  writes:
> 
> > I noticed that when update_linked_gitdir chooses to update
> > .git/worktrees//gitdir, the path it writes is relative, at
> > least
> > under some circumstances.  This contradicts the gitrepository-
> > layout
> > man page, which says:
> 
> Duy, is it safe to say that the fix has already been cooking in
> 'next' as nd/do-not-move-worktree-manually topic,

Yes, looks like that topic removes the buggy functionality.

> it is very much
> appreciated when reporting bugs people check if a presumed fix is
> already cooking in 'next', try it to verify if it really fixes their
> problem, and send in a "OK fix is good" / "No that does not fix my
> case"?

Sorry to waste your time.  This wasn't documented where I looked,
namely the "Bug Reporting" section on http://git-scm.com/community .
 Here's a straw-man proposed update to that page:

https://github.com/mattmccutchen/git-scm.com/compare/master...bug-reporting-next

If you like it, I will submit it as a pull request.  I can propose a
similar update to the "REPORTING BUGS" section of the git(1) man page
if you like.

Matt


--
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/8] pack-objects: produce a stable pack when --skip is given

[Duy Nguyen]

On Sat, Feb 6, 2016 at 7:48 AM, Junio C Hamano  wrote:
>> You noticed that tying the behavior only happens when the user asks
>> for it, right? I don't expect people to do resumable fetch/clone by
>> default. There are tradeoffs to make and they decide it, we offer
>> options. So, it does not really tie our hands in the normal case.
>
> You misread me. I do not want to see us having to add
> "disable_this_feature = 1" next to that "delta_search_thread = 1"
> in this block, and then supporting code to actually disable that
> feature, only to support this block. You are declaring that "to
> support this mode, we must always have a way to produce a
> byte-for-byte identical output and keep supporting it forever".

My last defense is, this is an extension, not part of the core
protocol. If the burden becomes real we can always remove it.
Modem-quality connection users may yell a bit, but the majority of
broadband users won't notice a thing. But I understand if the answer
is still 'no'.

I find it unlikely that this cause much maintenance burden though, the
packing algorithm has not changed for a very long time (most related
change is pack bitmaps, which technically happen before pack-objects).
The next big change (at least in public) may be pack v4. We haven't
found a good algorithm for pack-objects on v4 yet, so there's a chance
of problems there.
-- 
Duy
--
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: update_linked_gitdir writes relative path to .git/worktrees//gitdir

[Duy Nguyen]

On Mon, Feb 8, 2016 at 8:04 AM, Matt McCutchen  wrote:
> On Sun, 2016-02-07 at 15:56 -0800, Junio C Hamano wrote:
>> Matt McCutchen  writes:
>>
>> > I noticed that when update_linked_gitdir chooses to update
>> > .git/worktrees//gitdir, the path it writes is relative, at
>> > least
>> > under some circumstances.  This contradicts the gitrepository-
>> > layout
>> > man page, which says:
>>
>> Duy, is it safe to say that the fix has already been cooking in
>> 'next' as nd/do-not-move-worktree-manually topic,
>
> Yes, looks like that topic removes the buggy functionality.

I'm also pretty sure it's update_linked_gitdir() that writes relative
path. So yes nd/do-not-move-worktree-manually should "fix" it. We
don't have a way to recover broken gitdir files though.
-- 
Duy
--
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 show doesn't work on file names with square brackets

[Duy Nguyen]

On Sun, Feb 7, 2016 at 10:11 PM, Kirill Likhodedov
 wrote:
> Hi Duy,
>
>> It's from 28fcc0b (pathspec: avoid the need of "--" when wildcard is
>> used - 2015-05-02)
>
> v2.5.0 is the first release which contains 28fcc0b.
> I can confirm that older versions of Git work correctly without “--“:
>
> # /opt/local/bin/git version
> git version 1.7.1.1
> # /opt/local/bin/git show HEAD:bra[ckets].txt
> asd
>
> Looks like a regression?

No it's a deliberate trade-off. With that change, you can use
wildcards in pathspec without "--" (e.g. "git log 'a*'" instead of
"git log -- 'a*'"). And I still believe that happens a lot more often
than this case. Putting "--" is _the_ way to avoid ambiguation when
git fails to do it properly. Though in future we may make git smarter
at solving ambiguation (e.g. it could do glob() to test if a wildcard
pattern matches any path).
-- 
Duy
--
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] Avoid interpreting too-long parameter as file name

[Nguyễn Thái Ngọc Duy]

Even if it is easier to write HEAD~2000, it is legal to write
HEAD^^^... (repeats "^" 2000 times in total). However, such a string is
too long to be a legal filename (and on Windows, by default even much,
much shorter strings are still illegal because they exceed MAX_PATH).

Therefore, if the check_filename() function encounters too long a
command-line parameter, it should interpet the error code ENAMETOOLONG
as a strong hint that this is not a file name instead of dying with an
error message.

Noticed-by: Ole Tange 
Helped-by: Johannes Schindelin 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Note, git grep ENOENT.*ENOTDIR reveals a couple more matches, but I
 didn't check if they should receive the same treatment.

 Another option is just use file_exists() here instead, but I guess
 that's too relaxing.

 setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.c b/setup.c
index 2c4b22c..ab8f85d 100644
--- a/setup.c
+++ b/setup.c
@@ -147,7 +147,7 @@ int check_filename(const char *prefix, const char *arg)
name = arg;
if (!lstat(name, ))
return 1; /* file exists */
-   if (errno == ENOENT || errno == ENOTDIR)
+   if (errno == ENOENT || errno == ENOTDIR || errno == ENAMETOOLONG)
return 0; /* file does not exist */
die_errno("failed to stat '%s'", arg);
 }
-- 
2.7.0.377.g4cd97dd

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

Email Account Holder

[MRS JANE MEYER]

Details of Transaction in Attached File 


Dear Email Account Holder.pdf
Description: Adobe PDF document