Re: [PATCH 01/11] t9902-completion: add tests demonstrating issues with quoted pathnames

[Junio C Hamano]

SZEDER Gábor  writes:

> Do any more new tests need FUNNYNAMES* prereq?

Hmph, all of these look like they involve some funnynames ;-)

> +test_expect_failure 'complete files - escaped characters on cmdline' '
> + test_when_finished "rm -rf \"New|Dir\"" &&
> + mkdir "New|Dir" &&
> + >"New|Dir/New" &&
> +
> + test_completion "git test-path-comp N" \
> + "New|Dir" &&# Bash will turn this into "New\|Dir/"
> + test_completion "git test-path-comp New\\|D" \
> + "New|Dir" &&
> + test_completion "git test-path-comp New\\|Dir/N" \
> + "New|Dir/New" && # Bash will turn this into
> + # "New\|Dir/New\ "
> + test_completion "git test-path-comp New\\|Dir/New\\" \
> + "New|Dir/New"
> +'
> +
> +test_expect_failure 'complete files - quoted characters on cmdline' '
> + test_when_finished "rm -r \"New(Dir\"" &&

This does not use -rf unlike the previous one?

> + mkdir "New(Dir" &&
> + >"New(Dir/New)File.c" &&
> +
> + test_completion "git test-path-comp \"New(D" "New(Dir" &&
> + test_completion "git test-path-comp \"New(Dir/New)F" \
> + "New(Dir/New)File.c"
> +'

OK.

> +test_expect_failure 'complete files - UTF-8 in ls-files output' '
> + test_when_finished "rm -r árvíztűrő" &&
> + mkdir árvíztűrő &&
> + >"árvíztűrő/Сайн яваарай" &&
> +
> + test_completion "git test-path-comp á" "árvíztűrő" &&
> + test_completion "git test-path-comp árvíztűrő/С" \
> + "árvíztűrő/Сайн яваарай"
> +'
> +
> +if test_have_prereq !MINGW &&
> +   mkdir 'New\Dir' 2>/dev/null &&
> +   touch 'New\Dir/New"File.c' 2>/dev/null
> +then
> + test_set_prereq FUNNYNAMES_BS_DQ
> +else
> + say "Your filesystem does not allow \\ and \" in filenames."
> + rm -rf 'New\Dir'
> +fi
> +test_expect_failure FUNNYNAMES_BS_DQ \
> +'complete files - C-style escapes in ls-files output' '
> + test_when_finished "rm -r \"NewDir\"" &&
> +
> + test_completion "git test-path-comp N" "New\\Dir" &&
> + test_completion "git test-path-comp NewD" "New\\Dir" &&
> + test_completion "git test-path-comp NewDir/N" \
> + "New\\Dir/New\"File.c" &&
> + test_completion "git test-path-comp NewDir/New\\\"F" \
> + "New\\Dir/New\"File.c"
> +'
> +
> +if test_have_prereq !MINGW &&
> +   mkdir $'New\034Special\035Dir' 2>/dev/null &&
> +   touch $'New\034Special\035Dir/New\036Special\037File' 2>/dev/null

The $'...' quote is bash-ism, but this is about testing bash
completion, so as long as we make sure non-bash shells won't touch
this part of the test, it is OK.

Do we want to test a more common case of a filename that is two
words with SP in between, i.e. 

$ >'hello world' && git add hel

or is it known to work just fine without quoting/escaping (because
the funny we care about is output from ls-files and SP is not special
in its one-item-at-a-time-on-a-line output) and not worth checking?

Re: [PATCH 2/2] builtin/blame: highlight recently changed lines

[Junio C Hamano]

It seems that this

$ git -c color.blame.repeatedlines=cyan blame --heated-lines builtin/blame.c

refuses to run.

Would it work if the configuration is in .git/config instead, or
would it forever disable --heated-lines once somebody choses to use
--color-lines feature by default by configuring it in?

Re: [PATCH 2/2] builtin/blame: highlight recently changed lines

[Junio C Hamano]

Stefan Beller  writes:

> Choose a different color for dates and imitate a 'temperature cool down'
> depending upon age.
>
> Originally I had planned to have the temperature cooldown dependent on
> the age of the project or file for example, as that might scale better,
> but that can be added on top of this commit, e.g. instead of giving a
> date, you could imagine giving a percentage that would be the linearly
> interpolated between now and the beginning of the file.
> ...
> @@ -323,6 +324,7 @@ static const char *format_time(timestamp_t time, const 
> char *tz_str,
>  #define OUTPUT_SHOW_EMAIL0400
>  #define OUTPUT_LINE_PORCELAIN01000
>  #define OUTPUT_COLOR_LINE02000
> +#define OUTPUT_HEATED_LINES  04000

How about calling it OUTPUT_SHOW_AGE_WITH_COLOR or something like
that instead?  Anything with "AGE" in it, if that is what you are
trying to indicate, would be more appropriate than "HEATED", which
does not convey much meaning to readers unless you explain what
determines the temperature in your mind.

Re: [PATCH 1/2] builtin/blame: dim uninteresting metadata lines

[Junio C Hamano]

Stefan Beller  writes:

> @@ -316,10 +318,11 @@ static const char *format_time(timestamp_t time, const 
> char *tz_str,
>  #define OUTPUT_PORCELAIN 010
>  #define OUTPUT_SHOW_NAME 020
>  #define OUTPUT_SHOW_NUMBER   040
> -#define OUTPUT_SHOW_SCORE  0100
> -#define OUTPUT_NO_AUTHOR   0200
> +#define OUTPUT_SHOW_SCORE0100
> +#define OUTPUT_NO_AUTHOR 0200

I wondered what these are about; they are about aligning with HT
assuming 8-place tab stop like the other lines.

>  #define OUTPUT_SHOW_EMAIL0400
> -#define OUTPUT_LINE_PORCELAIN 01000
> +#define OUTPUT_LINE_PORCELAIN01000

But then this line has SP plus HT here; it should have been just a
single HT (i.e. replace a single SP with HT), to be consistent.

> +#define OUTPUT_COLOR_LINE02000
>  
>  static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
>  {
> @@ -375,6 +378,7 @@ static void emit_other(struct blame_scoreboard *sb, 
> struct blame_entry *ent, int
>   struct commit_info ci;
>   char hex[GIT_MAX_HEXSZ + 1];
>   int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
> + const char *color = "", *reset = "";
>  
>   get_commit_info(suspect->commit, , 1);
>   oid_to_hex_r(hex, >commit->object.oid);
> @@ -384,6 +388,19 @@ static void emit_other(struct blame_scoreboard *sb, 
> struct blame_entry *ent, int
>   char ch;
>   int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : 
> abbrev;
>  
> + if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {
> + if (opt & OUTPUT_COLOR_LINE) {
> + if (cnt > 0) {
> + color = repeated_meta_color;
> + reset = GIT_COLOR_RESET;
> + } else  {
> + color ="";

You need a SP after '=' that assigns an empty string to 'color'.

> + reset = "";
> + }
> + }
> + fputs(color, stdout);
> + }

This fputs() should be hidden to the case where color is *NOT* an
empty string, no?  In any case, it should be inside "if color-line
is in effect" block, I would think.

Users of "git annotate" would not pass the --color option, so I do
not think the outer if () block is worth the loss of readability due
to increased indent level.  

I would say that it is a job of the caller of git_config() to make
sure color.blame.lines would not take effect when the command is
annotate, if what you are worried about is the configuration in this
code.

> @@ -433,6 +450,9 @@ static void emit_other(struct blame_scoreboard *sb, 
> struct blame_entry *ent, int
>   printf(" %*d) ",
>  max_digits, ent->lno + 1 + cnt);
>   }
> + if (!(opt & OUTPUT_ANNOTATE_COMPAT) &&
> + (opt & OUTPUT_COLOR_LINE))
> + fputs(reset, stdout);

Likewise.

> @@ -949,8 +979,12 @@ int cmd_blame(int argc, const char **argv, const char 
> *prefix)
>  
>   blame_coalesce();
>  
> - if (!(output_option & OUTPUT_PORCELAIN))
> + if (!(output_option & OUTPUT_PORCELAIN)) {
>   find_alignment(, _option);
> + if (!*repeated_meta_color &&
> + (output_option & OUTPUT_COLOR_LINE))
> + strcpy(repeated_meta_color, GIT_COLOR_DARK);
> + }

This code does not check OUTPUT_ANNOTATE_COMPAT because it assumes
that OUTPUT_COLOR_LINE won't be in output_option when we are working
in annotate compatibility mode.  The deeper codepaths we saw above
should do the same.  It should be enough to drop color-line when
anno-compat is set, or something like that, immediately after
reading the config and overriding them from the command line.

> diff --git a/color.h b/color.h
> index cd0bcedd08..196be16058 100644
> --- a/color.h
> +++ b/color.h
> @@ -30,6 +30,7 @@ struct strbuf;
>  #define GIT_COLOR_BLUE   "\033[34m"
>  #define GIT_COLOR_MAGENTA"\033[35m"
>  #define GIT_COLOR_CYAN   "\033[36m"
> +#define GIT_COLOR_DARK   "\033[1;30m"
>  #define GIT_COLOR_BOLD_RED   "\033[1;31m"
>  #define GIT_COLOR_BOLD_GREEN "\033[1;32m"
>  #define GIT_COLOR_BOLD_YELLOW"\033[1;33m"

I wonder if it is worth adding a new color only to give this a
different default.  

Traditionally, we use CYAN for lines that are less interesting than
others (e.g. hunk header), so reusing it might make the life easier
to the users, especially because I envision that we may want to
introduce another "logical" level to give another redirection
between the configuration keys like color.diff.frag and
color.blame.repeatedlines and the actual ANSI sequence like
"\033[36m".  I.e. we update the system so that these two
configuration keys take "uninteresting" (which is one of the
"logical" colors) by default, and 

Re: [PATCH] glossary: substitute "ancestor" for "direct ancestor" in 'push' description.

[Junio C Hamano]

Sergey Organov  writes:

> Even though "direct ancestor" is not defined in the glossary, the
> common meaning of the term is simply "parent", parents being the only
> direct ancestors, and the rest of ancestors being indirect ancestors.

Makes sense.  If there were distinction among parents of a single
child, perhaps a direct ancestor might be a term to refer to an
ancestor that is reached by following only the special kind of
parent (rather than "side" ancestor that involves at least one hop
of other lessor kind of parent), but there isn't anything like that
in the system, so there is no need to use a confusing and undefined
term here in the documentation.

> As "parent" is obviously wrong in this place in the description, we
> should simply say "ancestor", as everywhere else.

Yup.  Great.

>
> Signed-off-by: Sergey Organov 
> ---
>  Documentation/glossary-content.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/glossary-content.txt 
> b/Documentation/glossary-content.txt
> index 6bd..6c2d23d 100644
> --- a/Documentation/glossary-content.txt
> +++ b/Documentation/glossary-content.txt
> @@ -463,7 +463,7 @@ exclude;;
>  [[def_push]]push::
>   Pushing a <> means to get the branch's
>   <> from a remote <>,
> - find out if it is a direct ancestor to the branch's local
> + find out if it is an ancestor to the branch's local
>   head ref, and in that case, putting all
>   objects, which are <> from the local
>   head ref, and which are missing from the remote

Re: Draft of Git Rev News edition 38

[Kaartic Sivaraam]

Hi,

On Tuesday 17 April 2018 03:56 AM, Christian Couder wrote:
> Hi,
> 
> On Mon, Apr 16, 2018 at 5:07 PM, Kaartic Sivaraam
>  wrote:
>>
>> That said, I read the draft and found it good except for two minor issues,
> 
> Thanks for your comments!
> 

You're welcome!

I'm sorry to say that I read only part of the draft  when I sent my
previous email though I accidentally didn't mention it explicitly.

Now that I have read the draft completely I find a few typos in the
"Developer Spotlight: Jiang Xin" section:

1.
"... because I feel it is hard to track changes of GitHub UI and the
book will become obsolte very quickly."

obsolte -> obsolete


2.
"We also developped ..."

developped -> developed


On seeing the section "Light reading" to be empty, I thought I could
suggest something. I'm not sure whether you take Stack Overflow answers
for a light reading but I found the following answer to be interesting,

https://stackoverflow.com/a/6521223/5614968

That's all.

-- 
Kaartic

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v3] bisect: create 'bisect_flags' parameter in find_bisection()

[Junio C Hamano]

Harald Nordgren  writes:

> Make it possible to implement bisecting only on first parents or on
> merge commits by passing flags to find_bisection(), instead of just
> a 'find_all' boolean.
>
> Signed-off-by: Harald Nordgren 
> ---
>
> Notes:
> Updating commit message

Looks like a reasonable preparatory step *if* we want to later be
able to pass other options down the same callchain.  It would be
nice to have the second step that this praparatory step is meant to
help as a follow-up (or 2/2 of a two-patch series, where this one is
1/2) not in too distant future.

Re: [PATCH 8/8] gpg-interface: handle alternative signature types

[Junio C Hamano]

"brian m. carlson"  writes:

> If we just want to add gpgsm support, that's fine, but we should be
> transparent about that fact and try to avoid making an interface which
> is at once too generic and not generic enough.

One thing that makes me somewhat worried is that "add gpgsm support"
may mean "don't encourage people to use the same PGP like everybody
else does" and instead promote fragmenting the world.

But that aside, assuming that it is a good idea to support gpgsm, I
fully agree with your above assessment.

Thanks.

Re: Bug: rebase -i creates committer time inversions on 'reword'

[Junio C Hamano]

Johannes Sixt  writes:

> Am 15.04.2018 um 23:35 schrieb Junio C Hamano:
>> Ah, do you mean we have an internal sequence like this, when "rebase
>> --continue" wants to conclude an edit/reword?
>
> Yes, it's only 'reword' that is affected, because then subsequent
> picks are processed by the original process.

Ah, OK, that is a good explanation.

Re: man page for "git remote set-url" seems confusing/contradictory

[Jacob Keller]

On Mon, Apr 16, 2018 at 4:13 PM, Junio C Hamano  wrote:
> Jacob Keller  writes:
>
>> Things won't work so well if you set the push url and fetch url to
>> different repositories. Git assumes that refs updated by "push" will
>> also be reflected via "fetch".
>>
>> I don't know offhand what will break, but likely something will. For
>> one, when you fetch again it will rewind your remotes after the push.
>
> Exactly.  I still haven't fully embraced it myself, but for a long
> time, "git push" pretends as if it fetched from that remote and
> updates the corresponding remote tracking branches (if you have
> any), so you'll be inviting inconsistent behaviour if you set your
> fetch and push URLs pointing at two logically separate places.
>
> This is a tangent, but there probably should be a boolean that
> disables this feature in "git push" per destination repository,
> i.e. "when pushing into this repository, pretend that we immediately
> fetched from the refs we just pushed to and update the remote
> tracking branches we have for them: yes/no".  It is not entirely
> implausible to envision an overly smart remote repository that turns
> a non-fast-forward push into an automatic rebase when it is safe to
> do so, instead of failing such a push, and you'd disable the "assume
> what we pushed would appear there" when talking to such a remote.
>

Not to mention something like gerrit which uses magical references
"refs/publish/branchname" which don't actually get generated on the
server.

Thanks,
Jake

Re: Draft of Git Rev News edition 38

[Jacob Keller]

On Mon, Apr 16, 2018 at 3:30 PM, Christian Couder
 wrote:
> On Mon, Apr 16, 2018 at 5:19 PM, Sergey Organov  wrote:
>> Kaartic Sivaraam  writes:
>>
>>> 1. I see the following sentence in the "Rebasing merges: a jorney to the
>>> ultimate solution (Road Clear) (written by Jacob Keller)" article
>>>
>>>   "A few examples were tried, but it was proven that the original
>>>   concept did not work, as dropped commits could end up being
>>>   replaid into the merge commits, turning them into "evil"
>>>   merges."
>>>
>>> I'm not sure if 'replaid' is proper English assuming the past tense of
>>> replay was intended there (which I think is 'replayed').
>>
>> It could have meant, say, "reapplied", -- we need to ask the author.
>
> Yeah it could but I would say that it is not very likely compared to
> "replayed", so I changed it to "replayed". And yeah I can change it to
> something else if Jake (who is Cc'ed) prefers.
>
>> While we are at it, please also consider to replace "original concept"
>> by "original algorithm", as it didn't work due to a mistake in the
>> algorithm as opposed to failure of the concept itself.
>
> Ok, it's now "original algorithm".
>
> Thanks,
> Christian.

Replayed is accurate.

Thanks,
Jake

Re: [PATCH] worktree: accept -f as short for --force for removal

[Eric Sunshine]

On Mon, Apr 16, 2018 at 6:16 PM, Stefan Beller  wrote:
> The force flag is very common in many commands and is commonly
> abbreviated with '-f', so add that to worktree removal, too
>
> Signed-off-by: Stefan Beller 
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -783,7 +783,7 @@ static int remove_worktree(int ac, const char **av, const 
> char *prefix)
>  {
> int force = 0;
> struct option options[] = {
> -   OPT_BOOL(0, "force", ,
> +   OPT_BOOL('f', "force", ,
>  N_("force removing even if the worktree is dirty")),

Should this be using OPT__FORCE, rather than OPT_BOOL, to be
consistent with worktree.c:add()?

Also, can you amend the commit message to mention that "git worktree
remove -f" was already documented, and that it was only the
implementation which was lacking? Thanks.

> OPT_END()
> };

Re: [PATCH 8/8] gpg-interface: handle alternative signature types

[brian m. carlson]

On Mon, Apr 16, 2018 at 02:05:32PM +0900, Junio C Hamano wrote:
> "brian m. carlson"  writes:
> 
> > On Tue, Apr 10, 2018 at 04:24:27AM -0400, Eric Sunshine wrote:
> >> How confident are we that _all_ possible signing programs will conform
> >> to the "-BEGIN %s-" pattern? If we're not confident, then
> >> perhaps the user should be providing the full string here, not just
> >> the '%s' part?
> >
> > This is not likely to be true of other signing schemes.  In fact, other
> > than OpenPGP, PEM, and CMS (S/MIME), this is probably not true at all.
> 
> Hmph.  
> 
> That argues more strongly that we would regret unless we make the
> end-user configuration to at least the whole string (which later can
> be promoted to "a pattern that matches the whole string"), not just
> the part after mandatory "-BEGIN ", methinks.

Yeah, I think this patch set is "add gpgsm support", which I can see as
a valuable goal in and of itself, but I'm not sure the attempt to make
it generic is in the right place.  If we want to be truly generic, the
way to do that is to invoke a helper based on signature type (e.g.
git-sign-gpg, git-sign-gpgsm, git-sign-signify) to do the signing and
verification.  We need not ship these helpers ourselves; interested
third-parties can provide them, and we can add configuration to match
against regexes for non-built-in types (which is required for many other
formats).

If we just want to add gpgsm support, that's fine, but we should be
transparent about that fact and try to avoid making an interface which
is at once too generic and not generic enough.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

[BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key

[Thandesha VK]

git p4 clone fails when p4 sizes does not return 'fileSize' key. There
are few cases when p4 sizes returens 0 size and with marshaled output,
it doesn’t return the fileSize attribute.

Here is the demonstration and potential fix



$ cd /tmp/git/



$ git remote -v

origin  https://github.com/git/git.git (fetch)

origin  https://github.com/git/git.git (push)



$ git branch  -v

* master fe0a9eaf3 Merge branch 'svn/authors-prog-2' of
git://bogomips.org/git-svn



Problem:



$ /tmp/git/git-p4.py clone //depot//@all   . –verbose

.

.

.

Traceback (most recent call last):

  File "/tmp/git/git-p4.py", line 3840, in 

main()

  File "/tmp/git/git-p4.py", line 3834, in main

if not cmd.run(args):

  File "/tmp/git/git-p4.py", line 3706, in run

if not P4Sync.run(self, depotPaths):

  File "/tmp/git/git-p4.py", line 3568, in run

self.importChanges(changes)

  File "/tmp/git/git-p4.py", line 3240, in importChanges

self.initialParent)

  File "/tmp/git/git-p4.py", line 2858, in commit

self.streamP4Files(files)

  File "/tmp/git/git-p4.py", line 2750, in streamP4Files

cb=streamP4FilesCbSelf)

  File "/tmp/git/git-p4.py", line 552, in p4CmdList

cb(entry)

  File "/tmp/git/git-p4.py", line 2744, in streamP4FilesCbSelf

self.streamP4FilesCb(entry)

  File "/tmp/git/git-p4.py", line 2692, in streamP4FilesCb

self.streamOneP4File(self.stream_file, self.stream_contents)

  File "/tmp/git/git-p4.py", line 2569, in streamOneP4File

size = int(self.stream_file['fileSize'])

KeyError: 'fileSize'



Signature of the sizes output resulting in this problem:

$ p4 -p   sizes //foo.c

//foo.c#5  bytes



$ p4 -p   -G sizes //foo.c

{scodesstatsdepotFiles4//fooc.c50



Signature for a file without problem:



$ p4 -p   sizes //bar.c

//bar.c#5 1105 bytes



$ p4 -p  -G  sizes //bar.c

{scodesstatsdepotFiles;//bar.csrevs5fileSizes11050



Patch:

$ git diff

diff --git a/git-p4.py b/git-p4.py

index 7bb9cadc6..f908e805e 100755

--- a/git-p4.py

+++ b/git-p4.py

@@ -2565,7 +2565,7 @@ class P4Sync(Command, P4UserMap):

 def streamOneP4File(self, file, contents):

 relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)

 relPath = self.encodeWithUTF8(relPath)

-if verbose:

+if verbose and 'fileSize' in self.stream_file:

 size = int(self.stream_file['fileSize'])

 sys.stdout.write('\r%s --> %s (%i MB)\n' %
(file['depotFile'], relPath, size/1024/1024))

 sys.stdout.flush()



Thanks & Regards

Thandesha

Re: man page for "git remote set-url" seems confusing/contradictory

[Junio C Hamano]

Jacob Keller  writes:

> Things won't work so well if you set the push url and fetch url to
> different repositories. Git assumes that refs updated by "push" will
> also be reflected via "fetch".
>
> I don't know offhand what will break, but likely something will. For
> one, when you fetch again it will rewind your remotes after the push.

Exactly.  I still haven't fully embraced it myself, but for a long
time, "git push" pretends as if it fetched from that remote and
updates the corresponding remote tracking branches (if you have
any), so you'll be inviting inconsistent behaviour if you set your
fetch and push URLs pointing at two logically separate places.

This is a tangent, but there probably should be a boolean that
disables this feature in "git push" per destination repository,
i.e. "when pushing into this repository, pretend that we immediately
fetched from the refs we just pushed to and update the remote
tracking branches we have for them: yes/no".  It is not entirely
implausible to envision an overly smart remote repository that turns
a non-fast-forward push into an automatic rebase when it is safe to
do so, instead of failing such a push, and you'd disable the "assume
what we pushed would appear there" when talking to such a remote.

THIS MESSAGE IS IMPORTANT FOR YOU

[Elizabeth White]

I HAVE AN IMPORTANT MESSAGE TO DISCUSS WITH YOU, PLEASE WRITE BACK TO ME FOR 
DETAILS.

REGARDS,
ELIZABETH WHITE


-Sent from IPad

Re: [PATCH] show: add --follow-symlinks option for :

[Junio C Hamano]

Michael Vogt  writes:

> Add a --follow-symlinks option that'll resolve symlinks to their
> targets when the target is of the form :.

This not only affects "show" but all in the "log" family of
commands, because the change is made to revision.[ch] that is shared
by them.  I doubt that is desirable.

I offhand do not think of any command other than "show", to which
this feature makes any sense [*1*].  And I certainly do not mind if
the feature is limited to "show" and nothing else for that reason.

But I do mind the implementation that seeps through to other
commands (because "log_init_finish()" is shared with commands in the
family other than "show", and because "struct rev_info" is shared
across all the commands in the "log" famil) and not limited to
"show", which is a sign of typical end-user confusion waiting to
happen.

Thanks.


[Footnote]

For example, "git log" is affected by this patch but I am not sure
what it even means that we can ask this question:

$ git log -p --follow-symlinks -- RelNotes

Can we see how each update to Documentation/RelNotes/2.17.0.txt as
well as change of RelNotes symlink from 2.17.0.txt to 2.18.0.txt in
the patch form?  If that were the case, it might make some sense to
allow the feature to be triggered by "git log" like your change to
builtin/log.c did, but I somehow do not think that is what your
patch does.

Re: Optimizing writes to unchanged files during merges?

[Elijah Newren]

On Sun, Apr 15, 2018 at 6:44 PM, Junio C Hamano  wrote:

> One thing that makes me curious is what happens (and what we want to
> happen) when such a "we already have the changes the side branch
> tries to bring in" path has local (i.e. not yet in the index)
> changes.  For a dirty file that trivially merges (e.g. a path we
> modified since our histories forked, while the other side didn't do
> anything, has local changes in the working tree), we try hard to
> make the merge succeed while keeping the local changes, and we
> should be able to do the same in this case, too.
>
> Your illustration patch that reads from the working tree to compare
> with the contents we are about to write out of course won't cope
> with this case ;-).  If we do things in the index like the approach
> to fix was_tracked(), I suspect that we would just say "as long as
> the index and the HEAD matches, i.e. we are keeping the path as it is
> found in HEAD as the merge result, we do not touch the working tree"
> and leave such a local modification alone.

I could see it going either way (fail early, or succeed because we
don't need to overwrite anything), but my suspicion is also towards
letting the merge succeed.

It looks like my patches need a little more fixing up (the was_dirty()
calls would be mixing both the old and the new index -- it used the
original index via was_tracked(), but used the current index via
cache_file_exists(); it should be more consistent).  I'm fixing that
up, will add this as another testcase to t6046, and will submit a
newer RFC in a day or so.

Re: Optimizing writes to unchanged files during merges?

[Elijah Newren]

On Sun, Apr 15, 2018 at 7:03 PM, Linus Torvalds
 wrote:
> On Sun, Apr 15, 2018 at 6:44 PM, Junio C Hamano  wrote:

>> One thing that makes me curious is what happens (and what we want to
>> happen) when such a "we already have the changes the side branch
>> tries to bring in" path has local (i.e. not yet in the index)
>> changes.  For a dirty file that trivially merges (e.g. a path we
>> modified since our histories forked, while the other side didn't do
>> anything, has local changes in the working tree), we try hard to
>> make the merge succeed while keeping the local changes, and we
>> should be able to do the same in this case, too.
>
> I think it might be nice, but probably not really worth it.

> So I don't think it's a big deal, and I'd rather have the merge fail
> very early with "that file has seen changes in the branch you are
> merging" than add any real complexity to the merge logic.

That's actually problematic, and not yet possible with the current
merge-recursive code.  The fail-very-early code is in unpack_trees(),
and it doesn't understand renames.  By the time we get to the rest of
the logic of merge-recursive, unpack_trees() has already written
updates to lots of files throughout the tree, so bailing and leaving
the tree in a half-merged state is no longer an option.  (The logic in
merge-recursive.c is excessively complex in part due to this issue,
making it implement what amounts to a four-way merge instead of just a
three-way merge.  It's gross.)

So, if we were to use the brute-force scheme here, when renames are
involved, we'd need to have some special logic for handling dirty
files.  That logic would probably include checking the original index
for both modification times (to determine if the file is dirty), and
for comparison of contents.  In short, we'd still need all the logic
that this scheme was trying to get rid of in the first place.

Re: [PATCH] completion: reduce overhead of clearing cached --options

[Junio C Hamano]

Jakub Narębski  writes:

> On 16 April 2018 at 15:15, SZEDER Gábor  wrote:
>> No.  'sed' would only need need help when its input comes from a buggy
>> 'set' builtin of a particular version of Bash from a particular vendor.
>>
>> As far as I can test this in Travis CI's OSX builds, ZSH doesn't seem to
>> be affected, neither the version Apple ships by default nor the version
>> installed via homebrew.
>
> That's nice - this means that the patch fixes all of the issue.
> The above information should be, in my opinion, included
> in the commit message, though.

Yeah, I tend to agree.  Thanks.

[PATCH 11/11] completion: fill COMPREPLY directly when completing paths

[SZEDER Gábor]

During git-aware path completion, when a lot of path components have
to be listed, a significant amount of time is spent in
__gitcomp_file(), or more accurately in the shell loop of
__gitcompappend(), iterating over all the path components filtering
path components matching the current word to be completed, adding
prefix path components, and placing the resulting matching paths into
the COMPREPLY array.

Now, a previous patch in this series made 'git ls-files' and 'git
diff-index' list only paths matching the current word to be completed,
so an additional filtering in __gitcomp_file() is not necessary
anymore.  Adding the prefix path components could be done much more
efficiently in __git_index_files()'s 'awk' script while stripping
trailing path components and removing duplicates and quoting.  And
then the resulting paths won't require any more filtering or
processing before being handed over to Bash, so we could fill the
COMPREPLY array directly.

Unfortunately, we can't simply use the __gitcomp_direct() helper
function to do that, because __gitcomp_file() does one additional
thing: it tells Bash that we are doing filename completion, so the
shell will kindly do four important things for us:

  1. Append a trailing space to all filenames.
  2. Append a trailing '/' to all directory names.
  3. Escape any meta, globbing, separator, etc. characters.
  4. List only the current path component when listing possible
 completions (i.e. 'dir/subdir/f' will list 'file1', 'file2',
 etc. instead of the whole 'dir/subdir/file1',
 'dir/subdir/file2').

While we could let __git_index_files()'s 'awk' script take care of the
first two points, the third one gets tricky, and we absolutely need
the shell's support for the fourth.

Add the helper function __gitcomp_file_direct(), which, just like
__gitcomp_direct(), fills the COMPREPLY array with prefiltered and
preprocessed paths without any additional processing, without a shell
loop, with just one single compound assignment, and, similar to
__gitcomp_file(), tells Bash and ZSH that we are doing filename
completion.  Extend __git_index_files()'s 'awk' script a bit to
prepend any prefix path components to all listed paths.  Finally,
modify __git_complete_index_file() to feed __git_index_files()'s
output to ___gitcomp_file_direct() instead of __gitcomp_file().

After this patch there is no shell loop left in the path completion
code path.

This speeds up path completion when there are a lot of paths matching
the current word to be completed.  In a pathological repository with
100k files in a single directory, listing all those files:

  Before this patch, best of five, using GNU awk on Linux:

$ time cur=dir/ __git_complete_index_file

real0m0.983s
user0m1.004s
sys 0m0.033s

  After:

real0m0.313s
user0m0.341s
sys 0m0.029s

  Difference: -68.2%
  Speedup:  3.1x

  To see the benefits of the whole patch series, the same command with
  v2.17.0:

real0m2.736s
user0m2.472s
sys 0m0.610s

  Difference: -88.6%
  Speedup:  8.7x

Note that this patch changes the output of the __git_index_files()
helper function by unconditionally prepending the prefix path
components to every listed path.  This would break users' completion
scriptlets that directly run:

  __gitcomp_file "$(__git_index_files ...)" "$pfx" "$cur_"

because that would add the prefix path components once more.
However, __git_index_files() is kind of a "helper function of a helper
function", and users' completion scriptlets should have been using
__git_complete_index_file() for git-aware path completion in the first
place, so this is likely doesn't worth worrying about.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 34 +++---
 contrib/completion/git-completion.zsh  |  9 +++
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index e97d57024b..360ee9ca51 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -404,6 +404,23 @@ __gitcomp_nl ()
__gitcomp_nl_append "$@"
 }
 
+# Fills the COMPREPLY array with prefiltered paths without any additional
+# processing.
+# Callers must take care of providing only paths that match the current path
+# to be completed and adding any prefix path components, if necessary.
+# 1: List of newline-separated matching paths, complete with all prefix
+#path componens.
+__gitcomp_file_direct ()
+{
+   local IFS=$'\n'
+
+   COMPREPLY=($1)
+
+   # use a hack to enable file mode in bash < 4
+   compopt -o filenames +o nospace 2>/dev/null ||
+   compgen -f /non-existing-dir/ > /dev/null
+}
+
 # Generates completion reply with compgen from newline-separated possible
 # completion filenames.
 # It accepts 1 to 3 arguments:
@@ -455,14 +472,14 @@ __git_index_files ()

[PATCH 10/11] completion: improve handling quoted paths in 'git ls-files's output

[SZEDER Gábor]

If any pathname contains backslash, double quote, tab, newline, or any
control characters, 'git ls-files' and 'git diff-index' will enclose
that pathname in double quotes and escape those special characters
using C-style one-character escape sequences or \nnn octal values.
This prevents those files from being listed during git-aware path
completion, because due to the quoting they will never match the
current word to be completed.

Extend __git_index_files()'s 'awk' script to remove all that quoting
and escaping from unique path components, so even paths containing
(almost all) such special characters can be completed.

Paths containing newline characters are still an issue, though.  We
use newlines as separator character when filling the COMPREPLY array,
so a path with one or more newline will end up split to two or more
elements in COMPREPLY, basically breaking completion.  There is
nothing we can do about it without a significant performance hit, so
let's just ignore such paths for now.  As far as paths with newlines
are concerned, this isn't any different from the previous behavior,
because those paths were always omitted, though in the past they were
omitted because due to the quoting they didn't match the current word
to be completed.  Anyway, Bash's own filename completion (Meta-/) can
complete even those paths, if need be.

Note:

  - We don't dequote path components right away as they are coming in,
because then we would have to dequote each directory name
repeatedly, as many times as it appears in the input, i.e. as many
times as the number of listed paths it contains.  Instead, we
dequote them at the end, as we print unique path components.

  - Even when a directory name itself does not contain any special
characters, it will still be quoted if any of its trailing path
components do.  If a directory contains paths both with and
without special characters, then the name of that directory will
appear both quoted and unquoted in the output of 'git ls-files'
and 'git diff-index'.  Consequently, we will add such a directory
name to the deduplicating associative array twice: once quoted and
once unquoted.

This means that we have to be careful after dequoting a directory
name, and only print it if we haven't seen the same directory name
unquoted.

  - It would be wonderful if we could just pass '-z' to those git
commands to output \0-separated unquoted paths, and use \0 as
record separator in the 'awk' script processing their output...
this patch would be so much simpler, almost trivial even.
Unfortunately, however, POSIX and most 'awk' implementations don't
support \0 as record separator (GNU awk does support it).

  - This patch makes the earlier change to list paths with
'core.quotePath=false' basically redundant, because this could
decode any \nnn-escaped non-ASCII character just fine, as well.
However, I suspect that 'git ls-files' can deal with those
non-ASCII characters faster than this updated 'awk' script; just
in case someone is burdened with tons of pathnames containing
non-ASCII characters.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 66 +-
 t/t9902-completion.sh  | 17 ++-
 2 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 70bc75dfc7..e97d57024b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -459,8 +459,70 @@ __git_index_files ()
paths[$1] = 1
}
END {
-   for (p in paths)
-   print p
+   for (p in paths) {
+   if (substr(p, 1, 1) != "\"") {
+   # No special characters, easy!
+   print p
+   continue
+   }
+
+   # The path is quoted.
+   p = dequote(p)
+   if (p == "")
+   continue
+
+   # Even when a directory name itself does not contain
+   # any special characters, it will still be quoted if
+   # any of its (stripped) trailing path components do.
+   # Because of this we may have seen the same direcory
+   # both quoted and unquoted.
+   if (p in paths)
+   # We have seen the same directory unquoted,
+   # skip it.
+   continue
+   else
+   print p
+   }
+   }
+   function dequote(p,bs_idx, out, esc, esc_idx, dec) {
+   # Skip opening double quote.
+   p = 

Re: [PATCH v2 3/6] generate-cmdlist.sh: keep all information in common-cmds.h

[Junio C Hamano]

SZEDER Gábor  writes:

>> +while read cmd category tags
>>  do
>> -   tag=$(echo "$tags" | sed "$substnum; s/[^0-9]//g")
>> +   name=${cmd/git-}
>
> There are two issues with this line:
>
> - This is a "regular" shell script, therefore it must not use pattern
>   substitution.

Oops.  I missed that.  Thanks.

>
> - The pattern substitution would remove the string "git-" in the middle of
>   the variable as well; I suspect this is undesired.
>
> I think that the remove matching prefix pattern substitution should be
> used here.

[PATCH 00/11] completion: path completion improvements: speedup and quoted paths

[SZEDER Gábor]

So, the highlights of this patch series are:

  - At the top of the worktree in linux.git with over 62k files the time
needed for 'git rm ' goes down from 2.15s to 0.052s.

  - Same place, uniquely completing Makefile with 'git rm Mak' goes
down from 2.16s to 0.033s.

  - Completing paths containing escaped or quoted characters (except
newline) will work properly, and won't fall back to Bash's filename
completion, e.g. git add File\\w' will list
'File\with\backslashes.c', but not the corresponding ignored object
file.

Well, at least it appears to be working for me, but dealing with quoting
especially is nuts.  If someone knows simpler ways to remove escaping
and quoting than in the functions added in patches 5 and 10, then
please, I would really love to learn something :)

Someone using ZSH regularly should have a look and test it.

SZEDER Gábor (11):
  t9902-completion: add tests demonstrating issues with quoted pathnames
  completion: move __git_complete_index_file() next to its helpers
  completion: simplify prefix path component handling during path
completion
  completion: support completing non-ASCII pathnames
  completion: improve handling quoted paths on the command line
  completion: let 'ls-files' and 'diff-index' filter matching paths
  completion: use 'awk' to strip trailing path components
  t9902-completion: ignore COMPREPLY element order in some tests
  completion: remove repeated dirnames with 'awk' during path completion
  completion: improve handling quoted paths in 'git ls-files's output
  completion: fill COMPREPLY directly when completing paths

 contrib/completion/git-completion.bash | 218 +
 contrib/completion/git-completion.zsh  |   9 +
 t/t9902-completion.sh  | 153 -
 3 files changed, 348 insertions(+), 32 deletions(-)

-- 
2.17.0.366.gbe216a3084

[PATCH 04/11] completion: support completing non-ASCII pathnames

[SZEDER Gábor]

Unless the user has 'core.quotePath=false' somewhere in the
configuration, both 'git ls-files' and 'git diff-index' will by
default quote any pathnames that contain bytes with values higher than
0x80, and escape those bytes as '\nnn' octal values.  This prevents
completing paths when the current path component to be completed
contains any non-ASCII, most notably UTF-8, characters, because none
of the listed quoted paths will match the current word on the command
line.

Set 'core.quotePath=false' for those 'git ls-files' and 'git
diff-index' invocations, so they won't consider bytes higher than 0x80
as "unusual", and won't quote pathnames containing such characters.

Note that pathnames containing backslash, double quote, or control
characters will still be quoted; a later patch in this series will
deal with those.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 6 --
 t/t9902-completion.sh  | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 72cd3add19..7072555960 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -369,10 +369,12 @@ __gitcomp_file ()
 __git_ls_files_helper ()
 {
if [ "$2" == "--committable" ]; then
-   __git -C "$1" diff-index --name-only --relative HEAD
+   __git -C "$1" -c core.quotePath=false diff-index \
+   --name-only --relative HEAD
else
# NOTE: $2 is not quoted in order to support multiple options
-   __git -C "$1" ls-files --exclude-standard $2
+   __git -C "$1" -c core.quotePath=false ls-files \
+   --exclude-standard $2
fi
 }
 
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index ff2e4a8f5f..a4f2c03b93 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1463,7 +1463,7 @@ test_expect_failure 'complete files - quoted characters 
on cmdline' '
"New(Dir/New)File.c"
 '
 
-test_expect_failure 'complete files - UTF-8 in ls-files output' '
+test_expect_success 'complete files - UTF-8 in ls-files output' '
test_when_finished "rm -r árvíztűrő" &&
mkdir árvíztűrő &&
>"árvíztűrő/Сайн яваарай" &&
-- 
2.17.0.366.gbe216a3084

[PATCH 03/11] completion: simplify prefix path component handling during path completion

[SZEDER Gábor]

Once upon a time 'git -C "" cmd' errored out with "Cannot change to
'': No such file or directory", therefore the completion script took
extra steps to run 'git -C "." cmd' instead; see fca416a41e
(completion: use "git -C $there" instead of (cd $there && git ...),
2014-10-09).

Those extra steps are not needed since 6a536e2076 (git: treat "git -C
''" as a no-op when  is empty, 2015-03-06), so remove
them.

While at it, also simplify how the trailing '/' is appended to the
variable holding the prefix path components.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 36d3c6f928..72cd3add19 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -385,7 +385,7 @@ __git_ls_files_helper ()
 #slash.
 __git_index_files ()
 {
-   local root="${2-.}" file
+   local root="$2" file
 
__git_ls_files_helper "$root" "$1" |
while read -r file; do
@@ -406,13 +406,12 @@ __git_complete_index_file ()
 
case "$cur_" in
?*/*)
-   pfx="${cur_%/*}"
+   pfx="${cur_%/*}/"
cur_="${cur_##*/}"
-   pfx="${pfx}/"
;;
esac
 
-   __gitcomp_file "$(__git_index_files "$1" ${pfx:+"$pfx"})" "$pfx" "$cur_"
+   __gitcomp_file "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_"
 }
 
 # Lists branches from the local repository.
-- 
2.17.0.366.gbe216a3084

[PATCH 07/11] completion: use 'awk' to strip trailing path components

[SZEDER Gábor]

During git-aware path completion we complete one path component at a
time, i.e. 'git add ' offers only 'dir/' at first, not
'dir/subdir/file' right away, just like Bash's own filename
completion.  However, since both 'git ls-files' and 'git diff-index'
dive deep into subdirectories, we have to strip all trailing path
components from the listed paths, keeping only the leading path
component.  This stripping is currently done in a shell loop in
__git_index_files(), which can take a significant amount of time when
it has to iterate through a large number of paths.

Replace this shell loop with a little 'awk' script using '/' as input
field separator and printing the first field, which produces the same
output much faster.

Listing all tracked files (12) and directories (23) at the top of the
worktree in linux.git (over 62k files), i.e. what's doing all the hard
work behind 'git rm ':

  Before this patch, best of five, using GNU awk on Linux:

$ time cur= __git_complete_index_file

real0m2.149s
user0m1.307s
sys 0m1.086s

  After:

real0m0.067s
user0m0.089s
sys 0m0.023s

  Difference: -96.9%
  Speedup: 32.1x

Note that this could be done with 'sed', or even with 'cut', just as
well, but the upcoming patches require 'awk's scriptability.

Note also that this change means one more fork()+exec()ed process
during path completion, adding more overhead especially on Windows,
but a later patch will more than make up for it by eliminating two
other processes in the same function.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 3948265d32..0abba88462 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -452,15 +452,12 @@ __git_ls_files_helper ()
 # 3: List only paths matching this path component (optional).
 __git_index_files ()
 {
-   local root="$2" match="$3" file
+   local root="$2" match="$3"
 
__git_ls_files_helper "$root" "$1" "$match" |
-   while read -r file; do
-   case "$file" in
-   ?*/*) echo "${file%%/*}" ;;
-   *) echo "$file" ;;
-   esac
-   done | sort | uniq
+   awk -F / '{
+   print $1
+   }' | sort | uniq
 }
 
 # __git_complete_index_file requires 1 argument:
-- 
2.17.0.366.gbe216a3084

[PATCH 08/11] t9902-completion: ignore COMPREPLY element order in some tests

[SZEDER Gábor]

The order or possible completion words in the COMPREPLY array doesn't
actually matter, as long as all the right words are in there, because
Bash will sort them anyway.  Yet, our tests looking at the elements of
COMPREPLY always expect them to be in a specific order.

Now, this hasn't been an issue before, but the next patch is about to
optimize a bit more our git-aware path completion, and as a harmless
side effect the order of elements in COMPREPLY will change.  Worse,
the order will be downright undefined, because after the next patch
path components will come directly from iterating through an
associative array in 'awk', and the order of iteration over the
elements in those arrays is undefined, and indeed different 'awk'
implementations produce different order.  Consequently, we can't get
away with simply adjusting the expected results in the affected tests.

Modify the 'test_completion' helper function to sort both the expected
and the actual results, i.e. the elements in COMPREPLY, before
comparing them, so the tests using this helper function will work
regardless of the order of elements.

Note that this change still leaves a bunch of tests depending on the
order of elements in COMPREPLY, tests that focus on a specific helper
function and therefore don't use the 'test_completion' helper.  I
would rather deal with those later, when (if ever) the need actually
arises, than create unnecessary code churn now.

Signed-off-by: SZEDER Gábor 
---
 t/t9902-completion.sh | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f7a9dd446d..a747998d6c 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -84,10 +84,11 @@ test_completion ()
then
printf '%s\n' "$2" >expected
else
-   sed -e 's/Z$//' >expected
+   sed -e 's/Z$//' |sort >expected
fi &&
run_completion "$1" &&
-   test_cmp expected out
+   sort out >out_sorted &&
+   test_cmp expected out_sorted
 }
 
 # Test __gitcomp.
@@ -1405,6 +1406,7 @@ test_expect_success 'complete files' '
 
echo "expected" > .gitignore &&
echo "out" >> .gitignore &&
+   echo "out_sorted" >> .gitignore &&
 
git add .gitignore &&
test_completion "git commit " ".gitignore" &&
-- 
2.17.0.366.gbe216a3084

[PATCH 06/11] completion: let 'ls-files' and 'diff-index' filter matching paths

[SZEDER Gábor]

During git-aware path completion, e.g. 'git rm dir/fil', both
'git ls-files' and 'git diff-index' list all paths in the given 'dir/'
matching certain criteria (cached, modified, untracked, etc.)
appropriate for the given git command, even paths whose names don't
begin with 'fil'.  This comes with a considerable performance
penalty when the directory in question contains a lot of paths, but
the current word can be uniquely completed or when only a handful of
those paths match the current word.

Reduce the number of iterations in this codepath from the number of
paths to the number of matching paths by specifying an appropriate
globbing pattern to 'git ls-files' and 'git diff-index' to list only
paths that match the current word to be completed.

Note that both commands treat backslashes as escape characters in
their file arguments, e.g. to preserve the literal meaning of globbing
characters, so we have to double every backslash in the globbing
pattern.  This is why one of the path completion tests specifically
checks the completion of a path containing a literal backslash
character (that test still fails, though, because both commands output
such paths enclosed in double quotes and the special characters
escaped; a later patch in this series will deal with those).

This speeds up path completion considerably when there are a lot of
non-matching paths to be filtered out.  Uniquely completing a tracked
filename at the top of the worktree in linux.git (over 62k files),
i.e. what's doing all the hard work behind 'git rm Mak' to
complete 'Makefile':

  Before this patch, best of five, on Linux:

$ time cur=Mak __git_complete_index_file

real0m2.159s
user0m1.299s
sys 0m1.089s

  After:

real0m0.033s
user0m0.023s
sys 0m0.015s

  Difference: -98.5%
  Speedup: 65.4x

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 11 ++-
 t/t9902-completion.sh  |  1 +
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index ae6127155e..3948265d32 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -434,11 +434,11 @@ __git_ls_files_helper ()
 {
if [ "$2" == "--committable" ]; then
__git -C "$1" -c core.quotePath=false diff-index \
-   --name-only --relative HEAD
+   --name-only --relative HEAD -- "${3//\\/}*"
else
# NOTE: $2 is not quoted in order to support multiple options
__git -C "$1" -c core.quotePath=false ls-files \
-   --exclude-standard $2
+   --exclude-standard $2 -- "${3//\\/}*"
fi
 }
 
@@ -449,11 +449,12 @@ __git_ls_files_helper ()
 #If provided, only files within the specified directory are listed.
 #Sub directories are never recursed.  Path must have a trailing
 #slash.
+# 3: List only paths matching this path component (optional).
 __git_index_files ()
 {
-   local root="$2" file
+   local root="$2" match="$3" file
 
-   __git_ls_files_helper "$root" "$1" |
+   __git_ls_files_helper "$root" "$1" "$match" |
while read -r file; do
case "$file" in
?*/*) echo "${file%%/*}" ;;
@@ -481,7 +482,7 @@ __git_complete_index_file ()
cur_="$dequoted_word"
esac
 
-   __gitcomp_file "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_"
+   __gitcomp_file "$(__git_index_files "$1" "$pfx" "$cur_")" "$pfx" "$cur_"
 }
 
 # Lists branches from the local repository.
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 6856b263f8..f7a9dd446d 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1515,6 +1515,7 @@ test_expect_success 'complete files - UTF-8 in ls-files 
output' '
"árvíztűrő/Сайн яваарай"
 '
 
+# Testing with a path containing a backslash is important.
 if test_have_prereq !MINGW &&
mkdir 'New\Dir' 2>/dev/null &&
touch 'New\Dir/New"File.c' 2>/dev/null
-- 
2.17.0.366.gbe216a3084

[PATCH 09/11] completion: remove repeated dirnames with 'awk' during path completion

[SZEDER Gábor]

During git-aware path completion, after all the trailing path
components have been removed from the output of 'git ls-files' and
'git diff-index' (see previous patch), each directory name is repeated
as many times as the number of listed paths it contains.  This can be
a lot of repetitions, especially when invoking path completion close
to the root of a big worktree, which would cause a considerable
overhead downstream of __git_index_files(), in particular in the shell
loop that fills the COMPREPLY array.  To reduce this overhead,
__git_index_files() runs the classic '... |sort |uniq' pattern to
remove those repetitions from the function's output.

While removing repeated directory names is effective in reducing the
number of iterations in that shell loop, it still imposes the overhead
of fork()+exec()ing two external processes, and two additional stages
in the pipeline, where potentially relatively large amount of data can
be passed between two subsequent pipeline stages.

Extend __git_index_files()'s 'awk' script to remove repeated path
components by first creating and filling an associative array indexed
by all encountered path components (after the trailing path components
have been removed), and then iterating over this array and printing
the indices, i.e. unique path components.  This way we can remove the
'|sort |uniq' pipeline stages, and their eliminated overhead results
in faster path completion.

Listing all tracked files (12) and directories (23) at the top of the
worktree in linux.git (over 62k files), i.e. what's doing all the hard
work behind 'git rm ':

  Before this patch, best of five, using GNU awk on Linux:

real0m0.069s
user0m0.089s
sys 0m0.026s

  After:

real0m0.052s
user0m0.072s
sys 0m0.014s

  Difference: -24.6%

Note that this changes order of elements in __git_index_files()'s
output.  This is not an issue, because this function was only ever
intended to feed paths into the COMPREPLY array, and Bash will sort
its elements (according to the users locale) anyway.

Note also that using 'awk' to remove repeated path components is also
beneficial for the performance of the next two patches:

  - The first will extend this 'awk' script to dequote quoted paths in
the output of 'git ls-files' and 'git diff-index'.  With this
patch it will only have to dequote unique path components, not
all.

  - The second will, among other things, extend this 'awk' script to
prepend prefix path components from the command line to the
currently completed path component.  Consequently, each line in
'awk's output will grow longer.  Without this patch that '|sort
|uniq' would have to exchange and process that much more data.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 0abba88462..70bc75dfc7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -456,8 +456,12 @@ __git_index_files ()
 
__git_ls_files_helper "$root" "$1" "$match" |
awk -F / '{
-   print $1
-   }' | sort | uniq
+   paths[$1] = 1
+   }
+   END {
+   for (p in paths)
+   print p
+   }'
 }
 
 # __git_complete_index_file requires 1 argument:
-- 
2.17.0.366.gbe216a3084

[PATCH 01/11] t9902-completion: add tests demonstrating issues with quoted pathnames

[SZEDER Gábor]

Completion functions see all words on the command line verbatim,
including any backslash-escapes, single and double quotes that might
be there.  Furthermore, git commands quote pathnames if they contain
certain special characters.  All these create various issues when
doing git-aware path completion.

Add a couple of failing tests to demonstrate these issues.

Later patches in this series will discuss these issues in detail as
they fix them.

Signed-off-by: SZEDER Gábor 
---

Notes:
Do any more new tests need FUNNYNAMES* prereq?

 t/t9902-completion.sh | 91 +++
 1 file changed, 91 insertions(+)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index b7f5b1e632..ff2e4a8f5f 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1427,6 +1427,97 @@ test_expect_success 'complete files' '
test_completion "git add mom" "momified"
 '
 
+# The next tests only care about how the completion script deals with
+# unusual characters in path names.  By defining a custom completion
+# function to list untracked files they won't be influenced by future
+# changes of the completion functions of real git commands, and we
+# don't have to bother with adding files to the index in these tests.
+_git_test_path_comp ()
+{
+   __git_complete_index_file --others
+}
+
+test_expect_failure 'complete files - escaped characters on cmdline' '
+   test_when_finished "rm -rf \"New|Dir\"" &&
+   mkdir "New|Dir" &&
+   >"New|Dir/New" &&
+
+   test_completion "git test-path-comp N" \
+   "New|Dir" &&# Bash will turn this into "New\|Dir/"
+   test_completion "git test-path-comp New\\|D" \
+   "New|Dir" &&
+   test_completion "git test-path-comp New\\|Dir/N" \
+   "New|Dir/New" && # Bash will turn this into
+   # "New\|Dir/New\ "
+   test_completion "git test-path-comp New\\|Dir/New\\" \
+   "New|Dir/New"
+'
+
+test_expect_failure 'complete files - quoted characters on cmdline' '
+   test_when_finished "rm -r \"New(Dir\"" &&
+   mkdir "New(Dir" &&
+   >"New(Dir/New)File.c" &&
+
+   test_completion "git test-path-comp \"New(D" "New(Dir" &&
+   test_completion "git test-path-comp \"New(Dir/New)F" \
+   "New(Dir/New)File.c"
+'
+
+test_expect_failure 'complete files - UTF-8 in ls-files output' '
+   test_when_finished "rm -r árvíztűrő" &&
+   mkdir árvíztűrő &&
+   >"árvíztűrő/Сайн яваарай" &&
+
+   test_completion "git test-path-comp á" "árvíztűrő" &&
+   test_completion "git test-path-comp árvíztűrő/С" \
+   "árvíztűrő/Сайн яваарай"
+'
+
+if test_have_prereq !MINGW &&
+   mkdir 'New\Dir' 2>/dev/null &&
+   touch 'New\Dir/New"File.c' 2>/dev/null
+then
+   test_set_prereq FUNNYNAMES_BS_DQ
+else
+   say "Your filesystem does not allow \\ and \" in filenames."
+   rm -rf 'New\Dir'
+fi
+test_expect_failure FUNNYNAMES_BS_DQ \
+'complete files - C-style escapes in ls-files output' '
+   test_when_finished "rm -r \"NewDir\"" &&
+
+   test_completion "git test-path-comp N" "New\\Dir" &&
+   test_completion "git test-path-comp NewD" "New\\Dir" &&
+   test_completion "git test-path-comp NewDir/N" \
+   "New\\Dir/New\"File.c" &&
+   test_completion "git test-path-comp NewDir/New\\\"F" \
+   "New\\Dir/New\"File.c"
+'
+
+if test_have_prereq !MINGW &&
+   mkdir $'New\034Special\035Dir' 2>/dev/null &&
+   touch $'New\034Special\035Dir/New\036Special\037File' 2>/dev/null
+then
+   test_set_prereq FUNNYNAMES_SEPARATORS
+else
+   say 'Your filesystem does not allow special separator characters (FS, 
GS, RS, US) in filenames.'
+   rm -rf $'New\034Special\035Dir'
+fi
+test_expect_failure FUNNYNAMES_SEPARATORS \
+'complete files - \nnn-escaped control characters in ls-files output' '
+   test_when_finished "rm -r '$'New\034Special\035Dir''" &&
+
+   # Note: these will be literal separator characters on the cmdline.
+   test_completion "git test-path-comp N" "'$'New\034Special\035Dir''" &&
+   test_completion "git test-path-comp '$'New\034S''" \
+   "'$'New\034Special\035Dir''" &&
+   test_completion "git test-path-comp '$'New\034Special\035Dir/''" \
+   "'$'New\034Special\035Dir/New\036Special\037File''" &&
+   test_completion "git test-path-comp 
'$'New\034Special\035Dir/New\036S''" \
+   "'$'New\034Special\035Dir/New\036Special\037File''"
+'
+
+
 test_expect_success "completion uses  completion for alias: !sh -c 'git 
 ...'" '
test_config alias.co "!sh -c '"'"'git checkout ...'"'"'" &&
test_completion "git co m" <<-\EOF
-- 
2.17.0.366.gbe216a3084

[PATCH 05/11] completion: improve handling quoted paths on the command line

[SZEDER Gábor]

Our git-aware path completion doesn't work when it has to complete a
word already containing quoted and/or backslash-escaped characters on
the command line.  The root cause of the issue is that completion
functions see all words on the command line verbatim, i.e. including
all backslash, single and double quote characters that the shell would
eventually remove when executing the finished command.  These
quoting/escaping characters cause different issues depending on which
path component of the word to be completed contains them:

  - The quoting/escaping is in the prefix path component(s).

Let's suppose we have a directory called 'New Dir', containing two
untracked files 'file.c' and 'file.o', and we have a gitignore
rule ignoring object files.  In this case all of these:

  git add New\ Dir/
  git add "New Dir/
  git add 'New Dir/

should uniquely complete 'file.c' right away, but Bash offers both
'file.c' and 'file.o' instead.  The reason for this behavior is
that our completion script uses the prefix directory name like
'git -C "New\ Dir/" ls-files ...", i.e. with the backslash inside
double quotes.  Git then tries to enter a directory called
'New\ Dir', which (most likely) fails because such a directory
doesn't exists.  As a result our completion script doesn't list
any files, leaves the COMPREPLY array empty, which in turn causes
Bash to fall back to its simple filename completion and lists all
files in that directory, i.e. both 'file.c' and 'file.o'.

  - The quoting/escaping is in the path component to be completed.

Let's suppose we have two untracked files 'New File.c' and
'New File.o', and we have a gitignore rule ignoring object files.
In this case all of these:

  git add New\ Fi
  git add "New Fi
  git add 'New Fi

should uniquely complete 'New File.c' right away, but Bash offers
both 'New File.c' and 'New File.o' instead.  The reason for this
behavior is that our completion script uses this 'New\ Fi' or
'"New Fi' etc. word to filter matching paths, and of course none
of the potential filenames will match because of the included
backslash or double quote.  The end result is the same as above:
the completion script doesn't list any files, Bash falls back to
its filename completion, which then lists the matching object file
as well.

Add the new helper function __git_dequote() [1], which removes (most
of[2]) the quoting and escaping from the word it gets as argument.  To
minimize the overhead of calling this function, store its result in
the variable $dequoted_word, supposed to be declared local in the
caller; simply printing the result would require a command
substitution imposing the overhead of fork()ing a subshell.  Use this
function in __git_complete_index_file() to dequote the current word,
i.e. the path, to be completed, to avoid the above described
quoting-related issues, thereby fixing two of the failing quoted path
completion tests.

[1] The bash-completion project already has a dequote() function,
which I hoped I could borrow to deal with this, but unfortunately
it doesn't work quite well for this purpose (perhaps that's why
even the bash-completion project only rarely uses it).  The main
issue is that their dequote() is implemented as:

  eval printf %s "$1" 2> /dev/null

where $1 would contain the word to be completed.  While it's a
short and sweet one-liner, the use of 'eval' requires that $1 is a
syntactically valid string, which is not the case when quoting the
path like 'git add "New Dir/'.  This causes 'eval' to fail,
because it can't find the matching closing double quote, and the
function returns nothing.  The result is totally broken behavior,
as if the current word were empty, and the completion script would
then list all files from the current directory.  This is why one
of the quoted path completion tests specifically checks the
completion of a path with an opening but without a corresponding
closing double quote character.  Furthermore, the 'eval' performs
all kinds of expansions, which may or may not be desired; I think
it's the latter.  Finally, using this function would require a
command substitution.

[2] Bash understands the $'string' quoting as well, which "expands to
'string', with backslash-escaped characters replaced as specified
by the ANSI C standard" (quoted from Bash manpage).  Since shell
metacharacters, field separators, globbing, etc. can all be easily
entered using standard shell escaping or quoting, this type of
quoting comes in handly when dealing with control characters that
are otherwise difficult both to "type" and to see on the command
line.  Because of this difficulty I would assume that people do
avoid pathnames with such control characters anyway, so I didn't
bother implementing it.  This function is already way 

[PATCH 02/11] completion: move __git_complete_index_file() next to its helpers

[SZEDER Gábor]

It's much easier to read, understand and modify the functions related
to git-aware path completion when they are right next to each other.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 39 +-
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index b09c8a2362..36d3c6f928 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -396,6 +396,25 @@ __git_index_files ()
done | sort | uniq
 }
 
+# __git_complete_index_file requires 1 argument:
+# 1: the options to pass to ls-file
+#
+# The exception is --committable, which finds the files appropriate commit.
+__git_complete_index_file ()
+{
+   local pfx="" cur_="$cur"
+
+   case "$cur_" in
+   ?*/*)
+   pfx="${cur_%/*}"
+   cur_="${cur_##*/}"
+   pfx="${pfx}/"
+   ;;
+   esac
+
+   __gitcomp_file "$(__git_index_files "$1" ${pfx:+"$pfx"})" "$pfx" "$cur_"
+}
+
 # Lists branches from the local repository.
 # 1: A prefix to be added to each listed branch (optional).
 # 2: List only branches matching this word (optional; list all branches if
@@ -712,26 +731,6 @@ __git_complete_revlist_file ()
esac
 }
 
-
-# __git_complete_index_file requires 1 argument:
-# 1: the options to pass to ls-file
-#
-# The exception is --committable, which finds the files appropriate commit.
-__git_complete_index_file ()
-{
-   local pfx="" cur_="$cur"
-
-   case "$cur_" in
-   ?*/*)
-   pfx="${cur_%/*}"
-   cur_="${cur_##*/}"
-   pfx="${pfx}/"
-   ;;
-   esac
-
-   __gitcomp_file "$(__git_index_files "$1" ${pfx:+"$pfx"})" "$pfx" "$cur_"
-}
-
 __git_complete_file ()
 {
__git_complete_revlist_file
-- 
2.17.0.366.gbe216a3084

Re: Optimizing writes to unchanged files during merges?

[Junio C Hamano]

Jacob Keller  writes:

> On Mon, Apr 16, 2018 at 10:43 AM, Jacob Keller  wrote:
> ...
> I think a better solution for your problem would be to extend the
> build system you're using to avoid rebuilding when the contents
> haven't changed since last build (possibly by using hashes?). At the
> very least, I would not want this to be default, as it could possibly
> result in *no* build when there should be one, which is far more
> confusing to debug.

Yup.

Even though I take many optional features that I do not see need to
support on the core side and I do not plan to use personally, as
long as the implementation is cleanly separated to make it clear
that those who do not use them won't negatively affected, I would
not want to have this in a system I maintain and get blamed by those
who get burned by it.  Something like ccache, not a version control
system, is meant to serve this audience.

Re: Draft of Git Rev News edition 38

[Christian Couder]

On Mon, Apr 16, 2018 at 5:19 PM, Sergey Organov  wrote:
> Kaartic Sivaraam  writes:
>
>> 1. I see the following sentence in the "Rebasing merges: a jorney to the
>> ultimate solution (Road Clear) (written by Jacob Keller)" article
>>
>>   "A few examples were tried, but it was proven that the original
>>   concept did not work, as dropped commits could end up being
>>   replaid into the merge commits, turning them into "evil"
>>   merges."
>>
>> I'm not sure if 'replaid' is proper English assuming the past tense of
>> replay was intended there (which I think is 'replayed').
>
> It could have meant, say, "reapplied", -- we need to ask the author.

Yeah it could but I would say that it is not very likely compared to
"replayed", so I changed it to "replayed". And yeah I can change it to
something else if Jake (who is Cc'ed) prefers.

> While we are at it, please also consider to replace "original concept"
> by "original algorithm", as it didn't work due to a mistake in the
> algorithm as opposed to failure of the concept itself.

Ok, it's now "original algorithm".

Thanks,
Christian.

Re: Draft of Git Rev News edition 38

[Christian Couder]

Hi,

On Mon, Apr 16, 2018 at 5:07 PM, Kaartic Sivaraam
 wrote:
>
> That said, I read the draft and found it good except for two minor issues,

Thanks for your comments!

> 1. I see the following sentence in the "Rebasing merges: a jorney to the
> ultimate solution (Road Clear) (written by Jacob Keller)" article
>
> "A few examples were tried, but it was proven that the original
> concept did not work, as dropped commits could end up being
> replaid into the merge commits, turning them into "evil"
> merges."
>
> I'm not sure if 'replaid' is proper English assuming the past tense of
> replay was intended there (which I think is 'replayed').

I agree and changed it to "replayed".

> 2. I see a minor Markdown syntax issue in the "branch -l: print useful
> info whilst rebasing a non-local branch" article.
>
> ... reworked his original patch to improve `git branch
> --list̀
>
> Specifically, in the '--list̀' part. I guess it should be "--list`".

Yeah, it's fixed too.

Thanks,
Christian.

[PATCH] worktree: accept -f as short for --force for removal

[Stefan Beller]

The force flag is very common in many commands and is commonly
abbreviated with '-f', so add that to worktree removal, too

Signed-off-by: Stefan Beller 
---

As a side note:

 $ git worktree add ../test HEAD^
 $ cd ../test
 $ make
 $ git worktree remove test
 fatal: working trees containing submodules cannot be moved or removed
 
 (This is on git.git, which does have submodules, but they should not 
 be relevant here? Instead we rather want to point out files left over.
 Not sure. I also plan on having worktrees and submodules to work well
 together eventually)


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

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 40a438ed6c..d734874d58 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -783,7 +783,7 @@ static int remove_worktree(int ac, const char **av, const 
char *prefix)
 {
int force = 0;
struct option options[] = {
-   OPT_BOOL(0, "force", ,
+   OPT_BOOL('f', "force", ,
 N_("force removing even if the worktree is dirty")),
OPT_END()
};
-- 
2.17.0.484.g0c8726318c-goog

Proposal

[MS Zeliha Omer Faruk]

Hello

Greeetings to you please did you get my previous email regarding my
investment proposal last week friday ?

MS.Zeliha ömer faruk
zeliha.omer.fa...@gmail.com

[PATCH 2/2] builtin/blame: highlight recently changed lines

[Stefan Beller]

Choose a different color for dates and imitate a 'temperature cool down'
depending upon age.

Originally I had planned to have the temperature cooldown dependent on
the age of the project or file for example, as that might scale better,
but that can be added on top of this commit, e.g. instead of giving a
date, you could imagine giving a percentage that would be the linearly
interpolated between now and the beginning of the file.

Signed-off-by: Stefan Beller 
---
 Documentation/config.txt | 18 ++
 builtin/blame.c  | 78 +++-
 t/t8012-blame-colors.sh  | 25 +
 3 files changed, 120 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a223232263..acc456e1fd 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1223,6 +1223,24 @@ color.blame.repeatedMeta::
is repeated meta information per line (such as commit id,
author name, date and timezone). Defaults to dark gray.
 
+color.blame.highlightRecent::
+   This can be used to color the author and date of a blame line.
+   This overrides `color.blame.repeatedMeta` setting, which colors
+   repetitions.
++
+This setting should be set to a comma-separated list of color and date 
settings,
+starting and ending with a color, the dates should be set from oldest to 
newest.
+The metadata will be colored given the colors if the the line was introduced
+before the given timestamp, overwriting older timestamped colors.
++
+Instead of an absolute timestamp relative timestamps work as well, e.g.
+2.weeks.ago is valid to address anything older than 2 weeks.
++
+It defaults to "blue,12 month ago,white,1 month ago,red", which colors
+everything older than one year blue, recent changes between one month and
+one year old are kept white, and lines introduced within the last month are
+colored red.
+
 color.ui::
This variable determines the default value for variables such
as `color.diff` and `color.grep` that control the use of color
diff --git a/builtin/blame.c b/builtin/blame.c
index b49fee70a9..9b1021f10d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -24,6 +24,7 @@
 #include "dir.h"
 #include "progress.h"
 #include "blame.h"
+#include "string-list.h"
 
 static char blame_usage[] = N_("git blame [] [] [] 
[--] ");
 
@@ -323,6 +324,7 @@ static const char *format_time(timestamp_t time, const char 
*tz_str,
 #define OUTPUT_SHOW_EMAIL  0400
 #define OUTPUT_LINE_PORCELAIN  01000
 #define OUTPUT_COLOR_LINE  02000
+#define OUTPUT_HEATED_LINES04000
 
 static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
 {
@@ -370,6 +372,63 @@ static void emit_porcelain(struct blame_scoreboard *sb, 
struct blame_entry *ent,
putchar('\n');
 }
 
+static struct color_field {
+   timestamp_t hop;
+   char col[COLOR_MAXLEN];
+} *colorfield;
+static int colorfield_nr, colorfield_alloc;
+
+static void parse_color_fields(const char *s)
+{
+   struct string_list l = STRING_LIST_INIT_DUP;
+   struct string_list_item *item;
+   enum { EXPECT_DATE, EXPECT_COLOR } next = EXPECT_COLOR;
+
+   colorfield_nr = 0;
+
+   /* Ideally this would be stripped and split at the same time? */
+   string_list_split(, s, ',', -1);
+   ALLOC_GROW(colorfield, colorfield_nr + 1, colorfield_alloc);
+
+   for_each_string_list_item(item, ) {
+   switch (next) {
+   case EXPECT_DATE:
+   colorfield[colorfield_nr].hop = 
approxidate(item->string);
+   next = EXPECT_COLOR;
+   colorfield_nr++;
+   ALLOC_GROW(colorfield, colorfield_nr + 1, 
colorfield_alloc);
+   break;
+   case EXPECT_COLOR:
+   if (color_parse(item->string, 
colorfield[colorfield_nr].col))
+   die(_("expecting a color: %s"), item->string);
+   next = EXPECT_DATE;
+   break;
+   }
+   }
+
+   if (next == EXPECT_COLOR)
+   die (_("must end with a color"));
+
+   colorfield[colorfield_nr].hop = TIME_MAX;
+}
+
+static void setup_default_colors_heated_lines(void)
+{
+   parse_color_fields("blue,12 month ago,white,1 month ago,red");
+}
+
+static void determine_line_heat(struct blame_entry *ent, const char 
**dest_color)
+{
+   int i = 0;
+   struct commit_info ci;
+   get_commit_info(ent->suspect->commit, , 1);
+
+   while (i < colorfield_nr && ci.author_time > colorfield[i].hop)
+   i++;
+
+   *dest_color = colorfield[i].col;
+}
+
 static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, 
int opt)
 {
int cnt;
@@ -384,6 +443,12 @@ static void emit_other(struct blame_scoreboard *sb, struct 
blame_entry *ent, int
oid_to_hex_r(hex, >commit->object.oid);
 

[PATCH 1/2] builtin/blame: dim uninteresting metadata lines

[Stefan Beller]

When using git-blame lots of lines contain redundant information, for
example in hunks that consist of multiple lines, the metadata (commit
name, author, date) are repeated. A reader may not be interested in those,
so offer an option to color the information that is repeated from the
previous line differently. Both the command line option '--color-lines'
as well as a config option 'color.blame.colorLines' is provided.

Signed-off-by: Stefan Beller 
---
 Documentation/config.txt |  5 +
 builtin/blame.c  | 42 
 color.h  |  1 +
 t/t8012-blame-colors.sh  | 29 +++
 4 files changed, 73 insertions(+), 4 deletions(-)
 create mode 100755 t/t8012-blame-colors.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2659153cb3..a223232263 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1218,6 +1218,11 @@ color.status.::
status short-format), or
`unmerged` (files which have unmerged changes).
 
+color.blame.repeatedMeta::
+   Use the customized color for the part of git-blame output that
+   is repeated meta information per line (such as commit id,
+   author name, date and timezone). Defaults to dark gray.
+
 color.ui::
This variable determines the default value for variables such
as `color.diff` and `color.grep` that control the use of color
diff --git a/builtin/blame.c b/builtin/blame.c
index db38c0b307..b49fee70a9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -7,6 +7,7 @@
 
 #include "cache.h"
 #include "config.h"
+#include "color.h"
 #include "builtin.h"
 #include "commit.h"
 #include "diff.h"
@@ -46,6 +47,7 @@ static int xdl_opts;
 static int abbrev = -1;
 static int no_whole_file_rename;
 static int show_progress;
+static char repeated_meta_color[COLOR_MAXLEN];
 
 static struct date_mode blame_date_mode = { DATE_ISO8601 };
 static size_t blame_date_width;
@@ -316,10 +318,11 @@ static const char *format_time(timestamp_t time, const 
char *tz_str,
 #define OUTPUT_PORCELAIN   010
 #define OUTPUT_SHOW_NAME   020
 #define OUTPUT_SHOW_NUMBER 040
-#define OUTPUT_SHOW_SCORE  0100
-#define OUTPUT_NO_AUTHOR   0200
+#define OUTPUT_SHOW_SCORE  0100
+#define OUTPUT_NO_AUTHOR   0200
 #define OUTPUT_SHOW_EMAIL  0400
-#define OUTPUT_LINE_PORCELAIN 01000
+#define OUTPUT_LINE_PORCELAIN  01000
+#define OUTPUT_COLOR_LINE  02000
 
 static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
 {
@@ -375,6 +378,7 @@ static void emit_other(struct blame_scoreboard *sb, struct 
blame_entry *ent, int
struct commit_info ci;
char hex[GIT_MAX_HEXSZ + 1];
int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
+   const char *color = "", *reset = "";
 
get_commit_info(suspect->commit, , 1);
oid_to_hex_r(hex, >commit->object.oid);
@@ -384,6 +388,19 @@ static void emit_other(struct blame_scoreboard *sb, struct 
blame_entry *ent, int
char ch;
int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : 
abbrev;
 
+   if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {
+   if (opt & OUTPUT_COLOR_LINE) {
+   if (cnt > 0) {
+   color = repeated_meta_color;
+   reset = GIT_COLOR_RESET;
+   } else  {
+   color ="";
+   reset = "";
+   }
+   }
+   fputs(color, stdout);
+   }
+
if (suspect->commit->object.flags & UNINTERESTING) {
if (blank_boundary)
memset(hex, ' ', length);
@@ -433,6 +450,9 @@ static void emit_other(struct blame_scoreboard *sb, struct 
blame_entry *ent, int
printf(" %*d) ",
   max_digits, ent->lno + 1 + cnt);
}
+   if (!(opt & OUTPUT_ANNOTATE_COMPAT) &&
+   (opt & OUTPUT_COLOR_LINE))
+   fputs(reset, stdout);
do {
ch = *cp++;
putchar(ch);
@@ -585,6 +605,8 @@ static const char *add_prefix(const char *prefix, const 
char *path)
 
 static int git_blame_config(const char *var, const char *value, void *cb)
 {
+   int *output_option = cb;
+
if (!strcmp(var, "blame.showroot")) {
show_root = git_config_bool(var, value);
return 0;
@@ -607,6 +629,13 @@ static int git_blame_config(const char *var, const char 
*value, void *cb)
parse_date_format(value, _date_mode);
return 0;
}
+   if (!strcmp(var, "color.blame.repeatedlines")) {
+   if (color_parse_mem(value, strlen(value), 

[PATCH 0/2] blame: color line by commit

[Stefan Beller]

This is a resend of sb/blame-color, dropping the patch
"add option to color metadata fields separately" as I think it is not needed.

> Expecting a reroll.
> cf. https://public-inbox.org/git/20171110011002.10179-1-sbel...@google.com/#t
> error messages are funny, can segfault, ...

The error messages have been addressed. However a default mode that is enabled
by color.ui has not been added yet.

Thanks,
Stefan

Stefan Beller (2):
  builtin/blame: dim uninteresting metadata lines
  builtin/blame: highlight recently changed lines

 Documentation/config.txt |  23 
 builtin/blame.c  | 118 +--
 color.h  |   1 +
 t/t8012-blame-colors.sh  |  54 ++
 4 files changed, 192 insertions(+), 4 deletions(-)
 create mode 100755 t/t8012-blame-colors.sh

-- 
2.17.0.484.g0c8726318c-goog

Re: [RFC 01/10] submodule: add 'core.submodulesFile' to override the '.gitmodules' path

[Stefan Beller]

On Mon, Apr 16, 2018 at 9:37 AM, Antonio Ospite  wrote:
> On Thu, 12 Apr 2018 16:50:03 -0700
> Stefan Beller  wrote:
>
>> Hi Antonio,
>>
>> On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite  wrote:
>> > When multiple repositories with detached work-trees take turns using the
>> > same directory as their work-tree, and more than one of them want to use
>> > submodules, there will be conflicts about the '.gitmodules' file.
>>
>> unlike other files which would not conflict?
>> There might be file names such as LICENSE, Readme.md etc,
>> which are common enough that they would produce conflicts as well?
>> I find this argument on its own rather weak. ("Just delete everything in
>> the working dir before using it with another repository"). I might be
>> missing a crucial bit here?
>>
>
> All the vcsh repositories _share_ the same work-tree; they may control
> it taking turns but, in general, all files are meant to be checked out
> at all times as the basic use case is: *distinct* sets of config files.
>
> Maybe saying that the repositories "take turns" is confusing.
> It's an unnecessary information, so I will omit that part form the
> commit message.

So they all have the same workdir, do they track the same set of files
or do they track a disjoint set of files, and ignoring the other repositories
files via the ignore mechanism?

This sounds like an interesting setup. I never though of that as something
useful (in either configuration).

> After your question I've done some research and I've seen other vcsh
> users managing conflicting LICENSE and README files using git
> sparse-checkouts, to have these files in the single repositories but
> not checked out in the shared work-tree:
> https://github.com/RichiH/vcsh/issues/120#issuecomment-42639619
> https://github.com/jwhitley/vcsh-root/commit/30b0d495c2cbe47ae9617ace9c2c14720d961d78
>
> However I guess that my point here is that the gitmodules file is
> something that influences git behavior so it should not be on the user's
> shoulder to manage conflicts for it, and most importantly it needs to
> be checked out for git to access it, doesn't it?

Good point! I wonder if the cleaner solution would be to just
tell git to use HEAD:.gitmodules and not check out the file?
then you would not need to come up with a namespace for names
of the .gitmodules files and scatter them into the worktree as well?


>> > -   value=$(git config -f .gitmodules 
>> > submodule."$name"."$option")
>> > +   gitmodules_file=$(git config core.submodulesfile)
>> > +   : ${gitmodules_file:=.gitmodules}
>> > +   value=$(git config -f "$gitmodules_file" 
>> > submodule."$name"."$option")
>>
>> I wonder if it would be cheaper to write a special config lookup now, e.g.
>> in builtin/submodule--helper.c we could have a "config-from-gitmodules"
>> subcommand that is looking up the modules file and then running the config
>> on that file.
>>
>
> Can you give an example from the user point of view of such a
> "config-from-gitmodules" command?
>

git submodule config  

as an 'alias' for

   gitmodules_file=$(git config core.submodulesfile)
   : ${gitmodules_file:=.gitmodules}
   value=$(git config -f "$gitmodules_file"
submodule."$name"."$option")

The helper would figure out which config file to load form
(.gitmodules in tree, HEAD:.gitmodules, your new proposed gitmodules file,
.git/config... or the special ref) and then return the  for 

So maybe:

$ git clone https://gerrit.googlesource.com/gerrit && cd gerrit
# ^ My goto-repo with submodules

$ git submodule config "plugins/hooks" URL
../plugins/hooks



> I might look into it, but that can also be a followup change.


>> > diff --git a/submodule.c b/submodule.c
>> > index 9a50168b2..2afbdb644 100644
>> > --- a/submodule.c
>> > +++ b/submodule.c
>> > @@ -36,13 +36,13 @@ static struct oid_array ref_tips_after_fetch;
>> >   */
>> >  int is_gitmodules_unmerged(const struct index_state *istate)
>> >  {
>> > -   int pos = index_name_pos(istate, GITMODULES_FILE, 
>> > strlen(GITMODULES_FILE));
>> > +   int pos = index_name_pos(istate, submodules_file, 
>> > strlen(submodules_file));
>>
>> Ah, regarding the coverletter: This clearly assumes the modules
>> file is in the tree. So at least here we would make an exception
>> for files outside the tree to either not check for un-merged-ness or
>> disallow that case entirely.
>>
>
> Sorry I am not sure I follow what you are saying here, keep in mind
> that I am new to git internals.
>
> Do you mean that, even if we ensure (in
> config.c::git_default_core_config) that only paths relative to
> the work-tree are allowed, we still have to check here that the
> constraint is respected? And is so, why?

index_name_pos looks up a position of a file in the index,
which would fail for any file not in the index.

So if we give a path outside the tree, the 

Re: [PATCH v9 29.75/30] merge-recursive: Fix was_tracked() to quit lying with some renamed paths

[Elijah Newren]

On Sun, Apr 15, 2018 at 5:37 PM, Junio C Hamano  wrote:
> Elijah Newren  writes:
>
>> @@ -362,13 +363,17 @@ static int git_merge_trees(struct merge_options *o,
>>   init_tree_desc_from_tree(t+2, merge);
>>
>>   rc = unpack_trees(3, t, >unpack_opts);
>> + cache_tree_free(_cache_tree);
>> +
>> + o->orig_index = the_index;
>> + the_index = tmp_index;
>> +
>>   /*
>> -  * unpack_trees NULLifies src_index, but it's used in verify_uptodate,
>> -  * so set to the new index which will usually have modification
>> -  * timestamp info copied over.
>> +  * src_index is used in verify_uptodate, but was NULLified in
>> +  * unpack_trees, so we need to set it back to the original index.
>>*/
>
> Was NULLified?  I thought that the point of src/dst distinction
> Linus introduced long time ago at 34110cd4 ("Make 'unpack_trees()'
> have a separate source and destination index", 2008-03-06) was that
> we can then keep the source side of the traversal unmodified.

That comment is messed up; maybe I edited and re-edited the comment
multiple times and then didn't notice the big problems when
re-reading?

Anyway, I should move the comment a few lines up, and make the code
instead read:

/*
 * Update the_index to match the new results, AFTER saving a copy
 * in o->orig_index.  Update src_index to point to the saved copy.
 * (verify_uptodate() checks src_index, and the original index is
 * the one that had the necessary modification timestamps.)
 */
o->orig_index = the_index;
the_index = tmp_index;
o->unpack_opts.src_index = >orig_index;

>>  static int would_lose_untracked(const char *path)
>>  {
>> - return !was_tracked(path) && file_exists(path);
>> + /*
>> +  * This may look like it can be simplified to:
>> +  *   return !was_tracked(o, path) && file_exists(path)
>> +  * but it can't.  This function needs to know whether path was
>> +  * in the working tree due to EITHER having been tracked in the
>> +  * index before the merge OR having been put into the working copy
>> +  * and index by unpack_trees().  Due to that either-or requirement,
>> +  * we check the current index instead of the original one.
>> +  */
>
> If this path was created by merge-recursive, not by unpack_trees(),
> what does this function want to say?  Say, we are looking at path P,
> the other branch we are merging moved some other path Q to P (while
> our side modified contents at path Q).  Then path P we are looking
> at has contents of Q at the merge base at stage #1, the contents of
> Q from our HEAD at stage #2 and the contents of P from the other
> branch at stage #3.  The code below says "path P is OK, we won't
> lose it" in such a case, but it is unclear if the above comment
> wants to also cover that case.

Oh, boy, here be dragons...

The comment as-is actually does cover your example case with Q and P:
unpack_trees(), which is unaware of renames, will see that P only
exists on one side of history and thus load it into the index at stage
0 rather than stage 3.

But your general comment about whether something else in
merge-recursive could create a path in the current index after
unpack_trees() is interesting...it touches on a pitfall that has bit
me multiple times.  There is a required ordering in merge-recursive.c
that for any given path, the working directory must be updated before
the index is -- otherwise, would_lose_untracked() will return faulty
information.  update_file_flags() has this ordering builtin,
update_stages() has a big obnoxious comment at the beginning about how
it should not be called until after update_file() is,
apply_directory_rename_modifications() has a big comment about this
~80% of the way through the function (look for
"would_lose_untracked"), and conflict_rename_rename_2to1() has a big
obnoxious comment near the end painstakingly pointing out that some
code that feels like it would make more sense being combined with a
previous function cannot be due to the
update-working-directory-before-index requirement.

I should probably add to this comment something about this annoying
(and error-prone) ordering restriction, since this function is the
source of those particular pains.  Your suggested ideal-world rewrite
(run unpack_trees() with unpack_opts.index_only=1, do merge in memory,
then update working tree), would make this whole problem go away.

Re: [RFC 02/10] submodule: fix getting custom gitmodule file in fetch command

[Antonio Ospite]

On Mon, 16 Apr 2018 12:23:59 -0700
Stefan Beller  wrote:

> On Mon, Apr 16, 2018 at 9:18 AM, Antonio Ospite  wrote:
> 
> >
> > Is there an API to just load one config setting?
> 
> Do you mean to
> 
>   git -c key=value foo-command --options ...

I meant, instead of:

  git_config(git_fetch_config, NULL);

which updates a series of config settings, something which loads
_in_the_code_ just the one value for 'core.submodulesFile' previously
set either via "git -c" or "git config".

It turns out what I was looking for is git_config_get_string_const().

The new patch will be simply this:

diff --git a/config.c b/config.c
index 6ffb1d501..1ef9801d3 100644
--- a/config.c
+++ b/config.c
@@ -2087,6 +2087,7 @@ int git_config_get_pathname(const char *key, const char 
**dest)
  */
 void config_from_gitmodules(config_fn_t fn, void *data)
 {
+   git_config_get_string_const("core.submodulesFile", _file);
if (the_repository->worktree) {
char *file = repo_worktree_path(the_repository, 
submodules_file);
git_config_from_file(fn, file, data);


Which covers my use case without changing the fetch behavior for previous
users, as "core.submodulesFile" is a new option.

Thanks,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: [PATCH] completion: reduce overhead of clearing cached --options

[Matthew Coleman]

Disclaimer: I'm not a zsh user, so please correct anything I might have gotten 
wrong.

I created a .zshrc with the following contents:
autoload -Uz compinit
compinit
source 
/usr/local/lib/python3.6/site-packages/powerline/bindings/zsh/powerline.zsh

zsh doesn't have broken Unicode output in its `set` builtin, so it was not 
affected by the original issue. Applying the patch does not cause any change in 
behavior.

Since the commit only changes a file with "bash" in its name, but the 
conditional references zsh variables, I think it's worth mentioning something 
about it in the commit message.

I think the bash side of things is all set for this commit, but can a similar 
improvement be made (using a builtin instead of parsing set|sed) for zsh? 
Again, I'm not a zsh user, so some input from someone who's written zsh 
completion rules would be very helpful. Can any of the builtins mentioned in 
this zsh documentation be used instead of set|sed?
http://zsh.sourceforge.net/Doc/Release/Zsh-Modules.html#The-zsh_002fcomputil-Module

> On Apr 16, 2018, at 2:23 PM, Jacob Keller  wrote:
> 
> On Sat, Apr 14, 2018 at 6:27 AM, Jakub Narebski  wrote:
>> SZEDER Gábor  writes:
>>> On Fri, Apr 13, 2018 at 11:44 PM, Jakub Narebski  wrote:
 SZEDER Gábor  writes:
> 
> In Bash we can do better: run the 'compgen -v __gitcomp_builtin_'
> builtin command, which lists the same variables, but without a
> pipeline and 'sed' it can do so with lower overhead.
 
 What about ZSH?
>>> 
>>> Nothing, ZSH is unaffected by this patch.
>> 
>> All right, so for ZSH we would need LC_ALL=C trick, or come with some
>> equivalent of 'compgen -v __gitcomp_builtin_' for this shell.
>> 
>> Good patch, though it does not solve whole of the problem.
>> 
>> Best,
>> --
>> Jakub Narębski
> 
> Is ZSH actually affected by the broken set behavior, though?
> 
> Thanks,
> Jake

Re: Optimizing writes to unchanged files during merges?

[Stefan Haller]

Lars Schneider  wrote:

> An engineer works on a task branch and runs incremental builds — all 
> is good. The engineer switches to another branch to review another 
> engineer's work. This other branch changes a low-level header file, 
> but no rebuild is triggered. The engineer switches back to the previous
> task branch. At this point, the incremental build will rebuild 
> everything, as the compiler thinks that the low-level header file has
> been changed (because the mtime is different).

If you are using C or C++, look into ccache. We're using it to work
around this problem, and it's a near-perfect solution, I'd say.

shortstat / stat / numstat for git log

[Alexander Mills]

@git

I have this question:

https://stackoverflow.com/questions/49863038/get-numstat-or-stat-for-pretty-format

I am looking to generage JSON with the pretty format, but apparent

* stat
* shortstat
* numstat

etc

cannot be interpolated.

can this be updated / improved?

-alex


-- 
Alexander D. Mills
! New cell phone number: (415)766-8243
alexander.d.mi...@gmail.com

www.linkedin.com/pub/alexander-mills/b/7a5/418/

Git fast-export with import marks file omits merge commits

[Isaac Chou]

Hello,

I came across a change of behavior with Git version 2.15 and later where the 
fast-export command would omit the merge commits.  The same use case works 
correctly with Git version 2.14 and older.  Here is the detail of the use case:

0> git --version
git version 2.16.2.windows.1

1> git init
Initialized empty Git repository in c:/./.git/

2> echo  >> file.txt

3> git add file.txt

4> git commit -m "first commit"
[master (root-commit) 711d4d5] first commit
1 file changed, 1 insertion(+)
create mode 100644 file.txt

5> git checkout -b test
Switched to a new branch 'test'

6> echo  >> file.txt

7> git add file.txt

8> git commit -m "commit on test branch"
[test 76d231c] commit on test branch
1 file changed, 1 insertion(+)

9> git checkout master
Switched to branch 'master'

10> echo  >> file.txt

11> git add file.txt

12> git commit -m "commit on master branch"
[master 61c55fd] commit on master branch
1 file changed, 1 insertion(+)

13> git merge test
Auto-merging file.txt
CONFLICT (content): Merge conflict in file.txt
Automatic merge failed; fix conflicts and then commit the result.

14> notepad file.txt

15> git add file.txt

16> git commit -m "merged with test branch"
[master 1442e0e] merged with test branch

17> git log
commit 1442e0ee728c831e74550329e39d27d4188b4422 (HEAD -> master)
Merge: 61c55fd 76d231c
Author: isaac <...>
Date:   Mon Apr 16 15:08:39 2018 -0400

    merged with test branch

commit 61c55fdb883fc403e63c91b49bc11bdade62b3e8
Author: isaac <...>
Date:   Mon Apr 16 15:07:41 2018 -0400

    commit on master branch

commit 76d231cdb12eb84f45abdebede06a56f912613d3 (test)
Author: isaac <...>
Date:   Mon Apr 16 15:07:07 2018 -0400

    commit on test branch

commit 711d4d5781df41924421f8629af040e7c91a8d2e
Author: isaac <...>
Date:   Mon Apr 16 15:06:07 2018 -0400

    first commit

18> echo :1 711d4d5781df41924421f8629af040e7c91a8d2e > git-marks

19> cat git-marks
:1 711d4d5781df41924421f8629af040e7c91a8d2e

20> git fast-export --use-done-feature --import-marks=git-marks 
refs/heads/master --
feature done
blob
mark :2
data 12



commit refs/heads/master
mark :3
author isaac <...> 1523905627 -0400
committer isaac <...> 1523905627 -0400
data 22
commit on test branch
from :1
M 100644 :2 file.txt

blob
mark :4
data 12



commit refs/heads/master
mark :5
author isaac <...> 1523905661 -0400
committer isaac <...> 1523905661 -0400
data 24
commit on master branch
from :1
M 100644 :4 file.txt

done

Thanks,

Isaac

Attn: Sir/Madam,

[Aziz Dake]

Attn: Sir/Madam,

I am Mr. Aziz Dake,  a Minister confide on me to look for foreign
partner who will assist him to invest the sum of  Thirty  Million
Dollars  ($30,000,000) in your country.

He has investment interest in mining, exotic properties for commercial
resident, development properties, hotels and any other viable
investment opportunities in your country based on your recommendation
will be highly welcomed.

Hence your co -operation is highly needed to actualize this investment project.

Sincerely yours

Mr Aziz Dake.

Re: [RFC 02/10] submodule: fix getting custom gitmodule file in fetch command

[Stefan Beller]

On Mon, Apr 16, 2018 at 9:18 AM, Antonio Ospite  wrote:

>
> Is there an API to just load one config setting?

Do you mean to

  git -c key=value foo-command --options ...

Re: [RFC 00/10] Make .the gitmodules file path configurable

[Stefan Beller]

Hi Antonio,

> I acknowledge that the two mechanisms are different, in particular
> because git *writes* the gitmodules file itself.
>
> I am not opposed to change the name, something like
> 'core.submodulesConfigFile' might highlight that the syntax of the file
> is the one of git config, but I think it's too long.

I was not speculating on a better name, but on the nature of the
configuration, gitignore is additive and errors are easy to see, but in
gitmodules, there is only one "correct" path+name, so the problem
space is not additive, rather we can have a discussion where we get
the correct config from with low odds of errors.

>> > The new configuration setting can be used to set an *alternative*
>> > location for the gitmodules file; IMHO there is no need to provide
>> > *additional* locations like in the case of exclude files.
>>
>> I think there *may* be a need for additional files.
>> Currently there is only the .gitmodules file and the configuration
>> in .git/config overriding settings from .gitmodules.
>>
>> There was some discussion on the mailing list in the past, which
>> presented a intermediate layer in between these two places, in
>> a special ref, such that:
>> base is in .gitmodules
>> overwritten via refs/meta/submodules:.gitmodules
>> overwritten via the .git/config
>>
>> The intermediate would be a config file that is tracked on another
>> ref. This (a) decouples main project history from submodule history
>> and (b) makes it easier to distribute as it is part of the repository.
>>
>> For example (a) is desired if you dig up an old project and the
>> submodules have all moved from one git hosting provider to another.
>> Another example would be when you fork a project with submodules
>> and don't want to mess with the main history but you just want to
>> adjust the submodule URLs. That is possible with such an intermediate
>> additional place.
>>
>> For (b) you can imagine the fork that you want to distribute in your
>> community and you don't want to tell everyone to change the
>> submodule URLs, but instead you can provide them with a prepared
>> .gitmodules file, that they have to place into that special ref (which
>> can be done via fetching).
>>
>> I digress as these ideas seem to be orthogonal to your patch series,
>> just FYI. prior discussion starting at:
>> https://public-inbox.org/git/1462317985-640-1-git-send-email-sbel...@google.com/
>> I recall there was a better discussion even prior to that, but have no
>> link handy.
>>
>
> Just to understand, is that 'refs/meta/submodules:.gitmodules' file
> meant to be managed manually? Or do you imagine some options to
> instruct "git submodule" to *write* to that file instead of .gitmodules?

I imagined it to be managed manually as it would enable some
workflows for downstream users of superproject repos.

But I'd think a convenient way to write to this location would be
super useful, so we ought to have that eventually.

> In the latter case your idea could cover my use case too, couldn't it?

I would think so, yes.

> But most importantly, is this realistically going to be added?

I plan on adding it eventually. It depends on the priorities and
schedule, no promises, though.

> Currently I am not that familiar with the git code base to do it
> myself, and I've never explicitly dealt with special refs in git.

I think core git only uses them for "actual refs", e.g. remote
tracking branches are used to know about the sha1. This new
special ref would be used to store content outside the main tree,
so we'd have too lookup the blob in that not-checked-out commit
and read and write there.

> The approach of this patch series is a lot simpler, and something I can
> work on in my spare time.
> Presumably (b) could even be partially supported with my changes, by
> having both '.gitmodules' and some custom '.gitmodules-alternative'
> files in the repository and tell users to clone with:
>
>   git clone --config core.submodulesFile=.gitmodules-alternative URL
>
> Not as clean as your idea but doable.

Traditionally Git had a strong stance on not transporting config
in the repo (except submodule URLs :/) itself, so I guess
this .gitmodules-alternative would not be a file in the tree, but
a URL or something?

> [...]
>> > Since the gitmodules file is meant to be checked in into the repository,
>> > the overridden file path should be relative to the work-tree; is there
>> > a way to enforce this constraint at run time (i.e. validate the config
>> > value), or is it enough to have it documented?
>>
>> I think we'd want to check at run time, if we need this constraint.
>>
>
> I'll look into it once we decide which the way to go.
>
> Thank you.
>

Thanks for bringing up this easier approach.
Stefan

Re: man page for "git remote set-url" seems confusing/contradictory

[Jacob Keller]

On Mon, Apr 16, 2018 at 9:19 AM, Robert P. J. Day  wrote:
> On Mon, 16 Apr 2018, Andreas Schwab wrote:
>
>> On Apr 16 2018, "Robert P. J. Day"  wrote:
>>
>> > i don't understand how you can clearly set the fetch and push URLs
>> > of a remote independently, while the man page nonetheless insists
>> > that "even though they can be set differently, must still refer to
>> > the same place". how can they be set differently yet still must
>> > refer to the same place?
>>
>> They could be using different transport methods.  For example, for
>> fetching the unauthenticated git: method could be used, but for
>> pushing an authenticated method like ssh: is usually needed.
>
>   ok, point taken, but does that mean the two must actually refer to
> the same "place"? it seems that i'm perfectly free to use this command
> to set the push and fetch URLs to totally different locations.
>
> rday

Things won't work so well if you set the push url and fetch url to
different repositories. Git assumes that refs updated by "push" will
also be reflected via "fetch".

I don't know offhand what will break, but likely something will. For
one, when you fetch again it will rewind your remotes after the push.

The URLs could be different for lots of reasons, but they need to
ultimately link to the same remote repository. As Andreas said, the
most likely reason is differing transport protocols.

Thanks,
Jake

Re: [PATCH] completion: reduce overhead of clearing cached --options

[Jacob Keller]

On Sat, Apr 14, 2018 at 6:27 AM, Jakub Narebski  wrote:
> SZEDER Gábor  writes:
>> On Fri, Apr 13, 2018 at 11:44 PM, Jakub Narebski  wrote:
>>> SZEDER Gábor  writes:

 In Bash we can do better: run the 'compgen -v __gitcomp_builtin_'
 builtin command, which lists the same variables, but without a
 pipeline and 'sed' it can do so with lower overhead.
>>>
>>> What about ZSH?
>>
>> Nothing, ZSH is unaffected by this patch.
>
> All right, so for ZSH we would need LC_ALL=C trick, or come with some
> equivalent of 'compgen -v __gitcomp_builtin_' for this shell.
>
> Good patch, though it does not solve whole of the problem.
>
> Best,
> --
> Jakub Narębski

Is ZSH actually affected by the broken set behavior, though?

Thanks,
Jake

Re: "proper" way to deactivate push for a remote?

[Jacob Keller]

On Mon, Apr 16, 2018 at 7:03 AM, Robert P. J. Day  wrote:
>
>  another feature i've seen for the very first time ... working with
> kubernetes so i checked it out of github, and part of the instructions
> for that is to make sure you don't accidentally try to push back to
> the github remote, so the directions suggest:
>
> $ git remote add upstream https://github.com/kubernetes/kubernetes.git
> $ git remote set-url --push upstream no_push
>
>   fair enough, i just assumed the word "no_push" was some magical
> keyword in that context, but as i read it, all you need to do is put
> *some* invalid URL value there, is that correct?
>
>   and is that the accepted way to do that? what about just deleting
> that line from .git/config? is that valid, or is there a different
> recommendation for doing that? thanks.
>
> rday

I've done this in the past by using a push hook which just reports an
error. That's how I'd do it.

I think if you also set the push url to the empty string it would also work.

Thanks,
Jake

Re: Optimizing writes to unchanged files during merges?

[Phillip Wood]

On 16/04/18 17:07, Lars Schneider wrote:
> 
> I am happy to see this discussion and the patches, because long rebuilds 
> are a constant annoyance for us. We might have been bitten by the exact 
> case discussed here, but more often, we have a slightly different 
> situation:
> 
> An engineer works on a task branch and runs incremental builds — all 
> is good. The engineer switches to another branch to review another 
> engineer's work. This other branch changes a low-level header file, 
> but no rebuild is triggered. The engineer switches back to the previous 
> task branch. At this point, the incremental build will rebuild 
> everything, as the compiler thinks that the low-level header file has
> been changed (because the mtime is different).
> 
> Of course, this problem can be solved with a separate worktree. However, 
> our engineers forget about that sometimes, and then, they are annoyed by 
> a 4h rebuild.
> 
> Is this situation a problem for others too?
> If yes, what do you think about the following approach:
> 
> What if Git kept a LRU list that contains file path, content hash, and 
> mtime of any file that is removed or modified during a checkout. If a 
> file is checked out later with the exact same path and content hash, 
> then Git could set the mtime to the previous value. This way the 
> compiler would not think that the content has been changed since the 
> last rebuild.

Hi Lars

But if there has been rebuild between the checkouts then you
want the compiler to rebuild. I've been using the script below
recently to save and restore mtimes around running rebase to squash
fixup commits. To avoid restoring the mtimes if there has been a
rebuild since they were stored it takes a list of build sentinels and
stores their mtimes too - if any of those change then it will refuse
to restore the original mtimes of the tracked files (if you give a
path that does exist when the mtimes are stored then it will refuse to
restore the mtimes if that path exists when you run 'git mtimes
restore'). The sentinels can be specified on the commandline when
running 'git mtimes save' or stored in multiple mtimes.sentinal config
keys.

Best Wishes

Phillip

--->8---

#!/usr/bin/perl

# Copyright (C) 2018 Phillip Wood 
#
# git-mtimes.perl
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, see .

use 5.008;
use strict;
use warnings;
use File::Copy (qw(copy));
use File::Spec::Functions (qw(abs2rel catfile file_name_is_absolute rel2abs));
use Storable ();

sub git;

my $GIT_DIR = git(qw(rev-parse --git-dir)) or exit 1;
$GIT_DIR = rel2abs($GIT_DIR);
my $mtimes_path = "$GIT_DIR/mtimes";

sub git {
my @lines;
# in a scalar context slurp removing any trailing $/
# in an array context return a list of lines
{
local $/ = wantarray ? $/ : undef;
local $,=' ';
open my $fh, '-|', 'git', @_ or die "git @_ failed $!";
@lines = <$fh>;
chomp @lines;
unless (close $fh) {
$? == -1 and die "git @_ not found";
my $code = $? >> 8;
$_[0] eq 'config' and $code == 1 or
die "git @_ failed with exit code $code"
}
}
wantarray and return @lines;
@lines and chomp @lines;
return $lines[0];
}

sub ls_files {
# mode, uid, gid, mtime and maybe atime
my @stat_indices = $_[0] ? (2, 4, 5, 9, 8) : (2, 4, 5, 9);
local $_;
local $/ = "\0";
my @files;
for (git(qw(ls-files --stage -z))) {
if (/^[^ ]+ ([^\t]+) 0\t(.*)/) {
my @stat = stat($2);
# store name, hash, mode, uid, gid, mtime and maybe atime
push @files, [ $2, $1, @stat[@stat_indices] ];
}
}
return @files;
}

sub get_config {
local $/ = "\0";
my $get = wantarray ? '--get-all' : '--get';
git(qw(config -z), $get, @_);
}

sub save {
local $_;
my @sentinels = get_config('mtimes.sentinel');
push @sentinels, @ARGV;
@sentinels or die "No sentinels given";
@sentinels = map { [ $_, [ stat $_ ] ] } @sentinels;
my @files = ls_files();
Storable::nstore( [ [ @sentinels ] , [ @files ] ], $mtimes_path) or
die "unable to store mtimes $!";
}

sub match_sentinel_data {
local $_;
my ($old, $new, $trustctime) = @_;
if (!@$old) {
return (@$new) ? undef : 1;
} else {
@$new or return undef;
}
# Skip hardlink count, atime, 

Re: Optimizing writes to unchanged files during merges?

[Jacob Keller]

On Mon, Apr 16, 2018 at 10:43 AM, Jacob Keller  wrote:
> On Mon, Apr 16, 2018 at 9:07 AM, Lars Schneider
>  wrote:
>> What if Git kept a LRU list that contains file path, content hash, and
>> mtime of any file that is removed or modified during a checkout. If a
>> file is checked out later with the exact same path and content hash,
>> then Git could set the mtime to the previous value. This way the
>> compiler would not think that the content has been changed since the
>> last rebuild.
>
> That would only work until they actuall *did* a build on the second
> branch, and upon changing back, how would this detect that it needs to
> update mtime again? I don't think this solution really works.
> Ultimately, the problem is that the build tool relies on the mtime to
> determine what to rebuild. I think this would cause worse problems
> because we *wouldn't* rebuild in the case. How is git supposed to know
> that we rebuilt when switching branches or not?
>
> Thanks,
> Jake

I think a better solution for your problem would be to extend the
build system you're using to avoid rebuilding when the contents
haven't changed since last build (possibly by using hashes?). At the
very least, I would not want this to be default, as it could possibly
result in *no* build when there should be one, which is far more
confusing to debug.

Thanks,
Jake

Re: Optimizing writes to unchanged files during merges?

[Jacob Keller]

On Mon, Apr 16, 2018 at 9:07 AM, Lars Schneider
 wrote:
>
>> On 16 Apr 2018, at 04:03, Linus Torvalds  
>> wrote:
>>
>> On Sun, Apr 15, 2018 at 6:44 PM, Junio C Hamano  wrote:
>>>
>>> I think Elijah's corrected was_tracked() also does not care "has
>>> this been renamed".
>>
>> I'm perfectly happy with the slightly smarter patches. My patch was
>> really just an RFC and because I had tried it out.
>>
>>> One thing that makes me curious is what happens (and what we want to
>>> happen) when such a "we already have the changes the side branch
>>> tries to bring in" path has local (i.e. not yet in the index)
>>> changes.  For a dirty file that trivially merges (e.g. a path we
>>> modified since our histories forked, while the other side didn't do
>>> anything, has local changes in the working tree), we try hard to
>>> make the merge succeed while keeping the local changes, and we
>>> should be able to do the same in this case, too.
>>
>> I think it might be nice, but probably not really worth it.
>>
>> I find the "you can merge even if some files are dirty" to be really
>> convenient, because I often keep stupid test patches in my tree that I
>> may not even intend to commit, and I then use the same tree for
>> merging.
>>
>> For example, I sometimes end up editing the Makefile for the release
>> version early, but I won't *commit* that until I actually cut the
>> release. But if I pull some branch that has also changed the Makefile,
>> it's not worth any complexity to try to be nice about the dirty state.
>>
>> If it's a file that actually *has* been changed in the branch I'm
>> merging, and I'm more than happy to just stage the patch (or throw it
>> away - I think it's about 50:50 for me).
>>
>> So I don't think it's a big deal, and I'd rather have the merge fail
>> very early with "that file has seen changes in the branch you are
>> merging" than add any real complexity to the merge logic.
>
> I am happy to see this discussion and the patches, because long rebuilds
> are a constant annoyance for us. We might have been bitten by the exact
> case discussed here, but more often, we have a slightly different
> situation:
>
> An engineer works on a task branch and runs incremental builds — all
> is good. The engineer switches to another branch to review another
> engineer's work. This other branch changes a low-level header file,
> but no rebuild is triggered. The engineer switches back to the previous
> task branch. At this point, the incremental build will rebuild
> everything, as the compiler thinks that the low-level header file has
> been changed (because the mtime is different).
>
> Of course, this problem can be solved with a separate worktree. However,
> our engineers forget about that sometimes, and then, they are annoyed by
> a 4h rebuild.
>
> Is this situation a problem for others too?
> If yes, what do you think about the following approach:
>
> What if Git kept a LRU list that contains file path, content hash, and
> mtime of any file that is removed or modified during a checkout. If a
> file is checked out later with the exact same path and content hash,
> then Git could set the mtime to the previous value. This way the
> compiler would not think that the content has been changed since the
> last rebuild.

That would only work until they actuall *did* a build on the second
branch, and upon changing back, how would this detect that it needs to
update mtime again? I don't think this solution really works.
Ultimately, the problem is that the build tool relies on the mtime to
determine what to rebuild. I think this would cause worse problems
because we *wouldn't* rebuild in the case. How is git supposed to know
that we rebuilt when switching branches or not?

Thanks,
Jake

>
> I think that would fix the problem that our engineers run into and also
> the problem that Linus experienced during the merge, wouldn't it?
>
> Thanks,
> Lars

Re: Optimizing writes to unchanged files during merges?

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 16 2018, Lars Schneider wrote:

>> On 16 Apr 2018, at 04:03, Linus Torvalds  
>> wrote:
>>
>> On Sun, Apr 15, 2018 at 6:44 PM, Junio C Hamano  wrote:
>>>
>>> I think Elijah's corrected was_tracked() also does not care "has
>>> this been renamed".
>>
>> I'm perfectly happy with the slightly smarter patches. My patch was
>> really just an RFC and because I had tried it out.
>>
>>> One thing that makes me curious is what happens (and what we want to
>>> happen) when such a "we already have the changes the side branch
>>> tries to bring in" path has local (i.e. not yet in the index)
>>> changes.  For a dirty file that trivially merges (e.g. a path we
>>> modified since our histories forked, while the other side didn't do
>>> anything, has local changes in the working tree), we try hard to
>>> make the merge succeed while keeping the local changes, and we
>>> should be able to do the same in this case, too.
>>
>> I think it might be nice, but probably not really worth it.
>>
>> I find the "you can merge even if some files are dirty" to be really
>> convenient, because I often keep stupid test patches in my tree that I
>> may not even intend to commit, and I then use the same tree for
>> merging.
>>
>> For example, I sometimes end up editing the Makefile for the release
>> version early, but I won't *commit* that until I actually cut the
>> release. But if I pull some branch that has also changed the Makefile,
>> it's not worth any complexity to try to be nice about the dirty state.
>>
>> If it's a file that actually *has* been changed in the branch I'm
>> merging, and I'm more than happy to just stage the patch (or throw it
>> away - I think it's about 50:50 for me).
>>
>> So I don't think it's a big deal, and I'd rather have the merge fail
>> very early with "that file has seen changes in the branch you are
>> merging" than add any real complexity to the merge logic.
>
> I am happy to see this discussion and the patches, because long rebuilds
> are a constant annoyance for us. We might have been bitten by the exact
> case discussed here, but more often, we have a slightly different
> situation:
>
> An engineer works on a task branch and runs incremental builds — all
> is good. The engineer switches to another branch to review another
> engineer's work. This other branch changes a low-level header file,
> but no rebuild is triggered. The engineer switches back to the previous
> task branch. At this point, the incremental build will rebuild
> everything, as the compiler thinks that the low-level header file has
> been changed (because the mtime is different).
>
> Of course, this problem can be solved with a separate worktree. However,
> our engineers forget about that sometimes, and then, they are annoyed by
> a 4h rebuild.
>
> Is this situation a problem for others too?
> If yes, what do you think about the following approach:
>
> What if Git kept a LRU list that contains file path, content hash, and
> mtime of any file that is removed or modified during a checkout. If a
> file is checked out later with the exact same path and content hash,
> then Git could set the mtime to the previous value. This way the
> compiler would not think that the content has been changed since the
> last rebuild.
>
> I think that would fix the problem that our engineers run into and also
> the problem that Linus experienced during the merge, wouldn't it?

Could what you're describing be prototyped as a post-checkout hook that
looks at the reflog? It sounds to me like it could, but perhaps I've
missed some subtlety.

Not re-writing out a file that hasn't changed is one thing, but I think
for more complex behaviors (such as the "I want everything to have the
same mtime" mentioned in another thread on-list), and this, it makes
sense to provide some hook mechanism than have git itself do all the
work.

I also don't see how what you're describing could be generalized, or
even be made to work reliably in the case you're describing. If the
engineer runs "make" on this branch he's testing out that might produce
an object file that'll get used as-is once he switches back, since
you've set the mtime in the past for that file because you re-checked it
out.

Re: [RFC 01/10] submodule: add 'core.submodulesFile' to override the '.gitmodules' path

[Antonio Ospite]

On Thu, 12 Apr 2018 16:50:03 -0700
Stefan Beller  wrote:

> Hi Antonio,
> 
> On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite  wrote:
> > When multiple repositories with detached work-trees take turns using the
> > same directory as their work-tree, and more than one of them want to use
> > submodules, there will be conflicts about the '.gitmodules' file.
> 
> unlike other files which would not conflict?
> There might be file names such as LICENSE, Readme.md etc,
> which are common enough that they would produce conflicts as well?
> I find this argument on its own rather weak. ("Just delete everything in
> the working dir before using it with another repository"). I might be
> missing a crucial bit here?
>

All the vcsh repositories _share_ the same work-tree; they may control
it taking turns but, in general, all files are meant to be checked out
at all times as the basic use case is: *distinct* sets of config files.

Maybe saying that the repositories "take turns" is confusing.
It's an unnecessary information, so I will omit that part form the
commit message.

After your question I've done some research and I've seen other vcsh
users managing conflicting LICENSE and README files using git
sparse-checkouts, to have these files in the single repositories but
not checked out in the shared work-tree:
https://github.com/RichiH/vcsh/issues/120#issuecomment-42639619
https://github.com/jwhitley/vcsh-root/commit/30b0d495c2cbe47ae9617ace9c2c14720d961d78

However I guess that my point here is that the gitmodules file is
something that influences git behavior so it should not be on the user's
shoulder to manage conflicts for it, and most importantly it needs to
be checked out for git to access it, doesn't it?

> > git hardcodes this path so it's not possible to override its location on
> > a per-repository basis to allow such repositories to coexists
> > peacefully.
> >
> > Make the path of the "gitmodules file" customizable exposing
> > a 'core.submodulesFile' configuration setting.
> >
> > The default value will still be '.gitmodules' when 'core.submodulesFile'
> > is not set.
> 
> ok.
> 
> 
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -1774,6 +1774,7 @@ extern void prepare_pager_args(struct child_process 
> > *, const char *pager);
> >  extern const char *editor_program;
> >  extern const char *askpass_program;
> >  extern const char *excludes_file;
> > +extern const char *submodules_file;
> 
> Could you place this variable in repository.h in struct repository?
> (Some developers currently try to move any global state to that place,
> as that makes working with e.g. nested submodules easier in-process
> and you would not need to spawn processes for submodules)
> 
> Once migrated to the repository struct mentioned above, you'd access
> it via the_repository->submodules_file for the main repository.
>

OK, thanks, I didn't like the global variable either, I was just copying
from excludes_file.

> 
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index 24914963c..610fd0dc5 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -71,7 +71,9 @@ get_submodule_config () {
> > value=$(git config submodule."$name"."$option")
> > if test -z "$value"
> > then
> > -   value=$(git config -f .gitmodules 
> > submodule."$name"."$option")
> > +   gitmodules_file=$(git config core.submodulesfile)
> > +   : ${gitmodules_file:=.gitmodules}
> > +   value=$(git config -f "$gitmodules_file" 
> > submodule."$name"."$option")
> 
> I wonder if it would be cheaper to write a special config lookup now, e.g.
> in builtin/submodule--helper.c we could have a "config-from-gitmodules"
> subcommand that is looking up the modules file and then running the config
> on that file.
>

Can you give an example from the user point of view of such a
"config-from-gitmodules" command?

I might look into it, but that can also be a followup change. 

> I am surprised how little access of the .gitmodules is left in 
> git-submodule.sh
> (which is partially ported to the builtin/submodule--helper.c)
> 
> > diff --git a/submodule-config.c b/submodule-config.c
> > index 3f2075764..8a3396ade 100644
> > --- a/submodule-config.c
> > +++ b/submodule-config.c
> > @@ -468,7 +468,7 @@ static int gitmodule_oid_from_commit(const struct 
> > object_id *treeish_name,
> > return 1;
> > }
> >
> > -   strbuf_addf(rev, "%s:.gitmodules", oid_to_hex(treeish_name));
> > +   strbuf_addf(rev, "%s:%s", oid_to_hex(treeish_name), 
> > submodules_file);
> > if (get_oid(rev->buf, gitmodules_oid) >= 0)
> > ret = 1;
> >
> > @@ -583,7 +583,7 @@ void repo_read_gitmodules(struct repository *repo)
> > if (repo_read_index(repo) < 0)
> > return;
> >
> > -   gitmodules = repo_worktree_path(repo, GITMODULES_FILE);
> > +   gitmodules = repo_worktree_path(repo, 

Re: [PATCH v2 3/6] generate-cmdlist.sh: keep all information in common-cmds.h

[Ramsay Jones]

On 16/04/18 16:43, SZEDER Gábor wrote:
> On Sun, Apr 15, 2018 at 6:42 PM, Nguyễn Thái Ngọc Duy  
> wrote:
>> common-cmds.h is used to extract the list of common commands (by
>> group) and a one-line summary of each command. Some information is
>> dropped, for example command category or summary of other commands.
>> Update generate-cmdlist.sh to keep all the information. The extra info
>> will be used shortly.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>>  generate-cmdlist.sh | 61 +
>>  help.c  | 42 ++-
>>  2 files changed, 81 insertions(+), 22 deletions(-)
>>
>> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
>> index eeea4b67ea..e0893e979a 100755
>> --- a/generate-cmdlist.sh
>> +++ b/generate-cmdlist.sh
> 
>> -printf 'static struct cmdname_help common_cmds[] = {\n'
>> -grep -f "$match" "$1" |
>> -sed 's/^git-//' |
>> +printf 'static struct cmdname_help command_list[] = {\n'
>> +command_list "$1" |
>>  sort |
>> -while read cmd tags
>> +while read cmd category tags
>>  do
>> -   tag=$(echo "$tags" | sed "$substnum; s/[^0-9]//g")
>> +   name=${cmd/git-}
> 
> There are two issues with this line:
> 
> - This is a "regular" shell script, therefore it must not use pattern
>   substitution.
> 
> - The pattern substitution would remove the string "git-" in the middle of
>   the variable as well; I suspect this is undesired.
> 
> I think that the remove matching prefix pattern substitution should be
> used here.


This also, apparently, doesn't work with dash. On cygwin, which has
bash as /bin/sh, this builds common-cmds.h just fine. On linux, which
has dash as /bin/sh, this fails, leaving a truncated file:

  $ bash generate-cmdlist.sh command-list.txt >zzz
  $ tail -3 zzz
{"gitweb", N_("Git web interface (web frontend to Git repositories)"), 
CAT_ancillaryinterrogators, GROUP_NONE },
{"workflows", N_("An overview of recommended workflows with Git"), 
CAT_guide, GROUP_NONE },
  };

  $ dash generate-cmdlist.sh command-list.txt >zzz
  generate-cmdlist.sh: 73: generate-cmdlist.sh: Bad substitution
  $ tail -3 zzz

  static struct cmdname_help command_list[] = {
  };
  $ 

This leads to a very broken 'git help'.

ATB,
Ramsay Jones

Re: Git Bash Completion Not Working In Emacs

[Andreas Schwab]

On Apr 16 2018, Marko Vasic  wrote:

> Git bash completion script works perfectly under the terminal,
> however, it does not work in Emacs (neither shell nor gui mode). Did
> anyone encounter this issues and how can it be solved?

Depend on how you run the command in Emacs.  If you use M-x shell then
Emacs handles special keys like TAB itself (and the started shell
usually disables its own command line editing).  If you use M-x term
then Emacs emulates a terminal and lets the running process handle all
special keys.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: man page for "git remote set-url" seems confusing/contradictory

[Robert P. J. Day]

On Mon, 16 Apr 2018, Andreas Schwab wrote:

> On Apr 16 2018, "Robert P. J. Day"  wrote:
>
> > i don't understand how you can clearly set the fetch and push URLs
> > of a remote independently, while the man page nonetheless insists
> > that "even though they can be set differently, must still refer to
> > the same place". how can they be set differently yet still must
> > refer to the same place?
>
> They could be using different transport methods.  For example, for
> fetching the unauthenticated git: method could be used, but for
> pushing an authenticated method like ssh: is usually needed.

  ok, point taken, but does that mean the two must actually refer to
the same "place"? it seems that i'm perfectly free to use this command
to set the push and fetch URLs to totally different locations.

rday

Re: [RFC 02/10] submodule: fix getting custom gitmodule file in fetch command

[Antonio Ospite]

On Thu, 12 Apr 2018 16:55:15 -0700
Stefan Beller  wrote:

> Hi Antonio,
> 
> the subject line could also be
>   fetch: fix custom submodule location
> as I was not expecting this patch from the subject line.
>

OK.

> On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite  wrote:
> > Import the default git configuration before accessing the gitmodules
> > file.
> >
> > This is important when a value different from the default one is set via
> > the 'core.submodulesfile' config.
> >
> > Signed-off-by: Antonio Ospite 
> > ---
> >  builtin/fetch.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/builtin/fetch.c b/builtin/fetch.c
> > index dcdfc66f0..d56636f74 100644
> > --- a/builtin/fetch.c
> > +++ b/builtin/fetch.c
> > @@ -1428,8 +1428,8 @@ int cmd_fetch(int argc, const char **argv, const char 
> > *prefix)
> > for (i = 1; i < argc; i++)
> > strbuf_addf(_rla, " %s", argv[i]);
> >
> > -   config_from_gitmodules(gitmodules_fetch_config, NULL);
> > git_config(git_fetch_config, NULL);
> > +   config_from_gitmodules(gitmodules_fetch_config, NULL);
> 
> Wouldn't this break the overwriting behavior?
> e.g. you configure a submodule URL in .gitmodules and then override it
> in .git/config,
> then we'd currently first load config from the modules file and then override 
> it
> in git_config(git_fetch_config,..), whereas swapping them might have
> unintended consequences? Do the tests pass with this patch?

The patch changes the current behavior indeed, but it does not break
any of the existing tests.

Anyway, to be on the safe side, I could load only the
core.submodulesFile option from the global configuration, maybe even in
config_from_gitmodules() itself, before accessing the gitmodules file,
but I still don't know how to do that.

Is there an API to just load one config setting?

Ciao,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: man page for "git remote set-url" seems confusing/contradictory

[Andreas Schwab]

On Apr 16 2018, "Robert P. J. Day"  wrote:

> i don't understand how you can clearly set the fetch and push URLs of
> a remote independently, while the man page nonetheless insists that
> "even though they can be set differently, must still refer to the same
> place". how can they be set differently yet still must refer to the
> same place?

They could be using different transport methods.  For example, for
fetching the unauthenticated git: method could be used, but for pushing
an authenticated method like ssh: is usually needed.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Optimizing writes to unchanged files during merges?

[Lars Schneider]

> On 16 Apr 2018, at 04:03, Linus Torvalds  
> wrote:
> 
> On Sun, Apr 15, 2018 at 6:44 PM, Junio C Hamano  wrote:
>> 
>> I think Elijah's corrected was_tracked() also does not care "has
>> this been renamed".
> 
> I'm perfectly happy with the slightly smarter patches. My patch was
> really just an RFC and because I had tried it out.
> 
>> One thing that makes me curious is what happens (and what we want to
>> happen) when such a "we already have the changes the side branch
>> tries to bring in" path has local (i.e. not yet in the index)
>> changes.  For a dirty file that trivially merges (e.g. a path we
>> modified since our histories forked, while the other side didn't do
>> anything, has local changes in the working tree), we try hard to
>> make the merge succeed while keeping the local changes, and we
>> should be able to do the same in this case, too.
> 
> I think it might be nice, but probably not really worth it.
> 
> I find the "you can merge even if some files are dirty" to be really
> convenient, because I often keep stupid test patches in my tree that I
> may not even intend to commit, and I then use the same tree for
> merging.
> 
> For example, I sometimes end up editing the Makefile for the release
> version early, but I won't *commit* that until I actually cut the
> release. But if I pull some branch that has also changed the Makefile,
> it's not worth any complexity to try to be nice about the dirty state.
> 
> If it's a file that actually *has* been changed in the branch I'm
> merging, and I'm more than happy to just stage the patch (or throw it
> away - I think it's about 50:50 for me).
> 
> So I don't think it's a big deal, and I'd rather have the merge fail
> very early with "that file has seen changes in the branch you are
> merging" than add any real complexity to the merge logic.

I am happy to see this discussion and the patches, because long rebuilds 
are a constant annoyance for us. We might have been bitten by the exact 
case discussed here, but more often, we have a slightly different 
situation:

An engineer works on a task branch and runs incremental builds — all 
is good. The engineer switches to another branch to review another 
engineer's work. This other branch changes a low-level header file, 
but no rebuild is triggered. The engineer switches back to the previous 
task branch. At this point, the incremental build will rebuild 
everything, as the compiler thinks that the low-level header file has
been changed (because the mtime is different).

Of course, this problem can be solved with a separate worktree. However, 
our engineers forget about that sometimes, and then, they are annoyed by 
a 4h rebuild.

Is this situation a problem for others too?
If yes, what do you think about the following approach:

What if Git kept a LRU list that contains file path, content hash, and 
mtime of any file that is removed or modified during a checkout. If a 
file is checked out later with the exact same path and content hash, 
then Git could set the mtime to the previous value. This way the 
compiler would not think that the content has been changed since the 
last rebuild.

I think that would fix the problem that our engineers run into and also 
the problem that Linus experienced during the merge, wouldn't it?

Thanks,
Lars

Re: Draft of Git Rev News edition 38

[Jacob Keller]

On Mon, Apr 16, 2018 at 5:29 AM, Sergey Organov  wrote:
> Hi Christian,
>
> Christian Couder  writes:
>> On Mon, Apr 16, 2018 at 12:11 AM, Christian Couder
>>  wrote:
>>>
>>> A draft of a new Git Rev News edition is available here:
>>>
>>>   
>>> https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-38.md
>>
>> The draft has just been updated with 2 articles contributed by Jake
>> about rebasing merges, so I am cc'ing more people involved in those
>> discussions.
>
> I find this section of the draft pretty close to my own vision of what
> and how has been discussed, except for a few issues.
>
> [all quotations below are taken from the draft]
>
>> Some discussion about --preserve-merges and compatibility with scripts
>> (i.e. should we change or fix it? or should we deprecate it?)
>> followed.
>>
>>Rebasing merges: a jorney to the ultimate solution (Road Clear)
>>(written by Jacob Keller)
>
> What article by Jacob is actually meant here I have no idea, please
> check, as this one, and the RFC this refers to, was written by me, not
> by Jacob, and it is the outline of potential method of actually rebasing
> merges that is discussed in the next paragraph, so it likely belongs
> right after the next paragraph:

I believe he meant that the summary on git rev news was written by me,
that's all :)

>
>> After the discussions in the above article Sergey posted an outline of a
>> potential method for actually rebasing a merge (as opposed to recreating
>> it from scratch) which used a process of git cherry-pick -mN of the
>> merge onto each topic branch being merged, and then merging the result.
>
> The reference to:
>
> Rebasing merges: a jorney to the ultimate solution (Road Clear)
> (written by Sergey Organov)
>
> belongs here, if at all.
>
> In addition, I'd like to see a minor edition to the following:
>
>> Sergey replied that he thinks the solution produces the same result as
>> his updated strategy.
>
> This has been said in the context that assumed lack of conflicts during
> application of both strategies. Something like this, maybe:
>
> "Sergey replied that he thinks the solution produces the same result as
> his updated strategy, at least when none of the strategies produce any
> conflicts."
>
> Next, this is very close, but not exactly right:
>
>> Further suggestions to the strategy were proposed and tested, ultimately
>> resulting in Sergey proposing the addition of using the original merge
>> commit as a merge base during the final step.
>
> This was not an addition, this was a fix of particular mistake in the
> original RFC that has been revealed during testing. I didn't get it
> right at first that it's original merge commit that must be used as
> merge base, so my original proposal ended up implicitly using wrong
> merge base, that is the one computed by "git merge-base U1' U2'".
>
> Something along these lines may fit better:
>
> "Further suggestions to the strategy were proposed and tested,
> ultimately resulting in Sergey proposing the fix to his method,
> specifically using the original merge commit as a merge base during the
> final step."
>
> I'd also like a reference to the final fixed [RFC v2] be added right
> here. The reference is:
>
> https://public-inbox.org/git/87r2oxe3o1@javad.com/
>
> Thanks a lot!
>
> -- Sergey

Yep that all sounds right to me also.

Thanks,
Jake

Git Bash Completion Not Working In Emacs

[Marko Vasic]

Hello,

Git bash completion script works perfectly under the terminal,
however, it does not work in Emacs (neither shell nor gui mode). Did
anyone encounter this issues and how can it be solved?

System I use:
OS: Ubuntu 17.10
Emacs: 25.2.2

Thanks,
Marko

Re: [PATCH v2 3/6] generate-cmdlist.sh: keep all information in common-cmds.h

[Duy Nguyen]

On Mon, Apr 16, 2018 at 8:28 AM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>> @@ -23,28 +36,44 @@ sed -n '
>>   ' "$1"
>>  printf '};\n\n'
>>
>> +echo "#define GROUP_NONE 0xff /* no common group */"
>
> Some later code forgets about this value, and causes "git" to
> segfault at the end of this entire series.
>
> Namely, here:
>
>> - for (i = 0; i < ARRAY_SIZE(common_cmds); i++) {
>> + for (i = 0; i < nr; i++) {
>>   if (common_cmds[i].group != current_grp) {
>>   printf("\n%s\n", 
>> _(common_cmd_groups[common_cmds[i].group]));
>>   current_grp = common_cmds[i].group;
>
> where common_cmd_groups[] gets overrun.

Argh!! I thought I tested everything. Sorry for the sloppy quality.

>
> Here is a squash I'll queue on top to keep the tip of 'pu' at least
> buildable.

Thanks. It's actually interesting that we have main porcelain commands
that belong to no group. I'll try to classify them so that they show
up as well.


-- 
Duy

Re: [PATCH v2 3/6] generate-cmdlist.sh: keep all information in common-cmds.h

[SZEDER Gábor]

On Sun, Apr 15, 2018 at 6:42 PM, Nguyễn Thái Ngọc Duy  wrote:
> common-cmds.h is used to extract the list of common commands (by
> group) and a one-line summary of each command. Some information is
> dropped, for example command category or summary of other commands.
> Update generate-cmdlist.sh to keep all the information. The extra info
> will be used shortly.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  generate-cmdlist.sh | 61 +
>  help.c  | 42 ++-
>  2 files changed, 81 insertions(+), 22 deletions(-)
>
> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> index eeea4b67ea..e0893e979a 100755
> --- a/generate-cmdlist.sh
> +++ b/generate-cmdlist.sh

> -printf 'static struct cmdname_help common_cmds[] = {\n'
> -grep -f "$match" "$1" |
> -sed 's/^git-//' |
> +printf 'static struct cmdname_help command_list[] = {\n'
> +command_list "$1" |
>  sort |
> -while read cmd tags
> +while read cmd category tags
>  do
> -   tag=$(echo "$tags" | sed "$substnum; s/[^0-9]//g")
> +   name=${cmd/git-}

There are two issues with this line:

- This is a "regular" shell script, therefore it must not use pattern
  substitution.

- The pattern substitution would remove the string "git-" in the middle of
  the variable as well; I suspect this is undesired.

I think that the remove matching prefix pattern substitution should be
used here.

Re: .gitattributes lookup doesn't respect GIT_WORK_TREE

[Duy Nguyen]

On Mon, Apr 16, 2018 at 12:40 AM, Andreas Schwab  wrote:
> On Apr 16 2018, Junio C Hamano  wrote:
>
>> I may be mistaken (I do not have the code in front of me right now)
>> but IIRC after the setup.c code runs (which happens quite early in
>> the sequence that starts from git.c::cmd_main()), the Git process
>> moves to the top level of the working tree,
>
> git log/show don't appear to do that.

Yeah we lazily set up worktree in some cases. Elsewhere in the
chdir-notify thread, I suggested that we set up worktree
unconditionally (but do not die setup fails; only die when a command
specifically requests for worktree). That work would help make this
work in most cases. But it's not materialized yet.
-- 
Duy

Re: Draft of Git Rev News edition 38

[Sergey Organov]

Kaartic Sivaraam  writes:

> Hi,
>
> On Monday 16 April 2018 08:33 PM, Sergey Organov wrote:
>> Christian Couder  writes:
>>> Here "the above article" means the Jake's "branch -l: print useful
>>> info whilst rebasing a non-local branch" article above the current
>>> article.
>
> Just a little correction. I suppose Chris actually meant the "rebase -i:
> offer to recreate merge commits" article written by Jake and not the
> "branch -l: print useful info whilst rebasing a non-local branch" article.
>
> That said, I read the draft and found it good except for two minor issues,
>
> 1. I see the following sentence in the "Rebasing merges: a jorney to the
> ultimate solution (Road Clear) (written by Jacob Keller)" article
>
>   "A few examples were tried, but it was proven that the original
>   concept did not work, as dropped commits could end up being
>   replaid into the merge commits, turning them into "evil"
>   merges."
>
> I'm not sure if 'replaid' is proper English assuming the past tense of
> replay was intended there (which I think is 'replayed').

It could have meant, say, "reapplied", -- we need to ask the author.

While we are at it, please also consider to replace "original concept"
by "original algorithm", as it didn't work due to a mistake in the
algorithm as opposed to failure of the concept itself.

-- Sergey

Re: Draft of Git Rev News edition 38

[Kaartic Sivaraam]

Hi,

On Monday 16 April 2018 08:33 PM, Sergey Organov wrote:
> Christian Couder  writes:
>> Here "the above article" means the Jake's "branch -l: print useful
>> info whilst rebasing a non-local branch" article above the current
>> article.

Just a little correction. I suppose Chris actually meant the "rebase -i:
offer to recreate merge commits" article written by Jake and not the
"branch -l: print useful info whilst rebasing a non-local branch" article.

That said, I read the draft and found it good except for two minor issues,

1. I see the following sentence in the "Rebasing merges: a jorney to the
ultimate solution (Road Clear) (written by Jacob Keller)" article

"A few examples were tried, but it was proven that the original
concept did not work, as dropped commits could end up being
replaid into the merge commits, turning them into "evil"
merges."

I'm not sure if 'replaid' is proper English assuming the past tense of
replay was intended there (which I think is 'replayed').


2. I see a minor Markdown syntax issue in the "branch -l: print useful
info whilst rebasing a non-local branch" article.

... reworked his original patch to improve `git branch
--list̀

Specifically, in the '--list̀' part. I guess it should be "--list`".

-- 
Kaartic

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - Joel Spolsky



signature.asc
Description: OpenPGP digital signature

Re: Draft of Git Rev News edition 38

[Sergey Organov]

Christian Couder  writes:

> Hi Sergey,
>
[...]
> Jake wrote the article below the above line. His article summarizes
> the discussions that happened following your email that is linked to
> in the above line. The above line is actually the title of Jake's
> second article.
>
[...]
> Here "the above article" means the Jake's "branch -l: print useful
> info whilst rebasing a non-local branch" article above the current
> article.
>
[...]
> You call it a reference but it is actually the title of the article
> that Jake wrote. Yes, it contains a link to your email, but that is
> just because we want to make it easy and straightforward for people
> who are interested in all the discussions to find them.

Yeah, I see now, it was confusion on my side. Thanks for clarification!

The rest is also fine with me, and thanks for editorial changes!

-- Sergey

"proper" way to deactivate push for a remote?

[Robert P. J. Day]

 another feature i've seen for the very first time ... working with
kubernetes so i checked it out of github, and part of the instructions
for that is to make sure you don't accidentally try to push back to
the github remote, so the directions suggest:

$ git remote add upstream https://github.com/kubernetes/kubernetes.git
$ git remote set-url --push upstream no_push

  fair enough, i just assumed the word "no_push" was some magical
keyword in that context, but as i read it, all you need to do is put
*some* invalid URL value there, is that correct?

  and is that the accepted way to do that? what about just deleting
that line from .git/config? is that valid, or is there a different
recommendation for doing that? thanks.

rday

Re: [PATCH] completion: reduce overhead of clearing cached --options

[Jakub Narębski]

On 16 April 2018 at 15:15, SZEDER Gábor  wrote:
> On Mon, Apr 16, 2018 at 7:10 AM, Junio C Hamano  wrote:
> > SZEDER Gábor  writes:
> >> On Fri, Apr 13, 2018 at 11:44 PM, Jakub Narebski  wrote:
> >>> SZEDER Gábor  writes:
> 
>  In Bash we can do better: run the 'compgen -v __gitcomp_builtin_'
>  builtin command, which lists the same variables, but without a
>  pipeline and 'sed' it can do so with lower overhead.
> >>>
> >>> What about ZSH?
> >>
> >> Nothing, ZSH is unaffected by this patch.
> >
> > OK, do we want to follow it up with [PATCH 2/1] to add the LC_ALL=C
> > thing to the ZSH side to help that "sed", then?
>
> No.  'sed' would only need need help when its input comes from a buggy
> 'set' builtin of a particular version of Bash from a particular vendor.
>
> As far as I can test this in Travis CI's OSX builds, ZSH doesn't seem to
> be affected, neither the version Apple ships by default nor the version
> installed via homebrew.

That's nice - this means that the patch fixes all of the issue.
The above information should be, in my opinion, included
in the commit message, though.

Best,
-- 
Jakub Narębski

Re: [PATCH] completion: reduce overhead of clearing cached --options

[SZEDER Gábor]

On Mon, Apr 16, 2018 at 7:10 AM, Junio C Hamano  wrote:
> SZEDER Gábor  writes:
>
>> On Fri, Apr 13, 2018 at 11:44 PM, Jakub Narebski  wrote:
>>> SZEDER Gábor  writes:
 In Bash we can do better: run the 'compgen -v __gitcomp_builtin_'
 builtin command, which lists the same variables, but without a
 pipeline and 'sed' it can do so with lower overhead.
>>>
>>> What about ZSH?
>>
>> Nothing, ZSH is unaffected by this patch.
>
> OK, do we want to follow it up with [PATCH 2/1] to add the LC_ALL=C
> thing to the ZSH side to help that "sed", then?

No.  'sed' would only need need help when its input comes from a buggy
'set' builtin of a particular version of Bash from a particular vendor.

As far as I can test this in Travis CI's OSX builds, ZSH doesn't seem to
be affected, neither the version Apple ships by default nor the version
installed via homebrew.

Re: Draft of Git Rev News edition 38

[Christian Couder]

Hi Sergey,

On Mon, Apr 16, 2018 at 2:29 PM, Sergey Organov  wrote:
> Hi Christian,
>
> Christian Couder  writes:
>> On Mon, Apr 16, 2018 at 12:11 AM, Christian Couder
>>  wrote:
>>>
>>> A draft of a new Git Rev News edition is available here:
>>>
>>>   
>>> https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-38.md
>>
>> The draft has just been updated with 2 articles contributed by Jake
>> about rebasing merges, so I am cc'ing more people involved in those
>> discussions.
>
> I find this section of the draft pretty close to my own vision of what
> and how has been discussed, except for a few issues.
>
> [all quotations below are taken from the draft]
>
>> Some discussion about --preserve-merges and compatibility with scripts
>> (i.e. should we change or fix it? or should we deprecate it?)
>> followed.
>>
>>Rebasing merges: a jorney to the ultimate solution (Road Clear)
>>(written by Jacob Keller)
>
> What article by Jacob is actually meant here I have no idea, please
> check, as this one, and the RFC this refers to, was written by me, not
> by Jacob,

Jake wrote the article below the above line. His article summarizes
the discussions that happened following your email that is linked to
in the above line. The above line is actually the title of Jake's
second article.

> and it is the outline of potential method of actually rebasing
> merges that is discussed in the next paragraph, so it likely belongs
> right after the next paragraph:
>
>> After the discussions in the above article

Here "the above article" means the Jake's "branch -l: print useful
info whilst rebasing a non-local branch" article above the current
article.

>> Sergey posted an outline of a
>> potential method for actually rebasing a merge (as opposed to recreating
>> it from scratch) which used a process of git cherry-pick -mN of the
>> merge onto each topic branch being merged, and then merging the result.
>
> The reference to:
>
> Rebasing merges: a jorney to the ultimate solution (Road Clear)
> (written by Sergey Organov)
>
> belongs here, if at all.

You call it a reference but it is actually the title of the article
that Jake wrote. Yes, it contains a link to your email, but that is
just because we want to make it easy and straightforward for people
who are interested in all the discussions to find them.

It has been like this since the very beginning of Git Rev News. For
example in the first edition
(https://git.github.io/rev_news/2015/03/25/edition-1/) the first
article was contributed by Junio so you can see "Promoting Git
developers (written by Junio C Hamano)" where "Promoting Git
developers" is a link to the following email (yeah the link is not
valid anymore because Gmane is no more) that I wrote:

https://public-inbox.org/git/cap8ufd1+rc0fjissdddcyn1e_75wtbu9pepucqx5zntd4zk...@mail.gmail.com/

> In addition, I'd like to see a minor edition to the following:
>
>> Sergey replied that he thinks the solution produces the same result as
>> his updated strategy.
>
> This has been said in the context that assumed lack of conflicts during
> application of both strategies. Something like this, maybe:
>
> "Sergey replied that he thinks the solution produces the same result as
> his updated strategy, at least when none of the strategies produce any
> conflicts."

Ok with this change.

> Next, this is very close, but not exactly right:
>
>> Further suggestions to the strategy were proposed and tested, ultimately
>> resulting in Sergey proposing the addition of using the original merge
>> commit as a merge base during the final step.
>
> This was not an addition, this was a fix of particular mistake in the
> original RFC that has been revealed during testing. I didn't get it
> right at first that it's original merge commit that must be used as
> merge base, so my original proposal ended up implicitly using wrong
> merge base, that is the one computed by "git merge-base U1' U2'".
>
> Something along these lines may fit better:
>
> "Further suggestions to the strategy were proposed and tested,
> ultimately resulting in Sergey proposing the fix to his method,
> specifically using the original merge commit as a merge base during the
> final step."

Ok with this change except for s/the fix to his method/a fix to his
method/ as I think it reads better with "a".

> I'd also like a reference to the final fixed [RFC v2] be added right
> here. The reference is:
>
> https://public-inbox.org/git/87r2oxe3o1@javad.com/

Ok to add this link.

I just pushed the changes

Thanks for your comments,
Christian.

man page for "git remote set-url" seems confusing/contradictory

[Robert P. J. Day]

  never had cause to examine "git remote set-url" before so i'm a bit
puzzled by this part of the man page:

  set-url

  Changes URLs for the remote. Sets first URL for remote
   that matches regex  ...

  With --push, push URLs are manipulated instead of fetch URLs.
 .. snip ...

  Note that the push URL and the fetch URL, even though they can be
  set differently, must still refer to the same place. What you pushed to
  the push URL should be what you would see if you immediately fetched
  from the fetch URL. If you are trying to fetch from one place (e.g.
  your upstream) and push to another (e.g. your publishing
  repository), use two separate remotes.

i don't understand how you can clearly set the fetch and push URLs of
a remote independently, while the man page nonetheless insists that
"even though they can be set differently, must still refer to the same
place". how can they be set differently yet still must refer to the
same place?

  i suspect i'll have a couple more questions related to this shortly
after i do a bit more reading.

rday

Re: Draft of Git Rev News edition 38

[Sergey Organov]

Hi Christian,

Christian Couder  writes:
> On Mon, Apr 16, 2018 at 12:11 AM, Christian Couder
>  wrote:
>>
>> A draft of a new Git Rev News edition is available here:
>>
>>   
>> https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-38.md
>
> The draft has just been updated with 2 articles contributed by Jake
> about rebasing merges, so I am cc'ing more people involved in those
> discussions.

I find this section of the draft pretty close to my own vision of what
and how has been discussed, except for a few issues.

[all quotations below are taken from the draft]

> Some discussion about --preserve-merges and compatibility with scripts
> (i.e. should we change or fix it? or should we deprecate it?)
> followed.
>
>Rebasing merges: a jorney to the ultimate solution (Road Clear)
>(written by Jacob Keller)

What article by Jacob is actually meant here I have no idea, please
check, as this one, and the RFC this refers to, was written by me, not
by Jacob, and it is the outline of potential method of actually rebasing
merges that is discussed in the next paragraph, so it likely belongs
right after the next paragraph:

> After the discussions in the above article Sergey posted an outline of a
> potential method for actually rebasing a merge (as opposed to recreating
> it from scratch) which used a process of git cherry-pick -mN of the
> merge onto each topic branch being merged, and then merging the result. 

The reference to:

Rebasing merges: a jorney to the ultimate solution (Road Clear)
(written by Sergey Organov)

belongs here, if at all.

In addition, I'd like to see a minor edition to the following:

> Sergey replied that he thinks the solution produces the same result as
> his updated strategy.

This has been said in the context that assumed lack of conflicts during
application of both strategies. Something like this, maybe:

"Sergey replied that he thinks the solution produces the same result as
his updated strategy, at least when none of the strategies produce any
conflicts."

Next, this is very close, but not exactly right:

> Further suggestions to the strategy were proposed and tested, ultimately
> resulting in Sergey proposing the addition of using the original merge
> commit as a merge base during the final step.

This was not an addition, this was a fix of particular mistake in the
original RFC that has been revealed during testing. I didn't get it
right at first that it's original merge commit that must be used as
merge base, so my original proposal ended up implicitly using wrong
merge base, that is the one computed by "git merge-base U1' U2'".

Something along these lines may fit better:

"Further suggestions to the strategy were proposed and tested,
ultimately resulting in Sergey proposing the fix to his method,
specifically using the original merge commit as a merge base during the
final step."

I'd also like a reference to the final fixed [RFC v2] be added right
here. The reference is:

https://public-inbox.org/git/87r2oxe3o1@javad.com/

Thanks a lot!

-- Sergey

Re: [RFC 07/10] FIXME: wrap-for-bin.sh: set 'core.submodulesFile' for each git invocation

[Antonio Ospite]

On Fri, 13 Apr 2018 13:05:18 -0700
Stefan Beller  wrote:

> Hi Antonio,
> 
> On Fri, Apr 13, 2018 at 1:07 AM, Antonio Ospite  wrote:
> > This is to test custom gitmodules file paths. The default path can be
> > overridden using the 'GIT_MODULES_FILE' environmental variable.
> >
> > Maybe In the final patch the option should be set only when running
> > tests and not unconditionally in the wrapper script, but as a proof of
> > concept the wrapper script was a convenient location.
> >
> > Also, in the final patch a fixed custom file path could be used instead
> > of the environmental variable: to exercise the code it should be enough
> > to have a value different from the default one.
> >
> > The change to 't0001-init.sh' is needed to make the test pass, since now
> > a config is set on the command line.
> 
> Missing sign off.
> 
> So you'd think we'd have to rerun the test suite with GIT_MODULES_FILE set?
> That makes for an expensive test. Can we have just a few tests for a few
> commands to see that the basics are working correctly?
>

This approach of running the test suite twice was only "exploratory",
see the script in 10/10: it was never meant as a the final mechanism.

That's also why the FIXME patches do not have a sign-off.

As I tried to comment above a final version could just
replace the references to '.gitmodules' with some fixed non-default
value, provided that the same value is passed to git via
core.submodulesfile 

If the tests pass with a custom value they'll pass with the default
'.gitmodules' one which is a particular case.

Ciao,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: [RFC 00/10] Make .the gitmodules file path configurable

[Antonio Ospite]

On Thu, 12 Apr 2018 16:36:32 -0700
Stefan Beller  wrote:

> Hi Antonio,
>

Hi Stefan,

> On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite  wrote:
> > Hi,
> >
> > vcsh[1] uses bare git repositories and detached work-trees to manage
> > *distinct* sets of configuration files directly into $HOME.
> >
> > In general, submodules have worked perfectly fine with detached
> > work-trees for some time[2,3,4].
> >
> > However when multiple repositories take turns using the same directory
> > as their work-tree, and more than one of them want to use submodules,
> > there could still be conflicts about the '.gitmodules' file because git
> > hardcodes this path.
> >
[...]
> 
> > So this series proposes a mechanism to set an alternative path for the
> > submodules configuration file (from now on "gitmodules file").
> 
[...]
> > Expanding on that change the first patch in the series makes the path
> > customizable exposing a 'core.submodulesFile' configuration setting.
> 
> I guess the similarity to core.ignoreFile is desired here. Although these
> mechanisms are very different.
>

The option name is similar to 'core.excludesFile' also because, when I
started, I first looked at how multiple ignore files were used, so I
may have been influenced by that.

I acknowledge that the two mechanisms are different, in particular
because git *writes* the gitmodules file itself.

I am not opposed to change the name, something like
'core.submodulesConfigFile' might highlight that the syntax of the file
is the one of git config, but I think it's too long.

> > The new configuration setting can be used to set an *alternative*
> > location for the gitmodules file; IMHO there is no need to provide
> > *additional* locations like in the case of exclude files.
> 
> I think there *may* be a need for additional files.
> Currently there is only the .gitmodules file and the configuration
> in .git/config overriding settings from .gitmodules.
> 
> There was some discussion on the mailing list in the past, which
> presented a intermediate layer in between these two places, in
> a special ref, such that:
> base is in .gitmodules
> overwritten via refs/meta/submodules:.gitmodules
> overwritten via the .git/config
> 
> The intermediate would be a config file that is tracked on another
> ref. This (a) decouples main project history from submodule history
> and (b) makes it easier to distribute as it is part of the repository.
> 
> For example (a) is desired if you dig up an old project and the
> submodules have all moved from one git hosting provider to another.
> Another example would be when you fork a project with submodules
> and don't want to mess with the main history but you just want to
> adjust the submodule URLs. That is possible with such an intermediate
> additional place.
> 
> For (b) you can imagine the fork that you want to distribute in your
> community and you don't want to tell everyone to change the
> submodule URLs, but instead you can provide them with a prepared
> .gitmodules file, that they have to place into that special ref (which
> can be done via fetching).
> 
> I digress as these ideas seem to be orthogonal to your patch series,
> just FYI. prior discussion starting at:
> https://public-inbox.org/git/1462317985-640-1-git-send-email-sbel...@google.com/
> I recall there was a better discussion even prior to that, but have no
> link handy.
>

Just to understand, is that 'refs/meta/submodules:.gitmodules' file
meant to be managed manually? Or do you imagine some options to
instruct "git submodule" to *write* to that file instead of .gitmodules?

In the latter case your idea could cover my use case too, couldn't it?

But most importantly, is this realistically going to be added?
Currently I am not that familiar with the git code base to do it
myself, and I've never explicitly dealt with special refs in git.

The approach of this patch series is a lot simpler, and something I can
work on in my spare time.

Presumably (b) could even be partially supported with my changes, by
having both '.gitmodules' and some custom '.gitmodules-alternative'
files in the repository and tell users to clone with:

  git clone --config core.submodulesFile=.gitmodules-alternative URL

Not as clean as your idea but doable.

[...]
> > Since the gitmodules file is meant to be checked in into the repository,
> > the overridden file path should be relative to the work-tree; is there
> > a way to enforce this constraint at run time (i.e. validate the config
> > value), or is it enough to have it documented?
> 
> I think we'd want to check at run time, if we need this constraint.
> 

I'll look into it once we decide which the way to go.

Thank you.

Ciao,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: Regression in patch add?

[Phillip Wood]

On 15/04/18 14:59, Martin Ågren wrote:
> Hi Mahmoud
> 
> On 15 April 2018 at 14:21,   wrote:
>> I first run `git add -p`, then manually edit a chunk (after hitting `s`
>> once, if it matters). The chunk originally contains the following:
> 
> [...]
> 
>> Under git 2.7.4, I can edit it to the following, which is accepted
>> without a problem:
>>
>> ```diff
>> # Manual hunk edit mode -- see bottom for a quick guide
>> @@ -20,7 +20,7 @@
>> "call dein#add('Shougo/dein.vim', {'rev': 'master'})
>>
>> " Add or remove your plugins here:
>> -   " call dein#add('flazz/vim-colorschemes')
>> -   call dein#add('Haron-Prime/evening_vim')
>> +   call dein#add('flazz/vim-colorschemes')
>> +   call dein#add('Haron-Prime/evening_vim')
>>
>> "core plugins that change the behavior of vim and how we use it 
>> globally
>> ```
>>
>> All I did here was remove one `+` line and manually add another (which
>> is a variant of the second `-` line).
> 
> So the line is identical (sans s/^-/+/). Interesting.
> 
>> Under git 2.17.0.252.gfe0a9ea, the same piece is opened in $VISUAL for
>> editing (and if left unmodified applies OK), but when modified in the
>> to the same exact value, after exiting the editor I receive the
>> following error from git:
>>
>> error: patch fragment without header at line 15: @@ -25,7 +25,8 @@
> 
> I can't seem to reproduce this with some very simple testing. Are you
> able to share your files? Or even better, derive a minimal reproduction
> recipe?
> 
> What happens if you do not do a "remove this line, then add it again",
> but instead turn that unchanged line into context? That is, you edit the
> hunk into something like this (but without white-space damage):
> 
> ...
> -   " call dein#add('flazz/vim-colorschemes')
> +   call dein#add('flazz/vim-colorschemes')
> call dein#add('Haron-Prime/evening_vim')
> ...

That's a good idea to try

> Adding Phillip to cc, since he was recently working in this area

Thanks for cc-ing me

> and might have an idea.

I wish I did!

Best Wishes

Phillip
> 
> Martin
> 

Re: Regression in patch add?

[Phillip Wood]

On 15/04/18 13:21, mqudsi wrote:
> 
> Hello all,
> 
> I'm currently running the latest version of git built from `master`, and
> I'm running into what appears to be a regression in the behavior of the
> piecewise `git add -p` when applying a manually edited chunk.
> 
> I first run `git add -p`, then manually edit a chunk (after hitting `s`
> once, if it matters).

Thanks for mentioning that, it can matter as the code that stitches
split hunks back together can't cope with edited hunks properly (though
the code that checks the hunk immediately after it's been edited doesn't
bother to try and stitch things back together).

> The chunk originally contains the following:
> 
> ```diff
> # Manual hunk edit mode -- see bottom for a quick guide
> @@ -20,7 +20,7 @@
>   "call dein#add('Shougo/dein.vim', {'rev': 'master'})
> 
>   " Add or remove your plugins here:
> - " call dein#add('flazz/vim-colorschemes')
> - call dein#add('Haron-Prime/evening_vim')
> + call dein#add('flazz/vim-colorschemes')
> + call dein#add('danilo-augusto/vim-afterglow')
> 
>   "core plugins that change the behavior of vim and how we use it globally
> ```
> 
> Under git 2.7.4, I can edit it to the following, which is accepted
> without a problem:
> 
> ```diff
> # Manual hunk edit mode -- see bottom for a quick guide
> @@ -20,7 +20,7 @@
>   "call dein#add('Shougo/dein.vim', {'rev': 'master'})
> 
>   " Add or remove your plugins here:
> - " call dein#add('flazz/vim-colorschemes')
> - call dein#add('Haron-Prime/evening_vim')
> + call dein#add('flazz/vim-colorschemes')
> + call dein#add('Haron-Prime/evening_vim')
> 
>   "core plugins that change the behavior of vim and how we use it globally
> ```
> 
> All I did here was remove one `+` line and manually add another (which
> is a variant of the second `-` line).
> 
> Under git 2.17.0.252.gfe0a9ea, the same piece is opened in $VISUAL for
> editing (and if left unmodified applies OK), but when modified in the
> to the same exact value, after exiting the editor I receive the
> following error from git:
> 
> error: patch fragment without header at line 15: @@ -25,7 +25,8 @@

I'm not quite sure what that error message is telling us, I need to
spend some time understanding the code in apply.c that creates this
error message.

I assume that the header is coming from the next hunk which was created
when you split the original hunk. If you could post the original hunk
before it was split and the hunk starting at line 25 after it was split
that might help.

As Martin said if you could share the files or come up with a
reproducible example that would really help in figuring out what is
going wrong.

Thanks for reporting this

Phillip


> I'm not sure what to make of this.
> 
> Thank you,
> 
> Mahmoud Al-Qudsi
> NeoSmart Technologies
> 
> 

Re: Bug: rebase -i creates committer time inversions on 'reword'

[Phillip Wood]

On 14/04/18 14:11, Johannes Schindelin wrote:
> Hi,
> 
> On Sat, 14 Apr 2018, Phillip Wood wrote:
> 
>> On 13/04/18 17:52, Johannes Sixt wrote:
>>>
>>> I just noticed that all commits in a 70-commit branch have the same
>>> committer timestamp. This is very unusual on Windows, where rebase -i of
>>> such a long branch takes more than one second (but not more than 3 or
>>> so thanks to the builtin nature of the command!).
>>>
>>> And, in fact, if you mark some commits with 'reword' to delay the quick
>>> processing of the patches, then the reworded commits have later time
>>> stamps, but subsequent not reworded commits receive the earlier time
>>> stamp. This is clearly not intended.
>>
>> Oh dear, I think this is probably due to my series making rebase commit
>> in-process when the commit message isn't being edited. I didn't realize
>> that git cached the commit date rather than using the current time when
>> calling commit_tree_extended(). I'll take a look at it next week.
> 
> Thanks.
> 
> However, a quick lock at `git log @{u}.. --format=%ct` in my
> `sequencer-shears` branch thicket (which I rebase frequently on top of
> upstream's `master` using the last known-good `rebase-merges` sub-branch)
> shows that the commits have different-enough commit timestamps. (It is
> satisfying to see that multiple commits were made during the same second,
> of course.)
> 
> So while I cannot find anything in the code that disagrees with Hannes'
> assessment, it looks on the surface as if I did not encounter the bug
> here.
> 
> Curious.

That's strange (I'd have expected the picks after recreated merges to
have the earlier timestamps than the merge), if I do 'git rebase -i
--force-rebase --exec="sleep 2" @~5' then all the new commits have the
same timestamp.

> FWIW I agree with Hannes' patch.
> 
>> I think 'git am' probably gives all patches the same commit time as well
>> if the commit date is cached though it wont suffer from the time-travel
>> problem.
> 
> I thought that `git am` was the subject of such a complaint recently, but
> I thought that had been resolved? Apparently I misremember...

I had a quick look and couldn't see anything about that, it looks to me
like it just calls commit_tree() and only does anything to change the
default commit date if '--committer-date-is-author-date' was given.

Best Wishes

Phillip
> Ciao,
> Dscho
> 

[PATCH] show: add --follow-symlinks option for :

[Michael Vogt]

Add a --follow-symlinks option that'll resolve symlinks to their
targets when the target is of the form :.

Without it, git will show the path of the link itself if the symlink
is the leaf node of , or otherwise an error if some component
of  is a symlink to another location in the repository. With
the new --follow-symlinks option both will be resolved to their
target, and its content shown instead.

Signed-off-by: Michael Vogt 
---
 Documentation/git-show.txt |  7 +++
 builtin/log.c  |  6 +-
 revision.c |  2 ++
 revision.h |  1 +
 t/t1800-git-show.sh| 41 ++
 5 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100755 t/t1800-git-show.sh

diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
index e73ef5401..e2634b27e 100644
--- a/Documentation/git-show.txt
+++ b/Documentation/git-show.txt
@@ -39,6 +39,13 @@ OPTIONS
For a more complete list of ways to spell object names, see
"SPECIFYING REVISIONS" section in linkgit:gitrevisions[7].
 
+--follow-symlinks::
+   Follow symlinks inside the repository when requesting objects
+   in extended revision syntax of the form tree-ish:path-in-tree.
+   It will resolve any symlinks in  and shows the
+   content of the link if the symlink is the leaf node of
+   .
+
 include::pretty-options.txt[]
 
 
diff --git a/builtin/log.c b/builtin/log.c
index 94ee177d5..7d815b8ea 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -142,7 +142,7 @@ static void cmd_log_init_finish(int argc, const char 
**argv, const char *prefix,
 struct rev_info *rev, struct setup_revision_opt *opt)
 {
struct userformat_want w;
-   int quiet = 0, source = 0, mailmap = 0;
+   int quiet = 0, source = 0, mailmap = 0, follow_symlinks = 0;
static struct line_opt_callback_data line_cb = {NULL, NULL, 
STRING_LIST_INIT_DUP};
static struct string_list decorate_refs_exclude = 
STRING_LIST_INIT_NODUP;
static struct string_list decorate_refs_include = 
STRING_LIST_INIT_NODUP;
@@ -162,6 +162,8 @@ static void cmd_log_init_finish(int argc, const char 
**argv, const char *prefix,
OPT_CALLBACK('L', NULL, _cb, "n,m:file",
 N_("Process line range n,m in file, counting from 
1"),
 log_line_range_callback),
+   OPT_BOOL(0, "follow-symlinks", _symlinks,
+N_("follow in-tree symlinks (used when showing file 
content)")),
OPT_END()
};
 
@@ -176,6 +178,8 @@ static void cmd_log_init_finish(int argc, const char 
**argv, const char *prefix,
 
if (quiet)
rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
+   if (follow_symlinks)
+   rev->follow_symlinks = 1;
argc = setup_revisions(argc, argv, rev, opt);
 
/* Any arguments at this point are not recognized */
diff --git a/revision.c b/revision.c
index b42c836d7..4ab22313f 100644
--- a/revision.c
+++ b/revision.c
@@ -1678,6 +1678,8 @@ int handle_revision_arg(const char *arg_, struct rev_info 
*revs, int flags, unsi
 
if (revarg_opt & REVARG_COMMITTISH)
get_sha1_flags |= GET_OID_COMMITTISH;
+   if (revs && revs->follow_symlinks)
+   get_sha1_flags |= GET_OID_FOLLOW_SYMLINKS;
 
if (get_oid_with_context(arg, get_sha1_flags, , ))
return revs->ignore_missing ? 0 : -1;
diff --git a/revision.h b/revision.h
index b8c47b98e..060f1038a 100644
--- a/revision.h
+++ b/revision.h
@@ -122,6 +122,7 @@ struct rev_info {
first_parent_only:1,
line_level_traverse:1,
tree_blobs_in_commit_order:1,
+   follow_symlinks:1,
 
/* for internal use only */
exclude_promisor_objects:1;
diff --git a/t/t1800-git-show.sh b/t/t1800-git-show.sh
new file mode 100755
index 0..7a02438ec
--- /dev/null
+++ b/t/t1800-git-show.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='Test git show works'
+
+. ./test-lib.sh
+
+test_expect_success 'verify git show HEAD:foo works' '
+   test_commit A &&
+   git show HEAD:A.t >actual &&
+   echo A >expected &&
+   test_cmp expected actual
+'
+
+test_expect_success SYMLINKS 'verify git show HEAD:symlink shows symlink 
points to foo' '
+   ln -s A.t A.link &&
+   git add A.link &&
+   git commit -m"Added symlink to A.t" &&
+   git show HEAD:A.link >actual &&
+   printf "%s" A.t >expected &&
+   test_cmp expected actual
+'
+
+test_expect_success SYMLINKS 'verify git show --follow-symlinks HEAD:symlink 
shows foo' '
+   git show --follow-symlinks HEAD:A.link >actual &&
+   echo A >expected &&
+   test_cmp expected actual
+'
+
+test_expect_success SYMLINKS 'verify git show --follow-symlinks HEAD:symlink 
works 

[PATCH v2] show: add --follow-symlinks option for :

[Michael Vogt]

Updated version of the `git show --follow-symlink` patch. This version
includes the feedback from Ævar Arnfjörð Bjarmason and Stefan Beller:

- commit message updated
- test fixes merged from Ævar (thanks!)
- Documentation/git-show.txt clarified

It does not include a test for --follow-symlinks in the dirlink case
when a file is behind a dirlink symlink. This this currently fails with
a non-descriptive error message. I hope to find time to improve this
error message at some point and then a test for this will be added.

It also does not include a test for a repo with symlinks when running
on a system without SYMLINKS. I don't have access to such a system,
sorry.

Re: Draft of Git Rev News edition 38

[Christian Couder]

On Mon, Apr 16, 2018 at 12:11 AM, Christian Couder
 wrote:
>
> A draft of a new Git Rev News edition is available here:
>
>   
> https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-38.md

The draft has just been updated with 2 articles contributed by Jake
about rebasing merges, so I am cc'ing more people involved in those
discussions.

Thanks Jake!

Re: [PATCH v2 3/6] generate-cmdlist.sh: keep all information in common-cmds.h

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> @@ -23,28 +36,44 @@ sed -n '
>   ' "$1"
>  printf '};\n\n'
>  
> +echo "#define GROUP_NONE 0xff /* no common group */"

Some later code forgets about this value, and causes "git" to
segfault at the end of this entire series.

Namely, here:

> - for (i = 0; i < ARRAY_SIZE(common_cmds); i++) {
> + for (i = 0; i < nr; i++) {
>   if (common_cmds[i].group != current_grp) {
>   printf("\n%s\n", 
> _(common_cmd_groups[common_cmds[i].group]));
>   current_grp = common_cmds[i].group;

where common_cmd_groups[] gets overrun.

Here is a squash I'll queue on top to keep the tip of 'pu' at least
buildable.

 help.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/help.c b/help.c
index a44f4a113e..74591d5ebc 100644
--- a/help.c
+++ b/help.c
@@ -242,7 +242,9 @@ void list_common_cmds_help(void)
puts(_("These are common Git commands used in various situations:"));
 
for (i = 0; i < nr; i++) {
-   if (common_cmds[i].group != current_grp) {
+   if (ARRAY_SIZE(common_cmd_groups) <= common_cmds[i].group)
+   ; /* skip */
+   else if (common_cmds[i].group != current_grp) {
printf("\n%s\n", 
_(common_cmd_groups[common_cmds[i].group]));
current_grp = common_cmds[i].group;
}