Re: [PATCH 2/7] diff.c: take "prefix" argument in diff_opt_parse()

[Duy Nguyen]

On Wed, Jan 20, 2016 at 01:49:21PM -0800, Junio C Hamano wrote:
> Jeff King  writes:
> 
> > On Wed, Jan 20, 2016 at 12:23:38PM -0800, Junio C Hamano wrote:
> >
> >> Nguyễn Thái Ngọc Duy   writes:
> >> 
> >> > This will be important later when diff_opt_parse() accepts paths as
> >> > arguments. Paths must be prefixed before access because setup code
> >> > moves cwd but does not (and cannot) update command line options.
> >> 
> >> The above sounds like a sensible thing to do (note: I didn't read
> >> the patch or remainder of the series), but makes me wonder how the
> >> existing --orderfile option works without this support.  Perhaps it
> >> is not working and needs to be updated to take advantage of this
> >> change, too?
> >
> > Yeah, I think it simply does not work.
> >
> >   $ >main-order
> >   $ mkdir subdir && >subdir/sub-order
> >   $ cd subdir
> >   $ git show -Osub-order
> >   fatal: failed to read orderfile 'sub-order': No such file or directory
> >   $ git show -Omain-order
> >   [shows diff]
> 
> Good.
> 
> Then 2/7 can be rerolled and advertised as "make -O to work from
> subdirectories", and can gradulate regardless of the remainder of
> the series.  Even if the rest needs rerolls to get it right (or
> takes until 2019 to mature ;-), we will have one less change to
> re-review in the process as we can push these early and obviously
> correct part out separately.
> 

I didn't know there was already an option that takes a path. I read
through the function and found another one. So here's the standalone
patch that fixes both.

-- 8< --
Subject: [PATCH] diff: make -O and --output work in subdirectory

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/am.c  |  2 +-
 diff-no-index.c   |  3 ++-
 diff.c| 14 ++
 diff.h|  2 +-
 revision.c|  2 +-
 t/t4056-diff-order.sh |  6 ++
 6 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 9fb42fd..f009b6c 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1657,7 +1657,7 @@ static int fall_back_threeway(const struct am_state 
*state, const char *index_pa
 
init_revisions(_info, NULL);
rev_info.diffopt.output_format = DIFF_FORMAT_NAME_STATUS;
-   diff_opt_parse(_info.diffopt, _filter_str, 1);
+   diff_opt_parse(_info.diffopt, _filter_str, 1, 
rev_info.prefix);
add_pending_sha1(_info, "HEAD", our_tree, 0);
diff_setup_done(_info.diffopt);
run_diff_index(_info, 1);
diff --git a/diff-no-index.c b/diff-no-index.c
index 8e0fd27..aa81a15 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -252,7 +252,8 @@ void diff_no_index(struct rev_info *revs,
else if (!strcmp(argv[i], "--"))
i++;
else {
-   j = diff_opt_parse(>diffopt, argv + i, argc - i);
+   j = diff_opt_parse(>diffopt, argv + i, argc - i,
+  revs->prefix);
if (j <= 0)
die("invalid diff option/value: %s", argv[i]);
i += j;
diff --git a/diff.c b/diff.c
index 80eb0c2..2136b69 100644
--- a/diff.c
+++ b/diff.c
@@ -3693,12 +3693,16 @@ static int parse_ws_error_highlight(struct diff_options 
*opt, const char *arg)
return 1;
 }
 
-int diff_opt_parse(struct diff_options *options, const char **av, int ac)
+int diff_opt_parse(struct diff_options *options,
+  const char **av, int ac, const char *prefix)
 {
const char *arg = av[0];
const char *optarg;
int argcount;
 
+   if (!prefix)
+   prefix = "";
+
/* Output format options */
if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch")
|| opt_arg(arg, 'U', "unified", >context))
@@ -3915,7 +3919,8 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
else if (!strcmp(arg, "--pickaxe-regex"))
options->pickaxe_opts |= DIFF_PICKAXE_REGEX;
else if ((argcount = short_opt('O', av, ))) {
-   options->orderfile = optarg;
+   const char *path = prefix_filename(prefix, strlen(prefix), 
optarg);
+   options->orderfile = xstrdup(path);
return argcount;
}
else if ((argcount = parse_long_opt("diff-filter", av, ))) {
@@ -3954,9 +3959,10 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
else if (!strcmp(arg, "--no-function-context"))
DIFF_OPT_CLR(options, FUNCCONTEXT);
else if ((argcount = parse_long_opt("output", av, ))) {
-   options->file = fopen(optarg, "w");
+   const char *path = prefix_filename(prefix, strlen(prefix), 
optarg);
+   options->file = fopen(path, "w");
if 

jc/rerere-multi (was: What's cooking in git.git (Jan 2016, #04; Wed, 20))

[Johannes Sixt]

Am 21.01.2016 um 00:33 schrieb Junio C Hamano:
> * jc/rerere-multi (2015-09-14) 7 commits
>   - rerere: do use multiple variants
>   - t4200: rerere a merge with two identical conflicts
>   - rerere: allow multiple variants to exist
>   - rerere: delay the recording of preimage
>   - rerere: handle leftover rr-cache/$ID directory and postimage files
>   - rerere: scan $GIT_DIR/rr-cache/$ID when instantiating a rerere_id
>   - rerere: split conflict ID further
> 
>   "git rerere" can encounter two or more files with the same conflict
>   signature that have to be resolved in different ways, but there was
>   no way to record these separate resolutions.
> 
>   Needs review.

I finally found some time to test and review this series. I have one
case where there are many identical conflicts (up to 15!) that rerere
was unable to resolve. But with this series applied, all of them are
now resolved automatically and correctly. That's a nice achievement!

Tested-by: Johannes Sixt 

I don't have the original submission anymore. So, I'm responding here.

Generally, the patches make sense.

Except for 510936082eb4 "handle leftover rr-cache/$ID directory and
postimage files": After the subsequent e2a6344cca47 "delay the
recording of preimage" is in place, nothing of what the former patch
changed (except test cases) remains, and the problem that the former
solved is still solved, and in addition the NEEDSWORK that the former
introduced is resolved by the latter. I think the two should be
squashed together.

e2a6344cca47 (rerere: delay the recording of preimage) needs this
fixup, I think:

diff --git a/rerere.c b/rerere.c
index c0482b8..33b1348 100644
--- a/rerere.c
+++ b/rerere.c
@@ -765,7 +765,7 @@ static void do_rerere_one_path(struct string_list_item 
*rr_item,
const char *path = rerere_path(id, "postimage");
if (unlink(path))
die_errno("cannot unlink stray '%s'", path);
-   id->collection->status &= ~RR_HAS_PREIMAGE;
+   id->collection->status &= ~RR_HAS_POSTIMAGE;
}
id->collection->status |= RR_HAS_PREIMAGE;
fprintf(stderr, "Recorded preimage for '%s'\n", path);

and perhaps this change:

diff --git a/rerere.c b/rerere.c
index fbdade8..df6beb9 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1005,11 +1005,6 @@ static void unlink_rr_item(struct rerere_id *id)
unlink(rerere_path(id, "thisimage"));
unlink(rerere_path(id, "preimage"));
unlink(rerere_path(id, "postimage"));
-   /*
-* NEEDSWORK: what if this rmdir() fails?  Wouldn't we then
-* assume that we already have preimage recorded in
-* do_plain_rerere()?
-*/
rmdir(rerere_path(id, NULL));
 }
 

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

Re: [PATCH 2/5] config.mak.uname: supporting 64-bit MSys2

[Johannes Schindelin]

Hi Gábor,

On Thu, 21 Jan 2016, SZEDER Gábor wrote:

> > This just makes things compile, the test suite needs extra tender
> > loving care in addition to this change. We will address these issues
> > in later commits.

Please note this statement:

> > While at it, also allow building MSys2 Git (i.e. a Git that uses MSys2's
> > POSIX emulation layer).
> > [...]
> > diff --git a/config.mak.uname b/config.mak.uname
> > index b0592c1..4b2e1b8 100644
> > --- a/config.mak.uname
> > +++ b/config.mak.uname
> > @@ -545,8 +544,17 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
> >  else

... and that this here condition triggers only for MSYS2 [*1*], but in all
modes, i.e. 32-bit/64-bit MSYS2/MINGW...

> > ifeq ($(shell expr "$(uname_R)" : '2\.'),2)
> > # MSys2
> > +   prefix = /usr/
> > +   ifeq (MINGW32,$(MSYSTEM))
> > +   prefix = /mingw32
> 
> Here prefix is set for 32 bit MSys2.  OK.

Actually, it is not 32-bit MSYS2 but 32-bit MINGW. Please remember that
MSYS2 programs use the MSYS2 runtime (in essence a portable version of
Cygwin's runtime), i.e. a POSIX emulation layer, while MINGW programs do
not.

As a consequence, MINGW programs are 1) faster, and 2) cannot use POSIX
functionality such as fork().

So yes, this triggers when we are using the 32-bit MINGW environment,
which is just a mode in which you can start MSYS2 (or for that matter, the
Git for Windows SDK), simply by setting the environment variable `MSYSTEM`
accordingly.

The modes are MINGW32, MINGW64 and MSYS. You cannot switch between 32-bit
MSYS2 and 64-bit MSYS2 that way because you can run either with 32-bit
MSYS2 runtime & programs or 64-bit MSYS2 runtime & programs. You can
easily mix & match 32-bit and 64-bit MINGW programs, though.

> > +   endif
> > +   ifeq (MINGW64,$(MSYSTEM))
> > +   prefix = /mingw64
> 
> Here prefix is set for 64 bit MSys2.  Still OK.

Well, 64-bit *MINGW*, not 64-bit *MSYS2*.

> > +   else
> > +   COMPAT_CFLAGS += -D_USE_32BIT_TIME_T
> > +   BASIC_LDFLAGS += -Wl,--large-address-aware
> 
> But then these flags are set for any MSys2 that is not 64 bit, which
> also includes MINGW32, which we've already dealt with above
> explicitly.  Hmm.

Remember the second paragraph of the commit message here, please: we want
MSYS2 Git to be compiled, too. As opposed to MINGW Git. It is slower, yes,
but by being able to use POSIX functionality, it can side-step any bugs in
our Windows-specific code, if any.

And even 64-bit MSYS2's tools can handle those C and LD flags, last time I
checked, it is just 64-bit MINGW's tools that cannot.

Ciao,
Johannes

Footnote *1*: While "MSYS2" suggests that it is a four-letter acronym
rather than a two-letter one (it really only stands for "*M*inimal
*Sys*tem" version 2), at least one of the MSYS2 developers is adamant
about using all-caps. It's a fight I don't need, so I don't fight it.

Re: [PATCH v3 17/20] refs: allow ref backend to be set for clone

[SZEDER Gábor]

Quoting David Turner :


Thanks for the suggestions.

With your permission, I will add:

Signed-off-by: SZEDER Gábor 

to all three of these patches post-squash.  Is that OK?


Sure...

(But does such a trivial one-liner need a sign-off at all, when it's  
not a standalone patch?)




On Fri, 2016-01-15 at 12:32 +0100, SZEDER Gábor wrote:

Hi,

This change is more about 'git clone' than about refs, therefore
I think the subject line would be better as:

  clone: allow setting alternate ref backend

Could you please squash this in to keep the completion script up to
date?
Is there or will there be a way to list available ref backends, so we
could complete possible options for --ref-storage=, too?

-- >8 --

Subject: completion: git clone --ref-storage=
---
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index ab4da7f97917..c970d3c0d0a3 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1092,6 +1092,7 @@ _git_clone ()
--depth
--single-branch
--branch
+   --ref-storage=
"
return
;;




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

Re: git status during interactive rebase

[Johannes Schindelin]

Hi Stefan,

On Wed, 20 Jan 2016, Stefan Beller wrote:

> So I ran an interactive rebase, and while editing
> .git/rebase-merge/git-rebase-todo I tried to run
> `git status` in another terminal to inquire about a
> filename of an untracked file.

Heh, I don't think that anybody did that before, because the rebase has
not even quite started yet...

The cop-out would be to write an empty 'done' file before editing the
todo, but it would give the wrong impression that it is safe to run `git
rebase --continue` now... Interactive rebase is *definitely* not
thread-safe ;-)

So the proper fix might be to test for the presence of the "done" file and
otherwise tell the user that this rebase has not even started yet.

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

git-diff: unable to turn off diff.autorefreshindex with command line switch

[Andrew Stewart]

Hello,

I am seeing unexpected behaviour on my system with git-diff and stat-only 
changes: diff.autorefreshindex=0 only works when in a repo's config 
(./.git/config); it doesn't work via a -c switch.  Conversely -c 
diff.autorefreshindex=1 does indeed override a 0 setting in the repo's config.

# First I remove all my normal configs.
$ mv ~/.git* ~/stuff/

# Now some setup.
$ mkdir foo && cd foo
$ git init
$ echo 123 > README
$ git commit -am 'initial'
[master (root-commit) 92e793c] initial
 ...[stuff about name and email address being auto-configured]...
 1 file changed, 1 insertion(+)
 create mode 100644 README
$ touch README

# Now the strange behaviour: no output from the next command
$ git -c diff.autorefreshindex=0 diff --raw -- README

$ cat .git/config
[core]
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true
ignorecase = true
precomposeunicode = true

$ echo -e "[diff]\n  autorefreshindex = 0" >> .git/config
$ touch README

# Next command works as expected.
$ git diff --raw -- README
:100644 100644 190a180... 000... M  README

# Next command produces no output as expected
$ git -c diff.autorefreshindex=1 diff --raw -- README

I get this with both git v2.6.4 and v2.7.0 on my OS X 10.11.2.  I tried it on 
another system (Ubuntu 12.04 LTS, git 1.7.9.5) and everything worked as 
expected.  Somebody else tried it on their OS X 10.11.2 (as well as 
10.10.something) with git 2.5.4 and everything worked as expected.

Any help would be much appreciated.  Thanks in advance!

Yours,
Andrew Stewart

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

Re: [PATCH 2/5] config.mak.uname: supporting 64-bit MSys2

[SZEDER Gábor]

> This just makes things compile, the test suite needs extra tender loving
> care in addition to this change. We will address these issues in later
> commits.
> 
> While at it, also allow building MSys2 Git (i.e. a Git that uses MSys2's
> POSIX emulation layer).
> 
> Signed-off-by: Johannes Schindelin 
> ---
>  config.mak.uname | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/config.mak.uname b/config.mak.uname
> index b0592c1..4b2e1b8 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -518,13 +518,12 @@ ifneq (,$(findstring MINGW,$(uname_S)))
>   NO_INET_NTOP = YesPlease
>   NO_POSIX_GOODIES = UnfortunatelyYes
>   DEFAULT_HELP_FORMAT = html
> - COMPAT_CFLAGS += -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
> + COMPAT_CFLAGS += -DNOGDI -Icompat -Icompat/win32
>   COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
>   COMPAT_OBJS += compat/mingw.o compat/winansi.o \
>   compat/win32/pthread.o compat/win32/syslog.o \
>   compat/win32/dirent.o
>   BASIC_CFLAGS += -DPROTECT_NTFS_DEFAULT=1
> - BASIC_LDFLAGS += -Wl,--large-address-aware
>   EXTLIBS += -lws2_32
>   GITLIBS += git.res
>   PTHREAD_LIBS =

(On Windows I only used git, and only briefly, and it was back in the
v1.9.x era, so please pardon my ignorance...)

I'm puzzled by the if statements in the hunk below:

> @@ -545,8 +544,17 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
>  else
>   ifeq ($(shell expr "$(uname_R)" : '2\.'),2)
>   # MSys2
> + prefix = /usr/
> + ifeq (MINGW32,$(MSYSTEM))
> + prefix = /mingw32

Here prefix is set for 32 bit MSys2.  OK.

> + endif
> + ifeq (MINGW64,$(MSYSTEM))
> + prefix = /mingw64

Here prefix is set for 64 bit MSys2.  Still OK.

> + else
> + COMPAT_CFLAGS += -D_USE_32BIT_TIME_T
> + BASIC_LDFLAGS += -Wl,--large-address-aware

But then these flags are set for any MSys2 that is not 64 bit, which
also includes MINGW32, which we've already dealt with above
explicitly.  Hmm.

Is this intentional?  It would be easier to follow if the settings
specific to MINGW32 weren't scattered in two discontinuous blocks.
What values can MSYSTEM have here besides MINGW32 and MINGW64?

Thanks,
Gábor

> + endif
>   CC = gcc
> - prefix = /mingw32
>   COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0
>   INSTALL = /bin/install
>   NO_R_TO_GCC_LINKER = YesPlease
> -- 
> 2.6.3.windows.1.300.g1c25e49

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

[PATCH] Consistent usage of working tree in the git-add help

[Lars Vogel]

The usage of working directory is inconsistent in the git add help.
Also http://git-scm.com/docs/git-clone speaks only about working tree.
Remaining entry found by "git grep -B1 '^directory' git-add.txt" really
relates to a directory.

Signed-off-by: Lars Vogel 
---
 Documentation/git-add.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index fe5282f..6a96a66 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -24,7 +24,7 @@ remove paths that do not exist in the working tree anymore.
 
 The "index" holds a snapshot of the content of the working tree, and it
 is this snapshot that is taken as the contents of the next commit.  Thus
-after making any changes to the working directory, and before running
+after making any changes to the working tree, and before running
 the commit command, you must use the `add` command to add any new or
 modified files to the index.
 
-- 
2.7.0

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

Zombie tag

[stefan.naewe]

I'm having trouble to get rid of a deleted tag.
Here's what I did:

- push master branch from a non-bare repo (R1) into a bare repo (B1)
- push a tag (tag-a) from R1 into the same B1
- force-push master from another non-bare repo (R2) into B1
- do 'git push B1 :tag-a' from R2 to delete the tag
Now, in B1, 'git fsck' shows a dangling tag that can be identified
as 'tag-a' from above. There's obviously no reflog in B1.
I don't know how to get rid of that tag.

A cleaned-up 'script' session of the above is attached.

The problem initially occurred on Windows with git 2.7.0 but could
be reproduced on a Linux machine with git 2.7.0.

What's going on here ?

Thanks,
  Stefan
-- 

/dev/random says: Polls show that 9 out of 6 schizophrenics agree.
python -c "print 
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
 
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9  9666 829B 49C5 9221 27AF
~/tmp/zombie-tag ~
$ git init repo1
Initialized empty Git repository in /home/naewe_s/tmp/zombie-tag/repo1/.git/
$ git init repo2
Initialized empty Git repository in /home/naewe_s/tmp/zombie-tag/repo2/.git/
$ git init --bare bare1.git
Initialized empty Git repository in /home/naewe_s/tmp/zombie-tag/bare1.git/
$ cd repo1
$ echo A > A ; git add A ; git commit -m"add A"
[master (root-commit) d9e5baf] add A
 1 file changed, 1 insertion(+)
 create mode 100644 A
$ echo B > B ; git add B ; git commit -m"add B"
[master 3810738] add B
 1 file changed, 1 insertion(+)
 create mode 100644 B
$ git tag -m"initial" initial
$ git push ../bare1.git/ master initial
Counting objects: 7, done.
Delta compression using up to 2 threads.
Compressing objects:  25% (1/4)   
Compressing objects:  50% (2/4)   
Compressing objects:  75% (3/4)   
Compressing objects: 100% (4/4)   
Compressing objects: 100% (4/4), done.
Writing objects:  14% (1/7)   
Writing objects:  28% (2/7)   
Writing objects:  42% (3/7)   
Writing objects:  57% (4/7)   
Writing objects:  71% (5/7)   
Writing objects:  85% (6/7)   
Writing objects: 100% (7/7)   
Writing objects: 100% (7/7), 548 bytes | 0 bytes/s, done.
Total 7 (delta 1), reused 0 (delta 0)
To ../bare1.git
 * [new branch]  master -> master
 * [new tag] initial -> initial
$ cd ../repo2
$ for n in 1 2; do echo $n > $n ; git add $n; git commit -m"add $n"; done
[master (root-commit) e2ebcc4] add 1
 1 file changed, 1 insertion(+)
 create mode 100644 1
[master f8d1071] add 2
 1 file changed, 1 insertion(+)
 create mode 100644 2
$ git push -f ../bare1.git/ master
Counting objects: 6, done.
Delta compression using up to 2 threads.
Compressing objects:  33% (1/3)   
Compressing objects:  66% (2/3)   
Compressing objects: 100% (3/3)   
Compressing objects: 100% (3/3), done.
Writing objects:  16% (1/6)   
Writing objects:  33% (2/6)   
Writing objects:  50% (3/6)   
Writing objects:  66% (4/6)   
Writing objects:  83% (5/6)   
Writing objects: 100% (6/6)   
Writing objects: 100% (6/6), 384 bytes | 0 bytes/s, done.
Total 6 (delta 1), reused 0 (delta 0)
To ../bare1.git
 + 3810738...f8d1071 master -> master (forced update)
$ git push ../bare1.git/ :initial
To ../bare1.git
 - [deleted] initial
$ cd ../bare1.git/
$ git log --oneline
f8d1071 add 2
e2ebcc4 add 1

$ git for-each-ref
f8d10716fc887da36df53f58660bfaa77ff5a411 commit refs/heads/master
$ git fsck
Checking object directories:   5% (13/256)   
Checking object directories:  11% (30/256)   
Checking object directories:  13% (35/256)   
Checking object directories:  22% (57/256)   
Checking object directories:  53% (137/256)   
Checking object directories:  81% (209/256)   
Checking object directories:  83% (213/256)   
Checking object directories:  85% (218/256)   
Checking object directories:  87% (223/256)   
Checking object directories:  88% (227/256)   
Checking object directories:  96% (246/256)   
Checking object directories:  97% (249/256)   
Checking object directories: 100% (256/256)   
Checking object directories: 100% (256/256), done.
dangling tag 88a23db671d20eb6a4384207bf24b8e0c667a288
dangling commit 381073887a81082128b84945fe07c96c400e86da
$ git show  88a23db671d20e
tag initial
Tagger: Stefan Naewe 
Date:   Thu Jan 21 08:51:05 2016 +0100

initial

commit d9e5baf52f472c49fd4fd1faeb6f0752dfe3980b
Author: Stefan Naewe 
Date:   Thu Jan 21 08:50:41 2016 +0100

add A

diff --git a/A b/A
new file mode 100644
index 000..f70f10e
--- /dev/null
+++ b/A
@@ -0,0 +1 @@
+A

$ git reflog

$ git gc
Counting objects: 6, done.
Delta compression using up to 2 threads.
Compressing objects:  33% (1/3)   
Compressing objects:  66% (2/3)   
Compressing objects: 100% (3/3)   
Compressing objects: 100% (3/3), done.
Writing objects:  16% (1/6)   
Writing objects:  33% (2/6)   
Writing objects:  50% (3/6)   
Writing objects:  66% (4/6)   
Writing objects:  83% (5/6)   
Writing objects: 100% (6/6)   
Writing objects: 100% (6/6), 

Re: git status during interactive rebase

[Matthieu Moy]

Stefan Beller  writes:

> So I ran an interactive rebase, and while editing
> .git/rebase-merge/git-rebase-todo I tried to run
> `git status` in another terminal to inquire about a
> filename of an untracked file.
>
> However, I got:
>
> $ git status
> On branch submodule-groups-v3
> fatal: Could not open file .git/rebase-merge/done for reading: No such
> file or directory
>
> Was this behavior always the case? (IIRC it worked a long time ago?)

>From the list of recipients, I guess you already found that the issue
probably comes from 84e6fb9 (status: give more information during rebase
-i, 2015-07-06), which introduced
read_rebase_todolist("rebase-merge/done", _done); in wt-status.c.

> Would it make sense to not abort completely but give a limited status?

Yes.

I guess read_rebase_todolist should be changed:

static void read_rebase_todolist(const char *fname, struct string_list *lines)
{
struct strbuf line = STRBUF_INIT;
FILE *f = fopen(git_path("%s", fname), "r");

if (!f)
die_errno("Could not open file %s for reading",
  git_path("%s", fname));

This should return an empty string_list instead of dying when the file
does not exist. The case of an empty list is already dealt with later:

if (have_done.nr == 0)
status_printf_ln(s, color, _("No commands done."));

No time to work on a patch for now :-(.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Consistent usage of working tree in the git-add help

[Lars Vogel]

Thanks Junio for the feedback. I send a new patch with only the first change.

On Thu, Jan 21, 2016 at 1:24 AM, Junio C Hamano  wrote:
> Lars Vogel  writes:
>
>> The usage of working directory is inconsistent in the git add help.
>> Also http://git-scm.com/docs/giit-clone speaks only about working tree.
>> Remaining entry found by "git grep -B1 '^directory' git-add.txt" really
>> relates to a directory.
>>
>> Signed-off-by: Lars Vogel 
>> ---
>>  Documentation/git-add.txt | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
>> index fe5282f..cfef77b 100644
>> --- a/Documentation/git-add.txt
>> +++ b/Documentation/git-add.txt
>> @@ -24,7 +24,7 @@ remove paths that do not exist in the working tree anymore.
>>
>>  The "index" holds a snapshot of the content of the working tree, and it
>>  is this snapshot that is taken as the contents of the next commit.  Thus
>> -after making any changes to the working directory, and before running
>> +after making any changes to the working tree, and before running
>>  the commit command, you must use the `add` command to add any new or
>>  modified files to the index.
>>
>> @@ -85,7 +85,7 @@ OPTIONS
>>  -p::
>>  --patch::
>>   Interactively choose hunks of patch between the index and the
>> - work tree and add them to the index. This gives the user a chance
>> + working tree and add them to the index. This gives the user a chance
>>   to review the difference before adding modified contents to the
>>   index.
>>  +
>
> Thanks.  While the first hunk looks to me a definite improvement, I
> am lukewarm about s/work tree/working tree/ change.  Both terms are
> used fairly commonly in our documentation set.  "Work tree" has ~70
> hits vs ~350 for "working tree" (ignoring the case where these words
> are split across lines, i.e. "work/working" at the end of the line
> followed by a line that begins with "tree").



-- 
Eclipse Platform UI and e4 project co-lead
CEO vogella GmbH

Haindaalwisch 17a, 22395 Hamburg
Amtsgericht Hamburg: HRB 127058
Geschäftsführer: Lars Vogel, Jennifer Nerlich de Vogel
USt-IdNr.: DE284122352
Fax (040) 5247 6322, Email: lars.vo...@vogella.com, Web: http://www.vogella.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Zombie tag

[stefan.naewe]

Am 21.01.2016 um 09:17 schrieb stefan.na...@atlas-elektronik.com:
> I'm having trouble to get rid of a deleted tag.
> Here's what I did:
> 
> - push master branch from a non-bare repo (R1) into a bare repo (B1)
> - push a tag (tag-a) from R1 into the same B1
> - force-push master from another non-bare repo (R2) into B1
> - do 'git push B1 :tag-a' from R2 to delete the tag
> Now, in B1, 'git fsck' shows a dangling tag that can be identified
> as 'tag-a' from above. There's obviously no reflog in B1.
> I don't know how to get rid of that tag.

Just do 'git gc --prune=all' or 'git prune', stupid!

> A cleaned-up 'script' session of the above is attached.
> 
> The problem initially occurred on Windows with git 2.7.0 but could
> be reproduced on a Linux machine with git 2.7.0.
> 
> What's going on here ?

I guess one never stops learning when using git...

Stefan
-- 

/dev/random says: Socialism is the equal distribution of poverty.
python -c "print 
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
 
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9  9666 829B 49C5 9221 27AF

Re: [PATCH 7/7] merge: add --rename-notes

[Junio C Hamano]

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

> +/*
> + * Traverse through the given notes tree, convert all "path to path"
> + * rename lines into "blob to blob" and return it. If cache_file is
> + * non-NULL, return it's content if still valid. Otherwise save the
> + * new content in it.
> + */
> +static void merge_rename_notes(struct strbuf *cache,
> +struct notes_tree *t, const char *cache_file)
> +{
> + struct object_id notes_oid;
> +
> + if (cache_file) {
> + ...
> + }
> +
> + strbuf_reset(cache);
> + for_each_note(t, 0, merge_rename_note, cache);

This uses _all_ merge notes attached to _any_ commit in the history,
without even checking if the commit is involved in the current merge
being done?  How would that be useful?

I also suspect that the data structure to keep track renames by
using notes needs a better design.  You seem to have a note per
commit and one note records a set of "this goes to that", and
that is the reason why you need to discriminately read everything
under the sun.

I think the index into the notes tree for the purpose of this use
case should not be "which commit records this set of renames?",
but by "what is the destination blob of possible rename
operations?".  IOW, if a path that held blob X was moved to
another path that holds blob Z in commit A, and if a path that
held blob Y was moved to another path that holds blob Z in commit
B, attach a note to blob Z that records "moved from X in A" and
"moved from Y in B".

Then, after diffcore-rename has enumerated the potential rename
destinations, look these destination blobs up in the notes.  You
see a path with blob Z created, and you know if you have removed
path that held X or Y in the potential rename source set, you
found a manually recorded rename.  If you have ancestry
information when you do it, you could even reject "move from X to
Z" when you know commit A is not involved in the ancestry path
involved in the merge, but that is optional (as you may be
comparing two states that may not be related with each other).

For the purpose of helping "git log", a notes tree reorganized in
such a way would be useful.  Again, when you find a potential
rename destination that has blob Z as the result of the change,
you read the note attached to the blob to learn that it may have
come from a path that held blob X (if we are dealing with commit
A), or it may have come from a path that held blob Y (if we are
dealing with commit B).  So you can add a new field in diffopt
structure and allow the caller somewhere in the callchain
(log_tree_diff(), perhaps?) to pass which resulting commit it
wants the recorded renames to be used from.  When it is time for
"git log -M -p" to show commit A, diffcore-rename down in the
callchain will get the diff_queue that contains "diff A^..A",
among which there is a path that used to have blob Z that goes
away, reads the notes for blob Z and notice that commit A moved a
path with blob X that is going away was renamed to it, discarding
the other record for blob Z that talks about the change in commit
B that is not relevant for the purpose of showing commit A.

Hmm?

> @@ -1260,10 +1359,25 @@ int cmd_merge(int argc, const char **argv, const char 
> *prefix)
>   usage_with_options(builtin_merge_usage,
>   builtin_merge_options);
>  
> + if (rename_file && rename_note_ref)
> + die(_("--rename-file and --rename-notes are incompatible"));
> +
>   if (rename_file &&
>   strbuf_read_file(_renames, rename_file, 0) == -1)
>   die(_("unable to read %s"), rename_file);
>  
> + if (rename_note_ref) {
> + struct notes_tree rename_notes;
> + struct strbuf ref = STRBUF_INIT;
> +
> + strbuf_addstr(, rename_note_ref);
> + expand_notes_ref();
> + init_notes(_notes, ref.buf, NULL, 0);
> + strbuf_release();
> + merge_rename_notes(_renames, _notes,
> +git_path("GIT_RENAME_CACHE"));
> + }
> +

These new hunks must move way below the codepath and be run only
after we are certain that we need to do a real merge.  The merge
logic that follows this line tries to handle light-weight special
cases (like an obvious fast-forward and already up-to-date cases)
as early as possible to avoid doing unnecessary things.

This patch (and the earlier one adds strbuf-read-file of the
rename-file) breaks the design by doing these new heavyweight
operations whose result may not be used at all too early, I
think.

If you go the route of reorganizing the rename notes around the
destination blobs, then you wouldn't even be reading everything
here very high in the callchain (you would be doing so only after
diffcore-rename decides that rename notes may be worth reading),
and that would fix this.

>   if (!head_commit) {
>   struct commit *remote_head;
>   /*

Thanks.
--
To 

Re: [PATCH v3 04/15] ref-filter: introduce struct used_atom

[Eric Sunshine]

On Tuesday, January 5, 2016, Karthik Nayak  wrote:
> Introduce the 'used_atom' structure to replace the existing
> implementation of 'used_atom' (which is a list of atoms). This helps
> us parse atoms beforehand and store required details into the
> 'used_atom' for future usage.
>
> Helped-by: Eric Sunshine 
> Signed-off-by: Karthik Nayak 
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -17,7 +17,7 @@
>  typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
>
>  /*
> - * An atom is a valid field atom listed above, possibly prefixed with
> + * An atom is a valid field atom listed below, possibly prefixed with

This particular change should be in patch 3/15 which moved this comment block.

(Yes, it's true that this will make the patch something other than
"pure code movement", but this change keeps the moved block logically
consistent and that it's an appropriate thing to do -- plus it's quite
minor.)

>   * a "*" to denote deref_tag().
>   *
>   * We parse given format string and sort specifiers, and make a list
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] submodule update: Initialize all group-selected submodules by default

[Stefan Beller]

On Wed, Jan 20, 2016 at 8:40 PM, Junio C Hamano  wrote:
> I am not sure what you mean by the above snippet.  With
>
> [submodule "foo"]
> group = default
> [submodule "bar"]
> group = optional
> [submodule "baz"]
> group = optional
> path = sub/baz
>
> it would be nice if you can say "modules in the default group",
> "modules in the default group and the module 'bar'", "modules at
> path sub/baz", etc.
>
> Am I repeating more or less the same thing as you wanted to say with
> the above?  If so, yes, I do think there should be a uniform syntax
> that lets users specify set of modules via different mechanisms.

Yes, we have the same vision here. However I wonder if we really need
syntax to differentiate between groups, names and paths. At least
for groups we can make it a requirement to have the same identifier as
another name or path.

Technically it is possible for names and paths to mix up such as
 [submodule "bar"]
 path = baz
 [submodule "baz"]
 path = bar

such that we would need to have a way to tell apart if the user means
the name or path. This could be solved implicit by some sort of precedence
(If there is a name then take it by name, otherwise take the path.)


>> Of course then the --group option would need to be named
>> differently in git clone and probably the submodule.groups should
>> also be named differently.
>
> I do think --group option is a mistake, as you are only saying
> "please give a name of a group to this option" without hinting how
> the modules in the specified group are to be treated differently, or
> more importantly, the option is about submodules.
>
> Side note: this is a common delusion developers fall into
> thinking that the feature they currently working on is the most
> important thing in the world.

I fell into that trap, sure.

> In the context of "clone", there
> is no reason to expect that "groups of submodules" is any
> special than groups of other things.  For the same reason, I
> think --init= is a mistake, as it is not clear from the
> option name that we are initializing submodules in the context
> of "clone".
>
> So perhaps --init-module.

Why not spell it out completely (--init-submodule)?
Then we don't have to add to the glossary that a module is the same as
a submodule. (Could there be other modules such as co-modules,
which happen to be not in the tree, but in "../sibling" relative to the
root of the repository?)



>
> Once we establish a uniform convention for specifying a group of
> submodules is by giving the names of groups, the "group" ness of the
> option argument becomes less and less important, as that would be
> implicitly known by users.  "clone --init=A" would be more
> understandable than "clone --group=A", as everybody would know A is
> naming a set of submodules either way, but the latter does not say
> what will happen to the chosen modules.

So keep the "groups" in .gitmodules, but ban the name from clone options.
In the .gitmodules file we could also name it "set" just as you used it in that
paragraph.

>
> There could be other ways to specify the modules, and as long as we
> can come up with the "uniform convention for specifying a group of
> submodules", "clone --init-module=$X --init-module=$Y", would be
> understandable by the users when $X specifies the modules by their
> group name and $Y specifies another set of modules by something else,
> e.g. their names or paths to them.

ok.

>
> A strawman.  You can pass (1) the path to a submodule, (2) you
> can pass a colon followed by the name of a submodule, or (3) you
> can pass an asterisk followed by the name of a group.  (1) and
> (2) specifies a single submodule, (3) specifies the submodules
> that belong to the group.  I.e.
>
> $ git clone --init-module='*default' --init-module=sub/module
>
> would be a way to say "clone and then initialize the submodule
> at path sub/module and also those in the default group.
>
> This strawman makes "path" the default way, merely because many
> subcommands of "git submodule" already specify which submodule
> to operate on by taking paths arguments, and '*' prefix as the
> sign to specify by a group, as an asterisk looks like specifying
> multiple things.  ':' is just another prefix that is unlikely to
> be in a pathname.

But it could be in a pathname. Another strawman:

You can pass either (1) the path (2) or the name (3) or the group
of submodules to --init-submodule if there are no collisions in the
three name spaces. When there are namespaces, you need to be
explicit, such as --init-submodule=path:foo:baz.
Note that "path:" is the selector which of the three namespaces
we mean and "foo:baz" is a path asking for pain.



>
> This is merely an illustration of the kind of 

Re: [PATCH v3 01/15] strbuf: introduce strbuf_split_str_omit_term()

[Eric Sunshine]

On Wed, Jan 6, 2016 at 2:27 AM, Karthik Nayak  wrote:
> On Wed, Jan 6, 2016 at 12:54 AM, Junio C Hamano  wrote:
>> Karthik Nayak  writes:
>>>   while (slen) {
>>>   int len = slen;
>>> + const char *end = NULL;
>>>   if (max <= 0 || nr + 1 < max) {
>>> - const char *end = memchr(str, terminator, slen);
>>> + end = memchr(str, terminator, slen);
>>>   if (end)
>>>   len = end - str + 1;
>>>   }
>>>   t = xmalloc(sizeof(struct strbuf));
>>>   strbuf_init(t, len);
>>> - strbuf_add(t, str, len);
>>> + strbuf_add(t, str, len - !!end * !!omit_term);
>>
>> Perhaps using another variable would make it easier to follow?
>> Either using a boolean that tells us that the terminating byte
>> is to be omitted, i.e.
>>
>> int len = slen;
>> int omit = 0;
>> if ( ... we are still splitting ... ) {
>> const char *end = memchr(...);
>> if (end) {
>> len = end - str + 1;
>> omit = !!omit_term;
>> }
>> }
>> strbuf_init(t, len - omit);
>> strbuf_add(t, str, len - omit);
>>
>> or an integer "copylen" that tells us how many bytes to copy, which
>> often is the same as "len" but sometimes different by 1 byte?
>
> This is done based on Eric's suggestion [1]. Although its a little off normal
> convention. I find it small and simple. So I'm okay with either, your 
> suggested
> change or the existing code.

A "copylen" variable would probably result in the clearest code since
it states explicitly what an otherwise opaque expression like (!!end *
!!omit_term) means, thus is easier to reason about.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] submodule update: Initialize all group-selected submodules by default

[Junio C Hamano]

Stefan Beller  writes:

> "submodule.$name.group" to be found in .gitmodules, maybe overwritten in
> .git/config tells for each submodule its memberships of groups.
>
> "submodule.group" should be found in .git/config only, to tell some time
> after cloning which group selection was made, such that we can check
> if new submodules need to be initialized (or even automatically removed).

Ahh, it wasn't clear that was what you were trying to do.  If that
is the case, then ...

> "git submodule update" may initialze and fetch new modules if the
> .gitmodules file changed their view of what "default" is.
>
>>  The name of the operation, i.e. what is to be done to the
>> chosen modules, should be orthogonal, so I do not think you should
>> have "submodule.autoinitgroup" or somesuch.
>
> I agree there.

... I do not agree that the name of "submodule.group" variable
should be neutral to the operation that is done to the groups of
submodules named by the variable at all.

You are recording the criteria to choose a set of submodules there
in the configuration, with a plan to keep doing something to them
(e.g. "they will never be left uninitialized, i.e. initialied if a
new submodule is added to the set").  The name of the configuration
must have something that tells what that "something" is, in order to
(1) hint the purpose of specifying the group selection criteria
there, and (2) allow different selection criteria to keep doning
another kind of something to them and distinguish these two group
selection criteria.

E.g. "submodule.autoInitialize = *default !:foo" may mean all the
modules in the default group (now or added in the future) except the
module with name 'foo' will be initialized as needed, while
"submodule.autoDistim = *optional" may want to be defined to allow
the system to automatically distim (whatever that operation is that
will be added to Git in later versions) the modules in the optional
group.

> I am hoping we can put that in shorter options, such as
>
> clone --init-module=A --init-module=\*B --init-module=/C \
> --remember-init-for-tracking
>
> whereas:
>
> --remember-init-for-tracking: Submodule groups which are given
> to clone will be remembered, such that each invocation of "update"
> will make sure that group is fully there, i.e. new submodules in
> the group will be initialized before updating.

Is there a need for --no-remember-init-for-tracking?  I do not think
it would be useful.

When the upstream adds a new module and defines it to be part of the
default group _after_ you cloned with --init set to 'default', and
you do not need that new module, at that point you can tweak your
submodule.autoInitialize definition to exclude that new module.

Tweaking submodule.autoInitialize definition to contain nothing
after cloning with --init because you do not want the autoinit
criteria kept in the resulting repository is merely a special case
of that.

So I do not think --remember-init-for-tracking is necessary.  Just
make it _always_ on and be done with it, I would say.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] completion: add missing git-rebase options

[John Keeping]

This adds the --no-* variants where those are documented in
git-rebase(1).

Signed-off-by: John Keeping 
---
 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 482ca84..7d6d63e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1685,8 +1685,12 @@ _git_rebase ()
--preserve-merges --stat --no-stat
--committer-date-is-author-date --ignore-date
--ignore-whitespace --whitespace=
-   --autosquash --fork-point --no-fork-point
-   --autostash
+   --autosquash --no-autosquash
+   --fork-point --no-fork-point
+   --autostash --no-autostash
+   --verify --no-verify
+   --keep-empty --root --force-rebase --no-ff
+   --exec
"
 
return
-- 
2.7.0.226.gfe986fe

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

Re: [PATCH 3/4] submodule update: Initialize all group-selected submodules by default

[Junio C Hamano]

Junio C Hamano  writes:

> Ahh, it wasn't clear that was what you were trying to do.  If that
> is the case, then ...
> ...
> So I do not think --remember-init-for-tracking is necessary.  Just
> make it _always_ on and be done with it, I would say.

By the way, I forgot to say that we're basically in agreement on
everything in your message that I didn't touch in my response.  Also
I forgot to say that I am happy to see that we are designing a new
mechanism to name a set of modules in a convenient way so that the
users can do things to them without hassle.

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

Re: [PATCH] unpack-trees: fix accidentally quadratic behavior

[David Turner]

On Thu, 2016-01-21 at 11:51 -0800, Junio C Hamano wrote:
> David Turner  writes:
> 
> > On Wed, 2016-01-20 at 20:58 -0800, Junio C Hamano wrote:
> > > David Turner  writes:
> > > 
> > > > While unpacking trees (e.g. during git checkout), when we hit a
> > > > cache
> > > > entry that's past and outside our path, we cut off iteration.
> > > > 
> > > > This provides about a 45% speedup on git checkout between
> > > > master
> > > > and
> > > > master^2 on Twitter's monorepo.  Speedup in general will
> > > > depend
> > > > on
> > > > repostitory structure, number of changes, and packfile packing
> > > > decisions.
> > > > 
> > > > Signed-off-by: David Turner 
> > > > ---
> > > 
> > > I haven't thought things through, but does this get fooled by the
> > > somewhat strange ordering rules of tree entries (i.e. a subtree
> > > sorts as if its name is suffixed with a '/' in a tree object)?
> > > 
> > > Other than that, I like this.  "We know the list is sorted, and
> > > after seeing this entry we know there is nothing that will match"
> > > is
> > > an obvious optimization that we already use elsewhere.
> > > 
> > > Thanks.
> > 
> > I think this is correct, because we first do the more complicated
> > check
> > (ce_in_traverse_path), and only check the ordering once that has
> > failed.  
> 
> But the patch does this:
> 
> > +   if (info->prev && info->traverse_path) {
> > +   int prefix_cmp = strncmp(ce->name,
> > info->traverse_path, info->pathlen);
> > +   if (prefix_cmp > 0)
> > +   break;
> > +   else if (prefix_cmp == 0 &&
> > +ce_namelen(ce) >= info
> > ->pathlen &&
> > +strcmp(ce->name + info
> > ->pathlen,
> > +info->name.path)
> > > 0) {
> > +   break;
> > +   }
> > +   }
> > continue;
> 
> The first break is correct, but I am not sure about the "else if"
> part.  Shouldn't it be doing something similar to the logic to "keep
> looking" that talks about "t-i", "t" and "t/a" at the end of the
> loop?

Rather than doing more complicated logic, let's just do the first
check; it seems about as fast for our repo, and I think will usually be
so.  does that seem reasonable to you?

> > The tests all pass, so this should be good.
> 
> Please don't ever say that again.  

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

Re: git status during interactive rebase

[Stefan Beller]

On Thu, Jan 21, 2016 at 12:23 AM, Matthieu Moy
 wrote:
> Stefan Beller  writes:
>
>> So I ran an interactive rebase, and while editing
>> .git/rebase-merge/git-rebase-todo I tried to run
>> `git status` in another terminal to inquire about a
>> filename of an untracked file.
>>
>> However, I got:
>>
>> $ git status
>> On branch submodule-groups-v3
>> fatal: Could not open file .git/rebase-merge/done for reading: No such
>> file or directory
>>
>> Was this behavior always the case? (IIRC it worked a long time ago?)
>
> From the list of recipients, I guess you already found that the issue
> probably comes from 84e6fb9 (status: give more information during rebase
> -i, 2015-07-06), which introduced
> read_rebase_todolist("rebase-merge/done", _done); in wt-status.c.

I just remembered you working on git-status specially during rebase,
so I figured you may have a clue here.

> No time to work on a patch for now :-(.

:-(

> The cop-out would be to write an empty 'done' file before editing the
> todo, but it would give the wrong impression that it is safe to run `git
> rebase --continue` now... Interactive rebase is *definitely* not
> thread-safe ;-)

ok. As I did not start the rebase yet, I figured I may still ask a thing. :P

> So the proper fix might be to test for the presence of the "done" file and
> otherwise tell the user that this rebase has not even started yet.

So what Matthieu said?

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

Re: jc/rerere-multi

[Junio C Hamano]

Johannes Sixt  writes:

> I finally found some time to test and review this series. I have one
> case where there are many identical conflicts (up to 15!) that rerere
> was unable to resolve. But with this series applied, all of them are
> now resolved automatically and correctly. That's a nice achievement!
>
> Tested-by: Johannes Sixt 
>
> I don't have the original submission anymore. So, I'm responding here.
>
> Generally, the patches make sense.

Thanks.  As the tip commit says, this is still incomplete in that
"record and replay" part should work reasonably well, but things
like "forget" and "gc" are areas that needs further looking into.

> Except for 510936082eb4 "handle leftover rr-cache/$ID directory and
> postimage files": After the subsequent e2a6344cca47 "delay the
> recording of preimage" is in place, nothing of what the former patch
> changed (except test cases) remains, and the problem that the former
> solved is still solved, and in addition the NEEDSWORK that the former
> introduced is resolved by the latter. I think the two should be
> squashed together.

Yeah, they were originally done as separate patches as I felt 'delay
the recording' step was much riskier and I was shooting for a series
whose earlier parts can be moved forward independently if necessary
in order to keep the size of the really hard part that has to be in
flight longer time manageably small.

But re-reading these two with fresh eyes, I think it is OK to squash
them into one.

> e2a6344cca47 (rerere: delay the recording of preimage) needs this
> fixup, I think:

Thanks for catching this (and this needs to be carried forward to
7/7 as well, I think).  This shows how little testing the new code
has gone through--a test certainly would have prepared an entry in
the rerere database with a stale postimage without the matching
preimage and noticed that things going wrong.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 2/2] submodule: port init from shell to C

[Stefan Beller]

By having the `init` functionality in C, we can reference it easier
from other parts in the code.

Signed-off-by: Stefan Beller 
---

Hi Junio,

please replace the top-most patch of sb/submodule-init with this patch.

This will print to stdout instead of stderr, as it's the most
faithful shell -> translation. The s/printf/fprintf(stderr, / patch
will come with the series that needs the behavior, i.e. as one of the
first patches in the groups series (that is under heavy mental
construction currently).

Thanks,
Stefan

Interdiff to the top of sb/submodule-init (6a514f75c25, submodule: port
init from shell to C, Committed: 2016-01-20 12:55:30):

#diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
#index 4684f16..6323048 100644
#--- a/builtin/submodule--helper.c
#+++ b/builtin/submodule--helper.c
#@@ -232,7 +232,7 @@ static void init_submodule(const char *path, const 
char *prefix, int quiet)
#struct strbuf sb = STRBUF_INIT;
#char *url = NULL;
#const char *upd = NULL;
#-   const char *displaypath = relative_path(xgetcwd(), prefix, 
);;
#+   const char *displaypath = relative_path(xgetcwd(), prefix, 
);
# 
#/* Only loads from .gitmodules, no overlay with .git/config */
#gitmodules_config();
#@@ -276,7 +276,7 @@ static void init_submodule(const char *path, const 
char *prefix, int quiet)
#die(_("Failed to register url for submodule 
path '%s'"),
#displaypath);
#if (!quiet)
#-   fprintf(stderr, _("Submodule '%s' (%s) 
registered for path '%s'\n"),
#+   printf(_("Submodule '%s' (%s) registered for 
path '%s'\n"),
#sub->name, url, displaypath);
#free(url);
#}


 builtin/submodule--helper.c | 115 ++--
 git-submodule.sh|  39 +--
 2 files changed, 111 insertions(+), 43 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 1484b36..6323048 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -221,6 +221,115 @@ static int resolve_relative_url_test(int argc, const char 
**argv, const char *pr
return 0;
 }
 
+static int git_submodule_config(const char *var, const char *value, void *cb)
+{
+   return parse_submodule_config_option(var, value);
+}
+
+static void init_submodule(const char *path, const char *prefix, int quiet)
+{
+   const struct submodule *sub;
+   struct strbuf sb = STRBUF_INIT;
+   char *url = NULL;
+   const char *upd = NULL;
+   const char *displaypath = relative_path(xgetcwd(), prefix, );
+
+   /* Only loads from .gitmodules, no overlay with .git/config */
+   gitmodules_config();
+
+   sub = submodule_from_path(null_sha1, path);
+
+   /*
+* Copy url setting when it is not set yet.
+* To look up the url in .git/config, we must not fall back to
+* .gitmodules, so look it up directly.
+*/
+   strbuf_reset();
+   strbuf_addf(, "submodule.%s.url", sub->name);
+   if (git_config_get_string(sb.buf, )) {
+   url = xstrdup(sub->url);
+
+   if (!url)
+   die(_("No url found for submodule path '%s' in 
.gitmodules"),
+   displaypath);
+
+   /* Possibly a url relative to parent */
+   if (starts_with_dot_dot_slash(url) ||
+   starts_with_dot_slash(url)) {
+   char *remoteurl;
+   char *remote = get_default_remote();
+   struct strbuf remotesb = STRBUF_INIT;
+   strbuf_addf(, "remote.%s.url", remote);
+   free(remote);
+
+   if (git_config_get_string(remotesb.buf, ))
+   /*
+* The repository is its own
+* authoritative upstream
+*/
+   remoteurl = xgetcwd();
+   url = relative_url(remoteurl, url, NULL);
+   strbuf_release();
+   }
+
+   if (git_config_set(sb.buf, url))
+   die(_("Failed to register url for submodule path '%s'"),
+   displaypath);
+   if (!quiet)
+   printf(_("Submodule '%s' (%s) registered for path 
'%s'\n"),
+   sub->name, url, displaypath);
+   free(url);
+   }
+
+   /* Copy "update" setting when it is not set yet */
+   strbuf_reset();
+   strbuf_addf(, "submodule.%s.update", sub->name);
+ 

Re: [PATCH 0/4] Submodule Groups

[Junio C Hamano]

Stefan Beller  writes:

> I think having both is bad as it may contradict each other?
> What is supposed to happen here:
>
>  [submodule "frotz"]
>  group = default
>
>  [submoduleGroup "default"]
>  member = !:frotz

What is supposed to happen is that you will write code to diagnose
this as an error, and give helpful error message to the user.

> So groups of groups, we discovered recursion today. :)
> Having this discussion makes me realize, the groups handling logic
> will be more complex than I anticipated for the first RFC patch series.

That is why I think we do not have to have a very complex group
logic from day one.

> The two basic questions the logic has to answer is:
>  * Give me all the submodules that fit these specifiers (i.e. the
> --init-submodules collection from clone)
>  * Given submodule X, does it fit the specifier ( a new uninitialized
> submodule during a later update)

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

Re: [PATCH 2/7] diff.c: take "prefix" argument in diff_opt_parse()

[Junio C Hamano]

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

Re: [PATCH 0/4] Submodule Groups

[Stefan Beller]

On Thu, Jan 21, 2016 at 1:17 PM, Sebastian Schuberth
 wrote:
> On 20.01.2016 04:34, Stefan Beller wrote:
>
>> So you could have a .gitmodules file such as:
>>
>> [submodule "gcc"]
>>  path = gcc
>>  url = git://...
>>  groups = default
>>  groups = devel
>
> On the quick I was unable to find the rationale why entries are now stored as 
> separated lines compared to v1. I liked the comma-separated approach better 
> as it's more compact.

IIUC the line oriented way is preferred as it is our standard. Do we
have any other options stored as a comma separated list?

>
> Anyway, if it's only one group per line, I'd find it more fitting to call the 
> entry "group" instead of "groups" as it will always refer to a single group 
> only. Also that would better match the "--group" command line option naming 
> for "submodule add".

Makes sense to use singular then. However per discussion with Junio in
[PATCH 3/4] submodule update: Initialize all group-selected submodules
by default, we want to not name it "group", as it's unclear what a group is
supposed to mean. What does a group do? which operations are supported?

So for git clone, we'd rather use "--init-submodule" which can be passed a
name, path or group.

For storing that selection we'd go with "submodule.autoInitialize" in
.git /config.

The third user visible place submodule.$NAME.group however can stay in that
variable name as to point out the the concept of a submodule set/collection.
The groups concept may be used in the future for more than initialzing
submodules.

Instead of having a submodule -> set assignment, we could do it the
other way round:

 [submodule "gcc"]
 ...

 [submodule-set "default"]
submodule = gcc
submodule = foo
submodule = by/path/*

This may be more handy from our perspective (while designing it and
writing the code),
but I'd assume this is less useful for the user. How often does a user ask:
"How many/Which submodules are in $GROUP" as opposed to "What about
submodule foo,
is that part of group $GROUP?"

>
> However, if I'd read the single line "group = default" in a .gitmodules file, 
> it wouldn't be immediately clear to me that "group" can appear multiple times 
> per submodule. "groups = default" would me more hinting is this regard 
> because the plural is used, but without reading the docs I'd assume multiple 
> groups would be specified like "groups = default,devel".
>
> Long story short, my personal favorite still would be
>
> [submodule "gcc"]
>  groups = default,devel
>
> followed by
>
> [submodule "gcc"]
>  group = default
>  group = devel

As asked above, how many comma separated things do we have in git configs?
I'd really not want to add more mental complexity to Git. As far as I
remember we have
rather double configs than one long line separated somehow.
(The only thing that comes to mind is multiple remote urls for pushing)

Thanks,
Stefan

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

Re: [PATCH 0/4] Submodule Groups

[Stefan Beller]

On Thu, Jan 21, 2016 at 2:18 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> Instead of having a submodule -> set assignment, we could do it the
>> other way round:
>>
>>  [submodule "gcc"]
>>  ...
>>
>>  [submodule-set "default"]
>> submodule = gcc
>> submodule = foo
>> submodule = by/path/*
>>
>> This may be more handy from our perspective (while designing it and
>> writing the code),
>> but I'd assume this is less useful for the user. How often does a user ask:
>> "How many/Which submodules are in $GROUP" as opposed to "What about
>> submodule foo,
>> is that part of group $GROUP?"
>
> I suspect that we will end up needing to support both styles.

I think having both is bad as it may contradict each other?
What is supposed to happen here:

 [submodule "frotz"]
 group = default

 [submoduleGroup "default"]
 member = !:frotz

Initially I disliked your proposal of : and * to indicate name and groups,
but the example you gave is very clear and understandable on the syntax level.

>  The
> latter style is easier when you want to express a larger set as a
> collection of groups, e.g.
>
> [submodule "gcc"]
> group = development-tools
>
> [submodule "emacs"]
> group = editors
>
> [submodule "frotz"]
> group = games
>
> [submoduleGroup "default"]
> member = *development-tools
> member = *editors
> member = :frotz
> member = !*xyzzy
>
> might be a way to say "the default group includes everything in the
> 'development-tools' or 'editors' group, plus 'frotz' module, but do
> not include modules in the xyzzy group" or something like that.

> I forgot to say that I personally do not think we need to support
> such synthetic groups from day one.

So groups of groups, we discovered recursion today. :)
Having this discussion makes me realize, the groups handling logic
will be more complex than I anticipated for the first RFC patch series.

The two basic questions the logic has to answer is:
 * Give me all the submodules that fit these specifiers (i.e. the
--init-submodules collection from clone)
 * Given submodule X, does it fit the specifier ( a new uninitialized
submodule during a later update)

The second question can be answered by answering the first question
and then checking if X is in the set. However that may be not the most
efficient solution.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] unpack-trees: fix accidentally quadratic behavior

[David Turner]

On Thu, 2016-01-21 at 16:30 -0500, Jeff King wrote:
> On Thu, Jan 21, 2016 at 04:11:48PM -0500, David Turner wrote:
> 
> > While unpacking trees (e.g. during git checkout), when we hit a
> > cache
> > entry that's past and outside our path, we cut off iteration.
> > 
> > This provides about a 45% speedup on git checkout between master
> > and
> > master^2 on Twitter's monorepo.  Speedup in general will depend
> > on
> > repostitory structure, number of changes, and packfile packing
> > decisions.
> 
> I feel like I'm missing the explanation of the quadratic part. From
> looking at the patch, my guess is:
> 
>   1. We're doing a linear walk in a data structure (a "struct
>  index_state").
> 
>   2. For each element, we look it up in another structure
>  ("struct traverse_info") with a linear search.
> 
>  That leaves us at O(m*n), but if we assume both are on the same
>  order of magnitude, that's quadratic.

No, I think, it's the opposite order: we're doing a linear walk over
the incoming tree and for each entry, we're calling find_cache_pos.
find_cache_pos was doing a linear walk over struct index_state.  But
the same algorithmic complexity holds.

>   3. The fix works by knowing that once a lookup in (2) fails once,
> it's
>  likely to fail for all the remainder, and we short-cut that case
>  and skip out of (1) completely.
> 
> But that makes me wonder. Aren't we still quadratic in the case that
> ce_in_traverse_path() returns true? 

I think that doesn't happen very often, because it requires that the
paths match up.  

> If so, would we benefit from either:
> 
>   a. Improving the complexity of ce_in_traverse_path, to say O(log
> n),
>  which would give us O(n log n) for the whole operation in all
>  cases?
> 
>   b. If both lists are already sorted, maybe doing a list-merge to
>  compare them in O(2n) time?

(b) appears to be now (roughly) what we're now doing.

> I'm fairly ignorant of this part of the code, so there's probably a
> good
> reason why my suggestion is unworkable.

I am also quite ignorant of this part of the code; I just looked at
perf and did some simple counting.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] unpack-trees: fix accidentally quadratic behavior

[Junio C Hamano]

David Turner  writes:

>> The first break is correct, but I am not sure about the "else if"
>> part.  Shouldn't it be doing something similar to the logic to "keep
>> looking" that talks about "t-i", "t" and "t/a" at the end of the
>> loop?
>
> Rather than doing more complicated logic, let's just do the first
> check; it seems about as fast for our repo, and I think will usually be
> so.  does that seem reasonable to you?

Very reasonable.  I was mostly afraid of leaving the loop
prematurely and giving a wrong answer to the caller.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] unpack-trees: fix accidentally quadratic behavior

[David Turner]

While unpacking trees (e.g. during git checkout), when we hit a cache
entry that's past and outside our path, we cut off iteration.

This provides about a 45% speedup on git checkout between master and
master^2 on Twitter's monorepo.  Speedup in general will depend on
repostitory structure, number of changes, and packfile packing
decisions.

Signed-off-by: David Turner 
---
 unpack-trees.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 5f541c2..d8e9685 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -695,8 +695,19 @@ static int find_cache_pos(struct traverse_info *info,
++o->cache_bottom;
continue;
}
-   if (!ce_in_traverse_path(ce, info))
+   if (!ce_in_traverse_path(ce, info)) {
+   /*
+* Check if we can skip future cache checks
+* (because we're already past all possible
+* entries in the traverse path).
+*/
+   if (info->prev && info->traverse_path) {
+   if (strncmp(ce->name, info->traverse_path,
+   info->pathlen) > 0)
+   break;
+   }
continue;
+   }
ce_name = ce->name + pfxlen;
ce_slash = strchr(ce_name, '/');
if (ce_slash)
-- 
2.4.2.749.g730654d-twtrsrc

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

Re: [PATCH 0/4] Submodule Groups

[Sebastian Schuberth]

On 20.01.2016 04:34, Stefan Beller wrote:

> So you could have a .gitmodules file such as:
> 
> [submodule "gcc"]
>  path = gcc
>  url = git://...
>  groups = default
>  groups = devel

On the quick I was unable to find the rationale why entries are now stored as 
separated lines compared to v1. I liked the comma-separated approach better as 
it's more compact.

Anyway, if it's only one group per line, I'd find it more fitting to call the 
entry "group" instead of "groups" as it will always refer to a single group 
only. Also that would better match the "--group" command line option naming for 
"submodule add".

However, if I'd read the single line "group = default" in a .gitmodules file, 
it wouldn't be immediately clear to me that "group" can appear multiple times 
per submodule. "groups = default" would me more hinting is this regard because 
the plural is used, but without reading the docs I'd assume multiple groups 
would be specified like "groups = default,devel".

Long story short, my personal favorite still would be 

[submodule "gcc"]
 groups = default,devel

followed by

[submodule "gcc"]
 group = default
 group = devel

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

Re: [PATCH v2] unpack-trees: fix accidentally quadratic behavior

[Jeff King]

On Thu, Jan 21, 2016 at 04:11:48PM -0500, David Turner wrote:

> While unpacking trees (e.g. during git checkout), when we hit a cache
> entry that's past and outside our path, we cut off iteration.
> 
> This provides about a 45% speedup on git checkout between master and
> master^2 on Twitter's monorepo.  Speedup in general will depend on
> repostitory structure, number of changes, and packfile packing
> decisions.

I feel like I'm missing the explanation of the quadratic part. From
looking at the patch, my guess is:

  1. We're doing a linear walk in a data structure (a "struct
 index_state").

  2. For each element, we look it up in another structure
 ("struct traverse_info") with a linear search.

 That leaves us at O(m*n), but if we assume both are on the same
 order of magnitude, that's quadratic.

  3. The fix works by knowing that once a lookup in (2) fails once, it's
 likely to fail for all the remainder, and we short-cut that case
 and skip out of (1) completely.

But that makes me wonder. Aren't we still quadratic in the case that
ce_in_traverse_path() returns true? If so, would we benefit from either:

  a. Improving the complexity of ce_in_traverse_path, to say O(log n),
 which would give us O(n log n) for the whole operation in all
 cases?

  b. If both lists are already sorted, maybe doing a list-merge to
 compare them in O(2n) time?

I'm fairly ignorant of this part of the code, so there's probably a good
reason why my suggestion is unworkable.

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

Re: git-diff: unable to turn off diff.autorefreshindex with command line switch

[Jeff King]

On Thu, Jan 21, 2016 at 10:29:49AM +, Andrew Stewart wrote:

> # Now the strange behaviour: no output from the next command
> $ git -c diff.autorefreshindex=0 diff --raw -- README

I can't reproduce here (v2.7.0, Linux).

But note that your whole test is going to be racy, and possibly depend
on the resolution of your filesystem's timestamps. Did you run your test
as a single script, where the initial index write and the `touch` may
happen in the same second? If so, trying running it slowly by hand,
or inserting "sleep 1" between the commands.

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

Re: [PATCH 0/4] Submodule Groups

[Junio C Hamano]

Stefan Beller  writes:

> Instead of having a submodule -> set assignment, we could do it the
> other way round:
>
>  [submodule "gcc"]
>  ...
>
>  [submodule-set "default"]
> submodule = gcc
> submodule = foo
> submodule = by/path/*
>
> This may be more handy from our perspective (while designing it and
> writing the code),
> but I'd assume this is less useful for the user. How often does a user ask:
> "How many/Which submodules are in $GROUP" as opposed to "What about
> submodule foo,
> is that part of group $GROUP?"

I suspect that we will end up needing to support both styles.  The
latter style is easier when you want to express a larger set as a
collection of groups, e.g.

[submodule "gcc"]
group = development-tools

[submodule "emacs"]
group = editors

[submodule "frotz"]
group = games

[submoduleGroup "default"]
member = *development-tools
member = *editors
member = :frotz
member = !*xyzzy

might be a way to say "the default group includes everything in the
'development-tools' or 'editors' group, plus 'frotz' module, but do
not include modules in the xyzzy group" or something like that.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] Submodule Groups

[Junio C Hamano]

Junio C Hamano  writes:

> I suspect that we will end up needing to support both styles.  The
> latter style is easier when you want to express a larger set as a
> collection of groups, e.g.
> ...
> might be a way to say "the default group includes everything in the
> 'development-tools' or 'editors' group, plus 'frotz' module, but do
> not include modules in the xyzzy group" or something like that.

I forgot to say that I personally do not think we need to support
such synthetic groups from day one.


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

Re: git status during interactive rebase

[Junio C Hamano]

Stefan Beller  writes:

>> So the proper fix might be to test for the presence of the "done" file and
>> otherwise tell the user that this rebase has not even started yet.
>
> So what Matthieu said?

Yup, I think that is the right thing to do.

Perhaps something along this line (not tested and done on 'pu', so I
am not committing it to anywhere myself)?

 wt-status.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index ab4f80d..90b2474 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1068,14 +1068,17 @@ static void abbrev_sha1_in_line(struct strbuf *line)
 
 }
 
-static void read_rebase_todolist(const char *fname, struct string_list *lines)
+static int read_rebase_todolist(const char *fname, struct string_list *lines)
 {
struct strbuf line = STRBUF_INIT;
FILE *f = fopen(git_path("%s", fname), "r");
 
-   if (!f)
+   if (!f) {
+   if (errno == ENOENT)
+   return -1;
die_errno("Could not open file %s for reading",
  git_path("%s", fname));
+   }
while (!strbuf_getline_lf(, f)) {
if (line.len && line.buf[0] == comment_line_char)
continue;
@@ -1085,6 +1088,7 @@ static void read_rebase_todolist(const char *fname, 
struct string_list *lines)
abbrev_sha1_in_line();
string_list_append(lines, line.buf);
}
+   return 0;
 }
 
 static void show_rebase_information(struct wt_status *s,
@@ -1098,12 +1102,12 @@ static void show_rebase_information(struct wt_status *s,
struct string_list have_done = STRING_LIST_INIT_DUP;
struct string_list yet_to_do = STRING_LIST_INIT_DUP;
 
-   read_rebase_todolist("rebase-merge/done", _done);
-   read_rebase_todolist("rebase-merge/git-rebase-todo", 
_to_do);
-
-   if (have_done.nr == 0)
+   if ((read_rebase_todolist("rebase-merge/done", _done) < 0) 
||
+   (read_rebase_todolist("rebase-merge/git-rebase-todo", 
_to_do) < 0)) {
+   status_printf_ln(s, color, _("rebase-i not started 
yet."));
+   } else if (have_done.nr == 0) {
status_printf_ln(s, color, _("No commands done."));
-   else {
+   } else {
status_printf_ln(s, color,
Q_("Last command done (%d command done):",
"Last commands done (%d commands 
done):",
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] travis-ci: run previously failed tests first, then slowest to fastest

[brian m. carlson]

On Wed, Jan 20, 2016 at 10:22:16AM +0100, Lars Schneider wrote:
> I tested different settings and found that running prove with "-j5" seems to 
> be
> the fastest option for the Travis CI machines. However, I also noticed that
> I got more test failures with higher parallelism (Dscho reported similar
> observations [1]).
> 
> Especially t0025-crlf-auto.sh failed multiple times ([2], [3]) on the OS X 
> builds
> when I increase the parallelism:
> 
> not ok 4 - text=true causes a CRLF file to be normalized
> not ok 9 - text=auto, autocrlf=true _does_ normalize CRLF files
> 
> Anyone an idea why that might be the case?

I've seen this on my personal box too[0] when running make -j4 all test.
I wasn't able to pin down why it was occurring, but if we're going to
run the tests in parallel, it's probably worth spending some time
figuring it out.

[0] Debian amd64/sid, ThinkPad X220.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: PGP signature

Re: [PATCH 4/7] log: add --rename-notes to correct renames per commit

[Duy Nguyen]

On Thu, Jan 21, 2016 at 6:29 AM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>> For simplicity, the note of commit A implies rename correction between
>> A^ and A. If parents are manipulated (e.g. "git log --reflog") then
>> the rename output may surprise people.
>
> I do not think "git log --reflog" attempts to show changes to bring
> the tree of @{2} into the shape of @{1}, even though for traversal
> purposes it pretends as if @{1}'s parent is @{2}.  So I am not sure
> what you are trying to say in the above sentence.

Hazy memory about hierarchy manipulation by reflog, maybe I'm wrong.

> A path limited "git log -- path1/ path2/..." also manipulates the
> commit->parents for traversal purposes, but I think the patch is
> still produced against the true parents (there is a call to
> get_saved_parents() in log_tree_diff() that shows the change for a
> given commit), and in that context, commit A that has notes about
> the change to bring the tree of commit A^ to its tree still makes
> sense.
>
> I'd be more worried about "git log -m -p"--the diff against the
> second and subsequent parents would not want to use the notes that
> describes the change between the first parent and the resulting
> merge.

I "fix" this by providing the tools and let the user go wild. The line
"A => B" says rename A to B for a diff between any source or target.
But they can also say rename A to B only if diff source is X, or diff
target is Y, or both. The syntax I'm having for that is something like
this

.source X # X can be anything that resolves to a tree SHA-1 with get_sha1()
.target Y
A => B
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation: remove unnecessary backslashes

[brian m. carlson]

On Wed, Jan 20, 2016 at 03:34:30PM -0500, Jeff King wrote:
> On Wed, Jan 20, 2016 at 12:28:53PM -0800, Junio C Hamano wrote:
> > On the other hand, if this line must be spelled like the above to
> > please asciidoctor, i.e. the first and the last must not have
> > backslashes and the second must have backslashes, I'd have to say
> > we have a bigger problem.  Perhaps asciidoctor needs to be fixed
> > until normal people like we can rely on it.
> 
> Yeah, that is the "insane" part I mentioned. It _does_ make sense
> syntactically ("-1" cannot possibly be an attribute name, so it does not
> parse as one), but I do not like the degree to which writers must know
> all of the arcane syntax rules (and cannot rely on something simple like
> "{ is special, so I must escape it, and over-escaping is not a
> problem").

The underlying issue is that both AsciiDoc and Asciidoctor use regexps
to parse their data, which we all know is a bad idea.  Asciidoctor does
less forward looking because it's much faster, so it's a bit less
flexible with overescaping.

There are plans for Asciidoctor to move to a defined grammar at some
point, which should hopefully make things a bit less insane.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: PGP signature

Re: [PATCH] travis-ci: run previously failed tests first, then slowest to fastest

[Jeff King]

On Fri, Jan 22, 2016 at 12:52:55AM -0500, Jeff King wrote:

> I get a few of the threads failing (in test 4) after 2-3 minutes. The
> "-v" output is pretty unenlightening, though. I don't see anything
> racy-looking in the test unless it is something with "read-tree" and
> stat mtimes.

And indeed, doing this:

diff --git a/t/t0025-crlf-auto.sh b/t/t0025-crlf-auto.sh
index c164b46..d34775b 100755
--- a/t/t0025-crlf-auto.sh
+++ b/t/t0025-crlf-auto.sh
@@ -56,6 +56,7 @@ test_expect_success 'text=true causes a CRLF file to be 
normalized' '
 
rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
echo "CRLFonly text" > .gitattributes &&
+   sleep 1 &&
git read-tree --reset -u HEAD &&
 
# Note, "normalized" means that git will normalize it if added

let me run for over 5 minutes with no failures in test 4 (I eventually
did hit one in test 9). I don't claim to understand what is going on,
though.

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

Re: [PATCH] unpack-trees: fix accidentally quadratic behavior

[David Turner]

On Wed, 2016-01-20 at 20:58 -0800, Junio C Hamano wrote:
> David Turner  writes:
> 
> > While unpacking trees (e.g. during git checkout), when we hit a
> > cache
> > entry that's past and outside our path, we cut off iteration.
> > 
> > This provides about a 45% speedup on git checkout between master
> > and
> > master^2 on Twitter's monorepo.  Speedup in general will depend
> > on
> > repostitory structure, number of changes, and packfile packing
> > decisions.
> > 
> > Signed-off-by: David Turner 
> > ---
> 
> I haven't thought things through, but does this get fooled by the
> somewhat strange ordering rules of tree entries (i.e. a subtree
> sorts as if its name is suffixed with a '/' in a tree object)?
> 
> Other than that, I like this.  "We know the list is sorted, and
> after seeing this entry we know there is nothing that will match" is
> an obvious optimization that we already use elsewhere.
> 
> Thanks.

I think this is correct, because we first do the more complicated check
(ce_in_traverse_path), and only check the ordering once that has
failed.  The tests all pass, so this should be good.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] unpack-trees: fix accidentally quadratic behavior

[Junio C Hamano]

David Turner  writes:

> On Wed, 2016-01-20 at 20:58 -0800, Junio C Hamano wrote:
>> David Turner  writes:
>> 
>> > While unpacking trees (e.g. during git checkout), when we hit a
>> > cache
>> > entry that's past and outside our path, we cut off iteration.
>> > 
>> > This provides about a 45% speedup on git checkout between master
>> > and
>> > master^2 on Twitter's monorepo.  Speedup in general will depend
>> > on
>> > repostitory structure, number of changes, and packfile packing
>> > decisions.
>> > 
>> > Signed-off-by: David Turner 
>> > ---
>> 
>> I haven't thought things through, but does this get fooled by the
>> somewhat strange ordering rules of tree entries (i.e. a subtree
>> sorts as if its name is suffixed with a '/' in a tree object)?
>> 
>> Other than that, I like this.  "We know the list is sorted, and
>> after seeing this entry we know there is nothing that will match" is
>> an obvious optimization that we already use elsewhere.
>> 
>> Thanks.
>
> I think this is correct, because we first do the more complicated check
> (ce_in_traverse_path), and only check the ordering once that has
> failed.  

But the patch does this:

> + if (info->prev && info->traverse_path) {
> + int prefix_cmp = strncmp(ce->name, 
> info->traverse_path, info->pathlen);
> + if (prefix_cmp > 0)
> + break;
> + else if (prefix_cmp == 0 &&
> +  ce_namelen(ce) >= info->pathlen &&
> +  strcmp(ce->name + info->pathlen,
> +  info->name.path) > 0) {
> + break;
> + }
> + }
>   continue;

The first break is correct, but I am not sure about the "else if"
part.  Shouldn't it be doing something similar to the logic to "keep
looking" that talks about "t-i", "t" and "t/a" at the end of the
loop?

> The tests all pass, so this should be good.

Please don't ever say that again.  The existing tests do not cover
all corner cases, and they fundamentally cannot cover the corner
cases future changes may introduce.

Saying "Even test X breaks, so it is clearly broken.  Why did you
send it out without testing?" is OK, though.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html