Git development
 help / color / mirror / Atom feed
* Re: [PATCH 01/10] submodule: decouple url and submodule existence
From: Brandon Williams @ 2017-03-01 20:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, sbeller
In-Reply-To: <xmqq8tov1ev7.fsf@gitster.mtv.corp.google.com>

On 02/24, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
> 
> > Currently the submodule.<name>.url config option is used to determine
> > if a given submodule exists and is interesting to the user.  This
> > however doesn't work very well because the URL is a config option for
> > the scope of a repository, whereas the existence of a submodule is an
> > option scoped to the working tree.
> >
> > In a future with worktree support for submodules, there will be multiple
> > working trees, each of which may only need a subset of the submodules
> > checked out.  The URL (which is where the submodule repository can be
> > obtained) should not differ between different working trees.
> >
> > It may also be convenient for users to more easily specify groups of
> > submodules they are interested in as apposed to running "git submodule
> > init <path>" on each submodule they want checked out in their working
> > tree.
> >
> > To this end, the config option submodule.active is introduced which
> > holds a pathspec that specifies which submodules should exist in the
> > working tree.
> 
> Hmph.  submodule.active in .git/config would be shared the same way
> submodule.<name>.url in .git/config is shared across the worktrees
> that stems from the same primary repository, no?
> 
> Perhaps there are some other uses of this submodule.active idea, but
> I do not see how it is relevant to solving "multiple worktrees"
> issue.  Per-worktree config would solve it with the current
> submodule.<name>.url without submodule.active list, I would think [*1*].

Correct, I should update the language to indicate this allows the URL to
be shared between worktrees, but a per-worktree config must exist before
submodule.active can actually be used to select different groups of
submodules per-worktree.  The idea is that if submodule.active is set
then you no longer look at the URLs to see what is interesting but
rather at the paths.

> Also as a grouping measure, submodule.active that lists submodule
> paths feels hard to use.  When switching between two branches in the
> superproject that have the same submodule bound at two different
> paths, who is responsible for updating submodule.active in
> superproject's config?  If it were a list of submodule names, this
> objection does not apply, though.

I agree that if you are listing every submodule path by hand then this
may not be the best approach and would be difficult to use.  The idea is
that this would allow a user to set a general pathspec to identify a
group of modules they are interested in.  Perhaps once attributes can be
used in pathspecs a user could group submodules by setting a particular
attribute and then submodule.active would have a value like
":(attr:foo)" to indicate I'm interested in all submodules with the
"foo" attribute.

> 
> 
> 
> [Footnote]
> 
> *1* At the conceptual level, I agree that .url that also means "we
>     are interested in this one" feels like somewhat an unclean
>     design, but that is not what you are "fixing", is it?
> 

-- 
Brandon Williams

^ permalink raw reply

* Re: Delta compression not so effective
From: Linus Torvalds @ 2017-03-01 18:30 UTC (permalink / raw)
  To: Marius Storm-Olsen; +Cc: Git Mailing List
In-Reply-To: <eba83461-34cf-6d64-4013-873b04af9b82@gmail.com>

On Wed, Mar 1, 2017 at 9:57 AM, Marius Storm-Olsen <mstormo@gmail.com> wrote:
>
> Indeed, I did do a
>     -c pack.threads=20 --window-memory=6g
> to 'git repack', since the machine is a 20-core (40 threads) machine with
> 126GB of RAM.
>
> So I guess with these sized objects, even at 6GB per thread, it's not enough
> to get a big enough Window for proper delta-packing?

Hmm. The 6GB window should be plenty good enough, unless your blobs
are in the gigabyte range too.

> This repo took >14hr to repack on 20 threads though ("compression" step was
> very fast, but stuck 95% of the time in "writing objects"), so I can only
> imagine how long a pack.threads=1 will take :)

Actually, it's usually the compression phase that should be slow - but
if something is limiting finding deltas (so that we abort early), then
that would certainly tend to speed up compression.

The "writing objects" phase should be mainly about the actual IO.
Which should be much faster *if* you actually find deltas.

> But arent't the blobs sorted by some metric for reasonable delta-pack
> locality, so even with a 6GB window it should have seen ~25 similar objects
> to deltify against?

Yes they are. The sorting for delta packing tries to make sure that
the window is effective. However, the sorting is also just a
heuristic, and it may well be that your repository layout ends up
screwing up the sorting, so that the windows just work very badly.

For example, the sorting code thinks that objects with the same name
across the history are good sources of deltas. But it may be that for
your case, the binary blobs that you have don't tend to actually
change in the history, so that heuristic doesn't end up doing
anything.

The sorting does use the size and the type too, but the "filename
hash" (which isn't really a hash, it's something nasty to give
reasonable results for the case where files get renamed) is the main
sort key.

So you might well want to look at the sorting code too. If filenames
(particularly the end of filenames) for the blobs aren't good hints
for the sorting code, that sort might end up spreading all the blobs
out rather than sort them by size.

And again, if that happens, the "can I delta these two objects" code
will notice that the size of the objects are wildly different and
won't even bother trying. Which speeds up the "compressing" phase, of
course, but then because you don't get any good deltas, the "writing
out" phase sucks donkey balls because it does zlib compression on big
objects and writes them out to disk.

So there are certainly multiple possible reasons for the deltification
to not work well for you.

Hos sensitive is your material? Could you make a smaller repo with
some of the blobs that still show the symptoms? I don't think I want
to download 206GB of data even if my internet access is good.

                    Linus

^ permalink raw reply

* Re: git status --> Out of memory, realloc failed
From: Carsten Fuchs @ 2017-03-01 20:12 UTC (permalink / raw)
  To: René Scharfe, git
In-Reply-To: <ea0722e2-c2bd-bd80-a233-50676efcafda@web.de>

Hi René,

Am 01.03.2017 um 11:02 schrieb René Scharfe:
>> I use Git at a web hosting service, where my user account has a memory
>> limit of 768 MB:
>>
>> (uiserver):p7715773:~$ uname -a
>> Linux infongp-de15 3.14.0-ui16322-uiabi1-infong-amd64 #1 SMP Debian
>> 3.14.79-2~ui80+4 (2016-11-17) x86_64 GNU/Linux
>
> What's the output of "ulimit -a"?

(uiserver):p7715773:~$ ulimit -a
core file size          (blocks, -c) 0
data seg size           (kbytes, -d) unlimited
scheduling priority             (-e) 1
file size               (blocks, -f) unlimited
pending signals                 (-i) 16382
max locked memory       (kbytes, -l) 64
max memory size         (kbytes, -m) unlimited
open files                      (-n) 512
pipe size            (512 bytes, -p) 8
POSIX message queues     (bytes, -q) 819200
real-time priority              (-r) 0
stack size              (kbytes, -s) 8192
cpu time               (seconds, -t) 1800
max user processes              (-u) 42
virtual memory          (kbytes, -v) 786432
file locks                      (-x) unlimited

>> (uiserver):p7715773:~$ git --version
>> git version 2.1.4
>
> That's quite old.  Can you try a more recent version easily (2.12.0 just came out)?

I don't think that they have any newer version available, having just upgraded from Git 
1.7 a couple of weeks ago... (1und1)

Git 1.7 used to work for me in the same environment, but iirc they also said they 
switched from 32-bit to 64-bit edition and blame the increased memory consumption on 
that change.

I'll ask for a newer version, but I'd be surprised if this happened.

>> The repository is tracking about 19000 files which together take 260 MB.
>> The git server version is 2.7.4.1.g5468f9e (Bitbucket)
>
> Is your repository publicly accessible?

Unfortunately, no. There are no big secrets in there, but just a couple of database 
details so that I cannot make it universally available. I can gladly give you access 
though. (E.g. by adding your public SSH key?)

Best regards,
Carsten

^ permalink raw reply

* Re: Delta compression not so effective
From: Martin Langhoff @ 2017-03-01 21:08 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Marius Storm-Olsen, Git Mailing List
In-Reply-To: <CA+55aFx7QFqrHw4e72vOdM5z0rw1CCkL2-UX8ej5CLSBWjLNLA@mail.gmail.com>

On Wed, Mar 1, 2017 at 1:30 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> For example, the sorting code thinks that objects with the same name
> across the history are good sources of deltas.

Marius has indicated he is working with jar files. IME jar and war
files, which are zipfiles containing Java bytecode, range from not
delta-ing in a useful fashion, to pretty good deltas.

Depending on the build process (hi Maven!) there can be enough
variance in the build metadata to throw all the compression machinery
off.

On a simple Maven-driven project I have at hand, two .war files
compiled from the same codebase compressed really well in git. I've
also seen projects where storage space is ~101% of the "uncompressed"
size.

my 2c,



m
-- 
 martin.langhoff@gmail.com
 - ask interesting questions  ~  http://linkedin.com/in/martinlanghoff
 - don't be distracted        ~  http://github.com/martin-langhoff
   by shiny stuff

^ permalink raw reply

* Re: Delta compression not so effective
From: Martin Langhoff @ 2017-03-01 20:19 UTC (permalink / raw)
  To: Marius Storm-Olsen; +Cc: Git Mailing List
In-Reply-To: <4d2a1852-8c84-2869-78ad-3c863f6dcaf7@gmail.com>

On Wed, Mar 1, 2017 at 8:51 AM, Marius Storm-Olsen <mstormo@gmail.com> wrote:
> BUT, even still, I would expect Git's delta compression to be quite effective, compared to the compression present in SVN.

jar files are zipfiles. They don't delta in any useful form, and in
fact they differ even if they contain identical binary files inside.

>     Commits: 32988
>     DB (server) size: 139GB

Are you certain of the on-disk storage at the SVN server? Ideally,
you've taken the size with a low-level tool like `du -sh
/path/to/SVNRoot`.

Even with no delta compression (as per Junio and Linus' discussion),
based on past experience importing jar/wars/binaries from SVN into
git... I'd expect git's worst case to be on-par with SVN, perhaps ~5%
larger due to compression headers on uncompressible data.

cheers,


m
-- 
 martin.langhoff@gmail.com
 - ask interesting questions  ~  http://linkedin.com/in/martinlanghoff
 - don't be distracted        ~  http://github.com/martin-langhoff
   by shiny stuff

^ permalink raw reply

* Re: [PATCH v1 1/1] git diff --quiet exits with 1 on clean tree with CRLF conversions
From: Junio C Hamano @ 2017-03-01 21:14 UTC (permalink / raw)
  To: tboegi; +Cc: git, mac
In-Reply-To: <20170301170444.14274-1-tboegi@web.de>

tboegi@web.de writes:

> From: Junio C Hamano <gitster@pobox.com>
>
> git diff --quiet may take a short-cut to see if a file is changed
> in the working tree:
> Whenever the file size differs from what is recorded in the index,
> the file is assumed to be changed and git diff --quiet returns
> exit with code 1
>
> This shortcut must be suppressed whenever the line endings are converted
> or a filter is in use.
> The attributes say "* text=auto" and a file has
> "Hello\nWorld\n" in the index with a length of 12.
> The file in the working tree has "Hello\r\nWorld\r\n" with a length of 14.
> (Or even "Hello\r\nWorld\n").
> In this case "git add" will not do any changes to the index, and
> "git diff -quiet" should exit 0.

The thing I find the most disturbing is that at this point in the
flow, p->one->size and p->two->size are supposed to be the sizes of
the blob object, not the contents of the file on the working tree.
IOW, p->two->size being 14 in the above example sounds like pointing
at a different bug, if it is 14.  

The early return in diff_populate_filespec(), where it does

	s->size = xsize_t(st.st_size);
	...
	if (size_only)
		return 0;

way before it runs convert_to_git(), may be the real culprit.

I am wondering if the real fix would be to do this, instead of the
two extra would_convert_to_git() call there in the patch you sent.
The result seems to still pass the new test in your patch.

Thanks for helping.

 diff.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 8c78fce49d..dc51dceb44 100644
--- a/diff.c
+++ b/diff.c
@@ -2792,8 +2792,25 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
 			s->should_free = 1;
 			return 0;
 		}
-		if (size_only)
+
+		/*
+		 * Even if the caller would be happy with getting
+		 * only the size, we cannot return early at this
+		 * point if the path requires us to run the content
+		 * conversion.
+		 */
+		if (!would_convert_to_git(s->path) && size_only)
 			return 0;
+
+		/*
+		 * Note: this check uses xsize_t(st.st_size) that may
+		 * not be the true size of the blob after it goes
+		 * through convert_to_git().  This may not strictly be
+		 * correct, but the whole point of big_file_threashold
+		 * and is_binary check is that we want to avoid
+		 * opening the file and inspecting the contents, so
+		 * this is probably fine.
+		 */
 		if ((flags & CHECK_BINARY) &&
 		    s->size > big_file_threshold && s->is_binary == -1) {
 			s->is_binary = 1;

^ permalink raw reply related

* Re: [PATCH v1 1/1] git diff --quiet exits with 1 on clean tree with CRLF conversions
From: Mike Crowe @ 2017-03-01 21:25 UTC (permalink / raw)
  To: tboegi; +Cc: git, Junio C Hamano
In-Reply-To: <20170301170444.14274-1-tboegi@web.de>

On Wednesday 01 March 2017 at 18:04:44 +0100, tboegi@web.de wrote:
> From: Junio C Hamano <gitster@pobox.com>
> 
> git diff --quiet may take a short-cut to see if a file is changed
> in the working tree:
> Whenever the file size differs from what is recorded in the index,
> the file is assumed to be changed and git diff --quiet returns
> exit with code 1
> 
> This shortcut must be suppressed whenever the line endings are converted
> or a filter is in use.
> The attributes say "* text=auto" and a file has
> "Hello\nWorld\n" in the index with a length of 12.
> The file in the working tree has "Hello\r\nWorld\r\n" with a length of 14.
> (Or even "Hello\r\nWorld\n").
> In this case "git add" will not do any changes to the index, and
> "git diff -quiet" should exit 0.
> 
> Add calls to would_convert_to_git() before blindly saying that a different
> size means different content.
> 
> Reported-By: Mike Crowe <mac@mcrowe.com>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
> This is what I can come up with, collecting all the loose ends.
> I'm not sure if Mike wan't to have the Reported-By with a
> Signed-off-by ?
> The other question is, if the commit message summarizes the discussion
> well enough ?
> 
> diff.c                    | 18 ++++++++++++++----
>  t/t0028-diff-converted.sh | 27 +++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 4 deletions(-)
>  create mode 100755 t/t0028-diff-converted.sh
> 
> diff --git a/diff.c b/diff.c
> index 051761b..c264758 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4921,9 +4921,10 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)
>  	 *    differences.
>  	 *
>  	 * 2. At this point, the file is known to be modified,
> -	 *    with the same mode and size, and the object
> -	 *    name of one side is unknown.  Need to inspect
> -	 *    the identical contents.
> +	 *    with the same mode and size, the object
> +	 *    name of one side is unknown, or size comparison
> +	 *    cannot be depended upon.  Need to inspect the
> +	 *    contents.
>  	 */
>  	if (!DIFF_FILE_VALID(p->one) || /* (1) */
>  	    !DIFF_FILE_VALID(p->two) ||
> @@ -4931,7 +4932,16 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)
>  	    (p->one->mode != p->two->mode) ||
>  	    diff_populate_filespec(p->one, CHECK_SIZE_ONLY) ||
>  	    diff_populate_filespec(p->two, CHECK_SIZE_ONLY) ||
> -	    (p->one->size != p->two->size) ||
> +
> +	    /*
> +	     * only if eol and other conversions are not involved,
> +	     * we can say that two contents of different sizes
> +	     * cannot be the same without checking their contents.
> +	     */
> +	    (!would_convert_to_git(p->one->path) &&
> +	     !would_convert_to_git(p->two->path) &&
> +	     (p->one->size != p->two->size)) ||
> +
>  	    !diff_filespec_is_identical(p->one, p->two)) /* (2) */
>  		p->skip_stat_unmatch_result = 1;
>  	return p->skip_stat_unmatch_result;
> diff --git a/t/t0028-diff-converted.sh b/t/t0028-diff-converted.sh
> new file mode 100755
> index 0000000..3d5ab95
> --- /dev/null
> +++ b/t/t0028-diff-converted.sh
> @@ -0,0 +1,27 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2017 Mike Crowe
> +#
> +# These tests ensure that files changing line endings in the presence
> +# of .gitattributes to indicate that line endings should be ignored
> +# don't cause 'git diff' or 'git diff --quiet' to think that they have
> +# been changed.
> +
> +test_description='git diff with files that require CRLF conversion'
> +
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> +	echo "* text=auto" >.gitattributes &&
> +	printf "Hello\r\nWorld\r\n" >crlf.txt &&
> +	git add .gitattributes crlf.txt &&
> +	git commit -m "initial"
> +'
> +
> +test_expect_success 'quiet diff works on file with line-ending change that has no effect on repository' '
> +	printf "Hello\r\nWorld\n" >crlf.txt &&
> +	git status &&
> +	git diff --quiet
> +'
> +
> +test_done

Hi Torsten,

Thanks for investigating this.

I think that you've simplified the test to the point where it doesn't
entirely prove the fix. Although you test the case where the file has
changed size, you don't test the case where it hasn't.

Unfortunately that was the part of my test that could only reproduce the
problem with the sleeps. Maybe someone who understands how the cache works
fully could explain an alternative way to force the cache not to be used.

Also, I think I've found a behaviour change with this fix. Consider:

 echo "* text=auto" >.gitattributes
 printf "Hello\r\nWorld\r\n" >crlf.txt
 git add .gitattributes crlf.txt
 git commit -m "initial"

 printf "\r\n" >>crlf.txt

With the above patch, both "git diff" and "git diff --quiet" report that
there are no changes. Previously Git would report the extra newline
correctly.

Mike.

^ permalink raw reply

* Re: [PATCH v4 15/22] read-cache: touch shared index files when used
From: Junio C Hamano @ 2017-03-01 21:34 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Ramsay Jones, Jeff King, Christian Couder
In-Reply-To: <20170227180019.18666-16-chriscool@tuxfamily.org>

Christian Couder <christian.couder@gmail.com> writes:

> +/*
> + * Signal that the shared index is used by updating its mtime.
> + *
> + * This way, shared index can be removed if they have not been used
> + * for some time.
> + */
> +static void freshen_shared_index(char *base_sha1_hex, int warn)
> +{
> +	const char *shared_index = git_path("sharedindex.%s", base_sha1_hex);
> +	if (!check_and_freshen_file(shared_index, 1) && warn)
> +		warning("Could not freshen shared index '%s'", shared_index);

I'll downcase "Could" to match existing convention (no need to
resend only to update this one).

^ permalink raw reply

* Re: [PATCH v4 00/22] Add configuration options for split-index
From: Junio C Hamano @ 2017-03-01 21:29 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Ramsay Jones, Jeff King, Christian Couder
In-Reply-To: <20170227180019.18666-1-chriscool@tuxfamily.org>

Christian Couder <christian.couder@gmail.com> writes:

> Highlevel view of the patches in the series
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Except for patch 1/22 and 1/22, there are 3 big steps, one for each
> new configuration variable introduced.
>
> There only small differences between this patch series and the v3
> patch series sent a few months ago.
>
>     - Patch 1/22 marks a message for translation. It is not new and
>       can be applied separately.
>
>     - Patch 2/22 improves the existing indentation style of t1700 by
>       using different here document style. It is a new preparatory
>       patch suggested by Junio.

OK.  I read interdiff against the previous round carefully, and
skimmed all patches less carefully.  

I may have comments on individual patches later, but this round
looked mostly sensible.

Thanks for following it through.


^ permalink raw reply

* Re: [PATCH] Put sha1dc on a diet
From: Jeff King @ 2017-03-01 20:34 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List, Dan Shumow, Marc Stevens
In-Reply-To: <CA+55aFwf3sxKW+dGTMjNAeHMOf=rvctEQohm+rbhEb=e3KLpHw@mail.gmail.com>

On Wed, Mar 01, 2017 at 12:14:34PM -0800, Linus Torvalds wrote:

> > My biggest concern is the index-pack operation. Try this:
> 
> I'm mobile right now, so I can't test, but I'd this perhaps at least partly
> due to the full checksum over the pack-file?
>
> We have two very different uses of SHA1: the actual object name hash, but
> also the sha1file checksums that we do on the index file and the pack files.
>
> And the checksum code really doesn't need the collision checking at all.

I don't think that helps. The sha1 over the pack-file takes about 1.3s
with openssl, and 5s with sha1dc. So we already know the increase there
is only a few seconds, not a few minutes.

And it makes sense if you think about the index-pack operation. It has
to inflate each object, resolving deltas, and checksum the result. And
the number of inflated bytes is _much_ larger than the on-disk bytes.
You can see the difference with:

  git cat-file --batch-all-objects \
    --batch-check='%(objectsize:disk) %(objectsize)' |
  perl -alne '
    $disk += $F[0]; $raw += $F[1];
    END { print "$disk $raw" }
  '

On linux.git that yields:

  1210521959 63279680406

That's over a 50x increase in the bytes we have to sha1 for objects
versus pack-checksums.

-Peff

^ permalink raw reply

* Re: [PATCH v4 17/22] read-cache: unlink old sharedindex files
From: Junio C Hamano @ 2017-03-01 21:39 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Ramsay Jones, Jeff King, Christian Couder
In-Reply-To: <20170227180019.18666-18-chriscool@tuxfamily.org>

Christian Couder <christian.couder@gmail.com> writes:

> +static int can_delete_shared_index(const char *shared_index_path)
> +{
> +	struct stat st;
> +	unsigned long expiration;
> +
> +	/* Check timestamp */
> +	expiration = get_shared_index_expire_date();
> +	if (!expiration)
> +		return 0;
> +	if (stat(shared_index_path, &st))
> +		return error_errno(_("could not stat '%s"), shared_index_path);
> +	if (st.st_mtime > expiration)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static int clean_shared_index_files(const char *current_hex)
> +{
> +	struct dirent *de;
> +	DIR *dir = opendir(get_git_dir());
> +
> +	if (!dir)
> +		return error_errno(_("unable to open git dir: %s"), get_git_dir());
> +
> +	while ((de = readdir(dir)) != NULL) {
> +		const char *sha1_hex;
> +		const char *shared_index_path;
> +		if (!skip_prefix(de->d_name, "sharedindex.", &sha1_hex))
> +			continue;
> +		if (!strcmp(sha1_hex, current_hex))
> +			continue;
> +		shared_index_path = git_path("%s", de->d_name);
> +		if (can_delete_shared_index(shared_index_path) > 0 &&

Is this "can" or "should"?  This sounds like the latter.

> +		    unlink(shared_index_path))
> +			error_errno(_("unable to unlink: %s"), shared_index_path);

This does not make the entire operation to fail (and I think the
behaviour you have here is preferrable--we just want to report
without failing the main operation).

But should it be reported as "error: unable to unlink"?  It may be
better to give this message as a warning.

^ permalink raw reply

* Re: SHA1 collisions found
From: Jeff King @ 2017-03-01 19:05 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Marc Stevens, Dan Shumow, Junio C Hamano, Joey Hess,
	Git Mailing List
In-Reply-To: <CA+55aFwjzbhYyFm_MqL=cDZZeKbSjqd-jSeb0yW_bJ_WQTzEpA@mail.gmail.com>

On Tue, Feb 28, 2017 at 03:11:32PM -0800, Linus Torvalds wrote:

> > Of course for dedicated code this can be simplified, and some parts
> > could be further optimized.
> 
> So I'd be worried about changing your tested code too much, since the
> only test-cases we have are the two pdf files. If we screw up too
> much, those will no longer show as collisions, but we could get tons
> of false positives that we wouldn't see, so..

I can probably help with collecting data for that part on GitHub.

I don't have an exact count of how many sha1 computations we do in a
day, but it's...a lot. Obviously every pushed object gets its sha1
computed, but read operations also cover every commit and tree via
parse_object() (though I think most of the blob reads do not).

So it would be trivial to start by swapping out the "die()" on collision
with something that writes to a log. This is the slow path that we don't
expect to trigger at all, so log volume shouldn't be a problem.

I've been waiting to see how speedups develop before deploying it in
production.

-Peff

^ permalink raw reply

* Re: [PATCH v1 1/1] git diff --quiet exits with 1 on clean tree with CRLF conversions
From: Junio C Hamano @ 2017-03-01 21:54 UTC (permalink / raw)
  To: tboegi; +Cc: git, mac
In-Reply-To: <xmqqr32gg0o6.fsf@gitster.mtv.corp.google.com>

Now I thought about it through a bit more thoroughly, I think this
is the right approach, so here is my (tenative) final version.

I seem to be getty really rusty---after all the codepaths involved
are practically all my code and I should have noticed the real
culprit during my first attempt X-<.

Thanks for helping.

-- >8 --
Subject: [PATCH] diff: do not short-cut CHECK_SIZE_ONLY check in diff_populate_filespec()

Callers of diff_populate_filespec() can choose to ask only for the
size of the blob without grabbing the blob data, and the function,
after running lstat() when the filespec points at a working tree
file, returns by copying the value in size field of the stat
structure into the size field of the filespec when this is the case.

However, this short-cut cannot be taken if the contents from the
path needs to go through convert_to_git(), whose resulting real blob
data may be different from what is in the working tree file.

As "git diff --quiet" compares the .size fields of filespec
structures to skip content comparison, this bug manifests as a
false "there are differences" for a file that needs eol conversion,
for example.

Reported-by: Mike Crowe <mac@mcrowe.com>
Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c                    | 19 ++++++++++++++++++-
 t/t0028-diff-converted.sh | 27 +++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100755 t/t0028-diff-converted.sh

diff --git a/diff.c b/diff.c
index 8c78fce49d..dc51dceb44 100644
--- a/diff.c
+++ b/diff.c
@@ -2792,8 +2792,25 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
 			s->should_free = 1;
 			return 0;
 		}
-		if (size_only)
+
+		/*
+		 * Even if the caller would be happy with getting
+		 * only the size, we cannot return early at this
+		 * point if the path requires us to run the content
+		 * conversion.
+		 */
+		if (!would_convert_to_git(s->path) && size_only)
 			return 0;
+
+		/*
+		 * Note: this check uses xsize_t(st.st_size) that may
+		 * not be the true size of the blob after it goes
+		 * through convert_to_git().  This may not strictly be
+		 * correct, but the whole point of big_file_threashold
+		 * and is_binary check being that we want to avoid
+		 * opening the file and inspecting the contents, this
+		 * is probably fine.
+		 */
 		if ((flags & CHECK_BINARY) &&
 		    s->size > big_file_threshold && s->is_binary == -1) {
 			s->is_binary = 1;
diff --git a/t/t0028-diff-converted.sh b/t/t0028-diff-converted.sh
new file mode 100755
index 0000000000..3d5ab9565b
--- /dev/null
+++ b/t/t0028-diff-converted.sh
@@ -0,0 +1,27 @@
+#!/bin/sh
+#
+# Copyright (c) 2017 Mike Crowe
+#
+# These tests ensure that files changing line endings in the presence
+# of .gitattributes to indicate that line endings should be ignored
+# don't cause 'git diff' or 'git diff --quiet' to think that they have
+# been changed.
+
+test_description='git diff with files that require CRLF conversion'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo "* text=auto" >.gitattributes &&
+	printf "Hello\r\nWorld\r\n" >crlf.txt &&
+	git add .gitattributes crlf.txt &&
+	git commit -m "initial"
+'
+
+test_expect_success 'quiet diff works on file with line-ending change that has no effect on repository' '
+	printf "Hello\r\nWorld\n" >crlf.txt &&
+	git status &&
+	git diff --quiet
+'
+
+test_done
-- 
2.12.0-319-gc5f21175ee


^ permalink raw reply related

* Re: [PATCH] Put sha1dc on a diet
From: Junio C Hamano @ 2017-03-01 22:05 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Linus Torvalds, Jeff King, Marc Stevens, Dan Shumow,
	Git Mailing List
In-Reply-To: <alpine.DEB.2.20.1703012227010.3767@virtualbox>

On Wed, Mar 1, 2017 at 1:56 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Wed, 1 Mar 2017, Junio C Hamano wrote:
>
>> Linus Torvalds <torvalds@linux-foundation.org> writes:
>>
>> > That said, I think that it would be lovely to just default to
>> > USE_SHA1DC and just put the whole attack behind us. Yes, it's slower.
>> > No, it doesn't really seem to matter that much in practice.
>>
>> Yes.  It would be a very good goal.
>
> So let me get this straight: not only do we now implicitly want to bump
> the required C compiler to C99 without any grace period worth mentioning
> [*1*], we are also all of a sudden no longer worried about a double digit
> percentage drop of speed [*2*]?

Before we get the code into shape suitable for 'next', it is more important to
make sure it operates correctly, adding necessary features if any (e.g. "hash
with or without check" knob) while it is in 'pu', and *1* is to allow
it to progress
faster without having to worry about something we could do mechanically
before making it ready for 'next'.

The performance thing is really "let's see how well it goes". With effort to
optimize still "just has began", I think it is too early to tell if
Linus's "doesn't
really seem to matter" is the case or not.

Queuing such a topic on 'pu' is one effective way to make sure people are
working off of the same codebase.

^ permalink raw reply

* Re: [PATCH] Put sha1dc on a diet
From: Johannes Schindelin @ 2017-03-01 21:56 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Linus Torvalds, Jeff King, Marc Stevens, Dan Shumow,
	Git Mailing List
In-Reply-To: <xmqq37ewhji1.fsf@gitster.mtv.corp.google.com>

Hi,

On Wed, 1 Mar 2017, Junio C Hamano wrote:

> Linus Torvalds <torvalds@linux-foundation.org> writes:
> 
> > That said, I think that it would be lovely to just default to
> > USE_SHA1DC and just put the whole attack behind us. Yes, it's slower.
> > No, it doesn't really seem to matter that much in practice.
> 
> Yes.  It would be a very good goal.

So let me get this straight: not only do we now implicitly want to bump
the required C compiler to C99 without any grace period worth mentioning
[*1*], we are also all of a sudden no longer worried about a double digit
percentage drop of speed [*2*]?

Puzzled,
Johannes

Footnote *1*: I know, it is easy to forget that some developers cannot
choose their tools, or even their hardware. In the past, we seemed to take
appropriate care, though.

Footnote *2*: With real-world repositories of notable size, that
performance regression hurts. A lot. We just spent time to get the speed
of SHA-1 down by a couple percent and it was a noticeable improvement here.

^ permalink raw reply

* [PATCH v2 2/2] gitweb tests: Skip tests when we don't have Time::HiRes
From: Ævar Arnfjörð Bjarmason @ 2017-03-01 21:15 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jakub Narębski,
	Ævar Arnfjörð Bjarmason
In-Reply-To: <4b34e3a0-3da7-d821-2a7f-9a420ac1d3f6@gmail.com>

Change the gitweb tests to skip when we can't load the Time::HiRes
module.

Gitweb needs this module to work. It has been in perl core since v5.8,
which is the oldest version we support. However CentOS (and perhaps
some other distributions) carve it into its own non-core-perl package
that's not installed along with /usr/bin/perl by default. Without this
we'll hard fail the gitweb tests when trying to load the module.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/gitweb-lib.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 59ef15efbd..b7a73874e7 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -114,4 +114,9 @@ perl -MCGI -MCGI::Util -MCGI::Carp -e 0 >/dev/null 2>&1 || {
 	test_done
 }
 
+perl -mTime::HiRes -e 0  >/dev/null 2>&1 || {
+	skip_all='skipping gitweb tests, Time::HiRes module not available'
+	test_done
+}
+
 gitweb_init
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH] Put sha1dc on a diet
From: Linus Torvalds @ 2017-03-01 22:16 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Jeff King, Marc Stevens, Dan Shumow,
	Git Mailing List
In-Reply-To: <alpine.DEB.2.20.1703012227010.3767@virtualbox>

On Wed, Mar 1, 2017 at 1:56 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Footnote *1*: I know, it is easy to forget that some developers cannot
> choose their tools, or even their hardware. In the past, we seemed to take
> appropriate care, though.

I don't think you need to worry about the Windows side. That can
continue to do something else.

When I advocated perhaps using  USE_SHA1DC by default, I definitely
did not mean it in a "everywhere, regardless of issues" manner.

For example, the conmfig.mak.uname script already explicitly asks for
"BLK_SHA1 = YesPlease" for Windows. Don't bother changing that, it's
an explicit choice.

But the Linux rules don't actually specify which SHA1 version to use,
so the main Makefile currently defaults to just using openssl.

So that's the "default" choice I think we might want to change. Not
the "we're windows, and explicitly want BLK_SHA1 because of
environment and build infrastructure".

             Linus

^ permalink raw reply

* Re: [PATCH v8 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
From: Junio C Hamano @ 2017-03-01 22:43 UTC (permalink / raw)
  To: Thomas Gummerer
  Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
	Matthieu Moy
In-Reply-To: <20170301215759.GA11067@hank>

Thomas Gummerer <t.gummerer@gmail.com> writes:

> On 02/28, Junio C Hamano wrote:
>> Thomas Gummerer <t.gummerer@gmail.com> writes:
>> 
>> > +			git reset ${GIT_QUIET:+-q} -- "$@"
>> > +			git ls-files -z --modified -- "$@" |
>> > +			git checkout-index -z --force --stdin
>> > +			git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
>> 
>> I think you forgot to remove this line, whose correction was added
>> as two lines immediately before it.  I'll remove it while queuing.
>
> Yes, sorry.  What you queued looks good to me, thanks!

Thanks for double-checking, and thanks for working on the topic.  I
think this is ready for 'next'.

^ permalink raw reply

* Re: [PATCH 5/6 v5] merge.c: delegate handling of "-" shorthand to revision.c:get_sha1
From: Junio C Hamano @ 2017-03-01 22:49 UTC (permalink / raw)
  To: Siddharth Kannan; +Cc: git, Matthieu.Moy, pranit.bauva, peff, pclouds, sandals
In-Reply-To: <1488007487-12965-6-git-send-email-kannan.siddharth12@gmail.com>

Siddharth Kannan <kannan.siddharth12@gmail.com> writes:

> The callchain for handling each argument contains the function
> revision.c:get_sha1 where the shorthand for "-" ~ "@{-1}" has already been
> implemented in a previous patch; the complete callchain leading to that
> function is:
>
> 1. merge.c:collect_parents
> 2. commit.c:get_merge_parent : this function calls revision.c:get_sha1
>
> This patch also adds a test for checking that the shorthand works properly

This breaks "git merge".

> +test_expect_success 'merge - should work' '
> +        git checkout testing-2 &&
> +        git merge - &&
> +        git rev-parse HEAD HEAD^^ | sort >actual &&
> +        git rev-parse master testing-1 | sort >expect &&
> +        test_cmp expect actual

This test is not sufficient to catch a regression I seem to be
seeing.

	$ git checkout side
	$ git checkout pu
	$ git merge -

used to say "Merge branch 'side' into pu".  With this series merged,
I seem to be getting "Merge commit '-' into pu".

^ permalink raw reply

* Re: [PATCH 01/10] submodule: decouple url and submodule existence
From: Stefan Beller @ 2017-03-01 21:53 UTC (permalink / raw)
  To: Brandon Williams; +Cc: Junio C Hamano, git@vger.kernel.org
In-Reply-To: <20170301200230.GA30622@google.com>

IIRC most of the series is actually refactoring, i.e.
providing a central function that answers
"is this submodule active/initialized?" and then making use of this
function.

Maybe it would be easier to reroll these refactorings first without adding
in the change of behavior.

Off the list we talked about different models how to indicate that
a submodule is active.

Current situation:
submodule.<name>.URL is used as a boolean flag

Possible future A:
1) If submodule.active exists use that
2) otherwise go be individual .URL configs

submodule.active is a config that contains a pathspec,
i.e. specifies the group of submodules instead of marking
each submodule individually.

How to get to future A:
* apply this patch series

Possible future B:
1) the boolean submodule.<name>.exists is used
    if existent
2) If that boolean is missing we'd respect a grouping
   feature such as submodule.active
3) fallback to the .URL as a boolean indicator.

How to get to future B:
1) possibly take the refactoring part from this series
2) implement the .exists flag (that can solve the worktree
  problem, as then we don't have to abuse the .URL
  as a boolean indicator any more.)
3) implement the grouping feature that takes precedence
  over .URL, but yields precedence to the .exists flag.
  (This would solve the grouping problem)

By having two different series (2) and (3) we'd solve
just one thing at a time with each series, such that
this seems easier to understand and review.

The question is if this future B is also easier to use for
users and I think it would be, despite being technically more
complex.

Most users only have a few submodules. Also the assumption
is to have either interest in very few specific submodules
or all of them. For both of these cases the .exists is a good idea
as it is very explicit, but verbose.

The grouping feature would help in the case of lots of
submodules, e.g. to select all of them you would just give
an easy pathspec "." and that would help going forward
as by such a submodule spec all new submodules would
be "auto-on".

Possible future C:
Similar to B, but with branch specific configuration.
submodule.<branchname>.active would work as a
grouping feature. I wonder how it would work with
the .exists flag.

Stefan

^ permalink raw reply

* [PATCH v2 1/2] gitweb tests: Change confusing "skip_all" phrasing
From: Ævar Arnfjörð Bjarmason @ 2017-03-01 21:15 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jakub Narębski,
	Ævar Arnfjörð Bjarmason
In-Reply-To: <4b34e3a0-3da7-d821-2a7f-9a420ac1d3f6@gmail.com>

Change the phrasing so that instead of saying that the CGI module is
unusable, we say that it's not available.

This came up on the git mailing list in
<4b34e3a0-3da7-d821-2a7f-9a420ac1d3f6@gmail.com> from Jakub Narębski.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/gitweb-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index d5dab5a94f..59ef15efbd 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -110,7 +110,7 @@ perl -MEncode -e '$e="";decode_utf8($e, Encode::FB_CROAK)' >/dev/null 2>&1 || {
 }
 
 perl -MCGI -MCGI::Util -MCGI::Carp -e 0 >/dev/null 2>&1 || {
-	skip_all='skipping gitweb tests, CGI module unusable'
+	skip_all='skipping gitweb tests, CGI & CGI::Util & CGI::Carp modules not available'
 	test_done
 }
 
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH v2 0/2] Minor changes to skip gitweb tests without Time::HiRes
From: Junio C Hamano @ 2017-03-01 22:42 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, Jakub Narębski
In-Reply-To: <20170301211540.4382-1-avarab@gmail.com>

Thanks, will replace what has been on 'pu'.

Note that I'd dropped a double-SP from the latter one while queuing.

^ permalink raw reply

* What's cooking in git.git (Mar 2017, #01; Wed, 1)
From: Junio C Hamano @ 2017-03-01 22:35 UTC (permalink / raw)
  To: git

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

Note: some may have sent updates to topics that are marked as "will
merge to 'next'" that cross with this message.  I'll try to pick them
up before merging the topics to 'next', but mistakes happen X-< and
it appears at least at my end that the latency of messages sent to
vger reaching recipients has got longer in the past few weeks?

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

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

--------------------------------------------------
[New Topics]

* jk/interpret-branch-name (2017-02-28) 8 commits
 - checkout: restrict @-expansions when finding branch
 - strbuf_check_ref_format(): expand only local branches
 - branch: restrict @-expansions when deleting
 - t3204: test git-branch @-expansion corner cases
 - interpret_branch_name: allow callers to restrict expansions
 - strbuf_branchname: add docstring
 - strbuf_branchname: drop return value
 - interpret_branch_name: move docstring to header file

 "git branch @" created refs/heads/@ as a branch, and in general the
 code that handled @{-1} and @{upstream} was a bit too loose in
 disambiguating.

 Expecting a reroll.
 cf. <20170228123331.wubqplp5zjwzz6is@sigill.intra.peff.net>


* jk/sha1dc (2017-03-01) 7 commits
 - Put sha1dc on a diet
 - sha1dc: avoid 'for' loop initial decl
 - sha1dc: resurrect LICENSE file
 - sha1dc: avoid c99 declaration-after-statement
 - Makefile: add USE_SHA1DC knob
 - sha1dc: adjust header includes for git
 - add collision-detecting sha1 implementation

 Borrow "detect attempt to create collisions" variant of SHA-1
 implementation by Marc Stevens (CWI) and Dan Shumow (Microsoft).

 Expecting a cleaned-up reroll after discussion settles.
 cf. <CA+55aFxTWqsTTiDKo4DBZT-8Z9t80bGMD3uijzKONa_bYEZABQ@mail.gmail.com>


* js/travis-32bit-linux (2017-02-28) 1 commit
 - Travis: also test on 32-bit Linux

 Add 32-bit Linux variant to the set of platforms to be tested with
 Travis CI.

 Will merge to 'next'.


* jt/http-base-url-update-upon-redirect (2017-02-28) 1 commit
 - http: attempt updating base URL only if no error

 When a redirected http transport gets an error during the
 redirected request, we ignored the error we got from the server,
 and ended up giving a not-so-useful error message.

 Will merge to 'next'.


* jt/mark-tree-uninteresting-for-uninteresting-commit (2017-02-28) 3 commits
 - upload-pack: compute blob reachability correctly
 - revision: exclude trees/blobs given commit
 - revision: unify {tree,blob}_objects in rev_info

 The revision/object traversal machinery did not mark all tree and
 blob objects that are contained in an uninteresting commit as
 uninteresting, because that is quite costly.  Instead, it only
 marked those that are contained in an uninteresting boundary commit
 as uninteresting.


* ps/docs-diffcore (2017-02-28) 2 commits
 - docs/diffcore: unquote "Complete Rewrites" in headers
 - docs/diffcore: fix grammar in diffcore-rename header

 Doc update.

 Will merge to 'next'.


* rj/remove-unused-mktemp (2017-02-28) 2 commits
 - wrapper.c: remove unused gitmkstemps() function
 - wrapper.c: remove unused git_mkstemp() function

 Code cleanup.

 Will merge to 'next'.


* sb/submodule-init-url-selection (2017-02-28) 1 commit
 - submodule init: warn about falling back to a local path

 Give a warning when "git submodule init" decides that the submodule
 in the working tree is its upstream, as it is not a very common
 setup.

 Will merge to 'next'.


* jc/diff-populate-filespec-size-only-fix (2017-03-01) 1 commit
 - diff: do not short-cut CHECK_SIZE_ONLY check in diff_populate_filespec()

 "git diff --quiet" relies on the size field in diff_filespec to be
 correctly populated, but diff_populate_filespec() helper function
 made an incorrect short-cut when asked only to populate the size
 field for paths that need to go through convert_to_git() (e.g. CRLF
 conversion).

 Will merge to 'next'.


* nd/conditional-config-include (2017-03-01) 4 commits
 - SQUASH???
 - config: add conditional include
 - config.txt: reflow the second include.path paragraph
 - config.txt: clarify multiple key values in include.path

 The configuration file learned a new "includeIf.<condition>.path"
 that includes the contents of the given path only when the
 condition holds.  This allows you to say "include this work-related
 bit only in the repositories under my ~/work/ directory".

 I think this is almost ready for 'next'.


* rs/log-email-subject (2017-03-01) 2 commits
 - pretty: use fmt_output_email_subject()
 - log-tree: factor out fmt_output_email_subject()

 Code clean-up.

 Will merge to 'next'.

--------------------------------------------------
[Stalled]

* nd/worktree-move (2017-01-27) 7 commits
 . fixup! worktree move: new command
 . worktree remove: new command
 . worktree move: refuse to move worktrees with submodules
 . worktree move: accept destination as directory
 . worktree move: new command
 . worktree.c: add update_worktree_location()
 . worktree.c: add validate_worktree()

 "git worktree" learned move and remove subcommands.

 Tentatively ejected as it seems to break 'pu' when merged.


* pb/bisect (2017-02-18) 28 commits
 - fixup! bisect--helper: `bisect_next_check` & bisect_voc shell function in C
 - bisect--helper: remove the dequote in bisect_start()
 - bisect--helper: retire `--bisect-auto-next` subcommand
 - bisect--helper: retire `--bisect-autostart` subcommand
 - bisect--helper: retire `--bisect-write` subcommand
 - bisect--helper: `bisect_replay` shell function in C
 - bisect--helper: `bisect_log` shell function in C
 - bisect--helper: retire `--write-terms` subcommand
 - bisect--helper: retire `--check-expected-revs` subcommand
 - bisect--helper: `bisect_state` & `bisect_head` shell function in C
 - bisect--helper: `bisect_autostart` shell function in C
 - bisect--helper: retire `--next-all` subcommand
 - bisect--helper: retire `--bisect-clean-state` subcommand
 - bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
 - t6030: no cleanup with bad merge base
 - bisect--helper: `bisect_start` shell function partially in C
 - bisect--helper: `get_terms` & `bisect_terms` shell function in C
 - bisect--helper: `bisect_next_check` & bisect_voc shell function in C
 - bisect--helper: `check_and_set_terms` shell function in C
 - bisect--helper: `bisect_write` shell function in C
 - bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
 - bisect--helper: `bisect_reset` shell function in C
 - wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
 - t6030: explicitly test for bisection cleanup
 - bisect--helper: `bisect_clean_state` shell function in C
 - bisect--helper: `write_terms` shell function in C
 - bisect: rewrite `check_term_format` shell function in C
 - bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

 Move more parts of "git bisect" to C.

 Expecting a reroll.
 cf. <CAFZEwPPXPPHi8KiEGS9ggzNHDCGhuqMgH9Z8-Pf9GLshg8+LPA@mail.gmail.com>
 cf. <CAFZEwPM9RSTGN54dzaw9gO9iZmsYjJ_d1SjUD4EzSDDbmh-XuA@mail.gmail.com>
 cf. <CAFZEwPNUXcNY9Qdz=_B7q2kQuaecPzJtTMGdv8YMUPEz2vnp8A@mail.gmail.com>


* ls/filter-process-delayed (2017-01-08) 1 commit
 . convert: add "status=delayed" to filter process protocol

 Ejected, as does not build when merged to 'pu'.


* sh/grep-tree-obj-tweak-output (2017-01-20) 2 commits
 - grep: use '/' delimiter for paths
 - grep: only add delimiter if there isn't one already

 "git grep", when fed a tree-ish as an input, shows each hit
 prefixed with "<tree-ish>:<path>:<lineno>:".  As <tree-ish> is
 almost always either a commit or a tag that points at a commit, the
 early part of the output "<tree-ish>:<path>" can be used as the
 name of the blob and given to "git show".  When <tree-ish> is a
 tree given in the extended SHA-1 syntax (e.g. "<commit>:", or
 "<commit>:<dir>"), however, this results in a string that does not
 name a blob (e.g. "<commit>::<path>" or "<commit>:<dir>:<path>").
 "git grep" has been taught to be a bit more intelligent about these
 cases and omit a colon (in the former case) or use slash (in the
 latter case) to produce "<commit>:<path>" and
 "<commit>:<dir>/<path>" that can be used as the name of a blob.

 Expecting a reroll?  Is this good enough with known limitations?


* jc/diff-b-m (2015-02-23) 5 commits
 . WIPWIP
 . WIP: diff-b-m
 - diffcore-rename: allow easier debugging
 - diffcore-rename.c: add locate_rename_src()
 - diffcore-break: allow debugging

 "git diff -B -M" produced incorrect patch when the postimage of a
 completely rewritten file is similar to the preimage of a removed
 file; such a resulting file must not be expressed as a rename from
 other place.

 The fix in this patch is broken, unfortunately.

 Will discard.

--------------------------------------------------
[Cooking]

* cc/split-index-config (2017-03-01) 22 commits
 - Documentation/git-update-index: explain splitIndex.*
 - Documentation/config: add splitIndex.sharedIndexExpire
 - read-cache: use freshen_shared_index() in read_index_from()
 - read-cache: refactor read_index_from()
 - t1700: test shared index file expiration
 - read-cache: unlink old sharedindex files
 - config: add git_config_get_expiry() from gc.c
 - read-cache: touch shared index files when used
 - sha1_file: make check_and_freshen_file() non static
 - Documentation/config: add splitIndex.maxPercentChange
 - t1700: add tests for splitIndex.maxPercentChange
 - read-cache: regenerate shared index if necessary
 - config: add git_config_get_max_percent_split_change()
 - Documentation/git-update-index: talk about core.splitIndex config var
 - Documentation/config: add information for core.splitIndex
 - t1700: add tests for core.splitIndex
 - update-index: warn in case of split-index incoherency
 - read-cache: add and then use tweak_split_index()
 - split-index: add {add,remove}_split_index() functions
 - config: add git_config_get_split_index()
 - t1700: change here document style
 - config: mark an error message up for translation

 The experimental "split index" feature has gained a few
 configuration variables to make it easier to use.

 I think this is almost ready for 'next'.


* dp/filter-branch-prune-empty (2017-02-23) 4 commits
 - p7000: add test for filter-branch with --prune-empty
 - filter-branch: fix --prune-empty on parentless commits
 - t7003: ensure --prune-empty removes entire branch when applicable
 - t7003: ensure --prune-empty can prune root commit

 "git filter-branch --prune-empty" drops a single-parent commit that
 becomes a no-op, but did not drop a root commit whose tree is empty.

 Needs review.


* jc/config-case-cmdline-take-2 (2017-02-23) 2 commits
  (merged to 'next' on 2017-03-01 at 2e9920eeeb)
 + config: use git_config_parse_key() in git_config_parse_parameter()
 + config: move a few helper functions up

 The code to parse "git -c VAR=VAL cmd" and set configuration
 variable for the duration of cmd had two small bugs, which have
 been fixed.

 Will merge to 'master'.
 This supersedes jc/config-case-cmdline topic that has been discarded.


* ab/cond-skip-tests (2017-02-27) 2 commits
 - gitweb tests: skip tests when we don't have Time::HiRes
 - cvs tests: skip tests that call "cvs commit" when running as root

 A few tests were run conditionally under (rare) conditions where
 they cannot be run (like running cvs tests under 'root' account).

 Will merge to 'next'.


* jk/auto-namelen-in-interpret-branch-name (2017-02-27) 1 commit
 - interpret_branch_name(): handle auto-namelen for @{-1}

 A small bug in the code that parses @{...} has been fixed.

 Will merge to 'next'.


* jk/interop-test (2017-02-27) 2 commits
 - t/interop: add test of old clients against modern git-daemon
 - t: add an interoperability test harness

 Picking two versions of Git and running tests to make sure the
 older one and the newer one interoperate happily has now become
 possible.

 Needs review.


* jk/parse-config-key-cleanup (2017-02-24) 3 commits
  (merged to 'next' on 2017-03-01 at e531d8d3a9)
 + parse_hide_refs_config: tell parse_config_key we don't want a subsection
 + parse_config_key: allow matching single-level config
 + parse_config_key: use skip_prefix instead of starts_with
 (this branch uses sb/parse-hide-refs-config-cleanup.)

 The "parse_config_key()" API function has been cleaned up.

 Will merge to 'master'.


* jk/t6300-cleanup (2017-02-27) 1 commit
 - t6300: avoid creating refs/heads/HEAD

 A test that creats a confusing branch whose name is HEAD when any
 branch name would have sufficed has been corrected.

 Will merge to 'next'.


* rs/commit-parsing-optim (2017-02-27) 2 commits
 - commit: don't check for space twice when looking for header
 - commit: be more precise when searching for headers

 The code that parses header fields in the commit object has been
 updated for (micro)performance and code hygiene.

 Will merge to 'next'.


* rs/sha1-file-plug-fallback-base-leak (2017-02-27) 1 commit
 - sha1_file: release fallback base's memory in unpack_entry()

 A leak in a codepath to read from a packed object in (rare) cases
 has been plugged.

 Will merge to 'next'.


* rs/strbuf-add-real-path (2017-02-27) 2 commits
 - strbuf: add strbuf_add_real_path()
 - cocci: use ALLOC_ARRAY

 An helper function to make it easier to append the result from
 real_path() to a strbuf has been added.

 Will merge to 'next'.


* sb/parse-hide-refs-config-cleanup (2017-02-24) 1 commit
  (merged to 'next' on 2017-03-01 at fd722ba039)
 + refs: parse_hide_refs_config to use parse_config_key
 (this branch is used by jk/parse-config-key-cleanup.)

 Code clean-up.

 Will merge to 'master'.


* sg/clone-refspec-from-command-line-config (2017-02-27) 1 commit
 - clone: respect configured fetch respecs during initial fetch

 Needs review.
 cf. <20170227211217.73gydlxb2qu2sp3m@sigill.intra.peff.net>


* sk/dash-is-previous (2017-02-27) 6 commits
 - revert.c: delegate handling of "-" shorthand to setup_revisions
 - merge.c: delegate handling of "-" shorthand to revision.c:get_sha1
 - sha1_name.c: teach get_sha1_1 "-" shorthand for "@{-1}"
 - revision.c: args starting with "-" might be a revision
 - revision.c: swap if/else blocks
 - revision.c: do not update argv with unknown option

 A dash "-" can be written to mean "the branch that was previously
 checked out" in more places.

 Needs review.
 cf. <1488007487-12965-1-git-send-email-kannan.siddharth12@gmail.com>


* jh/send-email-one-cc (2017-02-27) 1 commit
 - send-email: only allow one address per body tag

 "Cc:" on the trailer part does not have to conform to RFC strictly,
 unlike in the e-mail header.  "git send-email" has been updated to
 ignore anything after '>' when picking addresses, to allow non-address
 cruft like " # stable 4.4" after the address.

 Will merge to 'next'.


* jk/http-auth (2017-02-27) 2 commits
 - http: add an "auto" mode for http.emptyauth
 - http: restrict auth methods to what the server advertises

 Reduce authentication round-trip over HTTP when the server supports
 just a single authentication method.

 Will merge to 'next'.


* jk/ident-empty (2017-02-23) 4 commits
  (merged to 'next' on 2017-03-01 at ff80031ce6)
 + ident: do not ignore empty config name/email
 + ident: reject all-crud ident name
 + ident: handle NULL email when complaining of empty name
 + ident: mark error messages for translation

 user.email that consists of only cruft chars should have
 consistently errored out, but didn't.

 Will merge to 'master'.


* jt/upload-pack-error-report (2017-02-23) 1 commit
  (merged to 'next' on 2017-03-01 at aea583dbe5)
 + upload-pack: report "not our ref" to client

 "git upload-pack", which is a counter-part of "git fetch", did not
 report a request for a ref that was not advertised as invalid.
 This is generally not a problem (because "git fetch" will stop
 before making such a request), but is the right thing to do.

 Will merge to 'master'.


* ah/doc-ls-files-quotepath (2017-02-28) 3 commits
 - SQUASH???
 - Documentation: Link descriptions of -z to core.quotePath
 - Documentation: Improve description for core.quotePath

 Documentation for "git ls-files" did not refer to core.quotePath


* jh/memihash-opt (2017-02-17) 5 commits
 - name-hash: remember previous dir_entry during lazy_init_name_hash
 - name-hash: specify initial size for istate.dir_hash table
 - name-hash: precompute hash values during preload-index
 - hashmap: allow memihash computation to be continued
 - name-hash: eliminate duplicate memihash call

 Expecting an update for perf?


* nd/prune-in-worktree (2017-02-19) 15 commits
 . rev-list: expose and document --single-worktree
 . revision.c: --reflog add HEAD reflog from all worktrees
 . files-backend: make reflog iterator go through per-worktree reflog
 . refs: add refs_for_each_reflog[_ent]()
 . revision.c: --all adds HEAD from all worktrees
 . refs: remove dead for_each_*_submodule()
 . revision.c: use refs_for_each*() instead of for_each_*_submodule()
 . refs: add a refs_for_each_in() and friends
 . refs: add refs_for_each_ref()
 . refs: add refs_head_ref()
 . refs: add refs_read_ref[_full]()
 . refs: move submodule slash stripping code to get_submodule_ref_store
 . revision.c: --indexed-objects add objects from all worktrees
 . revision.c: refactor add_index_objects_to_pending()
 . revision.h: new flag in struct rev_info wrt. worktree-related refs
 (this branch uses nd/worktree-kill-parse-ref; is tangled with nd/files-backend-git-dir.)

 "git gc" and friends when multiple worktrees are used off of a
 single repository did not consider the index and per-worktree refs
 of other worktrees as the root for reachability traversal, making
 objects that are in use only in other worktrees to be subject to
 garbage collection.


* mm/fetch-show-error-message-on-unadvertised-object (2017-02-22) 4 commits
 - fetch-pack: add specific error for fetching an unadvertised object
 - fetch_refs_via_pack: call report_unmatched_refs
 - squash??? remove unfinished sentence
 - fetch-pack: move code to report unmatched refs to a function

 "git fetch" that requests a commit by object name, when the other
 side does not allow such an request, failed without much
 explanation.

 Will squash the change in before merging to 'next'.


* nd/worktree-kill-parse-ref (2017-02-19) 22 commits
 . refs: kill set_worktree_head_symref()
 . refs: add refs_create_symref()
 . worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()
 . refs.c: add refs_resolve_ref_unsafe()
 . refs: introduce get_worktree_ref_store()
 . refs: rename get_ref_store() to get_submodule_ref_store() and make it public
 . files-backend: remove submodule_allowed from files_downcast()
 . refs: move submodule code out of files-backend.c
 . path.c: move some code out of strbuf_git_path_submodule()
 . refs.c: make get_main_ref_store() public and use it
 . refs.c: kill register_ref_store(), add register_submodule_ref_store()
 . refs.c: flatten get_ref_store() a bit
 . refs: rename lookup_ref_store() to lookup_submodule_ref_store()
 . refs.c: introduce get_main_ref_store()
 . files-backend: remove the use of git_path()
 . refs.c: share is_per_worktree_ref() to files-backend.c
 . files-backend: replace *git_path*() with files_path()
 . files-backend: add files_path()
 . files-backend: convert git_path() to strbuf_git_path()
 . refs-internal.c: make files_log_ref_write() static
 . Merge branch 'mh/ref-remove-empty-directory' into nd/files-backend-git-dir
 . Merge branch 'mh/submodule-hash' into nd/files-backend-git-dir
 (this branch is used by nd/prune-in-worktree; is tangled with nd/files-backend-git-dir.)

 (hopefully) a beginning of safer "git worktree" that is resistant
 to "gc".

 Waiting for nd/files-backend-git-dir to settle.


* nd/files-backend-git-dir (2017-02-22) 26 commits
 . t1406: new tests for submodule ref store
 . t1405: some basic tests on main ref store
 . t/helper: add test-ref-store to test ref-store functions
 . refs: delete pack_refs() in favor of refs_pack_refs()
 . files-backend: avoid ref api targetting main ref store
 . refs: new transaction related ref-store api
 . refs: add new ref-store api
 . refs: rename get_ref_store() to get_submodule_ref_store() and make it public
 . files-backend: replace submodule_allowed check in files_downcast()
 . refs: move submodule code out of files-backend.c
 . path.c: move some code out of strbuf_git_path_submodule()
 . refs.c: make get_main_ref_store() public and use it
 . refs.c: kill register_ref_store(), add register_submodule_ref_store()
 . refs.c: flatten get_ref_store() a bit
 . refs: rename lookup_ref_store() to lookup_submodule_ref_store()
 . refs.c: introduce get_main_ref_store()
 . files-backend: remove the use of git_path()
 . files-backend: add and use files_refname_path()
 . files-backend: add and use files_reflog_path()
 . files-backend: move "logs/" out of TMP_RENAMED_LOG
 . files-backend: convert git_path() to strbuf_git_path()
 . files-backend: add and use files_packed_refs_path()
 . files-backend: make files_log_ref_write() static
 . refs.h: add forward declaration for structs used in this file
 . Merge branch 'mh/ref-remove-empty-directory' into nd/files-backend-git-dir
 . Merge branch 'mh/submodule-hash' into nd/files-backend-git-dir
 (this branch is tangled with nd/prune-in-worktree and nd/worktree-kill-parse-ref.)

 The "submodule" specific field in the ref_store structure is
 replaced with a more generic "gitdir" that can later be used also
 when dealing with ref_store that represents the set of refs visible
 from the other worktrees.

 Needs review.


* sb/checkout-recurse-submodules (2017-02-23) 15 commits
 - builtin/checkout: add --recurse-submodules switch
 - entry.c: update submodules when interesting
 - read-cache, remove_marked_cache_entries: wipe selected submodules.
 - unpack-trees: check if we can perform the operation for submodules
 - unpack-trees: pass old oid to verify_clean_submodule
 - update submodules: add submodule_move_head
 - update submodules: move up prepare_submodule_repo_env
 - submodules: introduce check to see whether to touch a submodule
 - update submodules: add a config option to determine if submodules are updated
 - update submodules: add submodule config parsing
 - connect_work_tree_and_git_dir: safely create leading directories
 - make is_submodule_populated gently
 - lib-submodule-update.sh: define tests for recursing into submodules
 - lib-submodule-update.sh: do not use ./. as submodule remote
 - lib-submodule-update.sh: reorder create_lib_submodule_repo

 "git checkout" is taught --recurse-submodules option.

 Needs review.


* tg/stash-push (2017-02-28) 6 commits
 - stash: allow pathspecs in the no verb form
 - stash: use stash_push for no verb form
 - stash: teach 'push' (and 'create_stash') to honor pathspec
 - stash: refactor stash_create
 - stash: add test for the create command line arguments
 - stash: introduce push verb

 Allow "git stash" to take pathspec so that the local changes can be
 stashed away only partially.

 Will merge to 'next'.


* bc/object-id (2017-02-22) 19 commits
 - wt-status: convert to struct object_id
 - builtin/merge-base: convert to struct object_id
 - Convert object iteration callbacks to struct object_id
 - sha1_file: introduce an nth_packed_object_oid function
 - refs: simplify parsing of reflog entries
 - refs: convert each_reflog_ent_fn to struct object_id
 - reflog-walk: convert struct reflog_info to struct object_id
 - builtin/replace: convert to struct object_id
 - Convert remaining callers of resolve_refdup to object_id
 - builtin/merge: convert to struct object_id
 - builtin/clone: convert to struct object_id
 - builtin/branch: convert to struct object_id
 - builtin/grep: convert to struct object_id
 - builtin/fmt-merge-message: convert to struct object_id
 - builtin/fast-export: convert to struct object_id
 - builtin/describe: convert to struct object_id
 - builtin/diff-tree: convert to struct object_id
 - builtin/commit: convert to struct object_id
 - hex: introduce parse_oid_hex

 "uchar [40]" to "struct object_id" conversion continues.

 Now at v5.
 cf. <20170221234737.894681-1-sandals@crustytoothpaste.net>


* jh/mingw-openssl-sha1 (2017-02-09) 1 commit
 - mingw: use OpenSSL's SHA-1 routines

 Windows port wants to use OpenSSL's implementation of SHA-1
 routines, so let them.

 Kicked back to 'pu'
 cf. <9913e513-553e-eba6-e81a-9c21030dd767@kdbg.org>


* sg/completion-refs-speedup (2017-02-13) 13 commits
 - squash! completion: fill COMPREPLY directly when completing refs
 - completion: fill COMPREPLY directly when completing refs
 - completion: list only matching symbolic and pseudorefs when completing refs
 - completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery
 - completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery
 - completion: let 'for-each-ref' strip the remote name from remote branches
 - completion: let 'for-each-ref' and 'ls-remote' filter matching refs
 - completion: don't disambiguate short refs
 - completion: don't disambiguate tags and branches
 - completion: support excluding full refs
 - completion: support completing full refs after '--option=refs/<TAB>'
 - completion: wrap __git_refs() for better option parsing
 - completion: remove redundant __gitcomp_nl() options from _git_commit()

 The refs completion for large number of refs has been sped up,
 partly by giving up disambiguating ambiguous refs and partly by
 eliminating most of the shell processing between 'git for-each-ref'
 and 'ls-remote' and Bash's completion facility.

 What's the donness of this topic?


* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
  (merged to 'next' on 2017-02-27 at 7373a1b73d)
 + setup_git_env: avoid blind fall-back to ".git"

 This is the endgame of the topic to avoid blindly falling back to
 ".git" when the setup sequence said we are _not_ in Git repository.
 A corner case that happens to work right now may be broken by a
 call to die("BUG").

 Will cook in 'next'.


* jc/merge-drop-old-syntax (2015-04-29) 1 commit
  (merged to 'next' on 2017-02-27 at 2c0f5f73d8)
 + merge: drop 'git merge <message> HEAD <commit>' syntax

 Stop supporting "git merge <message> HEAD <commit>" syntax that has
 been deprecated since October 2007, and issues a deprecation
 warning message since v2.5.0.

 Will cook in 'next'.


* jc/bundle (2016-03-03) 6 commits
 - index-pack: --clone-bundle option
 - Merge branch 'jc/index-pack' into jc/bundle
 - bundle v3: the beginning
 - bundle: keep a copy of bundle file name in the in-core bundle header
 - bundle: plug resource leak
 - bundle doc: 'verify' is not about verifying the bundle

 The beginning of "split bundle", which could be one of the
 ingredients to allow "git clone" traffic off of the core server
 network to CDN.

--------------------------------------------------
[Discarded]

* sb/push-make-submodule-check-the-default (2017-01-26) 2 commits
 . Revert "push: change submodule default to check when submodules exist"
 . push: change submodule default to check when submodules exist

 Turn the default of "push.recurseSubmodules" to "check" when
 submodules seem to be in use.

 Retracted.


* ls/submodule-config-ucase (2017-02-15) 2 commits
 . submodule config does not apply to upper case submodules?
 . t7400: cleanup "submodule add clone shallow submodule" test

 Demonstrate a breakage in handling submodule.UPPERCASENAME.update
 configuration variables.

 Superseded by jc/config-case-cmdline.


* js/curl-empty-auth-set-by-default (2017-02-22) 1 commit
 . http(s): automatically try NTLM authentication first

 Flip "http.emptyAuth" on by default to help OOB experience for
 users of HTTP Negotiate authentication scheme.

 Superseded by jk/http-auth topic.


* jc/config-case-cmdline (2017-02-21) 3 commits
 . config: squelch stupid compiler
 . config: reject invalid VAR in 'git -c VAR=VAL command'
 . config: preserve <subsection> case for one-shot config on the command line

 The code to parse "git -c VAR=VAL cmd" and set configuration
 variable for the duration of cmd had two small bugs, which have
 been fixed.

 Superseded by jc/config-case-cmdline-take-2


* lt/oneline-decoration-at-end (2017-02-21) 2 commits
 . log: fix regression to "--source" when "--decorate" was updated
 . show decorations at the end of the line

 The output from "git log --oneline --decorate" has been updated to
 show the extra information at the end of the line, not near the
 front.

 Retracted.
 cf. <CA+55aFwT2HUBzZO8Gpt9tHoJtdRxv9oe3TDoSH5jcEOixRNBXg@mail.gmail.com>


* sk/parse-remote-cleanup (2017-02-21) 2 commits
 . Revert "parse-remote: remove reference to unused op_prep"
 . parse-remote: remove reference to unused op_prep

 Code clean-up.

 Will discard.
 There may be third-party scripts that are dot-sourcing this one.

^ permalink raw reply

* Re: [PATCH v8 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
From: Thomas Gummerer @ 2017-03-01 21:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
	Matthieu Moy
In-Reply-To: <xmqqo9xmhshd.fsf@gitster.mtv.corp.google.com>

On 02/28, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > +			git reset ${GIT_QUIET:+-q} -- "$@"
> > +			git ls-files -z --modified -- "$@" |
> > +			git checkout-index -z --force --stdin
> > +			git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
> 
> I think you forgot to remove this line, whose correction was added
> as two lines immediately before it.  I'll remove it while queuing.

Yes, sorry.  What you queued looks good to me, thanks!

> > +			git clean --force ${GIT_QUIET:+-q} -d -- "$@"
> 
> Thanks.

^ permalink raw reply

* Re: [PATCH] Put sha1dc on a diet
From: Johannes Schindelin @ 2017-03-01 22:51 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Junio C Hamano, Jeff King, Marc Stevens, Dan Shumow,
	Git Mailing List
In-Reply-To: <CA+55aFys5oQ0RySQ+Xv0ZDussr-xZNh4_b3+Upx_d9VPWmpM8Q@mail.gmail.com>

Hi,

On Wed, 1 Mar 2017, Linus Torvalds wrote:

> On Wed, Mar 1, 2017 at 1:56 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Footnote *1*: I know, it is easy to forget that some developers cannot
> > choose their tools, or even their hardware. In the past, we seemed to take
> > appropriate care, though.
> 
> I don't think you need to worry about the Windows side.

I am not. I build G?t for Windows using GCC.

My concern is about that unexpected turn "oh, let's just switch to C99
because, well, because my compiler canehandle it, and everybody else
should just switch tn a modern compiler". That really sounded careless.

> That can continue to do something else.
> 
> When I advocated perhaps using  USE_SHA1DC by default, I definitely did
> not mean it in a "everywhere, regardless of issues" manner.
> 
> For example, the conmfig.mak.uname script already explicitly asks for
> "BLK_SHA1 = YesPlease" for Windows. Don't bother changing that, it's an
> explicit choice.

That setting is only in git.git's version, not in gxt-for-windows/git.git.
We switched to OpenSSL because of speed improvements, in particular with
recent Intel processors.

> But the Linux rules don't actually specify which SHA1 version to use,
> so the main Makefile currently defaults to just using openssl.
> 
> So that's the "default" choice I think we might want to change. Not
> the "we're windows, and explicitly want BLK_SHA1 because of
> environment and build infrastructure".

Since we switched away from BLOCK_SHA1, any such change would affect Git
for Windews.

But I think bigger than just developers on Windows OS. There are many
developers out there working on large repositories (yes, much larger than
Linux). Also using Macs and Linux. I am not at all sure that we want to
give them an updated Git they cannot fail to notice to be much slower than
before.

Don't get me wrong: I *hope* that you'll manage to get sha1dc
competitively fast. If you don't, well, then we simply cannot use it by
default for *all* of our calls (you already pointed out that the pack
index' checksum does not need collision detection, and in fact, *any*
operation that works on implicitly trusted data falls into the same court,
e.g. `git add`).

Ciao,
Johannes

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox