Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config

2017-05-12 Thread Jonathan Nieder
Johannes Schindelin wrote: > On Windows, `(1|2)` is not a valid file name, and therefore the tag > cannot be created as expected by the new test. > > So simply skip this test on Windows. > > Signed-off-by: Johannes Schindelin > --- Reviewed-by: Jonathan Nieder I wouldn&#x

Re: [PATCH v2] builtin/log: honor log.decorate

2017-05-12 Thread Jonathan Nieder
brian m. carlson wrote: > The recent change that introduced autodecorating of refs accidentally > broke the ability of users to set log.decorate = false to override it. Yikes. It sounds to me like we need a test to ensure we don't regress it again later. > When the git_log_config was traversed

Re: [PATCH v6] fetch-pack: always allow fetching of literal SHA1s

2017-05-12 Thread Jonathan Nieder
Jonathan Tan wrote: > Helped-by: Jeff King > Signed-off-by: Jonathan Tan > --- > fetch-pack.c | 35 +-- > t/t5500-fetch-pack.sh | 35 +++ > 2 files changed, 68 insertions(+), 2 deletions(-) Reviewed-b

Re: [PATCH v5] fetch-pack: always allow fetching of literal SHA1s

2017-05-12 Thread Jonathan Nieder
ef(ref); > newtail = &(*newtail)->next; > > (making the function-to-abstract be merely an initialization one, > instead of one that does 2 things). That decreases the scope of the > function that Jonathan Nieder and Peff wanted, but it might be a > warranted reduction in s

Re: [PATCH 00/11] Start retiring .git/remotes/ and .git/branches/ for good

2017-05-12 Thread Jonathan Nieder
Johannes Schindelin wrote: > On Fri, 12 May 2017, Junio C Hamano wrote: >> And this one is also important. I do not think we had to touch any >> code that handles .git/remotes/ or .git/branches when we extended >> the .git/config based configuration for remotes, simply because the >> old data sou

Re: [PATCH v5] fetch-pack: always allow fetching of literal SHA1s

2017-05-11 Thread Jonathan Nieder
> +{ > + if (!tip_oids->map.cmpfn) { This feels like a layering violation. Could it be e.g. a static inline function oidset_is_initialized in oidset.h? > + add_refs_to_oidset(tip_oids, unmatched); > + add_refs_to_oidset(tip_oids, newlist); > + } > + return

Re: [PATCH] C style: use standard style for "TRANSLATORS" comments

2017-05-11 Thread Jonathan Nieder
Hi, Ævar Arnfjörð Bjarmason wrote: > Change all the "TRANSLATORS: [...]" comments in the C code to use the > regular Git coding style, and amend the style guide so that the > example there uses that style. Hooray! [...] > --- a/Documentation/CodingGuidelines > +++ b/Documentation/CodingGuidelin

Re: [PATCH v4] fetch-pack: always allow fetching of literal SHA1s

2017-05-11 Thread Jonathan Nieder
Hi, Jonathan Tan wrote: > Thanks, peff. I've incorporated your suggestions - I don't feel very > strongly about this, but I guess it's worthwhile to avoid the quadratic > behavior if we can. > > Also incorporated Jonathan Nieder's suggestion about the placement of > the last line. The relevant fu

Re: [PATCH v4 0/2] perf: show that wildmatch() regressed for pathological cases in v2.0

2017-05-11 Thread Jonathan Nieder
+ > t/perf/perf-lib.sh | 19 +++ > 3 files changed, 59 insertions(+), 4 deletions(-) > create mode 100755 t/perf/p0100-globbing.sh Reviewed-by: Jonathan Nieder Thanks.

Re: [PATCH v3 2/3] read-tree -m: make error message for merging 0 trees less smart aleck

2017-05-11 Thread Jonathan Nieder
Hi, Jean-Noel Avila wrote: > Signed-off-by: Jean-Noel Avila > Signed-off-by: Jonathan Nieder Please remove my sign-off. I didn't write or carry this patch. If you want to acknowledge my contribution, you can use something like Helped-by, but it's not necessary. [...] >

Re: [PATCH] fetch-pack: always allow fetching of literal SHA1s

2017-05-11 Thread Jonathan Nieder
Jeff King wrote: > On Wed, May 10, 2017 at 10:00:44AM -0700, Jonathan Nieder wrote: >> Jeff King wrote: >>> [1] The reachability checks from upload-pack don't actually do much on >>> GitHub, because you can generally access the objects via the AP

Re: [PATCH v2 2/2] perf: add test showing exponential growth in path globbing

2017-05-10 Thread Jonathan Nieder
Hi, Ævar Arnfjörð Bjarmason wrote: > Add a test showing that ls-files times grow exponentially in the face > of some pathological globs, whereas refglobs via for-each-ref don't in > practice suffer from the same issue. Cool. [...] > --- /dev/null > +++ b/t/perf/p0100-globbing.sh > @@ -0,0 +1,48

Re: [PATCH v2 1/2] perf: add function to setup a fresh test repo

2017-05-10 Thread Jonathan Nieder
Hi, Ævar Arnfjörð Bjarmason wrote: [...] > # call at least one of these to establish an appropriately-sized repository > +test_perf_fresh_repo () { > + repo="${1:-$TRASH_DIRECTORY}" > + "$MODERN_GIT" init -q "$repo" && > + cd "$repo" && > + test_perf_do_repo_symlink_config_ > +}

Re: [PATCH v3] fetch-pack: always allow fetching of literal SHA1s

2017-05-10 Thread Jonathan Nieder
d make it even clearer.) > --- a/t/t5500-fetch-pack.sh > +++ b/t/t5500-fetch-pack.sh > @@ -547,6 +547,41 @@ test_expect_success 'fetch-pack can fetch a raw sha1' ' Yay, thanks much for these. [...] > +test_expect_success 'fetch-pack can fetch a raw sha1 overlapping a named > ref' ' Ha, you read my mind. :) Except for the search-ref-list-for-oid function needing to be factored out, Reviewed-by: Jonathan Nieder Thanks.

Re: [PATCH] fetch-pack: always allow fetching of literal SHA1s

2017-05-10 Thread Jonathan Nieder
On Wed, May 10, 2017 at 12:48:37PM -0600, Martin Fick wrote: >> Ævar Arnfjörð Bjarmason wrote: >>> Just a side question, what are the people who use this >>> feature using it for? The only thing I can think of >>> myself is some out of band ref advertisement because >>> you've got squillions of re

Re: [PATCH] fetch-pack: always allow fetching of literal SHA1s

2017-05-10 Thread Jonathan Nieder
Hi, Ævar Arnfjörð Bjarmason wrote: > Just a side question, what are the people who use this feature using > it for? The only thing I can think of myself is some out of band ref > advertisement because you've got squillions of refs as a hack around > git's limitations in that area. That's one use

Re: [PATCH v2] fetch-pack: always allow fetching of literal SHA1s

2017-05-10 Thread Jonathan Nieder
Hi, Jonathan Tan wrote: > fetch-pack, when fetching a literal SHA-1 from a server that is not > configured with uploadpack.allowtipsha1inwant (or similar), always > returns an error message of the form "Server does not allow request for > unadvertised object %s". However, it is sometimes the case

Re: [PATCH] fetch-pack: always allow fetching of literal SHA1s

2017-05-10 Thread Jonathan Nieder
Hi, Jeff King wrote: > Right, makes sense. I wondered if GitHub should be turning on > allowTipSHA1InWant, but it really doesn't make sense to. We _do_ hide > some internal refs[1], and they're things that users wouldn't want to > fetch. The problem for your case really is just on the client sid

Re: [ANNOUNCE] Git v2.12.3 and others

2017-05-09 Thread Jonathan Nieder
Junio C Hamano wrote: > Maintenance releases Git v2.4.12, v2.5.6, v2.6.7, v2.7.5, v2.8.5, > v2.9.4, v2.10.3, v2.11.2, and v2.12.3 have been tagged and are now > available at the usual places. > > These are primarily to fix a recently disclosed problem with "git > shell", which may allow a user who

Re: [PATCH 3/3] protocol docs: explain receive-pack push options

2017-05-05 Thread Jonathan Nieder
Hi, Jonathan Tan wrote: > Support for push options in the receive-pack protocol (and all Git > components that speak it) have been added over a few commits, but not > fully documented (especially its interaction with signed pushes). Update > the protocol documentation to include the relevant deta

Re: [PATCH 2/3] receive-pack: verify push options in cert

2017-05-05 Thread Jonathan Nieder
Hi, Stefan Beller wrote: > On Fri, May 5, 2017 at 4:46 PM, Jonathan Tan wrote: >> This sets in stone the requirement that send-pack redundantly send its >> push options in 2 places, but I think that this is better than the >> alternatives. Sending push options only within the cert is >> backward

Re: [PATCH 2/3] receive-pack: verify push options in cert

2017-05-05 Thread Jonathan Nieder
Hi, Jonathan Tan wrote: > In commit f6a4e61 ("push: accept push options", 2016-07-14), send-pack > was taught to include push options both within the signed cert (if the > push is a signed push) and outside the signed cert; however, > receive-pack ignores push options within the cert, only handli

Re: [PATCH 1/3] docs: correct receive.advertisePushOptions default

2017-05-05 Thread Jonathan Nieder
;t want to advertise this > - capability, set this variable to false. > + When set to true, git-receive-pack will advertise the push options > + capability to its clients. Good catch. Reviewed-by: Jonathan Nieder Possible improvements: - Should this also say "The defaul

Re: [PATCH 0/3] Port git-add--interactive.perl:status_cmd to C

2017-05-05 Thread Jonathan Nieder
Ævar Arnfjörð Bjarmason wrote: > On Fri, May 5, 2017 at 8:43 PM, Daniel Ferreira wrote: >> This series introduces git-add-interactive--helper (or should it be >> called git-add--interactive--helper?) as a builtin capable of doing >> what the Perl script's status_cmd() would do. > > The existing s

Re: [PATCH v1 2/2] travis-ci: add job to run tests with GETTEXT_POISON

2017-05-05 Thread Jonathan Nieder
that translatable strings are not affecting other functionality of Git. It's a valuable thing to test continuously. For what it's worth, Reviewed-by: Jonathan Nieder Thanks.

Re: [PATCH 0/5] Start of a journey: drop NO_THE_INDEX_COMPATIBILITY_MACROS

2017-05-04 Thread Jonathan Nieder
Hi, Junio C Hamano wrote: > Duy Nguyen writes: >> I attempted the same thing once or twice in the past, had the same >> impression and dropped it. But I think it's good to get rid of cache_* >> macros, at least outside builtin/ directory. > > One thing that needs to be kept in mind is that a mec

Re: [PATCH 00/10] RFC Partial Clone and Fetch

2017-05-04 Thread Jonathan Nieder
ated pack file. Another thing we're looking into is incorporating something like Martin Fick's "git exproll" ( http://public-inbox.org/git/1375756727-1275-1-git-send-email-artag...@gmail.com/) into "git gc --auto" to more aggressively keep the number of packs down. >

Re: [PATCH 00/10] RFC Partial Clone and Fetch

2017-05-03 Thread Jonathan Nieder
Hi, Jonathan Tan wrote: > The binary search to lookup a packfile offset from a .idx file > (which involves disk reads) would take longer for all lookups (not > just lookups for missing blobs) - I think I prefer keeping the lists > separate, to avoid pessimizing the (likely) usual case where the >

Re: [PATCH] config.mak.uname: set NO_REGEX=NeedsStartEnd on AIX

2017-05-03 Thread Jonathan Nieder
Hi, Stefan Beller wrote: > On Wed, May 3, 2017 at 12:47 PM, Jonathan Nieder wrote: >> Is there e.g. a build farm where we can check for this kind of thing >> more systematically on supported platforms? > > There is the OpenSuse build farm that provides builds for

Re: [PATCH] config.mak.uname: set NO_REGEX=NeedsStartEnd on AIX

2017-05-03 Thread Jonathan Nieder
Bjarmason > --- > config.mak.uname | 1 + > 1 file changed, 1 insertion(+) Thanks. Reviewed-by: Jonathan Nieder Is there e.g. a build farm where we can check for this kind of thing more systematically on supported platforms?

Re: [PATCH 00/10] RFC Partial Clone and Fetch

2017-05-03 Thread Jonathan Nieder
Hi, Jeff Hostetler wrote: > On 3/8/2017 1:50 PM, g...@jeffhostetler.com wrote: >> [RFC] Partial Clone and Fetch >> = >> [...] >> E. Unresolved Thoughts >> == >> >> *TODO* The server should optionally return (in a side-band?) a list >> of the blobs t

Re: [PATCH 4/4] git-filter-branch: be assertative on dying message

2017-05-03 Thread Jonathan Nieder
Hi, Jean-Noel Avila wrote: > Signed-off-by: Jean-Noel Avila As with the previous patches, this is a good place to put the motivation for the patch. > --- > git-filter-branch.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/git-filter-branch.sh b/git-filter-branch.s

Re: [PATCH 3/4] read-tree.c: rework UI when merging no trees

2017-05-03 Thread Jonathan Nieder
Hi, Jean-Noel Avila wrote: > Subject: read-tree.c: rework UI when merging no trees nit: this is about user-facing behavior, not an implementation detail, so the part before the colon can be the command that changed (read-tree:). nit: the word "rework" is dangerous in a commit message in the sam

Re: [PATCH 2/4] usability: fix am and checkout for nevermind questions

2017-05-03 Thread Jonathan Nieder
Jean-Noel Avila wrote: > Subject: usability: fix am and checkout for nevermind questions > > Signed-off-by: Jean-Noel Avila Thanks for working on improving Git's UX. I agree with the goal in general (we should not gratuitously surprise users) but I think I lack context for appreciating this par

Re: [PATCH 1/4] usability: don't ask questions if no reply is required

2017-05-03 Thread Jonathan Nieder
Hi, Jean-Noel Avila wrote: > As described in the bug report at > > https://github.com/git/git-scm.com/issues/999 External issue tracker URLs have been known to change or disappear and we try to make commit messages self-contained instead of relying on them. It is common to put a 'Requested-by:'

Re: [PATCH 0/5] Abide by our own rules regarding line endings

2017-05-02 Thread Jonathan Nieder
es Schindelin (5): > Fix build with core.autocrlf=true > git-new-workdir: mark script as LF-only > completion: mark bash script as LF-only > Fix the remaining tests that failed with core.autocrlf=true > t4051: mark supporting files as requiring LF-only line endings With or wit

Re: [PATCH] credential doc: make multiple-helper behavior more prominent (Re: [PATCH] clone: handle empty config values in -c)

2017-05-01 Thread Jonathan Nieder
Starting out the reviews: Jonathan Nieder wrote: [...] > configuration item to empty before giving it a new value. This is > already documented by the documentation is hard to find --- ^^ s/by/but/ Sorry for the confusion. [...] > +++ b/Documentation/gitcreden

[PATCH] credential doc: make multiple-helper behavior more prominent (Re: [PATCH] clone: handle empty config values in -c)

2017-05-01 Thread Jonathan Nieder
;t mention this issue. Move the documentation to the config reference to make it easier to find. Signed-off-by: Jonathan Nieder --- Brandon Williams wrote: >> Noticed while trying to set credential.helper during a clone to use a >> specific helper without inheriting from ~/.gitconfig a

[PATCH] clone: handle empty config values in -c

2017-05-01 Thread Jonathan Nieder
nfig gets used. Signed-off-by: Jonathan Nieder --- Thoughts? Thanks, Jonathan builtin/clone.c | 4 +++- t/t5611-clone-config.sh | 8 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/builtin/clone.c b/builtin/clone.c index de85b85254..a6ae7d6180 100644 --- a/builtin

Re: [PATCH v2] sequencer: require trailing NL in footers

2017-04-25 Thread Jonathan Nieder
Jonathan Tan wrote: > Reported-by: Brian Norris > Signed-off-by: Jonathan Tan > --- [...] > sequencer.c | 11 +++ > t/t3511-cherry-pick-x.sh | 14 ++ > 2 files changed, 25 insertions(+) Reviewed-by: Jonathan Nieder This is still pretty

Re: [PATCH v8 2/2] run-command: restrict PATH search to executable files

2017-04-25 Thread Jonathan Nieder
which check that the path is to a regular, > executable file. Now run-command won't try to execute the directory or > non-executable file 'git-hello': > > $ git hello > Hello World! > > Reported-by: Brian Hatfield > Signed-off-by: Brandon Wil

Re: [PATCH v8 1/2] run-command: expose is_executable function

2017-04-25 Thread Jonathan Nieder
> Signed-off-by: Brandon Williams > --- > help.c| 43 +-- > run-command.c | 42 ++ > run-command.h | 1 + > 3 files changed, 44 insertions(+), 42 deletions(-) Reviewed-by: Jonathan Nieder

Re: [PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Jonathan Nieder
Johannes Schindelin wrote: >> On Tue, Apr 25, 2017 at 2:51 PM, Jonathan Tan >> wrote: >>> On 04/25/2017 02:04 PM, Stefan Beller wrote: Do we want to test for this use case in the future? [...] >>> I'm not sure of the value of including a test for this specific use case, >>> because Git norm

Re: [PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Jonathan Nieder
n 0; > + if (sb->buf[sb->len - ignore_footer - 1] != '\n') > + return 0; > + This is super subtle, but it does the right thing. The caller will notice it's not a conforming footer, add a newline to separate the new footer from it, and repair the footer

Re: [PATCH v2] clone: add a --no-tags option to clone without tags

2017-04-25 Thread Jonathan Nieder
Hi, Ævar Arnfjörð Bjarmason wrote: > Add a --no-tags option to "git clone" to clone without tags. Currently > there's no easy way to clone a repository and end up with just a > "master" branch via --single-branch, or track all branches and no > tags. Now --no-tags can be added to "git clone" with

Re: [PATCH] clone: add a --no-tags option to clone without tags

2017-04-25 Thread Jonathan Nieder
Hi, Ævar Arnfjörð Bjarmason wrote: > Add a --no-tags option to "git clone" to clone without tags. Currently > there's no easy way to clone a repository and end up with just a > "master" branch via --single-branch, or track all branches and no > tags. Now --no-tags can be added to "git clone" with

Re: [PATCH v7 2/2] run-command: don't try to execute directories

2017-04-25 Thread Jonathan Nieder
t; + write_script bin2/greet <<-\EOF && > + cat bin2/greet > + EOF > + chmod -x bin2/greet && This probably implies that the test needs a POSIXPERM dependency. Should it be a separate test_expect_success case so that the other part can still run on

Re: [PATCH v7 1/2] exec_cmd: expose is_executable function

2017-04-25 Thread Jonathan Nieder
7;s a good practice to find a logical place for a new function in the existing file, instead of defaulting to the end. That way, the file is easier to read sequentially, and if two different patches want to add a function to the same file then they are less likely to conflict. This could go before

Re: [PATCH v6 12/11] run-command: don't try to execute directories

2017-04-24 Thread Jonathan Nieder
(cc-ing the reporter) Brandon Williams wrote: > In some situations run-command will incorrectly try (and fail) to > execute a directory instead of an executable. For example: > > Lets suppose a user has PATH=~/bin (where 'bin' is a directory) and they > happen to have another directory inside 'bi

Re: [PATCH v6 12/11] run-command: don't try to execute directories

2017-04-24 Thread Jonathan Nieder
Junio C Hamano wrote: > Jonathan Nieder writes: >> Until we switched from using execvp to execve, the symptom was very >> subtle: it only affected the error message when a program could not be >> found, instead of affecting functionality more substantially. > > Hm

Re: [PATCH v6 12/11] run-command: don't try to execute directories

2017-04-24 Thread Jonathan Nieder
Brandon Williams wrote: > In some situations run-command will incorrectly try (and fail) to > execute a directory instead of an executable. For example: > > Lets suppose a user has PATH=~/bin (where 'bin' is a directory) and they > happen to have another directory inside 'bin' named 'git-remote-b

Re: [bug?] docs in Documentation/technical/ do not seem to be distributed

2017-04-19 Thread Jonathan Nieder
Hi, Samuel Lijin wrote: > It's possible this may have nothing to do with the Git project itself > because I have absolutely no idea how this is handled on the packaging > side or, possibly, if this is actually intended. > > There are a couple of links floating around in the man pages pointing > t

Re: [PATCH v3 2/2] xgethostname: handle long hostnames

2017-04-18 Thread Jonathan Nieder
Hi, David Turner wrote: > If the full hostname doesn't fit in the buffer supplied to > gethostname, POSIX does not specify whether the buffer will be > null-terminated, so to be safe, we should do it ourselves. Introduce > new function, xgethostname, which ensures that there is always a \0 > at

Re: [PATCH v3 1/2] use HOST_NAME_MAX to size buffers for gethostname(2)

2017-04-18 Thread Jonathan Nieder
fscanf(fp, "%" SCNuMAX " %" STR(HOST_NAME_MAX) "c", &pid, locking_host); Unfortunately, I don't think there's anything stopping a platform from defining #define HOST_NAME_MAX 0x100 which would break that. So this run-time calculation appears to be necessary. Reviewed-by: Jonathan Nieder Thanks.

Re: [PATCH] doc: trivial typo in git-format-patch.txt

2017-04-17 Thread Jonathan Nieder
Giuseppe Bilotta wrote: > Signed-off-by: Giuseppe Bilotta > --- > Documentation/git-format-patch.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) It took me a while to see the typo. It's s/of/or/. Good eyes. Reviewed-by: Jonathan Nieder

Re: [PATCH v2] fetch-pack: show clearer error message upon ERR

2017-04-17 Thread Jonathan Nieder
gt; Documentation/technical/pack-protocol.txt | 7 ++- > fetch-pack.c | 2 ++ > 2 files changed, 8 insertions(+), 1 deletion(-) For what it's worth, Reviewed-by: Jonathan Nieder Thanks for writing it. Sorry I didn't reply sooner.

Re: What's cooking in git.git (Apr 2017, #02; Sun, 16)

2017-04-17 Thread Jonathan Nieder
by: David Turner > Signed-off-by: Rene Scharfe > --- > builtin/gc.c | 2 +- > builtin/receive-pack.c | 2 +- > daemon.c | 4 > fetch-pack.c | 2 +- > git-compat-util.h | 4 > ident.c| 2 +- > 6 files changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Jonathan Nieder Thank you.

Re: [PATCH] xgethostname: handle long hostnames

2017-04-13 Thread Jonathan Nieder
Hi, David Turner wrote: > If the full hostname doesn't fit in the buffer supplied to > gethostname, POSIX does not specify whether the buffer will be > null-terminated, so to be safe, we should do it ourselves. [...] > +++ b/wrapper.c > @@ -655,3 +655,16 @@ void sleep_millisec(int millisec) > {

Re: [PATCH 1/1] difftool: fix use-after-free

2017-04-13 Thread Jonathan Nieder
Johannes Schindelin wrote: > Signed-off-by: Johannes Schindelin > --- > builtin/difftool.c | 7 +-- > t/t7800-difftool.sh | 19 +++ > 2 files changed, 24 insertions(+), 2 deletions(-) Reviewed-by: Jonathan Nieder Thank you.

Re: [PATCH v2 2/6] run-command: prepare command before forking

2017-04-13 Thread Jonathan Nieder
Hi, Brandon Williams wrote: > In order to avoid allocation between 'fork()' and 'exec()' the argv > array used in the exec call is prepared prior to forking the process. nit: s/(the argv array.*) is prepared/prepare \1/ Git's commit messages are in the imperative mood, as if they are ordering t

Re: [PATCH v2 0/6] forking and threading

2017-04-13 Thread Jonathan Nieder
Brandon Williams wrote: > From what I can see, there are now no calls in the child process (after fork > and before exec/_exit) which are not Async-Signal-Safe. This means that > fork/exec in a threaded context should work without deadlock I don't see why the former implies the latter. Can you

Re: [PATCH v2 1/6] t5550: use write_script to generate post-update hook

2017-04-13 Thread Jonathan Nieder
Hi, Brandon Williams wrote: > The post-update hooks created in t5550-http-fetch-dumb.sh is missing the > "!#/bin/sh" line which can cause issues with portability. Instead > create the hook using the 'write_script' function which includes the > proper "#!" line. > > Signed-off-by: Brandon William

Re: What's cooking in git.git (Apr 2017, #01; Tue, 11)

2017-04-11 Thread Jonathan Nieder
is already present in the repository. When run outside any repository, this produces an error: fatal: BUG: setup_git_env called without repository Signed-off-by: Jonathan Nieder --- diff --git a/sha1_file.c b/sha1_file.c index 71063890ff..bf1ff2ef77 100644 --- a/sha1_file.c +++ b/sha1_file.

Re: [PATCH] fetch-pack: show clearer error message upon ERR

2017-04-11 Thread Jonathan Nieder
die(_("git fetch-pack: got remote error '%s'"), arg); nit about the error message: since this isn't a sign of an internal error, we don't need to tell the user that it happened in git fetch-pack. How about using the same error used e.g. in run_remote_

Re: [PATCH v4 2/2] p0005-status: time status on very large repo

2017-04-11 Thread Jonathan Nieder
Hi, Jeff Hostetler wrote: > Signed-off-by: Jeff Hostetler Usually the commit message is a place to put some of the motivation behind the patch or why I should want to apply it. In this example, everything is answered by the previous patch, which suggests that it would be easier to understand

Re: [PATCH v4 1/2] string-list: use ALLOC_GROW macro when reallocing string_list

2017-04-11 Thread Jonathan Nieder
quashing together) with the p0005 patch as peff suggested, Reviewed-by: Jonathan Nieder

Re: [PATCH v6] http.postbuffer: allow full range of ssize_t values

2017-04-11 Thread Jonathan Nieder
hat's unresolved is formalizing what the oldest curl version is that we want to support. And that doesn't need to hold this patch hostage. So for what it's worth, Reviewed-by: Jonathan Nieder Thank you.

Re: [PATCH 0/5] forking and threading

2017-04-11 Thread Jonathan Nieder
Brandon Williams wrote: > As far as I understand the only instance of threading and forking which exists > in the current code base is 'git grep --recurse-submodules', and the standard > builds against glibc shouldn't exhibit any of this deadlocking. I don't think we consider builds against glibc

Re: [PATCH 5/5] run-command: add note about forking and threading

2017-04-11 Thread Jonathan Nieder
Eric Wong wrote: > Jonathan Nieder wrote: >> Why can't git use e.g. posix_spawn to avoid this? > > posix_spawn does not support chdir, and it seems we run non-git > commands so no using "git -C" for those. On the other hand, a number of the non-git commands we

Re: [PATCH 4/5] run-command: prepare child environment before forking

2017-04-11 Thread Jonathan Nieder
Brandon Williams wrote: > On 04/10, Jonathan Nieder wrote: >> struct argv_array result = ARGV_ARRAY_INIT; >> struct string_list mods = STRING_LIST_INIT_DUP; >> struct strbuf key = STRBUF_INIT; >> const char **p; >> >> for (p = cm

Re: [PATCH 4/5] run-command: prepare child environment before forking

2017-04-10 Thread Jonathan Nieder
Brandon Williams wrote: > In order to avoid allocation between 'fork()' and 'exec()' prepare the > environment to be used in the child process prior to forking. If using something like posix_spawn(), this would be needed anyway, so I'll review it. [...] > +++ b/run-command.c [...] > +static char

Re: [PATCH 5/5] run-command: add note about forking and threading

2017-04-10 Thread Jonathan Nieder
Hi, Brandon Williams wrote: > --- a/run-command.c > +++ b/run-command.c > @@ -458,6 +458,14 @@ int start_command(struct child_process *cmd) > argv_array_pushv(&argv, cmd->argv); > } > > + /* > + * NOTE: In order to prevent deadlocking when using threads special > +

Re: [PATCH v2 1/2] string-list: use ALLOC_GROW macro when reallocing string_list

2017-04-05 Thread Jonathan Nieder
gt; string-list.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) With or without that wording tweak, Reviewed-by: Jonathan Nieder

Re: [PATCH v1 2/2] p0005-status: time status on very large repo

2017-04-05 Thread Jonathan Nieder
Hi, g...@jeffhostetler.com wrote: > +++ b/t/perf/p0005-status.sh > @@ -0,0 +1,70 @@ > +#!/bin/sh > + > +test_description="Tests performance of read-tree" > + > +. ./perf-lib.sh > + > +test_perf_default_repo > +test_checkout_worktree > + > +## usage: dir depth width files > +make_paths () { > +

Re: [PATCH v1 1/2] string-list: use ALLOC_GROW macro when reallocing string_list

2017-04-05 Thread Jonathan Nieder
nd the correct invariant. Fortunately (1) string_list_append_nodup already uses ALLOC_GROW the way this patch does and (2) REALLOC_ARRAY determines the meaning of list->alloc. The >= was just overeager and this is safe. After removing the unnecessary 'if' as described by Stefan, Reviewed-by: Jonathan Nieder Thanks.

Re: [PATCH v6] read-cache: force_verify_index_checksum

2017-04-05 Thread Jonathan Nieder
to replace the file. If that doesn't work, I expect there are a large number of other tests that would need fixing... > + test_when_finished "rm .git/index" && Likewise --- this seems redundant next to the "mv" enqueued previously. > + test_must_fail git fsck --cache That said, Reviewed-by: Jonathan Nieder

Re: [PATCH v5] read-cache: force_verify_index_checksum

2017-04-05 Thread Jonathan Nieder
t fsck --cache && > + rm .git/index && > + mv .git/index.backup .git/index && > + rm > +' This is great. optional: you can do the cleanup commands in test_when_finished to make sure they happen even if the test fails. Tests don

Re: [PATCH v5] http.postbuffer: allow full range of ssize_t values

2017-04-03 Thread Jonathan Nieder
ned and unsigned integers"). That makes it fodder for over-eager optimizers. Would something like the following work? With that change, Reviewed-by: Jonathan Nieder diff --git i/remote-curl.c w/remote-curl.c index b7b69e096a..cf171b1bc9 100644 --- i/remote-curl.c +++ w/remote-curl.c @@ -53

Re: [PATCH v2 2/2] push: propagate push-options with --recurse-submodules

2017-03-31 Thread Jonathan Nieder
| 1 + > t/t5545-push-options.sh | 39 +++ > transport.c | 1 + > 4 files changed, 52 insertions(+), 2 deletions(-) For what it's worth, Reviewed-by: Jonathan Nieder

Re: [PATCH v2 1/2] push: unmark a local variable as static

2017-03-31 Thread Jonathan Nieder
onnection dawned on me. > Signed-off-by: Brandon Williams > --- > builtin/push.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) For what it's worth, Reviewed-by: Jonathan Nieder

Re: [PATCH] push: propagate push-options with --recurse-submodules

2017-03-31 Thread Jonathan Nieder
Hi, Brandon Williams wrote: > Teach push --recurse-submodules to propagate push-options recursively to > the pushes performed in the submodules. Sounds like a good change. [...] > +++ b/submodule.c [...] > @@ -793,6 +794,12 @@ static int push_submodule(const char *path, int dry_run) >

Re: [PATCH 2/2] submodule.c: correctly handle nested submodules in is_submodule_modified

2017-03-29 Thread Jonathan Nieder
Documentation/git-status.txt | 2 ++ > submodule.c | 21 +++-- > t/t3600-rm.sh| 2 +- > t/t7506-status-submodule.sh | 4 ++-- > 4 files changed, 24 insertions(+), 5 deletions(-) Reviewed-by: Jonathan Nieder but I would be a lot mor

Re: [PATCH 1/2] short status: improve reporting for submodule changes

2017-03-29 Thread Jonathan Nieder
Stefan Beller wrote: > Signed-off-by: Stefan Beller > Reviewed-by: Jonathan Nieder > --- > Documentation/git-status.txt | 11 > t/t3600-rm.sh| 18 +-- > t/t7506-status-submodule.sh | 117 > +

Re: [PATCH 2/2] unpack-trees.c: align submodule error message to the other error messages

2017-03-29 Thread Jonathan Nieder
Stefan Beller wrote: > As the place holder in the error message is for multiple submodules, > we don't want to encapsulate the string place holder in single quotes. Makes sense. > Signed-off-by: Stefan Beller > --- > unpack-trees.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > d

Re: [PATCH 1/2] entry.c: submodule recursing: respect force flag correctly

2017-03-29 Thread Jonathan Nieder
Stefan Beller wrote: > In case of a non-forced worktree update, the submodule movement is tested > in a dry run first, such that it doesn't matter if the actual update is > done via the force flag. However for correctness, we want to give the > flag is specified by the user. "for correctness" mea

Re: BUG: Renaming a branch checked out in a different work tree

2017-03-29 Thread Jonathan Nieder
(+cc: Kazuki Yamaguchi --- thanks for writing the fix!) Hi Eyal, Eyal Lotem wrote: > git version: 2.7.4 > installed from Ubuntu repos: > Ubuntu Version: 1:2.7.4-0ubuntu1 > > When renaming a branch checked out in a different work tree, that work > tree's state is corrupted. Git status in that work

Re: [PATCH 2/2] submodule.c: correctly handle nested submodules in is_submodule_modified

2017-03-28 Thread Jonathan Nieder
Stefan Beller wrote: > Suppose I have a superproject 'super', with two submodules 'super/sub' > and 'super/sub1'. 'super/sub' itself contains a submodule > 'super/sub/subsub'. Now suppose I run, from within 'super': > > echo hi >sub/subsub/stray-file > echo hi >sub1/stray-file > > Currentl

Re: [PATCH 1/2] short status: improve reporting for submodule changes

2017-03-28 Thread Jonathan Nieder
Hi, Stefan Beller wrote: [...] > +++ b/t/t7506-status-submodule.sh [...] > @@ -287,4 +311,82 @@ test_expect_success 'diff --submodule with merge > conflict in .gitmodules' ' > test_cmp diff_submodule_actual diff_submodule_expect > ' > > +test_expect_success 'setup superproject with untr

Re: [RFC] should these two topics graduate to 'master' soon?

2017-03-28 Thread Jonathan Nieder
Hi Junio, Junio C Hamano wrote: > There are two topics that are marked as "Will cook in 'next'" for > practically forever in the "What's cooking" reports. The world may > have become ready for one or both of them, in which case we should > do the merge not too late in the cycle. > > * jc/merge-d

Re: UNS: Re: cherry-pick --message?

2017-03-28 Thread Jonathan Nieder
Andreas Krey wrote: > On Tue, 21 Mar 2017 13:33:35 +, Jeff King wrote: >> Probably "format-patch | sed | am -3" is your best bet if you want to >> modify the patches in transit _and_ have the user just use normal git >> tools. > > Except that 'git am' doesn't have --no-commit like cherry-pick

Re: [PATCH 7/7] submodule.c: correctly handle nested submodules in is_submodule_modified

2017-03-27 Thread Jonathan Nieder
Junio C Hamano wrote: > Shouldn't this done as part of 4/7 where is_submodule_modified() > starts reading from the porcelain v2 output? 4/7 does adjust for > the change from double question mark (porcelain v1) to a single one > for untracked, but at the same time it needs to prepare for these > '

Re: [PATCH] push: allow atomic flag via configuration

2017-03-27 Thread Jonathan Nieder
Hi, Romuald Brunet wrote: > On ven., 2017-03-24 at 12:29 -0700, Junio C Hamano wrote: >> Jeff King writes: >>> My one question would be whether people would want this to actually be >>> specific to a particular remote, and not just on for a given repository >>> (your "site-specific" in the descr

Re: [PATCH v2 1/2] read-cache: skip index SHA verification

2017-03-27 Thread Jonathan Nieder
Jeff King wrote: > Hrm, there shouldn't be any dependency of the config on the index (and > there are a handful of options which impact the index already). Did you > try it and run into problems? > > In general, I'd much rather see us either: > > 1. Rip the code out entirely if it is not meant t

Re: [PATCH v7 0/7] short status: improve reporting for submodule changes

2017-03-24 Thread Jonathan Nieder
module.sh | 57 > > wt-status.c | 13 -- > 5 files changed, 116 insertions(+), 37 deletions(-) Patches 1-6 are Reviewed-by: Jonathan Nieder The effect of patch 7 on --porcelain=2 output is subtle enough that I don't

Re: [PATCH 7/7] submodule.c: correctly handle nested submodules in is_submodule_modified

2017-03-24 Thread Jonathan Nieder
Stefan Beller wrote: > When a nested submodule has untracked files, it would be reported as > "modified submodule" in the superproject, because submodules are not > parsed correctly in is_submodule_modified as they are bucketed into > the modified pile as "they are not an untracked file". > > Sig

Re: [PATCH 6/7] short status: improve reporting for submodule changes

2017-03-24 Thread Jonathan Nieder
d->worktree_status = 'm'; > + else if (d->dirty_submodule & > DIRTY_SUBMODULE_UNTRACKED) > + d->worktree_status = '?'; > + } > +

Re: [PATCH 5/7] submodule.c: stricter checking for submodules in is_submodule_modified

2017-03-24 Thread Jonathan Nieder
Stefan Beller wrote: > By having a stricter check in the superproject we catch errors earlier, > instead of spawning a child process to tell us. > > Signed-off-by: Stefan Beller > Reviewed-by: Jonathan Nieder Yep. :)

Re: [PATCH 4/7] submodule.c: port is_submodule_modified to use porcelain 2

2017-03-24 Thread Jonathan Nieder
bmodule |= DIRTY_SUBMODULE_MODIFIED; At first I didn't believe it could be so simple. :) Unlike ordinary files that use 1, 2, or u, ignored files use '!'. You're not passing --ignored so you don't have to worry about them. Reviewed-by: Jonathan Nieder Very nice.

Re: [PATCH 3/7] submodule.c: convert is_submodule_modified to use strbuf_getwholeline

2017-03-24 Thread Jonathan Nieder
Stefan Beller wrote: > Instead of implementing line reading yet again, make use of our beautiful > library function to read one line. By using strbuf_getwholeline instead > of strbuf_read, we avoid having to allocate memory for the entire child > process output at once. That is, we limit maximum

Re: [PATCH 2/7] submodule.c: factor out early loop termination in is_submodule_modified

2017-03-24 Thread Jonathan Nieder
TY_SUBMODULE_UNTRACKED)) > - break; > - } > + > + if ((dirty_submodule & DIRTY_SUBMODULE_MODIFIED) && > + ((dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) || > ignore_untracked)) nit: long line Revie

<    7   8   9   10   11   12   13   14   15   16   >