Re: [PATCH v2 1/2] remote-curl: accept all encodings supported by curl

[Daniel Stenberg]

On Wed, 23 May 2018, Junio C Hamano wrote:


-> Accept-Encoding: gzip
+> Accept-Encoding: ENCODINGS


Is the ordering of these headers determined by the user of cURL library 
(i.e. Git), or whatever the version of cURL we happened to link with happens 
to produce?


The point is whether the order is expected to be stable, or we are better 
off sorting the actual log before comparing.


The order is not guaranteed by libcurl to be fixed, but it is likely to remain 
stable since we too have test cases and compare outputs with expected outputs! 
=)


Going forward, brotli (br) is going to become more commonly present in that 
header.


--

 / daniel.haxx.se

Re: [PATCH] t: make many tests depend less on the refs being files

[Christian Couder]

On Mon, May 21, 2018 at 9:34 PM, Stefan Beller  wrote:
> On Sun, May 20, 2018 at 10:51 PM, Christian Couder
>  wrote:
>> From: David Turner 
>>
>> So that they work under alternate ref storage backends.
>
> Sometimes I have a disconnect between the subject and the commit
> message, (e.g. in an email reader the subject is not displayed accurately on
> top of the message).
>
> So I would prefer if the first part of the body message is an actual
> sentence, and
> not a continuum from the subject.
>
> Maybe elaborate a bit more:
>
>   The current tests are very focused on the file system representation of
>   the loose and packed refs code.  As there are plans to implement other
>   ref storage systems, migrate most tests to a form that test the intent of 
> the
>   refs storage system instead of it internals. The internals of the loose and
>   packed refs are tested in , whereas the tests in this patch focus
>   on testing other aspects.

Thanks for this suggestion!

>> This will be really needed when such alternate ref storage backends are
>> developed. But this could already help by making clear to readers that
>> some tests do not depend on which ref backend is used.
>
> Ah, this is what I picked up already in the suggested edit above. :/

I actually mixed parts of your suggested message with parts of the
existing message in the V2 I just sent:

https://public-inbox.org/git/20180523052517.4443-1-chrisc...@tuxfamily.org/

Re: [PATCH] t: make many tests depend less on the refs being files

[Christian Couder]

On Mon, May 21, 2018 at 1:49 PM, SZEDER Gábor  wrote:
>> > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
>> > index 8f5c811dd7..c3b89ae783 100755
>> > --- a/t/t9903-bash-prompt.sh
>> > +++ b/t/t9903-bash-prompt.sh
>> > @@ -148,7 +148,7 @@ test_expect_success 'prompt - inside .git directory' '
>> >  test_expect_success 'prompt - deep inside .git directory' '
>> > printf " (GIT_DIR!)" >expected &&
>> > (
>> > -   cd .git/refs/heads &&
>> > +   cd .git/objects &&
>> > __git_ps1 >"$actual"
>> > ) &&
>> > test_cmp expected "$actual"
>>
>> This one looks wrong.
>
> It's right, though.
>
>> Why not `cd .git` instead?
>
> That case is covered in the previous test.
>
> Once upon a time it mattered how deep we were in the .git directory
> when running __git_ps1(), because it executed different branches of a
> long-ish if-elif chain.  For the prompt it doesn't matter anymore,
> because those ifs were eliminated, but for the completion script it
> still does, which brings us to:

Thanks for the explanations!

> Christian!  There is a similar test, '__git_find_repo_path - parent is
> a .git directory' in 't9902-completion.sh', which, too, performs a 'cd
> .git/refs/heads'.

Ok, I will take care of both of these tests in another patch.

[PATCH v2] t: make many tests depend less on the refs being files

[Christian Couder]

From: David Turner 

Many tests are very focused on the file system representation of the
loose and packed refs code. As there are plans to implement other
ref storage systems, let's migrate these tests to a form that test
the intent of the refs storage system instead of it internals.

This will make clear to readers that these tests do not depend on
which ref backend is used.

The internals of the loose refs backend are still tested in
t1400-update-ref.sh, whereas the tests changed in this patch focus
on testing other aspects.

This patch just takes care of many low hanging fruits. It does not
try to completely solves the issue.

Helped-by: Stefan Beller 
Helped-by: Johannes Schindelin 
Signed-off-by: David Turner 
Signed-off-by: Christian Couder 
---
 t/lib-t6000.sh   |  6 +++---
 t/t1401-symbolic-ref.sh  |  2 +-
 t/t3200-branch.sh| 18 +-
 t/t3903-stash.sh |  2 +-
 t/t5500-fetch-pack.sh| 10 +-
 t/t5510-fetch.sh |  6 +++---
 t/t6010-merge-base.sh|  2 +-
 t/t7201-co.sh|  2 +-
 t/t9104-git-svn-follow-parent.sh |  3 ++-
 9 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/t/lib-t6000.sh b/t/lib-t6000.sh
index 3f2d873fec..b0ed4767e3 100644
--- a/t/lib-t6000.sh
+++ b/t/lib-t6000.sh
@@ -4,11 +4,11 @@ mkdir -p .git/refs/tags
 
 >sed.script
 
-# Answer the sha1 has associated with the tag. The tag must exist in 
.git/refs/tags
+# Answer the sha1 has associated with the tag. The tag must exist under 
refs/tags
 tag () {
_tag=$1
-   test -f ".git/refs/tags/$_tag" || error "tag: \"$_tag\" does not exist"
-   cat ".git/refs/tags/$_tag"
+   git rev-parse --verify "refs/tags/$_tag" ||
+   error "tag: \"$_tag\" does not exist"
 }
 
 # Generate a commit using the text specified to make it unique and the tree
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index 9e782a8122..a4ebb0b65f 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -65,7 +65,7 @@ reset_to_sane
 test_expect_success 'symbolic-ref fails to delete real ref' '
echo "fatal: Cannot delete refs/heads/foo, not a symbolic ref" >expect 
&&
test_must_fail git symbolic-ref -d refs/heads/foo >actual 2>&1 &&
-   test_path_is_file .git/refs/heads/foo &&
+   git rev-parse --verify refs/heads/foo &&
test_cmp expect actual
 '
 reset_to_sane
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index c0ef946811..222dc2c377 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -234,34 +234,34 @@ test_expect_success 'git branch -M master2 master2 should 
work when master is ch
 
 test_expect_success 'git branch -v -d t should work' '
git branch t &&
-   test_path_is_file .git/refs/heads/t &&
+   git rev-parse --verify refs/heads/t &&
git branch -v -d t &&
-   test_path_is_missing .git/refs/heads/t
+   test_must_fail git rev-parse --verify refs/heads/t
 '
 
 test_expect_success 'git branch -v -m t s should work' '
git branch t &&
-   test_path_is_file .git/refs/heads/t &&
+   git rev-parse --verify refs/heads/t &&
git branch -v -m t s &&
-   test_path_is_missing .git/refs/heads/t &&
-   test_path_is_file .git/refs/heads/s &&
+   test_must_fail git rev-parse --verify refs/heads/t &&
+   git rev-parse --verify refs/heads/s &&
git branch -d s
 '
 
 test_expect_success 'git branch -m -d t s should fail' '
git branch t &&
-   test_path_is_file .git/refs/heads/t &&
+   git rev-parse refs/heads/t &&
test_must_fail git branch -m -d t s &&
git branch -d t &&
-   test_path_is_missing .git/refs/heads/t
+   test_must_fail git rev-parse refs/heads/t
 '
 
 test_expect_success 'git branch --list -d t should fail' '
git branch t &&
-   test_path_is_file .git/refs/heads/t &&
+   git rev-parse refs/heads/t &&
test_must_fail git branch --list -d t &&
git branch -d t &&
-   test_path_is_missing .git/refs/heads/t
+   test_must_fail git rev-parse refs/heads/t
 '
 
 test_expect_success 'git branch --list -v with --abbrev' '
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index aefde7b172..1f871d3cca 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -726,7 +726,7 @@ test_expect_success 'store updates stash ref and reflog' '
git reset --hard &&
! grep quux bazzy &&
git stash store -m quuxery $STASH_ID &&
-   test $(cat .git/refs/stash) = $STASH_ID &&
+   test $(git rev-parse stash) = $STASH_ID &&
git reflog --format=%H stash| grep $STASH_ID &&
git stash pop &&
grep quux bazzy
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 0680dec808..d4f435155f 100755
--- a/t/t5500-fetch-pack.sh
+++ 

Re: [PATCH] Add initial support for pax extended attributes

[Jeff King]

On Wed, May 23, 2018 at 11:34:52AM +0900, Junio C Hamano wrote:

> > @@ -90,13 +99,32 @@ foreach my $tar_file (@ARGV)
> > Z8 Z1 Z100 Z6
> > Z2 Z32 Z32 Z8 Z8 Z*', $_;
> > }
> > -   next if $name =~ m{/\z};
> > $mode = oct $mode;
> > $size = oct $size;
> > $mtime = oct $mtime;
> > next if $typeflag == 5; # directory
> >  
> > -   if ($typeflag != 1) { # handle hard links later
> > +   if ($typeflag eq 'x') { # extended header
> > +   # If extended header, check for path
> > +   my $pax_header = '';
> > +   while ($size > 0 && read(I, $_, 512) == 512) {
> 
> Would we ever get a short-read (i.e. we ask to read 512 bytes,
> syscall returns after reading only 256 bytes, even though next call
> to read would give the remaining 256 bytes and later ones)?

No, because perl's read() is buffered (you need sysread() to get a real
syscall read). We might read fewer than 512 if we hit EOF, but I think
that would be a truncated input, then, since ustar does everything in
512-byte records.

I do think we'd fail to notice the truncation, which isn't ideal. But it
looks like the rest of the script suffers from the same issue.

If anybody cares, it might not be too hard to wrap all of the 512-byte
read calls into a helper that dies on bogus input. I sort of assumed
this was mostly a proof of concept script and nobody used it, though. :)

It makes me wonder if there is a better-tested tar-reading module in
CPAN that could be used (though at the expense of requiring an extra
dependency).

-Peff

Re: [RFC PATCH] fetch-pack: space out sent "haves" in negotiation

[Junio C Hamano]

Jonathan Tan  writes:

>  Makefile   |   1 +
>  fetch-negotiator.c | 309 +
>  fetch-negotiator.h |  40 ++
>  fetch-pack.c   | 174 ++---
>  object.h   |   1 +
>  5 files changed, 392 insertions(+), 133 deletions(-)
>  create mode 100644 fetch-negotiator.c
>  create mode 100644 fetch-negotiator.h

Somehow this feels more like a WIP than RFC, primarily for two
reasons.  It was unclear what "edge" computation is trying to do; it
seems way under-explained, especially the part that takes min-max
while. merging two candidates.

It also was unclear if this should be organized as a "take it or
leave it" patch like this one, or eventually should be split into
multiple steps when it gets polished enough to be considered for
application, the early ones introducing a separate negotiator module
without changing the common ancestor discovery algorithm at all,
with later steps refining that negotiator and add more efficient
common ancestor discovery process.

> diff --git a/Makefile b/Makefile
> index ad880d1fc5..8bbedfa521 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -859,6 +859,7 @@ LIB_OBJS += ewah/ewah_bitmap.o
>  LIB_OBJS += ewah/ewah_io.o
>  LIB_OBJS += ewah/ewah_rlw.o
>  LIB_OBJS += exec-cmd.o
> +LIB_OBJS += fetch-negotiator.o
>  LIB_OBJS += fetch-object.o
>  LIB_OBJS += fetch-pack.o
>  LIB_OBJS += fsck.o
> diff --git a/fetch-negotiator.c b/fetch-negotiator.c
> new file mode 100644
> index 00..58975e1c37
> --- /dev/null
> +++ b/fetch-negotiator.c
> @@ -0,0 +1,309 @@
> +#include "cache.h"
> +#include "commit.h"
> +#include "fetch-negotiator.h"
> +
> +#define NO_THE_INDEX_COMPATIBILITY_MACROS

A totally unrelated tangent, but will we also benefit from
NO_THE_REPO_COMPATIBILITY_MACROS eventually?

Re: [GSoC] A blog about 'git stash' project

[Kaartic Sivaraam]

On 21 May 2018 01:03:22 GMT+05:30, Paul-Sebastian Ungureanu 
 wrote:
>
> I actually wrote a 
>short paragraph about one of them (the one regarding -p option) on the 
>blog (the first post).
>

That's interesting. I didn't realise that you wrote about one of the bugs in 
your blog. I might have missed it, somehow.

Anyhow, it happens to me all the time ;-)
-- 
Sivaraam

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: Why do we have both x*() and *_or_die() for "do or die"?

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> Just a side-question unrelated to this patch per-se, why do we have both
> x*() and *_or_die() functions in the codebase? I can't find any pattern
> for one or the other.

My understanding is that x*() were meant for system library
functions.  read-index-or-die should never be x-read-index.

Quite honestly, read-index should probably have designed to die from
the beginning, with read-index-gently as a variant to report an
error instead of dying.

Re: Officially supported Git versions

[Junio C Hamano]

Patrick Lühne  writes:

> Is there an official list of the Git versions that are still actively
> supported?

Depends on your definition of "official".  Distro with lts may patch
older maintenance tracks longer than the upstream releases do, and
as far as the normal end-users are concerned, Distro packaged
binaries are as "offcial" as they get, probably more "official" than
what comes from the upstream and then built from the source.

I however do not think distro folks advertise which maintenance
tracks they backport the patches here on this mailing list.

As to the "upstream", usually 'maint' track gets all fixes, and
probably one or two older maintenance tracks tend to get security
fixes as well.  Beyond that horizon, it's pretty much "as time
permits" basis.

Re: [PATCH] Add initial support for pax extended attributes

[Junio C Hamano]

Pedro Alvarez  writes:

> From: Pedro Alvarez Piedehierro 
> Subject: [PATCH] Add initial support for pax extended attributes

Lead it with the name of the area you are adding support for pax ext
header, e.g.

Subject: [PATCH] import-tars: read overlong names from pax extended header

or something.

> Sometimes the tar files will contain pax extended attributes to deal
> with cases where the information needed doesn't fit in a standard
> ustar entry.
>
> One of these cases is when the path is larger than 100 characters. A
> pax entry will appear containing two standard ustart entries. The first

u-start? us-tart? sound yummy.  I think s/ustart entries/ustar headers/

> entry will have an 'x' typeflag, and contain the the extended attributes.

s/contain//

>
> The pax extended attributes contain one or multiple records constructed as
> follows:
>
> "%d %s=%s\n", , , 

> This commit makes sure that we always read the extended attibutes from

s/This commit makes sure/Make sure/;

> pax entries, and in the case of finding one, we parse its records
> looking for 'path' information. If this information is found, it's
> stored to be used in the next ustar entry.
>
> Information about the Pax Interchange Format can be found at:
>
> 
> https://www.freebsd.org/cgi/man.cgi?manpath=FreeBSD+8-current=tar=5.

> Before this change, importing gcc tarballs[1] would fail with the
> following error:
>
> fast-import crash report:
> fast-import process: 82899
> parent process : 82897
> at 2018-05-21 12:35:27 +
>
> fatal: Unsupported command: 29 atime=1516870168.93527949

Drop "Before this change, " and move the above to the very beginning
of the proposed log message.  The problem description is always
"without this patch applied, we have this problem that needs to be
fixed", so "Before this change" is an unnecessary thing to say.

The remainder of the crash log may or may not be in the problem
description, if we want to shoot for brevity.  If I were writing a
log message for this patch, I'd go for even shorter version, e.g.

Importing gcc tarballs[1] with import-tars script (in
contrib/) fails when hitting a pax extended header that
records a long pathname.

Teach the code to parse and grab information from pax
extended headers, and reconstruct a long pathname that is
split into multiple records, to correct this problem.

The code to parse pax extended headers were written,
consulting the Pax Interchange Format documentation [2].

[1] http://ftp.gnu.org/gnu/gcc/gcc-7.3.0/gcc-7.3.0.tar.xz
[2] 
https://www.freebsd.org/cgi/man.cgi?manpath=FreeBSD+8-current=tar=5

> index d60b4315ed..c2e54ec7a3 100755
> --- a/contrib/fast-import/import-tars.perl
> +++ b/contrib/fast-import/import-tars.perl
> @@ -63,6 +63,8 @@ foreach my $tar_file (@ARGV)
>   my $have_top_dir = 1;
>   my ($top_dir, %files);
>  
> + my $next_path = '';
> +
>   while (read(I, $_, 512) == 512) {
>   my ($name, $mode, $uid, $gid, $size, $mtime,
>   $chksum, $typeflag, $linkname, $magic,
> @@ -70,6 +72,13 @@ foreach my $tar_file (@ARGV)
>   $prefix) = unpack 'Z100 Z8 Z8 Z8 Z12 Z12
>   Z8 Z1 Z100 Z6
>   Z2 Z32 Z32 Z8 Z8 Z*', $_;
> +
> + unless ($next_path eq '') {
> + # Recover name from previous extended header
> + $name = $next_path;
> + $next_path = '';
> + }
> +
>   last unless length($name);
>   if ($name eq '././@LongLink') {
>   # GNU tar extension
> @@ -90,13 +99,32 @@ foreach my $tar_file (@ARGV)
>   Z8 Z1 Z100 Z6
>   Z2 Z32 Z32 Z8 Z8 Z*', $_;
>   }
> - next if $name =~ m{/\z};
>   $mode = oct $mode;
>   $size = oct $size;
>   $mtime = oct $mtime;
>   next if $typeflag == 5; # directory
>  
> - if ($typeflag != 1) { # handle hard links later
> + if ($typeflag eq 'x') { # extended header
> + # If extended header, check for path
> + my $pax_header = '';
> + while ($size > 0 && read(I, $_, 512) == 512) {

Would we ever get a short-read (i.e. we ask to read 512 bytes,
syscall returns after reading only 256 bytes, even though next call
to read would give the remaining 256 bytes and later ones)?

If we do get a short-read, would that be an error (in which case,
how are we handling it)?  If it is not an error, should we continue
reading, instead of leaving the loop?

> + $pax_header = $pax_header . substr($_, 0, 
> $size);
> + $size -= 512;
> + }
> +
> + my @lines = 

Re: [PATCH v2 1/2] remote-curl: accept all encodings supported by curl

[brian m. carlson]

On Wed, May 23, 2018 at 10:23:26AM +0900, Junio C Hamano wrote:
> Similarly, how much control do we have to ensure that the test HTTPD
> server (1) supports gzip and (2) does not support encoding algos
> with confusing names e.g. "funnygzipalgo" that may accidentally
> match that pattern?

I feel it's quite likely indeed that pretty much any Apache instance is
going to have the gzip encoding.  Every distributor I know supports it.

As for whether there are confusing alternate algorithms, I think it's
best to just look at the IANA registration[0] to see what people are
using.  Potential matches include gzip, x-gzip (a deprecated alias that
versions of Apache we can use are not likely to support), and
pack200-gzip (a format for Java archives, which we hope the remote side
will not be sending).

Overall, I think this is not likely to be a problem, but if necessary in
the future, we can add a prerequisite that looks in the module directory
for the appropriate module.  We haven't seen an issue with it yet,
though, TTBOMK.

[0] 
https://www.iana.org/assignments/http-parameters/http-parameters.xml#content-coding
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Junio C Hamano]

Luke Diamand  writes:

>> However, instead of a separate patch, wouldn't it be better to squash
>> it into the previous one?  So 'make test' would succeed on every
>> commit even with a newer p4 version.
>
> Junio?
>
> I can squash together the original commit and the two fixes if that
> would be better?

Among the three hunks in this fix-up patch, the first two are
strictly fixing what you had in the previous patch, so it make sense
to fix them at the source by squashing.

The last one (i.e. "even if it is verbose, if fileSize is not
reported, do not write the verbose output") does not look like it is
limited to the unshelve feature, so it might, even though it is a
one-liner, deserve to be a separate preparatory patch if you want.
But I do not feel strongly about either way.

Thanks.

Re: should config options be treated as case-sensitive?

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> The issues you note about the docs using foo.barbaz instead of
> foo.barBaz should be fixed, but as noted in the "Syntax" section of
> "git-config" we already document that the config keys are all
> case-insensitive. We just like talking about them as foo.barBaz because
> it makes for easier reading.

The first and the last level of configuration variable names are
case insensitive.

I said "first and last", as there are variables with 2-level and
3-level names.  "foo.barBaz" is two-level and it is the same
variable as "Foo.barbaz".  "remote.origin.url" is three-level, and
it is the same variable as "Remote.origin.URL", but it is not the
same variable as "remote.ORIGIN.url".

If the documention does not make it clear, then we have
documentation bug.  As you said, I think we are OK in the sense that
we do say a section or a variable name is icase and a subsection, if
exist, is case sensitive, but there might be a better way to convey
that fact without having the reader read the whole three paragraphs.

Re: [PATCH] Doc: Mention core.excludesFile in "man git-clean"

[Junio C Hamano]

"Robert P. J. Day"  writes:

> Add a reference to the configuration setting "core.excludesFile" to
> the man page for git-clean.
>
> Signed-off-by: Robert P. J. Day 
>
> ---

I understand that you are trying to reduce the source of the
confusion you felt, which comes from mentioning only per-directory
.gitignore and per-repository info/exclude, but I am not sure if the
proposed solution is a good one that learned from our past mistakes.

Wouldn't it make more sense to _avoid_ appearing as if we are giving
a complete list and refer those who want a single authoritative list
to the source?  For example

In addition to those found in standard places for exclude
patterns such as `.gitignore` (cf. linkgit:gitignore[5]),
also consider these patterns...

After all, having an incomplete list and not hinting that it is
incomplete is what made you react to the current description.  It is
unlikely that we stop treating `.gitignore` as one of the standard
places, so phrasing like above will have a lot smaller chance to go
stale, even accounting for the possibility that we will grow Git
over time and the standard parttern sources may be updated in the
future.

>
> diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
> index 03056dad0..449cbc2af 100644
> --- a/Documentation/git-clean.txt
> +++ b/Documentation/git-clean.txt
> @@ -55,13 +55,15 @@ OPTIONS
>
>  -e ::
>  --exclude=::
> - In addition to those found in .gitignore (per directory) and
> - $GIT_DIR/info/exclude, also consider these patterns to be in the
> - set of the ignore rules in effect.
> + In addition to patterns found in any of .gitignore (per directory),
> + $GIT_DIR/info/exclude and the exclude file specified by the
> + configuration variable core.excludesFile, also consider these
> + patterns to be in the set of the ignore rules in effect.
>
>  -x::
>   Don't use the standard ignore rules read from .gitignore (per
> - directory) and $GIT_DIR/info/exclude, but do still use the ignore
> + directory), $GIT_DIR/info/exclude and the exclude file specified
> + by core.excludesFile, but do still use the ignore
>   rules given with `-e` options.  This allows removing all untracked
>   files, including build products.  This can be used (possibly in
>   conjunction with 'git reset') to create a pristine

Re: [PATCH v2 1/2] remote-curl: accept all encodings supported by curl

[Junio C Hamano]

Brandon Williams  writes:

> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index f5721b4a5..913089b14 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -26,14 +26,14 @@ setup_askpass_helper
>  cat >exp <  > GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1
>  > Accept: */*
> -> Accept-Encoding: gzip
> +> Accept-Encoding: ENCODINGS
>  > Pragma: no-cache

Is the ordering of these headers determined by the user of cURL
library (i.e. Git), or whatever the version of cURL we happened to
link with happens to produce?

The point is whether the order is expected to be stable, or we are
better off sorting the actual log before comparing.

>  < HTTP/1.1 200 OK
>  < Pragma: no-cache
>  < Cache-Control: no-cache, max-age=0, must-revalidate
>  < Content-Type: application/x-git-upload-pack-advertisement

A similar question for this response.

>  > POST /smart/repo.git/git-upload-pack HTTP/1.1
> -> Accept-Encoding: gzip
> +> Accept-Encoding: ENCODINGS
>  > Content-Type: application/x-git-upload-pack-request
>  > Accept: application/x-git-upload-pack-result
>  > Content-Length: xxx

Ditto for this request.

> @@ -79,8 +79,13 @@ test_expect_success 'clone http repository' '
>   /^< Date: /d
>   /^< Content-Length: /d
>   /^< Transfer-Encoding: /d
> - " >act &&
> - test_cmp exp act
> + " >actual &&
> + sed -e "s/^> Accept-Encoding: .*/> Accept-Encoding: ENCODINGS/" \
> + actual >actual.smudged &&
> + test_cmp exp actual.smudged &&
> +
> + grep "Accept-Encoding:.*gzip" actual >actual.gzip &&
> + test_line_count = 2 actual.gzip
>  '

Similarly, how much control do we have to ensure that the test HTTPD
server (1) supports gzip and (2) does not support encoding algos
with confusing names e.g. "funnygzipalgo" that may accidentally
match that pattern?

Thanks.  Not a new issue with this patch, but just being curious if
you or anybody thought about it as a possible issue.

Re: [RFC PATCH] fetch-pack: space out sent "haves" in negotiation

[Junio C Hamano]

Jonathan Tan  writes:

> I was thinking about fetch negotiation in some non-ideal situations
> (specifically, when the client repo contains two or more independent
> branches that meet only somewhere far in the past) and thought about
> skipping over intermediate commits, using exponentially larger skips as
> we proceed, when generating "have" lines.

I recall a "wouldn't it be wonderful if we had..." discussion on
this exact topic much earlier in the project's life to skip
exponentialy (I think fibonacci was what was suggested in the
thread) and then backfill after overshooting, but I do not recall
anybody following through and coming up with a concrete design
and/or implementation.  Maybe protocol v2 in place we can have
something this time around.

Looking forward to read the remainder of the message and the patch
;-)

Thanks.

Re: [PATCH v5 0/4] unpack_trees_options: free messages when done

[Junio C Hamano]

Martin Ågren  writes:

> (Maybe not to fix the leaking as such, but to keep 'maint' more up to
> date with 'master' for easier merging of other topics?)

I admit that I occasionally do such a "presumably no-op" merge to
'maint' out of sheer laziness, but in general I'd prefer to keep
'maint' quieter than that.  As we do not have any repeated caller
that calls unpack_trees unbounded number of times in a process, I
think it is OK to limit this update only to 'master', especially at
this late in the cycle.

Thanks.

Re: [PATCH v2] mailmap: update brian m. carlson's email address

[brian m. carlson]

On Tue, May 22, 2018 at 03:08:26PM -0700, Jonathan Nieder wrote:
> These commits use author Brian M. Carlson .
> Previously they matched
> 
>   brian m. carlson  
> 
> 
> but now they don't match any line in the .mailmap file.
> 
> Should we add a line
> 
>   brian m. carlson 
> 
> to handle these commits?

Ah, good point.  Probably so.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH 00/12] Die commit->util, die!

[Stefan Beller]

On Sat, May 12, 2018 at 1:00 AM, Nguyễn Thái Ngọc Duy  wrote:
> There's not much to write here. It's basically a copy from 12/12:
>
> This 'util' pointer can be used for many different purposes,
> controlled in different ways. Some are not even contained in a command
> code, but buried deep in common code with no clue who will use it and
> how. For example, if revs.show_source is set, then it's used for
> storing path name, but if you happen to call get_merge_parent() then
> some 'util' may end up storing another thing.

This is cool! It makes life easy when it comes to clearing up commits
in the object store (as it is unclear what the util pointer is used for
we leaked that memory so far in the object store conversion series).

> The move to using commit-slab gives us a much better picture of how
> some piece of data is associated with a commit and what for. Since
> nobody uses 'util' pointer anymore, we can retire it so that nobody will
> abuse it again. commit-slab will be the way forward for associating
> data to a commit.
>
> As a side benefit, this shrinks struct commit by 8 bytes (on 64-bit
> architecture) which should help reduce memory usage for reachability
> test a bit. This is also what commit-slab is invented for [1].

The object store series (called -lookup-final) is at a point where
I have to move one of the slabs (buffer_slab in commit.c) into
the repository or rather into the parsed_object_pool in object.h.

So I think I'll build a series on top of this one first, which allows
for not just defining the slabs all over the place, but actually allows
them to be embedded into other structs.

Then I merge that series with origin/sb/object-store-graft and build
the -final series on top of that.

Stefan

Re: [PATCH] config: document value 2 for protocol.version

[Jonathan Nieder]

Brandon Williams wrote:

> Update the config documentation to note the value `2` as an acceptable
> value for the protocol.version config.
>
> Signed-off-by: Brandon Williams 
> ---
>  Documentation/config.txt | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Jonathan Nieder 

Thanks.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index a05a88fec..ce778883d 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2637,6 +2637,8 @@ protocol.version::
>  * `1` - the original wire protocol with the addition of a version string
>in the initial response from the server.
>  
> +* `2` - wire protocol version 2.
> +
>  --

Should this mention where the user can find more details (e.g.  by
saying link:technical/protocol-v2.html[protocol version 2])?

See the reference to the credentials API in "git help credentials" for
an example of how that renders.

Jonathan

[GSoC] Week 3 - 'git stash' blog

[Paul-Sebastian Ungureanu]

Hello,

New week, new blog post [1]. Any feedback is greatly appreciated! Thank you!

[1]
https://ungps.github.io/

Best,
Paul

[PATCH] config: document value 2 for protocol.version

[Brandon Williams]

Update the config documentation to note the value `2` as an acceptable
value for the protocol.version config.

Signed-off-by: Brandon Williams 
---
 Documentation/config.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a05a88fec..ce778883d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2637,6 +2637,8 @@ protocol.version::
 * `1` - the original wire protocol with the addition of a version string
   in the initial response from the server.
 
+* `2` - wire protocol version 2.
+
 --
 
 pull.ff::
-- 
2.17.0.441.gb46fe60e1d-goog

Proposal

[Miss Zeliha Omer Faruk]

Hello

Greetings to you please i have a business proposal for you contact me
for more detailes asap thanks.

Best Regards,
Miss.Zeliha ömer faruk
Esentepe Mahallesi Büyükdere
Caddesi Kristal Kule Binasi
No:215
Sisli - Istanbul, Turkey

Re: [PATCH v2] mailmap: update brian m. carlson's email address

[Jonathan Nieder]

Hi,

brian m. carlson wrote:

> An earlier change, cdb6b5ac (".mailmap: Combine more (name, email) to
> individual persons", 2013-08-12), noted that there were two name
> spellings and two email addresses and mapped the crustytoothpaste.net
> address to the crustytoothpaste.ath.cx address.  The latter is an older,
> obsolete address, while the former is current, so switch the order of
> the addresses so that git log displays the correct address.
>
> Signed-off-by: brian m. carlson 
> ---
> I intentionally avoided the use of the first person here, because I
> wasn't sure what the preference of the list was on that.  Hopefully it
> reads naturally and isn't awkward.

I have no preference.  First or third person would work equally well
for me for this kind of thing.

[...]
> --- a/.mailmap
> +++ b/.mailmap
> @@ -25,8 +25,8 @@ Ben Walton  
>  Benoit Sigoure  
>  Bernt Hansen  
>  Brandon Casey  
> -brian m. carlson  Brian M. Carlson 
> 
> -brian m. carlson  
> 
> +brian m. carlson  Brian M. Carlson 
> 
> +brian m. carlson  
> 

Today I tried a "git shortlog" run and was surprised to find

  Brian M. Carlson (6):
  Add missing test file for UTF-16.
  submodule: fix confusing variable name
  submodule: don't print status output with ignore=all
  send-email: don't call methods on undefined values
  http-backend: provide Allow header for 405
  remote-curl: fix large pushes with GSSAPI

These commits use author Brian M. Carlson .
Previously they matched

brian m. carlson  


but now they don't match any line in the .mailmap file.

Should we add a line

brian m. carlson 

to handle these commits?

Thanks,
Jonathan

[GSoC][PATCH v2 4/4] rebase: remove -p code from git-rebase--interactive.sh

[Alban Gruin]

All the code specific to preserve-merges was moved to
git-rebase--preserve-merges.sh, and so it’s useless to keep it here.

Signed-off-by: Alban Gruin 
---
 git-rebase--interactive.sh | 708 +
 1 file changed, 8 insertions(+), 700 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2f4941d0f..657d32773 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -105,15 +105,6 @@ case "$comment_char" in
;;
 esac
 
-warn () {
-   printf '%s\n' "$*" >&2
-}
-
-# Output the commit message for the specified commit.
-commit_message () {
-   git cat-file commit "$1" | sed "1,/^$/d"
-}
-
 orig_reflog_action="$GIT_REFLOG_ACTION"
 
 comment_for_reflog () {
@@ -142,16 +133,6 @@ mark_action_done () {
fi
 }
 
-# Put the last action marked done at the beginning of the todo list
-# again. If there has not been an action marked done yet, leave the list of
-# items on the todo list unchanged.
-reschedule_last_action () {
-   tail -n 1 "$done" | cat - "$todo" >"$todo".new
-   sed -e \$d <"$done" >"$done".new
-   mv -f "$todo".new "$todo"
-   mv -f "$done".new "$done"
-}
-
 append_todo_help () {
gettext "
 Commands:
@@ -184,50 +165,6 @@ If you remove a line here THAT COMMIT WILL BE LOST.
fi
 }
 
-make_patch () {
-   sha1_and_parents="$(git rev-list --parents -1 "$1")"
-   case "$sha1_and_parents" in
-   ?*' '?*' '?*)
-   git diff --cc $sha1_and_parents
-   ;;
-   ?*' '?*)
-   git diff-tree -p "$1^!"
-   ;;
-   *)
-   echo "Root commit"
-   ;;
-   esac > "$state_dir"/patch
-   test -f "$msg" ||
-   commit_message "$1" > "$msg"
-   test -f "$author_script" ||
-   get_author_ident_from_commit "$1" > "$author_script"
-}
-
-die_with_patch () {
-   echo "$1" > "$state_dir"/stopped-sha
-   git update-ref REBASE_HEAD "$1"
-   make_patch "$1"
-   die "$2"
-}
-
-exit_with_patch () {
-   echo "$1" > "$state_dir"/stopped-sha
-   git update-ref REBASE_HEAD "$1"
-   make_patch $1
-   git rev-parse --verify HEAD > "$amend"
-   gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")}
-   warn "$(eval_gettext "\
-You can amend the commit now, with
-
-   git commit --amend \$gpg_sign_opt_quoted
-
-Once you are satisfied with your changes, run
-
-   git rebase --continue")"
-   warn
-   exit $2
-}
-
 die_abort () {
apply_autostash
rm -rf "$state_dir"
@@ -238,30 +175,6 @@ has_action () {
test -n "$(git stripspace --strip-comments <"$1")"
 }
 
-is_empty_commit() {
-   tree=$(git rev-parse -q --verify "$1"^{tree} 2>/dev/null) || {
-   sha1=$1
-   die "$(eval_gettext "\$sha1: not a commit that can be picked")"
-   }
-   ptree=$(git rev-parse -q --verify "$1"^^{tree} 2>/dev/null) ||
-   ptree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-   test "$tree" = "$ptree"
-}
-
-is_merge_commit()
-{
-   git rev-parse --verify --quiet "$1"^2 >/dev/null 2>&1
-}
-
-# Run command with GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
-# GIT_AUTHOR_DATE exported from the current environment.
-do_with_author () {
-   (
-   export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE
-   "$@"
-   )
-}
-
 git_sequence_editor () {
if test -z "$GIT_SEQUENCE_EDITOR"
then
@@ -275,455 +188,6 @@ git_sequence_editor () {
eval "$GIT_SEQUENCE_EDITOR" '"$@"'
 }
 
-pick_one () {
-   ff=--ff
-
-   case "$1" in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac
-   case "$force_rebase" in '') ;; ?*) ff= ;; esac
-   output git rev-parse --verify $sha1 || die "$(eval_gettext "Invalid 
commit name: \$sha1")"
-
-   if is_empty_commit "$sha1"
-   then
-   empty_args="--allow-empty"
-   fi
-
-   test -d "$rewritten" &&
-   pick_one_preserving_merges "$@" && return
-   output eval git cherry-pick $allow_rerere_autoupdate 
$allow_empty_message \
-   ${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
-   $signoff "$strategy_args" $empty_args $ff "$@"
-
-   # If cherry-pick dies it leaves the to-be-picked commit unrecorded. 
Reschedule
-   # previous task so this commit is not lost.
-   ret=$?
-   case "$ret" in [01]) ;; *) reschedule_last_action ;; esac
-   return $ret
-}
-
-pick_one_preserving_merges () {
-   fast_forward=t
-   case "$1" in
-   -n)
-   fast_forward=f
-   sha1=$2
-   ;;
-   *)
-   sha1=$1
-   ;;
-   esac
-   sha1=$(git rev-parse $sha1)
-
-   if test -f "$state_dir"/current-commit && test "$fast_forward" = t
-   then
-   while read current_commit
- 

[GSoC][PATCH v2 3/4] rebase: use the new git-rebase--preserve-merges.sh

[Alban Gruin]

Creates a new type of rebase, "preserve-merges", used when rebase is called with
-p.

Before that, the type for preserve-merges was "interactive", and some places of
this script compared $type to "interactive". Instead, the code now checks if
$interactive_rebase is empty or not, as it is set to "explicit" when calling an
interactive rebase (and, possibly, one of its submodes), and "implied" when
calling one of its submodes (eg. preserve-merges) *without* interactive rebase.

It also detects the presence of the directory "$merge_dir"/rewritten left by the
preserve-merges script when calling rebase --continue, --skip, etc., and, if it
exists, sets the rebase mode to preserve-merges. In this case,
interactive_rebase is set to "explicit", as "implied" would break some tests.

Signed-off-by: Alban Gruin 
---
 git-rebase.sh | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 40be59ecc..19bdebb48 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -207,7 +207,14 @@ run_specific_rebase () {
autosquash=
fi
. git-rebase--$type
-   git_rebase__$type${preserve_merges:+__preserve_merges}
+
+   if test -z "$preserve_merges"
+   then
+   git_rebase__$type
+   else
+   git_rebase__preserve_merges
+   fi
+
ret=$?
if test $ret -eq 0
then
@@ -239,7 +246,12 @@ then
state_dir="$apply_dir"
 elif test -d "$merge_dir"
 then
-   if test -f "$merge_dir"/interactive
+   if test -d "$merge_dir"/rewritten
+   then
+   type=preserve-merges
+   interactive_rebase=explicit
+   preserve_merges=t
+   elif test -f "$merge_dir"/interactive
then
type=interactive
interactive_rebase=explicit
@@ -402,14 +414,14 @@ if test -n "$action"
 then
test -z "$in_progress" && die "$(gettext "No rebase in progress?")"
# Only interactive rebase uses detailed reflog messages
-   if test "$type" = interactive && test "$GIT_REFLOG_ACTION" = rebase
+   if test -n "$interactive_rebase" && test "$GIT_REFLOG_ACTION" = rebase
then
GIT_REFLOG_ACTION="rebase -i ($action)"
export GIT_REFLOG_ACTION
fi
 fi
 
-if test "$action" = "edit-todo" && test "$type" != "interactive"
+if test "$action" = "edit-todo" && test -z "$interactive_rebase"
 then
die "$(gettext "The --edit-todo action can only be used during 
interactive rebase.")"
 fi
@@ -487,7 +499,13 @@ fi
 
 if test -n "$interactive_rebase"
 then
-   type=interactive
+   if test -z "$preserve_merges"
+   then
+   type=interactive
+   else
+   type=preserve-merges
+   fi
+
state_dir="$merge_dir"
 elif test -n "$do_merge"
 then
@@ -647,7 +665,7 @@ require_clean_work_tree "rebase" "$(gettext "Please commit 
or stash them.")"
 # but this should be done only when upstream and onto are the same
 # and if this is not an interactive rebase.
 mb=$(git merge-base "$onto" "$orig_head")
-if test "$type" != interactive && test "$upstream" = "$onto" &&
+if test -z "$interactive_rebase" && test "$upstream" = "$onto" &&
test "$mb" = "$onto" && test -z "$restrict_revision" &&
# linear history?
! (git rev-list --parents "$onto".."$orig_head" | sane_grep " .* ") > 
/dev/null
@@ -691,7 +709,7 @@ then
GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
 fi
 
-test "$type" = interactive && run_specific_rebase
+test -n "$interactive_rebase" && run_specific_rebase
 
 # Detach HEAD and reset the tree
 say "$(gettext "First, rewinding head to replay your work on top of it...")"
-- 
2.16.1

[GSoC][PATCH v2 1/4] rebase: duplicate git-rebase--interactive.sh to git-rebase--preserve-merges.sh

[Alban Gruin]

This duplicates git-rebase--interactive.sh to
git-rebase--preserve-merges.sh. This is done to split -p from -i. No
modifications are made to this file here, but any code that is not used by -p
will be stripped in the next commit.

Signed-off-by: Alban Gruin 
---
 .gitignore |1 +
 Makefile   |2 +
 git-rebase--preserve-merges.sh | 1069 
 3 files changed, 1072 insertions(+)
 create mode 100644 git-rebase--preserve-merges.sh

diff --git a/.gitignore b/.gitignore
index 833ef3b0b..ef4925485 100644
--- a/.gitignore
+++ b/.gitignore
@@ -117,6 +117,7 @@
 /git-rebase--helper
 /git-rebase--interactive
 /git-rebase--merge
+/git-rebase--preserve-merges
 /git-receive-pack
 /git-reflog
 /git-remote
diff --git a/Makefile b/Makefile
index 50da82b01..810a0d0f4 100644
--- a/Makefile
+++ b/Makefile
@@ -582,6 +582,7 @@ SCRIPT_LIB += git-mergetool--lib
 SCRIPT_LIB += git-parse-remote
 SCRIPT_LIB += git-rebase--am
 SCRIPT_LIB += git-rebase--interactive
+SCRIPT_LIB += git-rebase--preserve-merges
 SCRIPT_LIB += git-rebase--merge
 SCRIPT_LIB += git-sh-setup
 SCRIPT_LIB += git-sh-i18n
@@ -2271,6 +2272,7 @@ LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
 LOCALIZED_SH = $(SCRIPT_SH)
 LOCALIZED_SH += git-parse-remote.sh
 LOCALIZED_SH += git-rebase--interactive.sh
+LOCALIZED_SH += git-rebase--preserve-merges.sh
 LOCALIZED_SH += git-sh-setup.sh
 LOCALIZED_PERL = $(SCRIPT_PERL)
 
diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
new file mode 100644
index 0..2f4941d0f
--- /dev/null
+++ b/git-rebase--preserve-merges.sh
@@ -0,0 +1,1069 @@
+# This shell script fragment is sourced by git-rebase to implement
+# its interactive mode.  "git rebase --interactive" makes it easy
+# to fix up commits in the middle of a series and rearrange commits.
+#
+# Copyright (c) 2006 Johannes E. Schindelin
+#
+# The original idea comes from Eric W. Biederman, in
+# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
+#
+# The file containing rebase commands, comments, and empty lines.
+# This file is created by "git rebase -i" then edited by the user.  As
+# the lines are processed, they are removed from the front of this
+# file and written to the tail of $done.
+todo="$state_dir"/git-rebase-todo
+
+# The rebase command lines that have already been processed.  A line
+# is moved here when it is first handled, before any associated user
+# actions.
+done="$state_dir"/done
+
+# The commit message that is planned to be used for any changes that
+# need to be committed following a user interaction.
+msg="$state_dir"/message
+
+# The file into which is accumulated the suggested commit message for
+# squash/fixup commands.  When the first of a series of squash/fixups
+# is seen, the file is created and the commit message from the
+# previous commit and from the first squash/fixup commit are written
+# to it.  The commit message for each subsequent squash/fixup commit
+# is appended to the file as it is processed.
+#
+# The first line of the file is of the form
+# # This is a combination of $count commits.
+# where $count is the number of commits whose messages have been
+# written to the file so far (including the initial "pick" commit).
+# Each time that a commit message is processed, this line is read and
+# updated.  It is deleted just before the combined commit is made.
+squash_msg="$state_dir"/message-squash
+
+# If the current series of squash/fixups has not yet included a squash
+# command, then this file exists and holds the commit message of the
+# original "pick" commit.  (If the series ends without a "squash"
+# command, then this can be used as the commit message of the combined
+# commit without opening the editor.)
+fixup_msg="$state_dir"/message-fixup
+
+# $rewritten is the name of a directory containing files for each
+# commit that is reachable by at least one merge base of $head and
+# $upstream. They are not necessarily rewritten, but their children
+# might be.  This ensures that commits on merged, but otherwise
+# unrelated side branches are left alone. (Think "X" in the man page's
+# example.)
+rewritten="$state_dir"/rewritten
+
+dropped="$state_dir"/dropped
+
+end="$state_dir"/end
+msgnum="$state_dir"/msgnum
+
+# A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
+# GIT_AUTHOR_DATE that will be used for the commit that is currently
+# being rebased.
+author_script="$state_dir"/author-script
+
+# When an "edit" rebase command is being processed, the SHA1 of the
+# commit to be edited is recorded in this file.  When "git rebase
+# --continue" is executed, if there are any staged changes then they
+# will be amended to the HEAD commit, but only provided the HEAD
+# commit is still the commit to be edited.  When any other rebase
+# command is processed, this file is deleted.
+amend="$state_dir"/amend
+
+# For the post-rewrite hook, we make a list of rewritten 

[GSoC][PATCH v2 2/4] rebase: strip unused code in git-rebase--preserve-merges.sh

[Alban Gruin]

This removes the code coming from git-rebase--interactive.sh that is not needed
by preserve-merges.

Signed-off-by: Alban Gruin 
---
 git-rebase--preserve-merges.sh | 65 +++---
 1 file changed, 4 insertions(+), 61 deletions(-)

diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
index 2f4941d0f..c51c7828e 100644
--- a/git-rebase--preserve-merges.sh
+++ b/git-rebase--preserve-merges.sh
@@ -1,12 +1,8 @@
-# This shell script fragment is sourced by git-rebase to implement
-# its interactive mode.  "git rebase --interactive" makes it easy
-# to fix up commits in the middle of a series and rearrange commits.
+# This shell script fragment is sourced by git-rebase to implement its
+# preserve-merges mode.
 #
 # Copyright (c) 2006 Johannes E. Schindelin
 #
-# The original idea comes from Eric W. Biederman, in
-# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
-#
 # The file containing rebase commands, comments, and empty lines.
 # This file is created by "git rebase -i" then edited by the user.  As
 # the lines are processed, they are removed from the front of this
@@ -287,17 +283,7 @@ pick_one () {
empty_args="--allow-empty"
fi
 
-   test -d "$rewritten" &&
-   pick_one_preserving_merges "$@" && return
-   output eval git cherry-pick $allow_rerere_autoupdate 
$allow_empty_message \
-   ${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
-   $signoff "$strategy_args" $empty_args $ff "$@"
-
-   # If cherry-pick dies it leaves the to-be-picked commit unrecorded. 
Reschedule
-   # previous task so this commit is not lost.
-   ret=$?
-   case "$ret" in [01]) ;; *) reschedule_last_action ;; esac
-   return $ret
+   pick_one_preserving_merges "$@"
 }
 
 pick_one_preserving_merges () {
@@ -761,11 +747,6 @@ get_missing_commit_check_level () {
 initiate_action () {
case "$1" in
continue)
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
# do we have anything to commit?
if git diff-index --cached --quiet HEAD --
then
@@ -824,12 +805,6 @@ first and then run 'git rebase --continue' again.")"
;;
skip)
git rerere clear
-
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
do_rest
return 0
;;
@@ -944,43 +919,11 @@ EOF
}
 
expand_todo_ids
-
-   test -d "$rewritten" || test -n "$force_rebase" ||
-   onto="$(git rebase--helper --skip-unnecessary-picks)" ||
-   die "Could not skip unnecessary pick commands"
-
checkout_onto
-   if test ! -d "$rewritten"
-   then
-   require_clean_work_tree "rebase"
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
do_rest
 }
 
-git_rebase__interactive () {
-   initiate_action "$action"
-   ret=$?
-   if test $ret = 0; then
-   return 0
-   fi
-
-   setup_reflog_action
-   init_basic_state
-
-   init_revisions_and_shortrevisions
-
-   git rebase--helper --make-script ${keep_empty:+--keep-empty} \
-   ${rebase_merges:+--rebase-merges} \
-   ${rebase_cousins:+--rebase-cousins} \
-   $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
-   die "$(gettext "Could not generate todo list")"
-
-   complete_action
-}
-
-git_rebase__interactive__preserve_merges () {
+git_rebase__preserve_merges () {
initiate_action "$action"
ret=$?
if test $ret = 0; then
-- 
2.16.1

[GSoC][PATCH v2 0/4] rebase: split rebase -p from rebase -i

[Alban Gruin]

This splits the `rebase --preserve-merges` functionnality from
git-rebase--interactive.sh. All the dead code left by the duplication of
git-rebase--interactive.sh is also removed. This will make it easier to rewrite
this script in C.

This patch series is based of js/sequencer-and-root-commits.

Changes since v1:

 - Duplicating the correct version of git-rebase--interactive.sh (thanks
   Stefan!)

Alban Gruin (4):
  rebase: duplicate git-rebase--interactive.sh to
git-rebase--preserve-merges.sh
  rebase: strip unused code in git-rebase--preserve-merges.sh
  rebase: use the new git-rebase--preserve-merges.sh
  rebase: remove -p code from git-rebase--interactive.sh

 .gitignore |1 +
 Makefile   |2 +
 git-rebase--interactive.sh |  708 +---
 git-rebase--preserve-merges.sh | 1012 
 git-rebase.sh  |   32 +-
 5 files changed, 1048 insertions(+), 707 deletions(-)
 create mode 100644 git-rebase--preserve-merges.sh

-- 
2.16.1

bug: --shallow-since misbehaves on old branch heads

[Andreas Krey]

Hi everybody,

I think I have discovered a problem with clone/fetch --shallow-since:
When a ref that is fetches has a head that is already older than
the 'since' time, then the entire branch is fetched, instead of
just the current commit.

Repro:

  rm -rf tmp out deep
  git init tmp
  for i in `seq 10 31`; do
  d="2017-05-${i}T12:00"
  GIT_AUTOR_DATE="$d" GIT_COMMITTER_DATE="$d" git -C tmp commit -m nix$i 
--allow-empty
  done
  git -C tmp checkout -b test HEAD^^^
  for i in `seq 10 14`; do
  d="2017-06-${i}T12:00"
  GIT_AUTHOR_DATE="$d" GIT_COMMITTER_DATE="$d" git -C tmp commit -m nax$i 
--allow-empty
  done
  for i in `seq 1 4`; do
  git -C tmp commit -m new$i --allow-empty
  done

  echo "** This is fine:"

  git clone --shallow-since '1 month ago' file://`pwd`/tmp out --branch test
  git -C out log --oneline

  echo "** This should show one commit but shows all:"

  git clone --shallow-since '1 month ago' file://`pwd`/tmp deep --branch master
  git -C deep log --oneline

Do I expect the wrong thing?

- Andreas


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

Re: [GSoC][PATCH 1/4] rebase: duplicate git-rebase--interactive.sh to git-rebase--preserve-merges.sh

[Alban Gruin]

Hi Stefan,

Le 22/05/2018 à 20:26, Stefan Beller a écrit :
> On Tue, May 22, 2018 at 6:31 AM, Alban Gruin  wrote:
>> This duplicates git-rebase--interactive.sh to
>> git-rebase--preserve-merges.sh. This is done to split -p from -i. No
>> modifications are made to this file here, but any code that is not used by -p
>> will be stripped in the next commit.
>>
>> Signed-off-by: Alban Gruin 
> 
> So how would I best review this?
> 
> I applied the patches locally[1], and ran git-ls-tree on this commit
> hoping to find the same blob id for git-rebase--interactive.sh as for
> git-rebase--preserve-merges.sh; however I did not.
> 
> So I diffed them and had the diff below[2], which looks like that has parts
> of Johannes recent series?
> 

Thanks for the heads-up.

I started to work on that patch set on the master branch of git.git, and
I forgot to update git-rebase--preserve-merges.sh after rebasing on
Johannes’ branch.

So I’ll reroll the patch as soon as possible.

> Thanks,
> Stefan
> 

Re: [RFC PATCH] fetch-pack: space out sent "haves" in negotiation

[Stefan Beller]

Hi Jonathan,
> I wouldn't characterize the errors as "off by one errors".

Yes, I put it in quotes but realized that would not convey it very well.

>  They are
> more like...let me use a diagram:
>
> A
> |\
> B D
> | |
> C E
>
> Suppose we know that the server does not have A, has C, and may or may
> not have E (we sent "have E" but didn't get a response yet). My method
> restarts the walk at all the parents of A (that is, B and D), but D is
> irrelevant to the situation (and should not be walked over - this is the
> error).

D is irrelevant to the A, C situation, but it is still a useful probing point?
So I would not call it an error but an inefficiency?


> In this patch, I wrote the new algorithm and deleted the old one.
...
> You're proposing that if I proceed with this, I split the patch into 2 -
> one to move the negotiation algorithm, and one to update it? If yes,
> normally I would agree, but the current negotiation algorithm is not
> very sophisticated (and does not take up much code), so I think it's not
> worth it.

ok, in that case I'll just dive into the code.

>
>> > +struct fetch_negotiator {
>> > +   struct sent_commit **sent_commits;
>> > +   size_t sent_commit_nr, sent_commit_alloc;
>> > +   struct prio_queue candidates;
>> > +};
>>
>> Maybe we can just declare the struct fetch_negotiator here and not
>> define it, such that nobody outside the actual implementation tries
>> to access its internals?
>
> That's possible - I wanted to allow allocation of this on the stack (to
> save a malloc), but perhaps that's over-optimization.

Ah good call. Please keep it that way then.


>> So even if we do not use the skip commit logic, this would be a benefit for 
>> any
>> http(-v0) and v2 users of the protocol?
>
> It would conserve bandwidth, yes, but storing all the commits sent with
> additional metadata for each would require more memory.

I did not see the memory requirements here as a problem until now.
Are you saying this memory might be too much to keep around?

Stefan

Re: [PATCH v2 2/2] remote-curl: accept compressed responses with protocol v2

[Jonathan Nieder]

Brandon Williams wrote:

> Configure curl to accept compressed responses when using protocol v2 by
> setting `CURLOPT_ENCODING` to "", which indicates that curl should send
> an "Accept-Encoding" header with all supported compression encodings.
>
> Signed-off-by: Brandon Williams 
> ---
>  remote-curl.c | 1 +
>  1 file changed, 1 insertion(+)

Yay!

> diff --git a/remote-curl.c b/remote-curl.c
> index 565bba104..99b0bedc6 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -1259,6 +1259,7 @@ static int proxy_request(struct proxy_state *p)
>  
>   slot = get_active_slot();
>  
> + curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
>   curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
>   curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
>   curl_easy_setopt(slot->curl, CURLOPT_URL, p->service_url);

Can this get a test?

I'm particularly interested in it since it's easy to accidentally
apply this patch to the wrong duplicated place (luckily 'p' is a
different variable name than 'rpc' but it's an easy mistake to make if
applying the patch manually).

With or without such a test,
Reviewed-by: Jonathan Nieder 

Re: [PATCH v2 1/2] remote-curl: accept all encodings supported by curl

[Jonathan Nieder]

Brandon Williams wrote:

> Configure curl to accept all encodings which curl supports instead of
> only accepting gzip responses.
>
> This fixes an issue when using an installation of curl which is built
> without the "zlib" feature. Since aa90b9697 (Enable info/refs gzip
> decompression in HTTP client, 2012-09-19) we end up requesting "gzip"
> encoding anyway despite libcurl not being able to decode it.  Worse,
> instead of getting a clear error message indicating so, we end up
> falling back to "dumb" http, producing a confusing and difficult to
> debug result.
>
> Since curl doesn't do any checking to verify that it supports the a
> requested encoding, instead set the curl option `CURLOPT_ENCODING` with
> an empty string indicating that curl should send an "Accept-Encoding"
> header containing only the encodings supported by curl.

Even better, this means we get the benefit of future of even better
compression algorithms once libcurl learns them.

> Reported-by: Anton Golubev 
> Signed-off-by: Brandon Williams 
> ---
> Version 2 of this series just tweaks the commit message and the test per
> Jonathan's suggestion.
>
>  http.c  |  2 +-
>  remote-curl.c   |  2 +-
>  t/t5551-http-fetch-smart.sh | 13 +
>  3 files changed, 11 insertions(+), 6 deletions(-)

Reviewed-by: Jonathan Nieder 

Thanks for fixing it.

Patch left unsnipped for reference.

> --- a/http.c
> +++ b/http.c
> @@ -1788,7 +1788,7 @@ static int http_request(const char *url,
>  
>   curl_easy_setopt(slot->curl, CURLOPT_URL, url);
>   curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
> - curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
> + curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
>  
>   ret = run_one_slot(slot, );
>  
> diff --git a/remote-curl.c b/remote-curl.c
> index ceb05347b..565bba104 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -684,7 +684,7 @@ static int post_rpc(struct rpc_state *rpc)
>   curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
>   curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
>   curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
> - curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
> + curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
>  
>   if (large_request) {
>   /* The request body is large and the size cannot be predicted.
> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index f5721b4a5..913089b14 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -26,14 +26,14 @@ setup_askpass_helper
>  cat >exp <  > GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1
>  > Accept: */*
> -> Accept-Encoding: gzip
> +> Accept-Encoding: ENCODINGS
>  > Pragma: no-cache
>  < HTTP/1.1 200 OK
>  < Pragma: no-cache
>  < Cache-Control: no-cache, max-age=0, must-revalidate
>  < Content-Type: application/x-git-upload-pack-advertisement
>  > POST /smart/repo.git/git-upload-pack HTTP/1.1
> -> Accept-Encoding: gzip
> +> Accept-Encoding: ENCODINGS
>  > Content-Type: application/x-git-upload-pack-request
>  > Accept: application/x-git-upload-pack-result
>  > Content-Length: xxx
> @@ -79,8 +79,13 @@ test_expect_success 'clone http repository' '
>   /^< Date: /d
>   /^< Content-Length: /d
>   /^< Transfer-Encoding: /d
> - " >act &&
> - test_cmp exp act
> + " >actual &&
> + sed -e "s/^> Accept-Encoding: .*/> Accept-Encoding: ENCODINGS/" \
> + actual >actual.smudged &&
> + test_cmp exp actual.smudged &&
> +
> + grep "Accept-Encoding:.*gzip" actual >actual.gzip &&
> + test_line_count = 2 actual.gzip
>  '
>  
>  test_expect_success 'fetch changes via http' '

Re: [RFC PATCH] fetch-pack: space out sent "haves" in negotiation

[Jonathan Tan]

On Mon, 21 May 2018 15:57:18 -0700
Stefan Beller  wrote:

> In an ideal world, the server and client would both estimate the potential
> reduction of the packfile to send, and base the decision if to continue
> negotiating on the trade off if the packfile reduction savings are greater
> than the cost of negotiation (in terms of bandwidth or time).
> (e.g. the server could keep track of the "potential largest packfile to
> sent" as well as the "potential smallest packfile to sent" given the
> state of negotiation. And as soon as the difference between those
> two packs is smaller than the size of one round of negotiation,
> it is better to stop and just sent the large file).
> 
> You state that you do not want to change the server side, and stick to
> the current protocol, which makes this ideal world scenario moot, but
> shifts the problem to "picking haves more intelligently".

Thanks for thinking about this!

This requires a modification on the server side, as you said, but this
sounds like a good idea that can be combined with the approach in my
patch - once we've found a match, instead of the client restarting the
fine walk, the server could then send the hashes of the commits between
its tip and the match (or some subset thereof) and the client can send
the specific hashes it has.

> > I'm not sure if this is the best way,
> 
> I think it is the best for a short term gain, as the picking algorithm is
> not part of the protocol, so it can be easily extended/reverted/improved
> as we go. So I would continue this way.

That's true in that this can be subsequently modified without backwards
incompatibility - the only issue is the opportunity cost of the author
and reviewer's time and effort.

> > (1) The implementation that I have
> >
> > This patch contains some drop-in code that passes all existing tests,
> > but the new negotiation algorithm is not tested.
> >
> > To mitigate the effect of skipping, I included functionality wherein
> > the client will retry the commits in a skip if the server ACKs the
> > destination of the skip, but this is currently imperfect - in
> > particular, the server might end the negotiation early, and the commits
> > retried in my current implementation are a superset due to the fact that
> > I didn't store the commits in the skip.
> 
> So we start with exponential hops, fall back to linear probing and then
> "make off by one errors" in the linear probes?

I wouldn't characterize the errors as "off by one errors". They are
more like...let me use a diagram:

A
|\
B D
| |
C E

Suppose we know that the server does not have A, has C, and may or may
not have E (we sent "have E" but didn't get a response yet). My method
restarts the walk at all the parents of A (that is, B and D), but D is
irrelevant to the situation (and should not be walked over - this is the
error).

> > (3) Other ways of improving negotiation
> >
> > If we're prepared to commit-walk a significant part of the entire local
> > repo (as we are, in the situation I described in the first paragraph),
> 
> > and if we have access to corresponding remote-tracking information,
> 
> This is a dangerous assumption, as not everyone is having a 1:1 relationship
> with their remote server (for e.g. code review), but there are these triangle
> workflows in the kernel community for example, where you push in one
> remote direction and (re-)obtain the history merged into the bigger picture
> from another remote. And these two remotes are not special to each other
> on the client side.

Precisely for this reason (where the local repo could have obtained a
remote's commits through means other than through the remote - in this
case, written by the local repo's user themself) I wanted to include
both ancestors and descendants of the remote tracking tip.

> This patch is moving the algorithm driving the selection of new
> commits to pick to
> a new file, but there is no new algorithm, yet?
> As hinted at from (1), this is smarter than what we did before by
> picking commits
> non-linearly but with some sort of exponential back off, how does it end the
> exponential phase?

In this patch, I wrote the new algorithm and deleted the old one. I
think the answer to your question is that the exponential phase never
ends. If what you mean is what happens when we reach a parentless commit
- we will emit it (that is, send "have X") regardless of how many
commits have been skipped.

> The way forward out of RFC state, might be to separate the introduction of a 
> new
> improved algorithm and the refactoring. So first move code literally into the
> fetch-negotiator file, and then add improvements in there, or is it
> just not worth
> the refactoring and directly put in the new algorithm?

You're proposing that if I proceed with this, I split the patch into 2 -
one to move the negotiation algorithm, and one to update it? If yes,
normally I would agree, but the current negotiation algorithm is not
very sophisticated (and 

[PATCH v2 1/2] remote-curl: accept all encodings supported by curl

[Brandon Williams]

Configure curl to accept all encodings which curl supports instead of
only accepting gzip responses.

This fixes an issue when using an installation of curl which is built
without the "zlib" feature. Since aa90b9697 (Enable info/refs gzip
decompression in HTTP client, 2012-09-19) we end up requesting "gzip"
encoding anyway despite libcurl not being able to decode it.  Worse,
instead of getting a clear error message indicating so, we end up
falling back to "dumb" http, producing a confusing and difficult to
debug result.

Since curl doesn't do any checking to verify that it supports the a
requested encoding, instead set the curl option `CURLOPT_ENCODING` with
an empty string indicating that curl should send an "Accept-Encoding"
header containing only the encodings supported by curl.

Reported-by: Anton Golubev 
Signed-off-by: Brandon Williams 
---

Version 2 of this series just tweaks the commit message and the test per
Jonathan's suggestion.

 http.c  |  2 +-
 remote-curl.c   |  2 +-
 t/t5551-http-fetch-smart.sh | 13 +
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/http.c b/http.c
index fed13b216..709150fc7 100644
--- a/http.c
+++ b/http.c
@@ -1788,7 +1788,7 @@ static int http_request(const char *url,
 
curl_easy_setopt(slot->curl, CURLOPT_URL, url);
curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
-   curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
+   curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
 
ret = run_one_slot(slot, );
 
diff --git a/remote-curl.c b/remote-curl.c
index ceb05347b..565bba104 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -684,7 +684,7 @@ static int post_rpc(struct rpc_state *rpc)
curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
-   curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
+   curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
 
if (large_request) {
/* The request body is large and the size cannot be predicted.
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index f5721b4a5..913089b14 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -26,14 +26,14 @@ setup_askpass_helper
 cat >exp < GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1
 > Accept: */*
-> Accept-Encoding: gzip
+> Accept-Encoding: ENCODINGS
 > Pragma: no-cache
 < HTTP/1.1 200 OK
 < Pragma: no-cache
 < Cache-Control: no-cache, max-age=0, must-revalidate
 < Content-Type: application/x-git-upload-pack-advertisement
 > POST /smart/repo.git/git-upload-pack HTTP/1.1
-> Accept-Encoding: gzip
+> Accept-Encoding: ENCODINGS
 > Content-Type: application/x-git-upload-pack-request
 > Accept: application/x-git-upload-pack-result
 > Content-Length: xxx
@@ -79,8 +79,13 @@ test_expect_success 'clone http repository' '
/^< Date: /d
/^< Content-Length: /d
/^< Transfer-Encoding: /d
-   " >act &&
-   test_cmp exp act
+   " >actual &&
+   sed -e "s/^> Accept-Encoding: .*/> Accept-Encoding: ENCODINGS/" \
+   actual >actual.smudged &&
+   test_cmp exp actual.smudged &&
+
+   grep "Accept-Encoding:.*gzip" actual >actual.gzip &&
+   test_line_count = 2 actual.gzip
 '
 
 test_expect_success 'fetch changes via http' '
-- 
2.17.0.441.gb46fe60e1d-goog

[PATCH v2 2/2] remote-curl: accept compressed responses with protocol v2

[Brandon Williams]

Configure curl to accept compressed responses when using protocol v2 by
setting `CURLOPT_ENCODING` to "", which indicates that curl should send
an "Accept-Encoding" header with all supported compression encodings.

Signed-off-by: Brandon Williams 
---
 remote-curl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/remote-curl.c b/remote-curl.c
index 565bba104..99b0bedc6 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -1259,6 +1259,7 @@ static int proxy_request(struct proxy_state *p)
 
slot = get_active_slot();
 
+   curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
curl_easy_setopt(slot->curl, CURLOPT_URL, p->service_url);
-- 
2.17.0.441.gb46fe60e1d-goog

Re: [PATCH 1/2] remote-curl: accept all encoding supported by curl

[Brandon Williams]

On 05/22, Daniel Stenberg wrote:
> On Mon, 21 May 2018, Jonathan Nieder wrote:
> 
> > > Looking at the code here, this succeeds if enough memory is available.
> > > There is no check if the given parameter is part of
> > > Curl_all_content_encodings();
> > 
> > By "this" are you referring to the preimage or the postimage?  Are you
> > suggesting a change in git or in libcurl?
> > 
> > Curl_all_content_encodings() is an internal function in libcurl, so I'm
> > assuming the latter.
> 
> Ack, that certainly isn't the most wonderful API for selecting a compression
> method. In reality, almost everyone sticks to passing on a "" to that option
> to let libcurl pick and ask for the compression algos it knows since both
> gzip and brotli are present only conditionally depending on build options.

Thanks for the clarification.  Sounds like the best option is to
continue with this patch and let curl decide using "".

> 
> I would agree that the libcurl setopt call should probably be made to fail
> if asked to use a compression method not built-in/supported. Then an
> application could in fact try different algos in order until one works or
> ask to disable compression completely.
> 
> In the generic HTTP case, it usually makes sense to ask for more than one
> algorthim though, since this is asking the server for a compressed version
> and typically a HTTP client doesn't know which compression methods the
> server offers. Not sure this is actually true to the same extent for git.
> 
> -- 
> 
>  / daniel.haxx.se

-- 
Brandon Williams

Re: Why do we have both x*() and *_or_die() for "do or die"?

[Jonathan Nieder]

Duy Nguyen wrote:
> On Tue, May 22, 2018 at 7:49 PM, Ævar Arnfjörð Bjarmason
>  wrote:

>> Just a side-question unrelated to this patch per-se, why do we have both
>> x*() and *_or_die() functions in the codebase?
>
> I wondered about that myself shortly after suggesting
> repo_read_index_or_die(). My only guess is xfoo is better version of
> foo, which sometimes involves dying inside but that's not the only
> possible improvement. Later I guess people go with _or_die() more
> because it describes what the function does much better.

In particular, there are functions like xwrite that don't die on error
and write_or_die that do.

I'd probably be in favor of a series using cocinelle to rename the
functions that do die on error to _or_die.  The main case where I
pause for a moment is xmalloc, since I'm worried about the verboseness
of malloc_or_die, but I suspect I would get used to it.

Thanks,
Jonathan

Re: [GSoC][PATCH 0/4] rebase: split rebase -p from rebase -i

[Stefan Beller]

On Tue, May 22, 2018 at 6:31 AM, Alban Gruin  wrote:
> This splits the `rebase --preserve-merges` functionnality from
> git-rebase--interactive.sh. This is part of the effort to depreciate
> preserve-merges. The new script, git-rebase--preserve-merges.sh, should be 
> left
> to bitrot. All the dead code left by the duplication of
> git-rebase--interactive.sh is also removed.

... and I thought the original motivation was getting the rest of rebase
into a shape that rewriting it is easier, the potential bit rot of
--preserve-merges
is rather a side effect, but not the main goal.

I commented on patch 1, as I don't quite understand the changes,
the other patches look good to me.

Thanks,
Stefan

Re: [GSoC][PATCH 1/4] rebase: duplicate git-rebase--interactive.sh to git-rebase--preserve-merges.sh

[Stefan Beller]

On Tue, May 22, 2018 at 6:31 AM, Alban Gruin  wrote:
> This duplicates git-rebase--interactive.sh to
> git-rebase--preserve-merges.sh. This is done to split -p from -i. No
> modifications are made to this file here, but any code that is not used by -p
> will be stripped in the next commit.
>
> Signed-off-by: Alban Gruin 

So how would I best review this?

I applied the patches locally[1], and ran git-ls-tree on this commit
hoping to find the same blob id for git-rebase--interactive.sh as for
git-rebase--preserve-merges.sh; however I did not.

So I diffed them and had the diff below[2], which looks like that has parts
of Johannes recent series?

Thanks,
Stefan

[1] After applying I pushed it to
https://github.com/stefanbeller/git/tree/alban_split_off_-p
and this patch is commit 9f64342ea1b64d43e4675b5f202174c9e0f77dbf

[2] $ git diff 
9f64342ea1b64d43e4675b5f202174c9e0f77dbf:git-rebase--preserve-merges.sh
\
9f64342ea1b64d43e4675b5f202174c9e0f77dbf:git-rebase--interactive.sh
diff --git a/git-rebase--preserve-merges.sh b/git-rebase--interactive.sh
index 9947e6265fe..2f4941d0fc9 100644
--- a/git-rebase--preserve-merges.sh
+++ b/git-rebase--interactive.sh
@@ -155,13 +155,19 @@ reschedule_last_action () {
 append_todo_help () {
gettext "
 Commands:
-p, pick = use commit
-r, reword = use commit, but edit the commit message
-e, edit = use commit, but stop for amending
-s, squash = use commit, but meld into previous commit
-f, fixup = like \"squash\", but discard this commit's log message
-x, exec = run command (the rest of the line) using shell
-d, drop = remove commit
+p, pick  = use commit
+r, reword  = use commit, but edit the commit message
+e, edit  = use commit, but stop for amending
+s, squash  = use commit, but meld into previous commit
+f, fixup  = like \"squash\", but discard this commit's log message
+x, exec  = run command (the rest of the line) using shell
+d, drop  = remove commit
+l, label  = label current HEAD with a name
+t, reset  = reset HEAD to a label
+m, merge [-C  | -c ]  [# ]
+.   create a merge commit using the original merge commit's
+.   message (or the oneline, if no original merge commit was
+.   specified). Use -c  to reword the commit message.

 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
@@ -888,6 +894,8 @@ init_revisions_and_shortrevisions () {
else
revisions=$onto...$orig_head
shortrevisions=$shorthead
+   test -z "$squash_onto" ||
+   echo "$squash_onto" >"$state_dir"/squash-onto
fi
 }

@@ -942,7 +950,7 @@ EOF
die "Could not skip unnecessary pick commands"

checkout_onto
-   if test -z "$rebase_root" && test ! -d "$rewritten"
+   if test ! -d "$rewritten"
then
require_clean_work_tree "rebase"
exec git rebase--helper ${force_rebase:+--no-ff}
$allow_empty_message \
@@ -964,6 +972,8 @@ git_rebase__interactive () {
init_revisions_and_shortrevisions

git rebase--helper --make-script ${keep_empty:+--keep-empty} \
+   ${rebase_merges:+--rebase-merges} \
+   ${rebase_cousins:+--rebase-cousins} \
$revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
die "$(gettext "Could not generate todo list")"

Re: Why do we have both x*() and *_or_die() for "do or die"?

[Stefan Beller]

On Tue, May 22, 2018 at 10:49 AM, Ævar Arnfjörð Bjarmason
 wrote:
>
> On Wed, May 16 2018, Stefan Beller wrote:
>
>> A common pattern with the repo_read_index function is to die if the return
>> of repo_read_index is negative.  Move this pattern into a function.

This was done organically, i.e. taking the smallest available step that yields
a better code base. There was definitely no stepping back or looking at the
big picture involved.

> Just a side-question unrelated to this patch per-se, why do we have both
> x*() and *_or_die() functions in the codebase?

Digging into the history, the x*() emerges from 112db553b0d (Shrink the
git binary a bit by avoiding unnecessary inline functions, 2008-06-22) and
is wrapping common patterns to reduce the size of the binary.

The or_die pattern comes from 7230e6d042a (Add write_or_die(),
a helper function, 2006-08-21). This was motivated as better code
("making additional error handling unnecessary", "replace two similar ones")

None of them really hint at the _die() as a problem in the
libification efforts which came later IIUC.

> I can't find any pattern
> for one or the other, e.g. we have both xopen() and then write_or_die(),

hah! Up to now I assumed the x* is system calls with write_or_die as an
exception. There is also a xwrite and write_in_full, so they are slightly
different in the way that xwrite covers only the system call and copes
with some common error patterns, whereas write_or_die uses write_in_full
which itself uses xwrite.

> so it's not a matter of x*() just being for syscalls and *_or_die()
> being for our own functions (also as e.g. strbuf uses x*(), not
> *_or_die()).

I would tend to argue that way: x* are strictly to system calls (no die()
in there), and the _die() functions are "hands off *no* error handling
needed"

> I'm not trying to litigate the difference and understand it could have
> just emerged organically. I'm just wondering if that's the full story or
> if one is preferred, or we prefer one or the other in some
> circumstances.

I think we'd prefer the x() for lib code and the _or_die code in builtin/
due to the nature of effort needed to get it right.

Thanks,
Stefan

Re: Why do we have both x*() and *_or_die() for "do or die"?

[Duy Nguyen]

On Tue, May 22, 2018 at 7:49 PM, Ævar Arnfjörð Bjarmason
 wrote:
>
> On Wed, May 16 2018, Stefan Beller wrote:
>
>> A common pattern with the repo_read_index function is to die if the return
>> of repo_read_index is negative.  Move this pattern into a function.
>
> Just a side-question unrelated to this patch per-se, why do we have both
> x*() and *_or_die() functions in the codebase?

I wondered about that myself shortly after suggesting
repo_read_index_or_die(). My only guess is xfoo is better version of
foo, which sometimes involves dying inside but that's not the only
possible improvement. Later I guess people go with _or_die() more
because it describes what the function does much better.

> I can't find any pattern
> for one or the other, e.g. we have both xopen() and then write_or_die(),
> so it's not a matter of x*() just being for syscalls and *_or_die()
> being for our own functions (also as e.g. strbuf uses x*(), not
> *_or_die()).
>
> I'm not trying to litigate the difference and understand it could have
> just emerged organically. I'm just wondering if that's the full story or
> if one is preferred, or we prefer one or the other in some
> circumstances.



-- 
Duy

Why do we have both x*() and *_or_die() for "do or die"?

[Ævar Arnfjörð Bjarmason]

On Wed, May 16 2018, Stefan Beller wrote:

> A common pattern with the repo_read_index function is to die if the return
> of repo_read_index is negative.  Move this pattern into a function.

Just a side-question unrelated to this patch per-se, why do we have both
x*() and *_or_die() functions in the codebase? I can't find any pattern
for one or the other, e.g. we have both xopen() and then write_or_die(),
so it's not a matter of x*() just being for syscalls and *_or_die()
being for our own functions (also as e.g. strbuf uses x*(), not
*_or_die()).

I'm not trying to litigate the difference and understand it could have
just emerged organically. I'm just wondering if that's the full story or
if one is preferred, or we prefer one or the other in some
circumstances.

Officially supported Git versions

[Patrick Lühne]

Hi,

Is there an official list of the Git versions that are still actively
supported? According to hearsay from colleagues, the latest five release
series receive security patches. I can’t find a source for that, but
might that be correct?

There’s also the Wikipedia page on Git [1], but it doesn’t point to a
proper source either.

According to Wikipedia, versions 2.4.x to 2.9.x are still supported.
This surprised me, because the fix for CVE-2017-14867 [2] hasn’t been
backported to versions earlier than 2.10 if I see that correctly.
CVE-2017-14867 was fixed for Git series 2.10.x and newer on September
22, 2017, and publicly disclosed on September 29, 2017. However, the
latest releases for the 2.7.x, 2.8.x, and 2.9.x series date back to July
30, 2017 (and 2.4.x hasn’t been touched since September 4, 2015).

Best wishes,
Patrick




[1] https://en.wikipedia.org/wiki/Git#Releases

[2] https://www.cvedetails.com/cve/CVE-2017-14867/



signature.asc
Description: OpenPGP digital signature

Re: should config options be treated as case-sensitive?

[Robert P. J. Day]

On Tue, 22 May 2018, Ævar Arnfjörð Bjarmason wrote:

>
> On Tue, May 22 2018, Robert P. J. Day wrote:
>
> >   in my wanderings, more oddities, such as this:
> >
> > $ grep -ir blankboundary *
> > builtin/blame.c:if (!strcmp(var, "blame.blankboundary")) {
> > Documentation/config.txt:blame.blankBoundary::
> > Documentation/blame-options.txt:be controlled via the 
> > `blame.blankboundary` config option.
> > Documentation/RelNotes/2.15.1.txt: * Description of 
> > blame.{showroot,blankboundary,showemail,date}
> > Documentation/RelNotes/2.16.0.txt: * Description of 
> > blame.{showroot,blankboundary,showemail,date}
> > $
> >
> > where you can see the single instance of "blankBoundary" in
> > Doc/config.txt (with the upper case 'B'), while the rest have no such
> > thing.
> >
> >   for fun, i checked another of blame's config settings, with similar
> > results:
> >
> > builtin/blame.c:if (!strcmp(var, "blame.showemail")) {
> > Documentation/config.txt:blame.showEmail::
> > Documentation/RelNotes/2.15.1.txt: * Description of 
> > blame.{showroot,blankboundary,showemail,date}
> > Documentation/RelNotes/2.5.0.txt: * "git blame" learned blame.showEmail 
> > configuration variable.
> > Documentation/RelNotes/2.16.0.txt: * Description of 
> > blame.{showroot,blankboundary,showemail,date}
> > Documentation/git-blame.txt:This can also be controlled via the 
> > `blame.showEmail` config
> > t/t8002-blame.sh:test_expect_success 'setup showEmail tests' '
> > t/t8002-blame.sh:test_expect_success 'blame with showemail options' '
> > t/t8002-blame.sh:test_expect_success 'blame with showEmail config false' '
> > t/t8002-blame.sh:   git config blame.showEmail false &&
> > t/t8002-blame.sh:test_expect_success 'blame with showEmail config true' '
> > t/t8002-blame.sh:   git config blame.showEmail true &&
> >
> >   thoughts?
>
> The issues you note about the docs using foo.barbaz instead of
> foo.barBaz should be fixed, but as noted in the "Syntax" section of
> "git-config" we already document that the config keys are all
> case-insensitive. We just like talking about them as foo.barBaz
> because it makes for easier reading.

  ah, got it, so really, the only example above that would merit
tweaking would be:

  blame-options.txt:  be controlled via the `blame.blankboundary` config option.

i'll collect those in my travels and submit them all at once.

rday

Re: [PATCH 02/11] repository: introduce repo_read_index_or_die

[Duy Nguyen]

On Mon, May 21, 2018 at 9:27 PM, Stefan Beller  wrote:
> Hi Brandon,
>
>>> One of the reviewers of the series questioned the overall goal of the
>>> series as we want to move away from _die() in top level code but this
>>> series moves more towards it.
>>
>> I've heard every once in a while that we want to move toward this but I
>> don't believe there is an actual effort along those lines just yet.  For
>> that to be the case we would need a clearly defined error handling
>> methodology (aside from the existing "die"ing behavior), which we don't
>> currently have.
>
> We have the example in the refs code, which I would want to
> imitate. :)
>
> /*
>  * Return 0 if a reference named refname could be created without
>  * conflicting with the name of an existing reference. Otherwise,
>  * return a negative value and write an explanation to err. [...]
>  */
>
> int refs_verify_refname_available(struct ref_store *refs, ...
> struct strbuf *err);
>
> extern int refs_init_db(struct strbuf *err);

apply.c and sequencer.c too are libified and try hard to avoid die()
if I remember correctly.

> But it is true that there is no active effort currently being pushed.

Yeah. It took a lot of work to put refs code in the current shape
today. I think we just be careful about adding die(). If a function
has no way to return an error, then die() is unavoidable. If it's a
really fatal error, then die() might still be the right choice. But if
not we should try to return an error instead.

Speaking of avoiding die() (and going off-topic again), maybe we
should introduce NO_DIE_PLEASE macro in the same spirit as
NO_THE_INDEX_COMPATIBILITY_MACROS. If the macro is defined, die() and
friends are not declared (which will result in compile errors at least
on DEVELOPER=1 builds). This helps draw boundary between die-able code
and un-die-able code (not perfect, but good enough)
-- 
Duy

Re: [curl PATCH 2/2] ignore SIGPIPE during curl_multi_cleanup

[Daniel Stenberg]

On Tue, 22 May 2018, Suganthi wrote:

We may not be able to upgrade to 7.60.0 any soon, Is the fix present in 7.45 
, in this sequence of code. Please let me know.


I don't know.

I can't recall any SIGPIPE problems in recent days in libcurl, which is why I 
believe this problem doesn't exist anymore. libcurl 7.45.0 is 2.5 years and 
1500+ bug fixes old after all. My casual searches for a curl problem like this 
- fixed in 7.45.0 or later - also failed.


--

 / daniel.haxx.se

Re: [curl PATCH 2/2] ignore SIGPIPE during curl_multi_cleanup

[curlUser]

We may not be able to upgrade to 7.60.0 any soon, 
Is the fix present in 7.45 , in this sequence of code. 
Please let me know. 



--
Sent from: http://git.661346.n2.nabble.com/

Re: user name

[Dennis Powless]

Thanks for the info, yes your are correct it was for the git config
not github.  I'm brand new to this and wasn't sure if a real name was
required vs just a username.  I know is some cases official names are
required and didn't want to use the wrong one to start out with.  I'm
reading the help files/information currently and one of the first
steps was to set this up.

I'll set up github separately.


Thanks for the info!

Dennis

On Tue, May 22, 2018 at 9:45 AM, Christian Couder
 wrote:
> On Tue, May 22, 2018 at 3:06 PM, Dennis Powless  wrote:
>> Is it customary to use your real name or a user name when registering to GIT?
>
> I guess you are talking about using `git config --global user.name
> "XXX YYY"`. (Though maybe you are talking about github.com
> registration, but in this case you should not have sent this request
> to this mailing list.)
>
> When you use git config as described above, this is not a real
> registration. You just configure Git on your machine (only your
> ~/.gitconfig is changed), so that Git knows what to put in the
> "author" field of the commits you create.
>
> You are free to use whatever you want but some projects might ask
> contributors to put their real name in the author fields of the
> commits (and sometimes in other places too, like in the
> "Signed-off-by" in the commit messages).

Re: user name

[Christian Couder]

On Tue, May 22, 2018 at 3:06 PM, Dennis Powless  wrote:
> Is it customary to use your real name or a user name when registering to GIT?

I guess you are talking about using `git config --global user.name
"XXX YYY"`. (Though maybe you are talking about github.com
registration, but in this case you should not have sent this request
to this mailing list.)

When you use git config as described above, this is not a real
registration. You just configure Git on your machine (only your
~/.gitconfig is changed), so that Git knows what to put in the
"author" field of the commits you create.

You are free to use whatever you want but some projects might ask
contributors to put their real name in the author fields of the
commits (and sometimes in other places too, like in the
"Signed-off-by" in the commit messages).

[GSoC][PATCH 2/4] rebase: strip unused code in git-rebase--preserve-merges.sh

[Alban Gruin]

This removes the code coming from git-rebase--interactive.sh that is not needed
by preserve-merges.

Signed-off-by: Alban Gruin 
---
 git-rebase--preserve-merges.sh | 63 +++---
 1 file changed, 4 insertions(+), 59 deletions(-)

diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
index 9947e6265..72438 100644
--- a/git-rebase--preserve-merges.sh
+++ b/git-rebase--preserve-merges.sh
@@ -1,12 +1,8 @@
-# This shell script fragment is sourced by git-rebase to implement
-# its interactive mode.  "git rebase --interactive" makes it easy
-# to fix up commits in the middle of a series and rearrange commits.
+# This shell script fragment is sourced by git-rebase to implement its
+# preserve-merges mode.
 #
 # Copyright (c) 2006 Johannes E. Schindelin
 #
-# The original idea comes from Eric W. Biederman, in
-# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
-#
 # The file containing rebase commands, comments, and empty lines.
 # This file is created by "git rebase -i" then edited by the user.  As
 # the lines are processed, they are removed from the front of this
@@ -281,17 +277,7 @@ pick_one () {
empty_args="--allow-empty"
fi
 
-   test -d "$rewritten" &&
-   pick_one_preserving_merges "$@" && return
-   output eval git cherry-pick $allow_rerere_autoupdate 
$allow_empty_message \
-   ${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
-   $signoff "$strategy_args" $empty_args $ff "$@"
-
-   # If cherry-pick dies it leaves the to-be-picked commit unrecorded. 
Reschedule
-   # previous task so this commit is not lost.
-   ret=$?
-   case "$ret" in [01]) ;; *) reschedule_last_action ;; esac
-   return $ret
+   pick_one_preserving_merges "$@"
 }
 
 pick_one_preserving_merges () {
@@ -755,11 +741,6 @@ get_missing_commit_check_level () {
 initiate_action () {
case "$1" in
continue)
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
# do we have anything to commit?
if git diff-index --cached --quiet HEAD --
then
@@ -818,12 +799,6 @@ first and then run 'git rebase --continue' again.")"
;;
skip)
git rerere clear
-
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
do_rest
return 0
;;
@@ -936,41 +911,11 @@ EOF
}
 
expand_todo_ids
-
-   test -d "$rewritten" || test -n "$force_rebase" ||
-   onto="$(git rebase--helper --skip-unnecessary-picks)" ||
-   die "Could not skip unnecessary pick commands"
-
checkout_onto
-   if test -z "$rebase_root" && test ! -d "$rewritten"
-   then
-   require_clean_work_tree "rebase"
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
do_rest
 }
 
-git_rebase__interactive () {
-   initiate_action "$action"
-   ret=$?
-   if test $ret = 0; then
-   return 0
-   fi
-
-   setup_reflog_action
-   init_basic_state
-
-   init_revisions_and_shortrevisions
-
-   git rebase--helper --make-script ${keep_empty:+--keep-empty} \
-   $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
-   die "$(gettext "Could not generate todo list")"
-
-   complete_action
-}
-
-git_rebase__interactive__preserve_merges () {
+git_rebase__preserve_merges () {
initiate_action "$action"
ret=$?
if test $ret = 0; then
-- 
2.16.1

[GSoC][PATCH 1/4] rebase: duplicate git-rebase--interactive.sh to git-rebase--preserve-merges.sh

[Alban Gruin]

This duplicates git-rebase--interactive.sh to
git-rebase--preserve-merges.sh. This is done to split -p from -i. No
modifications are made to this file here, but any code that is not used by -p
will be stripped in the next commit.

Signed-off-by: Alban Gruin 
---
 .gitignore |1 +
 Makefile   |2 +
 git-rebase--preserve-merges.sh | 1059 
 3 files changed, 1062 insertions(+)
 create mode 100644 git-rebase--preserve-merges.sh

diff --git a/.gitignore b/.gitignore
index 833ef3b0b..ef4925485 100644
--- a/.gitignore
+++ b/.gitignore
@@ -117,6 +117,7 @@
 /git-rebase--helper
 /git-rebase--interactive
 /git-rebase--merge
+/git-rebase--preserve-merges
 /git-receive-pack
 /git-reflog
 /git-remote
diff --git a/Makefile b/Makefile
index 50da82b01..810a0d0f4 100644
--- a/Makefile
+++ b/Makefile
@@ -582,6 +582,7 @@ SCRIPT_LIB += git-mergetool--lib
 SCRIPT_LIB += git-parse-remote
 SCRIPT_LIB += git-rebase--am
 SCRIPT_LIB += git-rebase--interactive
+SCRIPT_LIB += git-rebase--preserve-merges
 SCRIPT_LIB += git-rebase--merge
 SCRIPT_LIB += git-sh-setup
 SCRIPT_LIB += git-sh-i18n
@@ -2271,6 +2272,7 @@ LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
 LOCALIZED_SH = $(SCRIPT_SH)
 LOCALIZED_SH += git-parse-remote.sh
 LOCALIZED_SH += git-rebase--interactive.sh
+LOCALIZED_SH += git-rebase--preserve-merges.sh
 LOCALIZED_SH += git-sh-setup.sh
 LOCALIZED_PERL = $(SCRIPT_PERL)
 
diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
new file mode 100644
index 0..9947e6265
--- /dev/null
+++ b/git-rebase--preserve-merges.sh
@@ -0,0 +1,1059 @@
+# This shell script fragment is sourced by git-rebase to implement
+# its interactive mode.  "git rebase --interactive" makes it easy
+# to fix up commits in the middle of a series and rearrange commits.
+#
+# Copyright (c) 2006 Johannes E. Schindelin
+#
+# The original idea comes from Eric W. Biederman, in
+# https://public-inbox.org/git/m1odwkyuf5.fsf...@ebiederm.dsl.xmission.com/
+#
+# The file containing rebase commands, comments, and empty lines.
+# This file is created by "git rebase -i" then edited by the user.  As
+# the lines are processed, they are removed from the front of this
+# file and written to the tail of $done.
+todo="$state_dir"/git-rebase-todo
+
+# The rebase command lines that have already been processed.  A line
+# is moved here when it is first handled, before any associated user
+# actions.
+done="$state_dir"/done
+
+# The commit message that is planned to be used for any changes that
+# need to be committed following a user interaction.
+msg="$state_dir"/message
+
+# The file into which is accumulated the suggested commit message for
+# squash/fixup commands.  When the first of a series of squash/fixups
+# is seen, the file is created and the commit message from the
+# previous commit and from the first squash/fixup commit are written
+# to it.  The commit message for each subsequent squash/fixup commit
+# is appended to the file as it is processed.
+#
+# The first line of the file is of the form
+# # This is a combination of $count commits.
+# where $count is the number of commits whose messages have been
+# written to the file so far (including the initial "pick" commit).
+# Each time that a commit message is processed, this line is read and
+# updated.  It is deleted just before the combined commit is made.
+squash_msg="$state_dir"/message-squash
+
+# If the current series of squash/fixups has not yet included a squash
+# command, then this file exists and holds the commit message of the
+# original "pick" commit.  (If the series ends without a "squash"
+# command, then this can be used as the commit message of the combined
+# commit without opening the editor.)
+fixup_msg="$state_dir"/message-fixup
+
+# $rewritten is the name of a directory containing files for each
+# commit that is reachable by at least one merge base of $head and
+# $upstream. They are not necessarily rewritten, but their children
+# might be.  This ensures that commits on merged, but otherwise
+# unrelated side branches are left alone. (Think "X" in the man page's
+# example.)
+rewritten="$state_dir"/rewritten
+
+dropped="$state_dir"/dropped
+
+end="$state_dir"/end
+msgnum="$state_dir"/msgnum
+
+# A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
+# GIT_AUTHOR_DATE that will be used for the commit that is currently
+# being rebased.
+author_script="$state_dir"/author-script
+
+# When an "edit" rebase command is being processed, the SHA1 of the
+# commit to be edited is recorded in this file.  When "git rebase
+# --continue" is executed, if there are any staged changes then they
+# will be amended to the HEAD commit, but only provided the HEAD
+# commit is still the commit to be edited.  When any other rebase
+# command is processed, this file is deleted.
+amend="$state_dir"/amend
+
+# For the post-rewrite hook, we make a list of rewritten 

[GSoC][PATCH 4/4] rebase: remove -p code from git-rebase--interactive.sh

[Alban Gruin]

All the code specific to preserve-merges was moved to
git-rebase--preserve-merges.sh, and so it’s useless to keep it here.

Signed-off-by: Alban Gruin 
---
 git-rebase--interactive.sh | 708 +
 1 file changed, 8 insertions(+), 700 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2f4941d0f..657d32773 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -105,15 +105,6 @@ case "$comment_char" in
;;
 esac
 
-warn () {
-   printf '%s\n' "$*" >&2
-}
-
-# Output the commit message for the specified commit.
-commit_message () {
-   git cat-file commit "$1" | sed "1,/^$/d"
-}
-
 orig_reflog_action="$GIT_REFLOG_ACTION"
 
 comment_for_reflog () {
@@ -142,16 +133,6 @@ mark_action_done () {
fi
 }
 
-# Put the last action marked done at the beginning of the todo list
-# again. If there has not been an action marked done yet, leave the list of
-# items on the todo list unchanged.
-reschedule_last_action () {
-   tail -n 1 "$done" | cat - "$todo" >"$todo".new
-   sed -e \$d <"$done" >"$done".new
-   mv -f "$todo".new "$todo"
-   mv -f "$done".new "$done"
-}
-
 append_todo_help () {
gettext "
 Commands:
@@ -184,50 +165,6 @@ If you remove a line here THAT COMMIT WILL BE LOST.
fi
 }
 
-make_patch () {
-   sha1_and_parents="$(git rev-list --parents -1 "$1")"
-   case "$sha1_and_parents" in
-   ?*' '?*' '?*)
-   git diff --cc $sha1_and_parents
-   ;;
-   ?*' '?*)
-   git diff-tree -p "$1^!"
-   ;;
-   *)
-   echo "Root commit"
-   ;;
-   esac > "$state_dir"/patch
-   test -f "$msg" ||
-   commit_message "$1" > "$msg"
-   test -f "$author_script" ||
-   get_author_ident_from_commit "$1" > "$author_script"
-}
-
-die_with_patch () {
-   echo "$1" > "$state_dir"/stopped-sha
-   git update-ref REBASE_HEAD "$1"
-   make_patch "$1"
-   die "$2"
-}
-
-exit_with_patch () {
-   echo "$1" > "$state_dir"/stopped-sha
-   git update-ref REBASE_HEAD "$1"
-   make_patch $1
-   git rev-parse --verify HEAD > "$amend"
-   gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")}
-   warn "$(eval_gettext "\
-You can amend the commit now, with
-
-   git commit --amend \$gpg_sign_opt_quoted
-
-Once you are satisfied with your changes, run
-
-   git rebase --continue")"
-   warn
-   exit $2
-}
-
 die_abort () {
apply_autostash
rm -rf "$state_dir"
@@ -238,30 +175,6 @@ has_action () {
test -n "$(git stripspace --strip-comments <"$1")"
 }
 
-is_empty_commit() {
-   tree=$(git rev-parse -q --verify "$1"^{tree} 2>/dev/null) || {
-   sha1=$1
-   die "$(eval_gettext "\$sha1: not a commit that can be picked")"
-   }
-   ptree=$(git rev-parse -q --verify "$1"^^{tree} 2>/dev/null) ||
-   ptree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-   test "$tree" = "$ptree"
-}
-
-is_merge_commit()
-{
-   git rev-parse --verify --quiet "$1"^2 >/dev/null 2>&1
-}
-
-# Run command with GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
-# GIT_AUTHOR_DATE exported from the current environment.
-do_with_author () {
-   (
-   export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE
-   "$@"
-   )
-}
-
 git_sequence_editor () {
if test -z "$GIT_SEQUENCE_EDITOR"
then
@@ -275,455 +188,6 @@ git_sequence_editor () {
eval "$GIT_SEQUENCE_EDITOR" '"$@"'
 }
 
-pick_one () {
-   ff=--ff
-
-   case "$1" in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac
-   case "$force_rebase" in '') ;; ?*) ff= ;; esac
-   output git rev-parse --verify $sha1 || die "$(eval_gettext "Invalid 
commit name: \$sha1")"
-
-   if is_empty_commit "$sha1"
-   then
-   empty_args="--allow-empty"
-   fi
-
-   test -d "$rewritten" &&
-   pick_one_preserving_merges "$@" && return
-   output eval git cherry-pick $allow_rerere_autoupdate 
$allow_empty_message \
-   ${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
-   $signoff "$strategy_args" $empty_args $ff "$@"
-
-   # If cherry-pick dies it leaves the to-be-picked commit unrecorded. 
Reschedule
-   # previous task so this commit is not lost.
-   ret=$?
-   case "$ret" in [01]) ;; *) reschedule_last_action ;; esac
-   return $ret
-}
-
-pick_one_preserving_merges () {
-   fast_forward=t
-   case "$1" in
-   -n)
-   fast_forward=f
-   sha1=$2
-   ;;
-   *)
-   sha1=$1
-   ;;
-   esac
-   sha1=$(git rev-parse $sha1)
-
-   if test -f "$state_dir"/current-commit && test "$fast_forward" = t
-   then
-   while read current_commit
- 

[GSoC][PATCH 3/4] rebase: use the new git-rebase--preserve-merges.sh

[Alban Gruin]

Creates a new type of rebase, "preserve-merges", used when rebase is called with
-p.

Before that, the type for preserve-merges was "interactive", and some places of
this script compared $type to "interactive". Instead, the code now checks if
$interactive_rebase is empty or not, as it is set to "explicit" when calling an
interactive rebase (and, possibly, one of its submodes), and "implied" when
calling one of its submodes (eg. preserve-merges) *without* interactive rebase.

It also detects the presence of the directory "$merge_dir"/rewritten left by the
preserve-merges script when calling rebase --continue, --skip, etc., and, if it
exists, sets the rebase mode to preserve-merges. In this case,
interactive_rebase is set to "explicit", as "implied" would break some tests.

Signed-off-by: Alban Gruin 
---
 git-rebase.sh | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 40be59ecc..19bdebb48 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -207,7 +207,14 @@ run_specific_rebase () {
autosquash=
fi
. git-rebase--$type
-   git_rebase__$type${preserve_merges:+__preserve_merges}
+
+   if test -z "$preserve_merges"
+   then
+   git_rebase__$type
+   else
+   git_rebase__preserve_merges
+   fi
+
ret=$?
if test $ret -eq 0
then
@@ -239,7 +246,12 @@ then
state_dir="$apply_dir"
 elif test -d "$merge_dir"
 then
-   if test -f "$merge_dir"/interactive
+   if test -d "$merge_dir"/rewritten
+   then
+   type=preserve-merges
+   interactive_rebase=explicit
+   preserve_merges=t
+   elif test -f "$merge_dir"/interactive
then
type=interactive
interactive_rebase=explicit
@@ -402,14 +414,14 @@ if test -n "$action"
 then
test -z "$in_progress" && die "$(gettext "No rebase in progress?")"
# Only interactive rebase uses detailed reflog messages
-   if test "$type" = interactive && test "$GIT_REFLOG_ACTION" = rebase
+   if test -n "$interactive_rebase" && test "$GIT_REFLOG_ACTION" = rebase
then
GIT_REFLOG_ACTION="rebase -i ($action)"
export GIT_REFLOG_ACTION
fi
 fi
 
-if test "$action" = "edit-todo" && test "$type" != "interactive"
+if test "$action" = "edit-todo" && test -z "$interactive_rebase"
 then
die "$(gettext "The --edit-todo action can only be used during 
interactive rebase.")"
 fi
@@ -487,7 +499,13 @@ fi
 
 if test -n "$interactive_rebase"
 then
-   type=interactive
+   if test -z "$preserve_merges"
+   then
+   type=interactive
+   else
+   type=preserve-merges
+   fi
+
state_dir="$merge_dir"
 elif test -n "$do_merge"
 then
@@ -647,7 +665,7 @@ require_clean_work_tree "rebase" "$(gettext "Please commit 
or stash them.")"
 # but this should be done only when upstream and onto are the same
 # and if this is not an interactive rebase.
 mb=$(git merge-base "$onto" "$orig_head")
-if test "$type" != interactive && test "$upstream" = "$onto" &&
+if test -z "$interactive_rebase" && test "$upstream" = "$onto" &&
test "$mb" = "$onto" && test -z "$restrict_revision" &&
# linear history?
! (git rev-list --parents "$onto".."$orig_head" | sane_grep " .* ") > 
/dev/null
@@ -691,7 +709,7 @@ then
GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
 fi
 
-test "$type" = interactive && run_specific_rebase
+test -n "$interactive_rebase" && run_specific_rebase
 
 # Detach HEAD and reset the tree
 say "$(gettext "First, rewinding head to replay your work on top of it...")"
-- 
2.16.1

[GSoC][PATCH 0/4] rebase: split rebase -p from rebase -i

[Alban Gruin]

This splits the `rebase --preserve-merges` functionnality from
git-rebase--interactive.sh. This is part of the effort to depreciate
preserve-merges. The new script, git-rebase--preserve-merges.sh, should be left
to bitrot. All the dead code left by the duplication of
git-rebase--interactive.sh is also removed.

This patch series is based of js/sequencer-and-root-commits.

Alban Gruin (4):
  rebase: duplicate git-rebase--interactive.sh to
git-rebase--preserve-merges.sh
  rebase: strip unused code in git-rebase--preserve-merges.sh
  rebase: use the new git-rebase--preserve-merges.sh
  rebase: remove -p code from git-rebase--interactive.sh

 .gitignore |1 +
 Makefile   |2 +
 git-rebase--interactive.sh |  708 +---
 git-rebase--preserve-merges.sh | 1004 
 git-rebase.sh  |   32 +-
 5 files changed, 1040 insertions(+), 707 deletions(-)
 create mode 100644 git-rebase--preserve-merges.sh

-- 
2.16.1

user name

[Dennis Powless]

Is it customary to use your real name or a user name when registering to GIT?

Dennis

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Luke Diamand]

On 22 May 2018 at 11:15, SZEDER Gábor  wrote:
> On Tue, May 22, 2018 at 10:41 AM, Luke Diamand  wrote:
>> SZEDER Gábor found that the unshelve tests fail with newer
>> versions of Perforce (2016 vs 2015).
>>
>> The problem arises because when a file is added in a P4
>> shelved changelist, the depot revision is shown as "none"
>> if using the older p4d (which makes sense - the file doesn't
>> yet exist, so can't have a revision), but as "1" in the newer
>> versions of p4d.
>>
>> For example, adding a file called "new" with 2015.1 and then
>> shelving that change gives this from "p4 describe" :
>>
>> ... //depot/new#none add
>>
>> Using the 2018.1 server gives this:
>>
>> ... //depot/new#1 add
>>
>> We can detect that a file has been added simply by using the
>> file status ("add") instead, rather than the depot revision,
>> which is what this change does.
>>
>> This also fixes a few verbose prints used for debugging this
>> to be more friendly.
>>
>> Signed-off-by: Luke Diamand 
>
> For what it's worth, I can confirm that 't9832-unshelve.sh' passes
> with these changes, here and in all Linux and OSX build jobs on Travis
> CI.

Thanks!

>
> However, instead of a separate patch, wouldn't it be better to squash
> it into the previous one?  So 'make test' would succeed on every
> commit even with a newer p4 version.

Junio?

I can squash together the original commit and the two fixes if that
would be better?

Thanks!
Luke

Re: commit-graph: change in "best" merge-base when ambiguous

[Derrick Stolee]

On 5/22/2018 1:39 AM, Michael Haggerty wrote:

On 05/21/2018 08:10 PM, Derrick Stolee wrote:

[...]
In the Discussion section of the `git merge-base` docs [1], we have the
following:

     When the history involves criss-cross merges, there can be more than
one best common ancestor for two commits. For example, with this topology:

     ---1---o---A
         \ /
      X
         / \
     ---2---o---o---B

     both 1 and 2 are merge-bases of A and B. Neither one is better than
the other (both are best merge bases). When the --all option is not
given,     it is unspecified which best one is output.

This means our official documentation mentions that we do not have a
concrete way to differentiate between these choices. This makes me think
that this change in behavior is not a bug, but it _is_ a change in
behavior. It's worth mentioning, but I don't think there is any value in
making sure `git merge-base` returns the same output.

Does anyone disagree? Is this something we should solidify so we always
have a "definitive" merge-base?
[...]

This may be beyond the scope of what you are working on, but there are
significant advantages to selecting a "best" merge base from among the
candidates. Long ago [1] I proposed that the "best" merge base is the
merge base candidate that minimizes the number of non-merge commits that
are in

 git rev-list $candidate..$branch

that are already in master:

 git rev-list $master

(assuming merging branch into master), which is equivalent to choosing
the merge base that minimizes

 git rev-list --count $candidate..$branch

In fact, this criterion is symmetric if you exchange branch ↔ master,
which is a nice property, and indeed generalizes pretty simply to
computing the merge base of more than two commits.

In that email I also included some data showing that the "best" merge
base almost always results in either the same or a shorter diff than the
more or less arbitrary algorithm that we currently use. Sometimes the
difference in diff length is dramatic.

To me it feels like the best *deterministic* merge base would be based
on the above criterion, maybe with first-parent reachability, commit
times, and SHA-1s used (in that order) to break ties.


Thanks, everyone, for your perspective on this. I'm walking away with 
these conclusions:


1. While this is a change in behavior, it is not a regression. We do not 
need to act immediately to preserve old behavior in these ambiguous cases.


2. We should (eventually) define tie-breaking conditions. I like 
Michael's suggestion above.


Thanks,
-Stolee

Re: should config options be treated as case-sensitive?

[Ævar Arnfjörð Bjarmason]

On Tue, May 22 2018, Robert P. J. Day wrote:

>   in my wanderings, more oddities, such as this:
>
> $ grep -ir blankboundary *
> builtin/blame.c:  if (!strcmp(var, "blame.blankboundary")) {
> Documentation/config.txt:blame.blankBoundary::
> Documentation/blame-options.txt:  be controlled via the 
> `blame.blankboundary` config option.
> Documentation/RelNotes/2.15.1.txt: * Description of 
> blame.{showroot,blankboundary,showemail,date}
> Documentation/RelNotes/2.16.0.txt: * Description of 
> blame.{showroot,blankboundary,showemail,date}
> $
>
> where you can see the single instance of "blankBoundary" in
> Doc/config.txt (with the upper case 'B'), while the rest have no such
> thing.
>
>   for fun, i checked another of blame's config settings, with similar
> results:
>
> builtin/blame.c:  if (!strcmp(var, "blame.showemail")) {
> Documentation/config.txt:blame.showEmail::
> Documentation/RelNotes/2.15.1.txt: * Description of 
> blame.{showroot,blankboundary,showemail,date}
> Documentation/RelNotes/2.5.0.txt: * "git blame" learned blame.showEmail 
> configuration variable.
> Documentation/RelNotes/2.16.0.txt: * Description of 
> blame.{showroot,blankboundary,showemail,date}
> Documentation/git-blame.txt:  This can also be controlled via the 
> `blame.showEmail` config
> t/t8002-blame.sh:test_expect_success 'setup showEmail tests' '
> t/t8002-blame.sh:test_expect_success 'blame with showemail options' '
> t/t8002-blame.sh:test_expect_success 'blame with showEmail config false' '
> t/t8002-blame.sh: git config blame.showEmail false &&
> t/t8002-blame.sh:test_expect_success 'blame with showEmail config true' '
> t/t8002-blame.sh: git config blame.showEmail true &&
>
>   thoughts?
>
> rday
>
> p.s. i am not *trying* to be annoyingly pedantic, i am merely
> succeeding.

The issues you note about the docs using foo.barbaz instead of
foo.barBaz should be fixed, but as noted in the "Syntax" section of
"git-config" we already document that the config keys are all
case-insensitive. We just like talking about them as foo.barBaz because
it makes for easier reading.

Re: [PATCH v2 02/18] Add a new builtin: branch-diff

[Ævar Arnfjörð Bjarmason]

On Tue, May 08 2018, Jeff King wrote:

> On Mon, May 07, 2018 at 03:24:59PM -0700, Stefan Beller wrote:
>
>> Hence I propose "git range-diff", similar to topic-diff, that
>> was proposed earlier.
>>
>> * it "diffs ranges" of commits.
>> * it can also deal with out-of-git things like patch series,
>>   but that is a mere by product and may not be desired.
>>   Just like git-diff can also compare two files outside a git
>>   repo, that would not be a good use case.
>>   Keep the name Git-centric!
>> * it autocompletes well.
>
> FWIW, I like this by far of all of the suggested names.

I agree, "range-diff" is the best one mentioned so far.

Re: [PATCH v5 0/4] unpack_trees_options: free messages when done

[Martin Ågren]

On 22 May 2018 at 04:54, Junio C Hamano  wrote:
> Junio C Hamano  writes:

>> Hmph, this unfortunately depends on 'next', which means we cannot
>> merge it down to 'maint' later to fix these leaks.  I guess it is
>> not a huge deal, though.  We've lived with these message leaks for
>> quite some time now and earth still kept rotating ;-)
>
> Oh, what was I thinking.  This, just like its previous rounds, is on
> top of bp/merge-rename-config^0 and it is expected *not* to be
> mergeable to 'maint' (or 'master', for that matter, at least not
> yet).

Right. The reason it depends on that topic is the user in
merge-recursive.c. Other than patch 2 and a small part of patch 4, this
should be mergeable to 'master' (as I recall) and probably also to
'maint'. I suppose this series could have been done as three patches to
fix all users except one, then one or two patches to fix
merge-recursive.c.

That would have allowed merging the first part of the series to 'maint'.
(Maybe not to fix the leaking as such, but to keep 'maint' more up to
date with 'master' for easier merging of other topics?) If you'd prefer
an ordering like that (now and/or in the future), just let me know.

Martin

Re: [PATCH] regex: do not call `regfree()` if compilation fails

[Martin Ågren]

On 22 May 2018 at 04:20, Eric Sunshine  wrote:
> On Mon, May 21, 2018 at 2:43 PM, Stefan Beller  wrote:
>> On Sun, May 20, 2018 at 3:50 AM, Martin Ågren  wrote:
>>> It is apparently undefined behavior to call `regfree()` on a regex where
>>> `regcomp()` failed. [...]
>>>
>>> diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
>>> @@ -215,7 +215,6 @@ static void regcomp_or_die(regex_t *regex, const char 
>>> *needle, int cflags)
>>> /* The POSIX.2 people are surely sick */
>>> char errbuf[1024];
>>> regerror(err, regex, errbuf, 1024);
>>> -   regfree(regex);
>>> die("invalid regex: %s", errbuf);
>>
>> While the commit message is very clear why we supposedly introduce a leak 
>> here,
>> it is hard to be found from the source code (as we only delete code
>> there, so digging
>> for history is not obvious), so maybe
>>
>>  /* regfree(regex) is invalid here */
>>
>> instead?
>
> The commit message doesn't say that we are supposedly introducing a
> leak (and, indeed, no resources should have been allocated to the
> 'regex' upon failed compile). It's saying that removing this call
> potentially avoids a crash under some implementations.
>
> Given that the very next line is die(), and that the function name has
> "_or_die" in it, I'm not sure that an in-code comment about regfree()
> being invalid upon failed compile would be all that helpful; indeed,
> it could be confusing, causing the reader to wonder why that is
> significant if we're just dying anyhow. I find that the patch, as is,
> clarifies rather than muddles the situation.

Like Eric, I feel that the possible leak here is somewhat uninteresting,
since the next line will die. That said, I seem to recall from my
grepping around earlier that we have other users where we return
with a failure instead of dying.

Any clarifying comments in such code would be a separate patch to me. I
also do not immediately see the need for adding such a comment in those
places. We can do that once we verify that we actually do leak (I would
expect that to happen only in some implementations, and I think there is
a fair chance that we will never encounter such an implementation.)

Martin

Re: symbol string_list_appendf() unused

[Martin Ågren]

Hi Ramsay

On 22 May 2018 at 02:08, Ramsay Jones  wrote:
> On 22/05/18 00:59, Junio C Hamano wrote:

>> There is a reroll by Martin that ties all the loose ends.
>
> Ah, OK, sorry for the noise.

No worry. Thanks for pointing out the unused function to me. I
appreciate it.

Martin

Re: [curl PATCH 2/2] ignore SIGPIPE during curl_multi_cleanup

[Daniel Stenberg]

On Tue, 22 May 2018, curlUser wrote:


Again SIGPIPE is seen with curl version 7.45.0 with multi interface.
Backtrace shows :


...

Looks like SIGPIPE_IGNORE to be added in prune_dead connections or in 
disconnect_if_dead? Can anyone comment on this.


I'm pretty sure this issue isn't present in any recent libcurl versions, but 
if you can reproduce it with 7.60.0, I'll be very interested.


--

 / daniel.haxx.se

Re: [curl PATCH 2/2] ignore SIGPIPE during curl_multi_cleanup

[curlUser]

Hi,

Again SIGPIPE is seen with curl version 7.45.0 with multi interface.
Backtrace shows :

#7  0x7f141bea40cd in Curl_ossl_close (conn=0x7f14193f9848, sockindex=0)
at vtls/openssl.c:881
#8  0x7f141bea8f54 in Curl_ssl_close (conn=0x7f14193f9848, sockindex=0)
at vtls/vtls.c:527
#9  0x7f141be63969 in Curl_disconnect (conn=0x7f14193f9848,
dead_connection=true) at url.c:2791
#10 0x7f141be63f4b in disconnect_if_dead (conn=0x7f14193f9848,
data=0xb6a598) at url.c:3050
#11 0x7f141be63f84 in call_disconnect_if_dead (conn=0x7f14193f9848,
param=0xb6a598) at url.c:3066
#12 0x7f141bea01c2 in Curl_conncache_foreach (connc=0xae0f48,
param=0xb6a598, func=0x7f141be63f59 )
at conncache.c:295
#13 0x7f141be6400f in prune_dead_connections (data=0xb6a598) at
url.c:3081

Looks like SIGPIPE_IGNORE to be added in prune_dead connections or in
disconnect_if_dead?
Can anyone comment on this. 



--
Sent from: http://git.661346.n2.nabble.com/

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[SZEDER Gábor]

On Tue, May 22, 2018 at 10:41 AM, Luke Diamand  wrote:
> SZEDER Gábor found that the unshelve tests fail with newer
> versions of Perforce (2016 vs 2015).
>
> The problem arises because when a file is added in a P4
> shelved changelist, the depot revision is shown as "none"
> if using the older p4d (which makes sense - the file doesn't
> yet exist, so can't have a revision), but as "1" in the newer
> versions of p4d.
>
> For example, adding a file called "new" with 2015.1 and then
> shelving that change gives this from "p4 describe" :
>
> ... //depot/new#none add
>
> Using the 2018.1 server gives this:
>
> ... //depot/new#1 add
>
> We can detect that a file has been added simply by using the
> file status ("add") instead, rather than the depot revision,
> which is what this change does.
>
> This also fixes a few verbose prints used for debugging this
> to be more friendly.
>
> Signed-off-by: Luke Diamand 

For what it's worth, I can confirm that 't9832-unshelve.sh' passes
with these changes, here and in all Linux and OSX build jobs on Travis
CI.

However, instead of a separate patch, wouldn't it be better to squash
it into the previous one?  So 'make test' would succeed on every
commit even with a newer p4 version.

[PATCH] Add initial support for pax extended attributes

[Pedro Alvarez]

From: Pedro Alvarez Piedehierro 

Sometimes the tar files will contain pax extended attributes to deal
with cases where the information needed doesn't fit in a standard
ustar entry.

One of these cases is when the path is larger than 100 characters. A
pax entry will appear containing two standard ustart entries. The first
entry will have an 'x' typeflag, and contain the the extended attributes.

The pax extended attributes contain one or multiple records constructed as
follows:

"%d %s=%s\n", , , 

This commit makes sure that we always read the extended attibutes from
pax entries, and in the case of finding one, we parse its records
looking for 'path' information. If this information is found, it's
stored to be used in the next ustar entry.

Information about the Pax Interchange Format can be found at:


https://www.freebsd.org/cgi/man.cgi?manpath=FreeBSD+8-current=tar=5.

Before this change, importing gcc tarballs[1] would fail with the
following error:

fast-import crash report:
fast-import process: 82899
parent process : 82897
at 2018-05-21 12:35:27 +

fatal: Unsupported command: 29 atime=1516870168.93527949

Most Recent Commands Before Crash
-
  M 644 :22495 
gcc-7.3.0/libstdc++-v3/testsuite/20_util/duration/PaxHeaders.4467/comparison_operators
  M 644 :140367 gcc-7.3.0/gcc/ada/s-gloloc-mingw.adb
  M 644 :75143 
gcc-7.3.0/gcc/testsuite/gcc.c-torture/execute/builtins/PaxHeaders.4467/strncat-chk-lib.c

  

  M 644 :135585 
gcc-7.3.0/gcc/testsuite/c-c++-common/attr-warn-unused-result.c
  M 644 :54956 
gcc-7.3.0/gcc/testsuite/go.test/test/fixedbugs/PaxHeaders.4467/bug335.dir
  M 644 :20632 27 mtime=1483272463.905435
* 29 atime=1516870168.93527949

[1]: http://ftp.gnu.org/gnu/gcc/gcc-7.3.0/gcc-7.3.0.tar.xz

Signed-off-by: Pedro Alvarez 
---
 contrib/fast-import/import-tars.perl | 32 ++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/contrib/fast-import/import-tars.perl 
b/contrib/fast-import/import-tars.perl
index d60b4315ed..c2e54ec7a3 100755
--- a/contrib/fast-import/import-tars.perl
+++ b/contrib/fast-import/import-tars.perl
@@ -63,6 +63,8 @@ foreach my $tar_file (@ARGV)
my $have_top_dir = 1;
my ($top_dir, %files);
 
+   my $next_path = '';
+
while (read(I, $_, 512) == 512) {
my ($name, $mode, $uid, $gid, $size, $mtime,
$chksum, $typeflag, $linkname, $magic,
@@ -70,6 +72,13 @@ foreach my $tar_file (@ARGV)
$prefix) = unpack 'Z100 Z8 Z8 Z8 Z12 Z12
Z8 Z1 Z100 Z6
Z2 Z32 Z32 Z8 Z8 Z*', $_;
+
+   unless ($next_path eq '') {
+   # Recover name from previous extended header
+   $name = $next_path;
+   $next_path = '';
+   }
+
last unless length($name);
if ($name eq '././@LongLink') {
# GNU tar extension
@@ -90,13 +99,32 @@ foreach my $tar_file (@ARGV)
Z8 Z1 Z100 Z6
Z2 Z32 Z32 Z8 Z8 Z*', $_;
}
-   next if $name =~ m{/\z};
$mode = oct $mode;
$size = oct $size;
$mtime = oct $mtime;
next if $typeflag == 5; # directory
 
-   if ($typeflag != 1) { # handle hard links later
+   if ($typeflag eq 'x') { # extended header
+   # If extended header, check for path
+   my $pax_header = '';
+   while ($size > 0 && read(I, $_, 512) == 512) {
+   $pax_header = $pax_header . substr($_, 0, 
$size);
+   $size -= 512;
+   }
+
+   my @lines = split /\n/, $pax_header;
+   foreach my $line (@lines) {
+   my ($len, $entry) = split / /, $line;
+   my ($key, $value) = split /=/, $entry;
+   if ($key eq 'path') {
+   $next_path = $value;
+   }
+   }
+   next;
+   } elsif ($name =~ m{/\z}) {
+   # If it's a folder, ignore
+   next;
+   } elsif ($typeflag != 1) { # handle hard links later
print FI "blob\n", "mark :$next_mark\n";
if ($typeflag == 2) { # symbolic link
print FI "data ", length($linkname), "\n",
-- 
2.11.0

[PATCH 0/1] git-p4: unshelving: fix problem with newer P4

[Luke Diamand]

This fixes a problem found by SZEDER Gábor with newer versions of
the Perforce database engine (2016 onwards). It looks like the
behaviour has change subtly when reporting the revision of newly
added files. The fix is to just use the file status.

Luke Diamand (1):
  git-p4: unshelve: use action==add instead of rev==none

 git-p4.py | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

-- 
2.17.0.392.gdeb1a6e9b7

[PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Luke Diamand]

SZEDER Gábor found that the unshelve tests fail with newer
versions of Perforce (2016 vs 2015).

The problem arises because when a file is added in a P4
shelved changelist, the depot revision is shown as "none"
if using the older p4d (which makes sense - the file doesn't
yet exist, so can't have a revision), but as "1" in the newer
versions of p4d.

For example, adding a file called "new" with 2015.1 and then
shelving that change gives this from "p4 describe" :

... //depot/new#none add

Using the 2018.1 server gives this:

... //depot/new#1 add

We can detect that a file has been added simply by using the
file status ("add") instead, rather than the depot revision,
which is what this change does.

This also fixes a few verbose prints used for debugging this
to be more friendly.

Signed-off-by: Luke Diamand 
---
 git-p4.py | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 364d86dbcc..c80d85af89 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2463,7 +2463,7 @@ class P4Sync(Command, P4UserMap):
 """
 ret = p4Cmd(["diff2", "{0}#{1}".format(path, filerev), 
"{0}@{1}".format(path, revision)])
 if verbose:
-print("p4 diff2 %s %s %s => %s" % (path, filerev, revision, ret))
+print("p4 diff2 path %s filerev %s revision %s => %s" % (path, 
filerev, revision, ret))
 return ret["status"] == "identical"
 
 def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0, 
origin_revision = 0):
@@ -2492,7 +2492,12 @@ class P4Sync(Command, P4UserMap):
 if shelved:
 file["shelved_cl"] = int(shelved_cl)
 
-if file["rev"] != "none" and \
+# For shelved changelists, check that the revision of each 
file that the
+# shelve was based on matches the revision that we are using 
for the
+# starting point for git-fast-import (self.initialParent). 
Otherwise
+# the resulting diff will contain deltas from multiple commits.
+
+if file["action"] != "add" and \
 not self.cmp_shelved(path, file["rev"], origin_revision):
 sys.exit("change {0} not based on {1} for {2}, cannot 
unshelve".format(
 commit["change"], self.initialParent, path))
@@ -2610,7 +2615,7 @@ class P4Sync(Command, P4UserMap):
 def streamOneP4File(self, file, contents):
 relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)
 relPath = self.encodeWithUTF8(relPath)
-if verbose:
+if verbose and 'fileSize' in self.stream_file:
 size = int(self.stream_file['fileSize'])
 sys.stdout.write('\r%s --> %s (%i MB)\n' % (file['depotFile'], 
relPath, size/1024/1024))
 sys.stdout.flush()
-- 
2.17.0.392.gdeb1a6e9b7

should config options be treated as case-sensitive?

[Robert P. J. Day]

  in my wanderings, more oddities, such as this:

$ grep -ir blankboundary *
builtin/blame.c:if (!strcmp(var, "blame.blankboundary")) {
Documentation/config.txt:blame.blankBoundary::
Documentation/blame-options.txt:be controlled via the 
`blame.blankboundary` config option.
Documentation/RelNotes/2.15.1.txt: * Description of 
blame.{showroot,blankboundary,showemail,date}
Documentation/RelNotes/2.16.0.txt: * Description of 
blame.{showroot,blankboundary,showemail,date}
$

where you can see the single instance of "blankBoundary" in
Doc/config.txt (with the upper case 'B'), while the rest have no such
thing.

  for fun, i checked another of blame's config settings, with similar
results:

builtin/blame.c:if (!strcmp(var, "blame.showemail")) {
Documentation/config.txt:blame.showEmail::
Documentation/RelNotes/2.15.1.txt: * Description of 
blame.{showroot,blankboundary,showemail,date}
Documentation/RelNotes/2.5.0.txt: * "git blame" learned blame.showEmail 
configuration variable.
Documentation/RelNotes/2.16.0.txt: * Description of 
blame.{showroot,blankboundary,showemail,date}
Documentation/git-blame.txt:This can also be controlled via the 
`blame.showEmail` config
t/t8002-blame.sh:test_expect_success 'setup showEmail tests' '
t/t8002-blame.sh:test_expect_success 'blame with showemail options' '
t/t8002-blame.sh:test_expect_success 'blame with showEmail config false' '
t/t8002-blame.sh:   git config blame.showEmail false &&
t/t8002-blame.sh:test_expect_success 'blame with showEmail config true' '
t/t8002-blame.sh:   git config blame.showEmail true &&

  thoughts?

rday

p.s. i am not *trying* to be annoyingly pedantic, i am merely
succeeding.

[PATCH] Doc: Mention core.excludesFile in "man git-clean"

[Robert P. J. Day]

Add a reference to the configuration setting "core.excludesFile" to
the man page for git-clean.

Signed-off-by: Robert P. J. Day 

---

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 03056dad0..449cbc2af 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -55,13 +55,15 @@ OPTIONS

 -e ::
 --exclude=::
-   In addition to those found in .gitignore (per directory) and
-   $GIT_DIR/info/exclude, also consider these patterns to be in the
-   set of the ignore rules in effect.
+   In addition to patterns found in any of .gitignore (per directory),
+   $GIT_DIR/info/exclude and the exclude file specified by the
+   configuration variable core.excludesFile, also consider these
+   patterns to be in the set of the ignore rules in effect.

 -x::
Don't use the standard ignore rules read from .gitignore (per
-   directory) and $GIT_DIR/info/exclude, but do still use the ignore
+   directory), $GIT_DIR/info/exclude and the exclude file specified
+   by core.excludesFile, but do still use the ignore
rules given with `-e` options.  This allows removing all untracked
files, including build products.  This can be used (possibly in
conjunction with 'git reset') to create a pristine

-- 


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

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

Re: [PATCH 1/2] remote-curl: accept all encoding supported by curl

[Daniel Stenberg]

On Mon, 21 May 2018, Jonathan Nieder wrote:


Looking at the code here, this succeeds if enough memory is available.
There is no check if the given parameter is part of
Curl_all_content_encodings();


By "this" are you referring to the preimage or the postimage?  Are you 
suggesting a change in git or in libcurl?


Curl_all_content_encodings() is an internal function in libcurl, so I'm 
assuming the latter.


Ack, that certainly isn't the most wonderful API for selecting a compression 
method. In reality, almost everyone sticks to passing on a "" to that option 
to let libcurl pick and ask for the compression algos it knows since both gzip 
and brotli are present only conditionally depending on build options.


I would agree that the libcurl setopt call should probably be made to fail if 
asked to use a compression method not built-in/supported. Then an application 
could in fact try different algos in order until one works or ask to disable 
compression completely.


In the generic HTTP case, it usually makes sense to ask for more than one 
algorthim though, since this is asking the server for a compressed version and 
typically a HTTP client doesn't know which compression methods the server 
offers. Not sure this is actually true to the same extent for git.


--

 / daniel.haxx.se