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

2017-09-25 Thread Jeff King
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

macOS Git installer on SourceForge

2017-09-25 Thread Lars Schneider
Hi, one of my colleagues made me aware today that the macOS Git installer is hosted on SourceForge and advertised on the official git-scm page: https://git-scm.com/download/mac SourceForge had some bad press as they apparently bundled junkware in their downloads:

Re: [PATCH 4/7] get-tar-commit-id: prefer "!=" for read_in_full() error check

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: > Suggested-by: Jonathan Nieder > Signed-off-by: Jeff King > --- > builtin/get-tar-commit-id.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Jonathan Nieder

Re: [PATCH 5/7] worktree: use xsize_t to access file size

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: > 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 >

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: > What do you think of patch 7 in light of this? If the short-read case > gives us a sane errno, should we even bother trying to consistently > handle its error separately? I like the read_exactly_or_die variant because it makes callers more concise, but on the other hand a

[PATCH 0/7] read/write_in_full leftovers

2017-09-25 Thread Jeff King
This series addresses the bits leftover from the discussion two weeks ago in: https://public-inbox.org/git/20170913170807.cyx7rrpoyhaau...@sigill.intra.peff.net/ and its subthread. I don't think any of these is a real problem in practice, so this can be considered as a cleanup. I'm on the

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

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: > 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',

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

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: > 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. > >

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jeff King
On Mon, Sep 25, 2017 at 03:09:14PM -0700, Jonathan Nieder wrote: > Jeff King wrote: > > > In an ideal world, callers would always distinguish between > > these cases and give a useful message for each. But as an > > easy way to make our imperfect world better, let's reset > > errno to a known

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jeff King
On Mon, Sep 25, 2017 at 07:37:32PM -0400, Jeff King wrote: > > Correct. Actually more than "frown on": except for with the few calls > > like strtoul that are advertised to work this way, POSIX does not make > > the guarantee the above code would rely on, at all. > > > > So it's not just

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: > [EOVERFLOW] > The file is a regular file, nbyte is greater than 0, the starting > position is before the end-of-file, and the starting position is > greater than or equal to the offset maximum established in the open > file description associated with fildes.

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jeff King
On Mon, Sep 25, 2017 at 05:06:58PM -0700, Stefan Beller wrote: > >> If you are okay with the too-long/too-short confusion in EOVERFLOW, then > >> there is EMSGSIZE: > >> > >> Message too long > > > > I don't really like that one either. > > > > I actually prefer "0" of the all of the

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jonathan Nieder
On Mon, Sep 25, 2017 at 08:09:13PM -0400, Jeff King wrote: > On Mon, Sep 25, 2017 at 05:06:58PM -0700, Stefan Beller wrote: >> After reading this discussion from the sideline, maybe >> >> ENODATA No message is available on the STREAM head read >> queue (POSIX.1) >> >> is not too bad

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jonathan Nieder
Jonathan Nieder wrote: > On Mon, Sep 25, 2017 at 08:09:13PM -0400, Jeff King wrote: >> ENODATA is not too bad. On my glibc system it yields "No data available" >> from strerror(), which is at least comprehensible. >> >> We're still left with the question of whether it is defined everywhere >>

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jeff King
On Mon, Sep 25, 2017 at 05:13:54PM -0700, Jonathan Nieder wrote: > > I actually prefer "0" of the all of the options discussed. At least it > > is immediately clear that it is not a syscall error. > > I have a basic aversion to ": Success" in error messages. Whenever I > see such an error

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: > On Mon, Sep 25, 2017 at 05:17:32PM -0700, Jonathan Nieder wrote: >> #ifndef EUNDERFLOW >> # ifdef ENODATA >> # define EUNDERFLOW ENODATA >> # else >> # define EUNDERFLOW ESPIPE >> # endif >> #endif > > Right, I think our mails just crossed but I'm leaning in this direction. >

Re: [PATCH] Documentation: consolidate submodule..update

2017-09-25 Thread Stefan Beller
On Mon, Sep 25, 2017 at 12:17 PM, Brandon Williams wrote: > On 09/25, Stefan Beller wrote: >> Have one place to explain the effects of setting submodule..update >> instead of two. >> >> Signed-off-by: Stefan Beller >> --- >> >> I disagree. Actually, I

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: >> --- a/wrapper.c >> +++ b/wrapper.c >> @@ -318,8 +318,10 @@ ssize_t read_in_full(int fd, void *buf, size_t count) >> ssize_t loaded = xread(fd, p, count); >> if (loaded < 0) >> return -1; >> -if (loaded == 0) >> +

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jeff King
On Mon, Sep 25, 2017 at 05:20:20PM -0700, Jonathan Nieder wrote: > > What do you think of ENODATA? The glibc text for it is pretty > > appropriate. If it's not available everywhere, we'd have to fallback to > > something else (EIO? 0?). I don't know how esoteric it is. The likely > > candidate to

Re: [PATCH] Documentation: consolidate submodule..update

2017-09-25 Thread Stefan Beller
On Mon, Sep 25, 2017 at 3:22 PM, Jonathan Nieder wrote: > Stefan Beller wrote: >> On Mon, Sep 25, 2017 at 12:17 PM, Brandon Williams wrote: >>> On 09/25, Stefan Beller wrote: > Have one place to explain the effects of setting submodule..update

[PATCH] t7406: submodule..update command must not be run from .gitmodules

2017-09-25 Thread Stefan Beller
submodule..update can be assigned an arbitrary command via setting it to "!command". When this command is found in the regular config, Git ought to just run that command instead of other update mechanisms. However if that command is just found in the .gitmodules file, it is potentially untrusted,

Re: [PATCH 7/7] add xread_in_full() helper

2017-09-25 Thread Jeff King
On Mon, Sep 25, 2017 at 03:16:08PM -0700, Jonathan Nieder wrote: > Jeff King wrote: > > > Many callers of read_in_full() expect to see exactly "len" > > bytes, and die if that isn't the case. > > micronit: Can this be named read_in_full_or_die? > > Otherwise it's too easy to mistake for a

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: > On Mon, Sep 25, 2017 at 04:33:16PM -0700, Jonathan Nieder wrote: >> Jef King wrote: >>> errno = 0; >>> read_in_full(fd, buf, sizeof(buf)); >>> if (errno) >>> die_errno("oops"); [...] >>> in general we frown on it for calls like >>> read(). >>

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Stefan Beller
On Mon, Sep 25, 2017 at 5:01 PM, Jeff King wrote: > >> If you are okay with the too-long/too-short confusion in EOVERFLOW, then >> there is EMSGSIZE: >> >> Message too long > > I don't really like that one either. > > I actually prefer "0" of the all of the options

[PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jeff King
Many callers of read_in_full() complain when we do not read their full byte-count. But a check like: if (read_in_full(fd, buf, len) != len) return error_errno("unable to read"); conflates two problem conditions: 1. A real error from read(). 2. There were fewer than "len" bytes

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

2017-09-25 Thread Jeff King
On Mon, Sep 25, 2017 at 03:14:26PM -0700, Jonathan Nieder wrote: > > diff --git a/builtin/worktree.c b/builtin/worktree.c > > index 2f4a4ef9cd..87b3d70b0b 100644 > > --- a/builtin/worktree.c > > +++ b/builtin/worktree.c > > @@ -59,7 +59,11 @@ static int prune_worktree(const char *id, struct

Re: [PATCH] t7406: submodule..update command must not be run from .gitmodules

2017-09-25 Thread Jonathan Nieder
Stefan Beller wrote: > submodule..update can be assigned an arbitrary command via setting > it to "!command". When this command is found in the regular config, Git > ought to just run that command instead of other update mechanisms. > > However if that command is just found in the .gitmodules

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jeff King
On Mon, Sep 25, 2017 at 04:55:10PM -0700, Jonathan Nieder wrote: > Jeff King wrote: > > > [EOVERFLOW] > > The file is a regular file, nbyte is greater than 0, the starting > > position is before the end-of-file, and the starting position is > > greater than or equal to the offset

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: > On Mon, Sep 25, 2017 at 04:55:10PM -0700, Jonathan Nieder wrote: >> All that really matters is the strerror() that the user would see. > > Agreed, though I didn't think those were necessarily standardized. The standard allows them to vary for the sake of internationalization,

Re: [PATCH 7/7] add xread_in_full() helper

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: > Many callers of read_in_full() expect to see exactly "len" > bytes, and die if that isn't the case. micronit: Can this be named read_in_full_or_die? Otherwise it's too easy to mistake for a function like xread, which has different semantics. I realize that xmalloc, xmemdupz,

Re: [PATCH v2 4/5] sha1_name: Parse less while finding common prefix

2017-09-25 Thread Stefan Beller
On Mon, Sep 25, 2017 at 2:54 AM, Derrick Stolee wrote: > Create get_hex_char_from_oid() to parse oids one hex character at a > time. This prevents unnecessary copying of hex characters in > extend_abbrev_len() when finding the length of a common prefix. > > p0008.1:

Re: [PATCH] Documentation: consolidate submodule..update

2017-09-25 Thread Jonathan Nieder
Stefan Beller wrote: > By as-is I refer to origin/pu. Ah, okay. I'm not in that habit because pu frequently loses topics --- it's mostly useful as a tool to (1) distribute topic branches and (2) check whether they are compatible with each other. [...] > On Mon, Sep 25, 2017 at 3:22 PM,

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

2017-09-25 Thread Jeff King
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

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: > In an ideal world, callers would always distinguish between > these cases and give a useful message for each. But as an > easy way to make our imperfect world better, let's reset > errno to a known value. The best we can do is "0", which > will yield something like: > >

Re: [PATCH] Documentation: consolidate submodule..update

2017-09-25 Thread Jonathan Nieder
Stefan Beller wrote: > On Mon, Sep 25, 2017 at 12:17 PM, Brandon Williams wrote: >> On 09/25, Stefan Beller wrote: >>> Have one place to explain the effects of setting submodule..update >>> instead of two. >>> >>> Signed-off-by: Stefan Beller >>> --- >

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jeff King
On Mon, Sep 25, 2017 at 05:17:32PM -0700, Jonathan Nieder wrote: > Jonathan Nieder wrote: > > On Mon, Sep 25, 2017 at 08:09:13PM -0400, Jeff King wrote: > > >> ENODATA is not too bad. On my glibc system it yields "No data available" > >> from strerror(), which is at least comprehensible. > >> >

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Jonathan Nieder
Jeff King wrote: > I definitely would prefer to avoid EIO, or anything that an actual > read() might return. > > What do you think of ENODATA? The glibc text for it is pretty > appropriate. If it's not available everywhere, we'd have to fallback to > something else (EIO? 0?). I don't know how

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

2017-09-25 Thread Junio C Hamano
Han-Wen Nienhuys writes: > This mirrors commit bdfdaa4978dd92992737e662f25adc01d32d0774 which did > the same for strbuf.h: > > * API documentation uses /** */ to set it apart from other comments. > > * Function names were stripped from the comments. > > * Ordering of the

Re: [PATCH] t7406: submodule..update command must not be run from .gitmodules

2017-09-25 Thread Johannes Sixt
Am 26.09.2017 um 00:50 schrieb Stefan Beller: submodule..update can be assigned an arbitrary command via setting it to "!command". When this command is found in the regular config, Git ought to just run that command instead of other update mechanisms. However if that command is just found in

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

2017-09-25 Thread Eric Rannaud
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: [PATCH 4/4] Move documentation of string_list into string-list.h

2017-09-25 Thread Junio C Hamano
Junio C Hamano writes: > > Thanks. I am not sure if you can safely reorder the contents of the > header files in general, but I trust that you made sure that this > does not introduce problems (e.g. referrals before definition). Alas, this time it seems my trust was grossly

Re: [PATCH 3/7] read_in_full: reset errno before reading

2017-09-25 Thread Junio C Hamano
Jeff King writes: >> #ifndef EUNDERFLOW >> # ifdef ENODATA >> # define EUNDERFLOW ENODATA >> # else >> # define EUNDERFLOW ESPIPE >> # endif >> #endif > > Right, I think our mails just crossed but I'm leaning in this direction. Hmph, I may be slow (or may be skimming the

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

2017-09-25 Thread Junio C Hamano
"Eric Rannaud" writes: > 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

Re: [PATCH 2/4] Clarify return value ownership of real_path and read_gitfile_gently

2017-09-25 Thread Junio C Hamano
Han-Wen Nienhuys writes: >>Subject: Re: [PATCH 2/4] Clarify return value ownership of real_path and >>read_gitfile_gently We try to make "shortlog --no-merges" output consistently say what area the change is about, followed by a colon, followed by a short description. >

Re: -X theirs does not resolve symlink conflict Was: BUG: merge -s theirs is not in effect

2017-09-25 Thread Junio C Hamano
Yaroslav Halchenko writes: > yes it does. Thanks. And that is where I realized that I should have used -X > theirs (not -s theirs), as the instruction on the option for the > (recursive) merge. And now problem is more specific: > > - conflict within file content editing

Re: -X theirs does not resolve symlink conflict Was: BUG: merge -s theirs is not in effect

2017-09-25 Thread Junio C Hamano
Junio C Hamano writes: > Junio C Hamano writes: > >> I do not recall people talking about symbolic links but the case of >> binary files has been on the wishlist for a long time, and I do not >> know of anybody who is working on (or is planning to work on)

Re: '--shallow-since' option is not available for git-pull in Git version 2.14.1

2017-09-25 Thread Kevin Daudt
On Mon, Sep 25, 2017 at 04:31:10PM +0900, Sanggyu Nam wrote: > I’ve found that some subcommands such as git-clone, git-fetch, and > git-pull support an option named ‘--shallow-since’, as of Git version > 2.11. This option is documented in the man page of each subcommand. In > Git 2.14.1, I’ve

-X theirs does not resolve symlink conflict Was: BUG: merge -s theirs is not in effect

2017-09-25 Thread Yaroslav Halchenko
On Mon, 25 Sep 2017, Junio C Hamano wrote: > Yaroslav Halchenko writes: > > d'oh, indeed there is no git-merge-theirs neither in debian pkg or a > > freshly > > built git and I found a rogue script in the PATH (which did nothing > > apparently, sorry!). BUT I was

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

2017-09-25 Thread Yaroslav Halchenko
On Mon, 25 Sep 2017, Junio C Hamano wrote: >It is a different matter to resurrect the age old discussion that >happend in the summer of 2008 if '-s theirs' should or should not >exist. In short, the previous discussion can be summarised to >"we don't want '-s theirs' as it

[PATCH v3 05/21] read_packed_refs(): use mmap to read the `packed-refs` file

2017-09-25 Thread Michael Haggerty
It's still done in a pretty stupid way, involving more data copying than necessary. That will improve in future commits. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 42 -- 1 file changed, 32 insertions(+), 10

[PATCH v3 07/21] read_packed_refs(): make parsing of the header line more robust

2017-09-25 Thread Michael Haggerty
The old code parsed the traits in the `packed-refs` header by looking for the string " trait " (i.e., the name of the trait with a space on either side) in the header line. This is fragile, because if any other implementation of Git forgets to write the trailing space, the last trait would

[PATCH v3 03/21] packed_ref_cache: add a backlink to the associated `packed_ref_store`

2017-09-25 Thread Michael Haggerty
It will prove convenient in upcoming patches. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index e411501871..a3d9210cb0

[PATCH v3 15/21] packed_ref_iterator_begin(): iterate using `mmapped_ref_iterator`

2017-09-25 Thread Michael Haggerty
Now that we have an efficient way to iterate, in order, over the mmapped contents of the `packed-refs` file, we can use that directly to implement reference iteration for the `packed_ref_store`, rather than iterating over the `ref_cache`. This is the next step towards getting rid of the

[PATCH v3 19/21] ref_cache: remove support for storing peeled values

2017-09-25 Thread Michael Haggerty
Now that the `packed-refs` backend doesn't use `ref_cache`, there is nobody left who might want to store peeled values of references in `ref_cache`. So remove that feature. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 9 - refs/ref-cache.c | 42

[PATCH v3 21/21] packed-backend.c: rename a bunch of things and update comments

2017-09-25 Thread Michael Haggerty
We've made huge changes to this file, and some of the old names and comments are no longer very fitting. So rename a bunch of things: * `struct packed_ref_cache` → `struct snapshot` * `acquire_packed_ref_cache()` → `acquire_snapshot()` * `release_packed_ref_buffer()` → `clear_snapshot_buffer()` *

[PATCH v3 20/21] mmapped_ref_iterator: inline into `packed_ref_iterator`

2017-09-25 Thread Michael Haggerty
Since `packed_ref_iterator` is now delegating to `mmapped_ref_iterator` rather than `cache_ref_iterator` to do the heavy lifting, there is no need to keep the two iterators separate. So "inline" `mmapped_ref_iterator` into `packed_ref_iterator`. This removes a bunch of boilerplate. Signed-off-by:

[PATCH v3 16/21] packed_read_raw_ref(): read the reference from the mmapped buffer

2017-09-25 Thread Michael Haggerty
Instead of reading the reference from the `ref_cache`, read it directly from the mmapped buffer. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/refs/packed-backend.c

[PATCH v3 10/21] mmapped_ref_iterator: add iterator over a packed-refs file

2017-09-25 Thread Michael Haggerty
Add a new `mmapped_ref_iterator`, which can iterate over the references in an mmapped `packed-refs` file directly. Use this iterator from `read_packed_refs()` to fill the packed refs cache. Note that we are not yet willing to promise that the new iterator generates its output in order. That

[PATCH v3 12/21] packed-backend.c: reorder some definitions

2017-09-25 Thread Michael Haggerty
No code has been changed. This will make subsequent patches more self-contained. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 48 1 file changed, 24 insertions(+), 24 deletions(-) diff --git

[PATCH v3 17/21] ref_store: implement `refs_peel_ref()` generically

2017-09-25 Thread Michael Haggerty
We're about to stop storing packed refs in a `ref_cache`. That means that the only way we have left to optimize `peel_ref()` is by checking whether the reference being peeled is the one currently being iterated over (in `current_ref_iter`), and if so, using `ref_iterator_peel()`. But this can be

[PATCH v3 13/21] packed_ref_cache: keep the `packed-refs` file mmapped if possible

2017-09-25 Thread Michael Haggerty
Keep a copy of the `packed-refs` file contents in memory for as long as a `packed_ref_cache` object is in use: * If the system allows it, keep the `packed-refs` file mmapped. * If not (either because the system doesn't support `mmap()` at all, or because a file that is currently mmapped cannot

[PATCH v3 08/21] read_packed_refs(): read references with minimal copying

2017-09-25 Thread Michael Haggerty
Instead of copying data from the `packed-refs` file one line at time and then processing it, process the data in place as much as possible. Also, instead of processing one line per iteration of the main loop, process a reference line plus its corresponding peeled line (if present) together. Note

[PATCH v3 14/21] read_packed_refs(): ensure that references are ordered when read

2017-09-25 Thread Michael Haggerty
It doesn't actually matter now, because the references are only iterated over to fill the associated `ref_cache`, which itself puts them in the correct order. But we want to get rid of the `ref_cache`, so we want to be able to iterate directly over the `packed-refs` buffer, and then the iteration

[PATCH v3 11/21] mmapped_ref_iterator_advance(): no peeled value for broken refs

2017-09-25 Thread Michael Haggerty
If a reference is broken, suppress its peeled value. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 312116a99d..724c88631d 100644

[PATCH v3 09/21] packed_ref_cache: remember the file-wide peeling state

2017-09-25 Thread Michael Haggerty
Rather than store the peeling state (i.e., the one defined by traits in the `packed-refs` file header line) in a local variable in `read_packed_refs()`, store it permanently in `packed_ref_cache`. This will be needed when we stop reading all packed refs at once. Signed-off-by: Michael Haggerty

[PATCH v3 01/21] ref_iterator: keep track of whether the iterator output is ordered

2017-09-25 Thread Michael Haggerty
References are iterated over in order by refname, but reflogs are not. Some consumers of reference iteration care about the difference. Teach each `ref_iterator` to keep track of whether its output is ordered. `overlay_ref_iterator` is one of the picky consumers. Add a sanity check in

[PATCH v3 02/21] prefix_ref_iterator: break when we leave the prefix

2017-09-25 Thread Michael Haggerty
From: Jeff King If the underlying iterator is ordered, then `prefix_ref_iterator` can stop as soon as it sees a refname that comes after the prefix. This will rarely make a big difference now, because `ref_cache_iterator` only iterates over the directory containing the prefix (and

[PATCH v3 04/21] die_unterminated_line(), die_invalid_line(): new functions

2017-09-25 Thread Michael Haggerty
Extract some helper functions for reporting errors. While we're at it, prevent them from spewing unlimited output to the terminal. These functions will soon have more callers. These functions accept the problematic line as a `(ptr, len)` pair rather than a NUL-terminated string, and

[PATCH v3 06/21] read_packed_refs(): only check for a header at the top of the file

2017-09-25 Thread Michael Haggerty
This tightens up the parsing a bit; previously, stray header-looking lines would have been processed. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 35 --- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git

[PATCH v3 00/21] Read `packed-refs` using mmap()

2017-09-25 Thread Michael Haggerty
This is v3 of a patch series that changes the reading and caching of the `packed-refs` file to use `mmap()`. Thanks to Stefan, Peff, Dscho, and Junio for their comments about v2. I think I have addressed all of the feedback from v1 [1] and v2 [2]. This version has only minor changes relative to

[PATCH v3 18/21] packed_ref_store: get rid of the `ref_cache` entirely

2017-09-25 Thread Michael Haggerty
Now that everything has been changed to read what it needs directly out of the `packed-refs` file, `packed_ref_store` doesn't need to maintain a `ref_cache` at all. So get rid of it. First of all, this will save a lot of memory and lots of little allocations. Instead of needing to store

[PATCHv2] docs: improve discoverability of exclude pathspec

2017-09-25 Thread Manav Rathi
On Mon, Sep 25, 2017 at 10:03:49AM +0900, Junio C Hamano wrote: > Manav Rathi writes: > > > The ability to exclude paths in pathspecs is not mentioned in the man > > pages of git grep and other commands where it might be useful. > > My reading stutters around "exclude paths in

[RFC PATCH v2 0/5] Give more useful error messages when renaming a branch

2017-09-25 Thread Kaartic Sivaraam
[1/5] and [2/5] - cleanup patches [3/5] - refactor of a function that seemed a little unintuitive [4/5] - just a preparatory step adding part of the logic [5/5] - main patch that tries to improve the error messages Changes in v2: - dropped [1/5] of v1 [1/5]

[RFC PATCH v2 3/5] branch: cleanup branch name validation

2017-09-25 Thread Kaartic Sivaraam
The function that validates a new branch name was clumsy because, 1. It did more than just validating the branch name. 2. Despite it's name, it is more often than not used to validate existing branch names through the 'force' and 'attr_only' parameters (whose names by the way

[RFC PATCH v2 2/5] branch: re-order function arguments to group related arguments

2017-09-25 Thread Kaartic Sivaraam
The ad-hoc patches to add new arguments to a function when needed resulted in the related arguments not being close to each other. This misleads the person reading the code to believe that there isn't much relation between those arguments while it's not the case. So, re-order the arguments to

[RFC PATCH v2 1/5] branch: improve documentation and naming of certain parameters

2017-09-25 Thread Kaartic Sivaraam
Documentation for a certain function was incomplete as it didn't say what certain parameters were used for. Further a parameter name wasn't very communicative. So, add missing documentation for the sake of completeness and easy reference. Also, rename the concerned parameter to make it's name

[RFC PATCH v2 5/5] builtin/branch: give more useful error messages when renaming

2017-09-25 Thread Kaartic Sivaraam
When trying to rename an inexistent branch to with a name of a branch that already exists the rename failed specifying the new branch name exists rather than specifying that the branch trying to be renamed doesn't exist. $ git branch -m tset master fatal: A branch named 'master' already

[RFC PATCH v2 4/5] branch: introduce dont_fail parameter for create validation

2017-09-25 Thread Kaartic Sivaraam
This parameter allows the branch create validation function to optionally return a flag specifying the reason for failure, when requested. This allows the caller to know why it was about to die. This allows more useful error messages to be given to the user when trying to rename a branch. The

[PATCH v3] doc: camelCase the config variables to improve readability

2017-09-25 Thread Kaartic Sivaraam
References to multi-word configuration variable names in our documentation must consistently use camelCase to highlight where the word boundaries are, even though these are treated case insensitively. Fix a few places that spell them in all lowercase, which makes them harder to read.

Re: [PATCH 3/4] merge: --no-verify to bypass pre-merge hook

2017-09-25 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 24.09.2017 01:48: > Michael J Gruber writes: > >> From: Michael J Gruber >> >> Analogous to commit, introduce a '--no-verify' option which bypasses the >> pre-merge hook. The shorthand '-n' is taken by the

[PATCH v2 0/1] Simplify loading of DLL functions on Windows

2017-09-25 Thread Johannes Schindelin
Changes since v1: - repeated the example from the commit message in the .h file - mentioned that the function is not thread-safe. - added Jonathan's Reviewed-by: footer Johannes Schindelin (1): Win32: simplify loading of DLL functions compat/win32/lazyload.h | 57

[PATCH v2 1/1] Win32: simplify loading of DLL functions

2017-09-25 Thread Johannes Schindelin
Dynamic loading of DLL functions is duplicated in several places in Git for Windows' source code. This patch adds a pair of macros to simplify the process: the DECLARE_PROC_ADDR(, , , ..) macro to be used at the beginning of a code block, and the INIT_PROC_ADDR() macro to call before using

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

2017-09-25 Thread Johannes Schindelin
Hi Peff, On Thu, 21 Sep 2017, Jeff King wrote: > Johannes, this is an adaptation of your 67e5ce7f63 (status: offer *not* > to lock the index and update it, 2016-08-12). Folks working on GitHub > Desktop complained to me that it's only available on Windows. :) Okay, so this is trying to help me

[PATCH 3/4] Document submodule_to_gitdir

2017-09-25 Thread Han-Wen Nienhuys
Signed-off-by: Han-Wen Nienhuys --- submodule.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/submodule.c b/submodule.c index b12600fc7..b66c23f5d 100644 --- a/submodule.c +++ b/submodule.c @@ -1997,6 +1997,9 @@ const char *get_superproject_working_tree(void)

[PATCH 2/4] Clarify return value ownership of real_path and read_gitfile_gently

2017-09-25 Thread Han-Wen Nienhuys
Signed-off-by: Han-Wen Nienhuys --- abspath.c | 3 +++ setup.c | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/abspath.c b/abspath.c index 708aff8d4..792a2d533 100644 --- a/abspath.c +++ b/abspath.c @@ -202,6 +202,9 @@ char *strbuf_realpath(struct

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

2017-09-25 Thread Han-Wen Nienhuys
This mirrors commit bdfdaa4978dd92992737e662f25adc01d32d0774 which did the same for strbuf.h: * API documentation uses /** */ to set it apart from other comments. * Function names were stripped from the comments. * Ordering of the header was adjusted to follow the one from the text file. *

[PATCH 0/4] Assorted comment fixes

2017-09-25 Thread Han-Wen Nienhuys
I followed Peff's advice for string-list.h comments. Han-Wen Nienhuys (4): Fix typo in submodule.h Clarify return value ownership of real_path and read_gitfile_gently Document submodule_to_gitdir Move documentation of string_list into string-list.h

[PATCH 1/4] Fix typo in submodule.h

2017-09-25 Thread Han-Wen Nienhuys
Signed-off-by: Han-Wen Nienhuys --- submodule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/submodule.h b/submodule.h index 6b52133c8..f0da0277a 100644 --- a/submodule.h +++ b/submodule.h @@ -120,7 +120,7 @@ extern int submodule_move_head(const char

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

2017-09-25 Thread Johannes Schindelin
Hi Kaartic, On Sun, 24 Sep 2017, 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 might be

Re: [PATCH v2 14/21] read_packed_refs(): ensure that references are ordered when read

2017-09-25 Thread Johannes Schindelin
Hi Michael, On Thu, 21 Sep 2017, Michael Haggerty wrote: > On 09/20/2017 08:50 PM, Jeff King wrote: > > On Tue, Sep 19, 2017 at 08:22:22AM +0200, Michael Haggerty wrote: > >> If `packed-refs` was already sorted, then (if the system allows it) > >> we can use the mmapped file contents directly.

Re: [PATCH v2 00/21] Read `packed-refs` using mmap()

2017-09-25 Thread Johannes Schindelin
Hi Peff, On Wed, 20 Sep 2017, Jeff King wrote: > On Tue, Sep 19, 2017 at 08:22:08AM +0200, Michael Haggerty wrote: > > > This branch is also available from my fork on GitHub as branch > > `mmap-packed-refs`. > > > > My main uncertainties are: > > > > 1. Does this code actually work on

Re: [PATCH v2 0/6] reroll ma/plugleaks; more `object_array`-fixes

2017-09-25 Thread Jeff King
On Sun, Sep 24, 2017 at 09:59:28PM +0200, Martin Ågren wrote: > > Anyway, doing: > > > > > > ASAN_OPTIONS=detect_leaks=1:abort_on_error=0:exitcode=0:log_path=/tmp/lsan/output > > \ > > make SANITIZE=address,leak test > > > > should pass the whole suite and give you a host of files to

VKTUR'un SİZE ÖZEL KAMPANYALARI DEVAM EDİYOR!!

2017-09-25 Thread vkt...@mynet.com

Re: [PATCH v2 1/1] Win32: simplify loading of DLL functions

2017-09-25 Thread Jonathan Nieder
Johannes Schindelin wrote: > Dynamic loading of DLL functions is duplicated in several places in Git > for Windows' source code. > > This patch adds a pair of macros to simplify the process: the > DECLARE_PROC_ADDR(, , , > ..) macro to be used at the beginning of a > code block, and the

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

2017-09-25 Thread Jeff King
On Mon, Sep 25, 2017 at 06:10:12PM +0200, Johannes Schindelin wrote: > On Thu, 21 Sep 2017, Jeff King wrote: > > > Johannes, this is an adaptation of your 67e5ce7f63 (status: offer *not* > > to lock the index and update it, 2016-08-12). Folks working on GitHub > > Desktop complained to me that

Re: [PATCH 2/4] Clarify return value ownership of real_path and read_gitfile_gently

2017-09-25 Thread Brandon Williams
On 09/25, Han-Wen Nienhuys wrote: > Signed-off-by: Han-Wen Nienhuys > --- > abspath.c | 3 +++ > setup.c | 3 ++- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/abspath.c b/abspath.c > index 708aff8d4..792a2d533 100644 > --- a/abspath.c > +++ b/abspath.c

Re: [PATCH 3/4] Document submodule_to_gitdir

2017-09-25 Thread Brandon Williams
On 09/25, Han-Wen Nienhuys wrote: > Signed-off-by: Han-Wen Nienhuys > --- > submodule.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/submodule.c b/submodule.c > index b12600fc7..b66c23f5d 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -1997,6 +1997,9 @@

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

2017-09-25 Thread Brandon Williams
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 (strbuf.h: integrate api-strbuf.txt documentation,

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

2017-09-25 Thread Stefan Beller
>> There might be another option to cope with the situation: >> >> 4. Teach all commands to spinlock / busywait shortly for important >> locks instead of giving up. In that case git-status rewriting >> the index ought to be fine? > > We do have all the infrastructure in place to do a

[PATCH] Documentation: consolidate submodule..update

2017-09-25 Thread Stefan Beller
Have one place to explain the effects of setting submodule..update instead of two. Signed-off-by: Stefan Beller --- >> I disagree. Actually, I think the git-config(1) blurb could just >> point here, and that the text here ought to be clear about what >> commands it affects

  1   2   >