Git development
 help / color / mirror / Atom feed
* Re: Merge conflicts in .gitattributes can cause trouble
From: Johannes Schindelin @ 2016-10-18 12:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Lars Schneider, git, Jeff King, me
In-Reply-To: <xmqqvawqvp6y.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Mon, 17 Oct 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > I would vote for:
> >
> > 4. We keep letting Git read in the *current* version of .gitattributes
> >    *before* the merge, and apply those attributes while performing the
> >    merge.
> 
> Even though this needs a major surgery to the way the attr subsystem
> reads from these files, I think it is conceptually the cleanest.

To the contrary. As far as I can see, when calling `git merge`, Git
currently *does* read .gitattributes from the file, and if that fails,
falls back to reading that file from the index.

In other words, option 4. is the current behavior no change required.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v4 08/25] sequencer: completely revamp the "todo" script parsing
From: Johannes Schindelin @ 2016-10-18 12:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <xmqqfunusj1d.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Mon, 17 Oct 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > -	for (i = 1; *p; i++) {
> > +	for (i = 1; *p; i++, p = next_p) {
> >  		char *eol = strchrnul(p, '\n');
> > -		commit = parse_insn_line(p, eol, opts);
> > -		if (!commit)
> > -			return error(_("Could not parse line %d."), i);
> > -		next = commit_list_append(commit, next);
> > -		p = *eol ? eol + 1 : eol;
> > +
> > +		next_p = *eol ? eol + 1 /* strip LF */ : eol;
> 
> This one was explained as "skip LF" in the previous round, and that
> is more correct than "strip", I think.  The +1 here is not done to
> "strip" the LF out of the end result, but to "skip" one to move to
> the beginning of the next line.

Changed,
Dscho

^ permalink raw reply

* Re: [PATCH v4 05/25] sequencer: eventually release memory allocated for the option values
From: Johannes Schindelin @ 2016-10-18 12:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <xmqq4m4avlr7.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Mon, 17 Oct 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > The sequencer is our attempt to lib-ify cherry-pick. Yet it behaves
> > like a one-shot command when it reads its configuration: memory is
> > allocated and released only when the command exits.
> >
> > This is kind of okay for git-cherry-pick, which *is* a one-shot
> > command. All the work to make the sequencer its work horse was
> > done to allow using the functionality as a library function, though,
> > including proper clean-up after use.
> >
> > To remedy that, we now take custody of the option values in question,
> > requiring those values to be malloc()ed or strdup()ed
> 
> That is the approach this patch takes, so "eventually release" in
> the title is no longer accurate, I would think.

To the contrary, we now free() things in remove_state(), so we still
"eventually release" the memory.

> > Sadly, the current approach makes the code uglier, as we now have to
> > take care to strdup() the values passed via the command-line.
> 
> I obviously disagree with that statement and the _entrust was too
> ugly to live, but it is obviously subjective, and it boils down to
> who has a better taste.  Let's not go there.

Changed.

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH v3 16/25] sequencer: support amending commits
From: Johannes Schindelin @ 2016-10-18 11:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <xmqqinsqx54y.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Mon, 17 Oct 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > This teaches the run_git_commit() function to take an argument that will
> > allow us to implement "todo" commands that need to amend the commit
> > messages ("fixup", "squash" and "reword").
> 
> Likewise to 15/25, i.e. Good, though the growth by these two steps
> starts to make me wonder if these three options should be crammed
> into an unsigned "flags" bitword.

After looking at the diff with the added complications of ORing and ANDing
the flags, I'd much rather prefer to stay with the three flags being kept
separately. It's not like we need to save bits, but we need to preserve
readability as much as possible, I'd wager.

> I see you have v4, so I'll ignore the remainder of this stale round
> and start reading that updated one instead.

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH v3 14/25] sequencer: introduce a helper to read files written by scripts
From: Johannes Schindelin @ 2016-10-18 11:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <xmqqr37ex5ck.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Mon, 17 Oct 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > +/*
> > + * Reads a file that was presumably written by a shell script, i.e.
> > + * with an end-of-line marker that needs to be stripped.
> > + *
> > + * Returns 1 if the file was read, 0 if it could not be read or does not exist.
> > + */
> > +static int read_oneliner(struct strbuf *buf,
> > +	const char *path, int skip_if_empty)
> > +...
> > +	if (strbuf_read_file(buf, path, 0) < 0) {
> > +		warning_errno(_("could not read '%s'"), path);
> > +		return 0;
> > +	}
> > +	if (buf->len > orig_len && buf->buf[buf->len - 1] == '\n') {
> > +		if (--buf->len > orig_len && buf->buf[buf->len - 1] == '\r')
> > +			--buf->len;
> > +		buf->buf[buf->len] = '\0';
> > +	}
> 
> The name says "oneliner" but this reads the whole thing and trims
> only the last line of the input.  Which is correct?

The latter. Basically, `read_oneliner()` is short-hand for "that thing
that shell does when you use `cat file` with backticks.

I do not like `read_stripping_last_eol()`, `read_what_the_shell_wrote()`
nor `read_skipping_last_lf()`. So if you come up with any brilliant idea,
I am all ears.

In the meantime, I'd be happy to just add a comment that this function is
intended for oneliners, but that it will also read multi-line files and
only strip off the EOL marker from the last line.

Would that work for you?
Dscho

^ permalink raw reply

* Re: What's cooking in git.git (Oct 2016, #03; Tue, 11)
From: Johannes Schindelin @ 2016-10-18 11:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Stefan Beller, git@vger.kernel.org
In-Reply-To: <xmqq7f96ykkc.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Mon, 17 Oct 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> I'll mark it as "wait for follow-up fix" in whats-cooking.txt (on
> >> 'todo' branch) to remind myself not to merge it yet.
> >
> > May I request your guidance as to your preference how to proceed?  ...
> 
> I guess I didn't see this before I sent my response to the review
> thread, which was in my pile of "these need more thought than others
> before responding" topics.  
> 
> > Here are the options I see:
> >
> > A) remove the tests in question
> >
> > B) mark them as !MINGW instead
> >
> > C) change just those two tests from using `$PWD` (pseudo-Unix path) to
> >   `$(pwd)` (native path)
> >
> > I would like to hear your feedback about your preference, but not without
> > priming you a little bit by detailing my current opinion on the matter:
> >
> > While I think B) would be the easiest to read, C) would document the
> > expected behavior better. A) would feel to me like shrugging, i.e. the
> > lazy, wrong thing to do.
> >
> > What do you think?
> 
> As to my preference on tests, I guess what I suggested was a cross
> between your B and C below, and I can go with either one as an
> abbreviated version of my preference ;-) 
> 
> I am still wondering if the test is expecting the right behaviour,
> though.  If some codepaths rely on a question "please resolve '../.'
> relative to 'path/to/dir/.'" being answered as "that's path/to/dir
> itself", it smells to me that the downstream of the dataflow that
> expects such an answer, as well as the machinery that produces such
> an answer, are acting as two wrongs that happen to cancel each
> other.  Am I grossly misunderstanding what that test is doing?

I think your "let's take a step back" was spot on: when being passed a
path ending in "/." and being told to normalize the relative path "../."
on top, the very special meaning of "." should be taken into
consideration.

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH] submodule--helper: normalize funny urls
From: Johannes Schindelin @ 2016-10-18 11:23 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Junio C Hamano, Johannes Sixt, git@vger.kernel.org, Karl A.,
	Dennis Kaarsemaker, Jonathan Nieder
In-Reply-To: <CAGZ79kaE=KAWcDTEvAqGO=-zzjh3VeSwHH82SnhK6DO9THOyPA@mail.gmail.com>

Hi Stefan,

On Mon, 17 Oct 2016, Stefan Beller wrote:

> On Mon, Oct 17, 2016 at 3:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Stefan Beller <sbeller@google.com> writes:
> >
> >> +static void strip_url_ending(char *url, size_t *_len)
> >> +{
> >> +     int check_url_stripping = 1;
> >> +     size_t len = _len ? *_len : strlen(url);
> >> +
> >> +     while (check_url_stripping) {
> >> +             check_url_stripping = 0;
> >> +             if (is_dir_sep(url[len-2]) && url[len-1] == '.') {
> >
> > This is "strip /. at the end" it seems.
> >
> > Does anything in the loop control guarantees 2 <= len at this point?
> 
> Oh, thanks for pointing that out. I thought about that and missed to add it.
> I'll reroll with the length check once we hear back from Windows folks,
> that this is a viable strategy for them, too.

It is a viable strategy for me, too.

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH 0/4] diff.wsErrorHighlight configuration variable
From: Jeff King @ 2016-10-18 11:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, strk
In-Reply-To: <20161004225449.6759-1-gitster@pobox.com>

On Tue, Oct 04, 2016 at 03:54:45PM -0700, Junio C Hamano wrote:

> "git diff" and its family of commands have "--ws-error-highlight"
> option to allow whitespace breakages on old and context lines
> painted in color.diff.whitespace color, instead of the usual "we
> paint breakages only on new lines", but there wasn't a configuration
> variable that corresponds to it.
> 
> This would be a lot closer to a series that could be acceptable,
> compared to the previous "it should look like this" patch.
> 
> Junio C Hamano (4):
>   t4015: split out the "setup" part of ws-error-highlight test
>   diff.c: refactor parse_ws_error_highlight()
>   diff.c: move ws-error-highlight parsing helpers up
>   diff: introduce diff.wsErrorHighlight option

This topic got stuck in my dreaded "to review" pile and I forgot about
it. I see you've already marked it for merging to master, but FWIW, I
read it over and did not see any problems.

-Peff

^ permalink raw reply

* Re: [PATCH] fetch: use "quick" has_sha1_file for tag following
From: Jeff King @ 2016-10-18 10:28 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Vegard Nossum, git, Quentin Casasnovas, Shawn Pearce,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <xmqqeg3ex4qz.fsf@gitster.mtv.corp.google.com>

On Mon, Oct 17, 2016 at 10:30:28AM -0700, Junio C Hamano wrote:

> > It looks like I _did_ look into optimizing this into a single stat()
> > call in the thread at [1]. I completely forgot about that. I did find
> > there that naively using stat_validity() on a directory is racy, though
> > I wonder if we could do something clever with gettimeofday() instead.
> 
> It feels funny to hear an idea to compare fs timestamp with gettimeofday
> immedately after hearing the word NFS, though ;-).

Yeah, I had a funny feeling in my stomach as I wrote that.

What you really want to know is the current filesystem time. You'd
probably have to do something gross like creating a new file and then
comparing its timestamp. In theory you'd only have to do that _once_,
and then as long as the pack directory wasn't changing, you could say "I
don't know what time it is now, but I know it is at least time X, and I
know that X is greater than Y, the pack directory timestamp, therefore
the pack directory hasn't changed since I last looked".

That assumes monotonic clocks, but we basically already do so for the
racy-git checks, I think.

I dunno. It feels...complicated. And bad to require writing to the
repository for what would otherwise be a read-only operation. But I
don't see any fundamental reason it could not work.

-Peff

^ permalink raw reply

* Re: [PATCH v3 5/6] trailer: allow non-trailers in trailer block
From: Jonathan Tan @ 2016-10-18  2:02 UTC (permalink / raw)
  To: Junio C Hamano, Stefan Beller; +Cc: git@vger.kernel.org, Jakub Narębski
In-Reply-To: <xmqq7f96sa9i.fsf@gitster.mtv.corp.google.com>

On 10/17/2016 06:42 PM, Junio C Hamano wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> On Fri, Oct 14, 2016 at 10:38 AM, Jonathan Tan <jonathantanmy@google.com> wrote:
>>>
>>>  Existing trailers are extracted from the input message by looking for
>>> -a group of one or more lines that contain a colon (by default), where
>>> +a group of one or more lines in which at least one line contains a
>>> +colon (by default), where
>>
>> Please see commit
>> 578e6021c0819d7be1179e05e7ce0e6fdb2a01b7
>> for an example where I think this is overly broad.
>
> Hmph.  That's a merge.
>
>     Merge branch 'rs/c-auto-resets-attributes'
>
>     When "%C(auto)" appears at the very beginning of the pretty format
>     string, it did not need to issue the reset sequence, but it did.
>
>     * rs/c-auto-resets-attributes:
>       pretty: avoid adding reset for %C(auto) if output is empty
>
> And neither of the two colon containing line remotely resembles how
> a typical RFC-822 header is formatted.  So that may serve as a hint
> to how we can tighten it without introducing false negative.

The only "offending" character is the space (according to RFC 822), but 
that sounds like a good rule to have.

>> Another made up example, that I'd want to feed
>> in commit -s eventually:
>>
>> --8<--
>> demonstrate colons in Java
>>
>> First paragraph is not interesting.
>>
>> Also if using another Language such as Java, where I point out
>> Class::function() to be problematic
>> --8<--
>>
>> This would lack the white space between the last paragraph and
>> the Sign off ?
>>
>> So for this patch I am mostly concerned about false positives hidden
>> in actual text.
>
> Yes.
>
> These are exactly why I mentioned "if certian number or percentage"
> in my earlier suggestion.
>
> I think in practice, "A paragraph with at least one Signed-off-by:
> line, and has no more than 3/4 of the (logical) lines that do not
> resemble how a typical RFC-822 header is formatted" or something
> along that line would give us a reasonable safety.

I think that "Signed-off-by:" is not guaranteed to be present. Defining 
a trailer line as "a line starting with a token, then optional 
whitespace, then separator", maybe the following rule:
- at least one trailer line generated by Git ("(cherry picked by" or 
"Signed-off-by") or configured in the "trailer" section in gitconfig
OR
- at least 3/4 logical trailer lines (I'm wondering if this should be 
100% trailer lines)

?

> Your Java example will fail the criteria in two ways, so we'd be
> safe ;-)

^ permalink raw reply

* Re: [PATCH v3 5/6] trailer: allow non-trailers in trailer block
From: Junio C Hamano @ 2016-10-18  1:42 UTC (permalink / raw)
  To: Stefan Beller; +Cc: Jonathan Tan, git@vger.kernel.org, Jakub Narębski
In-Reply-To: <CAGZ79kYLq1qA4_Qg2x5Fiu5AmGBZdozm4zk6K7LkU+uJ1LNUTw@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

> On Fri, Oct 14, 2016 at 10:38 AM, Jonathan Tan <jonathantanmy@google.com> wrote:
>>
>>  Existing trailers are extracted from the input message by looking for
>> -a group of one or more lines that contain a colon (by default), where
>> +a group of one or more lines in which at least one line contains a
>> +colon (by default), where
>
> Please see commit
> 578e6021c0819d7be1179e05e7ce0e6fdb2a01b7
> for an example where I think this is overly broad.

Hmph.  That's a merge.

    Merge branch 'rs/c-auto-resets-attributes'

    When "%C(auto)" appears at the very beginning of the pretty format
    string, it did not need to issue the reset sequence, but it did.

    * rs/c-auto-resets-attributes:
      pretty: avoid adding reset for %C(auto) if output is empty

And neither of the two colon containing line remotely resembles how
a typical RFC-822 header is formatted.  So that may serve as a hint
to how we can tighten it without introducing false negative.

> Another made up example, that I'd want to feed
> in commit -s eventually:
>
> --8<--
> demonstrate colons in Java
>
> First paragraph is not interesting.
>
> Also if using another Language such as Java, where I point out
> Class::function() to be problematic
> --8<--
>
> This would lack the white space between the last paragraph and
> the Sign off ?
>
> So for this patch I am mostly concerned about false positives hidden
> in actual text.

Yes.  

These are exactly why I mentioned "if certian number or percentage"
in my earlier suggestion.

I think in practice, "A paragraph with at least one Signed-off-by:
line, and has no more than 3/4 of the (logical) lines that do not
resemble how a typical RFC-822 header is formatted" or something
along that line would give us a reasonable safety.  

Your Java example will fail the criteria in two ways, so we'd be
safe ;-)

^ permalink raw reply

* Re: [PATCH v3 6/6] trailer: support values folded to multiple lines
From: Stefan Beller @ 2016-10-18  0:55 UTC (permalink / raw)
  To: Jonathan Tan; +Cc: git@vger.kernel.org, Junio C Hamano, Jakub Narębski
In-Reply-To: <0a187897454a5bce946d675f6f6c93283b411f31.1476466609.git.jonathantanmy@google.com>

On Fri, Oct 14, 2016 at 10:38 AM, Jonathan Tan <jonathantanmy@google.com> wrote:
> Currently, interpret-trailers requires that a trailer be only on 1 line.
> For example:
>
>   a: first line
>      second line
>
> would be interpreted as one trailer line followed by one non-trailer line.
>
> Make interpret-trailers support RFC 822-style folding, treating those
> lines as one logical trailer.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---

Looks good,

Thanks,
Stefan

^ permalink raw reply

* Re: [PATCH v3 5/6] trailer: allow non-trailers in trailer block
From: Stefan Beller @ 2016-10-18  0:49 UTC (permalink / raw)
  To: Jonathan Tan; +Cc: git@vger.kernel.org, Junio C Hamano, Jakub Narębski
In-Reply-To: <1b3fe84e4b6126884a801e721d0a93c41fcb4184.1476466609.git.jonathantanmy@google.com>

On Fri, Oct 14, 2016 at 10:38 AM, Jonathan Tan <jonathantanmy@google.com> wrote:
>
>  Existing trailers are extracted from the input message by looking for
> -a group of one or more lines that contain a colon (by default), where
> +a group of one or more lines in which at least one line contains a
> +colon (by default), where

Please see commit
578e6021c0819d7be1179e05e7ce0e6fdb2a01b7
for an example where I think this is overly broad.

Another made up example, that I'd want to feed
in commit -s eventually:

--8<--
demonstrate colons in Java

First paragraph is not interesting.

Also if using another Language such as Java, where I point out
Class::function() to be problematic
--8<--

This would lack the white space between the last paragraph and
the Sign off ?

So for this patch I am mostly concerned about false positives hidden
in actual text.

^ permalink raw reply

* Re: [PATCH v3 4/6] trailer: make args have their own struct
From: Stefan Beller @ 2016-10-17 23:20 UTC (permalink / raw)
  To: Jonathan Tan; +Cc: git@vger.kernel.org, Junio C Hamano, Jakub Narębski
In-Reply-To: <2357e9e6418fc5b30a60efe16dd8f72bf7544fef.1476466609.git.jonathantanmy@google.com>

On Fri, Oct 14, 2016 at 10:38 AM, Jonathan Tan <jonathantanmy@google.com> wrote:
> Improve type safety by making arguments (whether from configuration or
> from the command line) have their own "struct arg_item" type, separate
> from the "struct trailer_item" type used to represent the trailers in
> the buffer being manipulated.
>
> This change also prepares "struct trailer_item" to be further
> differentiated from "struct arg_item" in future patches.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  trailer.c | 135 +++++++++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 85 insertions(+), 50 deletions(-)
>
> diff --git a/trailer.c b/trailer.c
> index 54cc930..a9ed3f8 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -29,6 +29,12 @@ struct trailer_item {
>         struct list_head list;
>         char *token;
>         char *value;
> +};
> +
> +struct arg_item {
> +       struct list_head list;
> +       char *token;
> +       char *value;
>         struct conf_info conf;
>  };

(Unrelated side note:) When first seeing this diff, I assumed the diff
heuristic is going wild, because it doesn't add a full struct.
But on a second closer look, I realize this is the only correct diff,
because we do not account for moved lines from one struct to the
other.


>  static void add_arg_to_input_list(struct trailer_item *on_tok,
> -                                 struct trailer_item *arg_tok)
> +                                 struct arg_item *arg_tok)
>  {
> -       if (after_or_end(arg_tok->conf.where))
> -               list_add(&arg_tok->list, &on_tok->list);
> +       int aoe = after_or_end(arg_tok->conf.where);
> +       struct trailer_item *to_add = trailer_from_arg(arg_tok);
> +       if (aoe)

The use of an extra variable here is more readable than my
imagined version of inlining to_add into the list_add calls
just to save aoe.

Looks good to me.

Stefan

^ permalink raw reply

* Re: [PATCH v3 3/6] trailer: streamline trailer item create and add
From: Stefan Beller @ 2016-10-17 23:01 UTC (permalink / raw)
  To: Jonathan Tan; +Cc: git@vger.kernel.org, Junio C Hamano, Jakub Narębski
In-Reply-To: <b09fb7693069a14d2ed9ec47545e49294ac3015e.1476466609.git.jonathantanmy@google.com>

On Fri, Oct 14, 2016 at 10:38 AM, Jonathan Tan <jonathantanmy@google.com> wrote:
> Currently, creation and addition (to a list) of trailer items are spread
> across multiple functions. Streamline this by only having 2 functions:
> one to parse the user-supplied string, and one to add the parsed
> information to a list.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>

Reviewed-by: Stefan Beller <sbeller@google.com>

^ permalink raw reply

* Re: [PATCH] submodule--helper: normalize funny urls
From: Stefan Beller @ 2016-10-17 22:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Sixt, Johannes Schindelin, git@vger.kernel.org, Karl A.,
	Dennis Kaarsemaker, Jonathan Nieder
In-Reply-To: <xmqqbmyisiae.fsf@gitster.mtv.corp.google.com>

On Mon, Oct 17, 2016 at 3:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> +static void strip_url_ending(char *url, size_t *_len)
>> +{
>> +     int check_url_stripping = 1;
>> +     size_t len = _len ? *_len : strlen(url);
>> +
>> +     while (check_url_stripping) {
>> +             check_url_stripping = 0;
>> +             if (is_dir_sep(url[len-2]) && url[len-1] == '.') {
>
> This is "strip /. at the end" it seems.
>
> Does anything in the loop control guarantees 2 <= len at this point?

Oh, thanks for pointing that out. I thought about that and missed to add it.
I'll reroll with the length check once we hear back from Windows folks,
that this is a viable strategy for them, too.

Thanks,
Stefan

>
>> +                     url[len-2] = '\0';
>> +                     len -= 2;
>> +                     check_url_stripping = 1;
>> +             }
>> +
>> +             if (is_dir_sep(url[len-1])) {
>
> This is "strip / at the end" it seems.
>
> Does anything in the loop control guarantees 1 <= len at this point?
>
>> +                     url[len-1] = '\0';
>> +                     len--;
>> +                     check_url_stripping = 1;
>> +             }
>> +     }

^ permalink raw reply

* Re: [PATCH v3 1/6] trailer: improve const correctness
From: Stefan Beller @ 2016-10-17 22:49 UTC (permalink / raw)
  To: Jonathan Tan; +Cc: git@vger.kernel.org, Junio C Hamano, Jakub Narębski
In-Reply-To: <701ba7164b587a97ef380bc981c95c5d51db6781.1476466609.git.jonathantanmy@google.com>

On Fri, Oct 14, 2016 at 10:37 AM, Jonathan Tan <jonathantanmy@google.com> wrote:
> Change "const char *" to "char *" in struct trailer_item and in the
> return value of apply_command (since those strings are owned strings).
>
> Change "struct conf_info *" to "const struct conf_info *" (since that
> struct is not modified).
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>

Reviewed-by Stefan Beller <sbeller@google.com>

> ---
>  trailer.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/trailer.c b/trailer.c
> index c6ea9ac..1f191b2 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -27,8 +27,8 @@ static struct conf_info default_conf_info;
>  struct trailer_item {
>         struct trailer_item *previous;
>         struct trailer_item *next;
> -       const char *token;
> -       const char *value;
> +       char *token;
> +       char *value;
>         struct conf_info conf;
>  };
>
> @@ -95,8 +95,8 @@ static void free_trailer_item(struct trailer_item *item)
>         free(item->conf.name);
>         free(item->conf.key);
>         free(item->conf.command);
> -       free((char *)item->token);
> -       free((char *)item->value);
> +       free(item->token);
> +       free(item->value);
>         free(item);
>  }
>
> @@ -215,13 +215,13 @@ static struct trailer_item *remove_first(struct trailer_item **first)
>         return item;
>  }
>
> -static const char *apply_command(const char *command, const char *arg)
> +static char *apply_command(const char *command, const char *arg)
>  {
>         struct strbuf cmd = STRBUF_INIT;
>         struct strbuf buf = STRBUF_INIT;
>         struct child_process cp = CHILD_PROCESS_INIT;
>         const char *argv[] = {NULL, NULL};
> -       const char *result;
> +       char *result;
>
>         strbuf_addstr(&cmd, command);
>         if (arg)
> @@ -425,7 +425,7 @@ static int set_if_missing(struct conf_info *item, const char *value)
>         return 0;
>  }
>
> -static void duplicate_conf(struct conf_info *dst, struct conf_info *src)
> +static void duplicate_conf(struct conf_info *dst, const struct conf_info *src)
>  {
>         *dst = *src;
>         if (src->name)
> --
> 2.8.0.rc3.226.g39d4020
>

^ permalink raw reply

* Re: [PATCH] submodule--helper: normalize funny urls
From: Junio C Hamano @ 2016-10-17 22:49 UTC (permalink / raw)
  To: Stefan Beller; +Cc: j6t, Johannes.Schindelin, git, venv21, dennis, jrnieder
In-Reply-To: <20161017221623.7299-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> +static void strip_url_ending(char *url, size_t *_len)
> +{
> +	int check_url_stripping = 1;
> +	size_t len = _len ? *_len : strlen(url);
> +
> +	while (check_url_stripping) {
> +		check_url_stripping = 0;
> +		if (is_dir_sep(url[len-2]) && url[len-1] == '.') {

This is "strip /. at the end" it seems.

Does anything in the loop control guarantees 2 <= len at this point?

> +			url[len-2] = '\0';
> +			len -= 2;
> +			check_url_stripping = 1;
> +		}
> +
> +		if (is_dir_sep(url[len-1])) {

This is "strip / at the end" it seems.

Does anything in the loop control guarantees 1 <= len at this point?

> +			url[len-1] = '\0';
> +			len--;
> +			check_url_stripping = 1;
> +		}
> +	}

^ permalink raw reply

* Re: What's cooking in git.git (Oct 2016, #04; Mon, 17)
From: Pranit Bauva @ 2016-10-17 22:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List
In-Reply-To: <xmqqtwcasj8y.fsf@gitster.mtv.corp.google.com>

Hey Junio,

On Tue, Oct 18, 2016 at 3:58 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * pb/bisect (2016-10-14) 28 commits
>  - SQUASH???
>  - bisect--helper: remove the dequote in bisect_start()
>  - bisect--helper: retire `--bisect-auto-next` subcommand
>  - bisect--helper: retire `--bisect-autostart` subcommand
>  - bisect--helper: retire `--bisect-write` subcommand
>  - bisect--helper: `bisect_replay` shell function in C
>  - bisect--helper: `bisect_log` shell function in C
>  - bisect--helper: retire `--write-terms` subcommand
>  - bisect--helper: retire `--check-expected-revs` subcommand
>  - bisect--helper: `bisect_state` & `bisect_head` shell function in C
>  - bisect--helper: `bisect_autostart` shell function in C
>  - bisect--helper: retire `--next-all` subcommand
>  - bisect--helper: retire `--bisect-clean-state` subcommand
>  - bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
>  - t6030: no cleanup with bad merge base
>  - bisect--helper: `bisect_start` shell function partially in C
>  - bisect--helper: `get_terms` & `bisect_terms` shell function in C
>  - bisect--helper: `bisect_next_check` & bisect_voc shell function in C
>  - bisect--helper: `check_and_set_terms` shell function in C
>  - bisect--helper: `bisect_write` shell function in C
>  - bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
>  - bisect--helper: `bisect_reset` shell function in C
>  - wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
>  - t6030: explicitly test for bisection cleanup
>  - bisect--helper: `bisect_clean_state` shell function in C
>  - bisect--helper: `write_terms` shell function in C
>  - bisect: rewrite `check_term_format` shell function in C
>  - bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
>
>  GSoC "bisect" topic.

You could squash your commit. Thanks!

Regards,
Pranit Bauva

^ permalink raw reply

* Re: [PATCH v4 08/25] sequencer: completely revamp the "todo" script parsing
From: Junio C Hamano @ 2016-10-17 22:33 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <a5f5d784fcacc16555237b9750380d612387d0cb.1476450940.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> -	for (i = 1; *p; i++) {
> +	for (i = 1; *p; i++, p = next_p) {
>  		char *eol = strchrnul(p, '\n');
> -		commit = parse_insn_line(p, eol, opts);
> -		if (!commit)
> -			return error(_("Could not parse line %d."), i);
> -		next = commit_list_append(commit, next);
> -		p = *eol ? eol + 1 : eol;
> +
> +		next_p = *eol ? eol + 1 /* strip LF */ : eol;

This one was explained as "skip LF" in the previous round, and that
is more correct than "strip", I think.  The +1 here is not done to
"strip" the LF out of the end result, but to "skip" one to move to
the beginning of the next line.

The one in v3 08/25 decremented the pointer to denote the end of the
line with the explicit purpose of not including the CR in the end
result, which was explained as "skip CR", but it was stripping CR.
Correcting that explanation to "strip" was a right fix and I think
your v4 09/25 still has it, which is good.

Other than this unnecessary change since the previous round, I
didn't spot a difference in this step, which was already good.

Thanks.



^ permalink raw reply

* [RFD] should all merge bases be equal?
From: Junio C Hamano @ 2016-10-17 22:28 UTC (permalink / raw)
  To: git

People can see how fast the usual merges I see everyday are by
looking at output from

    $ git log --first-parent --format='%ct %s' master..pu

and noticing the seconds since epoch.  Most of the days, these are
recreated directly on top of 'master' from scratch, and they get
timestamps that are very close to each other (or the same), meaning
that I am getting multiple merges per second.

Being accustomed how fast my merges go, there is one merge that
hiccups for me every once in a few days: merging back from 'master'
to 'next'.  This happens after having multiple topics (that by
definition have to be the ones that were already merged to 'next'
some time ago) to 'master', and 'master' may also have its own
commit (e.g. update to "RelNotes") and merges of side branches that
were not in 'next' (e.g. merge from submaintainers like i18n, etc.)

The reason why this merge is slow is because it typically have many
merge bases.  For example, today's tip of 'next' is:

    commit 6021889cc14df07d4366829367d2c4a11d1eaa56
    Merge: 4868def05e 659889482a
    Author: Junio C Hamano <gitster@pobox.com>
    Date:   Mon Oct 17 14:02:05 2016 -0700

        Sync with master

        * master:
          Tenth batch for 2.11
          l10n: de.po: translate 260 new messages
          l10n: de.po: fix translation of autostash
          l10n: ru.po: update Russian translation

which is a merge that has 12 merge bases:

    $ git merge-base --all 4868def05e 659889482a | git name-rev --stdin
    3cdd5d19178a54d2e51b5098d43b57571241d0ab (ko/master)
    641c900b2c3a8c3d385eb353b3801a5f49ddbb47 (js/reset-usage)
    30cfe72d37ed8c174cae43923769730a94549dae (rs/pretty-format-color-doc-fix)
    654311bf6ee0fbf530c088271c2c76d46f31f82d (da/mergetool-diff-order)
    72710165c932edb2b8552aef7aef2f357dde4746 (sb/submodule-config-doc-drop-path)
    842a516cb02a53cf0291ff67ed6f8517966345c0 (js/regexec-buf)
    62fe0eb4804c297486a1d421a4f893865fcbc911 (jk/quarantine-received-objects)
    a94bb683970a111b467a36590ca36e52754ad504 (rs/cocci)
    e8c42cb9ce6a566aad797cc6c5bc1279d608d819 (jk/ref-symlink-loop)
    22d3b8de1b625813faec6f3d6ffe66124837b78b (jk/clone-copy-alternates-fix)
    7431596ab1f05a13adb93b44108f27cfd6578a31 (nd/commit-p-doc)
    5275c3081c2b2c6166a2fc6b253a3acb20f8ae89 (dt/http-empty-auth)

The tip of each topic that was merged recently to 'master' since
'master' was last merged to 'next' becomes a valid merge-base by
design of the workflow.  We merge a topic to 'master' whose tip
has been already in 'next' for a while, so the tip of 'next' before
merging 'master' back is a descendant of the tips of these topics,
and the tip of 'master' before I make this merge has just become a
descendant of the tips of these topics.  That makes them common
ancestors between 'master' and 'next'.

But for the purpose of figuring out the 3-way merge result, I
suspect that they are pretty much useless common ancestor to use as
the merge base.  The first one in the list, the old 'master' that
was merged the last time to 'next', would be a lot more useful one.

And of course, if I do this:

    $ git checkout next
    $ git merge master ;# this is slow due to the 12-base above
    $ git checkout HEAD^ ;# detach at the previous state
    $ git merge-recursive ko/master -- HEAD master

the merge is instantaneous.  I'd get only what truly happened
uniquely on 'master', e.g. RelNotes update and i18n merge.

I am wondering if it is worth trying to optimize it by taking
advantage of the specific workflow (i.e. give me an option to use
when merging 'master' back to 'next') and allows me to exclude the
tips of these topic branches.  Would I be making the result open to
the criss-cross merge gotchas the "recursive merge" machinery was
designed to prevent in the first place by doing so?  Offhand, I do
not think that would be the case.

Assuming that it is a good idea, there is another question of how to
tell the more meaningful merge bases (ko/master in this case) out of
the less useful ones (all the others).  I think it would be
sufficient to reject commits that are not on the first-parent chain
of neither branch being merged.  Among the 12 merge bases, ko/master
is the only one that appears on the first-parent chain from 'master'
being merged (it does not appear on the first-parent chain from
'next').  All others were topic tips that by definition were merged
as second parent to integration branches ('next' and later 'master').
The place to do this would be a new option to 'merge-base'; instead
of "--all", perhaps "--major" option gives only the major merge bases
(with the definition of 'major' being the above heuristics), and then
"git merge-recursive" would learn "-Xmajor-base-only" strategy option,
or something along that line.

Thoughts?

^ permalink raw reply

* What's cooking in git.git (Oct 2016, #04; Mon, 17)
From: Junio C Hamano @ 2016-10-17 22:28 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

We may have to start thinking about 2.10.2; there are about a dozen
and half fixes accumulated on the 'maint' front.

You can find the changes described here in the integration branches
of the repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

--------------------------------------------------
[Graduated to "master"]

* da/mergetool-diff-order (2016-10-11) 4 commits
  (merged to 'next' on 2016-10-11 at 3d1b98c16d)
 + mergetool: honor -O<orderfile>
 + mergetool: honor diff.orderFile
 + mergetool: move main program flow into a main() function
 + mergetool: add copyright

 "git mergetool" learned to honor "-O<orderfile>" to control the
 order of paths to present to the end user.


* dt/http-empty-auth (2016-10-04) 1 commit
  (merged to 'next' on 2016-10-10 at 10b7b0a6a5)
 + http: http.emptyauth should allow empty (not just NULL) usernames

 http.emptyauth configuration is a way to allow an empty username to
 pass when attempting to authenticate using mechanisms like
 Kerberos.  We took an unspecified (NULL) username and sent ":"
 (i.e. no username, no password) to CURLOPT_USERPWD, but did not do
 the same when the username is explicitly set to an empty string.


* jk/alt-odb-cleanup (2016-10-10) 18 commits
  (merged to 'next' on 2016-10-10 at d2ed6b6d21)
 + alternates: use fspathcmp to detect duplicates
 + sha1_file: always allow relative paths to alternates
 + count-objects: report alternates via verbose mode
 + fill_sha1_file: write into a strbuf
 + alternates: store scratch buffer as strbuf
 + fill_sha1_file: write "boring" characters
 + alternates: use a separate scratch space
 + alternates: encapsulate alt->base munging
 + alternates: provide helper for allocating alternate
 + alternates: provide helper for adding to alternates list
 + link_alt_odb_entry: refactor string handling
 + link_alt_odb_entry: handle normalize_path errors
 + t5613: clarify "too deep" recursion tests
 + t5613: do not chdir in main process
 + t5613: whitespace/style cleanups
 + t5613: use test_must_fail
 + t5613: drop test_valid_repo function
 + t5613: drop reachable_via function
 (this branch is used by jk/quarantine-received-objects.)

 Codepaths involved in interacting alternate object store have
 been cleaned up.


* jk/clone-copy-alternates-fix (2016-10-05) 1 commit
  (merged to 'next' on 2016-10-10 at 8154134c8c)
 + clone: detect errors in normalize_path_copy

 "git clone" of a local repository can be done at the filesystem
 level, but the codepath did not check errors while copying and
 adjusting the file that lists alternate object stores.


* jk/quarantine-received-objects (2016-10-10) 5 commits
  (merged to 'next' on 2016-10-10 at 0fd3e3b2ef)
 + tmp-objdir: do not migrate files starting with '.'
 + tmp-objdir: put quarantine information in the environment
 + receive-pack: quarantine objects until pre-receive accepts
 + tmp-objdir: introduce API for temporary object directories
 + check_connected: accept an env argument
 (this branch uses jk/alt-odb-cleanup.)

 In order for the receiving end of "git push" to inspect the
 received history and decide to reject the push, the objects sent
 from the sending end need to be made available to the hook and
 the mechanism for the connectivity check, and this was done
 traditionally by storing the objects in the receiving repository
 and letting "git gc" to expire it.  Instead, store the newly
 received objects in a temporary area, and make them available by
 reusing the alternate object store mechanism to them only while we
 decide if we accept the check, and once we decide, either migrate
 them to the repository or purge them immediately.


* jk/ref-symlink-loop (2016-10-10) 2 commits
  (merged to 'next' on 2016-10-11 at ac5c35f87f)
 + files_read_raw_ref: prevent infinite retry loops in general
 + files_read_raw_ref: avoid infinite loop on broken symlinks

 A stray symbolic link in $GIT_DIR/refs/ directory could make name
 resolution loop forever, which has been corrected.


* js/regexec-buf (2016-10-10) 1 commit
  (merged to 'next' on 2016-10-11 at 466a26548c)
 + configure.ac: improve description of NO_REGEX test

 A follow-up to an already graduated topic.


* js/reset-usage (2016-10-11) 1 commit
  (merged to 'next' on 2016-10-11 at 61ad4a7c0e)
 + reset: fix usage

 Message fix-up.


* nd/commit-p-doc (2016-10-05) 1 commit
  (merged to 'next' on 2016-10-10 at 5a9996dd7b)
 + git-commit.txt: clarify --patch mode with pathspec

 Documentation for "git commit" was updated to clarify that "commit
 -p <paths>" adds to the current contents of the index to come up
 with what to commit.


* rs/cocci (2016-10-10) 2 commits
  (merged to 'next' on 2016-10-11 at bbd6a88402)
 + use strbuf_add_unique_abbrev() for adding short hashes, part 3
 + remove unnecessary NULL check before free(3)

 Code cleanup.


* rs/pretty-format-color-doc-fix (2016-10-11) 1 commit
  (merged to 'next' on 2016-10-11 at f0ae64bcbd)
 + pretty: fix document link for color specification

 Small doc update.


* sb/submodule-config-doc-drop-path (2016-10-11) 1 commit
  (merged to 'next' on 2016-10-11 at 28fc3e53f9)
 + documentation: improve submodule.<name>.{url, path} description

 The "submodule.<name>.path" stored in .gitmodules is never copied
 to .git/config and such a key in .git/config has no meaning, but
 the documentation described it and submodule.<name>.url next to
 each other as if both belong to .git/config.  This has been fixed.

--------------------------------------------------
[New Topics]

* dk/worktree-dup-checkout-with-bare-is-ok (2016-10-14) 1 commit
  (merged to 'next' on 2016-10-17 at 24594d3e56)
 + worktree: allow the main brach of a bare repository to be checked out

 In a worktree connected to a repository elsewhere, created via "git
 worktree", "git checkout" attempts to protect users from confusion
 by refusing to check out a branch that is already checked out in
 another worktree.  However, this also prevented checking out a
 branch, which is designated as the primary branch of a bare
 reopsitory, in a worktree that is connected to the bare
 repository.  The check has been corrected to allow it.

 Will merge to 'master'.


* jc/cocci-xstrdup-or-null (2016-10-12) 1 commit
  (merged to 'next' on 2016-10-17 at 55ceaa465a)
 + cocci: refactor common patterns to use xstrdup_or_null()

 Code cleanup.

 Will merge to 'master'.


* tb/convert-stream-check (2016-10-12) 2 commits
 - convert.c: stream and fast search for binary
 - read-cache: factor out get_sha1_from_index() helper

 End-of-line conversion sometimes needs to see if the current blob
 in the index has NULs and CRs to base its decision.  We used to
 always get a full statistics over the blob, but in many cases we
 can return early when we have seen "enough" (e.g. if we see a
 single NUL, the blob will be handled as binary).  The codepaths
 have been optimized by using streaming interface.

 Waiting for review.


* jk/ambiguous-short-object-names (2016-10-12) 1 commit
 - t1512: become resilient to GETTEXT_POISON build

 A test fixup to recently graduated topic.

 Will merge to 'next'.


* jk/merge-base-fork-point-without-reflog (2016-10-12) 1 commit
 - merge-base: handle --fork-point without reflog

 "git rebase" immediately after "git clone" failed to find the fork
 point from the upstream.

 Will merge to 'next'.


* jk/upload-pack-use-prio-queue (2016-10-11) 1 commit
 - upload-pack: use priority queue in reachable() check

 Code clean-up and performance improvement to reduce use of
 timestamp-ordered commit-list by replacing it with a priority
 queue.

 Will merge to 'next'.


* jk/fetch-quick-tag-following (2016-10-14) 1 commit
 - fetch: use "quick" has_sha1_file for tag following

 When fetching from a remote that has many tags that are irrelevant
 to branches we are following, we used to waste way too many cycles 
 when checking if the object pointed at by a tag (that we are not
 going to fetch!) exists in our repository too carefully.

 Will merge to 'next'.


* jt/trailer-with-cruft (2016-10-14) 6 commits
 - trailer: support values folded to multiple lines
 - trailer: allow non-trailers in trailer block
 - trailer: make args have their own struct
 - trailer: streamline trailer item create and add
 - trailer: use list.h for doubly-linked list
 - trailer: improve const correctness

 Update "interpret-trailers" machinery and teaches it that people in
 real world write all sorts of crufts in the "trailer" that was
 originally designed to have the neat-o "Mail-Header: like thing"
 and nothing else.

 Waiting for review.


* mm/send-email-cc-cruft-after-address (2016-10-14) 1 commit
 - parse_mailboxes: accept extra text after <...> address

 "git send-email" attempts to pick up valid e-mails from the
 trailers, but people in real world write non-addresses there, like
 "Cc: Stable <add@re.ss> # 4.8+", which broke the output depending
 on the availability and vintage of Mail::Address perl module.

 Will merge to 'next'.


* va/i18n (2016-10-17) 7 commits
 - i18n: diff: mark warnings for translation
 - i18n: credential-cache--daemon: mark advice for translation
 - i18n: convert mark error messages for translation
 - i18n: apply: mark error message for translation
 - i18n: apply: mark error messages for translation
 - i18n: apply: mark info messages for translation
 - i18n: apply: mark plural string for translation

 More i18n.

 Will merge to 'next'.


* jk/tighten-alloc (2016-10-17) 2 commits
 - inline xalloc_flex() into FLEXPTR_ALLOC_MEM
 - avoid pointer arithmetic involving NULL in FLEX_ALLOC_MEM

 Protect our code from over-eager compilers.

 Will merge to 'next'.


* pb/test-parse-options-expect (2016-10-17) 1 commit
 - t0040: convert all possible tests to use `test-parse-options --expect`

 Test clean-up.

 Will merge to 'next'.

--------------------------------------------------
[Stalled]

* jc/bundle (2016-03-03) 6 commits
 - index-pack: --clone-bundle option
 - Merge branch 'jc/index-pack' into jc/bundle
 - bundle v3: the beginning
 - bundle: keep a copy of bundle file name in the in-core bundle header
 - bundle: plug resource leak
 - bundle doc: 'verify' is not about verifying the bundle

 The beginning of "split bundle", which could be one of the
 ingredients to allow "git clone" traffic off of the core server
 network to CDN.

 While I think it would make it easier for people to experiment and
 build on if the topic is merged to 'next', I am at the same time a
 bit reluctant to merge an unproven new topic that introduces a new
 file format, which we may end up having to support til the end of
 time.  It is likely that to support a "prime clone from CDN", it
 would need a lot more than just "these are the heads and the pack
 data is over there", so this may not be sufficient.

 Will discard.


* mh/connect (2016-06-06) 10 commits
 - connect: [host:port] is legacy for ssh
 - connect: move ssh command line preparation to a separate function
 - connect: actively reject git:// urls with a user part
 - connect: change the --diag-url output to separate user and host
 - connect: make parse_connect_url() return the user part of the url as a separate value
 - connect: group CONNECT_DIAG_URL handling code
 - connect: make parse_connect_url() return separated host and port
 - connect: re-derive a host:port string from the separate host and port variables
 - connect: call get_host_and_port() earlier
 - connect: document why we sometimes call get_port after get_host_and_port

 Rewrite Git-URL parsing routine (hopefully) without changing any
 behaviour.

 It has been two months without any support.  We may want to discard
 this.


* kn/ref-filter-branch-list (2016-05-17) 17 commits
 - branch: implement '--format' option
 - branch: use ref-filter printing APIs
 - branch, tag: use porcelain output
 - ref-filter: allow porcelain to translate messages in the output
 - ref-filter: add `:dir` and `:base` options for ref printing atoms
 - ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
 - ref-filter: introduce symref_atom_parser() and refname_atom_parser()
 - ref-filter: introduce refname_atom_parser_internal()
 - ref-filter: make "%(symref)" atom work with the ':short' modifier
 - ref-filter: add support for %(upstream:track,nobracket)
 - ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
 - ref-filter: introduce format_ref_array_item()
 - ref-filter: move get_head_description() from branch.c
 - ref-filter: modify "%(objectname:short)" to take length
 - ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
 - ref-filter: include reference to 'used_atom' within 'atom_value'
 - ref-filter: implement %(if), %(then), and %(else) atoms

 The code to list branches in "git branch" has been consolidated
 with the more generic ref-filter API.

 Rerolled.
 Needs review.


* ec/annotate-deleted (2015-11-20) 1 commit
 - annotate: skip checking working tree if a revision is provided

 Usability fix for annotate-specific "<file> <rev>" syntax with deleted
 files.

 Has been waiting for a review for too long without seeing anything.

 Will discard.


* dk/gc-more-wo-pack (2016-01-13) 4 commits
 - gc: clean garbage .bitmap files from pack dir
 - t5304: ensure non-garbage files are not deleted
 - t5304: test .bitmap garbage files
 - prepare_packed_git(): find more garbage

 Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
 .bitmap and .keep files.

 Has been waiting for a reroll for too long.
 cf. <xmqq60ypbeng.fsf@gitster.mtv.corp.google.com>

 Will discard.


* jc/diff-b-m (2015-02-23) 5 commits
 . WIPWIP
 . WIP: diff-b-m
 - diffcore-rename: allow easier debugging
 - diffcore-rename.c: add locate_rename_src()
 - diffcore-break: allow debugging

 "git diff -B -M" produced incorrect patch when the postimage of a
 completely rewritten file is similar to the preimage of a removed
 file; such a resulting file must not be expressed as a rename from
 other place.

 The fix in this patch is broken, unfortunately.

 Will discard.

--------------------------------------------------
[Cooking]

* pb/bisect (2016-10-14) 28 commits
 - SQUASH???
 - bisect--helper: remove the dequote in bisect_start()
 - bisect--helper: retire `--bisect-auto-next` subcommand
 - bisect--helper: retire `--bisect-autostart` subcommand
 - bisect--helper: retire `--bisect-write` subcommand
 - bisect--helper: `bisect_replay` shell function in C
 - bisect--helper: `bisect_log` shell function in C
 - bisect--helper: retire `--write-terms` subcommand
 - bisect--helper: retire `--check-expected-revs` subcommand
 - bisect--helper: `bisect_state` & `bisect_head` shell function in C
 - bisect--helper: `bisect_autostart` shell function in C
 - bisect--helper: retire `--next-all` subcommand
 - bisect--helper: retire `--bisect-clean-state` subcommand
 - bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
 - t6030: no cleanup with bad merge base
 - bisect--helper: `bisect_start` shell function partially in C
 - bisect--helper: `get_terms` & `bisect_terms` shell function in C
 - bisect--helper: `bisect_next_check` & bisect_voc shell function in C
 - bisect--helper: `check_and_set_terms` shell function in C
 - bisect--helper: `bisect_write` shell function in C
 - bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
 - bisect--helper: `bisect_reset` shell function in C
 - wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
 - t6030: explicitly test for bisection cleanup
 - bisect--helper: `bisect_clean_state` shell function in C
 - bisect--helper: `write_terms` shell function in C
 - bisect: rewrite `check_term_format` shell function in C
 - bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

 GSoC "bisect" topic.


* ab/gitweb-abbrev-links (2016-10-14) 3 commits
  (merged to 'next' on 2016-10-17 at 4868def05e)
 + gitweb: link to "git describe"'d commits in log messages
 + gitweb: link to 7-char+ SHA-1s, not only 8-char+
 + gitweb: fix a typo in a comment

 In addition to purely abbreviated commit object names, "gitweb"
 learned to turn "git describe" output (e.g. v2.9.3-599-g2376d31787)
 into clickable links in its output.

 Will merge to 'master'.


* js/prepare-sequencer (2016-10-17) 25 commits
 - sequencer: mark all error messages for translation
 - sequencer: start error messages consistently with lower case
 - sequencer: quote filenames in error messages
 - sequencer: mark action_name() for translation
 - sequencer: remove overzealous assumption in rebase -i mode
 - sequencer: refactor write_message()
 - sequencer: left-trim lines read from the script
 - sequencer: do not try to commit when there were merge conflicts
 - sequencer: support cleaning up commit messages
 - sequencer: support amending commits
 - sequencer: allow editing the commit message on a case-by-case basis
 - sequencer: introduce a helper to read files written by scripts
 - sequencer: prepare for rebase -i's commit functionality
 - sequencer: remember the onelines when parsing the todo file
 - sequencer: get rid of the subcommand field
 - sequencer: avoid completely different messages for different actions
 - sequencer: strip CR from the todo script
 - sequencer: completely revamp the "todo" script parsing
 - sequencer: refactor the code to obtain a short commit name
 - sequencer: future-proof read_populate_todo()
 - sequencer: eventually release memory allocated for the option values
 - sequencer: future-proof remove_sequencer_state()
 - sequencer: avoid unnecessary indirection
 - sequencer: use memoized sequencer directory path
 - sequencer: use static initializers for replay_opts

 Update of the sequencer codebase to make it reusable to reimplement
 "rebase -i" continues.

 Waiting for review.


* sb/submodule-ignore-trailing-slash (2016-10-10) 2 commits
  (merged to 'next' on 2016-10-11 at e37425ed17)
 + submodule: ignore trailing slash in relative url
 + submodule: ignore trailing slash on superproject URL

 A minor regression fix for "git submodule".

 It seems that POSIX emulation layer of Windows is not cooperating;
 this may have to wait (or tentatively reverted in Windows port) for
 the resolution of the issue.

 cf. <alpine.DEB.2.20.1610131255001.197091@virtualbox>


* st/verify-tag (2016-10-10) 7 commits
 - t/t7004-tag: Add --format specifier tests
 - t/t7030-verify-tag: Add --format specifier tests
 - builtin/tag: add --format argument for tag -v
 - builtin/verify-tag: add --format to verify-tag
 - tag: add format specifier to gpg_verify_tag
 - ref-filter: add function to print single ref_array_item
 - gpg-interface, tag: add GPG_VERIFY_QUIET flag

 "git tag" and "git verify-tag" learned to put GPG verification
 status in their "--format=<placeholders>" output format.

 Is this ready for 'next'?


* mm/credential-libsecret (2016-10-11) 1 commit
  (merged to 'next' on 2016-10-17 at 1b4af03ba4)
 + contrib: add credential helper for libsecret

 A new credential helper that talks via "libsecret" with
 implementations of XDG Secret Service API has been added to
 contrib/credential/.

 Will merge to 'master'.


* sb/attr (2016-10-11) 28 commits
 - attr: convert to new threadsafe API
 - attr: make git_check_attr_counted static
 - attr: make git_attr_counted static
 - attr.c: outline the future plans by heavily commenting
 - attr.c: always pass check[] to collect_some_attrs()
 - attr.c: introduce empty_attr_check_elems()
 - attr.c: correct ugly hack for git_all_attrs()
 - attr.c: rename a local variable check
 - attr.c: pass struct git_attr_check down the callchain
 - attr.c: add push_stack() helper
 - attr: support quoting pathname patterns in C style
 - attr: expose validity check for attribute names
 - attr: add counted string version of git_attr()
 - attr: add counted string version of git_check_attr()
 - attr: retire git_check_attrs() API
 - attr: convert git_check_attrs() callers to use the new API
 - attr: convert git_all_attrs() to use "struct git_attr_check"
 - attr: (re)introduce git_check_attr() and struct git_attr_check
 - attr: rename function and struct related to checking attributes
 - attr.c: plug small leak in parse_attr_line()
 - attr.c: tighten constness around "git_attr" structure
 - attr.c: simplify macroexpand_one()
 - attr.c: mark where #if DEBUG ends more clearly
 - attr.c: complete a sentence in a comment
 - attr.c: explain the lack of attr-name syntax check in parse_attr()
 - attr.c: update a stale comment on "struct match_attr"
 - attr.c: use strchrnul() to scan for one line
 - commit.c: use strchrnul() to scan for one line

 The attributes API has been updated so that it can later be
 optimized using the knowledge of which attributes are queried.


* jc/ws-error-highlight (2016-10-04) 4 commits
  (merged to 'next' on 2016-10-17 at ecbdc57d77)
 + diff: introduce diff.wsErrorHighlight option
 + diff.c: move ws-error-highlight parsing helpers up
 + diff.c: refactor parse_ws_error_highlight()
 + t4015: split out the "setup" part of ws-error-highlight test

 "git diff/log --ws-error-highlight=<kind>" lacked the corresponding
 configuration variable to set it by default.

 Will merge to 'master'.


* jk/abbrev-auto (2016-10-03) 1 commit
 - find_unique_abbrev: move logic out of get_short_sha1()
 (this branch uses lt/abbrev-auto.)

 Updates the way approximate count of total objects is computed
 while attempting to come up with a unique abbreviated object name,
 which in turn needs to estimate how many hexdigits are necessary to
 ensure uniqueness.

 Undecided.


* nd/ita-empty-commit (2016-09-28) 3 commits
 - commit: don't be fooled by ita entries when creating initial commit
 - diff-lib.c: enable --shift-ita in index_differs_from()
 - Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"

 When new paths were added by "git add -N" to the index, it was
 enough to circumvent the check by "git commit" to refrain from
 making an empty commit without "--allow-empty".  The same logic
 prevented "git status" to show such a path as "new file" in the
 "Changes not staged for commit" section.

 Expecting a reroll.
 cf. <xmqqzimrj03j.fsf@gitster.mtv.corp.google.com>
 cf. <xmqq8tubkgg5.fsf@gitster.mtv.corp.google.com>


* lt/abbrev-auto (2016-10-03) 3 commits
  (merged to 'next' on 2016-10-03 at bb188d00f7)
 + abbrev: auto size the default abbreviation
 + abbrev: prepare for new world order
 + abbrev: add FALLBACK_DEFAULT_ABBREV to prepare for auto sizing
 (this branch is used by jk/abbrev-auto.)

 Allow the default abbreviation length, which has historically been
 7, to scale as the repository grows.  The logic suggests to use 12
 hexdigits for the Linux kernel, and 9 to 10 for Git itself.

 Will hold to see if people scream.


* jc/diff-unique-abbrev-comments (2016-09-30) 1 commit
  (merged to 'next' on 2016-10-17 at c7fb286102)
 + diff_unique_abbrev(): document its assumption and limitation

 A bit more comments in a tricky code.

 Will merge to 'master'.


* va/i18n-perl-scripts (2016-10-10) 14 commits
 - i18n: difftool: mark warnings for translation
 - i18n: send-email: mark string with interpolation for translation
 - i18n: send-email: mark warnings and errors for translation
 - i18n: send-email: mark strings for translation
 - i18n: add--interactive: mark status words for translation
 - i18n: add--interactive: remove %patch_modes entries
 - i18n: add--interactive: mark edit_hunk_manually message for translation
 - i18n: add--interactive: i18n of help_patch_cmd
 - i18n: add--interactive: mark patch prompt for translation
 - i18n: add--interactive: mark plural strings
 - i18n: clean.c: match string with git-add--interactive.perl
 - i18n: add--interactive: mark strings with interpolation for translation
 - i18n: add--interactive: mark simple here-documents for translation
 - i18n: add--interactive: mark strings for translation

 Porcelain scripts written in Perl are getting internationalized.

 Waiting for review.
 cf. <20161010125449.7929-1-vascomalmeida@sapo.pt>


* jc/latin-1 (2016-09-26) 2 commits
  (merged to 'next' on 2016-09-28 at c8673e03c2)
 + utf8: accept "latin-1" as ISO-8859-1
 + utf8: refactor code to decide fallback encoding

 Some platforms no longer understand "latin-1" that is still seen in
 the wild in e-mail headers; replace them with "iso-8859-1" that is
 more widely known when conversion fails from/to it.

 Will hold to see if people scream.


* mg/gpg-richer-status (2016-10-12) 1 commit
  (merged to 'next' on 2016-10-17 at 8843a6a8be)
 + gpg-interface: use more status letters

 The GPG verification status shown in "%G?" pretty format specifier
 was not rich enough to differentiate a signature made by an expired
 key, a signature made by a revoked key, etc.  New output letters
 have been assigned to express them.

 Will merge to 'master'.


* js/libify-require-clean-work-tree (2016-10-07) 6 commits
  (merged to 'next' on 2016-10-17 at f5c20df38b)
 + wt-status: begin error messages with lower-case
 + wt-status: teach has_{unstaged,uncommitted}_changes() about submodules
 + wt-status: export also the has_un{staged,committed}_changes() functions
 + wt-status: make the require_clean_work_tree() function reusable
 + pull: make code more similar to the shell script again
 + pull: drop confusing prefix parameter of die_on_unclean_work_tree()

 The require_clean_work_tree() helper was recreated in C when "git
 pull" was rewritten from shell; the helper is now made available to
 other callers in preparation for upcoming "rebase -i" work.

 Will merge to 'master'.


* bw/ls-files-recurse-submodules (2016-10-10) 4 commits
  (merged to 'next' on 2016-10-17 at f0e398946a)
 + ls-files: add pathspec matching for submodules
 + ls-files: pass through safe options for --recurse-submodules
 + ls-files: optionally recurse into submodules
 + git: make super-prefix option

 "git ls-files" learned "--recurse-submodules" option that can be
 used to get a listing of tracked files across submodules (i.e. this
 only works with "--cached" option, not for listing untracked or
 ignored files).  This would be a useful tool to sit on the upstream
 side of a pipe that is read with xargs to work on all working tree
 files from the top-level superproject.

 Will merge to 'master'.


* ls/filter-process (2016-10-17) 14 commits
 - contrib/long-running-filter: add long running filter example
 - convert: add filter.<driver>.process option
 - convert: prepare filter.<driver>.process option
 - convert: make apply_filter() adhere to standard Git error handling
 - pkt-line: add functions to read/write flush terminated packet streams
 - pkt-line: add packet_write_gently()
 - pkt-line: add packet_flush_gently()
 - pkt-line: add packet_write_fmt_gently()
 - pkt-line: extract set_packet_header()
 - pkt-line: rename packet_write() to packet_write_fmt()
 - run-command: add clean_on_exit_handler
 - run-command: move check_pipe() from write_or_die to run_command
 - convert: modernize tests
 - convert: quote filter names in error messages

 The smudge/clean filter API expect an external process is spawned
 to filter the contents for each path that has a filter defined.  A
 new type of "process" filter API has been added to allow the first
 request to run the filter for a path to spawn a single process, and
 all filtering need is served by this single process for multiple
 paths, reducing the process creation overhead.


* hv/submodule-not-yet-pushed-fix (2016-10-10) 3 commits
 - batch check whether submodule needs pushing into one call
 - serialize collection of refs that contain submodule changes
 - serialize collection of changed submodules

 The code in "git push" to compute if any commit being pushed in the
 superproject binds a commit in a submodule that hasn't been pushed
 out was overly inefficient, making it unusable even for a small
 project that does not have any submodule but have a reasonable
 number of refs.

 Waiting for review.
 cf. <cover.1475851621.git.hvoigt@hvoigt.net>


* sg/fix-versioncmp-with-common-suffix (2016-09-08) 5 commits
 - versioncmp: cope with common leading parts in versionsort.prereleaseSuffix
 - versioncmp: pass full tagnames to swap_prereleases()
 - t7004-tag: add version sort tests to show prerelease reordering issues
 - t7004-tag: use test_config helper
 - t7004-tag: delete unnecessary tags with test_when_finished

 The prereleaseSuffix feature of version comparison that is used in
 "git tag -l" did not correctly when two or more prereleases for the
 same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
 are there and the code needs to compare 2.0-beta1 and 2.0-beta2).

 Waiting for a reroll.
 cf. <20160908223727.Horde.jVOOJ278ssZ3qkyjkmyqZD-@webmail.informatik.kit.edu>


* sb/push-make-submodule-check-the-default (2016-10-10) 2 commits
 - push: change submodule default to check when submodules exist
 - submodule add: extend force flag to add existing repos

 Turn the default of "push.recurseSubmodules" to "check" when
 submodules seem to be in use.

 Will hold to wait for hv/submodule-not-yet-pushed-fix


* jc/pull-rebase-ff (2016-07-28) 1 commit
 - pull: fast-forward "pull --rebase=true"

 "git pull --rebase", when there is no new commits on our side since
 we forked from the upstream, should be able to fast-forward without
 invoking "git rebase", but it didn't.

 Needs a real log message and a few tests.


* ex/deprecate-empty-pathspec-as-match-all (2016-06-22) 1 commit
  (merged to 'next' on 2016-09-21 at e19148ea63)
 + pathspec: warn on empty strings as pathspec

 Originally merged to 'next' on 2016-07-13

 An empty string used as a pathspec element has always meant
 'everything matches', but it is too easy to write a script that
 finds a path to remove in $path and run 'git rm "$paht"', which
 ends up removing everything.  Start warning about this use of an
 empty string used for 'everything matches' and ask users to use a
 more explicit '.' for that instead.

 The hope is that existing users will not mind this change, and
 eventually the warning can be turned into a hard error, upgrading
 the deprecation into removal of this (mis)feature.

 Will hold to see if people scream.


* jc/merge-drop-old-syntax (2015-04-29) 1 commit
  (merged to 'next' on 2016-10-11 at 8928c8b9b3)
 + merge: drop 'git merge <message> HEAD <commit>' syntax

 Stop supporting "git merge <message> HEAD <commit>" syntax that has
 been deprecated since October 2007, and issues a deprecation
 warning message since v2.5.0.

 It has been reported that git-gui still uses the deprecated syntax,
 which needs to be fixed before this final step can proceed.
 cf. <5671DB28.8020901@kdbg.org>

 Will merge to 'master'.

^ permalink raw reply

* [PATCH] submodule--helper: normalize funny urls
From: Stefan Beller @ 2016-10-17 22:16 UTC (permalink / raw)
  To: gitster, j6t, Johannes.Schindelin
  Cc: git, venv21, dennis, jrnieder, Stefan Beller

Currently a URL for the superproject ending in

(A)    .../path/to/dir
(B)    .../path/to/dir/
(C)    .../path/to/dir/.
(D)    .../path/to/dir/./.
(E)    .../path/to/dir/.///.//.

is treated the same in (A) and (B), but (C, D, E) are different.

We never produce the URLs in (C,D,E) ourselves, they come to use, because
the user used it as the URL for cloning a superproject.
Normalize these paths.

Signed-off-by: Stefan Beller <sbeller@google.com>
---

 By being strict in Git, I think we also fix the Git for Windows painpoints.
 
 This goes on top of origin/sb/submodule-ignore-trailing-slash.
 
 Thanks,
 Stefan

 builtin/submodule--helper.c | 49 ++++++++++++++++++++++++++++++++++-----------
 t/t0060-path-utils.sh       | 11 ++++++----
 2 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 260f46f..ca90763 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -76,6 +76,30 @@ static int chop_last_dir(char **remoteurl, int is_relative)
 	return 0;
 }
 
+static void strip_url_ending(char *url, size_t *_len)
+{
+	int check_url_stripping = 1;
+	size_t len = _len ? *_len : strlen(url);
+
+	while (check_url_stripping) {
+		check_url_stripping = 0;
+		if (is_dir_sep(url[len-2]) && url[len-1] == '.') {
+			url[len-2] = '\0';
+			len -= 2;
+			check_url_stripping = 1;
+		}
+
+		if (is_dir_sep(url[len-1])) {
+			url[len-1] = '\0';
+			len --;
+			check_url_stripping = 1;
+		}
+	}
+
+	if (_len)
+		*_len = len;
+}
+
 /*
  * The `url` argument is the URL that navigates to the submodule origin
  * repo. When relative, this URL is relative to the superproject origin
@@ -93,14 +117,16 @@ static int chop_last_dir(char **remoteurl, int is_relative)
  * the superproject working tree otherwise.
  *
  * NEEDSWORK: This works incorrectly on the domain and protocol part.
- * remote_url      url              outcome          expectation
- * http://a.com/b  ../c             http://a.com/c   as is
- * http://a.com/b/ ../c             http://a.com/c   same as previous line, but
- *                                                   ignore trailing slash in url
- * http://a.com/b  ../../c          http://c         error out
- * http://a.com/b  ../../../c       http:/c          error out
- * http://a.com/b  ../../../../c    http:c           error out
- * http://a.com/b  ../../../../../c    .:c           error out
+ * remote_url       url              outcome          expectation
+ * http://a.com/b   ../c             http://a.com/c   as is
+ * http://a.com/b/  ../c             http://a.com/c   same as previous line, but
+ *                                                    ignore trailing '/' in url
+ * http://a.com/b/. ../c             http://a.com/c   same as previous line, but
+ *                                                    ignore trailing '/.' in url
+ * http://a.com/b   ../../c          http://c         error out
+ * http://a.com/b   ../../../c       http:/c          error out
+ * http://a.com/b   ../../../../c    http:c           error out
+ * http://a.com/b   ../../../../../c    .:c           error out
  * NEEDSWORK: Given how chop_last_dir() works, this function is broken
  * when a local part has a colon in its path component, too.
  */
@@ -115,8 +141,7 @@ static char *relative_url(const char *remote_url,
 	struct strbuf sb = STRBUF_INIT;
 	size_t len = strlen(remoteurl);
 
-	if (is_dir_sep(remoteurl[len-1]))
-		remoteurl[len-1] = '\0';
+	strip_url_ending(remoteurl, &len);
 
 	if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))
 		is_relative = 0;
@@ -149,10 +174,10 @@ static char *relative_url(const char *remote_url,
 	}
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s%s%s", remoteurl, colonsep ? ":" : "/", url);
-	if (ends_with(url, "/"))
-		strbuf_setlen(&sb, sb.len - 1);
 	free(remoteurl);
 
+	strip_url_ending(sb.buf, &sb.len);
+
 	if (starts_with_dot_slash(sb.buf))
 		out = xstrdup(sb.buf + 2);
 	else
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 25b48e5..e154e5f 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -329,14 +329,17 @@ test_submodule_relative_url "(null)" "./foo" "../submodule" "submodule"
 test_submodule_relative_url "(null)" "//somewhere else/repo" "../subrepo" "//somewhere else/subrepo"
 test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
 test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
-test_submodule_relative_url "(null)" "$PWD/." "../." "$(pwd)/."
-test_submodule_relative_url "(null)" "$PWD" "./." "$(pwd)/."
+test_submodule_relative_url "(null)" "$PWD/sub/." "../." "$(pwd)"
+test_submodule_relative_url "(null)" "$PWD/sub/./." "../." "$(pwd)"
+test_submodule_relative_url "(null)" "$PWD/sub/.////././/./." "../." "$(pwd)"
+test_submodule_relative_url "(null)" "$PWD" "./." "$(pwd)"
 test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$(pwd)/repo"
 test_submodule_relative_url "(null)" "$PWD" "./å äö" "$(pwd)/å äö"
-test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$(pwd)/submodule"
+test_submodule_relative_url "(null)" "$PWD/sub" "../submodule" "$(pwd)/submodule"
+test_submodule_relative_url "(null)" "$PWD/sub/." "../submodule" "$(pwd)/submodule"
 test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$(pwd)/submodule"
 test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$(pwd)/home2/../bundle1"
-test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$(pwd)/submodule_update_repo/."
+test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$(pwd)/submodule_update_repo"
 test_submodule_relative_url "(null)" "file:///tmp/repo" "../subrepo" "file:///tmp/subrepo"
 test_submodule_relative_url "(null)" "foo/bar" "../submodule" "foo/submodule"
 test_submodule_relative_url "(null)" "foo" "../submodule" "submodule"
-- 
2.10.1.480.g573bd76


^ permalink raw reply related

* Re: [PATCH] t0040: convert all possible tests to use `test-parse-options --expect`
From: Junio C Hamano @ 2016-10-17 21:51 UTC (permalink / raw)
  To: Pranit Bauva; +Cc: git
In-Reply-To: <01020157c8505fc5-cb313aa8-0182-4e19-95fa-cb36c573ee4c-000000@eu-west-1.amazonses.com>

Pranit Bauva <pranit.bauva@gmail.com> writes:

> Use "test-parse-options --expect" to rewrite the tests to avoid checking
> the whole variable dump by just testing what is required. This commit is
> based on 8ca65aeb (t0040: convert a few tests to use test-parse-options;
> Junio C Hamano; May 6, 2016).
>
> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> ---
>  t/t0040-parse-options.sh | 183 ++++-------------------------------------------
>  1 file changed, 13 insertions(+), 170 deletions(-)

Whoa.  Quite a lot of repetitions removed.


^ permalink raw reply

* Re: [PATCH] convert: mark a file-local symbol static
From: Ramsay Jones @ 2016-10-17 21:48 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Johannes Schindelin, Lars Schneider, GIT Mailing-list
In-Reply-To: <xmqqmvi2u4c0.fsf@gitster.mtv.corp.google.com>



On 17/10/16 21:07, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
>> Heh, I actually have the following in my config.mak already:
>>
>> extra-clean: clean
>> 	find . -iname '*.o' -exec rm {} \;
>>
>> But for some reason I _always_ type 'make clean' and then, to top
>> it off, I _always_ type the 'find' command by hand (I have no idea
>> why) :-D
> 
> "git clean -x" anybody?

I don't use 'git clean' because, on the very few occasions that I have
tried to use it, it always deletes _far_ more than I thought it would
or should. (particularly config.mak). Hmm, "git clean -X -- '*.o'"
_might_ do what I want, but 'find' is so much easier ... :-D

ATB,
Ramsay Jones


^ permalink raw reply


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