Re: [PATCH v6] Makefile: replace perl/Makefile.PL with simple make rules

[Alex Riesen]

Ævar Arnfjörð Bjarmason, Wed, Dec 20, 2017 19:24:19 +0100:
> diff --git a/INSTALL b/INSTALL
> index ffb071e9f0..808e07b659 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -84,9 +84,24 @@ Issues of note:
>  
>   GIT_EXEC_PATH=`pwd`
>   PATH=`pwd`:$PATH
> - GITPERLLIB=`pwd`/perl/blib/lib
> + GITPERLLIB=`pwd`/perl/build/lib
>   export GIT_EXEC_PATH PATH GITPERLLIB

Sincerely sorry for off-topic, I just wonder why this section of INSTALL
uses the backticks for command substitution. Is there a shell which does not
support the alternative form $(...) but has all the rest of the used syntax?


---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus

Re: Usability outrage as far as I am concerned

[Jacob Keller]

On Wed, Dec 20, 2017 at 8:40 AM, Cristian Achim  wrote:
> All I will do is paste the stackoverflow question below. It covers the
> commands I made in chronological order and the way I would have
> expected git to behave differently.
>
> So I did
>
> git pull home_subfolder
>

This by itself should be attempting to pull from a remote named
home_subfolder. Can you show the output of "git remote" and also
clearly explain with details the layout of what the folders are and
what is or is not a repository? I can't really make heads or tails of
your problem without more details.

> I do not want to resolve merge conflicts because git doesn't error out
> with a sane message of the situation when it should. I just want to
> get the home_subfolder and usb_subfolder repositories state to where
> it was before my breakage generating git pull and git clone commands.
>
> Ubuntu 14.04 64 bit, git 1.9.1.
>
> Thank you for your time.

So you have home_subfolder and usb_subfolder which are repositories?
Are they submodules within a parent project? Are they simply folders?
Were you trying to merge the two together?

Without knowing more of what you intended to do, vs what you did, and
without more information about the setup it's not really possible to
understand the situation or what broke.

>From the looks of it, I think you might have tried to do something
like try to pull from usb_subfolder directly into the home_subfolder.

Are these two repositories nested? ie: you have "home_subfolder" as
"/home/subfolder" and you have usb_subfolder as
"/home/subfolder/usb/subfolder"?

That might explain why git pull would interpret such a command as a
path from which to do a file based pull...

Thanks,
Jake

Re: [PATCH 4/6] fsmonitor: Add a trailing newline to test-dump-fsmonitor

[Alex Vandiver]

On Tue, 19 Dec 2017, Junio C Hamano wrote:
> That (and existing) uses of printf() all feel a bit overkill ;-)
> Perhaps putchar() would suffice.
> 
> I am not sure if the above wants to become something like
> 
>   for (i = 0; i < istate->cache_nr; i++) {
>   putchar(istate->cache[i]->ce_flags & CE_FSMONITOR_VALID ? '+' : 
> '-');
>   quote_c_style(istate->cache[i]->name, NULL, stdout, 0);
>   putchar('\n');
>   }
> 
> instead of "a single long incomplete line" in the first place.  Your
> "fix" merely turns it into "a single long complete line", which does
> not quite feel big enough an improvement, at least to me.

The more user-digestable form like you describe already exists by way
of `git ls-files -f`.  I am not sure it is worth replicating it.

The only current uses of this tool are in tests, which only examine
the first ("no fsmonitor" / "fsmonitor last update ...") line.  I find
it useful as a brief summary view of the fsmonitor bits, but I suppose
I'd also be happy with just presence/absence and a count of set/unset
bits.

Barring objections from Dscho or Ben, I'll reroll with a version that
shows something like:

fsmonitor last update 1513821151547101894 (5 seconds ago)
5 files valid / 10 files invalid

 - Alex

[PATCH] t/helper/test-lazy-name-hash: fix compilation

[Stefan Beller]

I was compiling origin/master today with the DEVELOPER compiler flags
today and was greeted by

t/helper/test-lazy-init-name-hash.c: In function ‘cmd_main’:
t/helper/test-lazy-init-name-hash.c:172:5: error: ‘nr_threads_used’ may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
 printf("avg [size %8d] [single %f] %c [multi %f %d]\n",
 ^~~
 nr,
 ~~~
 (double)avg_single/10,
 ~~
 (avg_single < avg_multi ? '<' : '>'),
 ~
 (double)avg_multi/10,
 ~
 nr_threads_used);
 
t/helper/test-lazy-init-name-hash.c:115:6: note: ‘nr_threads_used’ was declared 
here
  int nr_threads_used;
  ^~~

Fix this issue by assigning 0 to 'nr_threads_used'.

Acked-by: Jonathan Tan 
Signed-off-by: Jeff Hostetler 
Signed-off-by: Stefan Beller 
---

Slightly reworded the commit message. I'd really like this patch to be included
such that I can compile git with the DEVELOPER_CFLAGS flags.

 t/helper/test-lazy-init-name-hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/helper/test-lazy-init-name-hash.c 
b/t/helper/test-lazy-init-name-hash.c
index 6368a89345..297fb01d61 100644
--- a/t/helper/test-lazy-init-name-hash.c
+++ b/t/helper/test-lazy-init-name-hash.c
@@ -112,7 +112,7 @@ static void analyze_run(void)
 {
uint64_t t1s, t1m, t2s, t2m;
int cache_nr_limit;
-   int nr_threads_used;
+   int nr_threads_used = 0;
int i;
int nr;
 
-- 
2.15.1.620.gb9897f4670-goog

Re: Usability outrage as far as I am concerned

[Ævar Arnfjörð Bjarmason]

On Wed, Dec 20, 2017 at 5:40 PM, Cristian Achim  wrote:
> All I will do is paste the stackoverflow question below. It covers the
> commands I made in chronological order and the way I would have
> expected git to behave differently.
>
> So I did
>
> git pull home_subfolder
>
> while in usb_subfolder. Can't remember the immediate output, but it
> included a part about two repos having no commits in common. Would
> have been wiser for git to just error out with a message that in one
> of the folders there is not git repository.
>
> At all times
>
> git status
>
> in home_subfolder gives
>
> branch master
> Working directory clean
>
> This is so stupid considering the circumstances.
>
> Doing again
>
> git pull home_subfolder
>
> at all times from usb_folder now returns U in front of 2 files in
> home_folder and A in front of another file and then says
>
> Pull is not possible because you have unmerged files
>
> Had the bright idea to do
>
> git clone home_subfolder
>
> from the usb_subfolder because of my worries going away when
>
> git status
>
> in home_subfolder conforted me that there must be nothing bad about
> the situation. Therefore I suspect that now doing
>
> git pull usb_subfolder
>
> from home_subfolder will do the same broken result as above.
>
> I do not want to resolve merge conflicts because git doesn't error out
> with a sane message of the situation when it should. I just want to
> get the home_subfolder and usb_subfolder repositories state to where
> it was before my breakage generating git pull and git clone commands.
>
> Ubuntu 14.04 64 bit, git 1.9.1.

Hi. While I'm sure you'll still find some fault with git's UI could you please:

1) Try to see if the latest git version (or some approximation) fixes
some of this, as you said you're on 1.9.1 which was released in 2014,
there's been a lot of UI work since then, including some that by my
reading of your mail should address some of your concerns

2) It would be really useful if you could distill the complaints you
have down to some sequence of commands to run, preferably just a small
shellscript with comments saying "the message at this point is bad for
such-and-such a reason". I may have just misread what you said (or
maybe it's since been fixed), but per my reading we're already doing
some of what you've pointed out we should be doing, and if not maybe
that's because I'm not imagining how you're running git exactly, a
reproducible recipe would really help.

Thanks.

[PATCHv2 0/4] Fix --recurse-submodules for submodule worktree changes

[Stefan Beller]

v2:
* squashed the test patch and the bugfix, rewriting the commit message entirely.
  This might not the way Jonathan imagined how I address the potential user
  confusion[1], but I think it does the job.

[1] 
https://public-inbox.org/git/2017122148.gj240...@aiede.mtv.corp.google.com/

v1:

The fix is in the last patch, the first patches are just massaging the code
base to make the fix easy.

The second patch fixes a bug in the test, which was ineffective at testing.
The third patch shows the problem this series addresses,
the fourth patch is a little refactoring, which I want to keep separate
as I would expect it to be a performance regression[1].
The first patch is unrelated, but improves the readability of submodule test
cases, which we'd want to improve further.

Thanks,
Stefan

[1] The performance should improve once we don't use so many processes
any more, but that repository series is stalled for now.


Stefan Beller (4):
  t/lib-submodule-update.sh: clarify test
  t/lib-submodule-update.sh: Fix test ignoring ignored files in
submodules
  unpack-trees: Fix same() for submodules
  submodule: submodule_move_head omits old argument in forced case

 submodule.c   |  4 +++-
 t/lib-submodule-update.sh | 16 ++--
 unpack-trees.c|  2 ++
 3 files changed, 19 insertions(+), 3 deletions(-)

-- 
2.15.1.620.gb9897f4670-goog

[PATCH 1/4] t/lib-submodule-update.sh: clarify test

[Stefan Beller]

Keep the local branch name as the upstream branch name to avoid confusion.

Signed-off-by: Stefan Beller 
---
 t/lib-submodule-update.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 38dadd2c29..d7699046f6 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -664,8 +664,8 @@ test_submodule_recursing_with_args_common() {
cd submodule_update &&
git -C sub1 checkout -b keep_branch &&
git -C sub1 rev-parse HEAD >expect &&
-   git branch -t check-keep origin/modify_sub1 &&
-   $command check-keep &&
+   git branch -t modify_sub1 origin/modify_sub1 &&
+   $command modify_sub1 &&
test_superproject_content origin/modify_sub1 &&
test_submodule_content sub1 origin/modify_sub1 &&
git -C sub1 rev-parse keep_branch >actual &&
-- 
2.15.1.620.gb9897f4670-goog

[PATCH 2/4] t/lib-submodule-update.sh: Fix test ignoring ignored files in submodules

[Stefan Beller]

It turns out that this buggy test passed due to the buggy implementation,
which will soon be corrected.  Let's fix the test first.

Signed-off-by: Stefan Beller 
---
 t/lib-submodule-update.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index d7699046f6..fb0173ea87 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -885,6 +885,7 @@ test_submodule_switch_recursing_with_args () {
(
cd submodule_update &&
git branch -t replace_sub1_with_file 
origin/replace_sub1_with_file &&
+   echo ignored >.git/modules/sub1/info/exclude &&
: >sub1/ignored &&
$command replace_sub1_with_file &&
test_superproject_content origin/replace_sub1_with_file 
&&
-- 
2.15.1.620.gb9897f4670-goog

[PATCH 4/4] submodule: submodule_move_head omits old argument in forced case

[Stefan Beller]

When using hard reset or forced checkout with the option to recurse into
submodules, the submodules need to be reset, too.

It turns out that we need to omit the duplicate old argument to read-tree
in all forced cases to omit the 2 way merge and use the more assertive
behavior of reading the specific new tree into the index and updating
the working tree.

Signed-off-by: Stefan Beller 
---
 submodule.c   |  4 +++-
 t/lib-submodule-update.sh | 11 +++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/submodule.c b/submodule.c
index fa25888783..db0f7ac51e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1653,7 +1653,9 @@ int submodule_move_head(const char *path,
else
argv_array_push(, "-m");
 
-   argv_array_push(, old ? old : EMPTY_TREE_SHA1_HEX);
+   if (!(flags & SUBMODULE_MOVE_HEAD_FORCE))
+   argv_array_push(, old ? old : EMPTY_TREE_SHA1_HEX);
+
argv_array_push(, new ? new : EMPTY_TREE_SHA1_HEX);
 
if (run_command()) {
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index fb0173ea87..380ef4b4ae 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -1015,4 +1015,15 @@ test_submodule_forced_switch_recursing_with_args () {
test_submodule_content sub1 origin/modify_sub1
)
'
+
+   test_expect_success "$command: changed submodule worktree is reset" '
+   prolog &&
+   reset_work_tree_to_interested add_sub1 &&
+   (
+   cd submodule_update &&
+   rm sub1/file1 &&
+   $command HEAD &&
+   test_path_is_file sub1/file1
+   )
+   '
 }
-- 
2.15.1.620.gb9897f4670-goog

[PATCH 3/4] unpack-trees: Fix same() for submodules

[Stefan Beller]

The function same(a, b) is used to check if two entries a and b are the
same.  As the index contains the staged files the same() function can be
used to check if files between a given revision and the index are the same.

In case of submodules, the gitlink entry is showing up as not modified
despite potential changes inside the submodule.

Fix the function to examine submodules by looking inside the submodule.
This patch alone doesn't affect any correctness garantuees, but in
combination with the next patch this fixes the new test introduced
earlier in this series.

This may be a slight performance regression as now we have to
inspect any submodule thouroughly.

Signed-off-by: Stefan Beller 
---
 unpack-trees.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/unpack-trees.c b/unpack-trees.c
index bf8b602901..4d839e8edb 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1427,6 +1427,8 @@ static int same(const struct cache_entry *a, const struct 
cache_entry *b)
return 1;
if ((a->ce_flags | b->ce_flags) & CE_CONFLICTED)
return 0;
+   if (S_ISGITLINK(b->ce_mode) && should_update_submodules())
+   return !oidcmp(>oid, >oid) && 
!is_submodule_modified(b->name, 0);
return a->ce_mode == b->ce_mode &&
   !oidcmp(>oid, >oid);
 }
-- 
2.15.1.620.gb9897f4670-goog

Re: [PATCH 2/2] commit: add support for --fixup -m""

[Eric Sunshine]

On Wed, Dec 20, 2017 at 4:40 PM, Ævar Arnfjörð Bjarmason
 wrote:
> On Wed, Dec 20 2017, Eric Sunshine jotted:
>> On Wed, Dec 20, 2017 at 2:38 PM, Ævar Arnfjörð Bjarmason
>>  wrote:
>>> +test_expect_success 'commit --fixup -m"something" -m"extra"' '
>>> +   commit_for_rebase_autosquash_setup &&
>>> +   git commit --fixup HEAD~1 -m"something" -m"extra" &&
>>> +   commit_msg_is "fixup! target message subject linesomething
>>> +
>>> +extra"
>>> +'
>>
>> Hmm, so the first -m appended to the "fixup!" line, but the second -m
>> appended after a blank line? That doesn't seem very intuitive.
>>
>> Also, doesn't the text following "fixup!" need to exactly match the
>> summary line of the commit message in order for "git rebase -i
>> --autosquash" to work? Am I overlooking something obvious?
>
> It does the right thing and it's actually
> "$fixup_line\n\n$first_m\n\n$second_m" etc. It's just that this
> commit_msg_is function is testing against the "%s%b" format, so the
> first line of the body comes right after the subject.

Thanks for explaining. I guess I should have delved into
commit_msg_is() before commenting.

Re: [PATCH 2/2] commit: add support for --fixup -m""

[Ævar Arnfjörð Bjarmason]

On Wed, Dec 20 2017, Eric Sunshine jotted:

> On Wed, Dec 20, 2017 at 2:38 PM, Ævar Arnfjörð Bjarmason
>  wrote:
>> Add support for supplying the -m option with --fixup. Doing so has
>> errored out ever since --fixup was introduced. Before this, the only
>> way to amend the fixup message while committing was to use --edit and
>> amend it in the editor.
>
> I can't tell, based upon this description, if '-m --edit' is a
> valid combination and, if so, does it correctly open the editor after
> appending ?

Yes, that's works as expected for all the options, and this doesn't
break that.

>> Signed-off-by: Ævar Arnfjörð Bjarmason 
>> ---
>> diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
>> @@ -272,6 +272,14 @@ test_expect_success 'commit --fixup provides correct 
>> one-line commit message' '
>> commit_msg_is "fixup! target message subject line"
>>  '
>>
>> +test_expect_success 'commit --fixup -m"something" -m"extra"' '
>> +   commit_for_rebase_autosquash_setup &&
>> +   git commit --fixup HEAD~1 -m"something" -m"extra" &&
>> +   commit_msg_is "fixup! target message subject linesomething
>> +
>> +extra"
>> +'
>
> Hmm, so the first -m appended to the "fixup!" line, but the second -m
> appended after a blank line? That doesn't seem very intuitive.
>
> Also, doesn't the text following "fixup!" need to exactly match the
> summary line of the commit message in order for "git rebase -i
> --autosquash" to work? Am I overlooking something obvious?

It does the right thing and it's actually
"$fixup_line\n\n$first_m\n\n$second_m" etc. It's just that this
commit_msg_is function is testing against the "%s%b" format, so the
first line of the body comes right after the subject.

Thanks for the review of both patches. I'll clarify the points raised in
commit message for v2 pending further feedback.

Re: [PATCH 3/6] fsmonitor: Update helper tool, now that flags are filled later

[Alex Vandiver]

On Mon, 18 Dec 2017, Alex Vandiver wrote:
> dd9005a0a ("fsmonitor: delay updating state until after split index is
> merged", 2017-10-27) began deferring the setting of the
> CE_FSMONITOR_VALID flag until later, such that do_read_index() does
> not perform those steps.  This means that t/helper/test-dump-fsmonitor
> showed all bits as always unset.

This isn't the right fix, actually.  With split indexes, this puts us
right back into "only shows a few bits" territory, because
do_read_index doesn't know about split indexes.

Which means we need a way to do the whole index load but _not_
add/remove the fsmonitor cache, even if the config says to do so.

The best I'm coming up with is the below -- but I'm not happy with
it, because 'keep' is meaningless as a configuration value outside of
testing, since it's normally treated as an executable path.  This uses
the fact that fsmonitor.c currently has a:

switch (fsmonitor_enabled) {
case -1: /* keep: do nothing */
break;

...despite get_config_get_fsmonitor() havong no way to return -1 at
present.

Is this sort of testing generally done via environment variables,
rather than magic config values?
 - Alex

-8<
diff --git a/config.c b/config.c
index 6fb06c213..75fcf1a52 100644
--- a/config.c
+++ b/config.c
@@ -2164,8 +2164,13 @@ int git_config_get_fsmonitor(void)
if (core_fsmonitor && !*core_fsmonitor)
core_fsmonitor = NULL;

-   if (core_fsmonitor)
-   return 1;
+
+   if (core_fsmonitor) {
+   if (!strcasecmp(core_fsmonitor, "keep"))
+   return -1;
+   else
+   return 1;
+   }

return 0;
 }
diff --git a/t/helper/test-dump-fsmonitor.c b/t/helper/test-dump-fsmonitor.c
index ad452707e..12e131530 100644
--- a/t/helper/test-dump-fsmonitor.c
+++ b/t/helper/test-dump-fsmonitor.c
@@ -5,8 +5,9 @@ int cmd_main(int ac, const char **av)
struct index_state *istate = _index;
int i;

+   git_config_push_parameter("core.fsmonitor=keep");
setup_git_directory();
-   if (do_read_index(istate, get_index_file(), 0) < 0)
+   if (read_index_from(istate, get_index_file()) < 0)
die("unable to read index file");
if (!istate->fsmonitor_last_update) {
printf("no fsmonitor\n");
-8<-

Re: [PATCH 2/6] fsmonitor: Add dir.h include, for untracked_cache_invalidate_path

[Alex Vandiver]

On Tue, 19 Dec 2017, Junio C Hamano wrote:
> Alex Vandiver  writes:
> 
> > Subject: Re: [PATCH 2/6] fsmonitor: Add dir.h include, for 
> > untracked_cache_invalidate_path
> 
> Perhaps
> 
> "Subject: fsmonitor.h: include dir.h"

Certainly more concise.

> But I am not sure if this is a right direction to go in.  If a .C
> user of fsmonitor needs (does not need) things from dir.h, that file
> can (does not need to) include dir.h itself.

Hm; I was patterning based on existing .h files, which don't seem shy
about pulling in other .h files.

> I think this header does excessive "static inline" as premature
> optimization, so a better "fix" to your perceived problem may be to
> make them not "static inline".

Yeah, quite possibly.  Ben, do you recall your rationale for inlining
them in 6a6da08f6 ("fsmonitor: teach git to optionally utilize a file
system monitor to speed up detecting new or changed files.",
2017-09-22) ?

 - Alex

Re: [PATCH v6] Makefile: replace perl/Makefile.PL with simple make rules

[Todd Zullinger]

Ævar Arnfjörð Bjarmason wrote:
> On Wed, Dec 20, 2017 at 6:41 PM, Todd Zullinger  wrote:
>> /usr/share/perl5/vendor_perl/Git
>> /usr/share/perl5/vendor_perl/Git.pm
>> /usr/share/perl5/vendor_perl/Git/Error.pm
>> [...]
>> /usr/share/perl5/vendor_perl/build
>> /usr/share/perl5/vendor_perl/build/lib
>> /usr/share/perl5/vendor_perl/build/lib/Git
>> /usr/share/perl5/vendor_perl/build/lib/Git.pm
>> /usr/share/perl5/vendor_perl/build/lib/Git/Error.pm
>> [...]
>> Note that not all of the .pm files are matched, which I
>> believe is due to the glob matches only going 4 levels deep
>> under the perl dir.
> 
> Ouch, that's a stupid mistake of mine. Didn't consider that changing
> it from *.pm to *.pmc would of course impact that glob match.
> 
> This fixes it, changes against v5:
> 
> @@ -224,7 +224,7 @@
>   po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
> $(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
>   
> -+LIB_PERL := $(wildcard perl/*.pm perl/*/*.pm perl/*/*/*.pm 
> perl/*/*/*/*.pm)
> ++LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm 
> perl/Git/*/*/*.pm)
>  +LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
>  +
>  +ifndef NO_PERL
> 
> I.e. let's keep calling it "build" for consistency with other stuff
> and so "ls" will show it, but just alter the glob so we'll only match
> modules like Git{,::*}. I don't think we'll ever add anything outside
> that namespace, so this seems like the best solution.

Sounds good.  While it might not have been too bad to have a
hidden dir for build artifacts, using the more explicit glob
pattern is much nicer.

I'll use this locally and let you know if I notice any
issues.  Thanks for working on this.

-- 
Todd
~~
Some people never go crazy. What truly horrible lives they must
live.
-- Charles Bukowski

Re: [PATCH 2/2] commit: add support for --fixup -m""

[Eric Sunshine]

On Wed, Dec 20, 2017 at 2:38 PM, Ævar Arnfjörð Bjarmason
 wrote:
> Add support for supplying the -m option with --fixup. Doing so has
> errored out ever since --fixup was introduced. Before this, the only
> way to amend the fixup message while committing was to use --edit and
> amend it in the editor.

I can't tell, based upon this description, if '-m --edit' is a
valid combination and, if so, does it correctly open the editor after
appending ?

> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
> diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
> @@ -272,6 +272,14 @@ test_expect_success 'commit --fixup provides correct 
> one-line commit message' '
> commit_msg_is "fixup! target message subject line"
>  '
>
> +test_expect_success 'commit --fixup -m"something" -m"extra"' '
> +   commit_for_rebase_autosquash_setup &&
> +   git commit --fixup HEAD~1 -m"something" -m"extra" &&
> +   commit_msg_is "fixup! target message subject linesomething
> +
> +extra"
> +'

Hmm, so the first -m appended to the "fixup!" line, but the second -m
appended after a blank line? That doesn't seem very intuitive.

Also, doesn't the text following "fixup!" need to exactly match the
summary line of the commit message in order for "git rebase -i
--autosquash" to work? Am I overlooking something obvious?

Re: [PATCH 1/2] commit doc: document that -c, -C, -F and --fixup with -m error

[Eric Sunshine]

On Wed, Dec 20, 2017 at 2:38 PM, Ævar Arnfjörð Bjarmason
 wrote:
> Document that providing any of -c, -C, -F and --fixup along with -m
> will result in an error. Some variant of this has been errored about
> explicitly since 0c091296c0 ("git-commit: log parameter updates.",
> 2005-08-08), but the documentation was never updated to reflect this.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> @@ -144,6 +144,9 @@ OPTIONS
> ++
> +Combining the `-m` option and any of `-c`, `-C`, `-F` or `--fixup`
> +will result in an error.

This sounds a bit scary, as if there is something wrong with Git.
Perhaps say instead that they are "mutually exclusive":

The `-m` option is mutually exclusive with `-c`, `-C`, `-F`, and
`--fixup`.

Or something.

Re: [PATCH 1/4] status: add --no-ahead-behind to porcelain V2

[Jeff Hostetler]

On 12/20/2017 11:33 AM, Jeff King wrote:

On Wed, Dec 20, 2017 at 02:42:42PM +, Jeff Hostetler wrote:


diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9593bfa..9ccdf2b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3082,6 +3082,11 @@ status.submoduleSummary::
submodule summary' command, which shows a similar output but does
not honor these settings.
  
+status.noaheadbehind::

+   Do not compute ahead/behind counts for a branch relative to its
+   upstream branch.  This can be used to avoid a possibly very
+   expensive computation on extremely large repositories.


I got tripped up by double-negatives here while testing something out
with your series. Could this be "status.aheadbehind", and default to
true?

-Peff



Yeah, we debated internally how to name it and that was the least bad.
I'll change it to the positive and (based on later comments) move it
to "core." so that we can pick up "branch -vv" in this round.

Thanks
Jeff

[PATCH 2/2] commit: add support for --fixup -m""

[Ævar Arnfjörð Bjarmason]

Add support for supplying the -m option with --fixup. Doing so has
errored out ever since --fixup was introduced. Before this, the only
way to amend the fixup message while committing was to use --edit and
amend it in the editor.

The use-case for this feature is one of:

 * Leaving a quick note to self when creating a --fixup commit when
   it's not self-evident why the commit should be squashed without a
   note into another one.

 * (Ab)using the --fixup feature to "fix up" commits that have already
   been pushed to a branch that doesn't allow non-fast-forwards,
   i.e. just noting "this should have been part of that other commit",
   and if the history ever got rewritten in the future the two should
   be combined.

   In such a case you might want to leave a small message,
   e.g. "forgot this part, which broke XYZ".

When the --fixup option was initially added the "Option -m cannot be
combined" error was expanded from -c, -C and -F to also include
--fixup[1]

Those options could also support combining with -m, but given what
they do I can't think of a good use-case for doing that, so I have not
made the more invasive change of splitting up the logic in commit.c to
first act on those, and then on -m options.

1. d71b8ba7c9 ("commit: --fixup option for use with rebase
   --autosquash", 2010-11-02)

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-commit.txt | 4 ++--
 builtin/commit.c | 8 +---
 t/t7500-commit.sh| 9 -
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index df83176314..4489677fd1 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -145,8 +145,8 @@ OPTIONS
If multiple `-m` options are given, their values are
concatenated as separate paragraphs.
 +
-Combining the `-m` option and any of `-c`, `-C`, `-F` or `--fixup`
-will result in an error.
+Combining the `-m` option and any of `-c`, `-C` or `-F` will result in
+an error.
 
 -t ::
 --template=::
diff --git a/builtin/commit.c b/builtin/commit.c
index 8a87701414..4e68394391 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -701,7 +701,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
}
}
 
-   if (have_option_m) {
+   if (have_option_m && !fixup_message) {
strbuf_addbuf(, );
hook_arg1 = "message";
} else if (logfile && !strcmp(logfile, "-")) {
@@ -731,6 +731,8 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
ctx.output_encoding = get_commit_output_encoding();
format_commit_message(commit, "fixup! %s\n\n",
  , );
+   if (have_option_m)
+   strbuf_addbuf(, );
hook_arg1 = "message";
} else if (!stat(git_path_merge_msg(), )) {
/*
@@ -1197,8 +1199,8 @@ static int parse_and_validate_options(int argc, const 
char *argv[],
f++;
if (f > 1)
die(_("Only one of -c/-C/-F/--fixup can be used."));
-   if (have_option_m && f > 0)
-   die((_("Option -m cannot be combined with -c/-C/-F/--fixup.")));
+   if (have_option_m && (edit_message || use_message || logfile))
+   die((_("Option -m cannot be combined with -c/-C/-F.")));
if (f || have_option_m)
template_file = NULL;
if (edit_message)
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index 5739d3ed23..2d95778b74 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -272,6 +272,14 @@ test_expect_success 'commit --fixup provides correct 
one-line commit message' '
commit_msg_is "fixup! target message subject line"
 '
 
+test_expect_success 'commit --fixup -m"something" -m"extra"' '
+   commit_for_rebase_autosquash_setup &&
+   git commit --fixup HEAD~1 -m"something" -m"extra" &&
+   commit_msg_is "fixup! target message subject linesomething
+
+extra"
+'
+
 test_expect_success 'commit --squash works with -F' '
commit_for_rebase_autosquash_setup &&
echo "log message from file" >msgfile &&
@@ -325,7 +333,6 @@ test_expect_success 'invalid message options when using 
--fixup' '
test_must_fail git commit --fixup HEAD~1 --squash HEAD~2 &&
test_must_fail git commit --fixup HEAD~1 -C HEAD~2 &&
test_must_fail git commit --fixup HEAD~1 -c HEAD~2 &&
-   test_must_fail git commit --fixup HEAD~1 -m "cmdline message" &&
test_must_fail git commit --fixup HEAD~1 -F log
 '
 
-- 
2.15.1.424.g9478a66081

[PATCH 1/2] commit doc: document that -c, -C, -F and --fixup with -m error

[Ævar Arnfjörð Bjarmason]

Document that providing any of -c, -C, -F and --fixup along with -m
will result in an error. Some variant of this has been errored about
explicitly since 0c091296c0 ("git-commit: log parameter updates.",
2005-08-08), but the documentation was never updated to reflect this.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-commit.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 8c74a2ca03..df83176314 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -144,6 +144,9 @@ OPTIONS
Use the given  as the commit message.
If multiple `-m` options are given, their values are
concatenated as separate paragraphs.
++
+Combining the `-m` option and any of `-c`, `-C`, `-F` or `--fixup`
+will result in an error.
 
 -t ::
 --template=::
-- 
2.15.1.424.g9478a66081

Re: [PATCH 5/5] submodule: submodule_move_head omits old argument in forced case

[Stefan Beller]

On Wed, Dec 20, 2017 at 11:36 AM, Stefan Beller  wrote:
> On Tue, Dec 19, 2017 at 4:01 PM, Jonathan Nieder  wrote:
>> Stefan Beller wrote:
>>> On Tue, Dec 19, 2017 at 2:44 PM, Jonathan Nieder  wrote:
>>
   checkout -f
 I think I would expect this not to touch a submodule that
 hasn't changed, since that would be consistent with its
 behavior on files that haven't changed.
>> [...]
>>> I may have a different understanding of git commands than you do,
>>> but a plain "checkout -f" with no further arguments is the same as
>>> a hard reset IMHO, and could be used interchangeably?
>>
>> A kind person pointed out privately that you were talking about
>> "git checkout -f" with no further arguments, not "git checkout -f
>> ".  In that context, the competing meanings I mentioned in
>> https://crbug.com/git/5 don't exist: either a given entry in the
>> worktree matches the index or it doesn't.
>>
>> So plain "git checkout -f" is similar to plain "git reset --hard"
>> in that it means "make the worktree (and index, in the reset case)
>> look just like this".
>
> with "this" == the argument that was given, if the argument
> was omitted, HEAD is assumed.
>
>>  checkout -f makes the worktree look like the index;
>
> No, here is what my installation of git (recent master) does:

Well, you are technically correct, the worktree is made look like the index,
but prior to that the index is reset to the HEAD, so for me it is
easier to understand
as "make worktree and index like HEAD"

Re: [PATCH 5/5] submodule: submodule_move_head omits old argument in forced case

[Stefan Beller]

On Tue, Dec 19, 2017 at 4:01 PM, Jonathan Nieder  wrote:
> Stefan Beller wrote:
>> On Tue, Dec 19, 2017 at 2:44 PM, Jonathan Nieder  wrote:
>
>>>   checkout -f
>>> I think I would expect this not to touch a submodule that
>>> hasn't changed, since that would be consistent with its
>>> behavior on files that haven't changed.
> [...]
>> I may have a different understanding of git commands than you do,
>> but a plain "checkout -f" with no further arguments is the same as
>> a hard reset IMHO, and could be used interchangeably?
>
> A kind person pointed out privately that you were talking about
> "git checkout -f" with no further arguments, not "git checkout -f
> ".  In that context, the competing meanings I mentioned in
> https://crbug.com/git/5 don't exist: either a given entry in the
> worktree matches the index or it doesn't.
>
> So plain "git checkout -f" is similar to plain "git reset --hard"
> in that it means "make the worktree (and index, in the reset case)
> look just like this".

with "this" == the argument that was given, if the argument
was omitted, HEAD is assumed.

>  checkout -f makes the worktree look like the index;

No, here is what my installation of git (recent master) does:

  $ git --version
git version 2.15.1.389.g52015aaf9d

  $ cat test.sh

  rm -rf tmp
  git init tmp

  cd tmp
  git commit --allow-empty -m initial
  echo commit >a
  echo commit >b
  echo commit >c
  git add .
  git commit -m commit

  echo index >a
  git add a
  echo worktree >a

  echo index >b
  git add b

  echo worktree>c

  $ sh test.sh
Initialized empty Git repository in /u/tmp/.git/
[master (root-commit) c109fb5] initial
[master fcc21ea] commit
 3 files changed, 3 insertions(+)
 create mode 100644 a
 create mode 100644 b
 create mode 100644 c
  $ cd tmp
  $ git status
On branch master
Changes to be committed:
  (use "git reset HEAD ..." to unstage)

modified:   a
modified:   b

Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

modified:   a
modified:   c

  $ git checkout -f
  $ git status
On branch master
nothing to commit, working tree clean

# Let's test the other commands as well:
  $ cd ..
  $ sh test.sh
   ...
  $ cd tmp
  $ git checkout -f HEAD
  $ git status
On branch master
nothing to commit, working tree clean

  # See, there is no difference between giving HEAD as an argument
  # or not! Try again with reset just for completeness:

  $ cd ..
  $ sh test.sh
   ...
  $ cd tmp
  $ git reset --hard
HEAD is now at b71ae70 commit
  $ git status
On branch master
nothing to commit, working tree clean

  # The only difference between reset and the checkout is that reset
  # tells me where we are.

> reset --hard makes the worktree and index look like HEAD.

I agree.

> In that context, more aggressively making the submodule in the
> worktree get into the defined state sounds to me like a good change.
>
> Hopefully my confusion helps illustrate what a commit message focusing
> on the end user experience might want to mention.

I'll try to come up with a better commit message. Thanks for bearing
with me here.

Stefan


>
> Thanks again,
> Jonathan

Error in `git': free(): invalid pointer (was Re: [PATCH] sequencer: improve config handling)

[Kaartic Sivaraam]

I recently encountered that error when trying to do an interactive
rebase after using filter-branch to remove a file completely in a
repository. I bisected this issue which pointed at this patch. I'm not
sure how it is related as I'm not too familiar with the sequencer code.
I could help in case any specific information is needed. As a first
step, I've posted the output of "strace /mnt/Source//Git/git rebase -i
HEAD~10" below.


-- 8< --
execve("/mnt/Source//Git/git-next/git", ["/mnt/Source//Git/git-next/git", 
"rebase", "-i", "HEAD~10"], [/* 62 vars */]) = 0
brk(NULL)   = 0x55a494d3
access("/etc/ld.so.nohwcap", F_OK)  = -1 ENOENT (No such file or directory)
mmap(NULL, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 
0x7f25bd94
access("/etc/ld.so.preload", R_OK)  = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=114620, ...}) = 0
mmap(NULL, 114620, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f25bd924000
close(3)= 0
access("/etc/ld.so.nohwcap", F_OK)  = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libz.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\300!\0\0\0\0\0\0"..., 
832) = 832
fstat(3, {st_mode=S_IFREG|0644, st_size=105088, ...}) = 0
mmap(NULL, 2200072, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 
0x7f25bd506000
mprotect(0x7f25bd51f000, 2093056, PROT_NONE) = 0
mmap(0x7f25bd71e000, 8192, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x18000) = 0x7f25bd71e000
close(3)= 0
access("/etc/ld.so.nohwcap", F_OK)  = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libpthread.so.0", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0Pa\0\0\0\0\0\0"..., 832) 
= 832
fstat(3, {st_mode=S_IFREG|0755, st_size=135440, ...}) = 0
mmap(NULL, 2212936, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 
0x7f25bd2e9000
mprotect(0x7f25bd301000, 2093056, PROT_NONE) = 0
mmap(0x7f25bd50, 8192, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x17000) = 0x7f25bd50
mmap(0x7f25bd502000, 13384, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f25bd502000
close(3)= 0
access("/etc/ld.so.nohwcap", F_OK)  = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/librt.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\340 \0\0\0\0\0\0"..., 
832) = 832
fstat(3, {st_mode=S_IFREG|0644, st_size=31744, ...}) = 0
mmap(NULL, 2128832, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 
0x7f25bd0e1000
mprotect(0x7f25bd0e8000, 2093056, PROT_NONE) = 0
mmap(0x7f25bd2e7000, 8192, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x6000) = 0x7f25bd2e7000
close(3)= 0
access("/etc/ld.so.nohwcap", F_OK)  = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\0\4\2\0\0\0\0\0"..., 
832) = 832
fstat(3, {st_mode=S_IFREG|0755, st_size=1689360, ...}) = 0
mmap(NULL, 3795296, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 
0x7f25bcd42000
mprotect(0x7f25bced7000, 2097152, PROT_NONE) = 0
mmap(0x7f25bd0d7000, 24576, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x195000) = 0x7f25bd0d7000
mmap(0x7f25bd0dd000, 14688, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f25bd0dd000
close(3)= 0
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 
0x7f25bd922000
arch_prctl(ARCH_SET_FS, 0x7f25bd922e80) = 0
mprotect(0x7f25bd0d7000, 16384, PROT_READ) = 0
mprotect(0x7f25bd50, 4096, PROT_READ) = 0
mprotect(0x7f25bd2e7000, 4096, PROT_READ) = 0
mprotect(0x7f25bd71e000, 4096, PROT_READ) = 0
mprotect(0x55a49424b000, 12288, PROT_READ) = 0
mprotect(0x7f25bd943000, 4096, PROT_READ) = 0
munmap(0x7f25bd924000, 114620)  = 0
set_tid_address(0x7f25bd923150) = 9667
set_robust_list(0x7f25bd923160, 24) = 0
rt_sigaction(SIGRTMIN, {sa_handler=0x7f25bd2eebd0, sa_mask=[], 
sa_flags=SA_RESTORER|SA_SIGINFO, sa_restorer=0x7f25bd2fa0c0}, NULL, 8) = 0
rt_sigaction(SIGRT_1, {sa_handler=0x7f25bd2eec60, sa_mask=[], 
sa_flags=SA_RESTORER|SA_RESTART|SA_SIGINFO, sa_restorer=0x7f25bd2fa0c0}, NULL, 
8) = 0
rt_sigprocmask(SIG_UNBLOCK, [RTMIN RT_1], NULL, 8) = 0
getrlimit(RLIMIT_STACK, {rlim_cur=8192*1024, rlim_max=RLIM64_INFINITY}) = 0
open("/dev/null", O_RDWR)   = 3
close(3)= 0
rt_sigprocmask(SIG_UNBLOCK, [PIPE], NULL, 8) = 0
rt_sigaction(SIGPIPE, {sa_handler=SIG_DFL, sa_mask=[PIPE], 
sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f25bcd75060}, 
{sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
brk(NULL) 

[PATCH v6] Makefile: replace perl/Makefile.PL with simple make rules

[Ævar Arnfjörð Bjarmason]

Replace the perl/Makefile.PL and the fallback perl/Makefile used under
NO_PERL_MAKEMAKER=NoThanks with a much simpler implementation heavily
inspired by how the i18n infrastructure's build process works[1].

The reason for having the Makefile.PL in the first place is that it
was initially[2] building a perl C binding to interface with libgit,
this functionality, that was removed[3] before Git.pm ever made it to
the master branch.

We've since since started maintaining a fallback perl/Makefile, as
MakeMaker wouldn't work on some platforms[4]. That's just the tip of
the iceberg. We have the PM.stamp hack in the top-level Makefile[5] to
detect whether we need to regenerate the perl/perl.mak, which I fixed
just recently to deal with issues like the perl version changing from
under us[6].

There is absolutely no reason for why this needs to be so complex
anymore. All we're getting out of this elaborate Rube Goldberg machine
was copying perl/* to perl/blib/* as we do a string-replacement on
the *.pm files to hardcode @@LOCALEDIR@@ in the source, as well as
pod2man-ing Git.pm & friends.

So replace the whole thing with something that's pretty much a copy of
how we generate po/build/**.mo from po/*.po, just with a small sed(1)
command instead of msgfmt.

While I'm at it, change the fallback for Error.pm from being something
where we'll ship our own Error.pm if one doesn't exist at build time
to one where we just use a Git::Error wrapper that'll always prefer
the system-wide Error.pm, only falling back to our own copy if it
really doesn't exist at runtime. It's now shipped as
Git::FromCPAN::Error, making it easy to add other modules to
Git::FromCPAN::* in the future if that's needed.

Functional changes:

 * This will not always install into perl's idea of its global
   "installsitelib". This only potentially matters for packagers that
   need to expose Git.pm for non-git use, and as explained in the
   INSTALL file there's a trivial workaround.

 * The scripts themselves will 'use lib' the target directory, but if
   INSTLIBDIR is set it overrides it. It doesn't have to be this way,
   it could be set in addition to INSTLIBDIR, but my reading of [7] is
   that this is the desired behavior.

 * We don't build man pages for all of the perl modules as we used to,
   only Git(3pm). As discussed on-list[8] that we were building
   installed manpages for purely internal APIs like Git::I18N or
   private-Error.pm was always a bug anyway, and all the Git::SVN::*
   ones say they're internal APIs.

   There are apparently external users of Git.pm, but I don't expect
   there to be any of the others.

   As a side-effect of these general changes the perl documentation
   now only installed by install-{doc,man}, not a mere "install" as
   before.

1. 5e9637c629 ("i18n: add infrastructure for translating Git with
   gettext", 2011-11-18)

2. b1edc53d06 ("Introduce Git.pm (v4)", 2006-06-24)

3. 18b0fc1ce1 ("Git.pm: Kill Git.xs for now", 2006-09-23)

4. f848718a69 ("Make perl/ build procedure ActiveState friendly.",
   2006-12-04)

5. ee9be06770 ("perl: detect new files in MakeMaker builds",
   2012-07-27)

6. c59c4939c2 ("perl: regenerate perl.mak if perl -V changes",
   2017-03-29)

7. 0386dd37b1 ("Makefile: add PERLLIB_EXTRA variable that adds to
   default perl path", 2013-11-15)

8. 87bmjjv1pu@evledraar.booking.com ("Re: [PATCH] Makefile:
   replace perl/Makefile.PL with simple make rules"

Signed-off-by: Ævar Arnfjörð Bjarmason 
---

On Wed, Dec 20, 2017 at 6:41 PM, Todd Zullinger  wrote:
> /usr/share/perl5/vendor_perl/Git
> /usr/share/perl5/vendor_perl/Git.pm
> /usr/share/perl5/vendor_perl/Git/Error.pm
> [...]
> /usr/share/perl5/vendor_perl/build
> /usr/share/perl5/vendor_perl/build/lib
> /usr/share/perl5/vendor_perl/build/lib/Git
> /usr/share/perl5/vendor_perl/build/lib/Git.pm
> /usr/share/perl5/vendor_perl/build/lib/Git/Error.pm
> [...]
> Note that not all of the .pm files are matched, which I
> believe is due to the glob matches only going 4 levels deep
> under the perl dir.

Ouch, that's a stupid mistake of mine. Didn't consider that changing
it from *.pm to *.pmc would of course impact that glob match.

This fixes it, changes against v5:

@@ -224,7 +224,7 @@
  po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
$(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
  
-+LIB_PERL := $(wildcard perl/*.pm perl/*/*.pm perl/*/*/*.pm 
perl/*/*/*/*.pm)
++LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm 
perl/Git/*/*/*.pm)
 +LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
 +
 +ifndef NO_PERL

I.e. let's keep calling it "build" for consistency with other stuff
and so "ls" will show it, but just alter the glob so we'll only match
modules like Git{,::*}. I don't think we'll ever add anything outside
that namespace, so this seems like the best solution.

With this "make install" gives the expected results, i.e. no 

Re: What's cooking in git.git (Dec 2017, #04; Tue, 19)

[Kaartic Sivaraam]

On Wednesday 20 December 2017 03:30 AM, Junio C Hamano wrote:

* pw/sequencer-in-process-commit (2017-12-13) 10 commits
   (merged to 'next' on 2017-12-13 at ec4d2b9c84)
   ...
  The sequencer infrastructure is shared across "git cherry-pick",
  "git rebase -i", etc., and has always spawned "git commit" when it
  needs to create a commit.  It has been taught to do so internally,
  when able, by reusing the codepath "git commit" itself uses, which
  gives performance boost for a few tens of percents in some sample
  scenarios.

  Will merge to 'master'.


I just encountered a small issue with 'rebase -i'. My bisection pointed at,

28d6daed4 (sequencer: improve config handling, 2017-12-13)

So, I guess it would be better if you could delay merging it to 
'master'. I'm not sure what how the commit and the patch are related but 
lets discuss that in the patch thread.


--
Kaartic

Re: [PATCH] Re: Bug with "git submodule update" + subrepo with differing path/name?

[Stefan Beller]

On Wed, Dec 20, 2017 at 12:22 AM, Andreas Urke  wrote:
> Thanks for looking into this.
>
> I was able to reproduce it from scratch, but I followed my earlier
> workflow where I first created the subrepos, and then later renamed
> it. At the time I was not able to find any command to rename without
> changing the path (and I was not able find one now either, is there
> any?), so I edited name and path in .gitmodules and ran git submodule
> sync. Am I asking for trouble doing it that way?

"rename without changing the path" sounds like a red flag to me,
as the submodule name was introduced specifically to be a constant
as the path may change, whereas the name ought to never change.




>
> Let me know if you need the exact steps I followed.

Well yes, ideally as a shell script (or even embedded into our test suite).

[ANNOUNCE] Git Rev News edition 34

[Christian Couder]

Hi everyone,

The 34th edition of Git Rev News is now published:

  https://git.github.io/rev_news/2017/12/20/edition-34/

Thanks a lot to all the contributors and helpers!

Enjoy,
Christian, Jakub and Markus.

Re: [PATCH v5] Makefile: replace perl/Makefile.PL with simple make rules

[Todd Zullinger]

Ævar Arnfjörð Bjarmason wrote:
> [Sorry for not CC-ing you on the last version. Forgot to update the
> giant send-email line I'm juggling for this series].

No worries.  It is a rather large CC list at this point. :)

> This *.pmc thing is just me being overly clever. Here's a v5 that gets
> rid of it. Diff with v4:

Ahh, thanks for the additional details on this.

> -+PMFILES := $(wildcard perl/*.pm perl/*/*.pm perl/*/*/*.pm 
> perl/*/*/*/*.pm)
> -+PMCFILES := $(patsubst perl/%.pm,perl/build/lib/%.pmc,$(PMFILES))
> ++LIB_PERL := $(wildcard perl/*.pm perl/*/*.pm perl/*/*/*.pm 
> perl/*/*/*/*.pm)
> ++LIB_PERL_GEN := $(patsubst 
> perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
>  +
>  +ifndef NO_PERL
> -+all:: $(PMCFILES)
> ++all:: $(LIB_PERL_GEN)
>  +endif
>  +
> -+perl/build/lib/%.pmc: perl/%.pm
> ++perl/build/lib/%.pm: perl/%.pm
>  +  $(QUIET_GEN)mkdir -p $(dir $@) && \
>  +  sed -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' < $< > $@
>  +

Without the .pmc extensions, rpm picks up the perl
dependencies, which is good.  But an additional build/lib
dir is created, which ends up in $perllibdir after install.

Here's an extract from a local build:

mkdir -p perl/build/lib/build/lib/ && \
sed -e 's|@@LOCALEDIR@@|/usr/share/locale|g' < perl/build/lib/Git.pm > 
perl/build/lib/build/lib/Git.pm
mkdir -p perl/build/lib/build/lib/Git/ && \
sed -e 's|@@LOCALEDIR@@|/usr/share/locale|g' < perl/build/lib/Git/IndexInfo.pm 
> perl/build/lib/build/lib/Git/IndexInfo.pm
mkdir -p perl/build/lib/build/lib/Git/ && \
sed -e 's|@@LOCALEDIR@@|/usr/share/locale|g' < perl/build/lib/Git/SVN.pm > 
perl/build/lib/build/lib/Git/SVN.pm
mkdir -p perl/build/lib/build/lib/Git/ && \
sed -e 's|@@LOCALEDIR@@|/usr/share/locale|g' < perl/build/lib/Git/Error.pm > 
perl/build/lib/build/lib/Git/Error.pm
mkdir -p perl/build/lib/build/lib/Git/ && \
sed -e 's|@@LOCALEDIR@@|/usr/share/locale|g' < perl/build/lib/Git/I18N.pm > 
perl/build/lib/build/lib/Git/I18N.pm

When PMFILES and PMCFILES matched .pm and .pmc respectively,
the glob didn't match duplicated files under build/lib, so
this wasn't an issue.  I'm not sure where this is best
fixed.  The build/lib dir could be moved outside of perl or
it could be made .build/lib to avoid the wildcard match,
perhaps.

Building with perllibdir=/usr/share/perl5/vendor_perl
results in:

/usr/share/perl5/vendor_perl/Git
/usr/share/perl5/vendor_perl/Git.pm
/usr/share/perl5/vendor_perl/Git/Error.pm
/usr/share/perl5/vendor_perl/Git/FromCPAN
/usr/share/perl5/vendor_perl/Git/FromCPAN/Error.pm
/usr/share/perl5/vendor_perl/Git/I18N.pm
/usr/share/perl5/vendor_perl/Git/IndexInfo.pm
/usr/share/perl5/vendor_perl/Git/SVN
/usr/share/perl5/vendor_perl/Git/SVN.pm
/usr/share/perl5/vendor_perl/Git/SVN/Editor.pm
/usr/share/perl5/vendor_perl/Git/SVN/Fetcher.pm
/usr/share/perl5/vendor_perl/Git/SVN/GlobSpec.pm
/usr/share/perl5/vendor_perl/Git/SVN/Log.pm
/usr/share/perl5/vendor_perl/Git/SVN/Memoize
/usr/share/perl5/vendor_perl/Git/SVN/Memoize/YAML.pm
/usr/share/perl5/vendor_perl/Git/SVN/Migration.pm
/usr/share/perl5/vendor_perl/Git/SVN/Prompt.pm
/usr/share/perl5/vendor_perl/Git/SVN/Ra.pm
/usr/share/perl5/vendor_perl/Git/SVN/Utils.pm
/usr/share/perl5/vendor_perl/build
/usr/share/perl5/vendor_perl/build/lib
/usr/share/perl5/vendor_perl/build/lib/Git
/usr/share/perl5/vendor_perl/build/lib/Git.pm
/usr/share/perl5/vendor_perl/build/lib/Git/Error.pm
/usr/share/perl5/vendor_perl/build/lib/Git/I18N.pm
/usr/share/perl5/vendor_perl/build/lib/Git/IndexInfo.pm
/usr/share/perl5/vendor_perl/build/lib/Git/SVN.pm

Note that not all of the .pm files are matched, which I
believe is due to the glob matches only going 4 levels deep
under the perl dir.

Thanks,

-- 
Todd
~~
Never do today that which will become someone else's responsibility
tomorrow.

Re: [PATCH 5/6] fsmonitor: Remove debugging lines from t/t7519-status-fsmonitor.sh

[Johannes Schindelin]

Hi Junio,

On Tue, 19 Dec 2017, Junio C Hamano wrote:

> Alex Vandiver  writes:
> 
> > These were mistakenly left in when the test was introduced, in
> > 1487372d3 ("fsmonitor: store fsmonitor bitmap before splitting index",
> > 2017-11-09)
> >
> > Signed-off-by: Alex Vandiver 
> > ---
> >  t/t7519-status-fsmonitor.sh | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
> > index eb2d13bbc..19b2a0a0f 100755
> > --- a/t/t7519-status-fsmonitor.sh
> > +++ b/t/t7519-status-fsmonitor.sh
> > @@ -307,9 +307,7 @@ test_expect_success 'splitting the index results in the 
> > same state' '
> > dirty_repo &&
> > git update-index --fsmonitor  &&
> > git ls-files -f >expect &&
> > -   test-dump-fsmonitor >&2 && echo &&
> > git update-index --fsmonitor --split-index &&
> > -   test-dump-fsmonitor >&2 && echo &&
> > git ls-files -f >actual &&
> > test_cmp expect actual
> >  '
> 
> Hmph, by default the standard output and standard error streams are
> not shown in the test output, and it would help while debugging test
> failures, so I am not sure if this is a good change.  With the
> previous step [4/6], we can lose the "echo", of course, and I do not
> think we need >&2 redirection there, either.

I think you got it backwards. Sure, this helps debugging, but it hurts
runtime of the entire test suite (which I might have happened to mention a
couple of times takes way too long on Windows, thanks to our choice of
test "framework").

And in the bigger picture, I think that it is very, very easy to insert
those debugging statements when something breaks (we have to do that with
other tests, anyways).

So I am in favor of this patch, and disagree with your assessment, Junio.

Ciao,
Dscho

Re: [PATCH 0/4] Add --no-ahead-behind to status

[Jeff King]

On Wed, Dec 20, 2017 at 02:42:41PM +, Jeff Hostetler wrote:

> From: Jeff Hostetler 
> 
> This patch series adds a "--no-ahead-behind" option to status
> to request that it avoid a possibly expensive ahead/behind
> computation for the current branch.  Instead, it just prints a
> not up to date message in place of the detailed counts.
> 
> This idea was previously discussed in [1].  Working with the
> enormous Windows repository, we found that 20+ seconds was being
> spent in the ahead/behind computation when the current branch was
> 150K commits behind the upstream branch.  (Yes, this happens and
> only took 3 weeks on the reporter's system.)

Overall this looks cleanly done. I raised a few minor points in the
individual patches, but certainly nothing that would be a show-stopper
for the general idea.

> I've only modified "git status" in this patch series.  A similar
> change could be added to "git branch -vv" and "git checkout" to
> avoid delays there too.  I avoided doing it here to keep this
> patch series focused.

I have a feeling that the same user who got annoyed by "git status" is
going to get annoyed by "git checkout" sooner or later. I'm OK with
handling the other commands separately, but I suspect we may want at
least "git checkout" support in the near future.

There is one thing that it's worth thinking about (because it will be
hard to take back later): config option naming. If your repository is so
large that ahead/behind checks are annoying, would you want to be able
to set a single config to tell Git that, rather than one for each
command? If so, then do we want to ditch "status.aheadbehind" in favor
of a more unified name?

-Peff

Usability outrage as far as I am concerned

[Cristian Achim]

All I will do is paste the stackoverflow question below. It covers the
commands I made in chronological order and the way I would have
expected git to behave differently.

So I did

git pull home_subfolder

while in usb_subfolder. Can't remember the immediate output, but it
included a part about two repos having no commits in common. Would
have been wiser for git to just error out with a message that in one
of the folders there is not git repository.

At all times

git status

in home_subfolder gives

branch master
Working directory clean

This is so stupid considering the circumstances.

Doing again

git pull home_subfolder

at all times from usb_folder now returns U in front of 2 files in
home_folder and A in front of another file and then says

Pull is not possible because you have unmerged files

Had the bright idea to do

git clone home_subfolder

from the usb_subfolder because of my worries going away when

git status

in home_subfolder conforted me that there must be nothing bad about
the situation. Therefore I suspect that now doing

git pull usb_subfolder

from home_subfolder will do the same broken result as above.

I do not want to resolve merge conflicts because git doesn't error out
with a sane message of the situation when it should. I just want to
get the home_subfolder and usb_subfolder repositories state to where
it was before my breakage generating git pull and git clone commands.

Ubuntu 14.04 64 bit, git 1.9.1.

Thank you for your time.

Re: [PATCH 2/4] stat_tracking_info: return +1 when branch and upstream differ

[Jeff King]

On Wed, Dec 20, 2017 at 11:14:07AM -0500, Jeff King wrote:

> On Wed, Dec 20, 2017 at 02:42:43PM +, Jeff Hostetler wrote:
> 
> > Extend stat_tracking_info() to return 1 when the branch is
> > not up to date with its upstream branch and only return 0
> > when they are equal.
> 
> This means that callers all need to be updated, but there's no change
> that the compiler could catch. You've updated all of the calls here, but
> any topics in flight would need to be fixed, too.
> 
> I don't see any any in pu, but there are a number of long-running forks
> hanging around these days.
> 
> Is it worth introducing a small change so that any other callers which
> get merged in force a human to look at them? I'm wondering if we could
> just re-order the "upstream_name" argument or something.

Having seen the change in the next patch, I wonder if we should add a
flag field to specify "don't bother doing extra work" rather than
passing NULL for the ours/theirs parameters. I.e., most callers would
become:

  if (stat_tracking_info(branch, , , , 0) >= 0)

and the ones you touch later in the series would become:

  
  if (stat_tracking_info(branch, NULL, NULL, , TRACKING_QUICK) >= 0)

or similar. And then any newly added calls would get flagged by the
compiler as missing the final parameter.

-Peff

Re: [PATCH 1/4] status: add --no-ahead-behind to porcelain V2

[Jeff King]

On Wed, Dec 20, 2017 at 02:42:42PM +, Jeff Hostetler wrote:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 9593bfa..9ccdf2b 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -3082,6 +3082,11 @@ status.submoduleSummary::
>   submodule summary' command, which shows a similar output but does
>   not honor these settings.
>  
> +status.noaheadbehind::
> + Do not compute ahead/behind counts for a branch relative to its
> + upstream branch.  This can be used to avoid a possibly very
> + expensive computation on extremely large repositories.

I got tripped up by double-negatives here while testing something out
with your series. Could this be "status.aheadbehind", and default to
true?

-Peff

Re: [PATCH 4/4] status: support --no-ahead-behind in long status format.

[Jeff King]

On Wed, Dec 20, 2017 at 02:42:45PM +, Jeff Hostetler wrote:

> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> index ea029ad..9a2f209 100644
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -120,6 +120,9 @@ configuration variable documented in 
> linkgit:git-config[1].
>  +
>   In short format with --branch, '[different]' will printed rather
>   than detailed ahead/behind counts.
> ++
> + In long (normal) format, a simple out of date message will be
> + printed rather than detailed ahead/behind counts.

Same asciidoc trickery here as in the first patch (and in the --short
one, too, but I forgot to mention it).

> + } else if (no_ahead_behind) {
> + strbuf_addf(sb, _("Your branch is out of date with '%s'.\n"),
> + base);
> +
> + /* TODO Do we need a generic hint here? */
> +

I'm not sure what we'd advise here. I'd consider:

  git log --oneline --graph HEAD..@{upstream}

to see the actual differences, but that's a bit verbose. Is there an
easy way to re-enable it? I guess repeating the command with
"--ahead-behind" should do so.

-Peff

Re: [PATCH 3/4] status: update short status to use --no-ahead-behind

[Jeff King]

On Wed, Dec 20, 2017 at 02:42:44PM +, Jeff Hostetler wrote:

> From: Jeff Hostetler 
> 
> Teach "git status --short --branch" to use "--no-ahead-behind"
> flag to skip computing ahead/behind counts for the branch and
> its upstream and just report '[different]'.

How come "--short" and "--long" get this smaller bit of data, but
"--porcelain=v2" just omits the line entirely?

I don't have a real preference for or against the "[different]" message
myself, but if we can get the information cheaply, it seems odd not to
provide it in all cases.

> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> index 6ce8cf8..ea029ad 100644
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -117,6 +117,9 @@ configuration variable documented in 
> linkgit:git-config[1].
>   expensive computation on extremely large repositories.
>  +
>   In porcelain V2 format, the 'branch.ab' line will not be present.
> ++
> + In short format with --branch, '[different]' will printed rather
> + than detailed ahead/behind counts.

s/will/will be/ ?

> diff --git a/remote.c b/remote.c
> index a38b42e..0a63ac1 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1978,9 +1978,12 @@ int ref_newer(const struct object_id *new_oid, const 
> struct object_id *old_oid)
>  
>  /*
>   * Compare a branch with its upstream, and save their differences (number
> - * of commits) in *num_ours and *num_theirs. The name of the upstream branch
> - * (or NULL if no upstream is defined) is returned via *upstream_name, if it
> - * is not itself NULL.
> + * of commits) in *num_ours and *num_theirs.  If either num_ours or 
> num_theirs
> + * are NULL, we skip counting the commits and just return whether they are
> + * different.

OK, this makes sense. I wondered in the last one why the caller could
not simply check "num_ours != num_theirs" themselves. And this is why:
we want to be able to signal to stat_tracking_info() that we want the
"cheap" version.

> diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
> index 8f17fd9..0190220 100755
> --- a/t/t6040-tracking-info.sh
> +++ b/t/t6040-tracking-info.sh
> @@ -147,6 +147,19 @@ test_expect_success 'status -s -b (diverged from 
> upstream)' '
>  '
>  
>  cat >expect <<\EOF
> +## b1...origin/master [different]
> +EOF
> +
> +test_expect_success 'status -s -b --no-ahead-behind (diverged from 
> upstream)' '

This patch will affect "git status --porcelain", too. That's not
supposed to change in incompatible ways. I guess it's up for debate
whether callers are meant to handle any arbitrary string inside the []
(we already show "[gone]" for some cases), since AFAICT the format of
the tracking info is left completely vague in the documentation.

(I'd also hope that everybody is using --porcelain=v2 if they can, but
we should still avoid breaking v1).

-Peff

Re: [PATCH 2/4] stat_tracking_info: return +1 when branch and upstream differ

[Jeff King]

On Wed, Dec 20, 2017 at 02:42:43PM +, Jeff Hostetler wrote:

> Extend stat_tracking_info() to return 1 when the branch is
> not up to date with its upstream branch and only return 0
> when they are equal.

This means that callers all need to be updated, but there's no change
that the compiler could catch. You've updated all of the calls here, but
any topics in flight would need to be fixed, too.

I don't see any any in pu, but there are a number of long-running forks
hanging around these days.

Is it worth introducing a small change so that any other callers which
get merged in force a human to look at them? I'm wondering if we could
just re-order the "upstream_name" argument or something.

> --- a/remote.c
> +++ b/remote.c
> @@ -1983,7 +1983,9 @@ int ref_newer(const struct object_id *new_oid, const 
> struct object_id *old_oid)
>   * is not itself NULL.
>   *
>   * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no
> - * upstream defined, or ref does not exist), 0 otherwise.
> + * upstream defined, or ref does not exist).
> + * Returns 0 if the commits are the same (the branch is up to date).
> + * Returns 1 if the commits are different (the branch is not up to date).

Slightly pedantic, but we'd return 1 here also if the branch is ahead of
its upstream, right?

-Peff

Re: [PATCH 1/4] status: add --no-ahead-behind to porcelain V2

[Jeff King]

On Wed, Dec 20, 2017 at 02:42:42PM +, Jeff Hostetler wrote:

> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> index 9f3a78a..6ce8cf8 100644
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -111,6 +111,13 @@ configuration variable documented in 
> linkgit:git-config[1].
>   without options are equivalent to 'always' and 'never'
>   respectively.
>  
> +--no-ahead-behind::
> + Do not compute ahead/behind counts for the current branch relative
> + to the upstream branch.  This can be used to avoid a possibly very
> + expensive computation on extremely large repositories.
> ++
> + In porcelain V2 format, the 'branch.ab' line will not be present.
> +

This second paragraph after the "+" continuation shouldn't be indented
(sadly it makes the source much uglier, but it's one of the vagaries of
asciidoc).

-Peff

[PATCH 4/4] status: support --no-ahead-behind in long status format.

[Jeff Hostetler]

From: Jeff Hostetler 

Teach long (normal) status format to respect the --no-ahead-behind
argument and skip the possibly expensive ahead/behind computation
when printing the branch tracking information.

When --no-ahead-behind is given or status.noaheadbehind is true,
status prints "Your branch is out of date with ''."
instead of the various ahead/behind messages.

TODO Should we have an advice hint for this case?

Signed-off-by: Jeff Hostetler 
---
 Documentation/git-status.txt |  3 +++
 builtin/checkout.c   |  2 +-
 remote.c | 18 +++---
 remote.h |  4 +++-
 t/t6040-tracking-info.sh | 29 +
 wt-status.c  |  2 +-
 6 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index ea029ad..9a2f209 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -120,6 +120,9 @@ configuration variable documented in linkgit:git-config[1].
 +
In short format with --branch, '[different]' will printed rather
than detailed ahead/behind counts.
++
+   In long (normal) format, a simple out of date message will be
+   printed rather than detailed ahead/behind counts.
 
 ...::
See the 'pathspec' entry in linkgit:gitglossary[7].
diff --git a/builtin/checkout.c b/builtin/checkout.c
index fc4f8fd..a3e7bde 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -605,7 +605,7 @@ static void report_tracking(struct branch_info *new)
struct strbuf sb = STRBUF_INIT;
struct branch *branch = branch_get(new->name);
 
-   if (!format_tracking_info(branch, ))
+   if (!format_tracking_info(branch, 0, ))
return;
fputs(sb.buf, stdout);
strbuf_release();
diff --git a/remote.c b/remote.c
index 0a63ac1..b75e62f 100644
--- a/remote.c
+++ b/remote.c
@@ -2065,14 +2065,20 @@ int stat_tracking_info(struct branch *branch, int 
*num_ours, int *num_theirs,
 /*
  * Return true when there is anything to report, otherwise false.
  */
-int format_tracking_info(struct branch *branch, struct strbuf *sb)
+int format_tracking_info(struct branch *branch, int no_ahead_behind,
+struct strbuf *sb)
 {
int ours, theirs;
const char *full_base;
char *base;
int upstream_is_gone = 0;
+   int sti;
 
-   if (stat_tracking_info(branch, , , _base) < 0) {
+   if (no_ahead_behind)
+   sti = stat_tracking_info(branch, NULL, NULL, _base);
+   else
+   sti = stat_tracking_info(branch, , , _base);
+   if (sti < 0) {
if (!full_base)
return 0;
upstream_is_gone = 1;
@@ -2086,10 +2092,16 @@ int format_tracking_info(struct branch *branch, struct 
strbuf *sb)
if (advice_status_hints)
strbuf_addstr(sb,
_("  (use \"git branch --unset-upstream\" to 
fixup)\n"));
-   } else if (!ours && !theirs) {
+   } else if (!sti) {
strbuf_addf(sb,
_("Your branch is up to date with '%s'.\n"),
base);
+   } else if (no_ahead_behind) {
+   strbuf_addf(sb, _("Your branch is out of date with '%s'.\n"),
+   base);
+
+   /* TODO Do we need a generic hint here? */
+
} else if (!theirs) {
strbuf_addf(sb,
Q_("Your branch is ahead of '%s' by %d commit.\n",
diff --git a/remote.h b/remote.h
index 2ecf4c8..559649d 100644
--- a/remote.h
+++ b/remote.h
@@ -258,7 +258,9 @@ enum match_refs_flags {
 /* Reporting of tracking info */
 int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
   const char **upstream_name);
-int format_tracking_info(struct branch *branch, struct strbuf *sb);
+
+int format_tracking_info(struct branch *branch, int no_ahead_behind,
+struct strbuf *sb);
 
 struct ref *get_local_heads(void);
 /*
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 0190220..00fbd0a 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -160,6 +160,35 @@ test_expect_success 'status -s -b --no-ahead-behind 
(diverged from upstream)' '
 '
 
 cat >expect <<\EOF
+On branch b1
+Your branch and 'origin/master' have diverged,
+and have 1 and 1 different commits each, respectively.
+EOF
+
+test_expect_success 'status --long --branch' '
+   (
+   cd test &&
+   git checkout b1 >/dev/null &&
+   git status --long -b | head -3
+   ) >actual &&
+   test_i18ncmp expect actual
+'
+
+cat >expect <<\EOF
+On branch b1
+Your branch is out of date with 'origin/master'.
+EOF
+
+test_expect_success 'status --long --branch --no-ahead-behind' '
+   (
+   cd 

[PATCH 3/4] status: update short status to use --no-ahead-behind

[Jeff Hostetler]

From: Jeff Hostetler 

Teach "git status --short --branch" to use "--no-ahead-behind"
flag to skip computing ahead/behind counts for the branch and
its upstream and just report '[different]'.

Signed-off-by: Jeff Hostetler 
---
 Documentation/git-status.txt |  3 +++
 remote.c | 12 +---
 t/t6040-tracking-info.sh | 13 +
 wt-status.c  | 11 +--
 4 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 6ce8cf8..ea029ad 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -117,6 +117,9 @@ configuration variable documented in linkgit:git-config[1].
expensive computation on extremely large repositories.
 +
In porcelain V2 format, the 'branch.ab' line will not be present.
++
+   In short format with --branch, '[different]' will printed rather
+   than detailed ahead/behind counts.
 
 ...::
See the 'pathspec' entry in linkgit:gitglossary[7].
diff --git a/remote.c b/remote.c
index a38b42e..0a63ac1 100644
--- a/remote.c
+++ b/remote.c
@@ -1978,9 +1978,12 @@ int ref_newer(const struct object_id *new_oid, const 
struct object_id *old_oid)
 
 /*
  * Compare a branch with its upstream, and save their differences (number
- * of commits) in *num_ours and *num_theirs. The name of the upstream branch
- * (or NULL if no upstream is defined) is returned via *upstream_name, if it
- * is not itself NULL.
+ * of commits) in *num_ours and *num_theirs.  If either num_ours or num_theirs
+ * are NULL, we skip counting the commits and just return whether they are
+ * different.
+ *
+ * The name of the upstream branch (or NULL if no upstream is defined) is
+ * returned via *upstream_name, if it is not itself NULL.
  *
  * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no
  * upstream defined, or ref does not exist).
@@ -2016,6 +2019,9 @@ int stat_tracking_info(struct branch *branch, int 
*num_ours, int *num_theirs,
if (!ours)
return -1;
 
+   if (!num_ours || !num_theirs)
+   return theirs != ours;
+
/* are we the same? */
if (theirs == ours) {
*num_theirs = *num_ours = 0;
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 8f17fd9..0190220 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -147,6 +147,19 @@ test_expect_success 'status -s -b (diverged from 
upstream)' '
 '
 
 cat >expect <<\EOF
+## b1...origin/master [different]
+EOF
+
+test_expect_success 'status -s -b --no-ahead-behind (diverged from upstream)' '
+   (
+   cd test &&
+   git checkout b1 >/dev/null &&
+   git status -s -b --no-ahead-behind | head -1
+   ) >actual &&
+   test_i18ncmp expect actual
+'
+
+cat >expect <<\EOF
 ## b5...brokenbase [gone]
 EOF
 
diff --git a/wt-status.c b/wt-status.c
index 471ba15..6b4f969 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1767,6 +1767,7 @@ static void wt_shortstatus_print_tracking(struct 
wt_status *s)
const char *branch_name;
int num_ours, num_theirs;
int upstream_is_gone = 0;
+   int sti;
 
color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "## ");
 
@@ -1791,7 +1792,11 @@ static void wt_shortstatus_print_tracking(struct 
wt_status *s)
 
color_fprintf(s->fp, branch_color_local, "%s", branch_name);
 
-   if (stat_tracking_info(branch, _ours, _theirs, ) < 0) {
+   if (s->no_ahead_behind)
+   sti = stat_tracking_info(branch, NULL, NULL, );
+   else
+   sti = stat_tracking_info(branch, _ours, _theirs, );
+   if (sti < 0) {
if (!base)
goto conclude;
 
@@ -1803,12 +1808,14 @@ static void wt_shortstatus_print_tracking(struct 
wt_status *s)
color_fprintf(s->fp, branch_color_remote, "%s", short_base);
free(short_base);
 
-   if (!upstream_is_gone && !num_ours && !num_theirs)
+   if (!upstream_is_gone && !sti)
goto conclude;
 
color_fprintf(s->fp, header_color, " [");
if (upstream_is_gone) {
color_fprintf(s->fp, header_color, LABEL(N_("gone")));
+   } else if (s->no_ahead_behind) {
+   color_fprintf(s->fp, header_color, LABEL(N_("different")));
} else if (!num_ours) {
color_fprintf(s->fp, header_color, LABEL(N_("behind ")));
color_fprintf(s->fp, branch_color_remote, "%d", num_theirs);
-- 
2.9.3

[PATCH 0/4] Add --no-ahead-behind to status

[Jeff Hostetler]

From: Jeff Hostetler 

This patch series adds a "--no-ahead-behind" option to status
to request that it avoid a possibly expensive ahead/behind
computation for the current branch.  Instead, it just prints a
not up to date message in place of the detailed counts.

This idea was previously discussed in [1].  Working with the
enormous Windows repository, we found that 20+ seconds was being
spent in the ahead/behind computation when the current branch was
150K commits behind the upstream branch.  (Yes, this happens and
only took 3 weeks on the reporter's system.)


I've only modified "git status" in this patch series.  A similar
change could be added to "git branch -vv" and "git checkout" to
avoid delays there too.  I avoided doing it here to keep this
patch series focused.


[1] 
https://public-inbox.org/git/030bf57c-7a23-3391-4fc0-93efee791...@jeffhostetler.com/T/

Jeff Hostetler (4):
  status: add --no-ahead-behind to porcelain V2
  stat_tracking_info: return +1 when branch and upstream differ
  status: update short status to use --no-ahead-behind
  status: support --no-ahead-behind in long status format.

 Documentation/config.txt |  5 +
 Documentation/git-status.txt | 16 
 builtin/checkout.c   |  2 +-
 builtin/commit.c |  6 ++
 ref-filter.c |  4 ++--
 remote.c | 36 
 remote.h |  4 +++-
 t/t6040-tracking-info.sh | 42 ++
 t/t7064-wtstatus-pv2.sh  | 23 +++
 wt-status.c  | 24 
 wt-status.h  |  1 +
 11 files changed, 147 insertions(+), 16 deletions(-)

-- 
2.9.3

[PATCH 2/4] stat_tracking_info: return +1 when branch and upstream differ

[Jeff Hostetler]

From: Jeff Hostetler 

Extend stat_tracking_info() to return 1 when the branch is
not up to date with its upstream branch and only return 0
when they are equal.

Signed-off-by: Jeff Hostetler 
---
 ref-filter.c | 4 ++--
 remote.c | 6 --
 wt-status.c  | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index e728b15..10ab4cd 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1239,7 +1239,7 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
*s = show_ref(>u.remote_ref.refname, refname);
else if (atom->u.remote_ref.option == RR_TRACK) {
if (stat_tracking_info(branch, _ours,
-  _theirs, NULL)) {
+  _theirs, NULL) < 0) {
*s = xstrdup(msgs.gone);
} else if (!num_ours && !num_theirs)
*s = "";
@@ -1257,7 +1257,7 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
}
} else if (atom->u.remote_ref.option == RR_TRACKSHORT) {
if (stat_tracking_info(branch, _ours,
-  _theirs, NULL))
+  _theirs, NULL) < 0)
return;
 
if (!num_ours && !num_theirs)
diff --git a/remote.c b/remote.c
index b220f0d..a38b42e 100644
--- a/remote.c
+++ b/remote.c
@@ -1983,7 +1983,9 @@ int ref_newer(const struct object_id *new_oid, const 
struct object_id *old_oid)
  * is not itself NULL.
  *
  * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no
- * upstream defined, or ref does not exist), 0 otherwise.
+ * upstream defined, or ref does not exist).
+ * Returns 0 if the commits are the same (the branch is up to date).
+ * Returns 1 if the commits are different (the branch is not up to date).
  */
 int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
   const char **upstream_name)
@@ -2051,7 +2053,7 @@ int stat_tracking_info(struct branch *branch, int 
*num_ours, int *num_theirs,
clear_commit_marks(theirs, ALL_REV_FLAGS);
 
argv_array_clear();
-   return 0;
+   return 1;
 }
 
 /*
diff --git a/wt-status.c b/wt-status.c
index 1bc53e1..471ba15 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1935,7 +1935,7 @@ static void wt_porcelain_v2_print_tracking(struct 
wt_status *s)
base = branch_get_upstream(branch, NULL);
ab_info = 0;
} else {
-   ab_info = (stat_tracking_info(branch, _ahead, 
_behind, ) == 0);
+   ab_info = (stat_tracking_info(branch, _ahead, 
_behind, ) >= 0);
}
 
if (base) {
-- 
2.9.3

[PATCH 1/4] status: add --no-ahead-behind to porcelain V2

[Jeff Hostetler]

From: Jeff Hostetler 

Teach "status --porcelain=v2 --branch" to omit "# branch.ab x y"
when "--no-ahead-behind" argument is used.

This allows the user to omit the (possibly extremely expensive)
ahead/behind computation when not needed.

Signed-off-by: Jeff Hostetler 
---
 Documentation/config.txt |  5 +
 Documentation/git-status.txt | 10 ++
 builtin/commit.c |  6 ++
 t/t7064-wtstatus-pv2.sh  | 23 +++
 wt-status.c  | 11 ++-
 wt-status.h  |  1 +
 6 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9593bfa..9ccdf2b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3082,6 +3082,11 @@ status.submoduleSummary::
submodule summary' command, which shows a similar output but does
not honor these settings.
 
+status.noaheadbehind::
+   Do not compute ahead/behind counts for a branch relative to its
+   upstream branch.  This can be used to avoid a possibly very
+   expensive computation on extremely large repositories.
+
 stash.showPatch::
If this is set to true, the `git stash show` command without an
option will show the stash entry in patch form.  Defaults to false.
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 9f3a78a..6ce8cf8 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -111,6 +111,13 @@ configuration variable documented in linkgit:git-config[1].
without options are equivalent to 'always' and 'never'
respectively.
 
+--no-ahead-behind::
+   Do not compute ahead/behind counts for the current branch relative
+   to the upstream branch.  This can be used to avoid a possibly very
+   expensive computation on extremely large repositories.
++
+   In porcelain V2 format, the 'branch.ab' line will not be present.
+
 ...::
See the 'pathspec' entry in linkgit:gitglossary[7].
 
@@ -253,6 +260,9 @@ information about the current branch.
 the commit is present.
 
 
+If `--no-ahead-behind` is given or 'status.noaheadbehind' is set, the
+'branch.ab' line will not be present.
+
 ### Changed Tracked Entries
 
 Following the headers, a series of lines are printed for tracked
diff --git a/builtin/commit.c b/builtin/commit.c
index be370f6..99ca5cb 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1335,6 +1335,10 @@ static int git_status_config(const char *k, const char 
*v, void *cb)
return error(_("Invalid untracked files mode '%s'"), v);
return 0;
}
+   if (!strcmp(k, "status.noaheadbehind")) {
+   s->no_ahead_behind = git_config_bool(k, v);
+   return 0;
+   }
return git_diff_ui_config(k, v, NULL);
 }
 
@@ -1369,6 +1373,8 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
  N_("ignore changes to submodules, optional when: all, dirty, 
untracked. (Default: all)"),
  PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
OPT_COLUMN(0, "column", , N_("list untracked files in 
columns")),
+   OPT_BOOL(0, "no-ahead-behind", _ahead_behind,
+N_("omit branch ahead/behind counts")),
OPT_END(),
};
 
diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
index e319fa2..4be2b20 100755
--- a/t/t7064-wtstatus-pv2.sh
+++ b/t/t7064-wtstatus-pv2.sh
@@ -390,6 +390,29 @@ test_expect_success 'verify upstream fields in branch 
header' '
)
 '
 
+test_expect_success 'verify --no-ahead-behind omits branch.ab' '
+   git checkout master &&
+   test_when_finished "rm -rf sub_repo" &&
+   git clone . sub_repo &&
+   (
+   ## Confirm local master tracks remote master.
+   cd sub_repo &&
+   HUF=$(git rev-parse HEAD) &&
+
+   cat >expect <<-EOF &&
+   # branch.oid $HUF
+   # branch.head master
+   # branch.upstream origin/master
+   EOF
+
+   git status --no-ahead-behind --porcelain=v2 --branch 
--untracked-files=all >actual &&
+   test_cmp expect actual &&
+
+   git -c status.noaheadbehind=true status --porcelain=v2 --branch 
--untracked-files=all >actual &&
+   test_cmp expect actual
+   )
+'
+
 test_expect_success 'create and add submodule, submodule appears clean (A. 
S...)' '
git checkout master &&
git clone . sub_repo &&
diff --git a/wt-status.c b/wt-status.c
index 94e5eba..1bc53e1 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1889,6 +1889,8 @@ static void wt_porcelain_print(struct wt_status *s)
  *  ::= NUL when -z,
  *   LF 

Re: [PATCH] doc: Fix specification of default (auto) for --color

[Jeff King]

On Wed, Dec 20, 2017 at 05:14:02AM -0800, William Pursell wrote:

> Current documentation for branch, show-branch, and grep contain the following:
> 
> --color[=]
>Show colored matches. The value must be always (the
> default), never, or auto.
> 
> This is incorrect, as the default is "auto".

I think what this is trying to say is that if you specify "--color" but
not "", then "" defaults to "always". I.e., this is talking
about a different default than if you didn't specify "--color" at all
(and that default relies on the config, which in turn defaults to
"auto").

The wording is quite confusing, though. Maybe we could clarify that and
mention both "defaults":

  --color[=]
Show colored matches. The value of `` must be `always`,
`never`, or `auto`. If `` is omitted (i.e., just
`--color`), behave as if `--color=always` was specified. If no
`--color` option is given, defaults, to the value of the
`color.grep` config variable.

There may be a less clunky way of saying all that, though.

-Peff

[PATCH] doc: Fix specification of default (auto) for --color

[William Pursell]

Current documentation for branch, show-branch, and grep contain the following:

--color[=]
   Show colored matches. The value must be always (the
default), never, or auto.

This is incorrect, as the default is "auto".

Signed-off-by: William Pursell 
---
 Documentation/git-branch.txt  | 2 +-
 Documentation/git-grep.txt| 2 +-
 Documentation/git-show-branch.txt | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index b3084c99c..1e4dc3d91 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -128,7 +128,7 @@ OPTIONS
 --color[=]::
  Color branches to highlight current, local, and
  remote-tracking branches.
- The value must be always (the default), never, or auto.
+ The value must be always, never, or auto (the default).

 --no-color::
  Turn off branch colors, even when the configuration file gives the
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 18b494731..cb62fa30e 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -201,7 +201,7 @@ providing this option will cause it to die.

 --color[=]::
  Show colored matches.
- The value must be always (the default), never, or auto.
+ The value must be always, never, or auto (the default).

 --no-color::
  Turn off match highlighting, even when the configuration file
diff --git a/Documentation/git-show-branch.txt
b/Documentation/git-show-branch.txt
index 7818e0f09..4a7be5f0c 100644
--- a/Documentation/git-show-branch.txt
+++ b/Documentation/git-show-branch.txt
@@ -119,7 +119,7 @@ OPTIONS
 --color[=]::
  Color the status sign (one of these: `*` `!` `+` `-`) of each commit
  corresponding to the branch it's in.
- The value must be always (the default), never, or auto.
+ The value must be always, never, or auto (the default).

 --no-color::
  Turn off colored output, even when the configuration file gives the

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

[Jeff King]

On Tue, Dec 19, 2017 at 10:33:55AM -0800, Junio C Hamano wrote:

> >> Why does prepare_revision_walk() clear the list of pending objects at
> >> all?  Assuming the list is append-only then perhaps remembering the
> >> last handled index would suffice.
> 
> For that matter, why does it copy, instead of using revs->pending
> directly?  I do not think I can answer this, as I think the design
> decisions led to this code predates me.

Me too, then. :) I think part of that copy is that we're moving the
items over to revs->commits, and dropping any non-commit objects.

> In any case, it seems that the discussion has veered into an
> interesting tangent but at this point it seems to me that it is not
> likely to produce an immediate improvement over the posted patch.

Fair enough.

> Should we take the patch posted as-is and move forward?

I suppose so.  I don't really have anything against the patch. My main
complaint was just that I don't think it's actually cleaning up the
gross parts of the interface. It's just substituting one gross thing (a
funny struct flag) for another (a special version of the prepare
function that takes a funny out-parameter).

-Peff

Re: [PATCH] fmt-merge-msg: avoid leaking strbuf in shortlog()

[Jeff King]

On Tue, Dec 19, 2017 at 07:26:06PM +0100, René Scharfe wrote:

> > That's the same duality we have now with string_list.
> 
> Hmm, I thought we *were* discussing string_list?

Right, I guess what I was wondering is if a wrapper over string_list
really ends up any better than having the dual-natured string_list.

If they both use the same struct, then your wrappers are all just
functions. And isn't that more or less what we have now?

If they're actually different structs, then that complicates call
signatures for functions that take a list (unless we are getting into
polymorphism, they need to specify one of the types, even if they don't
particularly care whether it's an allocated list or not).

-Peff

Re: Need help migrating workflow from svn to git.

[Josef Wolf]

On Wed, Dec 20, 2017 at 12:43:37PM +0100, Josef Wolf wrote:
> Thanks to you both for your patience with me. Sorry for the late reply, my day
> job was needing me ;-)
> 
> On Fri, Dec 15, 2017 at 07:58:14PM +0100, Igor Djordjevic wrote:
> > On 15/12/2017 17:33, Junio C Hamano wrote:
> > > 
> > > $ git fetch  
> > > $ git checkout -m -B  FETCH_HEAD
> 
> For some reason, this seems to double the local modifications. After executing
> the following commands:

Umm... Please ignore this "doubling" comment. My test script was faulty :-//

Sorry for the confusion!

-- 
Josef Wolf
j...@raven.inka.de

Re: Need help migrating workflow from svn to git.

[Josef Wolf]

On Fri, Dec 15, 2017 at 11:09:17AM -0800, Junio C Hamano wrote:
> Igor Djordjevic  writes:
> 
> > Junio, what about consecutive runs, while merge conflicts are still 
> > unresolved?
> 
> The impression I got was that the original running with svn does not
> deal with conflicting situation anyway, so I did not think about it
> at all, and I personally do not care ;-)

Yeah. Conflicts are not a big deal for this project.

I guess, this would no longer hold true if there are lots of developers with
lots of commits. Thus, I'd still like to learn how to do it correctly.

Never doing local modifications in the "live" directories would require to
install test systems and simulate their environment just for doing some minor
adjustments.

-- 
Josef Wolf
j...@raven.inka.de

[PATCH v5] Makefile: replace perl/Makefile.PL with simple make rules

[Ævar Arnfjörð Bjarmason]

Replace the perl/Makefile.PL and the fallback perl/Makefile used under
NO_PERL_MAKEMAKER=NoThanks with a much simpler implementation heavily
inspired by how the i18n infrastructure's build process works[1].

The reason for having the Makefile.PL in the first place is that it
was initially[2] building a perl C binding to interface with libgit,
this functionality, that was removed[3] before Git.pm ever made it to
the master branch.

We've since since started maintaining a fallback perl/Makefile, as
MakeMaker wouldn't work on some platforms[4]. That's just the tip of
the iceberg. We have the PM.stamp hack in the top-level Makefile[5] to
detect whether we need to regenerate the perl/perl.mak, which I fixed
just recently to deal with issues like the perl version changing from
under us[6].

There is absolutely no reason for why this needs to be so complex
anymore. All we're getting out of this elaborate Rube Goldberg machine
was copying perl/* to perl/blib/* as we do a string-replacement on
the *.pm files to hardcode @@LOCALEDIR@@ in the source, as well as
pod2man-ing Git.pm & friends.

So replace the whole thing with something that's pretty much a copy of
how we generate po/build/**.mo from po/*.po, just with a small sed(1)
command instead of msgfmt.

While I'm at it, change the fallback for Error.pm from being something
where we'll ship our own Error.pm if one doesn't exist at build time
to one where we just use a Git::Error wrapper that'll always prefer
the system-wide Error.pm, only falling back to our own copy if it
really doesn't exist at runtime. It's now shipped as
Git::FromCPAN::Error, making it easy to add other modules to
Git::FromCPAN::* in the future if that's needed.

Functional changes:

 * This will not always install into perl's idea of its global
   "installsitelib". This only potentially matters for packagers that
   need to expose Git.pm for non-git use, and as explained in the
   INSTALL file there's a trivial workaround.

 * The scripts themselves will 'use lib' the target directory, but if
   INSTLIBDIR is set it overrides it. It doesn't have to be this way,
   it could be set in addition to INSTLIBDIR, but my reading of [7] is
   that this is the desired behavior.

 * We don't build man pages for all of the perl modules as we used to,
   only Git(3pm). As discussed on-list[8] that we were building
   installed manpages for purely internal APIs like Git::I18N or
   private-Error.pm was always a bug anyway, and all the Git::SVN::*
   ones say they're internal APIs.

   There are apparently external users of Git.pm, but I don't expect
   there to be any of the others.

   As a side-effect of these general changes the perl documentation
   now only installed by install-{doc,man}, not a mere "install" as
   before.

1. 5e9637c629 ("i18n: add infrastructure for translating Git with
   gettext", 2011-11-18)

2. b1edc53d06 ("Introduce Git.pm (v4)", 2006-06-24)

3. 18b0fc1ce1 ("Git.pm: Kill Git.xs for now", 2006-09-23)

4. f848718a69 ("Make perl/ build procedure ActiveState friendly.",
   2006-12-04)

5. ee9be06770 ("perl: detect new files in MakeMaker builds",
   2012-07-27)

6. c59c4939c2 ("perl: regenerate perl.mak if perl -V changes",
   2017-03-29)

7. 0386dd37b1 ("Makefile: add PERLLIB_EXTRA variable that adds to
   default perl path", 2013-11-15)

8. 87bmjjv1pu@evledraar.booking.com ("Re: [PATCH] Makefile:
   replace perl/Makefile.PL with simple make rules"

Signed-off-by: Ævar Arnfjörð Bjarmason 
---

On Wed, Dec 20, 2017 at 7:15 AM, Todd Zullinger  wrote:

[Sorry for not CC-ing you on the last version. Forgot to update the
giant send-email line I'm juggling for this series].

> FWIW, I applied this version to next and tested it with
> CentOS 6 and 7.  The tests pass on both (though there are
> some unrelated failures on CentOS 6 in t5700-protocol-v1,
> which I haven't looked into further yet).
>
> I also applied this patch to 2.15.1 and ran the tests in the
> Fedora build system for all fedora and epel releases, which
> also passed (though with some spurious git-svn failures on
> x86_64 in fedora 28, AKA rawhide).

Great, thanks a lot.

> The .pmc extensions seem to cause rpm to fail to parse the
> files for rpm 'provides' as it normally would.  This causes
> scripts like git-send-email which generates a 'requires' on
> Git::Error to fail to find anything which provides it.
>
> I'm not familiar with the .pmc extenstion.  Searching the
> fedora repositories, there is only one other package - and
> one file within it - which has a .pmc extension.
>
> (The package is perl-test, the file is
> /usr/libexec/perl5-tests/perl-tests/t/run/flib/t2.pmc.)
>
> Perhaps it's a bug in rpm's perl dependency generator, but
> I'd like to think that git wouldn't be the first package to
> find it.
>
> Is the .pmc extension important to ensure these files are
> loaded in the right order?  Since they're all in the Git
> namespace, I don't imagine there should be anything 

Re: Need help migrating workflow from svn to git.

[Josef Wolf]

Thanks to you both for your patience with me. Sorry for the late reply, my day
job was needing me ;-)

On Fri, Dec 15, 2017 at 07:58:14PM +0100, Igor Djordjevic wrote:
> On 15/12/2017 17:33, Junio C Hamano wrote:
> > 
> > $ git fetch  
> > $ git checkout -m -B  FETCH_HEAD

For some reason, this seems to double the local modifications. After executing
the following commands:


  rm -rf reposA reposB
  
  git init reposA
  (
cd reposA
echo 1 >>1
echo 2 >>2
git add 1 2
git commit -m1
  )
  
  git clone reposA reposB
  
  (
cd reposA
echo 1 >>1
git commit -a -m2
  )
  
  (
cd reposB
echo 3 >>2
git fetch
git checkout -m -B master FETCH_HEAD
  )

git-diff gives me:

  $ diff --git a/2 b/2
  index 0cfbf08..4e8a2de 100644
  --- a/2
  +++ b/2
  @@ -1 +1,3 @@
   2
  +3
  +3

With Igor's set of commands, I did not see this doubling:

> git checkout -b temp &&   #1
> git fetch &&  #2
> git branch -f master origin/master && #3
> git checkout -m master && #4
> git add -u && #5
> git reset &&  #6
> git branch -d temp#7

> ... aaand that`s how you do it[1] without a temporary branch :)
> 
> Junio, what about consecutive runs, while merge conflicts are still 
> unresolved?
> 
> Seeing Josef having a pretty relaxed flow, and his cron job running 
> every 15 minutes, would adding something like:
> 
> $ git add -u
> $ git reset

This would be added after the "git checkout -m -B master FETCH_HEAD" command?

> ... to the mix, to "silence" actually still unresolved merge 
> conflicts, making next script execution possible, make sense?
> 
> Yes, `git diff` won`t be the same as if conflicts were still in, but 
> it might be worth it in this specific case, conflicting parts still 
> easily visible between conflict markers.

That means, the conflict is still there, but git would think this is an
ordinary modification?

-- 
Josef Wolf
j...@raven.inka.de

git merge commits staged files (when two trees are identical)

[Andreas Krey]

Hi everybody,

we just stumbled over a situation in which a merge commits
staged changes into the merge commit. This happens when the
merged-in branch does have commits ('main') but has the same
tree ('--allow-empty') as the merge base:

git init
echo eins >a
git add a
git commit -m initial
git branch sub
git commit -m main --allow-empty
git checkout sub
: two
echo zwei >>a
git add a
git commit -m underway
: three
echo drei >>a
git add a  # important
git status
git diff --cached
git merge master -m 'merge'
git status
git log --cc -1

If the change isn't staged (comment out the '# important' line)
the change survives as unstaged.

That is a bug?

- Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds 
Date: Fri, 22 Jan 2010 07:29:21 -0800

Re: [PATCH] http: support CURLPROXY_HTTPS

[Jeff King]

On Wed, Dec 20, 2017 at 10:22:06AM +0800, Wei Shuyu wrote:

> On 2017-12-20 04:59, Jonathan Nieder wrote:
> 
> > Thanks for writing this.  Can you give an example of how I'd use it
> > (ideally in the form of a test in t/ so we avoid this functionality
> > regressing, but if that's not straightforward then an example for the
> > commit message is fine as well)?
> 
> Hi Jonathan,
> Its usage is the same as other protocols. Just set http.proxy or
> http_proxy/https_proxy
> environment to https://url.
> 
> To use apache server as a proxy, just add `ProxyRequests On` to an https
> site.

Unfortunately I don't think we have any proxy tests at all in our test
suite right now. The sticking point is that we need an actual proxy to
test against. :)

If it really is as simple as "ProxyRequests On", then we might be able
to convince the existing apache process we run to proxy requests to
itself (perhaps on a secondary port?).

-Peff

Re: [PATCH] path: document path functions

[Eric Sunshine]

On Wed, Dec 13, 2017 at 1:28 PM, Brandon Williams  wrote:
> As promised here is an update to the documentation for the path generating
> functions.

Thanks for working on this. Aside from the review comments by
Junio[1], see a few more below. Although I had some familiarity with
the non-repo versions of these functions a couple year ago when
working on git-worktree, I've since forgotten much of it, so take my
comments as if from someone trying to learn this API from scratch
(which is effectively what I'm doing again). In particular, as I read
this patch, I find that I still have to consult the source code
(path.c) to figure out what some of these functions are for or what
they do.

[1]: https://public-inbox.org/git/xmqqr2rywid7@gitster.mtv.corp.google.com/

> diff --git a/path.h b/path.h
> @@ -4,53 +4,144 @@
>  /*
> + * The result to all functions which return statically allocated memory may 
> be
> + * overwritten by another call to _any_ one of these functions. Consider 
> using
> + * the safer variants which operate on strbufs or return allocated memory.
>   */
>
> +/*
> + * Return a statically allocated path.
> + */
> +extern const char *mkpath(const char *fmt, ...)
> +   __attribute__((format (printf, 1, 2)));

It's not, at first glance, clear what benefit this function provides
over simply constructing a path manually with, say, sprintf() or
strbuf. Should this mention that a certain amount of normalization is
performed on the path?

Furthermore, perhaps the comment block above which talks about
"statically allocated memory" could do a better job of conveying that
these functions rid the caller of the responsibility of managing
string storage itself. I wonder if it also might make sense to mention
that the returned path won't necessarily be overwritten by the very
next call (or is that an implementation detail we'd rather people not
rely upon?).

> +/*
> + * Return a path.
> + */
> +extern char *mkpathdup(const char *fmt, ...)
> +   __attribute__((format (printf, 1, 2)));

I realize that it's implied by "dup", but perhaps the documentation
should state explicitly that the caller is responsible for freeing the
result?

> +/*
> + * The `git_common_path` family of functions will construct a path into a
> + * repository's common git directory, which is shared by all worktrees.
> + */

My first question when reading this was "What is a common git
directory? (Is that where a bunch of repositories live or what?)". I
suppose if it had said "common .git/ directory", it would have been
clearer. Perhaps mentioning GIT_DIR might help clarify it, as well.

...repository's common .git/ directory (or $GIT_DIR), which
is shared by all worktrees.

> +/*
> + * Constructs a path into the common git directory of repository `repo` and
> + * append it in the provided buffer `sb`.
> + */
>  extern void strbuf_git_common_path(struct strbuf *sb,
>const struct repository *repo,
>const char *fmt, ...)
> __attribute__((format (printf, 3, 4)));

It's nice that this states explicitly that it _appends_ rather than
overwrites. So often, one has to consult the implementation to
determine the actual behavior.

Perhaps I'm simple-minded, but at this point in the read, the concept
of "common git directory" still feels nebulous. If the documentation
presented even a simple example of how this is used (such as, for
instance, calling this function to obtain ".git/HEAD"), it would help
make the concept more concrete. Or, perhaps the example could be
presented in the comment block just above which talks about this
family of functions.

> +/*
> + * Return a statically allocated path into the main repository's
> + * (the_repository) common git directory.
> + */
> +extern const char *git_common_path(const char *fmt, ...)
> __attribute__((format (printf, 1, 2)));

I suppose that other repositories would be submodules? Would it make
sense to mention something about that to clue in readers?

Return a statically allocated path into the common .git/ directory
of the main repository (not a submodule repository).

Or something.

Aside: After reading "main repository" here and seeing the 'repo'
variants below, one wonders why the corresponding
repo_git_common_path() doesn't exist. (Just an idle question; not
actionable, and outside the scope of this patch.)

> +/*
> + * The `git_path` family of functions will construct a path into a 
> repository's
> + * git directory.
> + *
> + * These functions will perform adjustments to the resultant path to account
> + * for special paths which are either considered common among worktrees (e.g.
> + * paths into the object directory) or have been explicitly set via an
> + * environment variable or config (e.g. path to the index file).
> + *
> + * For an exhaustive list of the adjustments made look at `common_list` and
> + * `adjust_git_path` in path.c.
> + */

I feel somewhat 

Re: [PATCH] Re: Bug with "git submodule update" + subrepo with differing path/name?

[Andreas Urke]

Thanks for looking into this.

I was able to reproduce it from scratch, but I followed my earlier
workflow where I first created the subrepos, and then later renamed
it. At the time I was not able to find any command to rename without
changing the path (and I was not able find one now either, is there
any?), so I edited name and path in .gitmodules and ran git submodule
sync. Am I asking for trouble doing it that way?

Let me know if you need the exact steps I followed.

Andreas


On 19 December 2017 at 23:33, Stefan Beller  wrote:
> On Tue, Dec 19, 2017 at 2:19 PM, Junio C Hamano  wrote:
>> Stefan Beller  writes:
>>
>>> I tried reproducing the issue (based on the `next` branch, not 2.15,
>>> but I do not recall any changes in the submodule area lately), and
>>> could not come up with a reproduction recipe,...
>>
>> I do not offhand recall anything; the closest I can think of is the
>> three-patch series <20171016135623.ga12...@book.hvoigt.net> that
>> taught the on-demand recursive fetch to pay attention to the location
>> in the superproject tree a submodule is bound to.
>
> I tried the same test on 2.15 and cannot reproduce there either.
>
>>
>> 4b4acedd61 submodule: simplify decision tree whether to or not to fetch
>> c68f837576 implement fetching of moved submodules
>> 01ce12252c fetch: add test to make sure we stay backwards compatible
>>
>> But IIRC, "submodule update" uses a separate codepath?
>
> Yes, any portion of git-submodule.sh that calls out to C is going
> through the submodule--helper. I want to revive the port of that
> shell script to C again.
>
> The "submodule update" uses the submodule helper to obtain
> the list of submodules and then does a "git -C $sub fetch" in there.
>
> Stefan