Re: [PATCH] t: add clone test for files differing only in case

[Eric Sunshine]

On Sun, Jan 21, 2018 at 2:33 AM, Junio C Hamano  wrote:
> "brian m. carlson"  writes:
>> +test_expect_success 'clone on case-insensitive fs' '
>> + o=$(git hash-object -w --stdin > + t=$(printf "100644 X\0${o}100644 x\0${o}" |
>> + git hash-object -w -t tree --stdin) &&
>> + c=$(git commit-tree -m bogus $t) &&
>> + git update-ref refs/heads/bogus $c &&
>> + git clone -b bogus . bogus
>> +'
>> +
>>  test_done
>
> Hmm, I seem to be seeing a failure from this thing:
>
> expecting success:
> o=$(git hash-object -w --stdin  t=$(printf "100644 X\0${o}100644 x\0${o}" |
> git hash-object -w -t tree --stdin) &&
> c=$(git commit-tree -m bogus $t) &&
> git update-ref refs/heads/bogus $c &&
> git clone -b bogus . bogus
>
> fatal: repository '.' does not exist
>
> even on a case sensitive platform.

Yep. In pretty much any other test script, this would work (it was
developed in a stand-alone script), but t5601 (which nukes .git as its
first action) isn't the most friendly place.

Re: [PATCH] t: add clone test for files differing only in case

[Junio C Hamano]

"brian m. carlson"  writes:

> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index 0f895478f0..53b2dda9d2 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -611,4 +611,17 @@ test_expect_success 'GIT_TRACE_PACKFILE produces a 
> usable pack' '
>   git -C replay.git index-pack -v --stdin   '
>  
> +hex2oct() {
> + perl -ne 'printf "\\%03o", hex for /../g'
> +}
> +
> +test_expect_success 'clone on case-insensitive fs' '
> + o=$(git hash-object -w --stdin  + t=$(printf "100644 X\0${o}100644 x\0${o}" |
> + git hash-object -w -t tree --stdin) &&
> + c=$(git commit-tree -m bogus $t) &&
> + git update-ref refs/heads/bogus $c &&
> + git clone -b bogus . bogus
> +'
> +
>  test_done

Hmm, I seem to be seeing a failure from this thing:

expecting success:
o=$(git hash-object -w --stdin 

Re: [PATCH v3] packed_ref_cache: don't use mmap() for small files

[Michael Haggerty]

On 01/17/2018 11:09 PM, Jeff King wrote:
> On Tue, Jan 16, 2018 at 08:38:15PM +0100, Kim Gybels wrote:
> 
>> Take a hint from commit ea68b0ce9f8 (hash-object: don't use mmap() for
>> small files, 2010-02-21) and use read() instead of mmap() for small
>> packed-refs files.
>>
>> This also fixes the problem[1] where xmmap() returns NULL for zero
>> length[2], for which munmap() later fails.
>>
>> Alternatively, we could simply check for NULL before munmap(), or
>> introduce xmunmap() that could be used together with xmmap(). However,
>> always setting snapshot->buf to a valid pointer, by relying on
>> xmalloc(0)'s fallback to 1-byte allocation, makes using snapshots
>> easier.
>>
>> [1] https://github.com/git-for-windows/git/issues/1410
>> [2] Logic introduced in commit 9130ac1e196 (Better error messages for
>> corrupt databases, 2007-01-11)
>>
>> Signed-off-by: Kim Gybels 
>> ---
>>
>> Change since v2: removed separate case for zero length as suggested by Peff,
>> ensuring that snapshot->buf is always a valid pointer.
> 
> Thanks, this looks fine to me (I'd be curious to hear from Michael if
> this eliminates the need for the other patches).

`snapshot->buf` can still be NULL if the `packed-refs` file didn't exist
(see the earlier code path in `load_contents()`). So either that code
path *also* has to get the `xmalloc()` treatment, or my third patch is
still necessary. (My second patch wouldn't be necessary because the
ENOENT case makes `load_contents()` return 0, triggering the early exit
from `create_snapshot()`.)

I don't have a strong preference either way.

Michael

Re: [PATCH v3] mru: Replace mru.[ch] with list.h implementation

[René Scharfe]

Am 20.01.2018 um 23:24 schrieb Gargi Sharma:
> On Sat, Jan 20, 2018 at 1:02 AM, Eric Wong  wrote:
>> Gargi Sharma  wrote:
>>> --- a/list.h
>>> +++ b/list.h
>>> @@ -93,6 +93,13 @@ static inline void list_move(struct list_head *elem, 
>>> struct list_head *head)
>>>list_add(elem, head);
>>>   }
>>>
>>> +/* Move to the front of the list. */
>>> +static inline void list_move_to_front(struct list_head *elem, struct 
>>> list_head *head)
>>> +{
>>> + list_del(elem);
>>> + list_add(elem, head);
>>> +}
>>> +
>>
>> Since we already have list_move and it does the same thing,
>> I don't think we need a new function, here.
>>
>> Hackers coming from other projects (glibc/urcu/Linux kernel)
>> might appreciate having fewer differences from what they're used
>> to.
> 
> I think the idea behind this function was that it is already being used in two
> places in the code and might be used in other places in the future. I agree
> with your stance, but a list_move_to_front is pretty self explanatory and not
> too different, so it should be alright.

Not sure I understand the point about the function being already used as
an argument for adding it, but if there is already one which has the
exact sane behavior (list_move() in this case) then that should be used
instead of adding a duplicate.

René

RE: [PATCH v2 4/6] Force test suite traps to be cleared for NonStop ksh

[Randall S. Becker]

On January 19, 2018 5:29 PM, I wrote:
> On January 19, 2018 4:27 PM, Jeff King wrote:
> > On Fri, Jan 19, 2018 at 12:34:04PM -0500, randall.s.bec...@rogers.com
> wrote:
> >
> > > From: "Randall S. Becker" 
> > >
> > > * t/lib-git-daemon.sh: fix incompatibilities with ksh traps not being
> > >   cleared automatically on platform. This caused tests to seem to fail
> > >   while actually succeeding.
> > >
> > > Signed-off-by: Randall S. Becker 
> > > ---
> > >  t/lib-git-daemon.sh | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index
> > > 987d40680..955beecd9 100644
> > > --- a/t/lib-git-daemon.sh
> > > +++ b/t/lib-git-daemon.sh
> > > @@ -68,6 +68,7 @@ start_git_daemon() {
> > >   test_skip_or_die $GIT_TEST_GIT_DAEMON \
> > >   "git daemon failed to start"
> > >   fi
> > > + trap '' EXIT
> > >  }
> >
> > I don't think this can be right. The way these traps are supposed to work 
> > is:
> >
> >   - as soon as test-lib.sh is loaded, we "trap die EXIT" to catch an
> > accidental death/exit from one of the scripts
> >
> >   - when test_done is called, we clear the trap (i.e., it is OK to exit
> > now because the script has given us a positive indication that all
> > tests have been run)
> >
> >   - while the child git-daemon is running, we'd want to clean up after
> > ourselves. So during start_git_daemon() we add our cleanup (followed
> > by the original "die", because shell traps don't push onto a stack).
> > And then at stop_git_daemon(), we restore the original die trap.
> >
> > But this patch means that while git-daemon is running, we have no trap at
> all!
> > That means that a failed test which causes us to exit would leave a
> > child daemon running.
> >
> > Furthermore, both of these functions now drop the 'die' trap entirely,
> > meaning the script would fail to notice premature exit from one of the
> > test snippets.
> >
> > So while this may be papering over a problem on NonStop, I think it's
> > breaking the intent of the traps.
> >
> > I'm not sure what the root of the problem is, but it sounds like ksh
> > may be triggering EXIT traps when we don't expect (during function
> returns?
> > Subshell exits? Other?)
> 
> The "unexpected" EXIT traps are exactly the issue we found when working
> with the platform support team. There was some discussion about what the
> right expectation was, and I was not able to have a change made to ksh on
> the platform. The problem may have a similar (identical?) root cause to the
> Perl exit issues we are also experiencing that is in their hands. I'm 
> currently
> retesting without this change (results in 36 hours ☹ ). Is there a preferred
> way you would like me to bypass this except on NonStop? I can add a check
> based on uname.

After running through the git test suite, it turns out that this particular 
need has gone away as of the latest OS releases. The test results without the 
trap '' EXIT are identical to that with the trap (6 breaks that look completion 
code handling-related on the platform). I'm going to drop the need for this and 
repackage the entire set of patches applying everyone's comments and removing 
this (4/6) and the GCC attribute (1/6) patch. This will be followed-up with 
generalizing the setbuf and tar patches for a broader audience, but I need a 
bit more time for that generalization.

Please accept my thanks and expect the updated set tomorrow, which will be 
sufficient to bring the NonStop NSE, NSX, and NSV platforms into the common 
code-base for git at long last.

Cheers,
Randall

git merge-tree: bug report and some feature requests

[Josh Bleecher Snyder]

Hi, all.

I'm experimenting with some new porcelain for interactive rebase. One
goal is to leave the work tree untouched for most operations. It looks
to me like 'git merge-tree' may be the right plumbing command for
doing the merge part of the pick work of the todo list, one commit at
a time. If I'm wrong about this, I'd love pointers; what follows may
still be interesting anyway.

I've encountered some bumps with 'git merge-tree'. A bug report and
some feature requests follow. Apologies for the long email.


1. Bug

When a binary file containing NUL is added on only one side, the
resulting patch is malformed. Reproduction script:

mkdir test
cd test
git init .
touch shared
git add shared
git commit -m "base"
git checkout -b left
echo "left" > x
printf '\1\0\1\n' > binary
git add x binary
git commit -m "left"
git checkout master
git checkout -b right
echo "right" > x
git add x
git commit -m "right"
git merge-tree master right left
git merge-tree master right left | xxd
cd ..
rm -rf test

The merge-tree results I get with 2.15.1 are:

added in remote
  their  100644 ddc50ce55647db1421b18aa33417442e29f63d2f binary
@@ -0,0 +1 @@
+added in both
  our100644 c376d892e8b105bd712d06ec5162b5f31ce949c3 x
  their  100644 45cf141ba67d59203f02a54f03162f3fcef57830 x
@@ -1 +1,5 @@
+<<< .our
 right
+===
+left
+>>> .their

: 6164 6465 6420 696e 2072 656d 6f74 650a  added in remote.
0010: 2020 7468 6569 7220 2031 3030 3634 3420their  100644
0020: 6464 6335 3063 6535 3536 3437 6462 3134  ddc50ce55647db14
0030: 3231 6231 3861 6133 3334 3137 3434 3265  21b18aa33417442e
0040: 3239 6636 3364 3266 2062 696e 6172 790a  29f63d2f binary.
0050: 4040 202d 302c 3020 2b31 2040 400a 2b01  @@ -0,0 +1 @@.+.
0060: 6164 6465 6420 696e 2062 6f74 680a 2020  added in both.
0070: 6f75 7220 2020 2031 3030 3634 3420 6333  our100644 c3
0080: 3736 6438 3932 6538 6231 3035 6264 3731  76d892e8b105bd71
0090: 3264 3036 6563 3531 3632 6235 6633 3163  2d06ec5162b5f31c
00a0: 6539 3439 6333 2078 0a20 2074 6865 6972  e949c3 x.  their
00b0: 2020 3130 3036 3434 2034 3563 6631 3431100644 45cf141
00c0: 6261 3637 6435 3932 3033 6630 3261 3534  ba67d59203f02a54
00d0: 6630 3331 3632 6633 6663 6566 3537 3833  f03162f3fcef5783
00e0: 3020 780a 4040 202d 3120 2b31 2c35 2040  0 x.@@ -1 +1,5 @
00f0: 400a 2b3c 3c3c 3c3c 3c3c 202e 6f75 720a  @.+<<< .our.
0100: 2072 6967 6874 0a2b 3d3d 3d3d 3d3d 3d0a   right.+===.
0110: 2b6c 6566 740a 2b3e 3e3e 3e3e 3e3e 202e  +left.+>>> .
0120: 7468 6569 720a   their.

Note that the "added in both" explanation appears to be part of the
diff for binary. The diff line should be '\1\0\1\n', but it is only
'\1', obviously suggesting a C string operation gone awry.

I haven't checked whether regular 'git diff' operations contain a
similar bug. (The NUL would have to be pretty far into the file, to
confuse the binary file detection heuristic, but that is possible.)

I don't see any particularly good work-arounds. Looking for all
possible explanations as a trigger to detect a malformed patch runs
into false positives with the explanation "merged", which occurs in
regular code.


2. Feature suggestion

Related to the bug, may I suggest a flag to omit unnecessary patches?
For "added in remote" and "deleted in remote", I don't actually need
the patch--I can grab the blob contents from the SHA myself if needed.
These cases need special handling anyway (to create/delete the file),
so the (often large) patch doesn't add much anyway. This would provide
a workaround for the bug.


3. Feature suggestion

There's no direct indication of whether any given file's merge
succeeded. Currently I sniff for merge conflicts by looking for
"+<<< .our", which feels like an ugly kludge. Could we provide an
explicit indicator? (And maybe also one for binary vs text
processing?)

Note that binary file merge conflicts don't generate patches with
three-way merge markers but instead say "warning: Cannot merge binary
files: binary (.our vs. .their)". Looking for this case even further
complicates the output parser.


4. API suggestion

Here's what I really want 'git merge-tree' to output. :)

If the merge had no conflicts, write the resulting tree to the object
database and give me its sha. I can always diff that tree against
branch1's tree if I want to see what has changed.

If the merge had conflicts, write the "as merged as possible" tree to
the object database and give me its sha, and then also give me the
three-way merge diff output for all conflicts, as a regular patch
against that tree, using full path names and shas. (Alternatively,
maybe better, give me a second sha for a tree containing all the
three-way merge diff patches applied, which I can diff against the
first tree to find the conflict patches.)

I'm not sure what to do about binary merge conflicts, since they
aren't representable with three-way markers. Maybe 

Re: [PATCH] t: add clone test for files differing only in case

[Eric Sunshine]

On Sat, Jan 20, 2018 at 3:33 PM, brian m. carlson
 wrote:
> We recently introduced a regression in cloning repositories onto
> case-insensitive file systems where the repository contains multiple
> files differing only in case.  In such a case, we would segfault.  This
> segfault has already been fixed (repository: pre-initialize hash algo
> pointer), but it's not the first time similar problems have arisen.
> Introduce a test to catch this case and ensure the behavior does not
> regress.

I guess you'd probably want a "From: Eric Sunshine "
header before this paragraph.

The patch itself looks correct... ;-)

> Signed-off-by: Eric Sunshine 
> Signed-off-by: brian m. carlson 
> ---
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index 0f895478f0..53b2dda9d2 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -611,4 +611,17 @@ test_expect_success 'GIT_TRACE_PACKFILE produces a 
> usable pack' '
> git -C replay.git index-pack -v --stdin   '
>
> +hex2oct() {
> +   perl -ne 'printf "\\%03o", hex for /../g'
> +}
> +
> +test_expect_success 'clone on case-insensitive fs' '
> +   o=$(git hash-object -w --stdin  +   t=$(printf "100644 X\0${o}100644 x\0${o}" |
> +   git hash-object -w -t tree --stdin) &&
> +   c=$(git commit-tree -m bogus $t) &&
> +   git update-ref refs/heads/bogus $c &&
> +   git clone -b bogus . bogus
> +'
> +
>  test_done

[PATCH v2 10/12] fetch: stop accessing "remote" variable indirectly

[Ævar Arnfjörð Bjarmason]

Access the "remote" variable passed to the fetch_one() directly rather
than through the gtransport wrapper struct constructed in this
function for other purposes.

This makes the code more readable, as it's now obvious that the remote
struct doesn't somehow get munged by the prepare_transport() function
above, which takes the "remote" struct as an argument and constructs
the "gtransport" struct, containing among other things the "remote"
struct.

This pattern of accessing the container struct was added in
737c5a9cde ("fetch: make --prune configurable", 2013-07-13) when this
code was initially introduced.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 builtin/fetch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index b34665db9e..a85c2002a9 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1280,8 +1280,8 @@ static int fetch_one(struct remote *remote, int argc, 
const char **argv)
 
if (prune < 0) {
/* no command line request */
-   if (0 <= gtransport->remote->prune)
-   prune = gtransport->remote->prune;
+   if (0 <= remote->prune)
+   prune = remote->prune;
else if (0 <= fetch_prune_config)
prune = fetch_prune_config;
else
-- 
2.15.1.424.g9478a66081

[PATCH v2 11/12] fetch tests: add scaffolding for the new fetch.pruneTags

[Ævar Arnfjörð Bjarmason]

The fetch.pruneTags configuration doesn't exist yet, but will be added
in a subsequent commit. Since testing for it requires adding new
parameters to the test_configured_prune function it's easier to review
this patch first to assert that no functional changes are introduced
yet.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5510-fetch.sh | 90 ++--
 1 file changed, 49 insertions(+), 41 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 576c2598c9..a1abea7e3f 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -551,18 +551,22 @@ set_config_tristate () {
 test_configured_prune () {
fetch_prune=$1
remote_origin_prune=$2
-   expected_branch=$3
-   expected_tag=$4
-   cmdline=$5
+   fetch_prune_tags=$3
+   remote_origin_prune_tags=$4
+   expected_branch=$5
+   expected_tag=$6
+   cmdline=$7
 
-   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${5:+ 
$5}; branch:$3 tag:$4" '
+   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2 
fetch.pruneTags=$3 remote.origin.pruneTags=$4${7:+ $7}; branch:$5 tag:$6" '
# make sure a newbranch is there in . and also in one
git branch -f newbranch &&
git tag -f newtag &&
(
cd one &&
test_unconfig fetch.prune &&
+   test_unconfig fetch.pruneTags &&
test_unconfig remote.origin.prune &&
+   test_unconfig remote.origin.pruneTags &&
git fetch &&
git rev-parse --verify refs/remotes/origin/newbranch &&
git rev-parse --verify refs/tags/newtag
@@ -576,7 +580,9 @@ test_configured_prune () {
(
cd one &&
set_config_tristate fetch.prune $fetch_prune &&
+   set_config_tristate fetch.pruneTags $fetch_prune_tags &&
set_config_tristate remote.origin.prune 
$remote_origin_prune &&
+   set_config_tristate remote.origin.pruneTags 
$remote_origin_prune_tags &&
 
git fetch '"$cmdline"' &&
case "$expected_branch" in
@@ -601,57 +607,59 @@ test_configured_prune () {
 
 # $1 config: fetch.prune
 # $2 config: remote..prune
-# $3 expect: branch to be pruned?
-# $4 expect: tag to be pruned?
-# $5 git-fetch $cmdline:
+# $3 config: fetch.pruneTags
+# $4 config: remote..pruneTags
+# $5 expect: branch to be pruned?
+# $6 expect: tag to be pruned?
+# $7 git-fetch $cmdline:
 #
-# $1$2$3 $4 $5
-test_configured_prune unset unset kept   kept   ""
-test_configured_prune unset unset kept   kept   "--no-prune"
-test_configured_prune unset unset pruned kept   "--prune"
-test_configured_prune unset unset kept   pruned \
+# $1$2$3$4$5 $6 $7
+test_configured_prune unset unset unset unset kept   kept   ""
+test_configured_prune unset unset unset unset kept   kept   "--no-prune"
+test_configured_prune unset unset unset unset pruned kept   "--prune"
+test_configured_prune unset unset unset unset kept   pruned \
"--prune origin 'refs/tags/*:refs/tags/*'"
-test_configured_prune unset unset pruned pruned \
+test_configured_prune unset unset unset unset pruned pruned \
"--prune origin 'refs/tags/*:refs/tags/*' 
'+refs/heads/*:refs/remotes/origin/*'"
 
-test_configured_prune false unset kept   kept   ""
-test_configured_prune false unset kept   kept   "--no-prune"
-test_configured_prune false unset pruned kept   "--prune"
+test_configured_prune false unset unset unset kept   kept   ""
+test_configured_prune false unset unset unset kept   kept   "--no-prune"
+test_configured_prune false unset unset unset pruned kept   "--prune"
 
-test_configured_prune true  unset pruned kept   ""
-test_configured_prune true  unset pruned kept   "--prune"
-test_configured_prune true  unset kept   kept   "--no-prune"
+test_configured_prune true  unset unset unset pruned kept   ""
+test_configured_prune true  unset unset unset pruned kept   "--prune"
+test_configured_prune true  unset unset unset kept   kept   "--no-prune"
 
-test_configured_prune unset false kept   kept   ""
-test_configured_prune unset false kept   kept   "--no-prune"
-test_configured_prune unset false pruned kept   "--prune"
+test_configured_prune unset false unset unset kept   kept   ""
+test_configured_prune unset false unset unset kept   kept   "--no-prune"
+test_configured_prune unset false unset unset pruned kept   "--prune"
 
-test_configured_prune false false kept   kept   ""
-test_configured_prune false false kept   kept   "--no-prune"
-test_configured_prune false false pruned kept   "--prune"
-test_configured_prune false false kept   pruned \
+test_configured_prune false false unset 

[PATCH v2 08/12] git-fetch & config doc: link to the new PRUNING section

[Ævar Arnfjörð Bjarmason]

Amend the documentation for fetch.prune, fetch..prune and
--prune to link to the recently added PRUNING section.

I'd have liked to link directly to it with "<>" from
fetch-options.txt, since it's included in git-fetch.txt (git-pull.txt
also includes it, but doesn't include that option). However making a
reference across files yields this error:

[...]/Documentation/git-fetch.xml:226: element xref: validity
error : IDREF attribute linkend references an unknown ID "PRUNING"

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config.txt| 6 +-
 Documentation/fetch-options.txt | 3 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0e25b2c92b..0f27af5760 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1398,7 +1398,8 @@ fetch.unpackLimit::
 
 fetch.prune::
If true, fetch will automatically behave as if the `--prune`
-   option was given on the command line.  See also `remote..prune`.
+   option was given on the command line.  See also `remote..prune`
+   and the PRUNING section of linkgit:git-fetch[1].
 
 fetch.output::
Control how ref update status is printed. Valid values are
@@ -2944,6 +2945,9 @@ remote..prune::
remove any remote-tracking references that no longer exist on the
remote (as if the `--prune` option was given on the command line).
Overrides `fetch.prune` settings, if any.
++
+See also `remote..prune` and the PRUNING section of
+linkgit:git-fetch[1].
 
 remotes.::
The list of remotes which are fetched by "git remote update
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index fb6bebbc61..9f5c85ad96 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -74,6 +74,9 @@ ifndef::git-pull[]
line or in the remote configuration, for example if the remote
was cloned with the --mirror option), then they are also
subject to pruning.
++
+See the PRUNING section below for more details.
+
 endif::git-pull[]
 
 ifndef::git-pull[]
-- 
2.15.1.424.g9478a66081

[PATCH v2 12/12] fetch: add a --fetch-prune option and fetch.pruneTags config

[Ævar Arnfjörð Bjarmason]

Add a --fetch-prune option to git-fetch, along with fetch.pruneTags
config option. This allows for doing any of:

git fetch -p -P
git fetch --prune --prune-tags
git fetch -p -P origin
git fetch --prune --prune-tags origin

Or simply:

git config fetch.prune true &&
git config fetch.pruneTags true &&
git fetch

Instead of the much more verbose:

git fetch --prune origin 'refs/tags/*:refs/tags/*' 
'+refs/heads/*:refs/remotes/origin/*'

Before this feature it was painful to support the use-case of pulling
from a repo which is having both its branches *and* tags deleted
regularly, and wanting our local references to reflect upstream.

At work we create deployment tags in the repo for each rollout, and
there's *lots* of those, so they're archived within weeks for
performance reasons.

Without this change it's hard to centrally configure such repos in
/etc/gitconfig (on servers that are only used for working with
them). You need to set fetch.prune=true globally, and then for each
repo:

git -C {} config --replace-all remote.origin.fetch 
"refs/tags/*:refs/tags/*" "^refs/tags/*:refs/tags/*$"

Now I can simply set fetch.pruneTags=true in /etc/gitconfig as well,
and users running "git pull" will automatically get the pruning
semantics we want.

See my "Re: [BUG] git remote prune removes local tags, depending on
fetch config" (87po6ahx87@evledraar.gmail.com;
https://public-inbox.org/git/87po6ahx87@evledraar.gmail.com/) for
more background info.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config.txt| 14 ++
 Documentation/fetch-options.txt | 15 ++-
 Documentation/git-fetch.txt | 24 
 builtin/fetch.c | 32 
 remote.c|  5 -
 remote.h|  3 +++
 t/t5510-fetch.sh| 30 ++
 7 files changed, 121 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0f27af5760..ae86455876 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1401,6 +1401,14 @@ fetch.prune::
option was given on the command line.  See also `remote..prune`
and the PRUNING section of linkgit:git-fetch[1].
 
+fetch.pruneTags::
+   If true, fetch will automatically behave as if the
+   `refs/tags/*:refs/tags/*` refspec was provided when pruning,
+   if not set already. This allows for setting both this option
+   and `fetch.prune` to maintain a 1=1 mapping to upstrem
+   refs. See also `remote..pruneTags` and the PRUNING
+   section of linkgit:git-fetch[1].
+
 fetch.output::
Control how ref update status is printed. Valid values are
`full` and `compact`. Default value is `full`. See section
@@ -2945,6 +2953,12 @@ remote..prune::
remove any remote-tracking references that no longer exist on the
remote (as if the `--prune` option was given on the command line).
Overrides `fetch.prune` settings, if any.
+
+remote..pruneTags::
+   When set to true, fetching from this remote by default will also
+   remove any local tags that no longer exist on the remote if pruning
+   is activated in general via `remote..prune`, `fetch.prune` or
+   `--prune`. Overrides `fetch.pruneTags` settings, if any.
 +
 See also `remote..prune` and the PRUNING section of
 linkgit:git-fetch[1].
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 9f5c85ad96..dc13bed741 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -73,7 +73,20 @@ ifndef::git-pull[]
are fetched due to an explicit refspec (either on the command
line or in the remote configuration, for example if the remote
was cloned with the --mirror option), then they are also
-   subject to pruning.
+   subject to pruning. Supplying `--prune-tags` is a shorthand for
+   providing the tag refspec.
++
+See the PRUNING section below for more details.
+
+-P::
+--prune-tags::
+   Before fetching, remove any local tags that no longer exist on
+   the remote if `--prune` is enabled. This option should be used
+   more carefully, unlike `--prune` it will remove any local
+   references (local tags) that have been created. This option is
+   merely a shorthand for providing the explicit tag refspec
+   along with `--prune`, see the discussion about that in its
+   documentation.
 +
 See the PRUNING section below for more details.
 
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index 18fac0da2e..5682ed4ae1 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -148,6 +148,30 @@ So be careful when using this with a refspec like
 `refs/tags/*:refs/tags/*`, or any other refspec which might map
 references from multiple remotes to the same local 

[PATCH v2 09/12] fetch: don't redundantly NULL something calloc() gave us

[Ævar Arnfjörð Bjarmason]

Stop redundantly NULL-ing the last element of the refs structure,
which was retrieved via calloc(), and is thus guaranteed to be
pre-NULL'd.

This code dates back to b888d61c83 ("Make fetch a builtin",
2007-09-10), where wasn't any reason to do this back then either, it's
just something left over from when git-fetch was initially introduced.

The initial motivation for this change was to make a subsequent change
which'll also modify the refs variable smaller, since it won't have to
copy this redundant "NULL the last + 1 item" pattern.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 builtin/fetch.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7bbcd26faf..b34665db9e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1302,7 +1302,6 @@ static int fetch_one(struct remote *remote, int argc, 
const char **argv)
} else
refs[j++] = argv[i];
}
-   refs[j] = NULL;
ref_nr = j;
}
 
-- 
2.15.1.424.g9478a66081

[PATCH v2 03/12] fetch tests: add a tag to be deleted to the pruning tests

[Ævar Arnfjörð Bjarmason]

Add a tag to be deleted to the fetch --prune tests. The tag is always
kept for now, which is the expected behavior, but now I can add a test
for tag pruning in a later commit.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5510-fetch.sh | 93 
 1 file changed, 53 insertions(+), 40 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index ab8b25344d..fad65bd885 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -552,21 +552,25 @@ test_configured_prune () {
fetch_prune=$1
remote_origin_prune=$2
expected_branch=$3
-   cmdline=$4
+   expected_tag=$4
+   cmdline=$5
 
-   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${4:+ 
$4}; branch:$3" '
+   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${5:+ 
$5}; branch:$3 tag:$4" '
# make sure a newbranch is there in . and also in one
git branch -f newbranch &&
+   git tag -f newtag &&
(
cd one &&
test_unconfig fetch.prune &&
test_unconfig remote.origin.prune &&
git fetch &&
-   git rev-parse --verify refs/remotes/origin/newbranch
+   git rev-parse --verify refs/remotes/origin/newbranch &&
+   git rev-parse --verify refs/tags/newtag
) &&
 
# now remove it
git branch -d newbranch &&
+   git tag -d newtag &&
 
# then test
(
@@ -582,6 +586,14 @@ test_configured_prune () {
kept)
git rev-parse --verify 
refs/remotes/origin/newbranch
;;
+   esac &&
+   case "$expected_tag" in
+   pruned)
+   test_must_fail git rev-parse --verify 
refs/tags/newtag
+   ;;
+   kept)
+   git rev-parse --verify refs/tags/newtag
+   ;;
esac
)
'
@@ -590,44 +602,45 @@ test_configured_prune () {
 # $1 config: fetch.prune
 # $2 config: remote..prune
 # $3 expect: branch to be pruned?
-# $4 git-fetch $cmdline:
+# $4 expect: tag to be pruned?
+# $5 git-fetch $cmdline:
 #
-# $1$2$3 $4
-test_configured_prune unset unset kept   ""
-test_configured_prune unset unset kept   "--no-prune"
-test_configured_prune unset unset pruned "--prune"
-
-test_configured_prune false unset kept   ""
-test_configured_prune false unset kept   "--no-prune"
-test_configured_prune false unset pruned "--prune"
-
-test_configured_prune true  unset pruned ""
-test_configured_prune true  unset pruned "--prune"
-test_configured_prune true  unset kept   "--no-prune"
-
-test_configured_prune unset false kept   ""
-test_configured_prune unset false kept   "--no-prune"
-test_configured_prune unset false pruned "--prune"
-
-test_configured_prune false false kept   ""
-test_configured_prune false false kept   "--no-prune"
-test_configured_prune false false pruned "--prune"
-
-test_configured_prune true  false kept   ""
-test_configured_prune true  false pruned "--prune"
-test_configured_prune true  false kept   "--no-prune"
-
-test_configured_prune unset true  pruned ""
-test_configured_prune unset true  kept   "--no-prune"
-test_configured_prune unset true  pruned "--prune"
-
-test_configured_prune false true  pruned ""
-test_configured_prune false true  kept   "--no-prune"
-test_configured_prune false true  pruned "--prune"
-
-test_configured_prune true  true  pruned ""
-test_configured_prune true  true  pruned "--prune"
-test_configured_prune true  true  kept   "--no-prune"
+# $1$2$3 $4 $5
+test_configured_prune unset unset kept   kept   ""
+test_configured_prune unset unset kept   kept   "--no-prune"
+test_configured_prune unset unset pruned kept   "--prune"
+
+test_configured_prune false unset kept   kept   ""
+test_configured_prune false unset kept   kept   "--no-prune"
+test_configured_prune false unset pruned kept   "--prune"
+
+test_configured_prune true  unset pruned kept   ""
+test_configured_prune true  unset pruned kept   "--prune"
+test_configured_prune true  unset kept   kept   "--no-prune"
+
+test_configured_prune unset false kept   kept   ""
+test_configured_prune unset false kept   kept   "--no-prune"
+test_configured_prune unset false pruned kept   "--prune"
+
+test_configured_prune false false kept   kept   ""
+test_configured_prune false false kept   kept   "--no-prune"
+test_configured_prune false false pruned kept   "--prune"
+
+test_configured_prune true  false kept   kept   ""
+test_configured_prune true  false pruned kept   "--prune"
+test_configured_prune 

[PATCH v2 04/12] fetch tests: double quote a variable for interpolation

[Ævar Arnfjörð Bjarmason]

If the $cmdline variable contains multiple arguments they won't be
interpolated correctly since the body of the test is single quoted. I
don't know what part of test-lib.sh is expanding variables within
single-quoted strings, but interpolating this inline is the desired
behavior here.

This will be used in a subsequent commit to pass more than one
variable to git-fetch.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5510-fetch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index fad65bd885..542eb53a99 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -578,7 +578,7 @@ test_configured_prune () {
set_config_tristate fetch.prune $fetch_prune &&
set_config_tristate remote.origin.prune 
$remote_origin_prune &&
 
-   git fetch $cmdline &&
+   git fetch '"$cmdline"' &&
case "$expected_branch" in
pruned)
test_must_fail git rev-parse --verify 
refs/remotes/origin/newbranch
-- 
2.15.1.424.g9478a66081

[PATCH v2 05/12] fetch tests: test --prune and refspec interaction

[Ævar Arnfjörð Bjarmason]

Add a test for the interaction between explicitly provided refspecs
and fetch.prune.

There's no point in adding this boilerplate to every combination of
unset/false/true, it's instructive and sufficient to show that no
matter if the variable is unset, false or true the refspec on the
command-line overrides any configuration variable.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5510-fetch.sh | 12 
 1 file changed, 12 insertions(+)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 542eb53a99..576c2598c9 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -609,6 +609,10 @@ test_configured_prune () {
 test_configured_prune unset unset kept   kept   ""
 test_configured_prune unset unset kept   kept   "--no-prune"
 test_configured_prune unset unset pruned kept   "--prune"
+test_configured_prune unset unset kept   pruned \
+   "--prune origin 'refs/tags/*:refs/tags/*'"
+test_configured_prune unset unset pruned pruned \
+   "--prune origin 'refs/tags/*:refs/tags/*' 
'+refs/heads/*:refs/remotes/origin/*'"
 
 test_configured_prune false unset kept   kept   ""
 test_configured_prune false unset kept   kept   "--no-prune"
@@ -625,6 +629,10 @@ test_configured_prune unset false pruned kept   "--prune"
 test_configured_prune false false kept   kept   ""
 test_configured_prune false false kept   kept   "--no-prune"
 test_configured_prune false false pruned kept   "--prune"
+test_configured_prune false false kept   pruned \
+   "--prune origin 'refs/tags/*:refs/tags/*'"
+test_configured_prune false false pruned pruned \
+   "--prune origin 'refs/tags/*:refs/tags/*' 
'+refs/heads/*:refs/remotes/origin/*'"
 
 test_configured_prune true  false kept   kept   ""
 test_configured_prune true  false pruned kept   "--prune"
@@ -641,6 +649,10 @@ test_configured_prune false true  pruned kept   "--prune"
 test_configured_prune true  true  pruned kept   ""
 test_configured_prune true  true  pruned kept   "--prune"
 test_configured_prune true  true  kept   kept   "--no-prune"
+test_configured_prune true  true  kept   pruned \
+   "--prune origin 'refs/tags/*:refs/tags/*'"
+test_configured_prune true  true  pruned pruned \
+   "--prune origin 'refs/tags/*:refs/tags/*' 
'+refs/heads/*:refs/remotes/origin/*'"
 
 test_expect_success 'all boundary commits are excluded' '
test_commit base &&
-- 
2.15.1.424.g9478a66081

[PATCH v2 06/12] git fetch doc: add a new section to explain the ins & outs of pruning

[Ævar Arnfjörð Bjarmason]

Add a new section to canonically explain how remote reference pruning
works, and how users should be careful about using it in conjunction
with tag refspecs in particular.

A subsequent commit will update the git-remote documentation to refer
to this section, and details the motivation for writing this in the
first place.

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

diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index b153aefa68..18fac0da2e 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -99,6 +99,55 @@ The latter use of the `remote..fetch` values can 
be
 overridden by giving the `--refmap=` parameter(s) on the
 command line.
 
+PRUNING
+---
+
+Git has a default disposition of keeping data unless it's explicitly
+thrown away; this extends to holding onto local references to branches
+on remotes that have themselves deleted those branches.
+
+If left to accumulate, these stale references might make performance
+worse on big and busy repos that have a lot of branch churn, and
+e.g. make the output of commands like `git branch -a --contains
+` needlessly verbose, as well as impacting anything else
+that'll work with the complete set of known references.
+
+These remote tracking references can be deleted as a one-off with
+either of:
+
+
+# While fetching
+$ git fetch --prune 
+
+# Only prune, don't fetch
+$ git remote prune 
+
+
+To prune references as part of your normal workflow without needing to
+remember to run that set `fetch.prune` globally, or
+`remote..prune` per-remote in the config. See
+linkgit:git-config[1].
+
+Here's where things get tricky and more specific. The pruning feature
+doesn't actually care about branches, instead it'll prune local <->
+remote references as a function of the refspec of the remote (see
+`` and <> above).
+
+Therefore if the refspec for the remote includes
+e.g. `refs/tags/*:refs/tags/*`, or you manually run e.g. `git fetch
+--prune  "refs/tags/*:refs/tags/*"` it won't be stale remote
+tracking branches that are deleted, but any local tag that doesn't
+exist on the remote.
+
+This might not be what you expect, i.e. you want to prune remote
+``, but also explicitly fetch tags from it, so when you fetch
+from it you delete all your local tags, most of which may not have
+come from the `` remote in the first place.
+
+So be careful when using this with a refspec like
+`refs/tags/*:refs/tags/*`, or any other refspec which might map
+references from multiple remotes to the same local namespace.
+
 OUTPUT
 --
 
-- 
2.15.1.424.g9478a66081

[PATCH v2 00/12] document & test fetch pruning & add fetch.pruneTags

[Ævar Arnfjörð Bjarmason]

Now v2 and fully non-RFC, changes:

Ævar Arnfjörð Bjarmason (12):
  fetch tests: refactor in preparation for testing tag pruning
  fetch tests: arrange arguments for future readability
  fetch tests: add a tag to be deleted to the pruning tests
  fetch tests: double quote a variable for interpolation
  fetch tests: test --prune and refspec interaction

No changes.

  git fetch doc: add a new section to explain the ins & outs of pruning

Grammar etc. fixes from Eric. Thanks!

  git remote doc: correct dangerous lies about what prune does
  git-fetch & config doc: link to the new PRUNING section

No changes.

  fetch: don't redundantly NULL something calloc() gave us

Minor rephrasing of the commit message.

  fetch: stop accessing "remote" variable indirectly

NEW: Amends some existing confusing code, whose pattern will be used
by 12/12.

  fetch tests: add scaffolding for the new fetch.pruneTags

I screwed up positional arguments in the test description, fixed.

  fetch: add a --fetch-prune option and fetch.pruneTags config

Now actually works, and with a very different implementation which
involves making the previously private add_fetch_refspec() function in
remote.c part of the API.

 Documentation/config.txt|  20 +-
 Documentation/fetch-options.txt |  18 -
 Documentation/git-fetch.txt |  73 +++
 Documentation/git-remote.txt|  14 ++--
 builtin/fetch.c |  37 +-
 remote.c|   5 +-
 remote.h|   3 +
 t/t5510-fetch.sh| 154 +---
 8 files changed, 272 insertions(+), 52 deletions(-)

-- 
2.15.1.424.g9478a66081

[PATCH v2 02/12] fetch tests: arrange arguments for future readability

[Ævar Arnfjörð Bjarmason]

Re-arrange the arguments to the test_configured_prune() function used
in this test to pass the arguments to --fetch last. A subsequent
change will test for more elaborate fetch arguments, including long
refspecs. It'll be more readable to be able to wrap those on a new
line of their own.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5510-fetch.sh | 82 ++--
 1 file changed, 44 insertions(+), 38 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 11da97f9b7..ab8b25344d 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -551,10 +551,10 @@ set_config_tristate () {
 test_configured_prune () {
fetch_prune=$1
remote_origin_prune=$2
-   cmdline=$3
-   expected_branch=$4
+   expected_branch=$3
+   cmdline=$4
 
-   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${3:+ 
$3}; branch:$4" '
+   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${4:+ 
$4}; branch:$3" '
# make sure a newbranch is there in . and also in one
git branch -f newbranch &&
(
@@ -587,41 +587,47 @@ test_configured_prune () {
'
 }
 
-test_configured_prune unset unset ""   kept
-test_configured_prune unset unset "--no-prune" kept
-test_configured_prune unset unset "--prune"pruned
-
-test_configured_prune false unset ""   kept
-test_configured_prune false unset "--no-prune" kept
-test_configured_prune false unset "--prune"pruned
-
-test_configured_prune true  unset ""   pruned
-test_configured_prune true  unset "--prune"pruned
-test_configured_prune true  unset "--no-prune" kept
-
-test_configured_prune unset false ""   kept
-test_configured_prune unset false "--no-prune" kept
-test_configured_prune unset false "--prune"pruned
-
-test_configured_prune false false ""   kept
-test_configured_prune false false "--no-prune" kept
-test_configured_prune false false "--prune"pruned
-
-test_configured_prune true  false ""   kept
-test_configured_prune true  false "--prune"pruned
-test_configured_prune true  false "--no-prune" kept
-
-test_configured_prune unset true  ""   pruned
-test_configured_prune unset true  "--no-prune" kept
-test_configured_prune unset true  "--prune"pruned
-
-test_configured_prune false true  ""   pruned
-test_configured_prune false true  "--no-prune" kept
-test_configured_prune false true  "--prune"pruned
-
-test_configured_prune true  true  ""   pruned
-test_configured_prune true  true  "--prune"pruned
-test_configured_prune true  true  "--no-prune" kept
+# $1 config: fetch.prune
+# $2 config: remote..prune
+# $3 expect: branch to be pruned?
+# $4 git-fetch $cmdline:
+#
+# $1$2$3 $4
+test_configured_prune unset unset kept   ""
+test_configured_prune unset unset kept   "--no-prune"
+test_configured_prune unset unset pruned "--prune"
+
+test_configured_prune false unset kept   ""
+test_configured_prune false unset kept   "--no-prune"
+test_configured_prune false unset pruned "--prune"
+
+test_configured_prune true  unset pruned ""
+test_configured_prune true  unset pruned "--prune"
+test_configured_prune true  unset kept   "--no-prune"
+
+test_configured_prune unset false kept   ""
+test_configured_prune unset false kept   "--no-prune"
+test_configured_prune unset false pruned "--prune"
+
+test_configured_prune false false kept   ""
+test_configured_prune false false kept   "--no-prune"
+test_configured_prune false false pruned "--prune"
+
+test_configured_prune true  false kept   ""
+test_configured_prune true  false pruned "--prune"
+test_configured_prune true  false kept   "--no-prune"
+
+test_configured_prune unset true  pruned ""
+test_configured_prune unset true  kept   "--no-prune"
+test_configured_prune unset true  pruned "--prune"
+
+test_configured_prune false true  pruned ""
+test_configured_prune false true  kept   "--no-prune"
+test_configured_prune false true  pruned "--prune"
+
+test_configured_prune true  true  pruned ""
+test_configured_prune true  true  pruned "--prune"
+test_configured_prune true  true  kept   "--no-prune"
 
 test_expect_success 'all boundary commits are excluded' '
test_commit base &&
-- 
2.15.1.424.g9478a66081

[PATCH v2 07/12] git remote doc: correct dangerous lies about what prune does

[Ævar Arnfjörð Bjarmason]

The "git remote prune " command uses the same machinery as "git
fetch  --prune", and shares all the same caveats, but its
documentation has suggested that it'll just "delete stale
remote-tracking branches under ".

This isn't true, and hasn't been true since at least v1.8.5.6 (the
oldest version I could be bothered to test).

E.g. if "+refs/tags/*:refs/tags/*" is explicitly set in the refspec of
the remote it'll delete all local tags  doesn't know about.

Instead, briefly give the reader just enough of a hint that this
option might constitute a shotgun aimed at their foot, and point them
to the new PRUNING section in the git-fetch documentation which
explains all the nuances of what this facility does.

See "[BUG] git remote prune removes local tags, depending on fetch
config" (caci5s_39wnrbfjlfn0xhcy+uewtfn2ymnacrc86z6pjutjw...@mail.gmail.com)
by Michael Giuffrida for the initial report.

Reported-by: Michael Giuffrida 
Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-remote.txt | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 577b969c1b..7183a72a23 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -172,10 +172,14 @@ With `-n` option, the remote heads are not queried first 
with
 
 'prune'::
 
-Deletes all stale remote-tracking branches under .
-These stale branches have already been removed from the remote repository
-referenced by , but are still locally available in
-"remotes/".
+Deletes stale references associated with . By default stale
+remote-tracking branches under , but depending on global
+configuration and the configuration of the remote we might even prune
+local tags. Equivalent to `git fetch  --prune`, except that no
+new references will be fetched.
++
+See the PRUNING section of linkgit:git-fetch[1] for what it'll prune
+depending on various configuration.
 +
 With `--dry-run` option, report what branches will be pruned, but do not
 actually prune them.
@@ -189,7 +193,7 @@ remotes.default is not defined, all remotes which do not 
have the
 configuration parameter remote..skipDefaultUpdate set to true will
 be updated.  (See linkgit:git-config[1]).
 +
-With `--prune` option, prune all the remotes that are updated.
+With `--prune` option, run pruning against all the remotes that are updated.
 
 
 DISCUSSION
-- 
2.15.1.424.g9478a66081

[PATCH v2 01/12] fetch tests: refactor in preparation for testing tag pruning

[Ævar Arnfjörð Bjarmason]

In a subsequent commit this function will learn to test for tag
pruning, prepare for that by making space for more variables, and
making it clear that "expected" here refers to branches.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t5510-fetch.sh | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 668c54be41..11da97f9b7 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -549,9 +549,12 @@ set_config_tristate () {
 }
 
 test_configured_prune () {
-   fetch_prune=$1 remote_origin_prune=$2 cmdline=$3 expected=$4
+   fetch_prune=$1
+   remote_origin_prune=$2
+   cmdline=$3
+   expected_branch=$4
 
-   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${3:+ 
$3}; $4" '
+   test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${3:+ 
$3}; branch:$4" '
# make sure a newbranch is there in . and also in one
git branch -f newbranch &&
(
@@ -572,7 +575,7 @@ test_configured_prune () {
set_config_tristate remote.origin.prune 
$remote_origin_prune &&
 
git fetch $cmdline &&
-   case "$expected" in
+   case "$expected_branch" in
pruned)
test_must_fail git rev-parse --verify 
refs/remotes/origin/newbranch
;;
-- 
2.15.1.424.g9478a66081

[PATCH v4] mru: Replace mru.[ch] with list.h implementation

[Gargi Sharma]

Replace the custom calls to mru.[ch] with calls to list.h. This patch is
the final step in removing the mru API completely and inlining the logic.
This patch leads to significant code reduction and the mru API hence, is
not a useful abstraction anymore.

Signed-off-by: Gargi Sharma 
---
Changes in v4:
- Fixing minor style issues.
Changes in v3:
- Make the commit message more descriptive.
- Remove braces after if statement.
Changes in v2:
- Add a move to front function to the list API.
- Remove memory leak.
- Remove redundant remove operations on the list.

The commit has been built on top of ot/mru-on-list branch.

A future improvement could be removing/changing the
type of nect pointer or dropping it entirely. See discussion
here:
https://public-inbox.org/git/caoci2dgyqr4jff5oby2buyhnjeaapqkf8tbojn2w0b18eo+...@mail.gmail.com/
---
 Makefile   |  1 -
 builtin/pack-objects.c |  9 -
 cache.h|  8 
 list.h |  7 +++
 mru.c  | 27 ---
 mru.h  | 40 
 packfile.c | 15 +++
 sha1_file.c|  1 -
 8 files changed, 22 insertions(+), 86 deletions(-)
 delete mode 100644 mru.c
 delete mode 100644 mru.h

diff --git a/Makefile b/Makefile
index ed4ca43..4a79ec5 100644
--- a/Makefile
+++ b/Makefile
@@ -814,7 +814,6 @@ LIB_OBJS += merge.o
 LIB_OBJS += merge-blobs.o
 LIB_OBJS += merge-recursive.o
 LIB_OBJS += mergesort.o
-LIB_OBJS += mru.o
 LIB_OBJS += name-hash.o
 LIB_OBJS += notes.o
 LIB_OBJS += notes-cache.o
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ba81234..188ba3e 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -24,7 +24,7 @@
 #include "reachable.h"
 #include "sha1-array.h"
 #include "argv-array.h"
-#include "mru.h"
+#include "list.h"
 #include "packfile.h"
 
 static const char *pack_usage[] = {
@@ -1012,9 +1012,8 @@ static int want_object_in_pack(const unsigned char *sha1,
return want;
}
 
-   list_for_each(pos, _git_mru.list) {
-   struct mru *entry = list_entry(pos, struct mru, list);
-   struct packed_git *p = entry->item;
+   list_for_each(pos, _git_mru) {
+   struct packed_git *p = list_entry(pos, struct packed_git, mru);
off_t offset;
 
if (p == *found_pack)
@@ -1031,7 +1030,7 @@ static int want_object_in_pack(const unsigned char *sha1,
}
want = want_found_object(exclude, p);
if (!exclude && want > 0)
-   mru_mark(_git_mru, entry);
+   list_move_to_front(>mru, _git_mru);
if (want != -1)
return want;
}
diff --git a/cache.h b/cache.h
index 49b083e..cc09e3b 100644
--- a/cache.h
+++ b/cache.h
@@ -4,7 +4,7 @@
 #include "git-compat-util.h"
 #include "strbuf.h"
 #include "hashmap.h"
-#include "mru.h"
+#include "list.h"
 #include "advice.h"
 #include "gettext.h"
 #include "convert.h"
@@ -1566,6 +1566,7 @@ struct pack_window {
 
 extern struct packed_git {
struct packed_git *next;
+   struct list_head mru;
struct pack_window *windows;
off_t pack_size;
const void *index_data;
@@ -1587,10 +1588,9 @@ extern struct packed_git {
 } *packed_git;
 
 /*
- * A most-recently-used ordered version of the packed_git list, which can
- * be iterated instead of packed_git (and marked via mru_mark).
+ * A most-recently-used ordered version of the packed_git list.
  */
-extern struct mru packed_git_mru;
+extern struct list_head packed_git_mru;
 
 struct pack_entry {
off_t offset;
diff --git a/list.h b/list.h
index eb60119..5129b0a 100644
--- a/list.h
+++ b/list.h
@@ -93,6 +93,13 @@ static inline void list_move(struct list_head *elem, struct 
list_head *head)
list_add(elem, head);
 }
 
+/* Move to the front of the list. */
+static inline void list_move_to_front(struct list_head *elem, struct list_head 
*head)
+{
+   list_del(elem);
+   list_add(elem, head);
+}
+
 /* Replace an old entry. */
 static inline void list_replace(struct list_head *old, struct list_head *newp)
 {
diff --git a/mru.c b/mru.c
deleted file mode 100644
index 8f3f34c..000
--- a/mru.c
+++ /dev/null
@@ -1,27 +0,0 @@
-#include "cache.h"
-#include "mru.h"
-
-void mru_append(struct mru *head, void *item)
-{
-   struct mru *cur = xmalloc(sizeof(*cur));
-   cur->item = item;
-   list_add_tail(>list, >list);
-}
-
-void mru_mark(struct mru *head, struct mru *entry)
-{
-   /* To mark means to put at the front of the list. */
-   list_del(>list);
-   list_add(>list, >list);
-}
-
-void mru_clear(struct mru *head)
-{
-   struct list_head *pos;
-   struct list_head *tmp;
-
-   

Re: [PATCH] enable core.fsyncObjectFiles by default

[Junio C Hamano]

Theodore Ts'o  writes:

>   I've never been fond of the "git repack -A" behavior
> where it can generate huge numbers of loose files.  I'd much prefer it
> if the other objects ended up in a separate pack file, and then some
> other provision made for nuking that pack file some time later

Yes, a "cruft pack" that holds unreachable object has been discussed
a few times recently on list, and I do agree that it is a desirable
thing to have in the longer run.

What's tricky is to devise a way to allow us to salvage objects that
are placed in a cruft pack because they are accessed recently,
proving themselves to be no longer crufts.  It could be that a good
way to resurrect them is to explode them to loose form when they are
accessed out of a cruft pack.  We need to worry about interactions
with read-only users if we go that route, but with the current
"explode unreachable to loose, touch their mtime when they are
accessed" scheme ends up ignoring accesses from read-only users that
cannot update mtime, so it might not be too bad.

Re: [PATCH v3] mru: Replace mru.[ch] with list.h implementation

[Gargi Sharma]

On Sat, Jan 20, 2018 at 12:59 AM, Eric Sunshine  wrote:
> On Fri, Jan 19, 2018 at 6:36 PM, Gargi Sharma  wrote:
>> Replace the custom calls to mru.[ch] with calls to list.h. This patch is
>> the final step in removing the mru API completely and inlining the logic.
>> This patch leads to significant code reduction and the mru API hence, is
>> not a useful abstraction anymore.
>
> A couple minor style nits below... (may not be worth a re-roll)

I can send a v4, it shouldn't be a problem. :)

>
>> Signed-off-by: Gargi Sharma 
>> ---
>> diff --git a/cache.h b/cache.h
>> @@ -1587,10 +1588,10 @@ extern struct packed_git {
>>  } *packed_git;
>>
>>  /*
>> - * A most-recently-used ordered version of the packed_git list, which can
>> - * be iterated instead of packed_git (and marked via mru_mark).
>> + * A most-recently-used ordered version of the packed_git list.
>>   */
>> -extern struct mru packed_git_mru;
>> +extern struct list_head packed_git_mru;
>> +
>
> Unnecessary extra blank line.
>
>>  struct pack_entry {
>> off_t offset;
>> diff --git a/packfile.c b/packfile.c
>> @@ -859,9 +859,9 @@ static void prepare_packed_git_mru(void)
>>  {
>> struct packed_git *p;
>>
>> -   mru_clear(_git_mru);
>> -   for (p = packed_git; p; p = p->next)
>> -   mru_append(_git_mru, p);
>> +   for (p = packed_git; p; p = p->next) {
>> +   list_add_tail(>mru, _git_mru);
>> +   }
>
> Unnecessary braces on for-loop.
>
>>  }

Re: [PATCH v3] mru: Replace mru.[ch] with list.h implementation

[Gargi Sharma]

On Sat, Jan 20, 2018 at 1:02 AM, Eric Wong  wrote:
> Gargi Sharma  wrote:
>> --- a/list.h
>> +++ b/list.h
>> @@ -93,6 +93,13 @@ static inline void list_move(struct list_head *elem, 
>> struct list_head *head)
>>   list_add(elem, head);
>>  }
>>
>> +/* Move to the front of the list. */
>> +static inline void list_move_to_front(struct list_head *elem, struct 
>> list_head *head)
>> +{
>> + list_del(elem);
>> + list_add(elem, head);
>> +}
>> +
>
> Since we already have list_move and it does the same thing,
> I don't think we need a new function, here.
>
> Hackers coming from other projects (glibc/urcu/Linux kernel)
> might appreciate having fewer differences from what they're used
> to.

I think the idea behind this function was that it is already being used in two
places in the code and might be used in other places in the future. I agree
with your stance, but a list_move_to_front is pretty self explanatory and not
too different, so it should be alright.

>
> Anyways thanks for working on this!

Re: [PATCH] enable core.fsyncObjectFiles by default

[Theodore Ts'o]

On Fri, Jan 19, 2018 at 11:08:46AM -0800, Junio C Hamano wrote:
> So..., is it fair to say that the one you sent in
> 
>   https://public-inbox.org/git/20180117193510.ga30...@lst.de/
> 
> is the best variant we have seen in this thread so far?  I'll keep
> that in my inbox so that I do not forget, but I think we would want
> to deal with a hotfix for 2.16 on case insensitive platforms before
> this topic.

It's a simplistic fix, but it will work.  There may very well be
certain workloads which generate a large number of loose objects
(e.g., git repack -A) which will make things go significantly more
slowly as a result.  It might very well be the case that if nothing
else is going on, something like "write all the files without
fsync(2), then use syncfs(2)" would be much faster.  The downside with
that approach is if indeed you were downloading a multi-gigabyte DVD
image at the same time, the syncfs(2) will force a writeback of the
partially writte DVD image, or some other unrelated files.

But if the goal is to just change the default, and then see what
shakes out, and then apply other optimizations later, that's certainly
a valid result.  I've never been fond of the "git repack -A" behavior
where it can generate huge numbers of loose files.  I'd much prefer it
if the other objects ended up in a separate pack file, and then some
other provision made for nuking that pack file some time later.  But
that's expanding the scope significantly over what's currently being
discussed.

- Ted

Re: [PATCH 00/11] Some fixes and bunch of object_id conversions

[brian m. carlson]

On Thu, Jan 18, 2018 at 03:50:52PM +0100, Patryk Obara wrote:
> * brian m. carlson implemented vtable for hash algorithm selection and pushed
> the repository format front - thanks to him it's now quite easy to
> experimentally replace hashing functions and, e.g. do some performance 
> testing.

Good, I'm glad this has been helpful.

> Patch 1 is not directly related to object_id conversions but helped with
> debugging t5540, which kept failing on master for me (spoiler: it was Fedora
> fault).  It helps with debugging of failing git-push over HTTP in case of
> internal server error, so I think it might be worthwhile.
> 
> Patch 2 is a small adjustment to .clang-format, which prevents unnecessary
> line breaks after function return type.

I have no strong opinions about these two patches, but didn't see
anything that looked problematic.  Better debugging is always nice.

> Patch 6 is a tiny fix in oidclr function.

I think this is a good direction to go in.

> All other patches are progressive conversions to struct object_id with some
> formatting fixes sprinkled in. These should be somewhat uncontroversial, I 
> hope.

Overall, I like the direction this series is going.

When I've made changes to the sha1_file functions, I've traditionally
moved them away from using "sha1_file" to "object_file" to ensure that
we make it a bit more obvious that they handle object_id structs and
aren't limited to SHA-1.  For consistency, it might be nice to make that
change.

Other than that and the question I had about the formatting, I think the
series looks good.
-- 
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 v2 2/6] Add tar extract install options override in installation processing.

[Randall S. Becker]

On January 20, 2018 3:34 PM, Ævar Arnfjörð Bjarmason
> On Sat, Jan 20 2018, Randall S. Becker jotted:
> 
> > I will reissue this particular patch shortly.
> 
> It makes sense to base that submission on the next branch instead of master.
> I have a patch queued up there which adds two new tar invocations.
> 
> Also re your commit message see the formatting guide in
> Documentation/SubmittingPatches, in particular: instead of:
> 
>  - Add a brief subject line
> 
>  - Just make the body be a normal paragraph instead of a bullet-point
>list with one item.

Got it. I've been swapping between contribution styles so got a little 
confused. I'm going to reissue all of the patches required for the NonStop port 
later tonight or tomorrow, once the test suite run is finished. I'll take 
everyone's comments into account for that. Thanks for your (and everyone 
else's) guidance.

Cheers,
Randall

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

Re: [PATCH 11/11] sha1_file: convert write_sha1_file to object_id

[brian m. carlson]

On Thu, Jan 18, 2018 at 03:51:03PM +0100, Patryk Obara wrote:
> diff --git a/sha1_file.c b/sha1_file.c
> index 88b960316c..b7baf69041 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1420,8 +1420,8 @@ void *read_object_with_reference(const unsigned char 
> *sha1,
>  }
>  
>  static void write_sha1_file_prepare(const void *buf, unsigned long len,
> -const char *type, unsigned char *sha1,
> -char *hdr, int *hdrlen)
> + const char *type, struct object_id *oid,
> + char *hdr, int *hdrlen)

It looks like the indentation has changed here.  Was that intentional?
-- 
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 v2 2/6] Add tar extract install options override in installation processing.

[Ævar Arnfjörð Bjarmason]

On Sat, Jan 20 2018, Randall S. Becker jotted:

> I will reissue this particular patch shortly.

It makes sense to base that submission on the next branch instead of
master. I have a patch queued up there which adds two new tar
invocations.

Also re your commit message see the formatting guide in
Documentation/SubmittingPatches, in particular: instead of:

 - Add a brief subject line

 - Just make the body be a normal paragraph instead of a bullet-point
   list with one item.

[PATCH] t: add clone test for files differing only in case

[brian m. carlson]

We recently introduced a regression in cloning repositories onto
case-insensitive file systems where the repository contains multiple
files differing only in case.  In such a case, we would segfault.  This
segfault has already been fixed (repository: pre-initialize hash algo
pointer), but it's not the first time similar problems have arisen.
Introduce a test to catch this case and ensure the behavior does not
regress.

Signed-off-by: Eric Sunshine 
Signed-off-by: brian m. carlson 
---
 t/t5601-clone.sh | 13 +
 1 file changed, 13 insertions(+)

I've verified that the test does fail without the patch on a vfat file
system.  However, many other tests also fail on a vfat file system on
Linux, so unfortunately that doesn't look like a viable testing strategy
going forward.

I didn't include an object ID for the commit referenced simply because I
didn't see one yet and I didn't want to insert a local one that wouldn't
work for anyone else.

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 0f895478f0..53b2dda9d2 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -611,4 +611,17 @@ test_expect_success 'GIT_TRACE_PACKFILE produces a usable 
pack' '
git -C replay.git index-pack -v --stdin 

RE: [PATCH v2 2/6] Add tar extract install options override in installation processing.

[Randall S. Becker]

On January 20, 2018 9:25 AM, René Scharfe wrote:
> To: Randall S. Becker ; git@vger.kernel.org
> Subject: Re: [PATCH v2 2/6] Add tar extract install options override in
> installation processing.
> 
> Am 20.01.2018 um 14:44 schrieb Randall S. Becker:
> > On January 20, 2018 7:31 AM, René Scharfe wrote:
> >> Am 19.01.2018 um 18:34 schrieb randall.s.bec...@rogers.com:
> >>> From: "Randall S. Becker" 
> >>>
> >>> * Makefile: Add TAR_EXTRACT_OPTIONS to allow platform options to be
> >>> specified if needed. The default is xof.
> >>>
> >>> Signed-off-by: Randall S. Becker  ---
> >>> Makefile | 6 +- 1 file changed, 5 insertions(+), 1
> >>> deletion(-)
> >>>
> >>> diff --git a/Makefile b/Makefile index 1a9b23b67..040e9eacd
> >>> 100644 --- a/Makefile +++ b/Makefile @@ -429,6 +429,9 @@ all:: #
> >>> running the test scripts (e.g., bash has better support for "set -x"
> >>> # tracing). # +# Define TAR_EXTRACT_OPTIONS if you want to change
> >>> the default +behaviour # from xvf to something else during
> >>> installation.
> >>
> >> "xof" instead of "xvf"?
> >
> > When I look at the parent commit, it says xof, so I wanted to preserve
> > existing behaviour by default. Our install process wants to see the
> > actual set of files, so we wanted to use xvof but that hardly seemed
> > of general interest. I was hoping an option to control it would be
> > agreeable.
> 
> Preserving the default is good. I meant that the default is "xof", but the
> added line implies it was "xvf" instead.
> 
> Seeing your actual use case confirms that my suggestion below would work
> for you.
> 
> >
> >>> +# # When cross-compiling, define HOST_CPU as the canonical name
> >>> of the
> >> CPU on
> >>> # which the built Git will run (for instance "x86_64").
> >>>
> >>> @@ -452,6 +455,7 @@ LDFLAGS = ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
> >>> ALL_LDFLAGS = $(LDFLAGS) STRIP ?= strip +TAR_EXTRACT_OPTIONS = xof
> >>>
> >>> # Create as necessary, replace existing, make ranlib unneeded.
> >>> ARFLAGS = rcs @@ -2569,7 +2573,7 @@ install: all ifndef NO_GETTEXT
> >>> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
> >>> (cd po/build/locale && $(TAR) cf - .) | \ -   (cd
> >>> '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xof -) + (cd
> >>> '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR)
> >>> +$(TAR_EXTRACT_OPTIONS) -)
> >>
> >> Hmm.  TAR_EXTRACT_OPTIONS always needs to have f (or -f, or --file)
> >> at the end to go together with the following dash, meaning to extract
> >> from stdin. And x (or -x, or --extract) is probably needed in all
> >> cases as well.  So wouldn't it make more sense to only put the o (or
> >> -o, or --no-same-owner) into TAR_EXTRACT_OPTIONS and enforce x and
> f?
> >
> > This is a good suggestion, and I'd love to do that, if I could
> > guarantee a modern tar, which I can't. The platform comes with a
> > really old-school tar from some old (seemingly BSD4.3) epoch that only
> > takes one option set. There is a more modern tar that can be
> > optionally installed if the sysadmin decides to that takes a slightly
> > more modern set, which could support your request, and my team also
> > has a gnu port that is very modern. I can't control what customers are
> > choosing to have installed, unfortunately. Your point is well made and
> > I am completely on board with it, but it introduces a configuration
> > requirement.
> 
> Long options would be nice to nice to have, but are not my main point; I
> included them mainly to spare readers from firing up "man tar" to look up
> the meaning of the short ones.
> 
> I just meant to say that something like this here would make more sense
> because you always need x and f- anyway:
> 
>   TAR_EXTRACT_OPTIONS = o
> 
>   ... ${TAR} x${TAR_EXTRACT_OPTIONS}f -
> 
> > As with the broadening setbuf (patch 2/6) change, I would like to
> > consider this for the future, having a slightly different more complex
> > idea. I could introduce something like this:
> >
> > 1. HAS_ANCIENT_TAR=UnfortunatelyYes in config.mak.uname that disables
> > this capability all together 2. HAS_ANCIENT_TAR=AreYouKiddingMe
> > (joke) then set up TAR_EXTRACT_ADDITIONAL_OPTIONS above and beyond
> the
> > default, so --file, --no-same-owner would always be in effect for that
> > operation.
> >
> > The micro-project would also, logically, need to apply to other tar
> > occurrences throughout the code and potentially need a test case
> > written for it (not entirely sure what that would test, yet).
> > Is that a reasonable approach?
> 
> As long as old-school dash-less flags suffice for our purposes (including
> yours) we can just keep using that style everywhere and avoid adding more
> settings.  It would be a different matter if we needed features that have no
> short flag, or are only offered by certain tar implementations.

Points taken. I will reissue this particular patch shortly.

[PATCH v4 6/6] convert: add tracing for 'working-tree-encoding' attribute

[lars . schneider]

From: Lars Schneider 

Add the GIT_TRACE_CHECKOUT_ENCODING environment variable to enable
tracing for content that is reencoded with the 'working-tree-encoding'
attribute. This is useful to debug encoding issues.

Signed-off-by: Lars Schneider 
---
 convert.c| 28 
 t/t0028-working-tree-encoding.sh |  2 ++
 2 files changed, 30 insertions(+)

diff --git a/convert.c b/convert.c
index 0c372069b1..13fad490ce 100644
--- a/convert.c
+++ b/convert.c
@@ -266,6 +266,29 @@ static int will_convert_lf_to_crlf(size_t len, struct 
text_stat *stats,
 
 }
 
+static void trace_encoding(const char *context, const char *path,
+  const char *encoding, const char *buf, size_t len)
+{
+   static struct trace_key coe = TRACE_KEY_INIT(CHECKOUT_ENCODING);
+   struct strbuf trace = STRBUF_INIT;
+   int i;
+
+   strbuf_addf(, "%s (%s, considered %s):\n", context, path, 
encoding);
+   for (i = 0; i < len && buf; ++i) {
+   strbuf_addf(
+   ,"| \e[2m%2i:\e[0m %2x \e[2m%c\e[0m%c",
+   i,
+   (unsigned char) buf[i],
+   (buf[i] > 32 && buf[i] < 127 ? buf[i] : ' '),
+   ((i+1) % 8 && (i+1) < len ? ' ' : '\n')
+   );
+   }
+   strbuf_addchars(, '\n', 1);
+
+   trace_strbuf(, );
+   strbuf_release();
+}
+
 static struct encoding {
const char *name;
struct encoding *next;
@@ -325,6 +348,7 @@ static int encode_to_git(const char *path, const char *src, 
size_t src_len,
error(error_msg, path, enc->name);
}
 
+   trace_encoding("source", path, enc->name, src, src_len);
dst = reencode_string_len(src, src_len, default_encoding, enc->name,
  _len);
if (!dst) {
@@ -340,6 +364,7 @@ static int encode_to_git(const char *path, const char *src, 
size_t src_len,
else
error(msg, path, enc->name, default_encoding);
}
+   trace_encoding("destination", path, default_encoding, dst, dst_len);
 
/*
 * UTF supports lossless round tripping [1]. UTF to other encoding are
@@ -365,6 +390,9 @@ static int encode_to_git(const char *path, const char *src, 
size_t src_len,
 enc->name, default_encoding,
 _src_len);
 
+   trace_encoding("reencoded source", path, enc->name,
+  re_src, re_src_len);
+
if (!re_src || src_len != re_src_len ||
memcmp(src, re_src, src_len)) {
const char* msg = _("encoding '%s' from %s to %s and "
diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh
index 4d85b42776..0f36d4990a 100755
--- a/t/t0028-working-tree-encoding.sh
+++ b/t/t0028-working-tree-encoding.sh
@@ -4,6 +4,8 @@ test_description='working-tree-encoding conversion via 
gitattributes'
 
 . ./test-lib.sh
 
+GIT_TRACE_CHECKOUT_ENCODING=1 && export GIT_TRACE_CHECKOUT_ENCODING
+
 test_expect_success 'setup test repo' '
git config core.eol lf &&
 
-- 
2.16.0

[PATCH v4 4/6] utf8: add function to detect a missing UTF-16/32 BOM

[lars . schneider]

From: Lars Schneider 

If the endianness is not defined in the encoding name, then let's
be strict and require a BOM to avoid any encoding confusion. The
has_missing_utf_bom() function returns true if a required BOM is
missing.

The Unicode standard instructs to assume big-endian if there in no BOM
for UTF-16/32 [1][2]. However, the W3C/WHATWG encoding standard used
in HTML5 recommends to assume little-endian to "deal with deployed
content" [3]. Strictly requiring a BOM seems to be the safest option
for content in Git.

This function is used in a subsequent commit.

[1] http://unicode.org/faq/utf_bom.html#gen6
[2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf
 Section 3.10, D98, page 132
[3] https://encoding.spec.whatwg.org/#utf-16le

Signed-off-by: Lars Schneider 
---
 utf8.c | 13 +
 utf8.h | 16 
 2 files changed, 29 insertions(+)

diff --git a/utf8.c b/utf8.c
index 914881cd1f..f033fec1c2 100644
--- a/utf8.c
+++ b/utf8.c
@@ -562,6 +562,19 @@ int has_prohibited_utf_bom(const char *enc, const char 
*data, size_t len)
);
 }
 
+int has_missing_utf_bom(const char *enc, const char *data, size_t len)
+{
+   return (
+  !strcmp(enc, "UTF-16") &&
+  !(has_bom_prefix(data, len, utf16_be_bom, sizeof(utf16_be_bom)) ||
+has_bom_prefix(data, len, utf16_le_bom, sizeof(utf16_le_bom)))
+   ) || (
+  !strcmp(enc, "UTF-32") &&
+  !(has_bom_prefix(data, len, utf32_be_bom, sizeof(utf32_be_bom)) ||
+has_bom_prefix(data, len, utf32_le_bom, sizeof(utf32_le_bom)))
+   );
+}
+
 /*
  * Returns first character length in bytes for multi-byte `text` according to
  * `encoding`.
diff --git a/utf8.h b/utf8.h
index 4711429af9..26b5e91852 100644
--- a/utf8.h
+++ b/utf8.h
@@ -79,4 +79,20 @@ void strbuf_utf8_align(struct strbuf *buf, align_type 
position, unsigned int wid
  */
 int has_prohibited_utf_bom(const char *enc, const char *data, size_t len);
 
+/*
+ * If the endianness is not defined in the encoding name, then we
+ * require a BOM. The function returns true if a required BOM is missing.
+ *
+ * The Unicode standard instructs to assume big-endian if there
+ * in no BOM for UTF-16/32 [1][2]. However, the W3C/WHATWG
+ * encoding standard used in HTML5 recommends to assume
+ * little-endian to "deal with deployed content" [3].
+ *
+ * [1] http://unicode.org/faq/utf_bom.html#gen6
+ * [2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf
+ * Section 3.10, D98, page 132
+ * [3] https://encoding.spec.whatwg.org/#utf-16le
+ */
+int has_missing_utf_bom(const char *enc, const char *data, size_t len);
+
 #endif
-- 
2.16.0

[PATCH v4 3/6] utf8: add function to detect prohibited UTF-16/32 BOM

[lars . schneider]

From: Lars Schneider 

Whenever a data stream is declared to be UTF-16BE, UTF-16LE, UTF-32BE
or UTF-32LE a BOM must not be used [1]. The function returns true if
this is the case.

This function is used in a subsequent commit.

[1] http://unicode.org/faq/utf_bom.html#bom10

Signed-off-by: Lars Schneider 
---
 utf8.c | 24 
 utf8.h |  9 +
 2 files changed, 33 insertions(+)

diff --git a/utf8.c b/utf8.c
index 2c27ce0137..914881cd1f 100644
--- a/utf8.c
+++ b/utf8.c
@@ -538,6 +538,30 @@ char *reencode_string_len(const char *in, int insz,
 }
 #endif
 
+static int has_bom_prefix(const char *data, size_t len,
+ const char *bom, size_t bom_len)
+{
+   return (len >= bom_len) && !memcmp(data, bom, bom_len);
+}
+
+static const char utf16_be_bom[] = {0xFE, 0xFF};
+static const char utf16_le_bom[] = {0xFF, 0xFE};
+static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};
+static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};
+
+int has_prohibited_utf_bom(const char *enc, const char *data, size_t len)
+{
+   return (
+ (!strcmp(enc, "UTF-16BE") || !strcmp(enc, "UTF-16LE")) &&
+ (has_bom_prefix(data, len, utf16_be_bom, sizeof(utf16_be_bom)) ||
+  has_bom_prefix(data, len, utf16_le_bom, sizeof(utf16_le_bom)))
+   ) || (
+ (!strcmp(enc, "UTF-32BE") || !strcmp(enc, "UTF-32LE")) &&
+ (has_bom_prefix(data, len, utf32_be_bom, sizeof(utf32_be_bom)) ||
+  has_bom_prefix(data, len, utf32_le_bom, sizeof(utf32_le_bom)))
+   );
+}
+
 /*
  * Returns first character length in bytes for multi-byte `text` according to
  * `encoding`.
diff --git a/utf8.h b/utf8.h
index 6bbcf31a83..4711429af9 100644
--- a/utf8.h
+++ b/utf8.h
@@ -70,4 +70,13 @@ typedef enum {
 void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int 
width,
   const char *s);
 
+/*
+ * Whenever a data stream is declared to be UTF-16BE, UTF-16LE, UTF-32BE
+ * or UTF-32LE a BOM must not be used [1]. The function returns true if
+ * this is the case.
+ *
+ * [1] http://unicode.org/faq/utf_bom.html#bom10
+ */
+int has_prohibited_utf_bom(const char *enc, const char *data, size_t len);
+
 #endif
-- 
2.16.0

[PATCH v4 5/6] convert: add 'working-tree-encoding' attribute

[lars . schneider]

From: Lars Schneider 

Git recognizes files encoded with ASCII or one of its supersets (e.g.
UTF-8 or ISO-8859-1) as text files. All other encodings are usually
interpreted as binary and consequently built-in Git text processing
tools (e.g. 'git diff') as well as most Git web front ends do not
visualize the content.

Add an attribute to tell Git what encoding the user has defined for a
given file. If the content is added to the index, then Git converts the
content to a canonical UTF-8 representation. On checkout Git will
reverse the conversion.

Signed-off-by: Lars Schneider 
---
 Documentation/gitattributes.txt  |  60 
 convert.c| 190 -
 convert.h|   1 +
 sha1_file.c  |   2 +-
 t/t0028-working-tree-encoding.sh | 196 +++
 5 files changed, 447 insertions(+), 2 deletions(-)
 create mode 100755 t/t0028-working-tree-encoding.sh

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 30687de81a..a8dbf4be30 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -272,6 +272,66 @@ few exceptions.  Even though...
   catch potential problems early, safety triggers.
 
 
+`working-tree-encoding`
+^^^
+
+Git recognizes files encoded with ASCII or one of its supersets (e.g.
+UTF-8 or ISO-8859-1) as text files.  All other encodings are usually
+interpreted as binary and consequently built-in Git text processing
+tools (e.g. 'git diff') as well as most Git web front ends do not
+visualize the content.
+
+In these cases you can tell Git the encoding of a file in the working
+directory with the `working-tree-encoding` attribute. If a file with this
+attributes is added to Git, then Git reencodes the content from the
+specified encoding to UTF-8 and stores the result in its internal data
+structure (called "the index"). On checkout the content is encoded
+back to the specified encoding.
+
+Please note that using the `working-tree-encoding` attribute may have a
+number of pitfalls:
+
+- Git clients that do not support the `working-tree-encoding` attribute
+  will checkout the respective files UTF-8 encoded and not in the
+  expected encoding. Consequently, these files will appear different
+  which typically causes trouble. This is in particular the case for
+  older Git versions and alternative Git implementations such as JGit
+  or libgit2 (as of January 2018).
+
+- Reencoding content to non-UTF encodings (e.g. SHIFT-JIS) can cause
+  errors as the conversion might not be round trip safe.
+
+- Reencoding content requires resources that might slow down certain
+  Git operations (e.g 'git checkout' or 'git add').
+
+Use the `working-tree-encoding` attribute only if you cannot store a file in
+UTF-8 encoding and if you want Git to be able to process the content as
+text.
+
+Use the following attributes if your '*.txt' files are UTF-16 encoded
+with byte order mark (BOM) and you want Git to perform automatic line
+ending conversion based on your platform.
+
+
+*.txt  text working-tree-encoding=UTF-16
+
+
+Use the following attributes if your '*.txt' files are UTF-16 little
+endian encoded without BOM and you want Git to use Windows line endings
+in the working directory.
+
+
+*.txt  working-tree-encoding=UTF-16LE text eol=CRLF
+
+
+You can get a list of all available encodings on your platform with the
+following command:
+
+
+iconv --list
+
+
+
 `ident`
 ^^^
 
diff --git a/convert.c b/convert.c
index b976eb968c..0c372069b1 100644
--- a/convert.c
+++ b/convert.c
@@ -7,6 +7,7 @@
 #include "sigchain.h"
 #include "pkt-line.h"
 #include "sub-process.h"
+#include "utf8.h"
 
 /*
  * convert.c - convert a file when checking it out and checking it in.
@@ -265,6 +266,147 @@ static int will_convert_lf_to_crlf(size_t len, struct 
text_stat *stats,
 
 }
 
+static struct encoding {
+   const char *name;
+   struct encoding *next;
+} *encoding, **encoding_tail;
+static const char *default_encoding = "UTF-8";
+
+static int encode_to_git(const char *path, const char *src, size_t src_len,
+struct strbuf *buf, struct encoding *enc, int 
conv_flags)
+{
+   char *dst;
+   int dst_len;
+
+   /*
+* No encoding is specified or there is nothing to encode.
+* Tell the caller that the content was not modified.
+*/
+   if (!enc || (src && !src_len))
+   return 0;
+
+   /*
+* Looks like we got called from "would_convert_to_git()".
+* This means Git wants to know if it would encode (= modify!)
+* the content. Let's answer with "yes", since an encoding was
+* specified.
+*/
+   if (!buf 

[PATCH v4 2/6] strbuf: add xstrdup_toupper()

[lars . schneider]

From: Lars Schneider 

Create a copy of an existing string and make all characters upper case.
Similar xstrdup_tolower().

This function is used in a subsequent commit.

Signed-off-by: Lars Schneider 
---
 strbuf.c | 12 
 strbuf.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/strbuf.c b/strbuf.c
index 55b7daeb35..b635f0bdc4 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -784,6 +784,18 @@ char *xstrdup_tolower(const char *string)
return result;
 }
 
+char *xstrdup_toupper(const char *string)
+{
+   char *result;
+   size_t len, i;
+
+   len = strlen(string);
+   result = xmallocz(len);
+   for (i = 0; i < len; i++)
+   result[i] = toupper(string[i]);
+   return result;
+}
+
 char *xstrvfmt(const char *fmt, va_list ap)
 {
struct strbuf buf = STRBUF_INIT;
diff --git a/strbuf.h b/strbuf.h
index 14c8c10d66..df7ced53ed 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -607,6 +607,7 @@ __attribute__((format (printf,2,3)))
 extern int fprintf_ln(FILE *fp, const char *fmt, ...);
 
 char *xstrdup_tolower(const char *);
+char *xstrdup_toupper(const char *);
 
 /**
  * Create a newly allocated string using printf format. You can do this easily
-- 
2.16.0

[PATCH v4 1/6] strbuf: remove unnecessary NUL assignment in xstrdup_tolower()

[lars . schneider]

From: Lars Schneider 

Since 3733e69464 (use xmallocz to avoid size arithmetic, 2016-02-22) we
allocate the buffer for the lower case string with xmallocz(). This
already ensures a NUL at the end of the allocated buffer.

Remove the unnecessary assignment.

Signed-off-by: Lars Schneider 
---
 strbuf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/strbuf.c b/strbuf.c
index 1df674e919..55b7daeb35 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -781,7 +781,6 @@ char *xstrdup_tolower(const char *string)
result = xmallocz(len);
for (i = 0; i < len; i++)
result[i] = tolower(string[i]);
-   result[i] = '\0';
return result;
 }
 
-- 
2.16.0

[PATCH v4 0/6] convert: add support for different encodings

[lars . schneider]

From: Lars Schneider 

Hi,

Patches 1-4 and 6 are preparation and helper functions.
Patch 5 is the actual change.

This series depends on Torsten's "convert_to_git(): safe_crlf/checksafe
becomes int conv_flags" patch:
https://public-inbox.org/git/20180113224931.27031-1-tbo...@web.de/

Changes since v3:

* I renamed the attribute from "checkout-encoding" to "working-tree-encoding"
  in the hope to convey better what the attribute is about.

* I rebased the series to Git 2.16 and removed Torsten's patch as he
  posted the patch on his own.

* Fix documentation wording. (Torsten)

* A macro was used in a commit before it's introduction. Fixed!(Junio)

Thanks,
Lars

   RFC: 
https://public-inbox.org/git/bdb9b884-6d17-4be3-a83c-f67e2afa2...@gmail.com/
v1: 
https://public-inbox.org/git/20171211155023.1405-1-lars.schnei...@autodesk.com/
v2: 
https://public-inbox.org/git/2017122915.39680-1-lars.schnei...@autodesk.com/
v3: 
https://public-inbox.org/git/20180106004808.77513-1-lars.schnei...@autodesk.com/


Base Ref:
Web-Diff: https://github.com/larsxschneider/git/commit/21f4dac5ab
Checkout: git fetch https://github.com/larsxschneider/git encoding-v4 && git 
checkout 21f4dac5ab


### Interdiff (v3-rebased-2.16..v4):

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 1bc03e69cb..a8dbf4be30 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -272,8 +272,8 @@ few exceptions.  Even though...
   catch potential problems early, safety triggers.


-`checkout-encoding`
-^^^
+`working-tree-encoding`
+^^^

 Git recognizes files encoded with ASCII or one of its supersets (e.g.
 UTF-8 or ISO-8859-1) as text files.  All other encodings are usually
@@ -281,17 +281,17 @@ interpreted as binary and consequently built-in Git text 
processing
 tools (e.g. 'git diff') as well as most Git web front ends do not
 visualize the content.

-In these cases you can teach Git the encoding of a file in the working
-directory with the `checkout-encoding` attribute. If a file with this
+In these cases you can tell Git the encoding of a file in the working
+directory with the `working-tree-encoding` attribute. If a file with this
 attributes is added to Git, then Git reencodes the content from the
 specified encoding to UTF-8 and stores the result in its internal data
 structure (called "the index"). On checkout the content is encoded
 back to the specified encoding.

-Please note that using the `checkout-encoding` attribute may have a
+Please note that using the `working-tree-encoding` attribute may have a
 number of pitfalls:

-- Git clients that do not support the `checkout-encoding` attribute
+- Git clients that do not support the `working-tree-encoding` attribute
   will checkout the respective files UTF-8 encoded and not in the
   expected encoding. Consequently, these files will appear different
   which typically causes trouble. This is in particular the case for
@@ -304,7 +304,7 @@ number of pitfalls:
 - Reencoding content requires resources that might slow down certain
   Git operations (e.g 'git checkout' or 'git add').

-Use the `checkout-encoding` attribute only if you cannot store a file in
+Use the `working-tree-encoding` attribute only if you cannot store a file in
 UTF-8 encoding and if you want Git to be able to process the content as
 text.

@@ -313,7 +313,7 @@ with byte order mark (BOM) and you want Git to perform 
automatic line
 ending conversion based on your platform.

 
-*.txttext checkout-encoding=UTF-16
+*.txttext working-tree-encoding=UTF-16
 

 Use the following attributes if your '*.txt' files are UTF-16 little
@@ -321,7 +321,7 @@ endian encoded without BOM and you want Git to use Windows 
line endings
 in the working directory.

 
-*.txtcheckout-encoding=UTF-16LE text eol=CRLF
+*.txtworking-tree-encoding=UTF-16LE text eol=CRLF
 

 You can get a list of all available encodings on your platform with the
diff --git a/convert.c b/convert.c
index 8559651b3f..13fad490ce 100644
--- a/convert.c
+++ b/convert.c
@@ -323,7 +323,7 @@ static int encode_to_git(const char *path, const char *src, 
size_t src_len,
const char *advise_msg = _(
  "You told Git to treat '%s' as %s. A byte order mark "
  "(BOM) is prohibited with this encoding. Either use "
- "%.6s as checkout encoding or remove the BOM from the "
+ "%.6s as working tree encoding or remove the BOM from the "
  "file.");

advise(advise_msg, path, enc->name, enc->name, enc->name);
@@ -339,7 +339,7 @@ static int encode_to_git(const char *path, const char *src, 
size_t src_len,
const char *advise_msg = _(
  "You told Git to treat '%s' as %s. A byte order mark "
  "(BOM) is required with this encoding. Either use "
- "%sBE/%sLE as checkout encoding or add a BOM to the "
+ 

Re: [PATCH v2 2/6] Add tar extract install options override in installation processing.

[René Scharfe]

Am 20.01.2018 um 14:44 schrieb Randall S. Becker:
> On January 20, 2018 7:31 AM, René Scharfe wrote:
>> Am 19.01.2018 um 18:34 schrieb randall.s.bec...@rogers.com:
>>> From: "Randall S. Becker" 
>>> 
>>> * Makefile: Add TAR_EXTRACT_OPTIONS to allow platform options to
>>> be specified if needed. The default is xof.
>>> 
>>> Signed-off-by: Randall S. Becker  --- 
>>> Makefile | 6 +- 1 file changed, 5 insertions(+), 1
>>> deletion(-)
>>> 
>>> diff --git a/Makefile b/Makefile index 1a9b23b67..040e9eacd
>>> 100644 --- a/Makefile +++ b/Makefile @@ -429,6 +429,9 @@ all:: #
>>> running the test scripts (e.g., bash has better support for "set
>>> -x" # tracing). # +# Define TAR_EXTRACT_OPTIONS if you want to
>>> change the default +behaviour # from xvf to something else during
>>> installation.
>> 
>> "xof" instead of "xvf"?
> 
> When I look at the parent commit, it says xof, so I wanted to
> preserve existing behaviour by default. Our install process wants to
> see the actual set of files, so we wanted to use xvof but that hardly
> seemed of general interest. I was hoping an option to control it
> would be agreeable.

Preserving the default is good. I meant that the default is "xof",
but the added line implies it was "xvf" instead.

Seeing your actual use case confirms that my suggestion below would
work for you.

> 
>>> +# # When cross-compiling, define HOST_CPU as the canonical name
>>> of the
>> CPU on
>>> # which the built Git will run (for instance "x86_64").
>>> 
>>> @@ -452,6 +455,7 @@ LDFLAGS = ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) 
>>> ALL_LDFLAGS = $(LDFLAGS) STRIP ?= strip +TAR_EXTRACT_OPTIONS =
>>> xof
>>> 
>>> # Create as necessary, replace existing, make ranlib unneeded. 
>>> ARFLAGS = rcs @@ -2569,7 +2573,7 @@ install: all ifndef
>>> NO_GETTEXT $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)' 
>>> (cd po/build/locale && $(TAR) cf - .) | \ - (cd
>>> '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xof -) +
>>> (cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) 
>>> +$(TAR_EXTRACT_OPTIONS) -)
>> 
>> Hmm.  TAR_EXTRACT_OPTIONS always needs to have f (or -f, or --file)
>> at the end to go together with the following dash, meaning to
>> extract from stdin. And x (or -x, or --extract) is probably needed
>> in all cases as well.  So wouldn't it make more sense to only put
>> the o (or -o, or --no-same-owner) into TAR_EXTRACT_OPTIONS and
>> enforce x and f?
> 
> This is a good suggestion, and I'd love to do that, if I could
> guarantee a modern tar, which I can't. The platform comes with a
> really old-school tar from some old (seemingly BSD4.3) epoch that
> only takes one option set. There is a more modern tar that can be
> optionally installed if the sysadmin decides to that takes a slightly
> more modern set, which could support your request, and my team also
> has a gnu port that is very modern. I can't control what customers
> are choosing to have installed, unfortunately. Your point is well
> made and I am completely on board with it, but it introduces a
> configuration requirement.

Long options would be nice to nice to have, but are not my main
point; I included them mainly to spare readers from firing up
"man tar" to look up the meaning of the short ones.

I just meant to say that something like this here would make more
sense because you always need x and f- anyway:

TAR_EXTRACT_OPTIONS = o

... ${TAR} x${TAR_EXTRACT_OPTIONS}f -

> As with the broadening setbuf (patch 2/6) change, I would like to
> consider this for the future, having a slightly different more
> complex idea. I could introduce something like this:
> 
> 1. HAS_ANCIENT_TAR=UnfortunatelyYes in config.mak.uname that disables
> this capability all together 2. HAS_ANCIENT_TAR=AreYouKiddingMe
> (joke) then set up TAR_EXTRACT_ADDITIONAL_OPTIONS above and beyond
> the default, so --file, --no-same-owner would always be in effect for
> that operation.
> 
> The micro-project would also, logically, need to apply to other tar
> occurrences throughout the code and potentially need a test case
> written for it (not entirely sure what that would test, yet).
> Is that a reasonable approach?

As long as old-school dash-less flags suffice for our purposes
(including yours) we can just keep using that style everywhere and
avoid adding more settings.  It would be a different matter if we
needed features that have no short flag, or are only offered by
certain tar implementations.

René

RE:

[Mr Sheng Li Hung]

I am Mr.Sheng Li Hung, from china I got your information while search for
a reliable person, I have a very profitable business proposition for you
and i can assure you that you will not regret been part of this mutual
beneficial transaction after completion. Kindly get back to me for more
details on this email id: shengl...@hotmail.com

Thanks
Mr Sheng Li Hung

RE: [PATCH v2 2/6] Add tar extract install options override in installation processing.

[Randall S. Becker]

On January 20, 2018 7:31 AM, René Scharfe wrote:
> Am 19.01.2018 um 18:34 schrieb randall.s.bec...@rogers.com:
> > From: "Randall S. Becker" 
> >
> > * Makefile: Add TAR_EXTRACT_OPTIONS to allow platform options to be
> >specified if needed. The default is xof.
> >
> > Signed-off-by: Randall S. Becker 
> > ---
> >   Makefile | 6 +-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 1a9b23b67..040e9eacd 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -429,6 +429,9 @@ all::
> >   # running the test scripts (e.g., bash has better support for "set -x"
> >   # tracing).
> >   #
> > +# Define TAR_EXTRACT_OPTIONS if you want to change the default
> > +behaviour # from xvf to something else during installation.
> 
> "xof" instead of "xvf"?

When I look at the parent commit, it says xof, so I wanted to preserve existing 
behaviour by default. Our install process wants to see the actual set of files, 
so we wanted to use xvof but that hardly seemed of general interest. I was 
hoping an option to control it would be agreeable.

> > +#
> >   # When cross-compiling, define HOST_CPU as the canonical name of the
> CPU on
> >   # which the built Git will run (for instance "x86_64").
> >
> > @@ -452,6 +455,7 @@ LDFLAGS =
> >   ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
> >   ALL_LDFLAGS = $(LDFLAGS)
> >   STRIP ?= strip
> > +TAR_EXTRACT_OPTIONS = xof
> >
> >   # Create as necessary, replace existing, make ranlib unneeded.
> >   ARFLAGS = rcs
> > @@ -2569,7 +2573,7 @@ install: all
> >   ifndef NO_GETTEXT
> > $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
> > (cd po/build/locale && $(TAR) cf - .) | \
> > -   (cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xof -)
> > +   (cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR)
> > +$(TAR_EXTRACT_OPTIONS) -)
> 
> Hmm.  TAR_EXTRACT_OPTIONS always needs to have f (or -f, or --file) at the
> end to go together with the following dash, meaning to extract from stdin.
> And x (or -x, or --extract) is probably needed in all cases as well.  So 
> wouldn't
> it make more sense to only put the o (or -o, or
> --no-same-owner) into TAR_EXTRACT_OPTIONS and enforce x and f?

This is a good suggestion, and I'd love to do that, if I could guarantee a 
modern tar, which I can't. The platform comes with a really old-school tar from 
some old (seemingly BSD4.3) epoch that only takes one option set. There is a 
more modern tar that can be optionally installed if the sysadmin decides to 
that takes a slightly more modern set, which could support your request, and my 
team also has a gnu port that is very modern. I can't control what customers 
are choosing to have installed, unfortunately. Your point is well made and I am 
completely on board with it, but it introduces a configuration requirement.

As with the broadening setbuf (patch 2/6) change, I would like to consider this 
for the future, having a slightly different more complex idea. I could 
introduce something like this:

1. HAS_ANCIENT_TAR=UnfortunatelyYes in config.mak.uname that disables this 
capability all together
2. HAS_ANCIENT_TAR=AreYouKiddingMe (joke) then set up 
TAR_EXTRACT_ADDITIONAL_OPTIONS above and beyond the default, so --file, 
--no-same-owner would always be in effect for that operation.

The micro-project would also, logically, need to apply to other tar occurrences 
throughout the code and potentially need a test case written for it (not 
entirely sure what that would test, yet).

Is that a reasonable approach?

> 
> >   endif
> >   ifndef NO_PERL
> > $(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)'
> > install
> >

RE: [PATCH v2 0/6] Force pipes to flush immediately on NonStop platform

[Randall S. Becker]

On January 20, 2018 6:10 AM,  Torsten Bögershausen wrote:
> On Fri, Jan 19, 2018 at 12:33:59PM -0500, randall.s.bec...@rogers.com
> wrote:
> > From: "Randall S. Becker" 
> >
> > * wrapper.c: called setbuf(stream,0) to force pipe flushes not enabled by
> >   default on the NonStop platform.
> >
> > Signed-off-by: Randall S. Becker 
> > ---
> >  wrapper.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/wrapper.c b/wrapper.c
> > index d20356a77..671cbb4b4 100644
> > --- a/wrapper.c
> > +++ b/wrapper.c
> > @@ -403,6 +403,9 @@ FILE *xfdopen(int fd, const char *mode)
> > FILE *stream = fdopen(fd, mode);
> > if (stream == NULL)
> > die_errno("Out of memory? fdopen failed");
> > +#ifdef __TANDEM
> > +   setbuf(stream,0);
> > +#endif
> 
> Reading the commit message, I would have expected someting similar to
> 
> #ifdef FORCE_PIPE_FLUSHES
>   setbuf(stream,0);
> #endif
> 
> (Because other systems may need the tweak as well, some day) Of course
> you need to change that in the Makefile and config.mak.uname
> 
> > return stream;
> >  }

I can definitely see the point. Would you be agreeable to expanding the scope 
of this as a separate patch after this one is applied? I would want to bring at 
least one more Not NonStop machine into the mix for testing, which is more than 
I can do this weekend .

Cheers,
Randall

Re: [PATCH v2 2/6] Add tar extract install options override in installation processing.

[René Scharfe]

Am 19.01.2018 um 18:34 schrieb randall.s.bec...@rogers.com:
> From: "Randall S. Becker" 
> 
> * Makefile: Add TAR_EXTRACT_OPTIONS to allow platform options to be
>specified if needed. The default is xof.
> 
> Signed-off-by: Randall S. Becker 
> ---
>   Makefile | 6 +-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 1a9b23b67..040e9eacd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -429,6 +429,9 @@ all::
>   # running the test scripts (e.g., bash has better support for "set -x"
>   # tracing).
>   #
> +# Define TAR_EXTRACT_OPTIONS if you want to change the default behaviour
> +# from xvf to something else during installation.

"xof" instead of "xvf"?

> +#
>   # When cross-compiling, define HOST_CPU as the canonical name of the CPU on
>   # which the built Git will run (for instance "x86_64").
>   
> @@ -452,6 +455,7 @@ LDFLAGS =
>   ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
>   ALL_LDFLAGS = $(LDFLAGS)
>   STRIP ?= strip
> +TAR_EXTRACT_OPTIONS = xof
>   
>   # Create as necessary, replace existing, make ranlib unneeded.
>   ARFLAGS = rcs
> @@ -2569,7 +2573,7 @@ install: all
>   ifndef NO_GETTEXT
>   $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
>   (cd po/build/locale && $(TAR) cf - .) | \
> - (cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xof -)
> + (cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) 
> $(TAR_EXTRACT_OPTIONS) -)

Hmm.  TAR_EXTRACT_OPTIONS always needs to have f (or -f, or --file) at
the end to go together with the following dash, meaning to extract from
stdin.  And x (or -x, or --extract) is probably needed in all cases as
well.  So wouldn't it make more sense to only put the o (or -o, or
--no-same-owner) into TAR_EXTRACT_OPTIONS and enforce x and f?

>   endif
>   ifndef NO_PERL
>   $(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
> 

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

[Thomas Gummerer]

On 01/19, Junio C Hamano wrote:
> Thomas Gummerer  writes:
> 
> > read_cache_from() defaults to using the gitdir of the_repository.  As it
> > is mostly a convenience macro, having to pass get_git_dir() for every
> > call seems overkill, and if necessary users can have more control by
> > using read_index_from().
> 
> 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?

> Please double check if I made the right git-dir to be passed to them
> when I push out 'pu' in a few hours.

I think one conversion was not quite correct, even though all tests
still pass with GIT_TEST_SPLIT_INDEX set.

The following diff fixes that conversation and has a test showing the
breakage:

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 6a49f9e628..4d86a3574f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -612,7 +612,8 @@ static void validate_no_submodules(const struct worktree 
*wt)
struct index_state istate = {0};
int i, found_submodules = 0;
 
-   if (read_index_from(, worktree_git_path(wt, "index"), 
get_git_dir()) > 0) {
+   if (read_index_from(, worktree_git_path(wt, "index"),
+   get_worktree_git_dir(wt)) > 0) {
for (i = 0; i < istate.cache_nr; i++) {
struct cache_entry *ce = istate.cache[i];
 
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index b3105eaaed..8faf61bbf5 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -90,6 +90,16 @@ test_expect_success 'move main worktree' '
test_must_fail git worktree move . def
 '
 
+test_expect_success 'move worktree with split index' '
+   git worktree add test &&
+   (
+   cd test &&
+   test_commit file &&
+   git update-index --split-index
+   ) &&
+   git worktree move test test-destination
+'
+
 test_expect_success 'remove main worktree' '
test_must_fail git worktree remove .
 '

With this applied what you have in pu looks good to me.  Does the
above help, or should I send this in another for for you to apply?

> Thanks.

Re: [PATCH v2 0/6] Force pipes to flush immediately on NonStop platform

[Torsten Bögershausen]

On Fri, Jan 19, 2018 at 12:33:59PM -0500, randall.s.bec...@rogers.com wrote:
> From: "Randall S. Becker" 
> 
> * wrapper.c: called setbuf(stream,0) to force pipe flushes not enabled by
>   default on the NonStop platform.
> 
> Signed-off-by: Randall S. Becker 
> ---
>  wrapper.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/wrapper.c b/wrapper.c
> index d20356a77..671cbb4b4 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -403,6 +403,9 @@ FILE *xfdopen(int fd, const char *mode)
>   FILE *stream = fdopen(fd, mode);
>   if (stream == NULL)
>   die_errno("Out of memory? fdopen failed");
> +#ifdef __TANDEM
> + setbuf(stream,0);
> +#endif

Reading the commit message, I would have expected someting similar to

#ifdef FORCE_PIPE_FLUSHES
setbuf(stream,0);
#endif

(Because other systems may need the tweak as well, some day)
Of course you need to change that in the Makefile and config.mak.uname

>   return stream;
>  }
>  
> -- 
> 2.16.0.31.gf1a482c
> 

Re: [PATCH] repository: pre-initialize hash algo pointer

[Eric Sunshine]

On Sat, Jan 20, 2018 at 2:01 AM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>> Eric Sunshine  writes:
>>> Now that we know (due to Duy's excellent detective work[1]) that the
>>> trigger is files with names differing only in case on case-insensitive
>>> filesystems, the commit message can be updated appropriately.
>>
>> Thanks.  Let me apply the following and do a 2.16.1, hopefully by
>> the end of day or mid tomorrow at the latest.  Test to protect the
>> fix can come as a separate follow-up patch.
>
> I actually changed my mind and decided not to rush 2.16.1 with just
> this change.  After all, even though it is better not to crash in
> such a problematic case, a "successful" clone of such a project on
> such a filesystem cannot be sanely used *anyway*, so in that sense
> the "fix" would stop "clone" from segfaulting but the resulting
> repository would still be "wrong" (e.g. "git status" immediately
> after clone would likely to say that the working tree is already
> dirty and missing one of the two paths, or something silly like
> that).

The counter-argument is that filenames differing only in case do not
necessarily render a project unusable on case-insensitive filesystems.
For instance, if the problematic files exist only in a project's test
suite or in some platform-specific resource directory, the project
itself may still be perfectly usable for a person cloning a project
merely to build and use it (not develop it). However, "clone" crashing
_does_ render the project unusable for the same person.

The crash[1] when cloning 'tcell' is an example of a project which is
still 100% usable on case-insensitive filesystems despite
case-conflicting filenames. Each of the conflicting file pairs [2] has
identical content, so everything works as intended.

Case-conflicting filenames also do not necessarily make it impossible
to do development work on a project. I have, myself, on several
occasions cloned such projects on MacOS to make improvements and track
down and/or fix bugs in parts of the projects not impacted by the
case-conflicting filenames.

Footnotes:

[1]: https://public-inbox.org/git/20180119180855.GA98561@smm.local/

[2]: case-conflicting files in tcell's bundled terminfo database:
terminfo/database/32/2621A
terminfo/database/32/2621a
terminfo/database/68/hp2621A
terminfo/database/68/hp2621a
terminfo/database/68/hp70092A
terminfo/database/68/hp70092a

Re: [PATCH 2/8] sequencer: introduce the `merge` command

[Jacob Keller]

On Fri, Jan 19, 2018 at 6:45 AM, Phillip Wood  wrote:
> On 18/01/18 15:35, Johannes Schindelin wrote:
>>
>> This patch is part of the effort to reimplement `--preserve-merges` with
>> a substantially improved design, a design that has been developed in the
>> Git for Windows project to maintain the dozens of Windows-specific patch
>> series on top of upstream Git.
>>
>> The previous patch implemented the `label`, `bud` and `reset` commands
>> to label commits and to reset to a labeled commits. This patch adds the
>> `merge` command, with the following syntax:
>>
>>   merge   
>
> I'm concerned that this will be confusing for users. All of the other
> rebase commands replay the changes in the commit hash immediately
> following the command name. This command instead uses the first commit
> to specify the message which is different to both 'git merge' and the
> existing rebase commands. I wonder if it would be clearer to have 'merge
> -C   ...' instead so it's clear which argument specifies
> the message and which the remote head to merge. It would also allow for
> 'merge -c   ...' in the future for rewording an existing
> merge message and also avoid the slightly odd 'merge -  ...'. Where
> it's creating new merges I'm not sure it's a good idea to encourage
> people to only have oneline commit messages by making it harder to edit
> them, perhaps it could take another argument to mean open the editor or
> not, though as Jake said I guess it's not that common.

I actually like the idea of re-using commit message options like -C,
-c,  and -m, so we could do:

merge -C  ... to take message from commit
merge -c  ...  to take the message from commit and open editor to edit
merge -m "" ... to take the message from the quoted test
merge ... to merge and open commit editor with default message

This also, I think, allows us to not need to put the oneline on the
end, meaning we wouldn't have to quote the parent commit arguments
since we could use option semantics?

>
> One thought that just struck me - if a merge or reset command specifies
> an invalid label is it rescheduled so that it's still in the to-do list
> when the user edits it after rebase stops?
>
> In the future it might be nice if the label, reset and merge commands
> were validated when the to-do list is parsed so that the user gets
> immediate feedback if they try to create a label that is not a valid ref
> name or that they have a typo in a name given to reset or merge rather
> than the rebase stopping later.
>

Re: [PATCH 9/8] [DO NOT APPLY, but squash?] git-rebase--interactive: clarify arguments

[Jacob Keller]

On Fri, Jan 19, 2018 at 12:30 PM, Junio C Hamano  wrote:
> Johannes Schindelin  writes:
>
>> Good idea! I would rather do it as an introductory patch (that only
>> converts the existing list).
>>
>> As to `merge`: it is a bit more complicated ;-)
>>
>>   m, merge  (  | "..." ) []
>>   create a merge commit using the original merge commit's
>>   message (or the oneline, if "-" is given). Use a quoted
>>   list of commits to be merged for octopus merges.
>
> Is it just the message that is being reused?
>
> Aren't the trees of the original commit and its parents participate
> in creating the tree of the recreated merge?  One way to preserve an
> originally evil merge is to notice how it was made by taking the
> difference between the result of mechanical merge of original merge
> parents and the original merge result, and carry it forward when
> recreating the merge across new parents.  Just being curious.
>

It looks like currently that only the commit is kept, with no attempt
to recreate evil merges.

Thanks,
Jake