Re: [PATCH] files_initial_transaction_commit(): only unlock if locked

2018-01-18 Thread Jeff King
On Thu, Jan 18, 2018 at 02:38:41PM +0100, Mathias Rav wrote: > Running git clone --single-branch --mirror -b TAGNAME previously > triggered the following error message: > > fatal: multiple updates for ref 'refs/tags/TAGNAME' not allowed. > > This error condition is handled in

Re: [PATCH 3/3] read-cache: don't write index twice if we can't write shared index

2018-01-18 Thread Jeff King
On Thu, Jan 18, 2018 at 07:47:50PM +0700, Duy Nguyen wrote: > I may need help getting that log (or even better the trash directory > of t1700). I tried -m32 build, then valgrind on amd64 (because it > didn't work with my 32 build), running the tests with dash and even > the linux32 docker version

Re: [PATCH] packfile: use get_be64() for large offsets

2018-01-17 Thread Jeff King
On Wed, Jan 17, 2018 at 02:08:23PM -0500, Derrick Stolee wrote: > The pack-index version 2 format uses two 4-byte integers in network-byte > order to represent one 8-byte value. The current implementation has several > code clones for stitching these integers together. Yeah, this seems like a

Re: [PATCH v2 2/2] sha1_file: improve sha1_file_name() perfs

2018-01-17 Thread Jeff King
On Wed, Jan 17, 2018 at 12:54:33PM -0800, Junio C Hamano wrote: > Jeff Hostetler writes: > > >> void sha1_file_name(struct strbuf *buf, const unsigned char > >> *sha1) > >> { > >> - strbuf_addf(buf, "%s/", get_object_directory()); > >> + const char *obj_dir =

Re: [PATCH v5 4/7] trace.c: introduce trace_run_command()

2018-01-17 Thread Jeff King
On Mon, Jan 15, 2018 at 05:59:46PM +0700, Nguyễn Thái Ngọc Duy wrote: > diff --git a/trace.c b/trace.c > index 7f3b08e148..da3db301e7 100644 > --- a/trace.c > +++ b/trace.c > @@ -23,6 +23,7 @@ > > #include "cache.h" > #include "quote.h" > +#include "run-command.h" > > struct trace_key

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

2018-01-17 Thread Jeff King
On Tue, Jan 16, 2018 at 08:38:15PM +0100, Kim Gybels wrote: > Take a hint from commit ea68b0ce9f8 (hash-object: don't use mmap() for > small files, 2010-02-21) and use read() instead of mmap() for small > packed-refs files. > > This also fixes the problem[1] where xmmap() returns NULL for zero >

Re: [PATCH v2 01/18] cat-file: split expand_atom into 2 functions

2018-01-17 Thread Jeff King
On Tue, Jan 16, 2018 at 10:22:23AM +0300, Оля Тележная wrote: > >> In other words, I think the endgame is that expand_atom() isn't there at > >> all, and we're calling the equivalent of format_ref_item() for each > >> object (except that in a unified formatting world, it probably doesn't > >>

Re: [PATCH v2 04/18] cat-file: move struct expand_data into ref-filter

2018-01-17 Thread Jeff King
On Tue, Jan 16, 2018 at 10:00:42AM +0300, Оля Тележная wrote: > > I think some of these will want to remain in cat-file.c. For instance, > > split_on_whitespace is not something that ref-filter.c itself would care > > about. I'd expect in the endgame for cat-file to keep its own > >

Re: [PATCH v2 03/18] ref-filter: make valid_atom as function parameter

2018-01-17 Thread Jeff King
On Tue, Jan 16, 2018 at 09:55:22AM +0300, Оля Тележная wrote: > > IOW, the progression I'd expect in a series like this is: > > > > 1. Teach ref-filter.c to support everything that cat-file can do. > > > > 2. Convert cat-file to use ref-filter.c. > > I agree, I even made this and it's

Re: [PATCH] enable core.fsyncObjectFiles by default

2018-01-17 Thread Jeff King
On Wed, Jan 17, 2018 at 07:48:28PM +0100, Christoph Hellwig wrote: > fsync is required for data integrity as there is no gurantee that > data makes it to disk at any specified time without it. Even for > ext3 with data=ordered mode the file system will only commit all > data at some point in

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

2018-01-15 Thread Jeff King
On Tue, Jan 16, 2018 at 12:37:51AM +0100, Kim Gybels wrote: > > This looks good to me, and since it's a recent-ish regression, I think > > we should take the minimal fix here. > > The minimal fix being a simple NULL check before munmap()? Sorry to be unclear. I just meant that your patch is

Re: [PATCH v2 01/18] cat-file: split expand_atom into 2 functions

2018-01-15 Thread Jeff King
On Mon, Jan 15, 2018 at 04:33:35PM -0500, Jeff King wrote: > That works, but I don't think it's where we want to end up in the long > run. Because: > > 1. We still have the set of formats duplicated between expand_atom() > and the "preparation" step. It's just th

Re: [PATCH v2 04/18] cat-file: move struct expand_data into ref-filter

2018-01-15 Thread Jeff King
On Wed, Jan 10, 2018 at 09:36:41AM +, Olga Telezhnaya wrote: > Need that for further reusing of formatting logic in cat-file. > Have plans to get rid of using expand_data in cat-file at all, > and use it only in ref-filter for collecting, formatting and printing > needed data. I think some

Re: [PATCH v2 03/18] ref-filter: make valid_atom as function parameter

2018-01-15 Thread Jeff King
On Wed, Jan 10, 2018 at 09:36:41AM +, Olga Telezhnaya wrote: > Make valid_atom as a function parameter, > there could be another variable further. > Need that for further reusing of formatting logic in cat-file.c. > > We do not need to allow users to pass their own valid_atom variable in >

Re: [PATCH v2 02/18] cat-file: reuse struct ref_format

2018-01-15 Thread Jeff King
On Wed, Jan 10, 2018 at 09:36:41AM +, Olga Telezhnaya wrote: > Start using ref_format struct instead of simple char*. > Need that for further reusing of formatting logic from ref-filter. OK, this makes sense (though again, at some point we want this to stop being a "ref_format" and just be a

Re: [PATCH v2 01/18] cat-file: split expand_atom into 2 functions

2018-01-15 Thread Jeff King
On Wed, Jan 10, 2018 at 09:36:41AM +, Olga Telezhnaya wrote: > Split expand_atom function into 2 different functions, > expand_atom_into_fields prepares variable for further filling, > (new) expand_atom creates resulting string. > Need that for further reusing of formatting logic from

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

2018-01-15 Thread Jeff King
On Sat, Jan 13, 2018 at 05:11:49PM +0100, Kim Gybels wrote: > Take a hint from commit ea68b0ce9f8ce8da3e360aed3cbd6720159ffbee and use > read() instead of mmap() for small packed-refs files. > > This also fixes the problem[1] where xmmap() returns NULL for zero > length[2], for which munmap()

Re: [PATCH] travis-ci: build Git during the 'script' phase

2018-01-14 Thread Jeff King
On Sun, Jan 14, 2018 at 11:43:05AM +0100, SZEDER Gábor wrote: > On Sat, Jan 13, 2018 at 11:54 AM, Jeff King <p...@peff.net> wrote: > > I think there's also a similar feature to include timings for each fold, > > which might be worth pursuing. > > If you look for '

Re: [PATCH] travis-ci: build Git during the 'script' phase

2018-01-14 Thread Jeff King
On Sun, Jan 14, 2018 at 11:37:07AM +0100, SZEDER Gábor wrote: > > +fold_cmd () { > > + local name=$1; shift > > + fold "$name" > > + "$@" > > + local ret=$? > > + unfold "$name" > > + return $ret > > +} > > We don't have to fiddle with the return value,

Re: [PATCH] travis-ci: build Git during the 'script' phase

2018-01-13 Thread Jeff King
On Sat, Jan 13, 2018 at 05:32:56AM -0500, Jeff King wrote: > According to: > > https://blog.travis-ci.com/2013-05-22-improving-build-visibility-log-folds > > they auto-fold individual commands _except_ the ones in the script > section. Is there a way to manually mark f

Re: [PATCH] travis-ci: build Git during the 'script' phase

2018-01-13 Thread Jeff King
On Fri, Jan 12, 2018 at 02:32:54PM +0100, SZEDER Gábor wrote: > That's the just beginning of a looong list of executed test scripts in > seemingly pseudo-random order. IMHO that's very rarely the interesting > part; I, for one, am only interested in that list in exceptional cases, > e.g. while

Re: git gc --auto yelling at users where a repo legitimately has >6700 loose objects

2018-01-13 Thread Jeff King
On Fri, Jan 12, 2018 at 09:23:05PM +0700, Duy Nguyen wrote: > > > Why can't we generate a new cruft-pack on every gc run that > > > detects too many unreachable objects? That would not be as > > > efficient as a single cruft-pack but it should be way more > > > efficient than the individual

Re: [PATCH v4 0/7] Trace env variables in run_command()

2018-01-12 Thread Jeff King
On Sat, Jan 13, 2018 at 01:49:42PM +0700, Nguyễn Thái Ngọc Duy wrote: > v4: > > - incorporates Jeff patches and moves them on top > - removes strbuf release from print_trace_line > - prints 'unset a b c' instead of 'unset a; unset b; unset c' > - squashes v3 3/4 and 4/4 and Junio's patch into

Re: [PATCH v4 6/7] trace.c: print env vars in trace_run_command()

2018-01-12 Thread Jeff King
On Sat, Jan 13, 2018 at 01:49:48PM +0700, Nguyễn Thái Ngọc Duy wrote: > diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c > index d24d157379..0ab71f14fb 100644 > --- a/t/helper/test-run-command.c > +++ b/t/helper/test-run-command.c > @@ -56,6 +56,10 @@ int cmd_main(int argc,

Re: [PATCH v4 3/7] trace.c: move strbuf_release() out of print_trace_line()

2018-01-12 Thread Jeff King
On Sat, Jan 13, 2018 at 01:49:45PM +0700, Nguyễn Thái Ngọc Duy wrote: > The function is about printing a trace line, not releasing the buffer it > receives too. Move strbuf_release() back outside. This makes it easier > to see how strbuf is managed. This is kind of a funny refactor, in that it

Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Jeff King
On Fri, Jan 12, 2018 at 11:19:44AM -0800, Junio C Hamano wrote: > Jeff King <p...@peff.net> writes: > > > The actual prep_childenv() code looks like it would always do > > last-one-wins, so we should treat FOO as unset in that final case. But > > tha

Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Jeff King
On Fri, Jan 12, 2018 at 10:24:28AM -0800, Junio C Hamano wrote: > Jeff King <p...@peff.net> writes: > > >> + /* > >> + * Do we have a sequence of "unset GIT_DIR; GIT_DIR=foo"? > >> + * Then don't bother with the unset

Re: [PATCH v2 5/9] bisect: avoid using the rev_info flag leak_pending

2018-01-12 Thread Jeff King
On Thu, Jan 11, 2018 at 07:57:51PM +0100, René Scharfe wrote: > > If we already have the list of tips, could we just feed it ourselves to > > bisect_rev_setup (I think that would require us remembering which were > > "good" and "bad", but that doesn't seem like a big deal). > > That's done

Re: [PATCH v2 4/9] object: add clear_commit_marks_all()

2018-01-12 Thread Jeff King
On Thu, Jan 11, 2018 at 07:57:42PM +0100, René Scharfe wrote: > > Is it worth having: > > > >void clear_object_flags_from_type(int type, unsigned flags); > > > > rather than having two near-identical functions? I guess we'd need some > > way of saying "all types" to reimplement

Re: [BUG] Weird breakages in t1450 #2 on NonStop

2018-01-12 Thread Jeff King
On Thu, Jan 11, 2018 at 04:39:04PM -0500, Randall S. Becker wrote: > > executed because the test_commit fails with a non-zero git commit > > completion code. There is no rn (actual r n 252 252 252 252) in > > the objects directory - even the 'rn' does not correspond to > > anything.. I am

Re: git gc --auto yelling at users where a repo legitimately has >6700 loose objects

2018-01-12 Thread Jeff King
On Thu, Jan 11, 2018 at 10:33:15PM +0100, Ævar Arnfjörð Bjarmason wrote: > 4. At the end of all this, we check *again* if we have >6700 objects, > if we do we print "run 'git prune'" to .git/gc.log, and will just > emit that error for the next day before trying again, at which point >

[PATCH 6/4] trace: avoid unnecessary quoting

2018-01-12 Thread Jeff King
-and-pasted to a shell, and we'll keep that property - it covers any cases which would make the output visually ambiguous (e.g., embedded whitespace or quotes) Signed-off-by: Jeff King <p...@peff.net> --- quote.c | 26 ++ quote.h | 8 trace.

[PATCH 5/4] sq_quote_argv: drop maxlen parameter

2018-01-12 Thread Jeff King
No caller passes anything but "0" for this parameter, which requests that the function ignore it completely. In fact, in all of history there was only one such caller, and it went away in 7f51f8bc2b (alias: use run_command api to execute aliases, 2011-01-07). Signed-off-by: Je

Re: [PATCH v3 0/4] run-command.c: print env vars when GIT_TRACE is set

2018-01-12 Thread Jeff King
On Fri, Jan 12, 2018 at 04:56:03PM +0700, Nguyễn Thái Ngọc Duy wrote: > v3 turns a single patch into a series. Changes from v2 > > - env var quoting is now done correctly (from shell syntax perspective) > - the program name is prepended in git_cmd mode > - cwd is now printed too (because I have

Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Jeff King
On Fri, Jan 12, 2018 at 04:56:07PM +0700, Nguyễn Thái Ngọc Duy wrote: > The previous concatenate_env() is kinda dumb. It receives a env delta > in child_process and blindly follows it. If the run_command() user > "adds" more vars of the same value, or deletes vars that do not exist > in parent

Re: [PATCH v3 3/4] trace.c: print env vars in trace_run_command()

2018-01-12 Thread Jeff King
On Fri, Jan 12, 2018 at 04:56:06PM +0700, Nguyễn Thái Ngọc Duy wrote: > Occasionally submodule code could execute new commands with GIT_DIR set > to some submodule. GIT_TRACE prints just the command line which makes it > hard to tell that it's not really executed on this repository. > > Print

Re: [PATCH v3 1/4] trace.c: introduce trace_run_command()

2018-01-12 Thread Jeff King
On Fri, Jan 12, 2018 at 08:05:22AM -0500, Jeff King wrote: > > +void trace_run_command(const struct child_process *cp) > > +{ > > + struct strbuf buf = STRBUF_INIT; > > + > > + if (!prepare_trace_line(__FILE__, __LINE__, > > + _de

Re: [PATCH v3 2/4] trace.c: print program 'git' when child_process.git_cmd is set

2018-01-12 Thread Jeff King
On Fri, Jan 12, 2018 at 04:56:05PM +0700, Nguyễn Thái Ngọc Duy wrote: > We normally print full command line, including the program and its > argument. When git_cmd is set, we have a special code path to run the > right "git" program and child_process.argv[0] will not contain the > program name

Re: [PATCH v3 1/4] trace.c: introduce trace_run_command()

2018-01-12 Thread Jeff King
On Fri, Jan 12, 2018 at 04:56:04PM +0700, Nguyễn Thái Ngọc Duy wrote: > This is the same as the old code that uses trace_argv_printf() in > run-command.c. This function will be improved in later patches to > print more information from struct child_process. > > A slight regression: the old code

Re: [PATCH] doc/read-tree: remove obsolete remark

2018-01-11 Thread Jeff King
On Thu, Jan 11, 2018 at 03:14:07PM -0800, Junio C Hamano wrote: > >> Doesn't "git read-tree --prefix=previous HEAD^" add paths like > >> "previous/Documentation/Makefile" to the index, i.e. instead of > >> forcing you to have the required slash at the end, we give one for > >> free when it is

Re: [PATCH] doc/read-tree: remove obsolete remark

2018-01-11 Thread Jeff King
On Thu, Jan 11, 2018 at 11:02:04AM -0800, Junio C Hamano wrote: > >> ---prefix=/:: > >> +--prefix=:: > >>Keep the current index contents, and read the contents > >>of the named tree-ish under the directory at ``. > >>The command will refuse to overwrite entries that already > >> -

Re: read test snippet from stdin [was: [PATCH] t3900: add some more quotes]

2018-01-11 Thread Jeff King
On Thu, Jan 11, 2018 at 12:39:28PM +0100, SZEDER Gábor wrote: > > diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh > > index 9e4e694d93..09ad4d8878 100755 > > --- a/t/t3900-i18n-commit.sh > > +++ b/t/t3900-i18n-commit.sh > > @@ -39,14 +39,14 @@ test_expect_success 'UTF-16 refused

Re: [PATCH] doc/read-tree: remove obsolete remark

2018-01-11 Thread Jeff King
On Tue, Jan 09, 2018 at 04:30:34PM +0100, Andreas G. Schacker wrote: > Earlier versions of `git read-tree` required the `--prefix` option value > to end with a slash. This restriction was eventually lifted without a > corresponding amendment to the documentation. Makes sense. > ---prefix=/:: >

Re: [PATCH] run-command.c: print env vars when GIT_TRACE is set

2018-01-11 Thread Jeff King
On Wed, Jan 10, 2018 at 05:48:35PM +0700, Nguyễn Thái Ngọc Duy wrote: > Occasionally submodule code could execute new commands with GIT_DIR set > to some submodule. GIT_TRACE prints just the command line which makes it > hard to tell that it's not really executed on this repository. > > Print

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

2018-01-11 Thread Jeff King
On Wed, Jan 10, 2018 at 12:22:10PM -0800, Junio C Hamano wrote: > Jeff King <p...@peff.net> writes: > > > To be clear, which approach are we talking about? I think there are > > three options: > > > > 1. The user tells us not to bother computing real ahead/b

Re: [PATCH] t3900: add some more quotes

2018-01-11 Thread Jeff King
On Wed, Jan 10, 2018 at 01:31:22PM -0800, Junio C Hamano wrote: > > +# Read from stdin into the variable given in $1. > > +test_read_to_eof () { > > + # Bash's "read" is sufficiently flexible that we can skip the extra > > + # process. > > + if test -n "$BASH_VERSION" > > + then > > +

Re: [PATCH] Replaced read with xread in transport-helper.c to fix SSIZE_MAX overun in t5509

2018-01-10 Thread Jeff King
-- Subject: [PATCH] transport-helper: drop read/write errno checks Since we use xread() and xwrite() here, EINTR, EAGAIN, and EWOULDBLOCK retries are already handled for us, and we will never see these errno values ourselves. We can drop these conditions entirely, making the code easier to follow.

Re: [PATCH] Replaced read with xread in transport-helper.c to fix SSIZE_MAX overun in t5509

2018-01-10 Thread Jeff King
On Thu, Jan 11, 2018 at 12:40:05AM -0500, Randall S. Becker wrote: > This fix was needed on HPE NonStop NSE where SSIZE_MAX is less than > BUFFERSIZE resulting in EINVAL. The call to read in transport-helper.c > was the only place outside of wrapper.c. For my own curiosity, what is SSIZE_MAX on

Re: [PATCH] t3900: add some more quotes

2018-01-10 Thread Jeff King
On Wed, Jan 10, 2018 at 08:02:09PM +0100, Johannes Sixt wrote: > > diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh > > index 9e4e694d9..dc00db87b 100755 > > --- a/t/t3900-i18n-commit.sh > > +++ b/t/t3900-i18n-commit.sh > > @@ -40,7 +40,7 @@ test_expect_success 'UTF-16 refused because

Re: [RFC PATCH 00/18] Multi-pack index (MIDX)

2018-01-10 Thread Jeff King
On Tue, Jan 09, 2018 at 02:05:09PM +0100, Johannes Schindelin wrote: > > I think that could be easily worked around for rebase by asking git to > > check ambiguity during the conversion. > > Sure. > > It also points to a flaw in your reasoning, and you should take my example > further:

Re: [PATCH] t3900: add some more quotes

2018-01-10 Thread Jeff King
On Wed, Jan 10, 2018 at 10:58:32AM +0100, Beat Bolli wrote: > In 89a70b80 ("t0302 & t3900: add forgotten quotes", 2018-01-03), quotes > were added to protect against spaces in $HOME. In the test_when_finished > hander, two files are deleted which must be quoted individually. Doh. I can't believe

Re: [PATCH v2 0/9] revision: get rid of the flag leak_pending

2018-01-10 Thread Jeff King
On Mon, Dec 25, 2017 at 06:41:18PM +0100, René Scharfe wrote: > The flag leak_pending is weird, let's get rid of it. > > Changes from v1: Everything. I left a few comments, but didn't see any show-stoppers (which is good, because this is already in 'next' ;) ). Any I didn't comment on looked

Re: [PATCH v2 6/9] bundle: avoid using the rev_info flag leak_pending

2018-01-10 Thread Jeff King
On Mon, Dec 25, 2017 at 06:46:14PM +0100, René Scharfe wrote: > The leak_pending flag is so awkward to use that multiple comments had to > be added around each occurrence. We use it for remembering the > prerequisites for the bundle. That is easy, though: We have the > ref_list named

Re: [PATCH v2 5/9] bisect: avoid using the rev_info flag leak_pending

2018-01-10 Thread Jeff King
On Mon, Dec 25, 2017 at 06:45:36PM +0100, René Scharfe wrote: > The leak_pending flag is so awkward to use that multiple comments had to > be added around each occurrence. We only use it for remembering the > commits whose marks we have to clear after checking if all of the good > ones are

Re: [PATCH v2 4/9] object: add clear_commit_marks_all()

2018-01-09 Thread Jeff King
On Mon, Dec 25, 2017 at 06:44:58PM +0100, René Scharfe wrote: > Add a function for clearing the commit marks of all in-core commit > objects. It's similar to clear_object_flags(), but more precise, since > it leaves the other object types alone. It still has to iterate through > them, though.

Re: [PATCH v2 1/9] commit: avoid allocation in clear_commit_marks_many()

2018-01-09 Thread Jeff King
On Mon, Dec 25, 2017 at 06:43:37PM +0100, René Scharfe wrote: > Pass the entries of the commit array directly to clear_commit_marks_1() > instead of adding them to a commit_list first. The function clears the > commit and any first parent without allocation; only higher numbered > parents are

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

2018-01-09 Thread Jeff King
On Tue, Jan 09, 2018 at 09:29:31AM -0500, Derrick Stolee wrote: > > > But even still, finding small answers quickly and accurately and punting > > > to "really far, I didn't bother to compute it" on the big ones would be > > > an improvement over always punting. > > Indeed. The longer I think

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

2018-01-09 Thread Jeff King
On Tue, Jan 09, 2018 at 02:15:47PM +0100, Johannes Schindelin wrote: > > I like this direction a lot. I had hoped we could say "100+ commits > > ahead", > > How about "100+ commits apart" instead? Yeah, that is probably more accurate for the general case. > > but I don't think we can do so

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

2018-01-08 Thread Jeff King
On Mon, Jan 08, 2018 at 03:04:20PM -0500, Jeff Hostetler wrote: > > I was thinking about something similar to the logic we use today > > about whether to start reporting progress on other long commands. > > That would mean you could still get the ahead/behind values if you > > aren't that far

Re: [RFC PATCH 00/18] Multi-pack index (MIDX)

2018-01-08 Thread Jeff King
On Mon, Jan 08, 2018 at 08:43:44AM -0500, Derrick Stolee wrote: > > Just to make sure I'm parsing this correctly: normal lookups do get faster > > when you have a single index, given the right setup? > > > > I'm curious what that setup looked like. Is it just tons and tons of > > packs? Is it

Re: [RFC PATCH 00/18] Multi-pack index (MIDX)

2018-01-08 Thread Jeff King
On Mon, Jan 08, 2018 at 02:43:00PM +0100, Johannes Schindelin wrote: > Take the interactive rebase for example. It generates todo lists with > abbreviated commit names, for readability (and it is *really* important to > keep this readable). As we expect new objects to be introduced by the >

Re: [RFC PATCH 00/18] Multi-pack index (MIDX)

2018-01-08 Thread Jeff King
On Mon, Jan 08, 2018 at 05:20:29AM -0500, Jeff King wrote: > I.e., what if we did something like this: > > diff --git a/sha1_name.c b/sha1_name.c > index 611c7d24dd..04c661ba85 100644 > --- a/sha1_name.c > +++ b/sha1_name.c > @@ -600,6 +600,15 @@ int find_unique_abbr

Re: [RFC PATCH 00/18] Multi-pack index (MIDX)

2018-01-08 Thread Jeff King
On Sun, Jan 07, 2018 at 07:08:54PM -0500, Derrick Stolee wrote: > > (Not a critique of this, just a (stupid) question) > > > > What's the practical use-case for this feature? Since it doesn't help > > with --abbrev=40 the speedup is all in the part that ensures we don't > > show an ambiguous

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

2018-01-07 Thread Jeff King
On Fri, Jan 05, 2018 at 11:46:24AM -0500, Jeff Hostetler wrote: > > I'm mildly negative on this "level 2" config. If influencing the > > porcelain via config creates compatibility headaches, then why would we > > allow it here? And if it doesn't, then why do we need to protect against > > it?

Re: Bug report: git clone with dest

2018-01-05 Thread Jeff King
On Fri, Jan 05, 2018 at 02:37:25PM -0800, Junio C Hamano wrote: > Well, MaintNotes on the 'todo' branch needs a bit of updating, as it > says something somewhat misleading. > > -- >8 -- > Subject: MaintNotes: clarify the purpose of maint->master upmerge Yup, this makes sense. Thanks for

Re: Bug report: git clone with dest

2018-01-05 Thread Jeff King
On Fri, Jan 05, 2018 at 12:45:03PM -0800, Junio C Hamano wrote: > Jeff King <p...@peff.net> writes: > > > Out of curiosity, did this change at some point? I thought the process > > used to be to merge to maint, and then pick up topics in master by > > merging mai

Re: Bug report: git clone with dest

2018-01-05 Thread Jeff King
On Fri, Jan 05, 2018 at 09:22:07PM +0100, Johannes Schindelin wrote: > On Fri, 5 Jan 2018, Isaac Shabtay wrote: > > > Done: https://github.com/git-for-windows/git/pull/1421 > > > > I added credit to Jeff in the PR's description. > > Sadly, the PR's description won't make it into the commit

Re: Bug report: git clone with dest

2018-01-05 Thread Jeff King
On Fri, Jan 05, 2018 at 11:53:50AM -0800, Junio C Hamano wrote: > > They haven't even been reviewed yet. If they get good feedback, then the > > maintainer will pick them up, then merge them to 'next', and then > > eventually to 'master', after which they'd become part of the next > > major

Re: Recommendation for consistent update on invoke of "sha1_to_hex()"

2018-01-04 Thread Jeff King
On Fri, Jan 05, 2018 at 11:26:01AM +0800, 牛旭 wrote: > By mining historical patches, we suggest that invokes of sha1_to_hex() > should be replaced with that of oid_to_hex(). One example for > recommendation and corresponding patch are listed as follows.  Note that these two functions take

Re: Patch recommendation for replace invoke of error() to that of error_errno()

2018-01-04 Thread Jeff King
On Fri, Jan 05, 2018 at 11:24:02AM +0800, 牛旭 wrote: > Our team researches on consistent update of Git during evolution. And > we have figured out several spots that may be missed.  > > > By mining historical patches, we suggest that invokes of error(..., > strerror(errno)) should be replaced

Re: [PATCH 3/4] clone: factor out dir_exists() helper

2018-01-04 Thread Jeff King
On Thu, Jan 04, 2018 at 06:54:12PM -0500, Jeff King wrote: > > If we really want to be anal, perhaps a new helper path_exists() > > that cares only about existence of paths (i.e. the implementation of > > these two helpers they currently have), together with update to > &

Re: [PATCH 3/4] clone: factor out dir_exists() helper

2018-01-04 Thread Jeff King
On Thu, Jan 04, 2018 at 03:47:18PM -0800, Junio C Hamano wrote: > Jeff King <p...@peff.net> writes: > > > Two parts of git-clone's setup logic check whether a > > directory exists, and they both call stat directly with the > > same scratch "struct stat" b

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

2018-01-04 Thread Jeff King
On Wed, Jan 03, 2018 at 09:47:28PM +, Jeff Hostetler wrote: > Config values of true and false control non-porcelain formats > for compatibility reasons as previously discussed. In the > last commit I added a new value of 2 for the config setting > to allow porcelain formats to inherit the

Re: [PATCH v2 1/5] core.aheadbehind: add new config setting

2018-01-04 Thread Jeff King
On Wed, Dec 27, 2017 at 09:41:30AM -0800, Junio C Hamano wrote: > Jeff King <p...@peff.net> writes: > > > I, too, had a funny feeling about calling this "core". But I didn't have > > a better name, as I'm not sure what other place we have for config > > op

Re: Misleading documentation for git-diff-files (diff-filter)

2018-01-04 Thread Jeff King
w up. - The direction of the diff matters. Doing "diff-files -R" can get you Added entries (but not Deleted ones). So it's best just to explain that the set of available types depends on the specific diff invocation. Reported-by: John Cheng <johnlich...@gmail.com> Signed-off-by: Jeff

Re: [RFC/PATCH] connect: add GIT_SSH_{SEND,RECEIVE}{,_COMMAND} env variables

2018-01-04 Thread Jeff King
On Thu, Jan 04, 2018 at 11:10:17AM +0100, Ævar Arnfjörð Bjarmason wrote: > That's badly explained, sorry, when I say "push" I mean "push and/or > pull". > > I don't know about Github, but on Gitlab when you provision a deploy key > and associate it with a repo it must be *globally* rw or ro,

Re: [RFC/PATCH] connect: add GIT_SSH_{SEND,RECEIVE}{,_COMMAND} env variables

2018-01-03 Thread Jeff King
On Thu, Jan 04, 2018 at 01:08:28AM +0100, Ævar Arnfjörð Bjarmason wrote: > Hopefully this is clearer, and depending on how the rest of the > discussion goes I'll submit v2 with something like this in the commit > message: > > SSH keys A and B are known to the remote service, and used to identify

Re: Bug report: git clone with dest

2018-01-03 Thread Jeff King
On Wed, Jan 03, 2018 at 02:42:51PM -0800, Isaac Shabtay wrote: > Indeed interesting... this one's for the books... > Thanks for the patches. Any idea when these are going to make it to the > official Git client builds? (specifically the Windows one) They haven't even been reviewed yet. If they

Re: Bug report: git clone with dest

2018-01-03 Thread Jeff King
On Wed, Jan 03, 2018 at 12:59:48PM -0800, Isaac Shabtay wrote: > Target directory is deleted on clone failures. > > Steps to reproduce, for example on Windows: > > cd /d %TEMP% > mkdir dest > git clone https://some-fake-url/whatever-makes-git-clone-fail dest > > Of course, the clone will fail

Re: [PATCH 0/2] Several fixes for the test suite related to spaces in filenames

2018-01-03 Thread Jeff King
On Wed, Jan 03, 2018 at 05:54:44PM +0100, Johannes Schindelin wrote: > The second issue was found long ago, and the patch carried in Git for > Windows, although nothing about it is specific to Windows. > > The first patch was developed today, when I tried to verify that Git's > test suite passes

Re: [PATCH v3] run-command: add hint when a hook is ignored

2018-01-03 Thread Jeff King
On Wed, Oct 11, 2017 at 03:26:43PM +0900, Junio C Hamano wrote: > Junio C Hamano writes: > > > Quite honestly, I do not particulary think this is confusing, and I > > expect that this change will irritate many people by forcing them to > > either set the advise config or move

Re: [PATCH] http: fix v1 protocol tests with apache httpd < 2.4

2018-01-02 Thread Jeff King
On Tue, Jan 02, 2018 at 07:39:46PM -0500, Todd Zullinger wrote: > Brandon Williams wrote: > > Seems good to me. I think I was just being overly cautious when i was > > implementing those patches. Thanks! > > Cool, thanks for taking a look. > > In this case, the test isn't dependent on apache

Re: [PATCH] http: fix v1 protocol tests with apache httpd < 2.4

2018-01-02 Thread Jeff King
On Sat, Dec 30, 2017 at 09:32:34PM -0500, Todd Zullinger wrote: > The apache config used by tests was updated to use the SetEnvIf > directive to set the Git-Protocol header in 19113a26b6 ("http: tell > server that the client understands v1", 2017-10-16). > > Setting the Git-Protocol header is

Re: [PATCH 4/4] clone: do not clean up directories we didn't create

2018-01-02 Thread Jeff King
On Tue, Jan 02, 2018 at 05:49:45PM -0500, Eric Sunshine wrote: > > diff --git a/t/t5600-clone-fail-cleanup.sh b/t/t5600-clone-fail-cleanup.sh > > @@ -42,13 +53,48 @@ test_expect_success 'successful clone must leave the > > directory' ' > > +test_expect_success 'failed clone into empty leaves

[PATCH 4/4] clone: do not clean up directories we didn't create

2018-01-02 Thread Jeff King
-tree and git-dir separately, though, as only one might exist (and the new tests in t5600 cover all cases). Reported-by: Stephan Janssen <sjans...@you-get.com> Signed-off-by: Jeff King <p...@peff.net> --- builtin/clone.c | 20 t/t5600-clone-fail-cle

[PATCH 3/4] clone: factor out dir_exists() helper

2018-01-02 Thread Jeff King
cifically care about directories, so this future-proofs us against that function later getting more picky about seeing actual files. Signed-off-by: Jeff King <p...@peff.net> --- builtin/clone.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/builtin/clone

[PATCH 2/4] t5600: modernize style

2018-01-02 Thread Jeff King
it -C" instead of manually entering sub-repo - use test_when_finished for cleanup steps - test_path_is_* as appropriate Signed-off-by: Jeff King <p...@peff.net> --- t/t5600-clone-fail-cleanup.sh | 48 ++- 1 file changed, 25 insertions(+), 23 de

[PATCH 1/4] t5600: fix outdated comment about unborn HEAD

2018-01-02 Thread Jeff King
corrupts the repository by temporarily removing its objects, which means we need to have some objects to move. Signed-off-by: Jeff King <p...@peff.net> --- t/t5600-clone-fail-cleanup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5600-clone-fail-cleanup.sh b/t

[PATCH 0/4] Git removes existing folder when cancelling clone

2018-01-02 Thread Jeff King
On Tue, Jan 02, 2018 at 03:04:43PM -0500, Jeff King wrote: > So I don't think there's an urgent data-loss bug here; we will only ever > destroy an empty directory. However, the original intent was to leave > the filesystem as we found it on a failed or aborted clone, and we > obviou

Re: Git removes existing folder when cancelling clone

2018-01-02 Thread Jeff King
On Tue, Jan 02, 2018 at 06:12:35AM -0500, Robert P. J. Day wrote: > > I just noticed the following behaviour on macOS 10.13.2 running Git v2.15.0: > > > > 1. `mkdir new-folder` > > 2. `ls` - shows new-folder > > 3. `git clone [repo] new-folder` > > 4. Git asks for password > > 5. I cancel by

Re: [PATCH] diff-tree: obey the color.ui configuration

2017-12-30 Thread Jeff King
On Sat, Dec 30, 2017 at 04:04:50PM +0100, Ævar Arnfjörð Bjarmason wrote: > > I do like the idea of using "show", though. We know the point is to show > > the output to the user, so we don't mind at all if the behavior or > > output of show changes in future versions (unless we consider the final

Re: possible completion bug with --set-upstream-to=

2017-12-30 Thread Jeff King
On Sat, Dec 30, 2017 at 09:28:06AM +0100, SZEDER Gábor wrote: > I couldn't reproduce the wrong behavior you saw using v2.1.4 in a > regular setup. > > However, I could reproduce it after I removed the '=' character from > the set of characters in $COMP_WORDBREAKS, but then all completions >

Re: [PATCH] diff-tree: obey the color.ui configuration

2017-12-30 Thread Jeff King
On Sat, Dec 30, 2017 at 01:33:06PM +0100, Ævar Arnfjörð Bjarmason wrote: > > - we ended up with 33c643bb08 (Revert "color: check color.ui in > > git_default_config()", 2017-10-13), which just reverts the whole > > mess back to the pre-v2.14 state. This shipped in v2.15. > > Thanks.

Re: [PATCH] diff-tree: obey the color.ui configuration

2017-12-29 Thread Jeff King
On Fri, Dec 29, 2017 at 06:16:31PM -0500, Todd Zullinger wrote: > Ævar Arnfjörð Bjarmason wrote: > > No idea how to test this, in particular trying to pipe the output of > > color.ui=never v.s. color.ui=auto to a file as "auto" will disable > > coloring when it detects a pipe, but this fixes the

Re: Rewrite cat-file.c : need help to find a bug

2017-12-29 Thread Jeff King
On Fri, Dec 29, 2017 at 01:05:50PM +0300, Оля Тележная wrote: > Hi everyone, > I am trying to reuse formatting logic from ref-filter in cat-file > command. Now cat-file uses its own formatting code. > I am trying to achieve that step-by-step, now I want to invoke > populate_value function, and I

Re: Improved error handling (Was: [PATCH 1/2] sequencer: factor out rewrite_file())

2017-12-24 Thread Jeff King
On Sat, Nov 18, 2017 at 10:01:45AM +0100, Johannes Sixt wrote: > > Yeah, I have mixed feelings on that. I think it does make the control > > flow less clear. At the same time, what I found was that handlers like > > die/ignore/warn were the thing that gave the most reduction in > > complexity in

Re: [RFC/PATCH] perl: bump the required Perl version to 5.10.0 from 5.8.0

2017-12-24 Thread Jeff King
On Sat, Dec 23, 2017 at 05:44:00PM +, Ævar Arnfjörð Bjarmason wrote: > This is similar to Jeff King's jk/drop-ancient-curl series in that > we're dropping perl releases that are rarely tested anymore, however > unlike those patches git still works on e.g. 5.8.8 (I couldn't build > anything

Re: [PATCH v2 1/5] core.aheadbehind: add new config setting

2017-12-24 Thread Jeff King
On Fri, Dec 22, 2017 at 10:21:23AM -0800, Junio C Hamano wrote: > >> +core.aheadbehind:: > >> + If true, tells commands like status and branch to print ahead and > >> + behind counts for the branch relative to its upstream branch. > >> + This computation may be very expensive when there is a

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-24 Thread Jeff King
On Thu, Dec 21, 2017 at 07:41:44PM +0100, René Scharfe wrote: > Am 20.12.2017 um 14:08 schrieb Jeff King: > > On Tue, Dec 19, 2017 at 10:33:55AM -0800, Junio C Hamano wrote: > > > >> Should we take the patch posted as-is and move forward? > > > > I suppo

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

2017-12-21 Thread Jeff King
On Thu, Dec 21, 2017 at 09:18:17AM -0500, Jeff Hostetler wrote: > > This patch will affect "git status --porcelain", too. That's not > > supposed to change in incompatible ways. I guess it's up for debate > > whether callers are meant to handle any arbitrary string inside the [] > > (we already

  1   2   3   4   5   6   7   8   9   10   >