[PATCH v4 0/4] Rerolling ma/split-symref-update-fix

> I'll take Peff's hint, tweak/add comments for correctness and symmetry > with the previous patch and add an if-BUG for symmetry. Here's a reroll of ma/split-symref-update-fix. The first three patches are v3 plus Michael's Reviewed-By. The fourth is the conceptual fix of adding `refname` instead

[PATCH v4 4/4] refs/files-backend: add `refname`, not "HEAD", to list

An earlier patch rewrote `split_symref_update()` to add a copy of a string to a string list instead of adding the original string. That was so that the original string could be freed in a later patch, but it is also conceptually cleaner, since now all calls to `string_list_insert()` and `string_lis

[PATCH v4 3/4] refs/files-backend: correct return value in lock_ref_for_update

In one code path we return a literal -1 and not a symbolic constant. The value -1 would be interpreted as TRANSACTION_NAME_CONFLICT, which is wrong. Use TRANSACTION_GENERIC_ERROR instead (that is the only other return value we have to choose from). Noticed-by: Michael Haggerty Reviewed-by: Michae

[PATCH v4 2/4] refs/files-backend: fix memory leak in lock_ref_for_update

After the previous patch, none of the functions we call hold on to `referent.buf`, so we can safely release the string buffer before returning. Reviewed-by: Michael Haggerty Signed-off-by: Martin Ågren --- refs/files-backend.c | 31 --- 1 file changed, 20 insertions(

[PATCH v4 1/4] refs/files-backend: add longer-scoped copy of string to list

split_symref_update() receives a string-pointer `referent` and adds it to the list of `affected_refnames`. The list simply holds on to the pointers it is given, it does not copy the strings and it does not ever free them. The `referent` string in split_symref_update() belongs to a string buffer in

Re: "git shortlog -sn --follow -- " counts all commits to entire repo

On Sat, Sep 09, 2017 at 02:37:20AM +0900, Junio C Hamano wrote: > > That said, I don't think we can go wrong by making shortlog's traversal > > more like log's. Any changes we make to --follow will be aimed at and > > tested with git-log, so the more code they share the more likely it is > > that

Re: [PATCH v4 00/16] Fix git-gc losing objects in multi worktree

On 08/23/2017 02:36 PM, Nguyễn Thái Ngọc Duy wrote: > "git gc" when used in multiple worktrees ignore some per-worktree > references: object references in the index, HEAD and reflog. This > series fixes it by making the revision walker include these from all > worktrees by default (and the series i

Re: [PATCH v4 16/16] refs.c: reindent get_submodule_ref_store()

On 08/23/2017 02:37 PM, Nguyễn Thái Ngọc Duy wrote: > With the new "if (!submodule) return NULL;" code added in the previous > commit, we don't need to check if submodule is not NULL anymore. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > refs.c | 12 +--- > 1 file changed, 5 insertions

Re: [PATCH v4 15/16] refs.c: remove fallback-to-main-store code get_submodule_ref_store()

On 08/23/2017 02:37 PM, Nguyễn Thái Ngọc Duy wrote: > At this state, there are three get_submodule_ref_store() callers: > > - for_each_remote_ref_submodule() > - handle_revision_pseudo_opt() > - resolve_gitlink_ref() > > The first two deal explicitly with submodules (and we should never fall >

Re: [PATCH v4 12/16] files-backend: make reflog iterator go through per-worktree reflog

On 08/23/2017 02:37 PM, Nguyễn Thái Ngọc Duy wrote: > refs/bisect is unfortunately per-worktree, so we need to look in > per-worktree logs/refs/bisect in addition to per-repo logs/refs. The > current iterator only goes through per-repo logs/refs. > > Use merge iterator to walk two ref stores at th

Re: [PATCH v4 11/16] revision.c: --all adds HEAD from all worktrees

On 08/23/2017 02:36 PM, Nguyễn Thái Ngọc Duy wrote: > [...] > diff --git a/revision.c b/revision.c > index 8d04516266..0e98444857 100644 > --- a/revision.c > +++ b/revision.c > @@ -2133,6 +2133,14 @@ static int handle_revision_pseudo_opt(const char > *submodule, > int argcount; > > i

Re: [PATCH v4 10/16] refs: remove dead for_each_*_submodule()

On 08/23/2017 02:36 PM, Nguyễn Thái Ngọc Duy wrote: > These are used in revision.c. After the last patch they are replaced > with the refs_ version. Delete them. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > Documentation/technical/api-ref-iteration.txt | 7 ++ > refs.c

Re: [PATCH v4 06/16] refs: move submodule slash stripping code to get_submodule_ref_store

On 08/23/2017 02:36 PM, Nguyễn Thái Ngọc Duy wrote: > This is a better place that will benefit all submodule callers instead > of just resolve_gitlink_ref() This is a nice sentiment, but I have to wonder whether we should rather be requiring callers to use the "canonical" submodule name rather tha

RE: [PATCH 1/1] reset: fix reset when using the sparse-checkout feature.

> From: Junio C Hamano [mailto:gits...@pobox.com] > Sent: Friday, September 8, 2017 9:18 PM > > Kevin Willford writes: > > > 1. reset mixed when there were files that were added > > > > In this case the index will no longer have the entry at all because > > the reset is making the index look lik

Re: [PATCH] commit-tree: don't append a newline with -F

Ross Kabus writes: > On Thu, Sep 7, 2017 at 9:35 PM, Junio C Hamano wrote: >> commit-tree: do not complete line in -F input >> >> "git commit-tree -F ", unlike "cat | git >> commit-tree" (i.e. feeding the same contents from the standard >> input), added a missing final newline w

Re: [PATCH 1/1] reset: fix reset when using the sparse-checkout feature.

Kevin Willford writes: > 1. reset mixed when there were files that were added > > In this case the index will no longer have the entry at all because > the reset is making the index look like before the file was added > which didn't have it. When not using the sparse-checkout this is fine > becau

Re: [PATCH] commit-tree: don't append a newline with -F

On Thu, Sep 7, 2017 at 9:35 PM, Junio C Hamano wrote: > commit-tree: do not complete line in -F input > > "git commit-tree -F ", unlike "cat | git > commit-tree" (i.e. feeding the same contents from the standard > input), added a missing final newline when the input ended in an >

RE: [PATCH 1/1] reset: fix reset when using the sparse-checkout feature.

From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Friday, September 8, 2017 1:02 PM > Kevin Willford writes: > > > diff --git a/builtin/reset.c b/builtin/reset.c > > index d72c7d1c96..1b8bb45989 100644 > > --- a/builtin/reset.c > > +++ b/builtin/reset.c > > @@ -24,6 +24,7 @@ > > #include "ca

Re: Re: cat-file timing window on Cygwin

On 27/08/17 16:47, Ramsay Jones wrote: > On 27/08/17 12:33, Adam Dinwoodie wrote: [snip] >> Cygwin 2.8.3 hasn't been released yet, > > Heh, yes, I found that out myself this afternoon. ;-) > >> but I've just tested the latest >> development snapshot wit

RE: [PATCH 1/1] reset: fix reset when using the sparse-checkout feature.

From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Friday, September 8, 2017 1:12 PM > Kevin Willford writes: > > > When using the sparse checkout feature the git reset command will add > > entries to the index that will have the skip-worktree bit off but will > > leave the working directory

Re: [PATCH 1/1] reset: fix reset when using the sparse-checkout feature.

Kevin Willford writes: > When using the sparse checkout feature the git reset command will add > entries to the index that will have the skip-worktree bit off but will > leave the working directory empty. File data is lost because the index > version of the files have been changed but there is n

Re: [PATCH 1/1] reset: fix reset when using the sparse-checkout feature.

Torsten Bögershausen writes: >> +test_expect_success 'setup' ' >> +test_tick && > > Do we need a test_tick here ? As the test does not check against exact object names, and it does not create commits, the order among which needs to be tiebroken by using the committer timestamp, it is not str

Re: [PATCH 1/1] reset: fix reset when using the sparse-checkout feature.

Kevin Willford writes: > diff --git a/builtin/reset.c b/builtin/reset.c > index d72c7d1c96..1b8bb45989 100644 > --- a/builtin/reset.c > +++ b/builtin/reset.c > @@ -24,6 +24,7 @@ > #include "cache-tree.h" > #include "submodule.h" > #include "submodule-config.h" > +#include "dir.h" > > static

Re: [PATCH 1/1] reset: fix reset when using the sparse-checkout feature.

On Fri, Sep 08, 2017 at 12:00:50PM -0600, Kevin Willford wrote: > When using the sparse checkout feature the git reset command will add > entries to the index that will have the skip-worktree bit off but will > leave the working directory empty. File data is lost because the index > version of the

[PATCH 0/1] reset: fix mixed reset when using sparse-checkout

Original discussion is here https://public-inbox.org/git/20170407192357.948-4-kewi...@microsoft.com/ When running a reset mixed and using the sparse-checkout the working directory needs to be updated so that there is not data loss when the index is updated. This is because the index is getting up

[PATCH 1/1] reset: fix reset when using the sparse-checkout feature.

When using the sparse checkout feature the git reset command will add entries to the index that will have the skip-worktree bit off but will leave the working directory empty. File data is lost because the index version of the files have been changed but there is nothing that is in the working dir

Re: "git shortlog -sn --follow -- " counts all commits to entire repo

Jeff King writes: > Yeah. It depends on exactly how such a fix is made. I think one > improvement would be to actually bump --follow handling into the > limit_list() stage, so that we properly handle history simplification > over followed paths. In which case get_revision() would just never > ret

Re: [PATCH] refs: make sure we never pass NULL to hashcpy

Michael Haggerty writes: > So `ref_transaction_update()` *does* need to set or clear the `HAVE_NEW` > and `HAVE_OLD` bits as I sketched, to impedance-match between the two > conventions. OK, so ignoring HAVE_NEW/HAVE_OLD bits that the callers of ref_transaction_update() may set in flags, and hav

[PATCH] load_subtree(): check that `prefix_len` is in the expected range

This value, which is stashed in the last byte of an object_id hash, gets handed around a lot. So add a sanity check before using it in `load_subtree()`. Signed-off-by: Michael Haggerty --- This patch is an addendum to v1 of the mh/notes-cleanup patch series [1]. It adds the assertion that was sug

Re: [PATCH] refs: make sure we never pass NULL to hashcpy

On 09/08/2017 02:46 AM, Junio C Hamano wrote: > Michael Haggerty writes: > >> I did just realize one thing: `ref_transaction_update()` takes `flags` >> as an argument and alters it using >> >>> flags |= (new_sha1 ? REF_HAVE_NEW : 0) | (old_sha1 ? REF_HAVE_OLD : >>> 0); >> >> Perhaps gcc

[PATCH v2 07/11] files_initial_transaction_commit(): use a transaction for packed refs

Use a `packed_ref_store` transaction in the implementation of `files_initial_transaction_commit()` rather than using internal features of the packed ref store. This further decouples `files_ref_store` from `packed_ref_store`. Signed-off-by: Michael Haggerty --- refs/files-backend.c | 29

[PATCH v2 11/11] files_transaction_finish(): delete reflogs before references

If the deletion steps unexpectedly fail, it is less bad to leave a reference without its reflog than it is to leave a reflog without its reference, since the latter is an invalid repository state. Signed-off-by: Michael Haggerty --- refs/files-backend.c | 35 +--

[PATCH v2 08/11] t1404: demonstrate two problems with reference transactions

Currently, a loose reference is deleted even before locking the `packed-refs` file, let alone deleting any packed version of the reference. This leads to two problems, demonstrated by two new tests: * While a reference is being deleted, other processes might see the old, packed value of the refe

[PATCH v2 06/11] prune_refs(): also free the linked list

At least since v1.7, the elements of the `refs_to_prune` linked list have been leaked. Fix the leak by teaching `prune_refs()` to free the list elements as it processes them. Signed-off-by: Michael Haggerty --- refs/files-backend.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletio

[PATCH v2 10/11] packed-backend: rip out some now-unused code

Now the outside world interacts with the packed ref store only via the generic refs API plus a few lock-related functions. This allows us to delete some functions that are no longer used, thereby completing the encapsulation of the packed ref store. Signed-off-by: Michael Haggerty --- refs/packe

[PATCH v2 09/11] files_ref_store: use a transaction to update packed refs

When processing a `files_ref_store` transaction, it is sometimes necessary to delete some references from the "packed-refs" file. Do that using a reference transaction conducted against the `packed_ref_store`. This change further decouples `files_ref_store` from `packed_ref_store`. It also fixes m

[PATCH v2 04/11] packed_delete_refs(): implement method

Implement `packed_delete_refs()` using a reference transaction. This means that `files_delete_refs()` can use `refs_delete_refs()` instead of `repack_without_refs()` to delete any packed references, decreasing the coupling between the classes. Signed-off-by: Michael Haggerty --- refs/files-backe

[PATCH v2 02/11] struct ref_transaction: add a place for backends to store data

`packed_ref_store` is going to want to store some transaction-wide data, so make a place for it. Signed-off-by: Michael Haggerty --- refs/refs-internal.h | 1 + 1 file changed, 1 insertion(+) diff --git a/refs/refs-internal.h b/refs/refs-internal.h index b02dc5a7e3..d7d344de73 100644 --- a/refs

[PATCH v2 03/11] packed_ref_store: implement reference transactions

Implement the methods needed to support reference transactions for the packed-refs backend. The new methods are not yet used. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 313 +- refs/packed-backend.h | 9 ++ 2 files changed, 319 i

[PATCH v2 05/11] files_pack_refs(): use a reference transaction to write packed refs

Now that the packed reference store supports transactions, we can use a transaction to write the packed versions of references that we want to pack. This decreases the coupling between `files_ref_store` and `packed_ref_store`. Signed-off-by: Michael Haggerty --- refs/files-backend.c | 24 +++

[PATCH v2 01/11] packed-backend: don't adjust the reference count on lock/unlock

The old code incremented the packed ref cache reference count when acquiring the packed-refs lock, and decremented the count when releasing the lock. This is unnecessary because: * Another process cannot change the packed-refs file because it is locked. * When we ourselves change the packed-ref

[PATCH v2 00/11] Implement transactions for the packed ref store

This is v2 of a patch series to implement reference transactions for the packed refs-store. Thanks to Stefan, Brandon, Junio, and Peff for your review of v1 [1]. I believe I have addressed all of your comments. Changes since v1: * Patch [01/11]: justify the change better in the log message. Add a

Re: git diff doesn't quite work as documented?

Junio C Hamano venit, vidit, dixit 08.09.2017 03:26: > Olaf Klischat writes: > >> `git diff --help' says: >> >> git diff [--options] [--] [...] >>This form is to view the changes you have in your >>working tree relative to the named . > > That help text is poorly phrased

Re: [PATCH 08/10] files_ref_store: use a transaction to update packed refs

On Fri, Sep 08, 2017 at 02:44:57PM +0200, Michael Haggerty wrote: > > That means we're holding the packed-refs lock for a slightly longer > > period. I think this could mean worse lock contention between otherwise > > unrelated transactions over the packed-refs file. I wonder if the > > lock-retry

Re: [PATCH 08/10] files_ref_store: use a transaction to update packed refs

On 09/08/2017 09:38 AM, Jeff King wrote: > On Tue, Aug 29, 2017 at 10:20:32AM +0200, Michael Haggerty wrote: > >> First, the old code didn't obtain the `packed-refs` lock until >> `files_transaction_finish()`. This means that a failure to acquire the >> `packed-refs` lock (e.g., due to contention

Re: [PATCH 0/4] Test name-rev with small stack

Jeff King venit, vidit, dixit 07.09.2017 16:54: > On Thu, Sep 07, 2017 at 04:02:19PM +0200, Michael J Gruber wrote: > >> name-rev segfaults for me in emacs.git with the typical 8102 stack size. >> The reason is the recursive walk that name-rev uses. >> >> This series adds a test to mark this as kn

[no subject]

I have a charity proposal worth 100million. Alice Walton. CONTACT ( cont...@alicewonders.us ) FOR YOUR FUNDS.

Re: [PATCH 07/10] t1404: demonstrate two problems with reference transactions

On 09/08/2017 06:44 AM, Junio C Hamano wrote: > Michael Haggerty writes: > >> +git for-each-ref $prefix >actual && >> +grep "Unable to create $Q.*packed-refs.lock$Q: File exists" err && > > I added a squash for doing s/grep/test_i18n&/ here Thanks for the fix. I always forget that gotch

Re: [PATCH 06/10] files_initial_transaction_commit(): use a transaction for packed refs

On 09/08/2017 09:27 AM, Jeff King wrote: > On Tue, Aug 29, 2017 at 10:20:30AM +0200, Michael Haggerty wrote: > >> Used a `packed_ref_store` transaction in the implementation of >> `files_initial_transaction_commit()` rather than using internal >> features of the packed ref store. This further deco

Re: [PATCH 01/10] packed-backend: don't adjust the reference count on lock/unlock

On 09/08/2017 08:52 AM, Jeff King wrote: > On Tue, Aug 29, 2017 at 10:20:25AM +0200, Michael Haggerty wrote: > >> The old code incremented the packed ref cache reference count when >> acquiring the packed-refs lock, and decremented the count when >> releasing the lock. This is unnecessary because

[PATCH] shortlog: skip format/parse roundtrip for internal traversal

On Fri, Sep 08, 2017 at 03:39:36PM +0900, Junio C Hamano wrote: > Jeff King writes: > > > IOW, something like the patch below, which pushes the re-parsing out to > > the stdin code-path, and lets the internal traversal format directly > > into the final buffer. It seems to be about 3% faster tha

MICROSOFT VERIFICATION UPDATE

MICROSOFT VERIFICATION UPDATE Geachte klant, Lees en volg de instructies om uw Microsoft Privacy te beschermen. Als onderdeel van onze inspanningen om uw ervaring te verbeteren in onze consumentendiensten, updaten we de Microsoft Services Agreement en de Microsoft Privacy Statement. We willen

GET BACK URGENT

Attention pls, I am Mohad I Kwame, presently the current Manager at the UBA Bank Capital (Europe) Ltd. I need a reliable and honest person to handle a very confidential transaction, which involves transfer of a huge sum of money to a foreign account. A foreigner (LATE ENGR NAZIR RUZU) an oil M

Re: [PATCH 02/10] struct ref_transaction: add a place for backends to store data

On Fri, Sep 08, 2017 at 10:19:48AM +0200, Michael Haggerty wrote: > > This is just one pointer. Once we start layering ref backends (and > > already we're moving towards a "files" layer which sits atop loose and > > packed backends, right?), how do we avoid backends stomping on each > > other (or

Re: [PATCH 02/10] struct ref_transaction: add a place for backends to store data

On 09/08/2017 09:02 AM, Jeff King wrote: > On Tue, Aug 29, 2017 at 10:20:26AM +0200, Michael Haggerty wrote: > >> `packed_ref_store` is going to want to store some transaction-wide >> data, so make a place for it. > > That makes sense, although... > >> diff --git a/refs/refs-internal.h b/refs/re

FINALE WAARSCHUWING VAN MICROSOFT

MICROSOFT VERIFICATION UPDATE Geachte klant, Lees en volg de instructies om uw Microsoft Privacy te beschermen. Als onderdeel van onze inspanningen om uw ervaring te verbeteren in onze consumentendiensten, updaten we de Microsoft Services Agreement en de Microsoft Privacy Statement. We willen

Re: "git shortlog -sn --follow -- " counts all commits to entire repo

On Fri, Sep 08, 2017 at 03:38:17PM +0900, Junio C Hamano wrote: > > I suspect a better solution might involve actually building on > > log-tree.c to do the traversal (since this internal traversal is > > supposed to be equivalent to "git log | git shortlog"). > > Probably. That approach would al

Re: [PATCH 07/10] t1404: demonstrate two problems with reference transactions

On Fri, Sep 08, 2017 at 01:44:30PM +0900, Junio C Hamano wrote: > Michael Haggerty writes: > > > + git for-each-ref $prefix >actual && > > + grep "Unable to create $Q.*packed-refs.lock$Q: File exists" err && > > I added a squash for doing s/grep/test_i18n&/ here; are there other > issues in

Re: [PATCH 00/10] Implement transactions for the packed ref store

On Tue, Aug 29, 2017 at 10:20:24AM +0200, Michael Haggerty wrote: > This should be the second-to-last patch series in the quest for > mmapped packed-refs. I just gave this a careful read, and it looks pretty good to me. I had a few questions, but no show-stoppers. I'll admit to glossing a little

Re: [PATCH 08/10] files_ref_store: use a transaction to update packed refs

On Tue, Aug 29, 2017 at 10:20:32AM +0200, Michael Haggerty wrote: > First, the old code didn't obtain the `packed-refs` lock until > `files_transaction_finish()`. This means that a failure to acquire the > `packed-refs` lock (e.g., due to contention with another process) > wasn't detected until it

Re: [PATCH 06/10] files_initial_transaction_commit(): use a transaction for packed refs

On Tue, Aug 29, 2017 at 10:20:30AM +0200, Michael Haggerty wrote: > Used a `packed_ref_store` transaction in the implementation of > `files_initial_transaction_commit()` rather than using internal > features of the packed ref store. This further decouples > `files_ref_store` from `packed_ref_store

Re: [PATCH 02/10] struct ref_transaction: add a place for backends to store data

On Tue, Aug 29, 2017 at 10:20:26AM +0200, Michael Haggerty wrote: > `packed_ref_store` is going to want to store some transaction-wide > data, so make a place for it. That makes sense, although... > diff --git a/refs/refs-internal.h b/refs/refs-internal.h > index b02dc5a7e3..d7d344de73 100644 >