Re: [PATCH] send-email: error out when relogin delay is missing

[xiaoqiang zhao]

> 在 2018年2月8日，上午7:43，Stefan Beller  写道：
> 
> +die __("When a batch size is given, the relogin delay must be set\n")
> +if defined $relogin_delay and not defined $batch_size;
> +
 
According the code, maybe you want to say “When relogin delay is given, a batch 
size must be set “ ?

[PATCH] docs/interpret-trailers: fix agreement error

[brian m. carlson]

In the description of git interpret-trailers, we describe "a group…of
lines" that have certain characteristics.  Because the first option uses
a plural verb (referring to "lines"), the second option must also use
plural verbs for parallelism.

Signed-off-by: brian m. carlson 
---
I'm somewhat on the fence about this patch.  To me, the number
disagreement is very jarring.  However, I'm also sympathetic to the fact
that the latter sentence reads more naturally in the singular.

Opinions on improvements welcome.

 Documentation/git-interpret-trailers.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-interpret-trailers.txt 
b/Documentation/git-interpret-trailers.txt
index 9dd19a1dd9..de5011e564 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -51,8 +51,8 @@ with only spaces at the end of the commit message part, one 
blank line
 will be added before the new trailer.
 
 Existing trailers are extracted from the input message by looking for
-a group of one or more lines that (i) are all trailers, or (ii) contains at
-least one Git-generated or user-configured trailer and consists of at
+a group of one or more lines that (i) are all trailers, or (ii) contain at
+least one Git-generated or user-configured trailer and consist of at
 least 25% trailers.
 The group must be preceded by one or more empty (or whitespace-only) lines.
 The group must either be at the end of the message or be the last

[PATCH v2] hash: update obsolete reference to SHA1_HEADER

[brian m. carlson]

We moved away from SHA1_HEADER to a preprocessor if chain, but didn't
update the comment discussing the platform defines.  Update this comment
so it reflects the current state of our codebase.

Signed-off-by: brian m. carlson 
---
 hash.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hash.h b/hash.h
index eb30f59be3..7c8238bc2e 100644
--- a/hash.h
+++ b/hash.h
@@ -18,8 +18,8 @@
 #ifndef platform_SHA_CTX
 /*
  * platform's underlying implementation of SHA-1; could be OpenSSL,
- * blk_SHA, Apple CommonCrypto, etc...  Note that including
- * SHA1_HEADER may have already defined platform_SHA_CTX for our
+ * blk_SHA, Apple CommonCrypto, etc...  Note that the relevant
+ * SHA-1 header may have already defined platform_SHA_CTX for our
  * own implementations like block-sha1 and ppc-sha1, so we list
  * the default for OpenSSL compatible SHA-1 implementations here.
  */

Re: [PATCH 025/194] object-store: allow prepare_alt_odb to handle arbitrary repositories

[brian m. carlson]

On Tue, Feb 06, 2018 at 09:48:56AM -0800, Stefan Beller wrote:
> On Mon, Feb 5, 2018 at 5:19 PM, brian m. carlson
>  wrote:
> > On Mon, Feb 05, 2018 at 03:54:46PM -0800, Stefan Beller wrote:
> >> @@ -434,12 +433,12 @@ static int link_alt_odb_entry_the_repository(const 
> >> char *entry,
> >>   ent = alloc_alt_odb(pathbuf.buf);
> >>
> >>   /* add the alternate entry */
> >> - *the_repository->objects.alt_odb_tail = ent;
> >> - the_repository->objects.alt_odb_tail = &(ent->next);
> >> + *r->objects.alt_odb_tail = ent;
> >> + r->objects.alt_odb_tail = &(ent->next);
> >>   ent->next = NULL;
> >
> > I'm sure I'm missing something obvious, but it's not clear to me that
> > this transformation is correct.  Could you perhaps say a few words about
> > why it is?
> 
> This is a pretty open ended question, so I'll give it a try:

I apologize.  My question was about the use of ent and ent->next, but it
appears I merely misread the patch as converting from ent to
&(ent->next), but that's not the case.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH v1] name-hash: properly fold directory names in adjust_dirname_case()

[David Turner]

On Wed, 2018-02-07 at 19:41 -0500, Ben Peart wrote:
> Correct the pointer arithmetic in adjust_dirname_case() so that it
> calls
> find_dir_entry() with the correct string length.  Previously passing
> in
> "dir1/foo" would pass a length of 6 instead of the correct 4.  This
> resulted in
> find_dir_entry() never finding the entry and so the subsequent memcpy
> that would
> fold the name to the version with the correct case never executed.
> 
> Add a test to validate the corrected behavior with name folding of
> directories.

Looks good to me.

[PATCH v1] name-hash: properly fold directory names in adjust_dirname_case()

[Ben Peart]

Correct the pointer arithmetic in adjust_dirname_case() so that it calls
find_dir_entry() with the correct string length.  Previously passing in
"dir1/foo" would pass a length of 6 instead of the correct 4.  This resulted in
find_dir_entry() never finding the entry and so the subsequent memcpy that would
fold the name to the version with the correct case never executed.

Add a test to validate the corrected behavior with name folding of directories.

Signed-off-by: Ben Peart 
---

Notes:
Base Ref: v2.16.1
Web-Diff: https://github.com/benpeart/git/commit/2944970f4e
Checkout: git fetch https://github.com/benpeart/git adjust_dirname-v1 && 
git checkout 2944970f4e

 name-hash.c   |  6 +++---
 t/t0050-filesystem.sh | 12 +++-
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/name-hash.c b/name-hash.c
index 45c98db0a0..2ddbb72647 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -696,12 +696,12 @@ void adjust_dirname_case(struct index_state *istate, char 
*name)
if (*ptr == '/') {
struct dir_entry *dir;
 
-   ptr++;
-   dir = find_dir_entry(istate, name, ptr - name + 1);
+   dir = find_dir_entry(istate, name, ptr - name);
if (dir) {
memcpy((void *)startPtr, dir->name + (startPtr 
- name), ptr - startPtr);
-   startPtr = ptr;
+   startPtr = ptr + 1;
}
+   ptr++;
}
}
 }
diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index b29d749bb7..219c96594c 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -80,7 +80,17 @@ test_expect_success 'merge (case change)' '
git merge topic
 '
 
-
+test_expect_success CASE_INSENSITIVE_FS 'add directory (with different case)' '
+   git reset --hard initial &&
+   mkdir -p dir1 &&
+   mkdir -p dir1/dir2 &&
+   touch dir1/dir2/a &&
+   touch dir1/dir2/b &&
+   git add dir1/dir2/a &&
+   git add dir1/DIR2/b &&
+   camel=$(git ls-files | grep dir2) &&
+   test $(echo "$camel" | wc -l) = 2
+'
 
 test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' '
git reset --hard initial &&

base-commit: 8279ed033f703d4115bee620dccd32a9ec94d9aa
-- 
2.15.0.windows.1

Re: Fetch-hooks

[Leo Gaspard]

On 02/07/2018 11:51 PM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Wed, Feb 07 2018, Leo Gaspard jotted:
> 
>> Hello,
>>
>> tl;dr: Is there currently a way to have fetch hooks, and if not do you
>> think it could be a nice feature?
>>
>> I was in the process of implementing hooks for git that ensure the
>> repository is always cleanly signed by someone allowed to by the
>> repository itself. I think I've completed the signature-checking part
>> [1] and the push hook [2] (even though it isn't really configurable at
>> the moment).
>>
>> However, I was starting to think about handling the fetch step, and
>> couldn't find any fetch hook. Is there one?
>>
>> If not, would you think it is would be a good idea to add one, that
>> would eg. be passed the commit-before, commit-after and could block the
>> changing of the reference if it failed?
>>
>> The only other solution I could think of is using a separate script for
>> fetching, but that would be fragile, as the user could always not think
>> about it well and run a git fetch, breaking the objective that after the
>> first clone all commits were correctly signature-checked.
>>
>> Thanks for reading me!
>> Leo
>>
>> PS1: I am not subscribed to the ML.
>>
>> PS2: I've tried asking freenode#git, without success so far.
>>
>>
>> [1]
>> https://github.com/Ekleog/signed-git/blob/master/git-hooks/check-range-signed.sh
>>
>> [2] https://github.com/Ekleog/signed-git/blob/master/git-hooks/pre-push
> 
> There is no fetch hook, however you may find that the
> post-{checkout,merge} hooks are suitable for what you want to do.
> 
> Setting those to some custom comand is a common pattern for
> e.g. compiling some assets on "git pull", so you could similarly check
> the commits from HEAD, of course those are post-* hooks, so they won't
> stop the checkout.

Hmm, I don't think these would fit the bill. For post-merge, simply
because I spend my life rebasing stuff around, and very rarely merge.
For post-checkout, it could work, but then I'd need to keep track
manually of up to where the commits have been checked and to search the
git graph for the latest checked ancestor (as otherwise checking-out
another branch then checking-out the first branch again would likely
trigger a failure, due to the keyring being dynamic), so it would likely
be a dealbreaker, due to the hook becoming too complex to be trusted.

(Just in case you wonder, by “the keyring being dynamic” I mean the PGP
keys allowed to sign commits are stored directly inside the git repository)

That said, I just came upon [1] (esp. the description [2] and the patch
[3]), and wondered: it looks like the patch was abandoned midway in
favor of a hook refactoring. Would you happen to know whether the hook
refactoring eventually took place, and/or whether this patch was
resubmitted later, and/or whether it would still be possible to merge
this now? (not having any experience with git's internals yet, I don't
really know whether these are stupid questions or not)

Thanks!
Leo

PS: Cc'ing Joey, as you most likely know best what eventually happened,
if you can remember it?


[1] https://marc.info/?t=13247704151=1=2

[2] https://marc.info/?l=git=132483581218382=2

[3] https://marc.info/?l=git=132486687023893=2

[PATCH] send-email: error out when relogin delay is missing

[Stefan Beller]

When the batch size is neither configured nor given on the command
line, but the relogin delay is given, then the current code ignores
the relogin delay setting.

This is unsafe as there was some intention when setting the batch size.
One workaround would be to just assume a batch size of 1 as a default.
This however may be bad UX, as then the user may wonder why it is sending
slowly without apparent batching.

Error out for now instead of potentially confusing the user.
As 5453b83bdf (send-email: --batch-size to work around some SMTP
server limit, 2017-05-21) lays out, we rather want to not have this
interface anyway and would rather want to react on the server throttling
dynamically.

Helped-by: Eric Sunshine 
Signed-off-by: Stefan Beller 
---
 git-send-email.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 340b5c8482..bc0d3ade16 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -379,6 +379,9 @@ unless ($rc) {
 die __("Cannot run git format-patch from outside a repository\n")
if $format_patch and not $repo;
 
+die __("When a batch size is given, the relogin delay must be set\n")
+   if defined $relogin_delay and not defined $batch_size;
+
 # Now, let's fill any that aren't set in with defaults:
 
 sub read_config {
-- 
2.16.0.rc1.238.g530d649a79-goog

Re: [PATCH v2 01/41] parse-options: support --git-completion-helper

[SZEDER Gábor]

> > OK how about some thing like this fixup patch? __gitcomp_builtin now
> > allows to add extra options as well as remove some.
> >
> > -- 8< --
> >  __gitcomp_builtin ()
> >  {
> > +   local incl="$2"
> > +   local excl="$3"
> > +   options="$(__git ${cmd/_/ } --git-completion-helper) $incl "
> > +   for i in $excl; do
> > +   options="${options/$i /}"
> 
> Is 'options' guaranteed to end with a space?

It is, note the space before the closing double quote in:

  options="$(__git ${cmd/_/ } --git-completion-helper) $incl "

> If not, then this
> expulsion will fail for the very last option. I'd think you can get by
> fine with just "${options/$i}".

I would prefer a space both at the beginning and at the end of the
pattern.  Please excuse the contrived corner case, but it could still
fail if the option to be excluded is a suffix of an other option:

  $ o="$(echo --foo--bar --baz --bar) "
  $ echo "'${o/--bar /}'"   # exclude '--bar'
  '--foo--baz --bar '

Maybe we'll never have --opt--ions with a doubledash in them[1], but
still...

  $ o=" $(echo --foo--bar --baz --bar) "
  $ echo "'${o/ --bar / }'"
  ' --foo--bar --baz '


[1] - Interestingly, grep shows that the German translation does
  contain a '--reset--author'.
 

Re: [PATCH 5/8] rebase: introduce the --recreate-merges option

[Junio C Hamano]

Øyvind Rønningstad  writes:

>> So no, I do not think that --recreate-merges --first-parent is a good
> idea
>> at all. Unless you try to do that non-interactively only, *and
> disallow it
>> in interactive mode*.

Correct.  If the original side branch has commits A, B and C, you
are rebuilding the topic to have only A and C but not B and then
recreate the merge of that rebuilt topic, then you absolutely do not
want "cherry-pick -m1" of the original merge when recreating the
merge, as that would resurrect the effect of having B.  The same
argument applies if you rebuilt the topic with A and C and then a
new commit D.  "cherry-pick -m1" of the original would do a wrong
thing.

When there is no such fixing up, "cherry-pick -m1" is the right
thing to do, though, so it probably makes sense to pick merges that
way when the side topic being merged consists of the same commits as
the original.  I do not think that the code structure in the topic
as posted makes it impossible (or unnecessarily hard) to give an
enhancement like that in the future as a follow-up series.

Re: [PATCH 038/194] pack: allow sha1_loose_object_info to handle arbitrary repositories

[Stefan Beller]

On Wed, Feb 7, 2018 at 2:33 PM, Jonathan Tan  wrote:
> On Mon,  5 Feb 2018 15:54:59 -0800
> Stefan Beller  wrote:
>
>> From: Jonathan Nieder 
>>
>> Signed-off-by: Stefan Beller 
>> Signed-off-by: Jonathan Nieder 
>> ---
>>  sha1_file.c | 11 +--
>>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> Thanks - I can see that this has been a lot of work, and it brings a lot
> of benefit. Among other things, this will probably allow me to remove
> the "fetch_if_missing" global variable that we need to set and reset in
> the partial clone series, replacing it with a setting in either the repo
> or the object store (and when fetching a missing object, first cloning
> the repo/store and then setting that setting, so that objects are not
> recursively fetched).

That sounds intriguing.

> If we're planning to split the series up for merging in batches (which,
> as a reviewer, I'm very much in favor of), I think this is a good
> stopping point for the first batch, so I'll stop my review here for now.

Thanks for identifying a good spot.

I'll look at this part of the series closer for a reroll, and need to make sure
there are no leftovers with the #define trick.

> After these 38 patches, the net benefit is a better position of the
> packed object variables (in struct repository) and permitting the
> reading of loose objects in any repository. (Permitting the reading of
> any object in any repository, I see, will only come later in patch 74.)

which would then be a good portion for the next series.

Thanks for the review!

What's cooking in git.git (Feb 2018, #01; Wed, 7)

[Junio C Hamano]

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

I am migrating my build and integration environment to a different
machine; if you notice anything out of ordinary, please let me know
before I decomission and reimage my usual environment ;-)

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

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

--
[New Topics]

* bc/hash-algo (2018-02-02) 12 commits
 - bulk-checkin: abstract SHA-1 usage
 - csum-file: abstract uses of SHA-1
 - csum-file: rename sha1file to hashfile
 - read-cache: abstract away uses of SHA-1
 - pack-write: switch various SHA-1 values to abstract forms
 - pack-check: convert various uses of SHA-1 to abstract forms
 - fast-import: switch various uses of SHA-1 to the_hash_algo
 - sha1_file: switch uses of SHA-1 to the_hash_algo
 - builtin/unpack-objects: switch uses of SHA-1 to the_hash_algo
 - builtin/index-pack: improve hash function abstraction
 - hash: create union for hash context allocation
 - hash: move SHA-1 macros to hash.h

 More abstraction of hash function from the codepath.

 Will merge to 'next'.


* bp/untracked-cache-noflush (2018-02-05) 1 commit
 - dir.c: don't flag the index as dirty for changes to the untracked cache

 Writing out the index file when the only thing that changed in it
 is the untracked cache information is often wasteful, and this has
 been optimized out.

 Waiting for the discussion to finish.
 cf. 

Re: [PATCH 5/8] rebase: introduce the --recreate-merges option

[Øyvind Rønningstad]

edit: Sending again, hopefully without HTML :). Sorry for spamming.

Hi, I think --recreate-merges is a very exciting feature.

I've also been puzzled by why we can't just pick merge commits directly
including
conflict resolutions, so allow me to join the discussion.

On Wed, Feb 7, 2018 at 6:36 PM, Johannes Schindelin  wrote:
>
> Hi,
>
> [...]
>
> And guess what happens if you drop that `pick` line in your todo list
and
> then the `merge` command simply tries to re-create the original merge
> commit's changes?
>
> Exactly. The merge will become an evil merge, and will introduce that
very
> much not-wanted and therefore-dropped changes.

I think I understand. Evil merges happen when we change the branch
that is not the mainline..? Is there any reason why the following
wouldn't work?

Imagine rebase is about to pick a merge commit, and we have edited at
least one
commit in each branch to be merged.

1. apply patch mainline_orig..merge_orig
2. apply patch branch1_orig..branch1
...
N. apply patch branchN_orig..branchN
N+1. Commit merge

I do see complications, like the fact that steps 2-N can be done in any
order, with
possibly quite different results. Moving commits from one branch to
another might
not work very well. And what to do when you remove branches or create
new ones?

These problems might be prohibitive, but picking merge commits seems
like
something that should be possible to do.

>
> [...]
>
> So --preserve-merges --first-parent is probably what you were looking
for.

I want this as well :). I don't quite see the risk if it's not used
with --interactive.

> [...]
>
> So no, I do not think that --recreate-merges --first-parent is a good
idea
> at all. Unless you try to do that non-interactively only, *and
disallow it
> in interactive mode*. Because the entire point of the interactive
rebase
> is to allow reordering and dropping commits, in --recreate-merges
even
> moving, introducing and dropping merge commits. The --first-parent
option
> flies in the face of this idea.

FWIW I'd be totally fine with disallowing it in --interactive. It would
be incredibly useful 
e.g. with pull --rebase in merge-based workflows.

BTW what is the difference between --recreate-merges and --preserve-
merges when
--interactive is not present? I apologize if you have explained this
somewhere
else in the patch series.

>
> Ciao,
> Johannes

Thanks,
Øyvind

Re: Fetch-hooks

[Ævar Arnfjörð Bjarmason]

On Wed, Feb 07 2018, Leo Gaspard jotted:

> Hello,
>
> tl;dr: Is there currently a way to have fetch hooks, and if not do you
> think it could be a nice feature?
>
> I was in the process of implementing hooks for git that ensure the
> repository is always cleanly signed by someone allowed to by the
> repository itself. I think I've completed the signature-checking part
> [1] and the push hook [2] (even though it isn't really configurable at
> the moment).
>
> However, I was starting to think about handling the fetch step, and
> couldn't find any fetch hook. Is there one?
>
> If not, would you think it is would be a good idea to add one, that
> would eg. be passed the commit-before, commit-after and could block the
> changing of the reference if it failed?
>
> The only other solution I could think of is using a separate script for
> fetching, but that would be fragile, as the user could always not think
> about it well and run a git fetch, breaking the objective that after the
> first clone all commits were correctly signature-checked.
>
> Thanks for reading me!
> Leo
>
> PS1: I am not subscribed to the ML.
>
> PS2: I've tried asking freenode#git, without success so far.
>
>
> [1]
> https://github.com/Ekleog/signed-git/blob/master/git-hooks/check-range-signed.sh
>
> [2] https://github.com/Ekleog/signed-git/blob/master/git-hooks/pre-push

There is no fetch hook, however you may find that the
post-{checkout,merge} hooks are suitable for what you want to do.

Setting those to some custom comand is a common pattern for
e.g. compiling some assets on "git pull", so you could similarly check
the commits from HEAD, of course those are post-* hooks, so they won't
stop the checkout.

Re: [PATCH v3 1/3] read-cache: fix reading the shared index for other repos

[Junio C Hamano]

Junio C Hamano  writes:

>>> This was a bit painful change, given that some changes in flight do
>>> add new callsites to read_index_from() and they got the function
>>> changed under their feet.
>>
>> Sorry about that.  Is there any way to make such a change less painful
>> in the future?
>
> One way is to do for read_index_from() what you did for the existing
> users of read_cache_from().  Introduce a _new_ helper that will not
> be known for any existing topics in flight, and use that to make the
> existing API a thin wrapper around it.

This continues to cause pain simply because read_index_from() is
something new topics want to stay stable X-<.  

I'll be merging this to 'next' hopefully as part of today's
integration run, so will need to endure the pain only until it (and
other topics that conflict with it) all graduate to 'master'.

Next time a patch with an internal API change like this appears,
please remind me to push it back a lot stronger.  I was too lenient
and ended up slowing down the progress of other topics this time.
Thanks.

Re: [PATCH 038/194] pack: allow sha1_loose_object_info to handle arbitrary repositories

[Jonathan Tan]

On Mon,  5 Feb 2018 15:54:59 -0800
Stefan Beller  wrote:

> From: Jonathan Nieder 
> 
> Signed-off-by: Stefan Beller 
> Signed-off-by: Jonathan Nieder 
> ---
>  sha1_file.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)

Thanks - I can see that this has been a lot of work, and it brings a lot
of benefit. Among other things, this will probably allow me to remove
the "fetch_if_missing" global variable that we need to set and reset in
the partial clone series, replacing it with a setting in either the repo
or the object store (and when fetching a missing object, first cloning
the repo/store and then setting that setting, so that objects are not
recursively fetched).

If we're planning to split the series up for merging in batches (which,
as a reviewer, I'm very much in favor of), I think this is a good
stopping point for the first batch, so I'll stop my review here for now.
After these 38 patches, the net benefit is a better position of the
packed object variables (in struct repository) and permitting the
reading of loose objects in any repository. (Permitting the reading of
any object in any repository, I see, will only come later in patch 74.)

Re: "git branch" issue in 2.16.1

[Lars Schneider]

> On 07 Feb 2018, at 21:08, Jeff King  wrote:
> 
> On Wed, Feb 07, 2018 at 06:54:23PM +0100, Lars Schneider wrote:
> 
>>> Maybe the number of branches changed since then?
>>> As the pager only comes to life when the output fills
>>> more than your screen. Quick workarounds:
>>> * buy a bigger screen
>>> * have fewer branches.
>> 
>> Hmmm... there might be more to it. I just noticed the
>> pager behavior on macOS, too. Consider this call:
>> 
>> $ git diff --shortstat
>> 
>> This should generate at most one line of output. On Linux
>> the pager is never used. On macOS the pager is always used.
>> 
>> I tried older versions of Git on macOS and experienced the
>> same behavior.
> 
> Keep in mind that we always run the pager, since we don't know ahead of
> time how much output will be generated. It's just that with certain
> configurations of "less", it may exit if it sees EOF before there's a
> whole screen worth of data.
> 
> This is controlled by the "-F" option. By default, Git will set LESS=FRX
> in the environment if you do not already have a $LESS environment. So
> some other possibilities are:
> 
>  1. You have $LESS in your environment (without "F") on one platform
> but not the other.

I think that's it. On my system LESS is defined to "-R".

This opens the pager:

$ echo "TEST" | less

This does not open the pager:

$ echo "TEST" | less -FRX

That means "F" works on macOS but Git doesn't set it because LESS is
already in my environment.

Question is, why is LESS set that way on my system? I can't find
it in .bashrc .bash_profile .zshrc and friends.

- Lars


> 
>  2. Git was built with a different PAGER_ENV Makefile variable on one
> platform versus the other (that's what controls the baked-in LESS
> defaults).
> 
>  3. "less" somehow behaves differently on macOS. The "F" behavior is
> quite old, but possibly there's some platform-specific bug.
> 
> -Peff

Re: [PATCH 025/194] object-store: allow prepare_alt_odb to handle arbitrary repositories

[Jonathan Tan]

On Mon,  5 Feb 2018 15:54:46 -0800
Stefan Beller  wrote:

> + /*
> +  * Path to the alternate object database, relative to the
> +  * current working directory.
> +  */
>   char path[FLEX_ARRAY];

I would prefer this to be commented:

  Path to the alternative object store. If this is a relative path, it
  is relative to the current working directory.

to show that it is not necessarily relative, but the current version is
fine too.

> + /*
> +  * Paths in alt are relative to the cwd. We ignore environment
> +  * settings like this for all repositories except for
> +  * the_repository, so we don't have to worry about transforming
> +  * the path to be relative to another repository.
> +  */
> + link_alt_odb_entries(r, alt, PATH_SEP, NULL, 0);

I find the comment confusing - it makes more sense for the reason for us
not worrying about transforming the path is that the paths as stored in
struct alternate_object_database are relative to the CWD, not that we
ignore environment variables for certain repositories.

I think it's best to remove this comment, and instead add a comment to
read_info_alternates() before its call to link_alt_odb_entries(),
explaining that paths in the alternates file are relative to
"info/alternates", not to the CWD (since that is the exceptional case).

All the patches prior to this look good. Thanks especially for the
consistent naming convention of the patch titles.

Fetch-hooks

[Leo Gaspard]

Hello,

tl;dr: Is there currently a way to have fetch hooks, and if not do you
think it could be a nice feature?

I was in the process of implementing hooks for git that ensure the
repository is always cleanly signed by someone allowed to by the
repository itself. I think I've completed the signature-checking part
[1] and the push hook [2] (even though it isn't really configurable at
the moment).

However, I was starting to think about handling the fetch step, and
couldn't find any fetch hook. Is there one?

If not, would you think it is would be a good idea to add one, that
would eg. be passed the commit-before, commit-after and could block the
changing of the reference if it failed?

The only other solution I could think of is using a separate script for
fetching, but that would be fragile, as the user could always not think
about it well and run a git fetch, breaking the objective that after the
first clone all commits were correctly signature-checked.

Thanks for reading me!
Leo

PS1: I am not subscribed to the ML.

PS2: I've tried asking freenode#git, without success so far.


[1]
https://github.com/Ekleog/signed-git/blob/master/git-hooks/check-range-signed.sh

[2] https://github.com/Ekleog/signed-git/blob/master/git-hooks/pre-push

Re: categorization, documentation and packaging of "git core" commands

[Todd Zullinger]

Robert P. J. Day wrote:
> not to belabour this (and i'm sure it's *way* too late for that),
> but fedora has the following packaging scheme.  first, there's a bunch
> of stuff in "git-core", which has no dependencies on any other
> git-related packages.

The split in Fedora between git and git-core is done to
minimize the dependencies required for a minimal git
install.  The initial reason was to to allow installing the
git-core package on systems, in containers, etc. without
requiring perl and its various dependencies to be installed.

The name git-core was not chosen to imply any official
status as core versus contrib from upstream.

(Farther back in the past, the main git package (and the
upstream tarball, IIRC) was named git-core due to conflicts
with another tool named git (GNU interactive tools).)

> so with fedora, "git" drags in "git-core" and a small number of
> additional git utilities. all of this leads one to wonder -- is there
> any comprehensible relationship between:
> 
>   1) commands that claim to be in the "git suite"
>   2) commands that come from contrib/
>   3) commands listed at
>  https://www.kernel.org/pub/software/scm/git/docs/
>   4) how different distros package all of the above
> 
> as i think we've noticed, it's not at all clear how git decides what
> is and isn't part of the "official" git suite.

I don't think there's any good reason to use the packaging
of any distribution as the source for what the git project
considers officially part of the suite.  For that, you
should look at the git source and particularly
contrib/README.  The first paragraph says:

Although these pieces are available as part of the official git
source tree, they are in somewhat different status.  The
intention is to keep interesting tools around git here, maybe
even experimental ones, to give users an easier access to them,
and to give tools wider exposure, so that they can be improved
faster.

If anything the Fedora packging does in the splitting or
naming of the git packages is something the git project
feels is incorrect or needlessly confusing, I (with my
Fedora maintainer hat on) would be happy to make any changes
I can to improve things.

-- 
Todd
~~
Some god must protect drunkards and fools, there are so many of them
still around.

Hello

[Merle Butler]

Hello,

I'm Mr. Merle Butler the mega winner of $218M In Mega Millions Jackpot,
I'm donating to 5 random individuals if you get this email then your
email was selected after a spin ball.I have spread most of my wealth
over a number of charities and organizations. I and my wife Patricia
Butler have voluntarily decided to donate the sum of $2 Million USD to
you as one of the selected 5, to verify my winnings please see my you
tube page below.

WATCH ME HERE: https://www.youtube.com/watch?v=VXDhZZFzJ34

THIS IS YOUR DONATION CODE: [ 0043034]

Reply with the DONATION CODE to this email: mmerlepbut...@gmail.com

Hope to make you and your family happy.

Regards

Merle and Patricia Butler

Re: [PATCH] files-backend: unlock packed store only if locked

[Junio C Hamano]

Jonathan Tan  writes:

> On Wed, 7 Feb 2018 09:42:51 -0500
> Jeff King  wrote:
>
>> But this all seemed strangely familiar... I think this is the same bug
>> as:
>> 
>>   https://public-inbox.org/git/20180118143841.1a4c674d@novascotia/
>> 
>> which is queued as mr/packed-ref-store-fix. It's listed as "will merge
>> to next" in the "what's cooking" from Jan 31st.
>
> Ah...thanks, I didn't notice that.
>
>> I actually like this double-label a bit more than what is queued on
>> mr/packed-ref-store-fix, though I am OK with either solution.
>
> Same here.

I do agree that the double-label approach is more future-proof way,
especially if we anticipate that there will be more code after the
"attempt initial ref transaction commit" block before the
packed-ref-store is unlocked.  On the other hand, introduction of
the locked_cleanup label can be done as part of such a change that
adds new code that needs to be skipped, so I am OK with what is
queued there.

Thanks, all.

Re: [PATCHv3] tag: add --edit option

[Junio C Hamano]

Eric Sunshine  writes:

> On Tue, Feb 6, 2018 at 3:36 AM, Nicolas Morey-Chaisemartin
>  wrote:
>> Add a --edit option whichs allows modifying the messages provided by -m or 
>> -F,
>> the same way git commit --edit does.
>>
>> Signed-off-by: Nicolas Morey-Chaisemartin 
>> ---
>> Changes since v2 ( 
>> https://public-inbox.org/git/e99947cf-93ba-9376-f059-7f6a369d3...@suse.com ):
>>  * Add [-e] to git tag summary
>
> Thanks, I think this addresses all my comments from previous rounds.
> Just a couple minor style issues below...
>
>> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
>> @@ -452,6 +452,21 @@ test_expect_success \
>> +> +test_expect_success \
>> +   'creating an annotated tag with -m message --edit should succeed' '
>> +   EDITOR=./fakeeditor git tag -m "A message" --edit 
>> annotated-tag-edit &&
>
> Whitespace between 'fakeeditor' and 'git' is a tab but should be a space.
>
>> +   get_tag_msg annotated-tag-edit >actual &&
>> +   test_cmp expect actual
>> +'
>> @@ -465,6 +480,21 @@ test_expect_success \
>> +test_expect_success \
>> +   'creating an annotated tag with -F messagefile --edit should 
>> succeed' '
>> +   EDITOR=./fakeeditor git tag -F msgfile --edit 
>> file-annotated-tag-edit &&
>
> Ditto.
>
>> +   get_tag_msg file-annotated-tag-edit >actual &&
>> +   test_cmp expect actual
>> +'

Also, GIT_EDITOR takes precedence over EDITOR, so these two new
tests should use it instead, just like other existing tests do.

Will try to amend locally before queuing, so unless I botch that (or
others find other things to tweak), no need to re-send.

Thanks, both.

Re: Bug? Error during commit

[Jeff King]

On Mon, Feb 05, 2018 at 08:59:52PM +0700, Duy Nguyen wrote:

> On Mon, Feb 5, 2018 at 8:48 PM, Andreas Kalz  wrote:
> > Hello,
> >
> > I am using git frequently and usually it is running great.
> >
> > I read to write to this eMail address regarding problems and possible bugs.
> > I am using git version 2.16.1.windows.2 / 64 Bit and during commit the 
> > following error message comes up:
> > e:\Internet>git commit -m 2018-01-27
> > fatal: unable to generate diffstat for 
> > Thunderbird/andreas-kalz.de/Mail/pop.gmx.net/Inbox
> > [master f74cf30] 2018-01-27
> >
> > I also tried this before with an older git version with same problem.
> >
> > Can you help me with this problem please? Thanks in advance.
> 
> I think if you add -q to that "git commit" command, diffstat is not
> generated and you can get past that. If that particular commit can be
> published in public, it'll help us find out why diffstat could not be
> generated.

I think that's the first time I've seen that particular error. :)

I think the only reason that xdiff would report failure is if malloc()
failed, or if one of the files exceeds MAX_XDIFF_SIZE, which is ~1GB.
I think we'd usually avoid doing a text diff on anything over
core.bigFileThreshold, though.

But it doesn't seem to work:

  $ yes | head -c $((1024*1024*1024 - 10*1024*1024)) >file
  $ git add file
  $ git commit -m one
  $ yes | head -c $((1024*1024*1024)) >file
  $ git commit -am two
  fatal: unable to generate diffstat for file

What's weird is that if I run "git show --stat" on the same commit, it
works! So there's something about how commit invokes the diff that
doesn't let the big-file check kick in.

It looks like the logic in diff_filespec_is_binary() will only check
big_file_threshold if we haven't already loaded the contents into RAM.
So "commit" does that, but "diff" is more careful about not loading the
file contents.

I think we probably ought to consider anything over big_file_threshold
to be binary, no matter what. Possibly even if the user gave us a
.gitattribute that says "no really, this is text". Because that 1GB
limit is a hard limit that the code can't cope with; our options are
either to generate a binary diff or to die.

-Peff

Re: categorization, documentation and packaging of "git core" commands

[Robert P. J. Day]

On Wed, 7 Feb 2018, Todd Zullinger wrote:

> Robert P. J. Day wrote:
> > first, here are the executables under /usr/libexec/git-core/ that
> > are unreferenced by that web page, but that should be fine as
> > almost all of them would be considered underlying helpers or
> > utilities (except for things like git-subtree, but we're still
> > unclear on its status, right?):
>
> I don't think there's anything unclear about git subtree's status.
> It's in contrib/ within the source, so it's not part of the core git
> suite.  Some distributions (Fedora being one of them) ship a
> git-subtree package to provide it for users who want it.
>
> > on the other hand (and this is not so much a git issue as a fedora
> > packaging issue), there are a number of command links at that web
> > page that are supplied by distinct RPM packages rather than by the
> > basic fedora git package, so one would need to install the
> > following packages to get some of those commands on fedora:
> >
> >   * gitk
> >   * git-cvs
> >   * git-svn
> >   * git-p4
> >   * git-email (provides git-send-email)
>
> These packages are in separate sub-packages in Fedora (and some
> other distributions) because they are no required by all users and
> they pull in dependencies which are not wanted on minimal installs.
> In Fedora, you can install git-all to get all the available git
> sub-packages.

  not to belabour this (and i'm sure it's *way* too late for that),
but fedora has the following packaging scheme.  first, there's a bunch
of stuff in "git-core", which has no dependencies on any other
git-related packages.

  then there's "git", which has the following property:

  $ rpm -qR git
  /bin/sh
  /usr/bin/perl
  emacs-filesystem >= 25.3
  git-core = 2.14.3-2.fc27
  git-core-doc = 2.14.3-2.fc27
  ... snip ...

  $ rpm -ql git
  ... snip ...
  /usr/libexec/git-core/git-add--interactive
  /usr/libexec/git-core/git-am
  /usr/libexec/git-core/git-credential-libsecret
  /usr/libexec/git-core/git-credential-netrc
  /usr/libexec/git-core/git-difftool
  /usr/libexec/git-core/git-difftool--helper
  /usr/libexec/git-core/git-instaweb
  /usr/libexec/git-core/git-request-pull
  /usr/libexec/git-core/git-submodule
  /usr/libexec/git-core/git-submodule--helper
  ... snip ...
  /usr/share/man/man1/git-am.1.gz
  /usr/share/man/man1/git-difftool.1.gz
  /usr/share/man/man1/git-instaweb.1.gz
  /usr/share/man/man1/git-request-pull.1.gz
  /usr/share/man/man1/git-submodule.1.gz
  /usr/share/man/man1/gitweb.1.gz
  /usr/share/man/man5/gitweb.conf.5.gz
  $

so with fedora, "git" drags in "git-core" and a small number of
additional git utilities. all of this leads one to wonder -- is there
any comprehensible relationship between:

  1) commands that claim to be in the "git suite"
  2) commands that come from contrib/
  3) commands listed at
 https://www.kernel.org/pub/software/scm/git/docs/
  4) how different distros package all of the above

as i think we've noticed, it's not at all clear how git decides what
is and isn't part of the "official" git suite.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: feature-request: git "cp" like there is git mv.

[Stefan Beller]

On Wed, Feb 7, 2018 at 11:49 AM, Junio C Hamano  wrote:
> Stefan Moch  writes:
>
>> * Jonathan Nieder  [2017-12-15T17:31:30-0800]:
>>> This sounds like a reasonable thing to add.  See builtin/mv.c for how
>>> "git mv" works if you're looking for inspiration.
>>>
>>> cmd_mv in that file looks rather long, so I'd also be happy if someone
>>> interested refactors to break it into multiple self-contained pieces
>>> for easier reading (git mostly follows
>>> https://www.kernel.org/doc/html/latest/process/coding-style.html#functions).
>>
>> I looked at builtin/mv.c and have a rough idea how to split it
>> up to support both mv and cp commands.
>>
>> But first I noticed and removed a redundant check in cmd_mv,
>> also added a test case to check if mv --dry-run does not move
>> the file.
>
> I guess these two patches went unnoticed when posted at the end of
> last year.  Reading them again, I think they are good changes.
>
> As a no-op clean-up of a127331c ("mv: allow moving nested
> submodules", 2016-04-19), the attached would also make sense, I
> would think.
>
> Thanks.
>
>  builtin/mv.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/mv.c b/builtin/mv.c
> index 9662804d23..9cb07990fd 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -266,10 +266,11 @@ int cmd_mv(int argc, const char **argv, const char 
> *prefix)
> const char *src = source[i], *dst = destination[i];
> enum update_mode mode = modes[i];
> int pos;
> -   if (show_only || verbose)
> -   printf(_("Renaming %s to %s\n"), src, dst);
> -   if (show_only)
> +   if (show_only) {
> +   if (verbose)
> +   printf(_("Renaming %s to %s\n"), src, dst);
> continue;
> +   }

This is actually changing behavior to

if (show_only && verbose)
print(...)

if show_only
continue

The second part is already there as is, only the printing behavior
actually changes.

So I might be missing the obvious here for the claim of no-op?

Looking up further we have (line 177):

if (show_only)
printf(_("Checking rename of '%s' to '%s'\n"), src, dst);

which prints regardless of verbosity.

Re: "git branch" issue in 2.16.1

[Junio C Hamano]

Jeff King  writes:

> Keep in mind that we always run the pager, since we don't know ahead of
> time how much output will be generated. It's just that with certain
> configurations of "less", it may exit if it sees EOF before there's a
> whole screen worth of data.
>
> This is controlled by the "-F" option. By default, Git will set LESS=FRX
> in the environment if you do not already have a $LESS environment. So
> some other possibilities are:
>
>   1. You have $LESS in your environment (without "F") on one platform
>  but not the other.
>
>   2. Git was built with a different PAGER_ENV Makefile variable on one
>  platform versus the other (that's what controls the baked-in LESS
>  defaults).
>
>   3. "less" somehow behaves differently on macOS. The "F" behavior is
>  quite old, but possibly there's some platform-specific bug.

4. Between your two runs, you do not have that many branches to fill
   the screen vertically in either case, but only in one case you
   have a branch with very long name (or perhaps "branch -l -v" made
   a line longer), and you have S and F in LESS environment.  The
   case that shows branches with short names will cause pager to
   exit at the end, while the other one, because it wants to allow
   you to scroll sideways, will not.

Re: categorization, documentation and packaging of "git core" commands

[Jeff King]

On Wed, Feb 07, 2018 at 03:03:31PM -0500, Robert P. J. Day wrote:

>   huh ... well, that raises the question, if tla has been unbuildable
> for that long (possibly for other distros), what is the value in
> continuing to support git-archimport?
> 
>   https://www.kernel.org/pub/software/scm/git/docs/git-archimport.html

I don't think that we do really support archimport. It's just that we
tend to leave things sitting around if they're not actively causing us
work, in the off chance that somebody might find them useful.

I'd be OK with declaring archimport dead and unmaintained, and dropping
it totally (I almost did so a few months ago when it _did_ cause me
extra work, because it contained a bunch of shell injections that I
turned up while auditing the whole code base).

-Peff

Re: "git branch" issue in 2.16.1

[Jeff King]

On Wed, Feb 07, 2018 at 06:54:23PM +0100, Lars Schneider wrote:

> > Maybe the number of branches changed since then?
> > As the pager only comes to life when the output fills
> > more than your screen. Quick workarounds:
> > * buy a bigger screen
> > * have fewer branches.
> 
> Hmmm... there might be more to it. I just noticed the
> pager behavior on macOS, too. Consider this call:
> 
> $ git diff --shortstat
> 
> This should generate at most one line of output. On Linux
> the pager is never used. On macOS the pager is always used.
> 
> I tried older versions of Git on macOS and experienced the
> same behavior.

Keep in mind that we always run the pager, since we don't know ahead of
time how much output will be generated. It's just that with certain
configurations of "less", it may exit if it sees EOF before there's a
whole screen worth of data.

This is controlled by the "-F" option. By default, Git will set LESS=FRX
in the environment if you do not already have a $LESS environment. So
some other possibilities are:

  1. You have $LESS in your environment (without "F") on one platform
 but not the other.

  2. Git was built with a different PAGER_ENV Makefile variable on one
 platform versus the other (that's what controls the baked-in LESS
 defaults).

  3. "less" somehow behaves differently on macOS. The "F" behavior is
 quite old, but possibly there's some platform-specific bug.

-Peff

Re: categorization, documentation and packaging of "git core" commands

[Robert P. J. Day]

On Wed, 7 Feb 2018, Todd Zullinger wrote:

> Robert P. J. Day wrote:

... snip ...

> > finally, from fedora, i am utterly unable to find a package that
> > provides git-archimport. pretty sure fedora used to have a
> > "git-arch" package but it's not there now.
>
> It hasn't been in Fedora since 2011.  The tla command which is
> required for git-archimport was retired, thus we removed the
> git-arch package.  The rpm changelog shows this:
>
> * Tue Jul 26 2011 Todd Zullinger  - 1.7.6-4
> - Drop git-arch on fedora >= 16, the tla package has been retired
>
> As does the git history for the package:
>
> https://src.fedoraproject.org/rpms/git/c/3f0dc974fa
>
> The tla package was retired because it failed to build for
> several releases:
>
> https://src.fedoraproject.org/rpms/tla/blob/master/f/dead.package

  huh ... well, that raises the question, if tla has been unbuildable
for that long (possibly for other distros), what is the value in
continuing to support git-archimport?

  https://www.kernel.org/pub/software/scm/git/docs/git-archimport.html

i don't really care one way or the other, but perhaps git-archimport
should be broken out as a "non-core" component of git. related post
coming shortly ...

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

[ANNOUNCE] Git for Windows 2.16.1(4)

[Johannes Schindelin]

Dear Git users,

It is my pleasure to announce that Git for Windows 2.16.1(4) is available from:

https://git-for-windows.github.io/

Changes since Git for Windows v2.16.1(3) (February 6th 2018)

Bug Fixes

  * When called from TortoiseGit, git.exe can now spawn processes again
.

Filename | SHA-256
 | ---
Git-2.16.1.4-64-bit.exe | 
5664d42d6ea636f760e6f926bf2239c30782742af6b10d5fb7b6541ba4356c8c
Git-2.16.1.4-32-bit.exe | 
742234e477afdfa07c446758adf7234ab2cd7231ba3bc17642073dd1267cc55a
PortableGit-2.16.1.4-64-bit.7z.exe | 
a2191f676d77f8b8ba88501b8d373dc5418845c52dca86313ec449881af14cbd
PortableGit-2.16.1.4-32-bit.7z.exe | 
5d3e89163cb8b88484d906f8ba53604279e3d7b8c170e39d89116aa5c7274f26
MinGit-2.16.1.4-64-bit.zip | 
4185d4510a82ca1a4f27297b800e9851832a183adb5a52644ae129b395c60fa6
MinGit-2.16.1.4-32-bit.zip | 
d8bd7f8d8145a0ba961a91884acb6fdeadd580b28b77218a833c1ade498b6aff
MinGit-2.16.1.4-busybox-64-bit.zip | 
28fbbc5c9d8ae587b332484dca8a8bee275560e40f5ebf41e9767c0cdadd40db
MinGit-2.16.1.4-busybox-32-bit.zip | 
170b4dffd84f781c03d98d16e89b772785a3081c4b85c91d46bfb53f5659c84f
Git-2.16.1.4-64-bit.tar.bz2 | 
edf22fd92f414a930a8dab4916930fe0b74bd0cf6ae664aa9600360a2ed52bd1
Git-2.16.1.4-32-bit.tar.bz2 | 
b5c0991479a29369841da78e713a850244cff6a600acc16803cf35c5dfe5fcdd

Ciao,
Johannes

Re: "git branch" issue in 2.16.1

[Lars Schneider]

> On 07 Feb 2018, at 19:09, Jason Racey  wrote:
> 
> Hi Lars,
> 
> Here’s what I’m certain of:
> 
> 1. Just set up a new MacBook Pro at work. Git version 2.16.1 installed via 
> Homebrew. “git branch” command always displays the list of branches in the 
> less pager, regardless of number of branches or screen size. I’ve never seen 
> this happen before.
> 2. Checked the “git branch” behavior on my personal MacBook Pro. Git version 
> there is 2.16.0. Does not have this always paging behavior.
> 3. Ran "brew upgrade git" on personal MacBook Pro. Git upgraded to 2.16.1.
> 4. Checked the “git branch” behavior on my personal MacBook Pro after version 
> upgrade. Pager now always used regardless of number of branches or screen 
> size. Oh no! Seems like there might be a bug in the new version, better email 
> the git bug list in just to be safe.
> 5. Meanwhile research problem on Stack Overflow. Mitigated (though not really 
> fixed) issue on both machines with this command: git config --global 
> core.pager ‘’
> 

That's great! Thank you. Can you share your exact OS version running
on your work and personal machine? Plus, what shell do you use and
what terminal application?

Thanks,
Lars


PS: Please don't top post on the git mailing list :-)
https://en.wikipedia.org/wiki/Posting_style


> Thanks!
> 
> - Jason
> 
> 
>> On Feb 7, 2018, at 9:54 AM, Lars Schneider  wrote:
>> 
>> 
>>> On 06 Feb 2018, at 21:05, Stefan Beller  wrote:
>>> 
>>> On Tue, Feb 6, 2018 at 11:57 AM, Todd Zullinger  wrote:
 Hi Jason,
 
 Jason Racey wrote:
> After upgrading git from 2.16.0 to 2.16.1 (via Homebrew -
> I’m on macOS) I noticed that the “git branch” command
> appears to display the branch listing in something similar
> to a vi editor - though not quite the same. I don’t know
> the technical term for this state. You can’t actually edit
> the output of the command, but you’re in a state where you
> have to type “q” to exit and then the list disappears.
> It’s very inconvenient and it doesn’t seem like it was by
> design. I’m using zsh in iTerm2 if that helps. Thanks.
 
 In 2.16.0 `git branch --list` is sent to a pager by default.
 (Without arguments, --list is the default, so this applies
 to `git branch`).
 
 You can set pager.branch to false to disable this in the
 config, or use git --no-pager branch to do so for a single
 invocation.
 
 I can't say why you're seeing this with 2.16.1 and not
 2.16.0, but I'm not familiar with homebrew, so perhaps
 something didn't work as intended in 2.16.0.
 
>>> 
>>> Maybe the number of branches changed since then?
>>> As the pager only comes to life when the output fills
>>> more than your screen. Quick workarounds:
>>> * buy a bigger screen
>>> * have fewer branches.
>> 
>> Hmmm... there might be more to it. I just noticed the
>> pager behavior on macOS, too. Consider this call:
>> 
>> $ git diff --shortstat
>> 
>> This should generate at most one line of output. On Linux
>> the pager is never used. On macOS the pager is always used.
>> 
>> I tried older versions of Git on macOS and experienced the
>> same behavior.
>> 
>> @Jason: That might be a bug on macOS. However, I am surprised
>> you only noticed it after upgrading from 2.16.0 to 2.16.1.
>> Do you recall anything else you've changed?
>> 
>> - Lars
>> 
> 

Re: [PATCH] send-email: have default batch size when relogin delay is given

[Eric Sunshine]

On Wed, Feb 7, 2018 at 2:45 PM, Stefan Beller  wrote:
> When the batch size is neither configured nor given on the command
> line, but the relogin delay is given, then the user is not using the
> the feature as intended. But as the user gave a relogin delay, there is
> clearly the intention to delay sending out emails. Assume a batch size
> of 1 instead of silently ignoring the given relogin delay.
>
> Signed-off-by: Stefan Beller 
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> @@ -379,6 +379,12 @@ unless ($rc) {
> +if (defined $relogin_delay) {
> +   if (not defined $batch_size) {
> +   $batch_size = 1;
> +   }
> +}

Maybe also print a message that this batch size has been used as
default lest the user wonder why it's sending "slowly" without
apparently batching anything.

Alternately, complain and die if both options are not specified.

Re: feature-request: git "cp" like there is git mv.

[Junio C Hamano]

Stefan Moch  writes:

> * Jonathan Nieder  [2017-12-15T17:31:30-0800]:
>> This sounds like a reasonable thing to add.  See builtin/mv.c for how
>> "git mv" works if you're looking for inspiration.
>> 
>> cmd_mv in that file looks rather long, so I'd also be happy if someone
>> interested refactors to break it into multiple self-contained pieces
>> for easier reading (git mostly follows
>> https://www.kernel.org/doc/html/latest/process/coding-style.html#functions).
>
> I looked at builtin/mv.c and have a rough idea how to split it
> up to support both mv and cp commands.
>
> But first I noticed and removed a redundant check in cmd_mv,
> also added a test case to check if mv --dry-run does not move
> the file.

I guess these two patches went unnoticed when posted at the end of
last year.  Reading them again, I think they are good changes.

As a no-op clean-up of a127331c ("mv: allow moving nested
submodules", 2016-04-19), the attached would also make sense, I
would think.

Thanks.

 builtin/mv.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 9662804d23..9cb07990fd 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -266,10 +266,11 @@ int cmd_mv(int argc, const char **argv, const char 
*prefix)
const char *src = source[i], *dst = destination[i];
enum update_mode mode = modes[i];
int pos;
-   if (show_only || verbose)
-   printf(_("Renaming %s to %s\n"), src, dst);
-   if (show_only)
+   if (show_only) {
+   if (verbose)
+   printf(_("Renaming %s to %s\n"), src, dst);
continue;
+   }
if (mode != INDEX && rename(src, dst) < 0) {
if (ignore_errors)
continue;

[PATCH] send-email: have default batch size when relogin delay is given

[Stefan Beller]

When the batch size is neither configured nor given on the command
line, but the relogin delay is given, then the user is not using the
the feature as intended. But as the user gave a relogin delay, there is
clearly the intention to delay sending out emails. Assume a batch size
of 1 instead of silently ignoring the given relogin delay.

Signed-off-by: Stefan Beller 
---
 git-send-email.perl | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 340b5c8482..5672e05b98 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -379,6 +379,12 @@ unless ($rc) {
 die __("Cannot run git format-patch from outside a repository\n")
if $format_patch and not $repo;
 
+if (defined $relogin_delay) {
+   if (not defined $batch_size) {
+   $batch_size = 1;
+   }
+}
+
 # Now, let's fill any that aren't set in with defaults:
 
 sub read_config {
-- 
2.15.1.433.g936d1b9894.dirty

Re: [PATCH v2] rebase: add --allow-empty-message option

[Junio C Hamano]

Johannes Schindelin  writes:

> Very nice. I looked over the patch (sorry, I have too little time to test
> this thoroughly, but then, it is the custom on this here mailing list to
> just review the patch as per the mail) and it looks very good to me.
>
> Junio, if you like, please add a "Reviewed-by:" line for me.

Will do.  You do not have to apologize for not "testing" it; nobody
expects _you_ to test every change you come across on the list, and
other people (including the CI) are testing without advertising ;-).

Re: [PATCH v2 00/41] Automate updating git-completion.bash a bit

[Junio C Hamano]

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

> I posted a proof of concept a while back [1]. This is the full version.
>
> This series lets "git" binary help git-completion.bash to complete
> -- so that when a new option is added, we don't have to update
> git-completion.bash manually too (people often forget it). As a side
> effect, about 180 more options are now completable.
>
> parse-options is updated to allow developers to flag certain options
> not to be completable if they want finer control over it.  But by
> default, new non-hidden options are completable. Negative forms must
> be handled manually. That's for the next step.

Everybody seems to be in favour of the approach taken by this
series.  Is it in a good enough shape that we can merge it to 'next'
and then go incremental from now on?  Or do we want to keep it in
'pu' to give easier access to volunteer guinea pigs and wait until
the way to handle '--no-foo' options are figured out?

Re: [PATCH] t0050: remove the unused $test_case variable

[Ævar Arnfjörð Bjarmason]

On Wed, Feb 07 2018, Johannes Sixt jotted:

> Am 07.02.2018 um 09:07 schrieb Ævar Arnfjörð Bjarmason:
>>
>> On Wed, Feb 07 2018, Johannes Sixt jotted:
>>
>>> Am 07.02.2018 um 00:13 schrieb Ævar Arnfjörð Bjarmason:
 The $test_case variable hasn't been used since
 decd3c0c28 ("t0050-*.sh: mark the rename (case change) test as
 passing", 2014-11-28) when its last user went away.

 Let's remove the "say" as well, since it's obvious from subsequent
 output that we're testing on a case sensitive filesystem.
>>>
>>> Am I misunderstanding the message? I think it reports properties of
>>> the test environment. And the tests do run on case-insensitive
>>> filesystems. IMO, the message should be kept.
>>
>> It's obvious from subsequent output whether the FS is case sensitive or
>> not, so I thought it was redundant to keep this report at the top since
>> we didn't have the variable setting anymore.
>
> There are test cases that do different things depending on whether the
> CASE_INSENSITIVE_FS prerequisite is set. I think it was the intent to
> report whether it is set and not whether one or the other value of the
> (now unused) variable is used somewhere.
>
> BTW, the message texts do not show which variant is taken (these are
> without your patch):
>
> On Windows:
>
> t>sh t0050-filesystem.sh
> will test on a case insensitive filesystem
> will test on a filesystem lacking symbolic links
> ok 1 - detection of case insensitive filesystem during repo init
> ok 2 - detection of filesystem w/o symlink support during repo init
> ok 3 - setup case tests
> ok 4 - rename (case change)
> ok 5 - merge (case change)
> not ok 6 - add (with different case) # TODO known breakage
> ok 7 - setup unicode normalization tests
> ok 8 - rename (silent unicode normalization)
> ok 9 - merge (silent unicode normalization)
> # still have 1 known breakage(s)
> # passed all remaining 8 test(s)
> 1..9
>
> On Linux:
>
> t@master:1002> ./t0050-filesystem.sh
> ok 1 - detection of case insensitive filesystem during repo init
> ok 2 - detection of filesystem w/o symlink support during repo init
> ok 3 - setup case tests
> ok 4 - rename (case change)
> ok 5 - merge (case change)
> ok 6 # skip add (with different case) (missing CASE_INSENSITIVE_FS)
> ok 7 - setup unicode normalization tests
> ok 8 - rename (silent unicode normalization)
> ok 9 - merge (silent unicode normalization)
> # passed all 9 test(s)
> 1..9
>
> I'd even argue that there should be messages on Linux, too.

Thanks. Let's just drop this patch. I thought it would still print out
something similar to that "missing CASE_INSENSITIVE_FS" at a quick
glance last night, but was obviously wrong.

Re: [PATCH v3 22/35] upload-pack: support shallow requests

[Stefan Beller]

On Tue, Feb 6, 2018 at 5:12 PM, Brandon Williams  wrote:
> Add the 'shallow' feature to the protocol version 2 command 'fetch'
> which indicates that the server supports shallow clients and deepen
> requets.
>
> Signed-off-by: Brandon Williams 
> ---
>  Documentation/technical/protocol-v2.txt |  67 +++-
>  serve.c |   2 +-
>  t/t5701-git-serve.sh|   2 +-
>  upload-pack.c   | 138 
> +++-
>  upload-pack.h   |   3 +
>  5 files changed, 173 insertions(+), 39 deletions(-)
>
> diff --git a/Documentation/technical/protocol-v2.txt 
> b/Documentation/technical/protocol-v2.txt
> index 4d5096dae..fedeb6b77 100644
> --- a/Documentation/technical/protocol-v2.txt
> +++ b/Documentation/technical/protocol-v2.txt
> @@ -201,12 +201,42 @@ packet-lines:
> to its base by position in pack rather than by an oid.  That is,
> they can read OBJ_OFS_DELTA (ake type 6) in a packfile.
>
> +shallow 
> +   A client must notify the server of all objects for which it only

s/all objects/all commits/ for preciseness

> +   has shallow copies of (meaning that it doesn't have the parents
> +   of a commit) by supplying a 'shallow ' line for each such
> +   object so that the serve is aware of the limitations of the
> +   client's history.
> +
> +deepen 
> +   Request that the fetch/clone should be shallow having a commit depth 
> of
> +relative to the remote side.

What does depth mean? number of commits, or number of edges?
Are there any special numbers (-1, 0, 1, max int) ?

>From reading ahead: "Cannot be used with deepen-since, but
can be combined with deepen-relative" ?


> +
> +deepen-relative
> +   Requests that the semantics of the "deepen" command be changed
> +   to indicate that the depth requested is relative to the clients
> +   current shallow boundary, instead of relative to the remote
> +   refs.
> +
> +deepen-since 
> +   Requests that the shallow clone/fetch should be cut at a
> +   specific time, instead of depth.  Internally it's equivalent of
> +   doing "rev-list --max-age=". Cannot be used with
> +   "deepen".
> +
> +deepen-not 
> +   Requests that the shallow clone/fetch should be cut at a
> +   specific revision specified by '', instead of a depth.
> +   Internally it's equivalent of doing "rev-list --not ".
> +   Cannot be used with "deepen", but can be used with
> +   "deepen-since".

What happens if those are given in combination?

Re: categorization, documentation and packaging of "git core" commands

[Todd Zullinger]

Robert P. J. Day wrote:
> On Wed, 7 Feb 2018, Todd Zullinger wrote:
> 
>> Robert P. J. Day wrote:
>>> first, here are the executables under /usr/libexec/git-core/ that
>>> are unreferenced by that web page, but that should be fine as
>>> almost all of them would be considered underlying helpers or
>>> utilities (except for things like git-subtree, but we're still
>>> unclear on its status, right?):
>>
>> I don't think there's anything unclear about git subtree's status.
>> It's in contrib/ within the source, so it's not part of the core git
>> suite.  Some distributions (Fedora being one of them) ship a
>> git-subtree package to provide it for users who want it.
> 
>   not true, "git-subtree" is part of fedora's lowest-level
> "git-core" package.

Eek, my apologies for providing bad information.  I really
should know the Fedora git packaging better than that. :/

Amusingly, I did suggest packaging it as a subpackage
specifically to avoid any confusion that it was a core
command in the Fedora bug which requested it be included in
the git packaging:

https://bugzilla.redhat.com/show_bug.cgi?id=864651

I'll see about changing that going forward in the Fedora
packaging.  I think it deserves to be in a subpackage.

-- 
Todd
~~
Doing a job RIGHT the first time gets the job done.
Doing the job WRONG fourteen times gives you job security.

Re: [PATCH] t0050: remove the unused $test_case variable

[Johannes Sixt]

Am 07.02.2018 um 09:07 schrieb Ævar Arnfjörð Bjarmason:


On Wed, Feb 07 2018, Johannes Sixt jotted:


Am 07.02.2018 um 00:13 schrieb Ævar Arnfjörð Bjarmason:

The $test_case variable hasn't been used since
decd3c0c28 ("t0050-*.sh: mark the rename (case change) test as
passing", 2014-11-28) when its last user went away.

Let's remove the "say" as well, since it's obvious from subsequent
output that we're testing on a case sensitive filesystem.


Am I misunderstanding the message? I think it reports properties of
the test environment. And the tests do run on case-insensitive
filesystems. IMO, the message should be kept.


It's obvious from subsequent output whether the FS is case sensitive or
not, so I thought it was redundant to keep this report at the top since
we didn't have the variable setting anymore.


There are test cases that do different things depending on whether the 
CASE_INSENSITIVE_FS prerequisite is set. I think it was the intent to 
report whether it is set and not whether one or the other value of the 
(now unused) variable is used somewhere.


BTW, the message texts do not show which variant is taken (these are 
without your patch):


On Windows:

t>sh t0050-filesystem.sh
will test on a case insensitive filesystem
will test on a filesystem lacking symbolic links
ok 1 - detection of case insensitive filesystem during repo init
ok 2 - detection of filesystem w/o symlink support during repo init
ok 3 - setup case tests
ok 4 - rename (case change)
ok 5 - merge (case change)
not ok 6 - add (with different case) # TODO known breakage
ok 7 - setup unicode normalization tests
ok 8 - rename (silent unicode normalization)
ok 9 - merge (silent unicode normalization)
# still have 1 known breakage(s)
# passed all remaining 8 test(s)
1..9

On Linux:

t@master:1002> ./t0050-filesystem.sh
ok 1 - detection of case insensitive filesystem during repo init
ok 2 - detection of filesystem w/o symlink support during repo init
ok 3 - setup case tests
ok 4 - rename (case change)
ok 5 - merge (case change)
ok 6 # skip add (with different case) (missing CASE_INSENSITIVE_FS)
ok 7 - setup unicode normalization tests
ok 8 - rename (silent unicode normalization)
ok 9 - merge (silent unicode normalization)
# passed all 9 test(s)
1..9

I'd even argue that there should be messages on Linux, too.

-- Hannes

Re: [PATCH] files-backend: unlock packed store only if locked

[Jonathan Tan]

On Wed, 7 Feb 2018 09:42:51 -0500
Jeff King  wrote:

> But this all seemed strangely familiar... I think this is the same bug
> as:
> 
>   https://public-inbox.org/git/20180118143841.1a4c674d@novascotia/
> 
> which is queued as mr/packed-ref-store-fix. It's listed as "will merge
> to next" in the "what's cooking" from Jan 31st.

Ah...thanks, I didn't notice that.

> I actually like this double-label a bit more than what is queued on
> mr/packed-ref-store-fix, though I am OK with either solution.

Same here.

Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)

[Junio C Hamano]

Duy Nguyen  writes:

> ...
> Then we still need to decide the new keyword for this feature, I feel
> compact is a bit too vague (I read --stat=compact as "it's compact
> stat", not "stat with compact summary"), so perhaps
> --stat=compact-summary, or just --stat=summary?

Yup, this is about giving summary in a compact way, not about giving
a compact stat information.  I agree with all the above reasoning,
and that is why I said that your "compact-summary" was a good way to
refer to the feature.

Re: BUG: fetch in certain repo always gives "did not send all necessary objects"

[Jeff King]

On Wed, Feb 07, 2018 at 09:25:42AM -0800, Elijah Newren wrote:

> > So other_head_refs knows that it's looking at the worktrees. And it
> > passes the alternate ref-store to refs_head_ref(), with "add_one_ref" as
> > the callback. But the knowledge that we're not talking about the real
> > "HEAD" is lost as we cross that callback boundary. We'd need to either
> > add another parameter to the callback, or have some way of talking about
> > "HEAD in this worktree" as a refname (which AFAIK we don't have).
> 
> Can we use "worktrees/${WORKTREE}/HEAD"?  It already satisfies all the
> necessary rev-parse rules...

True, but it's mostly an accident that it works. And once we have ref
backends besides the filesystem, it will probably stop working.

I think there was discussion at some point of embedding worktree refs
into the normal ref namespace, but I don't know what came of it (it's
not a feature I've followed very closely).

> (And on a slight tangent...do we want to start disallowing the
> creation of branches/tags whose name starts with "worktrees/",
> "refs/", "hooks/", or other paths that exists under gitdir?  Making a
> branch named "refs/heads/foo" so that it fully-qualifies as
> "refs/heads/refs/heads/foo" is always fun)

We recently taught the porcelain to disallow a branch named "HEAD".
Though I think there are actually two related problems with different
solutions. One is saying something like:

  git checkout -b HEAD

or:

  git checkout -b refs/heads/foo

both of which will not do what you want, and leave you with a
funnily-named branch in the ref namespace.

But that's separate from the fact that:

  git rev-parse info/refs

will look at a file that is not a ref at all. Long-term I think the
solution is storage formats that don't mingle with other files. But we
could probably teach even the files-backend that any ref at the
top-level is supposed to be either in refs/, or to consist only of
"[A-Z_]".

-Peff

Re: BUG: fetch in certain repo always gives "did not send all necessary objects"

[Elijah Newren]

On Wed, Feb 7, 2018 at 3:08 AM, Duy Nguyen  wrote:
> On Wed, Feb 7, 2018 at 7:00 AM, Elijah Newren  wrote:
>> and knew they had been using it, then I might have guessed that "HEAD"
>> meant "not your actual HEAD but the HEAD of the vestige of some other
>> worktree").
>>
>> Does anyone have pointers about what might be doable in terms of
>> providing a more useful error message to allow users to recover?
>
> I noticed this too. I was working on improving this message a bit but
> got side tracked and since I figured this did not happen often
> anymore, this fix got lower priority than others. I'll resume that
> work.

Sweet, that'd be great.  Let me know if I can help.

>> And/or ideas of what steps could cause corruption so I can send out a
>> PSA to help users avoid it?
>
> There is another thing we could do. One bad HEAD should not abort the
> entire operation (at least if it's not the current worktree's HEAD).
> We could still give a warning and move on, or don't warn at all and
> let "git worktree prune" collect it (which I see from your message
> that it also fails to do).
>
> I guess that's two more items on my todo list :)

Cool, if you're taking a look at those, I'll take a look at the other
one mentioned by Peff in this thread -- "make fsck pay attention to
other worktree HEADs"  :-)

> Sorry for all the trouble because of this bug of mine.

Let me apologize if my tone came across too harshly in the report; I
sometimes accidentally do that.  Anyway, bugs happen.  People were
able to get back up and running with "just reclone somewhere else"
pretty quickly, and no one lost any important data here, it was just
jarring or perplexing enough that I felt the need to dig and figure
out an improvement before more reports come in.

Besides, you've made git a lot better.  Thanks for that.

Re: [PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding

[Junio C Hamano]

Torsten Bögershausen  writes:

>> the user explicitly tells us it is in UTF-16, right?  Is there such a
>> thing as UTF-16 binary?
>
> I don't think so, by definiton UTF-16 is ment to be text.
> (this means that git ls-files --eol needs some update, I can have a look)
>
> Do we agree that UTF-16 is text ?
> If yes, could Git assume that the "text" attribute is set when
> working-tree-encoding is set ?

These are two different questions.  It seems that between the two of
us, we established that "UTF-16 binary" is a nonsense, and a path
that is given working-tree-encoding=UTF-16 must be treated as
holding a text file.  But that does not have direct relevance to the
other question you are asking: "is a question 'does this path have
text attribute set?' be answered with 'yes' if the path has wte
attribute set to UTF-16?"  I think the answer to that latter
question ought to be "no".

By the way, a related tangent is if it makes sense to give
working-tree-encoding to anything that is binary, regardless of the
value---I am inclined to say it is not, so the issue here is not "by
definition UTF-16 is text", but "any path that has wte set to some
enconding should be treated the same way as if the path also has
text attribute set as far as convert machinery is concerned.".

Re: BUG: On some Linux systems, "Stage To Commit" hotkey in git-gui stages one file only, even if multiple files are selected

[Eric Sunshine]

On Wed, Feb 7, 2018 at 8:29 AM, Birger Skogeng Pedersen
 wrote:
> Steps to reproduce (on Ubuntu 17.10):
> - Open git-gui in a repository with two or more unstaged, changed files
> - Select two or more files in the "Unstaged Changes"
> - Click CTRL+T to stage the files that you have selected
> (Only a single file will get staged)

This was fixed in Git 2.16.0.

76756d6706 (git-gui: allow Ctrl+T to toggle multiple paths, 2018-01-09)

Re: "git branch" issue in 2.16.1

[Jason Racey]

Hi Lars,

Here’s what I’m certain of:

1. Just set up a new MacBook Pro at work. Git version 2.16.1 installed via 
Homebrew. “git branch” command always displays the list of branches in the less 
pager, regardless of number of branches or screen size. I’ve never seen this 
happen before.
2. Checked the “git branch” behavior on my personal MacBook Pro. Git version 
there is 2.16.0. Does not have this always paging behavior.
3. Ran "brew upgrade git" on personal MacBook Pro. Git upgraded to 2.16.1.
4. Checked the “git branch” behavior on my personal MacBook Pro after version 
upgrade. Pager now always used regardless of number of branches or screen size. 
Oh no! Seems like there might be a bug in the new version, better email the git 
bug list in just to be safe.
5. Meanwhile research problem on Stack Overflow. Mitigated (though not really 
fixed) issue on both machines with this command: git config --global core.pager 
‘’

Thanks!

- Jason


> On Feb 7, 2018, at 9:54 AM, Lars Schneider  wrote:
> 
> 
>> On 06 Feb 2018, at 21:05, Stefan Beller  wrote:
>> 
>> On Tue, Feb 6, 2018 at 11:57 AM, Todd Zullinger  wrote:
>>> Hi Jason,
>>> 
>>> Jason Racey wrote:
 After upgrading git from 2.16.0 to 2.16.1 (via Homebrew -
 I’m on macOS) I noticed that the “git branch” command
 appears to display the branch listing in something similar
 to a vi editor - though not quite the same. I don’t know
 the technical term for this state. You can’t actually edit
 the output of the command, but you’re in a state where you
 have to type “q” to exit and then the list disappears.
 It’s very inconvenient and it doesn’t seem like it was by
 design. I’m using zsh in iTerm2 if that helps. Thanks.
>>> 
>>> In 2.16.0 `git branch --list` is sent to a pager by default.
>>> (Without arguments, --list is the default, so this applies
>>> to `git branch`).
>>> 
>>> You can set pager.branch to false to disable this in the
>>> config, or use git --no-pager branch to do so for a single
>>> invocation.
>>> 
>>> I can't say why you're seeing this with 2.16.1 and not
>>> 2.16.0, but I'm not familiar with homebrew, so perhaps
>>> something didn't work as intended in 2.16.0.
>>> 
>> 
>> Maybe the number of branches changed since then?
>> As the pager only comes to life when the output fills
>> more than your screen. Quick workarounds:
>> * buy a bigger screen
>> * have fewer branches.
> 
> Hmmm... there might be more to it. I just noticed the
> pager behavior on macOS, too. Consider this call:
> 
> $ git diff --shortstat
> 
> This should generate at most one line of output. On Linux
> the pager is never used. On macOS the pager is always used.
> 
> I tried older versions of Git on macOS and experienced the
> same behavior.
> 
> @Jason: That might be a bug on macOS. However, I am surprised
> you only noticed it after upgrading from 2.16.0 to 2.16.1.
> Do you recall anything else you've changed?
> 
> - Lars
> 

Re: categorization, documentation and packaging of "git core" commands

[Robert P. J. Day]

On Wed, 7 Feb 2018, Todd Zullinger wrote:

> Robert P. J. Day wrote:
> > first, here are the executables under /usr/libexec/git-core/ that
> > are unreferenced by that web page, but that should be fine as
> > almost all of them would be considered underlying helpers or
> > utilities (except for things like git-subtree, but we're still
> > unclear on its status, right?):
>
> I don't think there's anything unclear about git subtree's status.
> It's in contrib/ within the source, so it's not part of the core git
> suite.  Some distributions (Fedora being one of them) ship a
> git-subtree package to provide it for users who want it.

  not true, "git-subtree" is part of fedora's lowest-level
"git-core" package.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: [RFC PATCH 000/194] Moving global state into the repository object

[Stefan Beller]

On Wed, Feb 7, 2018 at 3:48 AM, Duy Nguyen  wrote:
> On Tue, Feb 6, 2018 at 6:51 AM, Stefan Beller  wrote:
>> This series moves a lot of global state into the repository struct.
>> It applies on top of 2512f15446149235156528dafbe75930c712b29e (2.16.0)
>> It can be found at https://github.com/stefanbeller/git/tree/object-store
>>
>> Motivation for this series:
>> * Easier to reason about code when all state is stored in one place,
>>   for example for multithreading
>> * Easier to move to processing submodules in-process instead of
>>   calling a processes for the submodule handling.
>>   The last patch demonstrates this.
>
> I have a mixed feeling about this. The end game is to keep
> "the_repository" references as minimum as possible, right? Like we
> only need to mention in in cmd_xxx() and not all over the place. If
> so, yay!!

Yes. And the super-end-game long after this series is to have
cmd_xxx(struct repository *r, argc, argv)
or so.

> Something else.. maybe "struct repository *" should not be a universal
> function argument to avoid global states. Like sha1_file.c is mostly about the
> object store, and I see that you added object store struct (nice!).
> These sha1 related function should take the object_store* (which
> probably is a combination of both packed and loose stores since they
> depend on each other), not repository*. This way if a function needs
> both access to object store and ref store, we can see the two
> dependencies from function arguments.

I tried that in the beginning and it blew up a couple of times when I realized
that I forgot to pass through one of these dependencies.
Maybe we can go to the repository and shrink the scope in a follow up?

> The alternate object store, if modeled right, could share the same
> object store interface. But I don't think we should do anything that
> big right now, just put alternate object store inside object_store.

yup that is the case, see struct raw_object_store at the end of the series
https://github.com/stefanbeller/git/blob/object-store-v2/object-store.h

> Similarly those functions in blob.c, commit.c, tree.c... should take
> object_parser* as argument instead of repository*. Maybe there's a
> better name for this than object_parser. parsed_object_store I guess.
> Or maybe just object_pool.

Note that the initial few patches are misleading in the name,
https://public-inbox.org/git/20180205235735.216710-59-sbel...@google.com/
which splits up the object handling into two layers, the "physical" layer
(where to get the data from, mmaping pack files, alternates, streams of bytes),
which is "struct raw_object_store objects;" in the repository, and the
"object" layer, which is about parsing the objects and making sense of the data
(which tree belongs to a commit, dereferencing tags)

So maybe I'll try to make the first layer into its own series, such
that we'll have a smaller series.

Thanks,
Stefan

> --
> Duy

Re: "git branch" issue in 2.16.1

[Lars Schneider]

> On 06 Feb 2018, at 21:05, Stefan Beller  wrote:
> 
> On Tue, Feb 6, 2018 at 11:57 AM, Todd Zullinger  wrote:
>> Hi Jason,
>> 
>> Jason Racey wrote:
>>> After upgrading git from 2.16.0 to 2.16.1 (via Homebrew -
>>> I’m on macOS) I noticed that the “git branch” command
>>> appears to display the branch listing in something similar
>>> to a vi editor - though not quite the same. I don’t know
>>> the technical term for this state. You can’t actually edit
>>> the output of the command, but you’re in a state where you
>>> have to type “q” to exit and then the list disappears.
>>> It’s very inconvenient and it doesn’t seem like it was by
>>> design. I’m using zsh in iTerm2 if that helps. Thanks.
>> 
>> In 2.16.0 `git branch --list` is sent to a pager by default.
>> (Without arguments, --list is the default, so this applies
>> to `git branch`).
>> 
>> You can set pager.branch to false to disable this in the
>> config, or use git --no-pager branch to do so for a single
>> invocation.
>> 
>> I can't say why you're seeing this with 2.16.1 and not
>> 2.16.0, but I'm not familiar with homebrew, so perhaps
>> something didn't work as intended in 2.16.0.
>> 
> 
> Maybe the number of branches changed since then?
> As the pager only comes to life when the output fills
> more than your screen. Quick workarounds:
> * buy a bigger screen
> * have fewer branches.

Hmmm... there might be more to it. I just noticed the
pager behavior on macOS, too. Consider this call:

$ git diff --shortstat

This should generate at most one line of output. On Linux
the pager is never used. On macOS the pager is always used.

I tried older versions of Git on macOS and experienced the
same behavior.

@Jason: That might be a bug on macOS. However, I am surprised
you only noticed it after upgrading from 2.16.0 to 2.16.1.
Do you recall anything else you've changed?

- Lars

Re: [PATCH 5/8] rebase: introduce the --recreate-merges option

[Johannes Schindelin]

Hi,

On Wed, 7 Feb 2018, Sergey Organov wrote:

> Johannes Schindelin  writes:
> 
> [...]
> 
> > +--recreate-merges::
> > +   Recreate merge commits instead of flattening the history by replaying
> > +   merges. Merge conflict resolutions or manual amendments to merge
> > +   commits are not preserved.
> 
> I wonder why you guys still hold on replaying "merge-the-operation"
> instead of replaying "merge-the-result"?

This misses the point of rebasing: you want to replay the changes.

> The latter, the merge commit itself, no matter how exactly it was
> created in the first place, is the most valuable thing git keeps about
> the merge, and you silently drop it entirely!

You miss another very crucial point. I don't blame you, as you certainly
have not used the Git garden shears for years.

Let me explain the scenario which comes up plenty of times in my work with
Git for Windows. We have a thicket of some 70 branches on top of git.git's
latest release. These branches often include fixup! and squash! commits
and even more complicated constructs that rebase cannot handle at all at
the moment, such as reorder-before! and reorder-after! (for commits that
really need to go into a different branch).

Even if you do not have such a complicated setup, it is quite possible
that you need to include a commit in your development that needs to be
dropped before contributing your work. Think e.g. removing the `-O2` flag
when compiling with GCC because GDB gets utterly confused with executables
compiled with `-O2` while single-stepping. This could be an initial commit
called `TO-DROP` or some such.

And guess what happens if you drop that `pick` line in your todo list and
then the `merge` command simply tries to re-create the original merge
commit's changes?

Exactly. The merge will become an evil merge, and will introduce that very
much not-wanted and therefore-dropped changes.

> OTOH, git keeps almost no information about "merge-the-operation", so
> it's virtually impossible to reliably replay the operation
> automatically, and yet you try to.

That is true. However, the intended use case is not to allow you to
recreate funny merges. Its use case is to allow you to recreate merges.

At a later stage, I might introduce support to detect `-s ours` merges,
because they are easy to detect. But even then, it will be an opt-in.

> IMHO that was severe mistake in the original --preserve-merges, and you
> bring with you to this new --recreate-merges... It's sad.

Please refrain from drawing this discussion into an emotional direction.
That is definitely not helpful.

> Even more sad as solution is already known for years:
> 
> bc00341838a8faddcd101da9e746902994eef38a
> Author: Johannes Sixt 
> Date:   Sun Jun 16 15:50:42 2013 +0200
> 
> rebase -p --first-parent: redo merge by cherry-picking first-parent 
> change
> 
> and it works like a charm.

It might work for you, as you probably used --preserve-merges, and dealt
with the fact that you could neither drop nor reorder commits.

So --preserve-merges --first-parent is probably what you were looking for.

Instead, --recreate-merges is all about allowing the same level of freedom
as with regular interactive rebases, but recreating the original commit
topology (and allowing to change it, too).

Therefore, I think that it would be even harmful to allow
--recreate-merges --first-parent *because it would cause evil merges*!

And I totally could see myself being vexed again about options that worked
perfectly well (just like --preserve-merges) being completely messed up by
allowing it to be combined with options *that they cannot work with* (just
like --preserve-merges --interactive, a *huge* mistake causing so many
annoying "bug" reports: I *never intended it that way because I knew it
would not work as users expect*).

So no, I do not think that --recreate-merges --first-parent is a good idea
at all. Unless you try to do that non-interactively only, *and disallow it
in interactive mode*. Because the entire point of the interactive rebase
is to allow reordering and dropping commits, in --recreate-merges even
moving, introducing and dropping merge commits. The --first-parent option
flies in the face of this idea.

Ciao,
Johannes

Re: categorization, documentation and packaging of "git core" commands

[Todd Zullinger]

Robert P. J. Day wrote:
> first, here are the executables under /usr/libexec/git-core/ that
> are unreferenced by that web page, but that should be fine as almost
> all of them would be considered underlying helpers or utilities
> (except for things like git-subtree, but we're still unclear on its
> status, right?):

I don't think there's anything unclear about git subtree's
status.  It's in contrib/ within the source, so it's not
part of the core git suite.  Some distributions (Fedora
being one of them) ship a git-subtree package to provide it
for users who want it.

> on the other hand (and this is not so much a git issue as a fedora
> packaging issue), there are a number of command links at that web page
> that are supplied by distinct RPM packages rather than by the basic
> fedora git package, so one would need to install the following
> packages to get some of those commands on fedora:
> 
>   * gitk
>   * git-cvs
>   * git-svn
>   * git-p4
>   * git-email (provides git-send-email)

These packages are in separate sub-packages in Fedora (and
some other distributions) because they are no required by
all users and they pull in dependencies which are not wanted
on minimal installs.  In Fedora, you can install git-all to
get all the available git sub-packages.

> finally, from fedora, i am utterly unable to find a package that
> provides git-archimport. pretty sure fedora used to have a "git-arch"
> package but it's not there now.

It hasn't been in Fedora since 2011.  The tla command which
is required for git-archimport  was retired, thus we removed
the git-arch package.  The rpm changelog shows this:

* Tue Jul 26 2011 Todd Zullinger  - 1.7.6-4
- Drop git-arch on fedora >= 16, the tla package has been retired

As does the git history for the package:

https://src.fedoraproject.org/rpms/git/c/3f0dc974fa

The tla package was retired because it failed to build for
several releases:

https://src.fedoraproject.org/rpms/tla/blob/master/f/dead.package

-- 
Todd
~~
Going to trial with a lawyer who considers your whole life-style a
Crime in Progress is not a happy prospect.
-- Hunter S. Thompson

Re: BUG: fetch in certain repo always gives "did not send all necessary objects"

[Elijah Newren]

On Wed, Feb 7, 2018 at 5:21 AM, Jeff King  wrote:
> On Tue, Feb 06, 2018 at 04:00:32PM -0800, Elijah Newren wrote:
>
>> It took me hours to figure it out, after users ran out of ideas and
>> came and asked me for help.  (Maybe if I was familiar with worktree,
>> and knew they had been using it, then I might have guessed that "HEAD"
>> meant "not your actual HEAD but the HEAD of the vestige of some other
>> worktree").
>
> Yeah, this is the obvious thing that seems like it ought to be improved.

> Unfortunately fixing that is a little tricky. In this case the stack
> looks like:
>
>   parse_object_or_die (oid=0x7fffd690, name=0x55792880 "HEAD") at 
> object.c:239
>   add_one_ref (path=0x55792880 "HEAD", oid=0x7fffd690, flag=0, 
> cb_data=0x7fffd8e0) at reachable.c:38
>   refs_head_ref (refs=0x55a65430, fn=0x556b6ef5 , 
> cb_data=0x7fffd8e0) at refs.c:1316
>   other_head_refs (fn=0x556b6ef5 , cb_data=0x7fffd8e0) 
> at worktree.c:404
>
> So other_head_refs knows that it's looking at the worktrees. And it
> passes the alternate ref-store to refs_head_ref(), with "add_one_ref" as
> the callback. But the knowledge that we're not talking about the real
> "HEAD" is lost as we cross that callback boundary. We'd need to either
> add another parameter to the callback, or have some way of talking about
> "HEAD in this worktree" as a refname (which AFAIK we don't have).

Can we use "worktrees/${WORKTREE}/HEAD"?  It already satisfies all the
necessary rev-parse rules...

(And on a slight tangent...do we want to start disallowing the
creation of branches/tags whose name starts with "worktrees/",
"refs/", "hooks/", or other paths that exists under gitdir?  Making a
branch named "refs/heads/foo" so that it fully-qualifies as
"refs/heads/refs/heads/foo" is always fun)

RE: git: CVE-2018-1000021: client prints server sent ANSI escape codes to the terminal, allowing for unverified messages to potentially execute arbitrary commands

[Randall S. Becker]

On February 7, 2018 11:53 AM, Andreas Schwab wrote:
> On Feb 06 2018, "Randall S. Becker"  wrote:
> 
> > What I don't know - and it's not explicitly in the CVE - is just how
> > many other terminal types with similar vulnerabilities are out there,
> > but I'm suspecting it's larger than one would guess - mostly, it seems
> > like this particular sequence is intended to be used for writing
> > status line output (line 25?) instead of sticking it in a prompt. This
> > can be used prettifies a lengthy bash prompt to display the current
> > branch and repository at the bottom of the screen instead of in the
> > inline prompt, but that's the user's choice and not something git has
> > to deal with. There were some green-screen terminals with other weird
> > ESC sequences back in the day that could really get into trouble with
> > this, including loading/executing programs in terminal memory via
> > output - really. I'm sure it seemed like a good idea at the time, but I
can see
> how it could have been used for evil.
> 
> Do you also want to block "+++AT"?  :-)

Oh dear. Oh dear. You *do* know that actually could be bad. I wonder how
many git users are still using dial-up to clone/push. Of course, they would
probably not even see this message after trying to download it.

Chuckles,
Randall

-- Brief whoami:
 NonStop developer since approximately 2112884442
 UNIX developer since approximately 421664400
-- In my real life, I talk too much.

Re: [PATCH v2] dir.c: ignore paths containing .git when invalidating untracked cache

[Ben Peart]

On 2/7/2018 4:21 AM, Nguyễn Thái Ngọc Duy wrote:

read_directory() code ignores all paths named ".git" even if it's not
a valid git repository. See treat_path() for details. Since ".git" is
basically invisible to read_directory(), when we are asked to
invalidate a path that contains ".git", we can safely ignore it
because the slow path would not consider it anyway.

This helps when fsmonitor is used and we have a real ".git" repo at
worktree top. Occasionally .git/index will be updated and if the
fsmonitor hook does not filter it, untracked cache is asked to
invalidate the path ".git/index".

Without this patch, we invalidate the root directory unncessarily,
which:

- makes read_directory() fall back to slow path for root directory
   (slower)

- makes the index dirty (because UNTR extension is updated). Depending
   on the index size, writing it down could also be slow.



Thank you again, this patch makes much more sense to me.


A note about the new "safe_path" knob. Since this new check could be
relatively expensive, avoid it when we know it's not needed. If the
path comes from the index, it can't contain ".git". If it does
contain, we may be screwed up at many more levels, not just this one.



I do have a simplifying suggestion to make.  I noticed that other uses 
of verify_path() check when the potentially erroneous path is passed in 
and then all the underlying code can assume it is valid.  I think that 
makes sense here as well and it makes for a smaller patch.




diff --git a/fsmonitor.h b/fsmonitor.h
index cd3cc0ccf2..65f3743636 100644
--- a/fsmonitor.h
+++ b/fsmonitor.h
@@ -65,7 +65,7 @@ static inline void mark_fsmonitor_invalid(struct index_state 
*istate, struct cac
  {
if (core_fsmonitor) {
ce->ce_flags &= ~CE_FSMONITOR_VALID;
-   untracked_cache_invalidate_path(istate, ce->name);
+   untracked_cache_invalidate_path(istate, ce->name, 1);


This test isn't needed because we're pulling the name right out of the 
cache entry so it doesn't need to be verified.


Here is a modified version of your patch for consideration:



read_directory() code ignores all paths named ".git" even if it's not
a valid git repository. See treat_path() for details. Since ".git" is
basically invisible to read_directory(), when we are asked to
invalidate a path that contains ".git", we can safely ignore it
because the slow path would not consider it anyway.

This helps when fsmonitor is used and we have a real ".git" repo at
worktree top. Occasionally .git/index will be updated and if the
fsmonitor hook does not filter it, untracked cache is asked to
invalidate the path ".git/index".

Without this patch, we invalidate the root directory unnecessarily,
which:

- makes read_directory() fall back to slow path for root directory
  (slower)

- makes the index dirty (because UNTR extension is updated). Depending
  on the index size, writing it down could also be slow.

Noticed-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Ben Peart 
---

Notes:
Base Ref: master
Web-Diff: https://github.com/benpeart/git/commit/218a577618
Checkout: git fetch https://github.com/benpeart/git verify_path-v1 
&& git checkout 218a577618


 dir.c   |  2 +-
 fsmonitor.c |  6 +-
 t/t7519-status-fsmonitor.sh | 39 +++
 3 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index 7c4b45e30e..d431da46f5 100644
--- a/dir.c
+++ b/dir.c
@@ -1773,7 +1773,7 @@ static enum path_treatment treat_path(struct 
dir_struct *dir,

if (!de)
return treat_path_fast(dir, untracked, cdir, istate, path,
   baselen, pathspec);
-   if (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, ".git"))
+   if (is_dot_or_dotdot(de->d_name) || !fspathcmp(de->d_name, ".git"))
return path_none;
strbuf_setlen(path, baselen);
strbuf_addstr(path, de->d_name);
diff --git a/fsmonitor.c b/fsmonitor.c
index 0af7c4edba..019576f306 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -118,8 +118,12 @@ static int query_fsmonitor(int version, uint64_t 
last_update, struct strbuf *que


 static void fsmonitor_refresh_callback(struct index_state *istate, 
const char *name)

 {
-   int pos = index_name_pos(istate, name, strlen(name));
+   int pos;

+   if (!verify_path(name))
+   return;
+
+   pos = index_name_pos(istate, name, strlen(name));
if (pos >= 0) {
struct cache_entry *ce = istate->cache[pos];
ce->ce_flags &= ~CE_FSMONITOR_VALID;
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index eb2d13bbcf..756beb0d8e 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -314,4 +314,43 @@ test_expect_success 'splitting the 

[PATCHv4] tag: add --edit option

[Nicolas Morey-Chaisemartin]

Add a --edit option whichs allows modifying the messages provided by -m or -F,
the same way git commit --edit does.

Signed-off-by: Nicolas Morey-Chaisemartin 
---

Fixes since v3 ( 
https://public-inbox.org/git/88e7c122-599f-4ab1-6d65-c75f7a3ae...@suse.com/ ):
 * Replace tab by space in t/t7004-tag.sh


 Documentation/git-tag.txt |  8 +++-
 builtin/tag.c | 11 +--
 t/t7004-tag.sh| 30 ++
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 956fc019f984..1d17101bac39 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -9,7 +9,7 @@ git-tag - Create, list, delete or verify a tag object signed 
with GPG
 SYNOPSIS
 
 [verse]
-'git tag' [-a | -s | -u ] [-f] [-m  | -F ]
+'git tag' [-a | -s | -u ] [-f] [-m  | -F ] [-e]
 [ | ]
 'git tag' -d ...
 'git tag' [-n[]] -l [--contains ] [--no-contains ]
@@ -167,6 +167,12 @@ This option is only applicable when listing tags without 
annotation lines.
Implies `-a` if none of `-a`, `-s`, or `-u `
is given.
 
+-e::
+--edit::
+   The message taken from file with `-F` and command line with
+   `-m` are usually used as the tag message unmodified.
+   This option lets you further edit the message taken from these sources.
+
 --cleanup=::
This option sets how the tag message is cleaned up.
The  '' can be one of 'verbatim', 'whitespace' and 'strip'.  The
diff --git a/builtin/tag.c b/builtin/tag.c
index a7e6a5b0f234..ce5cac3dd23f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -194,6 +194,7 @@ static int build_tag_object(struct strbuf *buf, int sign, 
struct object_id *resu
 
 struct create_tag_options {
unsigned int message_given:1;
+   unsigned int use_editor:1;
unsigned int sign;
enum {
CLEANUP_NONE,
@@ -224,7 +225,7 @@ static void create_tag(const struct object_id *object, 
const char *tag,
tag,
git_committer_info(IDENT_STRICT));
 
-   if (!opt->message_given) {
+   if (!opt->message_given || opt->use_editor) {
int fd;
 
/* write the template message before editing: */
@@ -233,7 +234,10 @@ static void create_tag(const struct object_id *object, 
const char *tag,
if (fd < 0)
die_errno(_("could not create file '%s'"), path);
 
-   if (!is_null_oid(prev)) {
+   if (opt->message_given) {
+   write_or_die(fd, buf->buf, buf->len);
+   strbuf_reset(buf);
+   } else if (!is_null_oid(prev)) {
write_tag_body(fd, prev);
} else {
struct strbuf buf = STRBUF_INIT;
@@ -372,6 +376,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
static struct ref_sorting *sorting = NULL, **sorting_tail = 
struct ref_format format = REF_FORMAT_INIT;
int icase = 0;
+   int edit_flag = 0;
struct option options[] = {
OPT_CMDMODE('l', "list", , N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, , N_("n"),
@@ -386,6 +391,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_CALLBACK('m', "message", , N_("message"),
 N_("tag message"), parse_msg_arg),
OPT_FILENAME('F', "file", , N_("read message from 
file")),
+   OPT_BOOL('e', "edit", _flag, N_("force edit of tag 
message")),
OPT_BOOL('s', "sign", , N_("annotated and GPG-signed 
tag")),
OPT_STRING(0, "cleanup", _arg, N_("mode"),
N_("how to strip spaces and #comments from message")),
@@ -524,6 +530,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die(_("tag '%s' already exists"), tag);
 
opt.message_given = msg.given || msgfile;
+   opt.use_editor = edit_flag;
 
if (!cleanup_arg || !strcmp(cleanup_arg, "strip"))
opt.cleanup_mode = CLEANUP_ALL;
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index a9af2de9960b..0630f2dee24b 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -452,6 +452,21 @@ test_expect_success \
test_cmp expect actual
 '
 
+get_tag_header annotated-tag-edit $commit commit $time >expect
+echo "An edited message" >>expect
+test_expect_success 'set up editor' '
+   write_script fakeeditor <<-\EOF
+   sed -e "s/A message/An edited message/g" <"$1" >"$1-"
+   mv "$1-" "$1"
+   EOF
+'
+test_expect_success \
+   'creating an annotated tag with -m message --edit should succeed' '
+   EDITOR=./fakeeditor git tag -m "A message" --edit annotated-tag-edit &&
+   get_tag_msg annotated-tag-edit >actual &&
+   test_cmp expect actual
+'
+
 cat >msgfile 

Re: git: CVE-2018-1000021: client prints server sent ANSI escape codes to the terminal, allowing for unverified messages to potentially execute arbitrary commands

[Andreas Schwab]

On Feb 06 2018, "Randall S. Becker"  wrote:

> What I don't know - and it's not explicitly in the CVE - is just how many
> other terminal types with similar vulnerabilities are out there, but I'm
> suspecting it's larger than one would guess - mostly, it seems like this
> particular sequence is intended to be used for writing status line output
> (line 25?) instead of sticking it in a prompt. This can be used prettifies a
> lengthy bash prompt to display the current branch and repository at the
> bottom of the screen instead of in the inline prompt, but that's the user's
> choice and not something git has to deal with. There were some green-screen
> terminals with other weird ESC sequences back in the day that could really
> get into trouble with this, including loading/executing programs in terminal
> memory via output - really. I'm sure it seemed like a good idea at the time,
> but I can see how it could have been used for evil.

Do you also want to block "+++AT"?  :-)

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFC PATCH 000/194] Moving global state into the repository object

[Jeff Hostetler]

On 2/5/2018 6:51 PM, Stefan Beller wrote:

This series moves a lot of global state into the repository struct.
It applies on top of 2512f15446149235156528dafbe75930c712b29e (2.16.0)
It can be found at https://github.com/stefanbeller/git/tree/object-store

Motivation for this series:
* Easier to reason about code when all state is stored in one place,
   for example for multithreading
* Easier to move to processing submodules in-process instead of
   calling a processes for the submodule handling.
   The last patch demonstrates this.

[...]

Very nice.  My eyes glazed over a few times, but I like the
direction you're heading here.

Thanks for tackling this!
Jeff

Re: Shawn Pearce has died

[Jeff King]

On Tue, Jan 30, 2018 at 01:49:08PM -0500, Jeff King wrote:

> On Mon, Jan 29, 2018 at 11:15:55PM -0800, Chris DiBona wrote:
> 
> > That's a fantastic idea.  When is the contrib summit and is it open to
> > others? I could send someone ...
> 
> It's March 7th in Barcelona. Details are here:
> 
>   https://public-inbox.org/git/20180119001034.ga29...@sigill.intra.peff.net/
> 
> There will be GitHub video folks on premises the following day for the
> Git Merge main conference, but I'm looking into whether they'll be
> around to record a few interviews on the contributor summit day.

Unfortunately I just learned that we (GitHub) are not actually sending
video folks to Barcelona this year. So we (Git) are on our own for
recording anything. Certainly we can record something with one of our
cameras, though I wonder if it might be simpler to just have people
record themselves and send it to Chris, then.

-Peff

Re: [PATCH v2 04/14] commit-graph: implement construct_commit_graph()

[Derrick Stolee]

On 2/7/2018 10:08 AM, SZEDER Gábor wrote:

On Mon, Feb 5, 2018 at 5:06 PM, Derrick Stolee  wrote:

On 2/2/2018 10:32 AM, SZEDER Gábor wrote:

In my git repo, with 9 pack files at the moment, i.e. not that big a
repo and not that many pack files:

$ time ./git commit-graph --write --update-head
4df41a3d1cc408b7ad34bea87b51ec4ccbf4b803

real0m27.550s
user0m27.113s
sys 0m0.376s

In comparison, performing a good old revision walk to gather all the
info that is written into the graph file:

$ time git log --all --topo-order --format='%H %T %P %cd' |wc -l
52954

real0m0.903s
user0m0.972s
sys 0m0.058s


Two reasons this is in here:

(1) It's easier to get the write implemented this way and add the reachable
closure later (which I do).

(2) For GVFS, we want to add all commits that arrived in a "prefetch pack"
to the graph even if we do not have a ref that points to the commit yet. We
expect many commits to become reachable soon and having them in the graph
saves a lot of time in merge-base calculations.

So, (1) is for patch simplicity, and (2) is why I want it to be an option in
the final version. See the --stdin-packs argument later for a way to do this
incrementally.

I expect almost all users to use the reachable closure method with
--stdin-commits (and that's how I will integrate automatic updates with
'fetch', 'repack', and 'gc' in a later patch).

I see.  I was about to ask about the expected use-cases of the
'--stdin-packs' option, considering how much slower it is to enumerate
all objects in pack files, but run out of time after patch 10.

The run-time using '--stdin-commits' is indeed great:

   $ time { git for-each-ref --format='%(objectname)' refs/heads/ | ./git
 commit-graph --write --update-head --stdin-commits ; }
   82fe9a5cd715ff578f01f7f44e0611d7902d20c8

   real  0m0.985s
   user  0m0.916s
   sys   0m0.024s

Considering the run-time difference, I think in the end it would be a
better default for a plain 'git commit-graph --write' to traverse
history from all refs, and it should enumerate pack files only if
explicitly told so via '--stdin-packs'.

To be clear: I'm not saying that traversing history should already be
the default when introducing construct_commit_graph() and '--write'.  If
enumerating pack files keeps the earlier patches simpler and easier to
review, then by all means stick with it, and only change the
'--stdin-*'-less behavior near the end of the series, when all the
building blocks are already in place (but then mention this in the early
commit messages).


I will consider this.


I have also noticed a segfault when feeding non-commit object names to
'--stdin-commits', i.e. when I run the above command without restricting
'git for-each-ref' to branches and it listed object names of tags as
well.

   $ git rev-parse v2.16.1 |./git commit-graph --write --update-head
--stdin-commits
   error: Object eb5fcb24f69e13335cf6a6a1b1d4553fa2b0f202 not a commit
   error: Object eb5fcb24f69e13335cf6a6a1b1d4553fa2b0f202 not a commit
   error: Object eb5fcb24f69e13335cf6a6a1b1d4553fa2b0f202 not a commit
   Segmentation fault

(gdb) bt
#0  __memcpy_avx_unaligned ()
 at ../sysdeps/x86_64/multiarch/memcpy-avx-unaligned.S:126
#1  0x004ea97c in sha1write (f=0x356bbf0, buf=0x4, count=20)
 at csum-file.c:104
#2  0x004d98e1 in write_graph_chunk_data (f=0x356bbf0, hash_len=20,
 commits=0x3508de0, nr_commits=50615) at commit-graph.c:506
#3  0x004da9ca in construct_commit_graph (
 pack_dir=0x8ff360 ".git/objects/pack", pack_indexes=0x0, nr_packs=0,
 commit_hex=0x8ff790, nr_commits=1) at commit-graph.c:818
#4  0x0044184e in graph_write () at builtin/commit-graph.c:149
#5  0x00441a8c in cmd_commit_graph (argc=0, argv=0x7fffe310,
 prefix=0x0) at builtin/commit-graph.c:224
#6  0x00405a0a in run_builtin (p=0x8ad950 , argc=4,
 argv=0x7fffe310) at git.c:346
#7  0x00405ce4 in handle_builtin (argc=4, argv=0x7fffe310)
 at git.c:555
#8  0x00405ec8 in run_argv (argcp=0x7fffe1cc, argv=0x7fffe1c0)
 at git.c:607
#9  0x00406079 in cmd_main (argc=4, argv=0x7fffe310) at git.c:684
#10 0x004a85c8 in main (argc=5, argv=0x7fffe308)
 at common-main.c:43


I noticed this while preparing v3. I have a fix, but you now remind me 
that I need to add tags to the test script.


Thanks,
-Stolee

Re: [PATCH v2 04/14] commit-graph: implement construct_commit_graph()

[SZEDER Gábor]

On Mon, Feb 5, 2018 at 5:06 PM, Derrick Stolee  wrote:
> On 2/2/2018 10:32 AM, SZEDER Gábor wrote:

>> In my git repo, with 9 pack files at the moment, i.e. not that big a
>> repo and not that many pack files:
>>
>>$ time ./git commit-graph --write --update-head
>>4df41a3d1cc408b7ad34bea87b51ec4ccbf4b803
>>
>>real0m27.550s
>>user0m27.113s
>>sys 0m0.376s
>>
>> In comparison, performing a good old revision walk to gather all the
>> info that is written into the graph file:
>>
>>$ time git log --all --topo-order --format='%H %T %P %cd' |wc -l
>>52954
>>
>>real0m0.903s
>>user0m0.972s
>>sys 0m0.058s
>
>
> Two reasons this is in here:
>
> (1) It's easier to get the write implemented this way and add the reachable
> closure later (which I do).
>
> (2) For GVFS, we want to add all commits that arrived in a "prefetch pack"
> to the graph even if we do not have a ref that points to the commit yet. We
> expect many commits to become reachable soon and having them in the graph
> saves a lot of time in merge-base calculations.
>
> So, (1) is for patch simplicity, and (2) is why I want it to be an option in
> the final version. See the --stdin-packs argument later for a way to do this
> incrementally.
>
> I expect almost all users to use the reachable closure method with
> --stdin-commits (and that's how I will integrate automatic updates with
> 'fetch', 'repack', and 'gc' in a later patch).

I see.  I was about to ask about the expected use-cases of the
'--stdin-packs' option, considering how much slower it is to enumerate
all objects in pack files, but run out of time after patch 10.

The run-time using '--stdin-commits' is indeed great:

  $ time { git for-each-ref --format='%(objectname)' refs/heads/ | ./git
commit-graph --write --update-head --stdin-commits ; }
  82fe9a5cd715ff578f01f7f44e0611d7902d20c8

  real  0m0.985s
  user  0m0.916s
  sys   0m0.024s

Considering the run-time difference, I think in the end it would be a
better default for a plain 'git commit-graph --write' to traverse
history from all refs, and it should enumerate pack files only if
explicitly told so via '--stdin-packs'.

To be clear: I'm not saying that traversing history should already be
the default when introducing construct_commit_graph() and '--write'.  If
enumerating pack files keeps the earlier patches simpler and easier to
review, then by all means stick with it, and only change the
'--stdin-*'-less behavior near the end of the series, when all the
building blocks are already in place (but then mention this in the early
commit messages).


I have also noticed a segfault when feeding non-commit object names to
'--stdin-commits', i.e. when I run the above command without restricting
'git for-each-ref' to branches and it listed object names of tags as
well.

  $ git rev-parse v2.16.1 |./git commit-graph --write --update-head
--stdin-commits
  error: Object eb5fcb24f69e13335cf6a6a1b1d4553fa2b0f202 not a commit
  error: Object eb5fcb24f69e13335cf6a6a1b1d4553fa2b0f202 not a commit
  error: Object eb5fcb24f69e13335cf6a6a1b1d4553fa2b0f202 not a commit
  Segmentation fault

(gdb) bt
#0  __memcpy_avx_unaligned ()
at ../sysdeps/x86_64/multiarch/memcpy-avx-unaligned.S:126
#1  0x004ea97c in sha1write (f=0x356bbf0, buf=0x4, count=20)
at csum-file.c:104
#2  0x004d98e1 in write_graph_chunk_data (f=0x356bbf0, hash_len=20,
commits=0x3508de0, nr_commits=50615) at commit-graph.c:506
#3  0x004da9ca in construct_commit_graph (
pack_dir=0x8ff360 ".git/objects/pack", pack_indexes=0x0, nr_packs=0,
commit_hex=0x8ff790, nr_commits=1) at commit-graph.c:818
#4  0x0044184e in graph_write () at builtin/commit-graph.c:149
#5  0x00441a8c in cmd_commit_graph (argc=0, argv=0x7fffe310,
prefix=0x0) at builtin/commit-graph.c:224
#6  0x00405a0a in run_builtin (p=0x8ad950 , argc=4,
argv=0x7fffe310) at git.c:346
#7  0x00405ce4 in handle_builtin (argc=4, argv=0x7fffe310)
at git.c:555
#8  0x00405ec8 in run_argv (argcp=0x7fffe1cc, argv=0x7fffe1c0)
at git.c:607
#9  0x00406079 in cmd_main (argc=4, argv=0x7fffe310) at git.c:684
#10 0x004a85c8 in main (argc=5, argv=0x7fffe308)
at common-main.c:43

Bug report: Subtree split including extra commits

[Daniel Karp]

Apologies if this is the wrong place to send a bug report for
Contributed software.

I've run into what seems like an issue/bug with git subtree.

I am trying to split a single directory of our repo into its own repo
using git subtree. I ran the the following command from our project
root:
git subtree split --prefix=geekui2 -b geekui2-split
where geekui2 is the name of the subdir.

For commits before mid 2017, the created split branch contains our
entire commit history, regardless whether or not they include changes
in geekui2. For commits after that point, the commits are properly
filtered to include only commits that contain changes in geekui2.

The upshot is that if I push the branch to a new repo, and check out
one of those earlier commits, I can essentially recover the entire
codebase of our application (at that point), not just the content in
geekui2. Since our goal was to share part of our repo while keeping
the rest of it private, this is obviously a problem.

I've also tried using git-subrepo for this split--it seems to
correctly filter the commits, excluding all commits without changes to
geekui2. So something seems to be going wrong with the way git-subtree
handles this relative to git subrepo.

Unfortunately, there is a lot going on in our repo--I have no idea how
I would generate a minimal reproduction of this.

While our repo is private, I'm happy to help try to help debug this if
someone wants to take a look at this issue, although it is not my area
of expertise.

My git version is 2.16.1, and I am running it on macOS 10.12.6.

I am not subscribed to the git mailing list, so if anyone wants to get
in touch with me about this, I'm most likely to see it if you send an
email.

--
Daniel Karp

Re: [PATCH] files-backend: unlock packed store only if locked

[Jeff King]

On Tue, Feb 06, 2018 at 12:36:15PM -0800, Jonathan Tan wrote:

> In commit 42c7f7ff9685 ("commit_packed_refs(): remove call to
> `packed_refs_unlock()`", 2017-06-23), a call to packed_refs_unlock() was
> added to files_initial_transaction_commit() in order to compensate for
> removing that call from commit_packed_refs(). However, that call was
> added in the cleanup section, which is run even if the packed_ref_store
> was never locked (which happens if an error occurs earlier in the
> function).
> 
> Create a new cleanup goto target which runs packed_refs_unlock(), and
> ensure that only goto statements after a successful invocation of
> packed_refs_lock() jump there.

The explanation and patch look good to me.

But this all seemed strangely familiar... I think this is the same bug
as:

  https://public-inbox.org/git/20180118143841.1a4c674d@novascotia/

which is queued as mr/packed-ref-store-fix. It's listed as "will merge
to next" in the "what's cooking" from Jan 31st.

> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index f75d960e1..89bc5584a 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -2931,13 +2931,14 @@ static int files_initial_transaction_commit(struct 
> ref_store *ref_store,
>  
>   if (initial_ref_transaction_commit(packed_transaction, err)) {
>   ret = TRANSACTION_GENERIC_ERROR;
> - goto cleanup;
> + goto locked_cleanup;
>   }
>  
> +locked_cleanup:
> + packed_refs_unlock(refs->packed_ref_store);
>  cleanup:
>   if (packed_transaction)
>   ref_transaction_free(packed_transaction);
> - packed_refs_unlock(refs->packed_ref_store);

I actually like this double-label a bit more than what is queued on
mr/packed-ref-store-fix, though I am OK with either solution.

-Peff

Re: [PATCH 02/10] t5812: add 'test_i18ngrep's missing filename parameter

[Jeff King]

On Wed, Feb 07, 2018 at 02:53:17PM +0100, SZEDER Gábor wrote:

> > The "too old" curl is older than 7.19.4, which we actually fail to build
> > with since v2.12.0. So they probably did not even get as far as the
> > tests. ;)
> 
> Oh, OK, I was not aware of that.  The oldest non-maintenance release
> with the missing filename parameter is v2.7.0, so that's still a 5
> releases time frame to notice it.

Actually, I'm wrong. It looks like we did finally fix it in f18777ba6e
(http: fix handling of missing CURLPROTO_*, 2017-08-11), which is in
v2.15. So:

> Anyway, I'm preparing v2 of this series, and I'm not sure what to do
> about this.
> 
>   - Should I simply drop the "your curl version is too old" pattern?  It
> would make sense, but it just doesn't feel quite right to remove it
> while the corresponding printf() is still there, even if it can't be
> triggered anymore.  However, cleaning up the curl version checks in
> http.c to remove this message is beyond the scope of this patch
> series.
> 
>   - Or leave it almost-as-is, only dropping the now unnecessary curly
> braces as Simon pointed out.  And perhaps a bit of update to the
> commit message.
> 
> I'd prefer the second option.

Yeah, I think just leave it as-is. Thanks.

-Peff

Re: [PATCH v1] dir.c: don't flag the index as dirty for changes to the untracked cache

[Ben Peart]

On 2/6/2018 7:27 AM, Duy Nguyen wrote:


This is another thing that bugs me. I know you're talking about huge
index files, but at what size should we start this sort of
optimization? Writing down a few MBs on linux is cheap enough that I
won't bother optimizing (and I get my UNTR extension repaired all the
time, so reduced lstat calls and stuff). This "typically" only comes
at certain size, what size?



It's important to identify what we're trading off here.  With the 
proposed optimization, we'll end up doing less writes of the index but 
potentially more lstat calls.  Even with a small index, writing the 
index is much more expensive than calling lstat on a file.  Exactly how 
much more expensive depends on a lot of variables but even with a SSD 
its still orders of magnitude difference.


That means we could potentially lstat hundreds or thousands of files and 
still come out ahead.  Given the untracked cache works at the directory 
level, the lstat cost actually scales with the number of directories 
that have had changes (ie changing a 2nd file in the same directory 
doesn't add any additional cost).  In "typical" workflows, developers 
don't often change hundreds of files across different directories so 
we'd "typically" still come out ahead.


We have internal performance tests based on common developer sequences 
of commands (status, edit a file, status, add, status, commit, log, 
push, etc) that show that having the untracked cache turned on actually 
makes these sequences slower.  At the time, we didn't have time to 
investigate why so we just turned off the untracked cache.


When writing the fsmonitor patch series which can utilize the untracked 
cache, I did some investigation into why the untracked cache was slowing 
things down in these tests and discovered the cause was the additional 
index writes.  That is what led to this patch.


I've been sitting on it for months now because I didn't have the time to 
write some performance tests for the git perf framework to demonstrate 
the problem and how this helps solve it.  With the discussion about the 
fsmonitor extension, I thought this might be a good time to send it out 
there.


If you have the cycles, time a typical series of commands with and 
without the untracked cache (ie don't just call status over and over in 
a loop) and you should see the same results.  Given my limited time 
right now, I'm OK putting this on the back burner again until a git perf 
test can be written to ensure it actually speeds things up as advertised.

Re: [bug report]: error doing_rebase

[Johannes Schindelin]

Hi Bulat,

Please make sure to keep the Git mailing list in Cc: (I get *very* prickly
when Git users treat me as a free-of-cost help desk, and when I get that
annoyed, I stop helping).

On Tue, 6 Feb 2018, Bulat Musin wrote:

> Yes, I tested again.
> 
> With built 2.16... and it shows error message. git rebase --abort restores
> 
> state before rebase.

You misunderstood me. I am convinced that that error message *is correct*.
It shows an incorrect usage. You cannot start off an interactive rebase by
a `squash` command.

> With git 2.14 from Ubuntu's repo it works - 3 commits are squashed into first
> one

Yes, but you called `git rebase -i HEAD~2`, which means that only two
commits were up for rebasing. The third commit is outside the range
`HEAD~2..` which the command `git rebase -i HEAD~2` wants to let you rebase.

If v2.14 indeed modified `HEAD~2` (as I suspected in my earlier mail),
then you successfully confirmed that we fixed a bug, and that you expected
the buggy behavior.

> - with change SHA.
> 
> It seems to be bug in recent version.
> 
> Should I provide additional information?

Ciao,
Johannes

> On 02/06/2018 12:47 PM, Johannes Schindelin wrote:
> > Hi,
> >
> > On Mon, 5 Feb 2018, Bulat Musin wrote:
> >
> > > Now there are 3 sequential commits, I want to squash them into 1:
> > >
> > > git rebase -i HEAD~2
> > >
> > > In editor I changed all "pick" to "squash", saved file, I got:
> > >
> > > error: cannot 'squash' without a previous commit
> > You cannot start with a squash. You have to pick the first one, then
> > squash the second into the first.
> >
> > > However, 2.14.1 from Ubuntu's repo does the job - squashes 3 commits into
> > > 1.
> > It may be careless enough to do that, however, it might now have modified
> > the *wrong* commit, i.e. squashed the two patches *into HEAD~2*.
> >
> > Please verify that your HEAD~2 is still intact and part of the rebased
> > history, otherwise you will have a problem.
> >
> > Ciao,
> > Johannes
> 
> 

Re: [PATCH 02/10] t5812: add 'test_i18ngrep's missing filename parameter

[SZEDER Gábor]

On Fri, Jan 26, 2018 at 7:27 PM, Jeff King  wrote:
> On Fri, Jan 26, 2018 at 01:37:00PM +0100, SZEDER Gábor wrote:
>
>> The second 'test_i18ngrep' invocation in the test 'curl redirects
>> respect whitelist' is missing its filename parameter.  This has
>> remained unnoticed since its introduction in f4113cac0 (http: limit
>> redirection to protocol-whitelist, 2015-09-22), because it would only
>> cause the test to fail if Git was built with a sufficiently old
>> libcurl version.  The test's two ||-chained 'test_i18ngrep'
>> invocations are supposed to check that either one of the two patterns
>> is present in 'git clone's error message.  As it happens, the first
>> invocation covers the error message from any reasonably up-to-date
>> libcurl, thus the second invocation, the one without the filename
>> parameter, isn't executed at all.  Apparently no one has run the test
>> suite's httpd tests with such an old libcurl in the last 2+ years, or
>> at least they haven't bothered to notify us about the failed test.
>
> Interesting find.
>
> The "too old" curl is older than 7.19.4, which we actually fail to build
> with since v2.12.0. So they probably did not even get as far as the
> tests. ;)

Oh, OK, I was not aware of that.  The oldest non-maintenance release
with the missing filename parameter is v2.7.0, so that's still a 5
releases time frame to notice it.

Anyway, I'm preparing v2 of this series, and I'm not sure what to do
about this.

  - Should I simply drop the "your curl version is too old" pattern?  It
would make sense, but it just doesn't feel quite right to remove it
while the corresponding printf() is still there, even if it can't be
triggered anymore.  However, cleaning up the curl version checks in
http.c to remove this message is beyond the scope of this patch
series.

  - Or leave it almost-as-is, only dropping the now unnecessary curly
braces as Simon pointed out.  And perhaps a bit of update to the
commit message.

I'd prefer the second option.

>> Fix this by consolidating the two patterns into a single extended
>> regexp, eliminating the need for an ||-chained second 'test_i18ngrep'
>> invocation.
>
> OK. Once upon a time I think we had trouble with "grep -E", since some
> older systems had only "egrep". But I see we've introduced some "grep
> -E" invocations as far back as 2013 and nobody has complained, so it's
> probably fine.

Yeah, first I went with the more traditional "\(this\|that\)" pattern,
but then noticed that 'grep -E' is already used in a couple of places,
and picked the format that uses less escape characters.

Re: [PATCH v1] dir.c: don't flag the index as dirty for changes to the untracked cache

[Ben Peart]

On 2/7/2018 5:59 AM, Duy Nguyen wrote:

On Tue, Feb 6, 2018 at 7:55 PM, Duy Nguyen  wrote:

On Tue, Feb 6, 2018 at 7:27 PM, Duy Nguyen  wrote:

On Tue, Feb 6, 2018 at 8:48 AM, Ben Peart  wrote:

With the new behavior, making a change in dir1/, then calling status would
update the dir1/ untracked cache entry but not write it out. On the next
status, git would detect the change in dir1/ again and update the untracked


Thing only missing piece here I would add is, this dir1/ detection is
done by watchman. We have to contact watchman and ask the set of
changed paths since $TIME where $TIME is the last time we updated
untracked cache and invalidate those paths in core. Then it should
work correctly. I checked the watchman query in the fsmonitor hook and
I think it's correct so far.


No I think I'm wrong. And worse, I think the interaction between FSNM
and UNTR extension is broken. This is partly from me guessing how
fsmonitor works so I may be double-wrong.

UNTR extension is supposed to cover the untracked state at timestamp
$X. Where $X is stored in FSNM extension. Correct?

When fsmonitor is used and read_directory() is executed (at timestamp
$Y), we ask watchman "what's new on worktree since $X?"). We get the
list, we invalidate relevant directories so we can see new untracked
entries (or lack of old untracked entries). We write FSNM with
timestamp $Y down. We may or may not write UNTR down because of this
patch, but let's suppose we do. All is good. UNTR now records the
state at $Y. FSNM says $Y. This is how it works (when there's no bugs)

UNTR extension is only updated when read_directory() is called. It
does not always happen. FSNM is updated all the time (almost at every


I was indeed doubly wrong. When FSMN is read, it does make calls to
invalidate stuff in untracked cache data structure. If the index is
written down (with updated FSMN extension and timestamp) then the UNTR
extension, which is generated from in-core untracked data structure,
also reflects the damages by the changed paths so next time even if
those changed paths are not reported again (between $Y and $Z below),
it's fine.

All is good in the world again :)


Thank you for looking into this closely.  It's always good to have 
someone else examine the logic to make sure there aren't errors that 
were missed.  Sorry my responses have been slow, my day job has had me 
busy lately.





git command since most of them needs to read index, many write it
down) with new timestamp. Suppose FSNM now records timestamp $Z, UNTR
still records the state at $Y because in the last index update,
read_directory() is not executed. 4 files have been added between $Y
and $Z. When we ask watchman "what's new since $Z?" we will not see
those files, so we don't invalidate relevant directories and
read_directory() will not see those files.

BUG: On some Linux systems, "Stage To Commit" hotkey in git-gui stages one file only, even if multiple files are selected

[Birger Skogeng Pedersen]

In git-gui, multiple files from the "Unstaged Changes" widget can be
selected. Once multiple files are selected, they can be staged by
clicking (toolbar) "Commit"->"Stage To Commit". All the files that
were selected then gets staged for the commit. The "Stage To Commit"
hotkey (CTRL+T) behaves like the command itself, staging all files
that are selected (Tested on Windows 10).
However, on some linux systems (currently tested on Ubuntu 17.10),
only one file (the most recently selected) gets staged when using the
"Stage To Commit" hotkey (CTRL+T), while the other, selected files
remain unstaged.

Steps to reproduce (on Ubuntu 17.10):
- Open git-gui in a repository with two or more unstaged, changed files
- Select two or more files in the "Unstaged Changes"
- Click CTRL+T to stage the files that you have selected
(Only a single file will get staged)

Re: BUG: fetch in certain repo always gives "did not send all necessary objects"

[Jeff King]

On Wed, Feb 07, 2018 at 08:21:57AM -0500, Jeff King wrote:

> The best PSA for this particular bug may be "try pruning the worktrees":
> 
>   $ git worktree prune -v
>   Removing worktrees/foo: gitdir file points to non-existent location
> 
>   $ git prune; echo $?
>   0

Sorry, I just read Stefan's response and not your original before
responding. I see that you did try that. So IMHO that's a separate bug
that "worktree prune" did not do the right thing for your particular
case.

-Peff

Re: BUG: fetch in certain repo always gives "did not send all necessary objects"

[Jeff King]

On Wed, Feb 07, 2018 at 06:08:40PM +0700, Duy Nguyen wrote:

> > And/or ideas of what steps could cause corruption so I can send out a
> > PSA to help users avoid it?
> 
> There is another thing we could do. One bad HEAD should not abort the
> entire operation (at least if it's not the current worktree's HEAD).
> We could still give a warning and move on, or don't warn at all and
> let "git worktree prune" collect it (which I see from your message
> that it also fails to do).

Whether that's safe or not depends very much on what the caller intends
to do with the ref. Skipping broken refs in "git prune" is a bad idea,
for instance.

-Peff

Re: BUG: fetch in certain repo always gives "did not send all necessary objects"

[Jeff King]

On Tue, Feb 06, 2018 at 04:00:32PM -0800, Elijah Newren wrote:

> > According to Peff this got fixed
> > https://public-inbox.org/git/20171020031630.44zvzh3d2vlhg...@sigill.intra.peff.net/
> > and but you've had a corrupted repo from back when you were using an older
> > version of Git.
> >
> > Did that repo exist before d0c39a49cc was rolled out? Then we can keep that
> > hypothesis of "left-over corruption" as Peff put it.
> 
> I'm somewhat confused by this explanation.  That precise commit is the
> one I bisected to that _caused_ the fetch to fail.  Also, there might
> be one important difference here -- in the link you provide, it
> suggests that you had a corrupted working directory that made use of a
> now gc'ed commit.  In the case I was able to dig into, we did not.
> (There was a left-over .git/worktree/ that had a now gc'ed
> commit, but no working directory that used it.)

If you had a corrupted .git/worktree//HEAD, then that does
sound like the same problem. It's true that the commit you bisected to
caused "fetch" to fail, but only because it started looking at more of
your corrupted repository. The corruption happened long before (and I
don't know exactly when it was fixed, but I couldn't replicate it
anymore; it might even still exist).

In your case it sounds like you have the extra twist that the matching
working directory for "" had gone away, but I don't think
that materially changes anything. Until you run "git worktree prune",
that HEAD file is still there and still supposed to be valid.

> I suspect you mean that there was another previous bug that induced
> corruption, that this commit fixed that other bug, while also
> introducing this new bug that makes folks' clones unusable because the
> error doesn't provide enough information for users to know how to fix.

If you want to call that last thing a bug, then I guess so. It's perhaps
a matter for the philosophers whether it is the fault of the new code to
start complaining about an existing on-disk corruption.

> It took me hours to figure it out, after users ran out of ideas and
> came and asked me for help.  (Maybe if I was familiar with worktree,
> and knew they had been using it, then I might have guessed that "HEAD"
> meant "not your actual HEAD but the HEAD of the vestige of some other
> worktree").

Yeah, this is the obvious thing that seems like it ought to be improved.

> Does anyone have pointers about what might be doable in terms of
> providing a more useful error message to allow users to recover?
> And/or ideas of what steps could cause corruption so I can send out a
> PSA to help users avoid it?

Here's a minimal manual reproduction:

  # new repo...
  git init
  git commit --allow-empty -m one

  # with a worktree...
  git worktree add foo
  git -C foo commit --allow-empty -m two
  obj=.git/objects/$(git rev-parse foo | sed 's#..#&/#')

  # now we stop using that worktree
  git -C foo checkout --detach
  git branch -f -D foo
  rm -rf foo

  # and this is the corruption; this might have happened ye olden days
  # because of a bug in the worktree code, but we'll assume that somehow
  # the object went away
  rm -f $obj

And now lots of commands may fail with confusing errors:

  $ git prune
  fatal: unable to parse object: HEAD

Unfortunately fixing that is a little tricky. In this case the stack
looks like:

  parse_object_or_die (oid=0x7fffd690, name=0x55792880 "HEAD") at 
object.c:239
  add_one_ref (path=0x55792880 "HEAD", oid=0x7fffd690, flag=0, 
cb_data=0x7fffd8e0) at reachable.c:38
  refs_head_ref (refs=0x55a65430, fn=0x556b6ef5 , 
cb_data=0x7fffd8e0) at refs.c:1316
  other_head_refs (fn=0x556b6ef5 , cb_data=0x7fffd8e0) at 
worktree.c:404

So other_head_refs knows that it's looking at the worktrees. And it
passes the alternate ref-store to refs_head_ref(), with "add_one_ref" as
the callback. But the knowledge that we're not talking about the real
"HEAD" is lost as we cross that callback boundary. We'd need to either
add another parameter to the callback, or have some way of talking about
"HEAD in this worktree" as a refname (which AFAIK we don't have).

As for PSAs, my normal go-to in confusing matters like this is git-fsck.
But it seems that it does not check worktree HEADs. :(

  $ git fsck
  Checking object directories: 100% (256/256), done.

So that seems like another bug.

The best PSA for this particular bug may be "try pruning the worktrees":

  $ git worktree prune -v
  Removing worktrees/foo: gitdir file points to non-existent location

  $ git prune; echo $?
  0

-Peff

categorization, documentation and packaging of "git core" commands

[Robert P. J. Day]

  (related to earlier thread but different enough that i'll start
fresh.)

  based on the collection of man page links here:

https://www.kernel.org/pub/software/scm/git/docs/

i took a look at how git 2.14.3 is laid out on my fedora 27 system,
particularly all of the executables under /usr/libexec/git-core/.
after manually cross-checking all of those executables against the
links on the docs page, here's what i learned.

  first, here are the executables under /usr/libexec/git-core/ that
are unreferenced by that web page, but that should be fine as almost
all of them would be considered underlying helpers or utilities
(except for things like git-subtree, but we're still unclear on its
status, right?):

  git-add--interactive
  git-bisect--helper
  git-credential-cache--daemon
  git-credential-libsecret
  git-credential-netrc
  git-difftool--helper
  git-fsck-objects
  git-gui--askpass
  git-init-db
  git-merge-octopus
  git-merge-ours
  git-merge-recursive
  git-merge-resolve
  git-merge-subtree
  git-mergetool--lib
  git-rebase--am
  git-rebase--helper
  git-rebase--interactive
  git-rebase--merge
  git-remote-ext
  git-remote-fd
  git-remote-ftp
  git-remote-ftps
  git-remote-http
  git-remote-https
  git-sh-i18n--envsubst
  git-stage
  git-submodule--helper
  git-subtree
  git-web--browse

on the other hand (and this is not so much a git issue as a fedora
packaging issue), there are a number of command links at that web page
that are supplied by distinct RPM packages rather than by the basic
fedora git package, so one would need to install the following
packages to get some of those commands on fedora:

  * gitk
  * git-cvs
  * git-svn
  * git-p4
  * git-email (provides git-send-email)

finally, from fedora, i am utterly unable to find a package that
provides git-archimport. pretty sure fedora used to have a "git-arch"
package but it's not there now.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: is http://git-scm.com an *official* git-affiliated site?

[Jean-Noël Avila]

On 07/02/2018, Jeff King wrote:

> On Wed, Feb 07, 2018 at 12:37:32PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>
>> It's not fully auto-generated so stuff like git-worktree doesn't get
>> added automatically, I just added a PR for that:
>> https://github.com/git/git-scm.com/pull/1133
> Thanks for doing that. I'm also open to auto-generating the index if we
> can come up with well-organized output.
>
> -Peff


I did not know that git-worktree is not considered ready for general
consumption. It has been present in the release notes for quite some
time now.


If there's something available from the git repo to drive the build of
the index, that would be a good way to advert the publicly available
commands of git.


JN

Re: is http://git-scm.com an *official* git-affiliated site?

[Jeff King]

On Wed, Feb 07, 2018 at 11:34:51AM +, pedro rijo wrote:

> The command list under https://git-scm.com/docs doesn't show all the
> commands. It's a manually curated list as we can see at
> 
> - https://github.com/git/git-scm.com/blob/master/app/views/doc/ref.html.erb
> - https://github.com/git/git-scm.com/tree/master/app/views/shared/ref
> 
> I'm not sure if the goal ever was to list all available commands or to just
> list some important existing commands (cc @Peff).
>
> If we want to list all available commands, there's some work that must be
> done in order to automate that, since it's not feasible to manually add
> each new command.

I think we _could_ just add all commands, but there's some value in
organizing them. I'm not sure if there's enough information in git.git
to do that organization. But we could also have a curated list of some
subset of the commands, and then dump the rest in an alphabetized index.

-Peff

Re: is http://git-scm.com an *official* git-affiliated site?

[Jeff King]

On Wed, Feb 07, 2018 at 12:37:32PM +0100, Ævar Arnfjörð Bjarmason wrote:

> >   however, while there is no link for the "worktree" command, there
> > does in fact exist a similarly-named web page
> > https://git-scm.com/docs/git-worktree.
> 
> It is an official site, of the git project. The project is legally
> managed by the SFC which owns the domain, Github happens to host it (for
> free) for us.

Actually, this isn't accurate anymore. The costs are covered by
donated services from a few companies. I've been meaning to write this
up (and thank them!), and will probably send something out around the
contributor summit.

> It's not fully auto-generated so stuff like git-worktree doesn't get
> added automatically, I just added a PR for that:
> https://github.com/git/git-scm.com/pull/1133

Thanks for doing that. I'm also open to auto-generating the index if we
can come up with well-organized output.

-Peff

Re: is http://git-scm.com an *official* git-affiliated site?

[Duy Nguyen]

On Wed, Feb 7, 2018 at 6:58 PM, Robert P. J. Day  wrote:
> On Wed, 7 Feb 2018, Duy Nguyen wrote:
>
>> On Wed, Feb 7, 2018 at 6:41 PM, Robert P. J. Day  
>> wrote:
>> > On Wed, 7 Feb 2018, Duy Nguyen wrote:
>> >
>> >> On Wed, Feb 7, 2018 at 5:54 PM, Robert P. J. Day  
>> >> wrote:
>> >> >
>> >> >   i ask WRT whether it should be up to date. i'm currently writing a
>> >> > number of git-related wiki pages, and i want to link to whatever are
>> >> > the canonical man pages for various git commands,
>> >>
>> >> I think this one is updated often by Junio (Git maintainer)
>> >>
>> >> https://www.kernel.org/pub/software/scm/git/docs/
>> >
>> >   whoops, just noticed that there is still no entry for "git subtree"
>> > there; is there something about that git command that makes it so hard
>> > to track down? :-)
>>
>> git-subtree is not an official command, so it's not there.
>
>   i'm going to push back a bit on that since, at the bottom of the man
> page for git-subtree, it states:
>
>   GIT
>Part of the git(1) suite
>
> so either it's part of the git suite, or it isn't.

It's a template that people use when write document for new commands.
That part should be deleted, I guess, if it's that confusing.
-- 
Duy

Re: is http://git-scm.com an *official* git-affiliated site?

[Robert P. J. Day]

On Wed, 7 Feb 2018, Duy Nguyen wrote:

> On Wed, Feb 7, 2018 at 6:41 PM, Robert P. J. Day  
> wrote:
> > On Wed, 7 Feb 2018, Duy Nguyen wrote:
> >
> >> On Wed, Feb 7, 2018 at 5:54 PM, Robert P. J. Day  
> >> wrote:
> >> >
> >> >   i ask WRT whether it should be up to date. i'm currently writing a
> >> > number of git-related wiki pages, and i want to link to whatever are
> >> > the canonical man pages for various git commands,
> >>
> >> I think this one is updated often by Junio (Git maintainer)
> >>
> >> https://www.kernel.org/pub/software/scm/git/docs/
> >
> >   whoops, just noticed that there is still no entry for "git subtree"
> > there; is there something about that git command that makes it so hard
> > to track down? :-)
>
> git-subtree is not an official command, so it's not there.

  i'm going to push back a bit on that since, at the bottom of the man
page for git-subtree, it states:

  GIT
   Part of the git(1) suite

so either it's part of the git suite, or it isn't.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: is http://git-scm.com an *official* git-affiliated site?

[Duy Nguyen]

On Wed, Feb 7, 2018 at 6:41 PM, Robert P. J. Day  wrote:
> On Wed, 7 Feb 2018, Duy Nguyen wrote:
>
>> On Wed, Feb 7, 2018 at 5:54 PM, Robert P. J. Day  
>> wrote:
>> >
>> >   i ask WRT whether it should be up to date. i'm currently writing a
>> > number of git-related wiki pages, and i want to link to whatever are
>> > the canonical man pages for various git commands,
>>
>> I think this one is updated often by Junio (Git maintainer)
>>
>> https://www.kernel.org/pub/software/scm/git/docs/
>
>   whoops, just noticed that there is still no entry for "git subtree"
> there; is there something about that git command that makes it so hard
> to track down? :-)

git-subtree is not an official command, so it's not there.
-- 
Duy

Re: [RFC PATCH 000/194] Moving global state into the repository object

[Duy Nguyen]

On Tue, Feb 6, 2018 at 6:51 AM, Stefan Beller  wrote:
> This series moves a lot of global state into the repository struct.
> It applies on top of 2512f15446149235156528dafbe75930c712b29e (2.16.0)
> It can be found at https://github.com/stefanbeller/git/tree/object-store
>
> Motivation for this series:
> * Easier to reason about code when all state is stored in one place,
>   for example for multithreading
> * Easier to move to processing submodules in-process instead of
>   calling a processes for the submodule handling.
>   The last patch demonstrates this.

I have a mixed feeling about this. The end game is to keep
"the_repository" references as minimum as possible, right? Like we
only need to mention in in cmd_xxx() and not all over the place. If
so, yay!!

Something else.. maybe "struct repository *" should not be a universal
function argument to avoid global states. Like sha1_file.c is mostly about the
object store, and I see that you added object store struct (nice!).
These sha1 related function should take the object_store* (which
probably is a combination of both packed and loose stores since they
depend on each other), not repository*. This way if a function needs
both access to object store and ref store, we can see the two
dependencies from function arguments.

The alternate object store, if modeled right, could share the same
object store interface. But I don't think we should do anything that
big right now, just put alternate object store inside object_store.

Similarly those functions in blob.c, commit.c, tree.c... should take
object_parser* as argument instead of repository*. Maybe there's a
better name for this than object_parser. parsed_object_store I guess.
Or maybe just object_pool.
-- 
Duy

Re: is http://git-scm.com an *official* git-affiliated site?

[Robert P. J. Day]

On Wed, 7 Feb 2018, Duy Nguyen wrote:

> On Wed, Feb 7, 2018 at 5:54 PM, Robert P. J. Day  
> wrote:
> >
> >   i ask WRT whether it should be up to date. i'm currently writing a
> > number of git-related wiki pages, and i want to link to whatever are
> > the canonical man pages for various git commands,
>
> I think this one is updated often by Junio (Git maintainer)
>
> https://www.kernel.org/pub/software/scm/git/docs/

  whoops, just noticed that there is still no entry for "git subtree"
there; is there something about that git command that makes it so hard
to track down? :-)

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: is http://git-scm.com an *official* git-affiliated site?

[Ævar Arnfjörð Bjarmason]

On Wed, Feb 07 2018, Robert P. J. Day jotted:

>   i ask WRT whether it should be up to date. i'm currently writing a
> number of git-related wiki pages, and i want to link to whatever are
> the canonical man pages for various git commands, but that site seems
> a bit off.
>
>   if one follows a link to get here:
>
> https://git-scm.com/doc
>
> there is another link to the "Reference Manual" that promises "The
> official and comprehensive man pages that are included in the Git
> package itself."
>
>   but upon getting there:
>
> https://git-scm.com/docs
>
> it seems clear that that page doesn't come close to covering all of
> the available git commands.
>
>   as an example, there is a link "submodule" on that page, which does
> indeed take one to the page https://git-scm.com/docs/git-submodule. so
> far, so good.
>
>   however, while there is no link for the "worktree" command, there
> does in fact exist a similarly-named web page
> https://git-scm.com/docs/git-worktree.

It is an official site, of the git project. The project is legally
managed by the SFC which owns the domain, Github happens to host it (for
free) for us.

It's not fully auto-generated so stuff like git-worktree doesn't get
added automatically, I just added a PR for that:
https://github.com/git/git-scm.com/pull/1133

>   finally, there is no reference to the git "subtree" command, either
> as a link *or* as an existing web page
> https://git-scm.com/docs/git-subtree. it all seems kind of arbitrary.

Unlike git-worktree, git-subtree is not shipped as a "proper" git
command, it lives in contrib/. What the status of that is I'm not sure,
but that's why it's not on git-scm or kernel.org in any form.

>   is there an actual, canonical set of online web pages for all
> current git commands one should use?

You can use git-scm.com or the kernel.org link Duy noted.

The thing that's the most official is the man pages we ship with any
given release, and as seen above the online presence may lag behind in
some ways, but that can always be fixed.

Even though we may have some official online sources, it's better to
direct users to the documentation that ships with git on their system,
since it'll reflect the version they're using.

Re: is http://git-scm.com an *official* git-affiliated site?

[Robert P. J. Day]

On Wed, 7 Feb 2018, Duy Nguyen wrote:

> On Wed, Feb 7, 2018 at 5:54 PM, Robert P. J. Day  
> wrote:
> >
> >   i ask WRT whether it should be up to date. i'm currently writing a
> > number of git-related wiki pages, and i want to link to whatever are
> > the canonical man pages for various git commands,
>
> I think this one is updated often by Junio (Git maintainer)
>
> https://www.kernel.org/pub/software/scm/git/docs/

  ah, better, thanks.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Compliment of the day to you. Dear Friend

[Mrs.Chantal Sonian Kadi]

 Compliment of the day to you. Dear Friend. for security reason
contact me Through this email (mrschantal.sonian.k...@gmail.com)

Dear Friend.

I am Mrs.Chantal Sonian Kadi. am sending this brief letter to solicit 
your
partnership to transfer $10.5 million US Dollars. I shall send you
more information and procedures when I receive positive response from
you.  please send me a message in my  Email box (mrschantal.sonian.
k...@gmail.com)


Coupy This Email Id (mrschantal.sonian.k...@gmail.com)To Reply Me with 
this
Id(mrschantal.sonian.k...@gmail.com)

Mrs.Chantal Sonian Kadi

Re: is http://git-scm.com an *official* git-affiliated site?

[Duy Nguyen]

On Wed, Feb 7, 2018 at 5:54 PM, Robert P. J. Day  wrote:
>
>   i ask WRT whether it should be up to date. i'm currently writing a
> number of git-related wiki pages, and i want to link to whatever are
> the canonical man pages for various git commands,

I think this one is updated often by Junio (Git maintainer)

https://www.kernel.org/pub/software/scm/git/docs/

> but that site seems a bit off.
-- 
Duy

Re: BUG: fetch in certain repo always gives "did not send all necessary objects"

[Duy Nguyen]

On Wed, Feb 7, 2018 at 7:00 AM, Elijah Newren  wrote:
> and knew they had been using it, then I might have guessed that "HEAD"
> meant "not your actual HEAD but the HEAD of the vestige of some other
> worktree").
>
> Does anyone have pointers about what might be doable in terms of
> providing a more useful error message to allow users to recover?

I noticed this too. I was working on improving this message a bit but
got side tracked and since I figured this did not happen often
anymore, this fix got lower priority than others. I'll resume that
work.

> And/or ideas of what steps could cause corruption so I can send out a
> PSA to help users avoid it?

There is another thing we could do. One bad HEAD should not abort the
entire operation (at least if it's not the current worktree's HEAD).
We could still give a warning and move on, or don't warn at all and
let "git worktree prune" collect it (which I see from your message
that it also fails to do).

I guess that's two more items on my todo list :) Sorry for all the
trouble because of this bug of mine.

> If not, I'll try to dig more, but I thought I'd ask others familiar
> with this area.
-- 
Duy

Re: [PATCH v1] dir.c: don't flag the index as dirty for changes to the untracked cache

[Duy Nguyen]

On Tue, Feb 6, 2018 at 7:55 PM, Duy Nguyen  wrote:
> On Tue, Feb 6, 2018 at 7:27 PM, Duy Nguyen  wrote:
>> On Tue, Feb 6, 2018 at 8:48 AM, Ben Peart  wrote:
>>> With the new behavior, making a change in dir1/, then calling status would
>>> update the dir1/ untracked cache entry but not write it out. On the next
>>> status, git would detect the change in dir1/ again and update the untracked
>>
>> Thing only missing piece here I would add is, this dir1/ detection is
>> done by watchman. We have to contact watchman and ask the set of
>> changed paths since $TIME where $TIME is the last time we updated
>> untracked cache and invalidate those paths in core. Then it should
>> work correctly. I checked the watchman query in the fsmonitor hook and
>> I think it's correct so far.
>
> No I think I'm wrong. And worse, I think the interaction between FSNM
> and UNTR extension is broken. This is partly from me guessing how
> fsmonitor works so I may be double-wrong.
>
> UNTR extension is supposed to cover the untracked state at timestamp
> $X. Where $X is stored in FSNM extension. Correct?
>
> When fsmonitor is used and read_directory() is executed (at timestamp
> $Y), we ask watchman "what's new on worktree since $X?"). We get the
> list, we invalidate relevant directories so we can see new untracked
> entries (or lack of old untracked entries). We write FSNM with
> timestamp $Y down. We may or may not write UNTR down because of this
> patch, but let's suppose we do. All is good. UNTR now records the
> state at $Y. FSNM says $Y. This is how it works (when there's no bugs)
>
> UNTR extension is only updated when read_directory() is called. It
> does not always happen. FSNM is updated all the time (almost at every

I was indeed doubly wrong. When FSMN is read, it does make calls to
invalidate stuff in untracked cache data structure. If the index is
written down (with updated FSMN extension and timestamp) then the UNTR
extension, which is generated from in-core untracked data structure,
also reflects the damages by the changed paths so next time even if
those changed paths are not reported again (between $Y and $Z below),
it's fine.

All is good in the world again :)

> git command since most of them needs to read index, many write it
> down) with new timestamp. Suppose FSNM now records timestamp $Z, UNTR
> still records the state at $Y because in the last index update,
> read_directory() is not executed. 4 files have been added between $Y
> and $Z. When we ask watchman "what's new since $Z?" we will not see
> those files, so we don't invalidate relevant directories and
> read_directory() will not see those files.
-- 
Duy

is http://git-scm.com an *official* git-affiliated site?

[Robert P. J. Day]

  i ask WRT whether it should be up to date. i'm currently writing a
number of git-related wiki pages, and i want to link to whatever are
the canonical man pages for various git commands, but that site seems
a bit off.

  if one follows a link to get here:

https://git-scm.com/doc

there is another link to the "Reference Manual" that promises "The
official and comprehensive man pages that are included in the Git
package itself."

  but upon getting there:

https://git-scm.com/docs

it seems clear that that page doesn't come close to covering all of
the available git commands.

  as an example, there is a link "submodule" on that page, which does
indeed take one to the page https://git-scm.com/docs/git-submodule. so
far, so good.

  however, while there is no link for the "worktree" command, there
does in fact exist a similarly-named web page
https://git-scm.com/docs/git-worktree.

  finally, there is no reference to the git "subtree" command, either
as a link *or* as an existing web page
https://git-scm.com/docs/git-subtree. it all seems kind of arbitrary.

  is there an actual, canonical set of online web pages for all
current git commands one should use?

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)

[Duy Nguyen]

On Wed, Feb 7, 2018 at 4:52 PM, Eric Sunshine  wrote:
> On Tue, Feb 6, 2018 at 5:20 AM, Duy Nguyen  wrote:
>> On Tue, Feb 6, 2018 at 1:56 AM, Junio C Hamano  wrote:
>>> Duy Nguyen  writes:
>>> I actually think compact-summary was a good way to phrase it.
>>>
>>> Personally, I think it was a UI mistake that --summary can be given
>>> independently with or without --stat (instead, there shouldn't have
>>> been the --summary option, and instead when it was added, --stat
>>> just should have gained an extra kind of output).  A single option
>>> that can give both kinds of info may be a good way forward, so
>>> another possibility may be --summary-in-stat (meaning: the info
>>> given by summary is included in stat output).  I dunno.
>>
>> +Eric maybe he has some idea (sorry I forgot to include people from
>> the last round).
>
> What about the earlier suggestion[1] (and minor follow-ups[2,3]) of
> making this another option to --stat= (for instance, --stat=compact)?
> Did that idea get shot down or am I misunderstanding the question
> here.

I thought that was something like
--stat[=[,[,,[compact and turning on
"compact" alone would get awkward because you need to specify all
those widths and counts too. --stat=compact as a separate form, with
no combination with any other stat params, does not feel justified. We
could just do --stat-compact then. Perhaps we can allow compact to
appear anywhere in --stat=, and not just the end? The --stat= syntax
would be

--stat=[[,[,...]]]

where option could be keyword ones like compact or anything else in
future, or = form.  could also be a number,
but in that case the three consecutive number options will present
width, name-width and count in this order.

Or we could simply add new --stat= syntax _without_ " as
numbers". widths and counts must be specified keywords as well, e.g.
--stat=width=40,name-width=20,count=10,compact and leave the old
syntax "--stat=,," alone.

Then we still need to decide the new keyword for this feature, I feel
compact is a bit too vague (I read --stat=compact as "it's compact
stat", not "stat with compact summary"), so perhaps
--stat=compact-summary, or just --stat=summary?

> [1]: 
> https://public-inbox.org/git/capig+cqlgs59jyxcmk30qy307arwqjx6pnoo95z39_jj9+d...@mail.gmail.com/
> [2]: 
> https://public-inbox.org/git/cacsjy8b5qrn8t1aai3y3nfec5baqn2xkk6vzozmp5lh-mpz...@mail.gmail.com/
> [3]: 
> https://public-inbox.org/git/CACsJy8CPHk+aXHr-mkHZi27s=c3+ny8d9csuzoso8pwevib...@mail.gmail.com/
-- 
Duy

Re: [PATCHv3] tag: add --edit option

[Eric Sunshine]

On Tue, Feb 6, 2018 at 3:36 AM, Nicolas Morey-Chaisemartin
 wrote:
> Add a --edit option whichs allows modifying the messages provided by -m or -F,
> the same way git commit --edit does.
>
> Signed-off-by: Nicolas Morey-Chaisemartin 
> ---
> Changes since v2 ( 
> https://public-inbox.org/git/e99947cf-93ba-9376-f059-7f6a369d3...@suse.com ):
>  * Add [-e] to git tag summary

Thanks, I think this addresses all my comments from previous rounds.
Just a couple minor style issues below...

> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> @@ -452,6 +452,21 @@ test_expect_success \
> +> +test_expect_success \
> +   'creating an annotated tag with -m message --edit should succeed' '
> +   EDITOR=./fakeeditor git tag -m "A message" --edit 
> annotated-tag-edit &&

Whitespace between 'fakeeditor' and 'git' is a tab but should be a space.

> +   get_tag_msg annotated-tag-edit >actual &&
> +   test_cmp expect actual
> +'
> @@ -465,6 +480,21 @@ test_expect_success \
> +test_expect_success \
> +   'creating an annotated tag with -F messagefile --edit should succeed' 
> '
> +   EDITOR=./fakeeditor git tag -F msgfile --edit 
> file-annotated-tag-edit &&

Ditto.

> +   get_tag_msg file-annotated-tag-edit >actual &&
> +   test_cmp expect actual
> +'

Re: [RFC PATCH 000/194] Moving global state into the repository object

[Eric Sunshine]

On Tue, Feb 6, 2018 at 3:25 PM, Stefan Beller  wrote:
>> Any suggestions welcome!
>
> Eric repeatedly points out leaking memory.
>
> As of today we do not care about memory leaking as it is cleaned
> up at the end of the program anyway, for example the objects
> hash table is never cleared.

Is this still true/desirable when multiple 'repos' are involved?

> In a resend I will put the infrastructure in place to free the memory via
> adding
>
>   raw_object_store_clear(struct raw_object_store *)
>   object_parser_clear(struct object_parser *)
>   ref_store_clear(struct ref_store *)
>
> and calling these functions on repo destruction. The functions
> itself will be empty code-wise and contain TODO comments listing
> all variables that need care.

I'm confused. If you go to the effort of inserting TODO's why not go
all the way and instead insert the actual code?

> Follow up patches can figure out what is best to do, such as closing
> the memleaks. As repo_clear is not called for the_repository
> we'd even keep the property of relying on fast cleanup by the OS.

Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)

[Eric Sunshine]

On Tue, Feb 6, 2018 at 5:20 AM, Duy Nguyen  wrote:
> On Tue, Feb 6, 2018 at 1:56 AM, Junio C Hamano  wrote:
>> Duy Nguyen  writes:
>> I actually think compact-summary was a good way to phrase it.
>>
>> Personally, I think it was a UI mistake that --summary can be given
>> independently with or without --stat (instead, there shouldn't have
>> been the --summary option, and instead when it was added, --stat
>> just should have gained an extra kind of output).  A single option
>> that can give both kinds of info may be a good way forward, so
>> another possibility may be --summary-in-stat (meaning: the info
>> given by summary is included in stat output).  I dunno.
>
> +Eric maybe he has some idea (sorry I forgot to include people from
> the last round).

What about the earlier suggestion[1] (and minor follow-ups[2,3]) of
making this another option to --stat= (for instance, --stat=compact)?
Did that idea get shot down or am I misunderstanding the question
here.

[1]: 
https://public-inbox.org/git/capig+cqlgs59jyxcmk30qy307arwqjx6pnoo95z39_jj9+d...@mail.gmail.com/
[2]: 
https://public-inbox.org/git/cacsjy8b5qrn8t1aai3y3nfec5baqn2xkk6vzozmp5lh-mpz...@mail.gmail.com/
[3]: 
https://public-inbox.org/git/CACsJy8CPHk+aXHr-mkHZi27s=c3+ny8d9csuzoso8pwevib...@mail.gmail.com/

Re: [PATCH 5/8] rebase: introduce the --recreate-merges option

[Sergey Organov]

Jacob Keller  writes:

> On Tue, Feb 6, 2018 at 10:16 PM, Sergey Organov  wrote:
>> Johannes Schindelin  writes:
>>
>> [...]
>>
>>> +--recreate-merges::
>>> + Recreate merge commits instead of flattening the history by replaying
>>> + merges. Merge conflict resolutions or manual amendments to merge
>>> + commits are not preserved.
>>
>> I wonder why you guys still hold on replaying "merge-the-operation"
>> instead of replaying "merge-the-result"? The latter, the merge commit
>> itself, no matter how exactly it was created in the first place, is the
>> most valuable thing git keeps about the merge, and you silently drop it
>> entirely! OTOH, git keeps almost no information about
>> "merge-the-operation", so it's virtually impossible to reliably replay
>> the operation automatically, and yet you try to.
>>
>
> I'm not sure I follow what you mean here?
>
> You mean that you'd want this to actually attempt to re-create the
> original merge including conflict resolutions by taking the contents
> of the result?

I mean just cherry-pick the merge the same way all other commits are
essentially cherry-picked during rebase. That's what Johannes Sixt did
in his patch I was reffering to.

> How do you handle if that result has conflicts? What UX do you present
> to the user to handle such conflicts? I don't think the normal 3-way
> conflicts would even be possible in this case?

No problem here. It goes exactly the same way as for non-merge commits
that are being rebased. You can try it right now using

$ git cherry-pick -m1 

that will induce conflicts.

The (somewhat tricky) functional difference is only in recording correct
additional parents to the final commit, but that part is hidden from the
user.

-- Sergey

[PATCH v2] dir.c: ignore paths containing .git when invalidating untracked cache

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

read_directory() code ignores all paths named ".git" even if it's not
a valid git repository. See treat_path() for details. Since ".git" is
basically invisible to read_directory(), when we are asked to
invalidate a path that contains ".git", we can safely ignore it
because the slow path would not consider it anyway.

This helps when fsmonitor is used and we have a real ".git" repo at
worktree top. Occasionally .git/index will be updated and if the
fsmonitor hook does not filter it, untracked cache is asked to
invalidate the path ".git/index".

Without this patch, we invalidate the root directory unncessarily,
which:

- makes read_directory() fall back to slow path for root directory
  (slower)

- makes the index dirty (because UNTR extension is updated). Depending
  on the index size, writing it down could also be slow.

A note about the new "safe_path" knob. Since this new check could be
relatively expensive, avoid it when we know it's not needed. If the
path comes from the index, it can't contain ".git". If it does
contain, we may be screwed up at many more levels, not just this one.

Noticed-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 My v1 was rubbish. It's no wonder Ben didn't see my intention. v2
 corrects the "is .git in a given path?" logic and adds a test to
 verify it.

 dir.c   | 10 ++
 dir.h   |  2 +-
 fsmonitor.c |  2 +-
 fsmonitor.h |  2 +-
 t/t7519-status-fsmonitor.sh | 39 +
 unpack-trees.c  |  2 +-
 6 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/dir.c b/dir.c
index 7c4b45e30e..fce45fc55e 100644
--- a/dir.c
+++ b/dir.c
@@ -1773,7 +1773,7 @@ static enum path_treatment treat_path(struct dir_struct 
*dir,
if (!de)
return treat_path_fast(dir, untracked, cdir, istate, path,
   baselen, pathspec);
-   if (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, ".git"))
+   if (is_dot_or_dotdot(de->d_name) || !fspathcmp(de->d_name, ".git"))
return path_none;
strbuf_setlen(path, baselen);
strbuf_addstr(path, de->d_name);
@@ -2968,10 +2968,12 @@ static int invalidate_one_component(struct 
untracked_cache *uc,
 }
 
 void untracked_cache_invalidate_path(struct index_state *istate,
-const char *path)
+const char *path, int safe_path)
 {
if (!istate->untracked || !istate->untracked->root)
return;
+   if (!safe_path && !verify_path(path))
+   return;
invalidate_one_component(istate->untracked, istate->untracked->root,
 path, strlen(path));
 }
@@ -2979,13 +2981,13 @@ void untracked_cache_invalidate_path(struct index_state 
*istate,
 void untracked_cache_remove_from_index(struct index_state *istate,
   const char *path)
 {
-   untracked_cache_invalidate_path(istate, path);
+   untracked_cache_invalidate_path(istate, path, 1);
 }
 
 void untracked_cache_add_to_index(struct index_state *istate,
  const char *path)
 {
-   untracked_cache_invalidate_path(istate, path);
+   untracked_cache_invalidate_path(istate, path, 1);
 }
 
 /* Update gitfile and core.worktree setting to connect work tree and git dir */
diff --git a/dir.h b/dir.h
index 11a047ba48..06df057054 100644
--- a/dir.h
+++ b/dir.h
@@ -350,7 +350,7 @@ static inline int dir_path_match(const struct dir_entry 
*ent,
 int cmp_dir_entry(const void *p1, const void *p2);
 int check_dir_entry_contains(const struct dir_entry *out, const struct 
dir_entry *in);
 
-void untracked_cache_invalidate_path(struct index_state *, const char *);
+void untracked_cache_invalidate_path(struct index_state *, const char *, int 
safe_path);
 void untracked_cache_remove_from_index(struct index_state *, const char *);
 void untracked_cache_add_to_index(struct index_state *, const char *);
 
diff --git a/fsmonitor.c b/fsmonitor.c
index 0af7c4edba..6d7bcd5d0e 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -130,7 +130,7 @@ static void fsmonitor_refresh_callback(struct index_state 
*istate, const char *n
 * as it could be a new untracked file.
 */
trace_printf_key(_fsmonitor, "fsmonitor_refresh_callback '%s'", 
name);
-   untracked_cache_invalidate_path(istate, name);
+   untracked_cache_invalidate_path(istate, name, 0);
 }
 
 void refresh_fsmonitor(struct index_state *istate)
diff --git a/fsmonitor.h b/fsmonitor.h
index cd3cc0ccf2..65f3743636 100644
--- a/fsmonitor.h
+++ b/fsmonitor.h
@@ -65,7 +65,7 @@ static inline void mark_fsmonitor_invalid(struct index_state 
*istate, struct cac
 {
if (core_fsmonitor) {
ce->ce_flags &= ~CE_FSMONITOR_VALID;
-   

[PATCH v2] dir.c: ignore paths containing .git when invalidating untracked cache

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

read_directory() code ignores all paths named ".git" even if it's not
a valid git repository. See treat_path() for details. Since ".git" is
basically invisible to read_directory(), when we are asked to
invalidate a path that contains ".git", we can safely ignore it
because the slow path would not consider it anyway.

This helps when fsmonitor is used and we have a real ".git" repo at
worktree top. Occasionally .git/index will be updated and if the
fsmonitor hook does not filter it, untracked cache is asked to
invalidate the path ".git/index".

Without this patch, we invalidate the root directory unncessarily,
which:

- makes read_directory() fall back to slow path for root directory
  (slower)

- makes the index dirty (because UNTR extension is updated). Depending
  on the index size, writing it down could also be slow.

A note about the new "safe_path" knob. Since this new check could be
relatively expensive, avoid it when we know it's not needed. If the
path comes from the index, it can't contain ".git". If it does
contain, we may be screwed up at many more levels, not just this one.

Noticed-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 My v1 was rubbish. It's no wonder Ben didn't see my intention. v2
 corrects the "is .git in a given path?" logic and adds a test to
 verify it.

 dir.c   | 10 ++
 dir.h   |  2 +-
 fsmonitor.c |  2 +-
 fsmonitor.h |  2 +-
 t/t7519-status-fsmonitor.sh | 39 +
 unpack-trees.c  |  2 +-
 6 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/dir.c b/dir.c
index 7c4b45e30e..fce45fc55e 100644
--- a/dir.c
+++ b/dir.c
@@ -1773,7 +1773,7 @@ static enum path_treatment treat_path(struct dir_struct 
*dir,
if (!de)
return treat_path_fast(dir, untracked, cdir, istate, path,
   baselen, pathspec);
-   if (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, ".git"))
+   if (is_dot_or_dotdot(de->d_name) || !fspathcmp(de->d_name, ".git"))
return path_none;
strbuf_setlen(path, baselen);
strbuf_addstr(path, de->d_name);
@@ -2968,10 +2968,12 @@ static int invalidate_one_component(struct 
untracked_cache *uc,
 }
 
 void untracked_cache_invalidate_path(struct index_state *istate,
-const char *path)
+const char *path, int safe_path)
 {
if (!istate->untracked || !istate->untracked->root)
return;
+   if (!safe_path && !verify_path(path))
+   return;
invalidate_one_component(istate->untracked, istate->untracked->root,
 path, strlen(path));
 }
@@ -2979,13 +2981,13 @@ void untracked_cache_invalidate_path(struct index_state 
*istate,
 void untracked_cache_remove_from_index(struct index_state *istate,
   const char *path)
 {
-   untracked_cache_invalidate_path(istate, path);
+   untracked_cache_invalidate_path(istate, path, 1);
 }
 
 void untracked_cache_add_to_index(struct index_state *istate,
  const char *path)
 {
-   untracked_cache_invalidate_path(istate, path);
+   untracked_cache_invalidate_path(istate, path, 1);
 }
 
 /* Update gitfile and core.worktree setting to connect work tree and git dir */
diff --git a/dir.h b/dir.h
index 11a047ba48..06df057054 100644
--- a/dir.h
+++ b/dir.h
@@ -350,7 +350,7 @@ static inline int dir_path_match(const struct dir_entry 
*ent,
 int cmp_dir_entry(const void *p1, const void *p2);
 int check_dir_entry_contains(const struct dir_entry *out, const struct 
dir_entry *in);
 
-void untracked_cache_invalidate_path(struct index_state *, const char *);
+void untracked_cache_invalidate_path(struct index_state *, const char *, int 
safe_path);
 void untracked_cache_remove_from_index(struct index_state *, const char *);
 void untracked_cache_add_to_index(struct index_state *, const char *);
 
diff --git a/fsmonitor.c b/fsmonitor.c
index 0af7c4edba..6d7bcd5d0e 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -130,7 +130,7 @@ static void fsmonitor_refresh_callback(struct index_state 
*istate, const char *n
 * as it could be a new untracked file.
 */
trace_printf_key(_fsmonitor, "fsmonitor_refresh_callback '%s'", 
name);
-   untracked_cache_invalidate_path(istate, name);
+   untracked_cache_invalidate_path(istate, name, 0);
 }
 
 void refresh_fsmonitor(struct index_state *istate)
diff --git a/fsmonitor.h b/fsmonitor.h
index cd3cc0ccf2..65f3743636 100644
--- a/fsmonitor.h
+++ b/fsmonitor.h
@@ -65,7 +65,7 @@ static inline void mark_fsmonitor_invalid(struct index_state 
*istate, struct cac
 {
if (core_fsmonitor) {
ce->ce_flags &= ~CE_FSMONITOR_VALID;
-   

Windows: mintty.exe classified as exploit by AV software

[Ehrt, Michael]

Hi everyone,

a few days ago I installed version 2.16.1.2, downloaded from 
https://git-scm.com/download/win on my Windows 7 system. The OS is Windows 7 
Enterprise 64bit, Build 7601/SP1, in case it matters. This is a first time 
install, not an upgrade.

Our current virus protection software is Cylance, from 
https://www.cylance.com/en_us/home.html

During install, several executions of 
C:\Program Files\Git\usr\bin\bash.exe
were blocked, the violation being given as "Stack Pivot". Our admins then 
temporarily lifted some rules for my device so that I could properly install it.

But now, when I start ...
"C:\Program Files\Git\git-bash.exe" --cd-to-home
... Cylance classifies it as an Exploit, and blocks execution with the 
following messages:
Category: Exploit
Event: Blocked
Details: Violation: StackProtect; Application: C:\Program 
Files\Git\usr\bin\mintty.exe
(Screenshot available if needed)

If I start ...
C:\Program Files\Git\usr\bin\mintty.exe
directly, and choose the 64 bit version from the dialog, it is allowes to start 
without getting blocked.

My current problem is that the security guys don't want to see this software 
installed on my machine because of this.
And as Cylance is not a pattern-based AV, it's not something that will go away 
by waiting for the next daily update ...

Any ideas about this?

Thanks

Michael

Re: [PATCH v2 1/3] worktree: improve message when creating a new worktree

[Eric Sunshine]

On Sun, Feb 4, 2018 at 9:12 PM, Duy Nguyen  wrote:
> As a former translator, I'm not thrilled to see a sentence broken into
> two pieces like this. I'm not a Japanese translator, but I think this
> sentence is translated differently when the translator sees the whole
> line "Preparing ..., setting ...".
>
> I think the purpose of "Preparing..." in the first place is to show
> something when git is busy checkout out the worktree. As long as we
> print it before git-reset, we should be good.

The original message was "Enter " which had the potential to
confuse someone into thinking the working directory had changed[1], so
it was changed to "Preparing...". The reason for keeping that message
(rather than dropping it outright) was to provide context to messages
printed after it, especially messages such as "HEAD is now at..."
which might otherwise confuse the reader into thinking that HEAD in
the current worktree changed rather than HEAD in the new
worktree[2,3].

[1]: https://public-inbox.org/git/55a8f4b1.9060...@drmicha.warpmail.net/
[2]: 
https://public-inbox.org/git/capig+crshwmmf9ccubrrdccw4kvg9peouxp5vqpsgfxzmxh...@mail.gmail.com/
[3]: 
https://public-inbox.org/git/capig+csls4-ukicvmbsknero_fyd722hs1_u6qztrim8cio...@mail.gmail.com/

Re: [PATCH] t0050: remove the unused $test_case variable

[Ævar Arnfjörð Bjarmason]

On Wed, Feb 07 2018, Johannes Sixt jotted:

> Am 07.02.2018 um 00:13 schrieb Ævar Arnfjörð Bjarmason:
>> The $test_case variable hasn't been used since
>> decd3c0c28 ("t0050-*.sh: mark the rename (case change) test as
>> passing", 2014-11-28) when its last user went away.
>>
>> Let's remove the "say" as well, since it's obvious from subsequent
>> output that we're testing on a case sensitive filesystem.
>
> Am I misunderstanding the message? I think it reports properties of
> the test environment. And the tests do run on case-insensitive
> filesystems. IMO, the message should be kept.

It's obvious from subsequent output whether the FS is case sensitive or
not, so I thought it was redundant to keep this report at the top since
we didn't have the variable setting anymore.