Re: [PATCH] pack-objects: warn on split packs disabling bitmaps

2016-04-27 Thread Jeff King
On Wed, Apr 27, 2016 at 09:53:24PM +, Eric Wong wrote: > It can be tempting for a server admin to want a stable set of > long-lived packs for dumb clients; but also want to enable > bitmaps to serve smart clients more quickly. > > Unfortunately, such a configuration is impossible; > so at

Re: [PATCH] pack-objects: warn on split packs disabling bitmaps

2016-04-27 Thread Jeff King
On Wed, Apr 27, 2016 at 03:56:46PM -0700, Junio C Hamano wrote: > Eric Wong writes: > > > It can be tempting for a server admin to want a stable set of > > long-lived packs for dumb clients; but also want to enable > > bitmaps to serve smart clients more quickly. > > > >

Re: [PATCH 01/15] string_list: add string_list_duplicate

2016-04-27 Thread Stefan Beller
On Wed, Apr 27, 2016 at 4:17 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> Another way to corrupt it is to change the configuration (e.g. add >> things to the config hashmap such that it reallocates and grows). > > You're right. But doesn't it

Re: [PATCH 01/15] string_list: add string_list_duplicate

2016-04-27 Thread Junio C Hamano
Stefan Beller writes: > Another way to corrupt it is to change the configuration (e.g. add > things to the config hashmap such that it reallocates and grows). You're right. But doesn't it hint that there is a deeper problem? By making a copy and keeping it, you would hold

Re: [PATCH 05/15] submodule-config: check if submodule a submodule is in a group

2016-04-27 Thread Stefan Beller
On Tue, Apr 26, 2016 at 4:17 PM, Junio C Hamano wrote: > Junio C Hamano writes: > >> I see room for bikeshedding here, but the material to bikeshed >> around is not even documented yet ;-) >> >> * a token prefixed with '*' is a label. >> * a token prefixed

Re: [PATCH] pack-objects: warn on split packs disabling bitmaps

2016-04-27 Thread Junio C Hamano
Eric Wong writes: > It can be tempting for a server admin to want a stable set of > long-lived packs for dumb clients; but also want to enable > bitmaps to serve smart clients more quickly. > > Unfortunately, such a configuration is impossible; > so at least warn users of

Announcing git-cinnabar 0.3.2

2016-04-27 Thread Mike Hommey
Git-cinnabar is a git remote helper to interact with mercurial repositories. It allows to clone, pull and push from/to mercurial remote repositories, using git. Code on https://github.com/glandium/git-cinnabar [ Previous announcements: http://marc.info/?l=git=145294370431454

Re: make test Unexpected passes

2016-04-27 Thread Elijah Newren
On Wed, Apr 27, 2016 at 3:05 PM, Junio C Hamano wrote: > Isn't what the test expects bogus in the first place? I'd suggest > removing the test as "pointless waste of resource". > > Comments? > > -- >8 -- Yes, toss it; I find your arguments below compelling. > Manual merge

Re: Bisect limited to Merge Commits

2016-04-27 Thread Junio C Hamano
Johannes Sixt writes: > Am 27.04.2016 um 22:56 schrieb Junio C Hamano: >> So being able to stop at only commits on the first-parent chain is a >> valid and useful tool. "git bisect --first-parent" is one of the >> things that are sometimes asked for. > > With origin pointing to

Re: [PATCH 05/29] refname_is_safe(): insist that the refname already be normalized

2016-04-27 Thread Jeff King
On Wed, Apr 27, 2016 at 04:37:28PM -0400, Jeff King wrote: > > > But anything writing a _new_ refname (whether the actual ref, or > > > referencing it via a symref) should be using check_refname_format() > > > before writing. > > > > Unfortunately, neither check is lesser -- refname_is_safe

Re: make test Unexpected passes

2016-04-27 Thread Junio C Hamano
Elijah Newren writes: > Yeah, the t6036 testcase 'git detects conflict w/ > criss-cross+contrived resolution' could be made to pass by tweaking > the conflict markers. In fact, any tweak would make it appear to > pass, but the test could be updated to still fail by updating

[PATCH] pack-objects: warn on split packs disabling bitmaps

2016-04-27 Thread Eric Wong
It can be tempting for a server admin to want a stable set of long-lived packs for dumb clients; but also want to enable bitmaps to serve smart clients more quickly. Unfortunately, such a configuration is impossible; so at least warn users of this incompatibility since commit

Re: Bisect limited to Merge Commits

2016-04-27 Thread Johannes Sixt
Am 27.04.2016 um 22:56 schrieb Junio C Hamano: So being able to stop at only commits on the first-parent chain is a valid and useful tool. "git bisect --first-parent" is one of the things that are sometimes asked for. With origin pointing to git.git, I attempted this: git bisect start git

[PATCH 1/2] config.c: drop local variable

2016-04-27 Thread Stefan Beller
As `ret` is not used for anything except determining an early return, we don't need a variable for that. Drop it. Signed-off-by: Stefan Beller --- While reading config code, I found 2 nits. Both improve readability, no bugfix or feature. As it is generally discouraged to

[PATCH 2/2] submodule-config: don't shadow `cache`

2016-04-27 Thread Stefan Beller
Lots of internal functions in submodule-confic.c have a first parameter `struct submodule_cache *cache`, which currently always refers to the global variable `cache` in the file. To avoid confusion rename the global `cache` variable. Signed-off-by: Stefan Beller ---

Re: [PATCH 01/15] string_list: add string_list_duplicate

2016-04-27 Thread Stefan Beller
On Wed, Apr 27, 2016 at 2:11 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> When not duplicating git_config_get_value_multi("submodule.defaultGroup"); >> I run into >> >> Program received signal SIGSEGV, Segmentation fault. >> ... >> So the string

Re: [PATCH 15/29] ref_transaction_create(): disallow recursive pruning

2016-04-27 Thread Junio C Hamano
Junio C Hamano writes: > If a casual reader sees this code: > > ref_transaction_delete(transaction, r->name, r->sha1, > REF_ISPRUNING | REF_NODEREF, NULL, ) > > it gives an incorrect impression that there may also be a valid case > to make a

Re: [PATCH 01/15] string_list: add string_list_duplicate

2016-04-27 Thread Junio C Hamano
Stefan Beller writes: > When not duplicating git_config_get_value_multi("submodule.defaultGroup"); > I run into > > Program received signal SIGSEGV, Segmentation fault. > ... > So the string list seems to be corrupted here. > Someone stomping over our memory? How long is the

Re: [PATCH v4] http: support sending custom HTTP headers

2016-04-27 Thread Junio C Hamano
Jeff King writes: > On Wed, Apr 27, 2016 at 02:20:37PM +0200, Johannes Schindelin wrote: > >> The only change vs v3 is that I replaced my flimsical test by Peff's (with >> *one* change: I realized that we need to group the Require statements in a >> block when I tried to verify

Re: Bisect limited to Merge Commits

2016-04-27 Thread Junio C Hamano
Hagen Paul Pfeifer writes: > Imagine a "rebase feature branch" style of development. All features are > developed on separate features branch which are rebased on master and > immediately merged into the upstream master. I do not want to imagine such ;-) The only semi-sensible

Re: BUG on OSX `git p4 submit` can fail when the workspace root doesn't exist locally.

2016-04-27 Thread Stefan Beller
On Wed, Apr 27, 2016 at 11:06 AM, Jacob Smith wrote: > I debugged the issue using the script here: > https://github.com/git/git/blob/master/git-p4.py > I'm not sure how close to the main repo that is. > > On Wed, Apr 27, 2016 at 11:28 AM, Stefan Beller

Bisect limited to Merge Commits

2016-04-27 Thread Hagen Paul Pfeifer
Hey, are there any plans to add an option to skip all non-merge commits via bisecting? git bisect start --merges-only Imagine a "rebase feature branch" style of development. All features are developed on separate features branch which are rebased on master and immediately merged into the

Re: [PATCH 08/29] ref_transaction_commit(): remove local variable n

2016-04-27 Thread Junio C Hamano
Junio C Hamano writes: > I expect that somewhere in this series transaction->nr will not stay s/series/& it is documented that/ > constant even if the client code of ref-transaction API makes no > direct call that adds a new update[] element, though, even if it is > not done

Re: [PATCH 15/29] ref_transaction_create(): disallow recursive pruning

2016-04-27 Thread Junio C Hamano
David Turner writes: > On Wed, 2016-04-27 at 11:47 -0700, Junio C Hamano wrote: >> Michael Haggerty writes: >> >> > It is nonsensical (and a little bit dangerous) to use REF_ISPRUNING >> > without REF_NODEREF. Forbid it explicitly. Change the one

Re: [PATCH 05/29] refname_is_safe(): insist that the refname already be normalized

2016-04-27 Thread Jeff King
On Wed, Apr 27, 2016 at 04:34:53PM -0400, David Turner wrote: > > I thought the point is that one is a lesser check than the other, and > > we > > need different rules for different situations. So we might allow > > deletion on a refname that does not pass check_refname_format(), but > > we > >

Re: [PATCH 05/29] refname_is_safe(): insist that the refname already be normalized

2016-04-27 Thread David Turner
On Wed, 2016-04-27 at 16:15 -0400, Jeff King wrote: > On Wed, Apr 27, 2016 at 04:10:32PM -0400, David Turner wrote: > > > On Wed, 2016-04-27 at 10:59 -0700, Junio C Hamano wrote: > > > > > There is another call to refname_is_safe() in > > > resolve_ref_unsafe(), > > > which applies the sanity

Re: [PATCH] trailer: load config to handle core.commentChar

2016-04-27 Thread Eric Sunshine
On Wed, Apr 27, 2016 at 4:31 PM, Junio C Hamano wrote: >> On Wed, Apr 27, 2016 at 9:24 PM, Rafal Klys wrote: >>> Add call to git_config(git_default_config, NULL) to update the >>> comment_char_line from default '#' to possible different value set in >>>

Re: [PATCH] trailer: load config to handle core.commentChar

2016-04-27 Thread Junio C Hamano
Christian Couder writes: > On Wed, Apr 27, 2016 at 9:24 PM, Rafal Klys wrote: >> Add call to git_config(git_default_config, NULL) to update the >> comment_char_line from default '#' to possible different value set in >> core.commentChar. > > It is

Re: [PATCH] Makefile: remove dependency on git.spec

2016-04-27 Thread Junio C Hamano
Dennis Kaarsemaker writes: > ab21433 dropped support for rpmbuild using our own specfile by removing > git.spec.in, but forgot to remove the dependency of dist on git.spec. > > Signed-off-by: Dennis Kaarsemaker > --- Thanks. > Makefile | 2 +- >

Re: [PATCH 15/29] ref_transaction_create(): disallow recursive pruning

2016-04-27 Thread David Turner
On Wed, 2016-04-27 at 11:47 -0700, Junio C Hamano wrote: > Michael Haggerty writes: > > > It is nonsensical (and a little bit dangerous) to use REF_ISPRUNING > > without REF_NODEREF. Forbid it explicitly. Change the one > > REF_ISPRUNING > > caller to pass REF_NODEREF too.

Re: [PATCH 05/29] refname_is_safe(): insist that the refname already be normalized

2016-04-27 Thread Jeff King
On Wed, Apr 27, 2016 at 04:10:32PM -0400, David Turner wrote: > On Wed, 2016-04-27 at 10:59 -0700, Junio C Hamano wrote: > > > There is another call to refname_is_safe() in resolve_ref_unsafe(), > > which applies the sanity check to the string from a symref. We seem > > to allow > > > > $

Re: [PATCH 24/29] ref_transaction_update(): check refname_is_safe() at a minimum

2016-04-27 Thread Junio C Hamano
Michael Haggerty writes: > If the user has asked that a new value be set for a reference, we use > check_refname_format() to verify that the reference name satisfies all > of the rules. But in other cases, at least check that refname_is_safe(). It isn't clear to me what

Re: [PATCH] trailer: load config to handle core.commentChar

2016-04-27 Thread Christian Couder
On Wed, Apr 27, 2016 at 9:24 PM, Rafal Klys wrote: > Add call to git_config(git_default_config, NULL) to update the > comment_char_line from default '#' to possible different value set in > core.commentChar. It is "comment_line_char" not "comment_char_line", but otherwise you

Re: [PATCH 05/29] refname_is_safe(): insist that the refname already be normalized

2016-04-27 Thread David Turner
On Wed, 2016-04-27 at 10:59 -0700, Junio C Hamano wrote: > There is another call to refname_is_safe() in resolve_ref_unsafe(), > which applies the sanity check to the string from a symref. We seem > to allow > > $ git symbolic-ref refs/heads/SSS refs/heads//master > > and we end up storing

[PATCH v6 00/19] index-helper/watchman

2016-04-27 Thread David Turner
What's new: 1. configs for automatically populating the untracked-cache and watchman extensions. 2. index-helper errors go to index-helper.log (index-helper redirects stdout, stderr) 3. Duy's "Add tracing to measure where most of the time is spent" 4. index-helper sends/receives watchman

[PATCH v6 05/19] index-helper: log warnings

2016-04-27 Thread David Turner
Instead of writing warnings to stderr, write them to a log. Later, we'll probably be daemonized, so writing to stderr will be pointless. Signed-off-by: David Turner --- Documentation/git-index-helper.txt | 3 +++ index-helper.c | 29

[PATCH v6 08/19] read-cache: add watchman 'WAMA' extension

2016-04-27 Thread David Turner
From: Nguyễn Thái Ngọc Duy The extension contains a bitmap, one bit for each entry in the index. If the n-th bit is zero, the n-th entry is considered unchanged, we can ce_mark_uptodate() it without refreshing. If the bit is non-zero and we found out the corresponding file is

[PATCH v6 02/19] read-cache: allow to keep mmap'd memory after reading

2016-04-27 Thread David Turner
From: Nguyễn Thái Ngọc Duy Later, we will introduce git index-helper to share this memory with other git processes. We only unmap it when we discard the index (although the kernel may of course choose to page it out). Signed-off-by: Nguyễn Thái Ngọc Duy

[PATCH v6 11/19] update-index: enable/disable watchman support

2016-04-27 Thread David Turner
From: Nguyễn Thái Ngọc Duy Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: David Turner --- Documentation/git-index-helper.txt | 3 +++ Documentation/git-update-index.txt | 6 ++ builtin/update-index.c | 16

[PATCH v6 18/19] Add tracing to measure where most of the time is spent

2016-04-27 Thread David Turner
From: Nguyễn Thái Ngọc Duy All the known heavy code blocks are measured (except object database access). This should help identify if an optimization is effective or not. An unoptimized git-status would give something like below (92% of time is accounted). To sum up the effort

[PATCH v6 19/19] untracked-cache: config option

2016-04-27 Thread David Turner
Add a config option to populate the untracked cache. For installations that have centrally-managed configuration, it's easier to set a config once than to run update-index on every repository. Signed-off-by: David Turner --- Documentation/config.txt | 4

[PATCH v6 06/19] daemonize(): set a flag before exiting the main process

2016-04-27 Thread David Turner
From: Nguyễn Thái Ngọc Duy This allows signal handlers and atexit functions to realize this situation and not clean up. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: David Turner --- builtin/gc.c | 2 +- cache.h | 2 +-

[PATCH v6 13/19] watchman: add a config option to enable the extension

2016-04-27 Thread David Turner
For installations that have centrally-managed configuration, it's easier to set a config once than to run update-index on every repository. Signed-off-by: David Turner --- .gitignore| 1 + Documentation/config.txt | 4 Makefile

[PATCH v6 17/19] index-helper: optionally automatically run

2016-04-27 Thread David Turner
Introduce a new config option, indexhelper.autorun, to automatically run git index-helper before starting up a builtin git command. This enables users to keep index-helper running without manual intervention. Signed-off-by: David Turner --- Documentation/config.txt |

[PATCH v6 16/19] index-helper: autorun mode

2016-04-27 Thread David Turner
Soon, we'll want to automatically start index-helper, so we need a mode that silently exits if it can't start up (either because it's not in a git dir, or because another one is already running). Signed-off-by: David Turner --- Documentation/git-index-helper.txt | 4

[PATCH v6 10/19] index-helper: use watchman to avoid refreshing index with lstat()

2016-04-27 Thread David Turner
From: Nguyễn Thái Ngọc Duy Watchman is hidden behind index-helper. Before git tries to read the index from shm, it notifies index-helper through the socket and waits for index-helper to prepare a file for sharing memory (with MAP_SHARED). index-helper then contacts watchman,

[PATCH v6 04/19] index-helper: add --strict

2016-04-27 Thread David Turner
From: Nguyễn Thái Ngọc Duy There are "holes" in the index-helper approach because the shared memory is not verified again by git. If $USER is compromised, shared memory could be modified. But anyone who could do this could already modify $GIT_DIR/index. A more realistic risk

[PATCH v6 12/19] unpack-trees: preserve index extensions

2016-04-27 Thread David Turner
Make git checkout (and other unpack_tree operations) preserve the untracked cache and watchman status. This is valuable for two reasons: 1. Often, an unpack_tree operation will not touch large parts of the working tree, and thus most of the untracked cache will continue to be valid. 2. Even if

[PATCH v6 15/19] index-helper: don't run if already running

2016-04-27 Thread David Turner
Signed-off-by: David Turner --- index-helper.c | 7 +++ t/t7900-index-helper.sh | 9 + 2 files changed, 16 insertions(+) diff --git a/index-helper.c b/index-helper.c index 60a71f2..092c814 100644 --- a/index-helper.c +++ b/index-helper.c @@ -463,6

[PATCH v6 09/19] Add watchman support to reduce index refresh cost

2016-04-27 Thread David Turner
From: Nguyễn Thái Ngọc Duy The previous patch has the logic to clear bits in 'WAMA' bitmap. This patch has logic to set bits as told by watchman. The missing bit, _using_ these bits, are not here yet. A lot of this code is written by David Turner originally, mostly from [1].

[PATCH v6 01/19] read-cache.c: fix constness of verify_hdr()

2016-04-27 Thread David Turner
From: Nguyễn Thái Ngọc Duy Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: David Turner --- read-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/read-cache.c b/read-cache.c index d9fb78b..16cc487

[PATCH v6 07/19] index-helper: add --detach

2016-04-27 Thread David Turner
From: Nguyễn Thái Ngọc Duy We detach after creating and opening the socket, because otherwise we might return control to the shell before index-helper is ready to accept commands. This might lead to flaky tests. Signed-off-by: Nguyễn Thái Ngọc Duy

[PATCH v6 03/19] index-helper: new daemon for caching index and related stuff

2016-04-27 Thread David Turner
From: Nguyễn Thái Ngọc Duy Instead of reading the index from disk and worrying about disk corruption, the index is cached in memory (memory bit-flips happen too, but hopefully less often). The result is faster read. Read time is reduced by 70%. The biggest gain is not having

[PATCH v6 14/19] index-helper: kill mode

2016-04-27 Thread David Turner
Add a new command (and command-line arg) to allow index-helpers to exit cleanly. This is mainly useful for tests. Signed-off-by: David Turner --- Documentation/git-index-helper.txt | 3 +++ index-helper.c | 31 ++-

[PATCH] trailer: load config to handle core.commentChar

2016-04-27 Thread Rafal Klys
Add call to git_config(git_default_config, NULL) to update the comment_char_line from default '#' to possible different value set in core.commentChar. Signed-off-by: Rafal Klys --- trailer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/trailer.c b/trailer.c index

Re: [PATCH v4] http: support sending custom HTTP headers

2016-04-27 Thread Jeff King
On Wed, Apr 27, 2016 at 02:20:37PM +0200, Johannes Schindelin wrote: > The only change vs v3 is that I replaced my flimsical test by Peff's (with > *one* change: I realized that we need to group the Require statements in a > block when I tried to verify that the test fails when I > modify the

Re: [PATCH 19/29] refs: don't dereference on rename

2016-04-27 Thread Junio C Hamano
Michael Haggerty writes: > @@ -2380,8 +2381,8 @@ int rename_ref(const char *oldrefname, const char > *newrefname, const char *logms > goto rollback; > } > > - if (!read_ref_full(newrefname, RESOLVE_REF_READING, sha1, NULL) && > -

Re: [PATCH 2/3] mmap(win32): avoid copy-on-write when it is unnecessary

2016-04-27 Thread Johannes Sixt
Am 27.04.2016 um 08:43 schrieb Johannes Schindelin: On Tue, 26 Apr 2016, Johannes Sixt wrote: Should we insert a check for MAP_PRIVATE to catch unexpected use-cases (think of the index-helper daemon effort)? I agree, we should have such a check. The line above the `die("Invalid usage ...")`

Re: [PATCH 15/29] ref_transaction_create(): disallow recursive pruning

2016-04-27 Thread Junio C Hamano
Michael Haggerty writes: > It is nonsensical (and a little bit dangerous) to use REF_ISPRUNING > without REF_NODEREF. Forbid it explicitly. Change the one REF_ISPRUNING > caller to pass REF_NODEREF too. > > Signed-off-by: Michael Haggerty > --- > This

Re: [PATCH 12/29] read_raw_ref(): improve docstring

2016-04-27 Thread Junio C Hamano
Michael Haggerty writes: > * Backend-specific flags might be set in type as well, regardless of > * outcome. > * > - * sb_path is workspace: the caller should allocate and free it. All made sense. A welcome bonus is the removal of this stale comment that 42a38cf7

Re: [PATCH v14 3/6] t0040-parse-options: improve test coverage

2016-04-27 Thread Pranit Bauva
On Wed, Apr 27, 2016 at 11:25 PM, Eric Sunshine wrote: > On Mon, Apr 25, 2016 at 2:40 PM, Pranit Bauva wrote: >> On Wed, Apr 13, 2016 at 10:57 PM, Eric Sunshine >> wrote: >>> Each of these patches should have a single

Re: [PATCH 08/29] ref_transaction_commit(): remove local variable n

2016-04-27 Thread Junio C Hamano
Michael Haggerty writes: > This microoptimization doesn't make a significant difference in speed. > And it causes problems if somebody ever wants to modify the function to > add updates to a transaction as part of processing it, as will happen > shortly. > > Make the same

Re: [PATCH 53/83] builtin/apply: make find_header() return -1 instead of die()ing

2016-04-27 Thread Eric Sunshine
On Mon, Apr 25, 2016 at 9:18 AM, Duy Nguyen wrote: > On Sun, Apr 24, 2016 at 8:33 PM, Christian Couder > wrote: >> To be compatible with the rest of the error handling in builtin/apply.c, >> find_header() should return -1 instead of calling die().

Re: [PATCH 53/83] builtin/apply: make find_header() return -1 instead of die()ing

2016-04-27 Thread Eric Sunshine
On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder wrote: > To be compatible with the rest of the error handling in builtin/apply.c, > find_header() should return -1 instead of calling die(). > > Unfortunately find_header() already returns -1 when no header is found, >

Re: BUG on OSX `git p4 submit` can fail when the workspace root doesn't exist locally.

2016-04-27 Thread Jacob Smith
I debugged the issue using the script here: https://github.com/git/git/blob/master/git-p4.py I'm not sure how close to the main repo that is. On Wed, Apr 27, 2016 at 11:28 AM, Stefan Beller wrote: > On Wed, Apr 27, 2016 at 9:15 AM, Jacob Smith wrote:

Re: [PATCH 01/15] string_list: add string_list_duplicate

2016-04-27 Thread Stefan Beller
On Tue, Apr 26, 2016 at 3:37 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> The result of git_config_get_value_multi do not seem to be stable and >> may get overwritten. Have an easy way to preserve the results of that >> query. >> >> Signed-off-by:

Re: [PATCH 05/29] refname_is_safe(): insist that the refname already be normalized

2016-04-27 Thread Junio C Hamano
Michael Haggerty writes: > Does anybody have a use case for allowing un-normalized reference > names like "refs/foo/../bar///baz"? I'm pretty certain they would not > be handled correctly anyway, especially if they are not stored as > loose references. I wondered what

Re: [PATCH v14 3/6] t0040-parse-options: improve test coverage

2016-04-27 Thread Eric Sunshine
On Mon, Apr 25, 2016 at 2:40 PM, Pranit Bauva wrote: > On Wed, Apr 13, 2016 at 10:57 PM, Eric Sunshine > wrote: >> Each of these patches should have a single conceptual purpose. It >> seems, from the above explanation, that you're mixing and

[PATCH] Makefile: remove dependency on git.spec

2016-04-27 Thread Dennis Kaarsemaker
ab21433 dropped support for rpmbuild using our own specfile by removing git.spec.in, but forgot to remove the dependency of dist on git.spec. Signed-off-by: Dennis Kaarsemaker --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile

Re: [PATCH 03/83] builtin/apply: avoid parameter shadowing 'linenr' global

2016-04-27 Thread Christian Couder
On Wed, Apr 27, 2016 at 6:27 PM, Junio C Hamano wrote: > > I think 02/83 that renamed the global-to-be-moved-to-state to > state_p_value was brilliant, and this should follow suit; you would > be moving linenr into the state eventually in later steps, right? Yeah, ok, I will

[PATCH 10/29] read_raw_ref(): clear *type at start of function

2016-04-27 Thread Michael Haggerty
This is more convenient and less error-prone for callers. Signed-off-by: Michael Haggerty --- refs/files-backend.c | 1 + 1 file changed, 1 insertion(+) diff --git a/refs/files-backend.c b/refs/files-backend.c index 303c43b..f10c80f 100644 --- a/refs/files-backend.c +++

[PATCH 09/29] read_raw_ref(): rename flags argument to type

2016-04-27 Thread Michael Haggerty
This will hopefully reduce confusion with the "flags" arguments that are used in many functions in this module as an input parameter to choose how the function should operate. Signed-off-by: Michael Haggerty --- refs/files-backend.c | 16

[PATCH 22/29] lock_ref_for_update(): new function

2016-04-27 Thread Michael Haggerty
Extract a new function, lock_ref_for_update(), from ref_transaction_commit(). Signed-off-by: Michael Haggerty --- refs/files-backend.c | 152 --- 1 file changed, 85 insertions(+), 67 deletions(-) diff --git

[PATCH 00/29] Yet more preparation for reference backends

2016-04-27 Thread Michael Haggerty
This started as a modest attempt to move the last couple of patches mentioned here [1] to before the vtable patches. I wanted to do that because having real changes mixed with the vtable refactoring was making rebasing and stuff awkward. But then it snowballed. A lot of what's new is pretty

[PATCH 07/29] rename_ref(): remove unneeded local variable

2016-04-27 Thread Michael Haggerty
Signed-off-by: Michael Haggerty --- refs/files-backend.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index de38517..0ade681 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2351,20

[PATCH 24/29] ref_transaction_update(): check refname_is_safe() at a minimum

2016-04-27 Thread Michael Haggerty
If the user has asked that a new value be set for a reference, we use check_refname_format() to verify that the reference name satisfies all of the rules. But in other cases, at least check that refname_is_safe(). Signed-off-by: Michael Haggerty --- There are remaining

[PATCH 17/29] delete_branches(): use resolve_refdup()

2016-04-27 Thread Michael Haggerty
The return value of resolve_ref_unsafe() is not guaranteed to stay around as long as we need it, so use resolve_refdup() instead. Signed-off-by: Michael Haggerty --- builtin/branch.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git

[PATCH 28/29] commit_ref_update(): remove the flags parameter

2016-04-27 Thread Michael Haggerty
commit_ref_update() is now only called with flags=0. So remove the flags parameter entirely. Signed-off-by: Michael Haggerty --- refs/files-backend.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/refs/files-backend.c

[PATCH 25/29] refs: resolve symbolic refs first

2016-04-27 Thread Michael Haggerty
Before committing ref updates, split symbolic ref updates into two parts: an update to the underlying ref, and a log-only update to the symbolic ref. This ensures that both references are locked correctly during the transaction, including while their reflogs are updated. Similarly, if the

[PATCH 19/29] refs: don't dereference on rename

2016-04-27 Thread Michael Haggerty
From: David Turner When renaming refs, don't dereference either the origin or the destination before renaming. The origin does not need to be dereferenced because it is presently forbidden to rename symbolic refs. Not dereferencing the destination fixes a bug where

[PATCH 27/29] lock_ref_for_update(): don't resolve symrefs

2016-04-27 Thread Michael Haggerty
If a transaction includes a non-NODEREF update to a symbolic reference, we don't have to look it up in lock_ref_for_update(). The reference will be dereferenced anyway when the split-off update is processed. This change requires that we store a backpointer from the split-off update to its parent

[PATCH 01/29] safe_create_leading_directories(): improve docstring

2016-04-27 Thread Michael Haggerty
Document the difference between this function and safe_create_leading_directories_const(), and that the former restores path before returning. Signed-off-by: Michael Haggerty --- cache.h | 5 + 1 file changed, 5 insertions(+) diff --git a/cache.h b/cache.h index

[PATCH 14/29] refs: make error messages more consistent

2016-04-27 Thread Michael Haggerty
* Always start error messages with a lower-case letter. * Always enclose reference names in single quotes. Signed-off-by: Michael Haggerty --- This change is not strictly needed, but I wanted to fix the old error messages before I started adding new ones (otherwise, should

[PATCH 02/29] remove_dir_recursively(): add docstring

2016-04-27 Thread Michael Haggerty
Add a docstring for the remove_dir_recursively() function and the REMOVE_DIR_* flags that can be passed to it. Signed-off-by: Michael Haggerty --- dir.h | 23 +++ 1 file changed, 23 insertions(+) diff --git a/dir.h b/dir.h index 301b737..5f19acc 100644

[PATCH 15/29] ref_transaction_create(): disallow recursive pruning

2016-04-27 Thread Michael Haggerty
It is nonsensical (and a little bit dangerous) to use REF_ISPRUNING without REF_NODEREF. Forbid it explicitly. Change the one REF_ISPRUNING caller to pass REF_NODEREF too. Signed-off-by: Michael Haggerty --- This also makes later patches a bit clearer. refs.c

[PATCH 29/29] lock_ref_sha1_basic(): only handle REF_NODEREF mode

2016-04-27 Thread Michael Haggerty
Now lock_ref_sha1_basic() is only called with flags==REF_NODEREF. So we don't have to handle other cases anymore. This enables several simplifications, the most interesting of which come from the fact that ref_lock::orig_ref_name is now always the same as ref_lock::ref_name: * Remove

[PATCH 04/29] refname_is_safe(): don't allow the empty string

2016-04-27 Thread Michael Haggerty
Signed-off-by: Michael Haggerty --- This fixes a coding error from the original implementation. refs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index 5789152..ca0280f 100644 --- a/refs.c +++ b/refs.c @@ -136,11 +136,12 @@ int

[PATCH 08/29] ref_transaction_commit(): remove local variable n

2016-04-27 Thread Michael Haggerty
This microoptimization doesn't make a significant difference in speed. And it causes problems if somebody ever wants to modify the function to add updates to a transaction as part of processing it, as will happen shortly. Make the same change in initial_ref_transaction_commit(). Signed-off-by:

[PATCH 03/29] refname_is_safe(): use skip_prefix()

2016-04-27 Thread Michael Haggerty
Signed-off-by: Michael Haggerty --- refs.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/refs.c b/refs.c index 87dc82f..5789152 100644 --- a/refs.c +++ b/refs.c @@ -120,17 +120,19 @@ int check_refname_format(const char *refname, int flags)

[PATCH 23/29] unlock_ref(): move definition higher in the file

2016-04-27 Thread Michael Haggerty
This avoids the need for a forward declaration in the next patch. Signed-off-by: Michael Haggerty --- refs/files-backend.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index

[PATCH 11/29] read_raw_ref(): rename symref argument to referent

2016-04-27 Thread Michael Haggerty
After all, it doesn't hold the symbolic reference, but rather the reference referred to. Signed-off-by: Michael Haggerty --- refs/files-backend.c | 21 +++-- refs/refs-internal.h | 2 +- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git

[PATCH 21/29] add_update(): initialize the whole ref_update

2016-04-27 Thread Michael Haggerty
Change add_update() to initialize all of the fields in the new ref_update object. Rename the function to ref_transaction_add_update(), and increase its visibility to all of the refs-related code. All of this makes the function more useful for other future callers. Signed-off-by: Michael Haggerty

[PATCH 18/29] refs: allow log-only updates

2016-04-27 Thread Michael Haggerty
From: David Turner The refs infrastructure learns about log-only ref updates, which only update the reflog. Later, we will use this to separate symbolic reference resolution from ref updating. Signed-off-by: David Turner Signed-off-by: Junio

[PATCH 12/29] read_raw_ref(): improve docstring

2016-04-27 Thread Michael Haggerty
Among other things, document the (important!) requirement that input refname be checked for safety before calling this function. Signed-off-by: Michael Haggerty --- refs/files-backend.c | 41 - 1 file changed, 24 insertions(+), 17

[PATCH 05/29] refname_is_safe(): insist that the refname already be normalized

2016-04-27 Thread Michael Haggerty
The reference name is going to be compared to other reference names, so it should be in its normalized form. Signed-off-by: Michael Haggerty --- Does anybody have a use case for allowing un-normalized reference names like "refs/foo/../bar///baz"? I'm pretty certain they

[PATCH 13/29] lock_ref_sha1_basic(): remove unneeded local variable

2016-04-27 Thread Michael Haggerty
resolve_ref_unsafe() can cope with being called with NULL passed to its flags argument. So lock_ref_sha1_basic() can just hand its own type parameter through. Signed-off-by: Michael Haggerty --- refs/files-backend.c | 9 +++-- 1 file changed, 3 insertions(+), 6

[PATCH 26/29] lock_ref_for_update(): don't re-read non-symbolic references

2016-04-27 Thread Michael Haggerty
Before the previous patch, our first read of the reference happened before the reference was locked, so we couldn't trust its value and had to read it again. But now that our first read of the reference happens after acquiring the lock, there is no need to read it a second time. So move the

[PATCH 20/29] verify_refname_available(): adjust constness in declaration

2016-04-27 Thread Michael Haggerty
The two string_list arguments can be const. Signed-off-by: Michael Haggerty --- This is needed because I want to add a new caller that has const versions of these lists in hand. refs/files-backend.c | 4 ++-- refs/refs-internal.h | 4 ++-- 2 files changed, 4

[PATCH 06/29] commit_ref_update(): write error message to *err, not stderr

2016-04-27 Thread Michael Haggerty
Signed-off-by: Michael Haggerty --- refs/files-backend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 1f38076..de38517 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2686,7 +2686,7 @@

[PATCH 16/29] ref_transaction_commit(): correctly report close_ref() failure

2016-04-27 Thread Michael Haggerty
Signed-off-by: Michael Haggerty --- close() rarely fails, but it is possible. refs/files-backend.c | 1 + 1 file changed, 1 insertion(+) diff --git a/refs/files-backend.c b/refs/files-backend.c index 8fcbd7d..e86e3de 100644 --- a/refs/files-backend.c +++

  1   2   >