Re: Updated to v2.14.2 on macOS; git add --patch broken

That's a very very old setting :) Honestly, I don't know why it's "always". I have set up this setting years ago and never thought about it again, as it worked out. I changed it to "auto" and the --patch options are working again on v2.14.2! Thank you very much for your time & efforts. If I can

Re: Updated to v2.14.2 on macOS; git add --patch broken

On Thu, Sep 28, 2017 at 07:03:55AM +0200, Toni Uebernickel wrote: > color.ui=always This is the problem, and Jonathan's guess was correct that 136c8c8b8f (color: check color.ui in git_default_config(), 2017-07-13) is related. Re-reading that commit message, I'm inclined to say that the commit

[PATCH 1/1] fast-import: checkpoint: dump branches/tags/marks even if object_count==0

The checkpoint command cycles packfiles if object_count != 0, a sensible test or there would be no pack files to write. Since 820b931012, the command also dumps branches, tags and marks, but still conditionally. However, it is possible for a command stream to modify refs or create marks without

Re: Updated to v2.14.2 on macOS; git add --patch broken

Hi Jonathan, my configuration reads as follows, I only removed private tokens content. I will try to get more details on which version exactly breaks the command. Kind Regards, Toni core.excludesfile=/usr/local/etc/gitignore

Re: [PATCH] fast-import: checkpoint: dump branches/tags/marks even if object_count==0

On Wed, Sep 27, 2017 at 8:48 PM, Junio C Hamano wrote: >> + cat $input_file >&8 > > It probably is a good idea to quote "$input_file" in case other > people later use a full path to the file or something; for now this > is OK. Right. > fd#8 at this point does not have a

Re: [PATCH] gitk: expand $config_file_tmp before reporting to user

Junio C Hamano writes: > Max Kirillov writes: > >> Tilda-based path may confise some users. First, tilda is not known >> for Window users, second, it may point to unexpected location >> depending on various environment setup. >> >> Expand the path to

[PATCH v4] technical doc: add a design doc for hash function transition

This document describes what a transition to a new hash function for Git would look like. Add it to Documentation/technical/ as the plan of record so that future changes can be recorded as patches. Also-by: Brandon Williams Also-by: Jonathan Tan

Re: [PATCH] gitk: expand $config_file_tmp before reporting to user

Max Kirillov writes: > Tilda-based path may confise some users. First, tilda is not known > for Window users, second, it may point to unexpected location > depending on various environment setup. > > Expand the path to "nativename", so that ~/.config/git/gitk-tmp > would be

[PATCH] gitk: expand $config_file_tmp before reporting to user

Tilda-based path may confise some users. First, tilda is not known for Window users, second, it may point to unexpected location depending on various environment setup. Expand the path to "nativename", so that ~/.config/git/gitk-tmp would be "C:\Users\user\.config\git\gitk-tmp", for example. It

Re: [PATCH] diff: correct newline in summary for renamed files

Ramsay Jones writes: >> +git checkout -b mode initial && >> +git update-index --chmod=+x file0 && > > would 'test_chmod +x file0 &&' work here? This one wants to set +x only in the index, leaving the working tree version without the executable bit.

Re: diffstat summary mode change bug

Linus Torvalds writes: > and the reason seems to be that the '\n' at the end got dropped as the > old code was very confusing (the old code had two different '\n' cases > for the "show filename or not"). > > I think the right fix is this whitespace-damaged trivial

Re: [PATCH] fast-import: checkpoint: dump branches/tags/marks even if object_count==0

"Eric Rannaud" writes: > diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh > index 67b8c50a5ab4..9aa3470d895b 100755 > --- a/t/t9300-fast-import.sh > +++ b/t/t9300-fast-import.sh > @@ -3120,4 +3120,133 @@ test_expect_success 'U: validate root delete result' ' >

Re: [PATCH] oidmap: map with OID as key

Jonathan Tan writes: > This is similar to using the hashmap in hashmap.c, but with an > easier-to-use API. In particular, custom entry comparisons no longer > need to be written, and lookups can be done without constructing a > temporary entry structure. A naïve

Re: Help

Hi, On Thu, Sep 28, 2017 at 3:21 AM, Nityananda wrote: > Hello, > I am new to this community. I am facing a problem while using the > "make" command inside the "t/" folder. > > The error is > > > "1..31 > Makefile:49: recipe for target 't5551-http-fetch-smart.sh'

Help

Hello, I am new to this community. I am facing a problem while using the "make" command inside the "t/" folder. The error is  "1..31 Makefile:49: recipe for target 't5551-http-fetch-smart.sh' failed make[1]: *** [t5551-http-fetch-smart.sh] Error 1 make[1]: Leaving directory

Re: [PATCH] oidmap: map with OID as key

On 09/27, Jonathan Tan wrote: > This is similar to using the hashmap in hashmap.c, but with an > easier-to-use API. In particular, custom entry comparisons no longer > need to be written, and lookups can be done without constructing a > temporary entry structure. > > oidset has been updated to

Re: [PATCH] diff: correct newline in summary for renamed files

On Wed, Sep 27, 2017 at 04:57:40PM -0700, Stefan Beller wrote: > >> + GIT_AUTHOR_DATE="2006-06-26 00:06:00 +" && > >> + GIT_COMMITTER_DATE="2006-06-26 00:06:00 +" && > >> + export GIT_AUTHOR_DATE GIT_COMMITTER_DATE && > >> + git checkout -b mode initial && > >> + git

Re: [PATCH] diff: correct newline in summary for renamed files

On Wed, Sep 27, 2017 at 03:51:26PM -0700, Stefan Beller wrote: > In 146fdb0dfe (diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY, > 2017-06-29), the conversion from direct printing to the symbol emission > dropped the new line character for renamed, copied and rewritten files. > > Add

Re: [PATCH 07/13] object-filter: common declarations for object filtering

On Wed, 27 Sep 2017 13:09:42 -0400 Jeff Hostetler wrote: > On 9/26/2017 6:39 PM, Jonathan Tan wrote: > > On Fri, 22 Sep 2017 20:30:11 + > > Jeff Hostetler wrote: > > > >> Makefile| 1 + > >> object-filter.c | 269 > >>

Re: [PATCH] diff: correct newline in summary for renamed files

>> + GIT_AUTHOR_DATE="2006-06-26 00:06:00 +" && >> + GIT_COMMITTER_DATE="2006-06-26 00:06:00 +" && >> + export GIT_AUTHOR_DATE GIT_COMMITTER_DATE && >> + git checkout -b mode initial && >> + git update-index --chmod=+x file0 && > > would 'test_chmod +x file0 &&' work

Re: [PATCH] diff: correct newline in summary for renamed files

On 27/09/17 23:51, Stefan Beller wrote: > In 146fdb0dfe (diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY, > 2017-06-29), the conversion from direct printing to the symbol emission > dropped the new line character for renamed, copied and rewritten files. > > Add the emission of a

Re: [PATCH] fast-import: checkpoint: dump branches/tags/marks even if object_count==0

On 27/09/17 20:46, Eric Rannaud wrote: > The checkpoint command cycles packfiles if object_count != 0, a sensible > test or there would be no pack files to write. Since 820b931012, the > command also dumps branches, tags and marks, but still conditionally. > However, it is possible for a command

Re: [PATCH] diff: correct newline in summary for renamed files

Stefan Beller wrote: > In 146fdb0dfe (diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY, > 2017-06-29), the conversion from direct printing to the symbol emission > dropped the new line character for renamed, copied and rewritten files. > > Add the emission of a newline, add a test for

[PATCH] diff: correct newline in summary for renamed files

In 146fdb0dfe (diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY, 2017-06-29), the conversion from direct printing to the symbol emission dropped the new line character for renamed, copied and rewritten files. Add the emission of a newline, add a test for this case. Reported-by: Linus

Re: [PATCH] diff: correct newline in summary for renamed files

On Wed, Sep 27, 2017 at 03:39:00PM -0700, Stefan Beller wrote: > On Wed, Sep 27, 2017 at 3:32 PM, Jeff King wrote: > > > The most appropriate place seems like t4013. I tried adding to its big > > list of tested formats, but it's quite fragile. The patch below is what > > I came

Re: [PATCH] diff: correct newline in summary for renamed files

On Wed, Sep 27, 2017 at 3:32 PM, Jeff King wrote: > The most appropriate place seems like t4013. I tried adding to its big > list of tested formats, but it's quite fragile. The patch below is what > I came up with, but it still needs updated to cover the cases which call > "log

Re: [PATCH] diff: correct newline in summary for renamed files

On Wed, Sep 27, 2017 at 3:09 PM, Jeff King wrote: > On Wed, Sep 27, 2017 at 02:58:52PM -0700, Stefan Beller wrote: > >> From: Linus Torvalds >> >> In 146fdb0dfe (diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY, >> 2017-06-29), the

Re: [PATCH] diff: correct newline in summary for renamed files

On Wed, Sep 27, 2017 at 06:09:25PM -0400, Jeff King wrote: > > diff --git a/t/t4016-diff-quote.sh b/t/t4016-diff-quote.sh > > index 9c48e5c2c9..514056dd10 100755 > > --- a/t/t4016-diff-quote.sh > > +++ b/t/t4016-diff-quote.sh > > @@ -30,6 +30,7 @@ test_expect_success setup ' > > git add . &&

[PATCH] oidmap: map with OID as key

This is similar to using the hashmap in hashmap.c, but with an easier-to-use API. In particular, custom entry comparisons no longer need to be written, and lookups can be done without constructing a temporary entry structure. oidset has been updated to use oidmap. Signed-off-by: Jonathan Tan

Re: [PATCH] diff: correct newline in summary for renamed files

On Wed, Sep 27, 2017 at 02:58:52PM -0700, Stefan Beller wrote: > From: Linus Torvalds > > In 146fdb0dfe (diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY, > 2017-06-29), the conversion from direct printing to the symbol emission > dropped the new line

hacktoberfest

Hey, As some may have noticed, GitHub and DigitalOcean have been promoting Open Source contributions in October for a few years now. While the git repository itself is not hosted under GitHub, the Pro Git book, git for Windows, and git-scm website (at least) projects are, and could use this

Re: [PATCH] diff: correct newline in summary for renamed files

On Wed, Sep 27, 2017 at 2:58 PM, Stefan Beller wrote: > > Linus, I assumed your sign off for the original patch. Thanks for spotting. > > Adding the mode change to t4016 seems like the easiest way to test it. Looks good to me, and you don't need to give me authorship

[PATCH] diff: correct newline in summary for renamed files

From: Linus Torvalds In 146fdb0dfe (diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY, 2017-06-29), the conversion from direct printing to the symbol emission dropped the new line character for renamed, copied and rewritten files. Add the emission of a

Re: [PATCH v2 8/9] http: tell server that the client understands v1

On 09/27, Junio C Hamano wrote: > Brandon Williams writes: > > > @@ -897,6 +898,21 @@ static void set_from_env(const char **var, const char > > *envname) > > *var = val; > > } > > > > +static void protocol_http_header(void) > > +{ > > + if

Re: [PATCH v2 5/9] upload-pack, receive-pack: introduce protocol version 1

On 09/27, Junio C Hamano wrote: > Brandon Williams writes: > > > @@ -1963,6 +1964,19 @@ int cmd_receive_pack(int argc, const char **argv, > > const char *prefix) > > else if (0 <= receive_unpack_limit) > > unpack_limit = receive_unpack_limit; > > > > +

Re: [PATCH v2 4/9] daemon: recognize hidden request arguments

On 09/27, Junio C Hamano wrote: > Brandon Williams writes: > > > A normal request to git-daemon is structured as > > "command path/to/repo\0host=..\0" and due to a bug in an old version of > > git-daemon 73bb33a94 (daemon: Strictly parse the "extra arg" part of the > >

Re: diffstat summary mode change bug

On Wed, Sep 27, 2017 at 2:02 PM, Linus Torvalds wrote: > On Wed, Sep 27, 2017 at 1:40 PM, Stefan Beller wrote: >> >> I disagree with this analysis, as the fix you propose adds the >> new line unconditionally, i.e. this code path would be broken

Wrong -dirty suffix set by setlocalversion (was: BUG in git diff-index)

+ linux-kbuild list which is not in the output of: ./scripts/get_maintainer.pl -f scripts/setlocalversion ... but seems relevant anyway. On 31/03/16 13:39, Junio C Hamano wrote: > Andy Lowry writes: > >> So I think now that the script should do "update-index --refresh"

Re: diffstat summary mode change bug

On Wed, Sep 27, 2017 at 1:40 PM, Stefan Beller wrote: > > I disagree with this analysis, as the fix you propose adds the > new line unconditionally, i.e. this code path would be broken > regardless of "show filename or not". Right. Because it is what we want. The old code

Re: [PATCH 03/13] list-objects: filter objects in traverse_commit_list

On Wed, 27 Sep 2017 15:09:43 -0400 Jeff Hostetler wrote: > By adding it to the set of provisionally omitted objects, we > have the option to capture a little extra information with it > and refer to that the next time we see the object in the traversal. > For example, in

Re: diffstat summary mode change bug

On Wed, Sep 27, 2017 at 11:15 AM, Linus Torvalds wrote: > (ok, linewrapping in this email may make that look wrong - but the > "mode change" land the "create mode" are both on the same line. Thanks for the bug report. > and the reason seems to be that the '\n' at

Re: -s theirs use-case(s) Was: BUG: merge -s theirs is not in effect

On Wed, 27 Sep 2017, Yaroslav Halchenko wrote: > > And at that point, use of "-s ours" is no longer a workaround for > > lack of "-s theirs". It is a proper part of the desired semantics, > > i.e. from the point of view of the surviving canonical history line, > > you want to preserve what it

Re: Updated to v2.14.2 on macOS; git add --patch broken

Jonathan Nieder wrote: > Jeff King wrote: >> There aren't a lot of changes to the script between v2.13.2 and v2.14.2. >> The most plausible culprit is d5addcf522 (add--interactive: handle EOF >> in prompt_yesno, 2017-06-21), but I'm scratching my head over how that >> could cause what you're

Re: Updated to v2.14.2 on macOS; git add --patch broken

Hi, Jeff King wrote: > On Wed, Sep 27, 2017 at 07:28:49PM +0200, Toni Uebernickel wrote: >> The previous version was v2.13.2. >> I switched back to this version, and it works perfectly fine; without any >> changes to my system. > > Thanks for confirming. > > There aren't a lot of changes to the

[PATCH] fast-import: checkpoint: dump branches/tags/marks even if object_count==0

The checkpoint command cycles packfiles if object_count != 0, a sensible test or there would be no pack files to write. Since 820b931012, the command also dumps branches, tags and marks, but still conditionally. However, it is possible for a command stream to modify refs or create marks without

Re: What's the idea of specifying binary=true together with textconv?

On Wed, Sep 27, 2017 at 10:06:25PM +0300, Ilya Kantor wrote: > When textconv is specified, the program is always used for conversion. Not always. It's enabled by default for the git-diff and git-log" porcelain, but not for plumbing commands, nor for git-format-patch (which needs to create a

Re: [PATCH 03/13] list-objects: filter objects in traverse_commit_list

On 9/27/2017 2:00 PM, Jonathan Tan wrote: On Wed, 27 Sep 2017 13:04:42 -0400 Jeff Hostetler wrote: The sparse filter is looking at pathnames and using the same rules as sparse-checkout to decide which to *include* in the result. This is essentially backwards from

What's the idea of specifying binary=true together with textconv?

Hello, When textconv is specified, the program is always used for conversion. E.g. consider this: [diff "png"] textconv = wc Even if a png file is ASCII (manually created), wc gets called, and the result is made of its output. So, the question is: why should one specify binary = true

diffstat summary mode change bug

Current git shows file-mode changes incorrectly in the diffstat summary, as I just noted from a pull request I did on the kernel. The pull request *should* have resulted in a summary like this: ... 21 files changed, 247 insertions(+), 67 deletions(-) mode change 100644 => 100755

Re: Updated to v2.14.2 on macOS; git add --patch broken

On Wed, Sep 27, 2017 at 07:28:49PM +0200, Toni Uebernickel wrote: > The previous version was v2.13.2. > I switched back to this version, and it works perfectly fine; without any > changes to my system. Thanks for confirming. There aren't a lot of changes to the script between v2.13.2 and

Re: [PATCH 03/13] list-objects: filter objects in traverse_commit_list

On Wed, 27 Sep 2017 13:04:42 -0400 Jeff Hostetler wrote: > The sparse filter is looking at pathnames and using the same rules > as sparse-checkout to decide which to *include* in the result. This > is essentially backwards from the other filters which are looking for >

Re: [PATCH v2 6/9] connect: teach client to recognize v1 server response

On 09/27, Junio C Hamano wrote: > Brandon Williams writes: > > > +/* Returns 1 if packet_buffer is a protocol version pkt-line, 0 otherwise. > > */ > > +static int process_protocol_version(void) > > +{ > > + switch (determine_protocol_version_client(packet_buffer)) { > > +

Re: Updated to v2.14.2 on macOS; git add --patch broken

The previous version was v2.13.2. I switched back to this version, and it works perfectly fine; without any changes to my system. -- Toni Uebernickel tuebernic...@gmail.com - https://keybase.io/havvg https://github.com/havvg - https://www.xing.com/profile/Toni_Uebernickel > On 27. Sep 2017, at

Re: [PATCH 09/13] rev-list: add object filtering support

On 9/26/2017 6:44 PM, Jonathan Tan wrote: On Fri, 22 Sep 2017 20:30:13 + Jeff Hostetler wrote: + if (filter_options.relax) { Add some documentation about how this differs from ignore_missing_links in struct rev_info. It's unclear what that

Re: [PATCH 07/13] object-filter: common declarations for object filtering

On 9/26/2017 6:39 PM, Jonathan Tan wrote: On Fri, 22 Sep 2017 20:30:11 + Jeff Hostetler wrote: Makefile| 1 + object-filter.c | 269 object-filter.h | 173

Re: Updated to v2.14.2 on macOS; git add --patch broken

On Wed, Sep 27, 2017 at 03:23:21PM +0200, Toni Uebernickel wrote: > Hi there, > > I updated to git version v2.14.2 on macOS using homebrew. > > Since then `git add --patch` and `git stash save --patch` are not > working anymore. It's just printing the complete diff without ever > stopping to

Re: [PATCH 03/13] list-objects: filter objects in traverse_commit_list

On 9/26/2017 6:31 PM, Jonathan Tan wrote: On Fri, 22 Sep 2017 20:26:22 + Jeff Hostetler wrote: From: Jeff Hostetler Create traverse_commit_list_filtered() and add filtering You mention _filtered() here, but this patch contains

Re: [PATCH v6 09/40] Add initial external odb support

On Tue, Sep 19, 2017 at 7:45 PM, Jonathan Tan wrote: > I wonder if it's better to get a change like this (PATCH v6 09/40 and > any of the previous patches that this depends on) in and then build on > it rather than to review the whole patch set at a time. This would >

Re: [PATCH] git: add --no-optional-locks option

On Wed, Sep 27, 2017 at 07:20:05PM +0530, Kaartic Sivaraam wrote: > On Wed, 2017-09-27 at 02:40 -0400, Jeff King wrote: > > On Sun, Sep 24, 2017 at 01:08:41PM +0530, Kaartic Sivaraam wrote: > > > > > > > > So, if I get that correctly "git status --no-optional-locks" is a way to > > > get > > >

Re: [PATCH 4/4] Move documentation of string_list into string-list.h

On 25.09.2017 20:40, Brandon Williams wrote: > On 09/25, Han-Wen Nienhuys wrote: >> This mirrors commit bdfdaa4978dd92992737e662f25adc01d32d0774 which did > > Not really important but most times we reference another commit from a > commit msg we include the one line summary like: > 'bdfdaa497

Re: [PATCH 02/13] oidset2: create oidset subclass with object length and pathname

On 9/26/2017 6:20 PM, Jonathan Tan wrote: On Fri, 22 Sep 2017 20:26:21 + Jeff Hostetler wrote: From: Jeff Hostetler Create subclass of oidset where each entry has a field to store the length of the object's content and an optional

Re: [PATCH] git: add --no-optional-locks option

On Wed, 2017-09-27 at 02:40 -0400, Jeff King wrote: > On Sun, Sep 24, 2017 at 01:08:41PM +0530, Kaartic Sivaraam wrote: > > > > > So, if I get that correctly "git status --no-optional-locks" is a way to get > > the "current" status without updating the on disk index file? > > It's actually "git

Updated to v2.14.2 on macOS; git add --patch broken

Hi there, I updated to git version v2.14.2 on macOS using homebrew. Since then `git add --patch` and `git stash save --patch` are not working anymore. It's just printing the complete diff without ever stopping to ask for actions. This results in an unusable state, as the whole command option

About Magit (git client) and its fundraiser

Hello, Magit is a Git client implemented as an Emacs extension. It has been around since 2008 and I have been maintaining it since 2013. Magit was recently featured in Git Rev News #31 and one of its curators encouraged me to write to this list as well. I haven't done that before because I was

Re: [PATCH] [Outreachy] cleanup: use list.h in mru.h and mru.c

About the title, we don't usually start with "fix:" or "cleanup:" or "feature:". We usually start with the (main) area of the code that is changed. So maybe something like "mru: use double-linked list implementation from list.h" would be better. On Wed, Sep 27, 2017 at 12:18 PM, Оля Тележная

Re: [PATCH v2 3/9] protocol: introduce protocol extention mechanisms

Junio C Hamano writes: >> +enum protocol_version determine_protocol_version_server(void) >> +{ >> +const char *git_protocol = getenv(GIT_PROTOCOL_ENVIRONMENT); >> +enum protocol_version version = protocol_v0; >> + >> +if (git_protocol) { >> +struct

[PATCH] [Outreachy] cleanup: use list.h in mru.h and mru.c

Remove implementation of double-linked list in mru.c and mru.h and use implementation from list.h. Signed-off-by: Olga Telezhnaia Mentored-by: Christian Couder , Jeff King --- builtin/pack-objects.c | 5 +++-- mru.c

Honorable Barrister Paul Williams.

Attn: Sir/Madam I am Honorable Barrister Paul Williams the personal resident Attorney here in Burkina Faso to Late Mr. Muammar Muhammad Abu Minyar AL-Gaddafi of Libya c. 1942 – 20 October 2011. Late Mr. Muammar Muhammad Abu Minyar al-Gaddafi c. 1942 – 20 October 2011, commonly known as Colonel

Re: Speeding up history traversals with caches

On Mon, Sep 25, 2017 at 05:28:43PM -0700, Sabelo Mhlambi wrote: > Hi Jeff (and the Git community), > > As my intro to open source contributions I'd like to attempt the "Speeding > up history traversals with caches" as outlined here > https://git.github.io/Outreachy-15/. > > It seems like a

Re: [PATCH 1/3] validate_headref: NUL-terminate HEAD buffer

Jeff King writes: > I had the same concern, but actually truncation is not a problem here > (for a symlink or a symref). We are only seeing if the contents look > vaguely correct, so really we never parse past "refs/" in either case. Ahh, OK. It seems that at least for this

Re: [PATCH 1/3] validate_headref: NUL-terminate HEAD buffer

On Wed, Sep 27, 2017 at 04:06:22PM +0900, Junio C Hamano wrote: > A few tangents I noticed: > > - the result of readlink should be checked with starts_with() in >the modern codebase (#leftoverbits). Yes, though it needs to NUL-terminate, too (readlink does not do so automatically). Again,

Re: [PATCH v2 3/7] prefer "!=" when checking read_in_full() result

On Wed, Sep 27, 2017 at 03:59:15PM +0900, Junio C Hamano wrote: > Jeff King writes: > > > diff --git a/csum-file.c b/csum-file.c > > index a172199e44..2adae04073 100644 > > --- a/csum-file.c > > +++ b/csum-file.c > > @@ -19,7 +19,7 @@ static void flush(struct sha1file *f, const

Re: [PATCH 2/3] validate_headref: use skip_prefix for symref parsing

Jeff King writes: > Since the previous commit guarantees that our symref buffer > is NUL-terminated, we can just use skip_prefix() and friends > to parse it. This is shorter and saves us having to deal > with magic numbers and keeping the "len" counter up to date. Ah, I should

Re: [PATCH 1/3] validate_headref: NUL-terminate HEAD buffer

Jeff King writes: > diff --git a/path.c b/path.c > index b533ec938d..3e4d7505ef 100644 > --- a/path.c > +++ b/path.c > @@ -662,6 +662,10 @@ int validate_headref(const char *path) > len = read_in_full(fd, buffer, sizeof(buffer)-1); > close(fd); > > + if (len < 0)

Re: [PATCH v2 3/7] prefer "!=" when checking read_in_full() result

Jeff King writes: > diff --git a/csum-file.c b/csum-file.c > index a172199e44..2adae04073 100644 > --- a/csum-file.c > +++ b/csum-file.c > @@ -19,7 +19,7 @@ static void flush(struct sha1file *f, const void *buf, > unsigned int count) > > if (ret < 0) >

Re: [PATCH v2 4/7] avoid looking at errno for short read_in_full() returns

Jeff King writes: > When a caller tries to read a particular set of bytes via > read_in_full(), there are three possible outcomes: > > 1. An error, in which case -1 is returned and errno is > set. > > 2. A short read, in which fewer bytes are returned and > errno is

Re: [PATCH v2 0/7] read/write_in_full leftovers

Jeff King writes: > I dropped the "read_in_full() should set errno on short reads" idea (3/7 > in the earlier series). It really is the caller's fault for looking at > errno when they know there hasn't been an error in the first place. We > should just bite the bullet and have the

[PATCH v2] git: add --no-optional-locks option

This is an update of my --no-optional-locks patch. The only difference is that I updated the documentation to be a bit less clunky (Thanks, Kaartic). For the other comments on v1, I decided not to make any changes: - JSixt suggested marking this as a local-repo variable. I think we really

Re: [PATCH] git: add --no-optional-locks option

On Mon, Sep 25, 2017 at 11:51:31AM -0700, Stefan Beller wrote: > > diff --git a/read-cache.c b/read-cache.c > > index 65f4fe8375..fc1ba122a3 100644 > > --- a/read-cache.c > > +++ b/read-cache.c > > @@ -1563,7 +1563,8 @@ static int read_index_extension(struct index_state > > *istate, > > > > int

Re: [PATCH] git: add --no-optional-locks option

On Sun, Sep 24, 2017 at 01:08:41PM +0530, Kaartic Sivaraam wrote: > On Thursday 21 September 2017 10:02 AM, Jeff King wrote: > > Some tools like IDEs or fancy editors may periodically run > > commands like "git status" in the background to keep track > > of the state of the repository. > > I

Re: [PATCH v2 3/9] protocol: introduce protocol extention mechanisms

> +extern enum protocol_version get_protocol_version_config(void); > +extern enum protocol_version determine_protocol_version_server(void); > +extern enum protocol_version determine_protocol_version_client(const char > *server_response); It would be cool to have some documentation here.

Re: [PATCH v2 7/9] connect: tell server that the client understands v1

Junio C Hamano writes: >> +# Client requested to use protocol v1 >> +grep "version=1" log && >> +# Server responded using protocol v1 >> +grep "clone< version 1" log > > This looked a bit strange to check "clone< version 1" for one > direction, but did not

Re: [PATCH v2 8/9] http: tell server that the client understands v1

Brandon Williams writes: > @@ -897,6 +898,21 @@ static void set_from_env(const char **var, const char > *envname) > *var = val; > } > > +static void protocol_http_header(void) > +{ > + if (get_protocol_version_config() > 0) { > + struct strbuf

Re: [PATCH v2 7/9] connect: tell server that the client understands v1

Brandon Williams writes: > Teach the connection logic to tell a serve that it understands protocol > v1. This is done in 2 different ways for the built in protocols. > > 1. git:// >A normal request is structured as "command path/to/repo\0host=..\0" >and due to a bug

[PATCH 3/3] validate_headref: use get_oid_hex for detached HEADs

If a candidate HEAD isn't a symref, we check that it contains a viable sha1. But in a post-sha1 world, we should be checking whether it has any plausible object-id. We can do that by switching to get_oid_hex(). Note that both before and after this patch, we only check for a plausible object id

[PATCH 0/3] validate_headref: avoid reading uninitialized bytes

While looking at read_in_full() callers for a nearby patch series, I noticed this fairly harmless bug. The first patch fixes it, and the other two do some cleanups on top. I'm tempted to say the whole thing ought to just use a strbuf. We typically only call this function once per program, and git

[PATCH 1/3] validate_headref: NUL-terminate HEAD buffer

When we are checking to see if we have a git repo, we peek into the HEAD file and see if it's a plausible symlink, symref, or detached HEAD. For the latter two, we read the contents with read_in_full(), which means they aren't NUL-terminated. The symref check is careful to respect the length we

[PATCH 2/3] validate_headref: use skip_prefix for symref parsing

Since the previous commit guarantees that our symref buffer is NUL-terminated, we can just use skip_prefix() and friends to parse it. This is shorter and saves us having to deal with magic numbers and keeping the "len" counter up to date. While we're at it, let's name the rather obscure "buf" to

[PATCH v2 7/7] worktree: check the result of read_in_full()

We try to read "len" bytes into a buffer and just assume that it happened correctly. In practice this should usually be the case, since we just stat'd the file to get the length. But we could be fooled by transient errors or by other processes racily truncating the file. Let's be more careful.

[PATCH v2 5/7] distinguish error versus short read from read_in_full()

Many callers of read_in_full() expect to see the exact number of bytes requested, but their error handling lumps together true read errors and short reads due to unexpected EOF. We can give more specific error messages by separating these cases (showing errno when appropriate, and otherwise

[PATCH v2 6/7] worktree: use xsize_t to access file size

To read the "gitdir" file into memory, we stat the file and allocate a buffer. But we store the size in an "int", which may be truncated. We should use a size_t and xsize_t(), which will detect truncation. An overflow is unlikely for a "gitdir" file, but it's a good practice to model.

[PATCH v2 4/7] avoid looking at errno for short read_in_full() returns

When a caller tries to read a particular set of bytes via read_in_full(), there are three possible outcomes: 1. An error, in which case -1 is returned and errno is set. 2. A short read, in which fewer bytes are returned and errno is unspecified (we never saw a read error, so we

[PATCH v2 3/7] prefer "!=" when checking read_in_full() result

Comparing the result of read_in_full() using less-than is potentially dangerous, as a negative return value may be converted to an unsigned type and be considered a success. This is discussed further in 561598cfcf (read_pack_header: handle signed/unsigned comparison in read result, 2017-09-13).

[PATCH v2 2/7] notes-merge: drop dead zero-write code

We call write_in_full() with a size that we know is greater than zero. The return value can never be zero, then, since write_in_full() converts such a failed write() into ENOSPC and returns -1. We can just drop this branch of the error handling entirely. Suggested-by: Jonathan Nieder

[PATCH v2 1/7] files-backend: prefer "0" for write_in_full() error check

Commit 06f46f237a (avoid "write_in_full(fd, buf, len) != len" pattern, 2017-09-13) converted this callsite from: write_in_full(...) != 1 to write_in_full(...) < 0 But during the conflict resolution in c50424a6f0 (Merge branch 'jk/write-in-full-fix', 2017-09-25), this morphed into