[PATCH v2 00/20] Separate `ref_cache` into a separate module

2017-03-31 Thread Michael Haggerty
efs(): use reference iteration This patch series is also available from my GitHub fork [2] as branch "separate-ref-cache". These patches depend on Duy's nd/files-backend-git-dir branch. [1] http://public-inbox.org/git/cover.1490026594.git.mhag...@alum.mit.edu/ [2] https://gith

[PATCH v2 04/20] refs_verify_refname_available(): implement once for all backends

2017-03-31 Thread Michael Haggerty
It turns out that we can now implement `refs_verify_refname_available()` based on the other virtual functions, so there is no need for it to be defined at the backend level. Instead, define it once in `refs.c` and remove the `files_backend` definition. Signed-off-by: Michael Haggerty <m

[PATCH v2 09/20] refs: split `ref_cache` code into separate files

2017-03-31 Thread Michael Haggerty
, adds declarations, and changes the visibility of some functions, but doesn't change any code. The modules are still too tightly coupled, but the situation will be improved in subsequent commits. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- Makefile | 1 + refs

[PATCH v2 15/20] get_loose_ref_dir(): function renamed from get_loose_refs()

2017-03-31 Thread Michael Haggerty
The new name is more analogous to `get_packed_ref_dir()`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 0ff5df6b46..0a16f6196c

[PATCH v2 02/20] refs_read_raw_ref(): new function

2017-03-31 Thread Michael Haggerty
Extract a new function from `refs_resolve_ref_unsafe()`. It will be useful elsewhere. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 11 +-- refs/refs-internal.h | 4 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/refs.c b/

[PATCH v2 07/20] ref-cache: rename `find_ref()` to `find_ref_entry()`

2017-03-31 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH v2 05/20] refs_verify_refname_available(): use function in more places

2017-03-31 Thread Michael Haggerty
(and will be regained later). These were the last callers of `verify_refname_available_dir()`, so also delete that (very complicated) function. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 171 --- 1 file chang

[PATCH v2 01/20] get_ref_dir(): don't call read_loose_refs() for "refs/bisect"

2017-03-31 Thread Michael Haggerty
nism, and this time the read was done correctly. This code has been broken since it was first introduced. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 42

[PATCH v2 03/20] refs_ref_iterator_begin(): new function

2017-03-31 Thread Michael Haggerty
Extract a new function from `do_for_each_ref()`. It will be useful elsewhere. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 15 +-- refs/refs-internal.h | 11 +++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/re

Re: [PATCH v5 0/6] [GSoC] remove_subtree(): reimplement using iterators

2017-03-30 Thread Michael Haggerty
On 03/30/2017 05:32 AM, Daniel Ferreira wrote: > This is the fifth version of a patch series that implements the GSoC > microproject of converting a recursive call to readdir() to use dir_iterator. > > v1: >

Re: [PATCH v5 6/6] remove_subtree(): test removing nested directories

2017-03-30 Thread Michael Haggerty
On 03/30/2017 05:32 AM, Daniel Ferreira wrote: > Test removing a nested directory when an attempt is made to restore the > index to a state where it does not exist. A similar test could be found > previously in t/t2000-checkout-cache-clash.sh, but it did not check for > nested directories, which

Re: [PATCH v5 3/6] dir_iterator: iterate over dir after its contents

2017-03-30 Thread Michael Haggerty
On 03/30/2017 05:32 AM, Daniel Ferreira wrote: > Create an option for the dir_iterator API to iterate over subdirectories > only after having iterated through their contents. This feature was > predicted, although not implemented by 0fe5043 ("dir_iterator: new API > for iterating over a directory

Re: [PATCH v5 2/6] dir_iterator: refactor state machine model

2017-03-30 Thread Michael Haggerty
On 03/30/2017 05:32 AM, Daniel Ferreira wrote: > Remove the "initialized" member of dir_iterator_level. Replace its > functionality with a DIR_STATE_PUSH state in the > dir_iterator_level.dir_state enum. > > This serves to remove a redundant property in the dir_iterator_level > struct and ease

Re: [PATCH v5 4/6] dir_iterator: add tests for dir_iterator API

2017-03-30 Thread Michael Haggerty
On 03/30/2017 05:32 AM, Daniel Ferreira wrote: > Create t/helper/test-dir-iterator.c, which prints relevant information > about a directory tree iterated over with dir_iterator. > > Create t/t0065-dir-iterator.sh, which tests that dir_iterator does > iterate through a whole directory tree and

Re: [PATCH v5 4/6] dir_iterator: add tests for dir_iterator API

2017-03-30 Thread Michael Haggerty
On 03/30/2017 05:32 AM, Daniel Ferreira wrote: > Create t/helper/test-dir-iterator.c, which prints relevant information > about a directory tree iterated over with dir_iterator. > > Create t/t0065-dir-iterator.sh, which tests that dir_iterator does > iterate through a whole directory tree and

Re: [PATCH v5 4/6] dir_iterator: add tests for dir_iterator API

2017-03-30 Thread Michael Haggerty
On 03/30/2017 05:32 AM, Daniel Ferreira wrote: > Create t/helper/test-dir-iterator.c, which prints relevant information > about a directory tree iterated over with dir_iterator. > > Create t/t0065-dir-iterator.sh, which tests that dir_iterator does > iterate through a whole directory tree and

Re: [PATCH v4 2/5] dir_iterator: iterate over dir after its contents

2017-03-30 Thread Michael Haggerty
On 03/30/2017 08:08 AM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> I think IN_ORDER really only applies to *binary* trees, not arbitrary >> trees like a filesystem. > > How true. Even if we were giving a sorted outp

Re: [PATCH v4 2/5] dir_iterator: iterate over dir after its contents

2017-03-29 Thread Michael Haggerty
On 03/29/2017 06:46 PM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> I also realize that I made a goof in my comments about v3 of this patch >> series. Your new option is not choosing between "depth-first" and >> "brea

Re: [PATCH v4 2/5] dir_iterator: iterate over dir after its contents

2017-03-29 Thread Michael Haggerty
On 03/29/2017 11:56 AM, Michael Haggerty wrote: > On 03/29/2017 02:32 AM, Daniel Ferreira wrote: >> [...] > [...] > The disagreement is not a surprise, because there isn't a corresponding > coding error in the code below that returns the directory itself in a > post-order iter

Re: [PATCH v6 18/27] files-backend: replace submodule_allowed check in files_downcast()

2017-03-29 Thread Michael Haggerty
On 03/26/2017 04:16 AM, Duy Nguyen wrote: > On Mon, Mar 20, 2017 at 4:18 AM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >>> +/* ref_store_init flags */ >>> +#define REF_STORE_READ (1 << 0) >> >> I asked [1] in reply to v5 wheth

Re: [PATCH v4 5/5] files_reflog_iterator: amend use of dir_iterator

2017-03-29 Thread Michael Haggerty
On 03/29/2017 02:32 AM, Daniel Ferreira wrote: > Amend a call to dir_iterator_begin() to pass the flags parameter > introduced in 3efb5c0 ("dir_iterator: iterate over dir after its > contents", 2017-28-03). > > Signed-off-by: Daniel Ferreira > --- > refs/files-backend.c | 2 +-

Re: [PATCH v4 3/5] remove_subtree(): reimplement using iterators

2017-03-29 Thread Michael Haggerty
On 03/29/2017 02:32 AM, Daniel Ferreira wrote: > Use dir_iterator to traverse through remove_subtree()'s directory tree, > avoiding the need for recursive calls to readdir(). Simplify > remove_subtree()'s code. > > A conversion similar in purpose was previously done at 46d092a >

Re: [PATCH v4 2/5] dir_iterator: iterate over dir after its contents

2017-03-29 Thread Michael Haggerty
On 03/29/2017 02:32 AM, Daniel Ferreira wrote: > Create an option for the dir_iterator API to iterate over subdirectories > only after having iterated through their contents. This feature was > predicted, although not implemented by 0fe5043 ("dir_iterator: new API > for iterating over a directory

Re: [PATCH v4 1/5] dir_iterator: add helpers to dir_iterator_advance

2017-03-28 Thread Michael Haggerty
On 03/29/2017 02:32 AM, Daniel Ferreira wrote: > Create inline helpers to dir_iterator_advance(). Make > dir_iterator_advance()'s code more legible and allow some behavior to > be reusable. Thanks for breaking up the patches. That makes them a lot easier to review. > Signed-off-by: Daniel

Re: [PATCH v3 1/2] [GSoC] dir_iterator: iterate over dir after its contents

2017-03-26 Thread Michael Haggerty
On 03/25/2017 07:12 PM, Daniel Ferreira wrote: > Create an option for the dir_iterator API to iterate over a directory > path only after having iterated through its contents. This feature was > predicted, although not implemented by 0fe5043 ("dir_iterator: new API > for iterating over a directory

Re: [PATCH v3 1/2] [GSoC] dir_iterator: iterate over dir after its contents

2017-03-26 Thread Michael Haggerty
On 03/25/2017 07:12 PM, Daniel Ferreira wrote: > Create an option for the dir_iterator API to iterate over a directory > path only after having iterated through its contents. This feature was > predicted, although not implemented by 0fe5043 ("dir_iterator: new API > for iterating over a directory

Re: [PATCH 18/20] commit_packed_refs(): use reference iteration

2017-03-22 Thread Michael Haggerty
On 03/20/2017 07:05 PM, Jeff King wrote: > On Mon, Mar 20, 2017 at 05:33:23PM +0100, Michael Haggerty wrote: > >> -/* >> - * An each_ref_entry_fn that writes the entry to a packed-refs file. >> - */ >> -static int write_packed_entry_fn(struct ref_entry *entry, voi

Re: [PATCH 00/20] Separate `ref_cache` into a separate module

2017-03-20 Thread Michael Haggerty
On 03/20/2017 11:32 PM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> Michael Haggerty (20): >> get_ref_dir(): don't call read_loose_refs() for "refs/bisect" >> refs_read_raw_ref(): new function >

Re: [PATCH 11/20] refs: record the ref_store in ref_cache, not ref_dir

2017-03-20 Thread Michael Haggerty
On 03/20/2017 06:51 PM, Jeff King wrote: > On Mon, Mar 20, 2017 at 05:33:16PM +0100, Michael Haggerty wrote: > >> Instead of keeping a pointer to the ref_store in every ref_dir entry, >> store it once in `struct ref_cache`, and change `struct ref_dir` to >> include a p

Re: [PATCH 04/20] refs_verify_refname_available(): implement once for all backends

2017-03-20 Thread Michael Haggerty
On 03/20/2017 06:42 PM, Jeff King wrote: > On Mon, Mar 20, 2017 at 05:33:09PM +0100, Michael Haggerty wrote: > >> It turns out that we can now implement >> `refs_verify_refname_available()` based on the other virtual >> functions, so there is no need for it to be defined

[PATCH 06/20] Rename `add_ref()` to `add_ref_entry()`

2017-03-20 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH 02/20] refs_read_raw_ref(): new function

2017-03-20 Thread Michael Haggerty
Extract a new function from `refs_resolve_ref_unsafe()`. It will be useful elsewhere. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 11 +-- refs/refs-internal.h | 4 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/refs.c b/

[PATCH 04/20] refs_verify_refname_available(): implement once for all backends

2017-03-20 Thread Michael Haggerty
It turns out that we can now implement `refs_verify_refname_available()` based on the other virtual functions, so there is no need for it to be defined at the backend level. Instead, define it once in `refs.c` and remove the `files_backend` definition. Signed-off-by: Michael Haggerty <m

[PATCH 05/20] refs_verify_refname_available(): use function in more places

2017-03-20 Thread Michael Haggerty
(and will be regained later). These were the last callers of `verify_refname_available_dir()`, so also delete that (very complicated) function. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 171 --- 1 file chang

[PATCH 03/20] refs_ref_iterator_begin(): new function

2017-03-20 Thread Michael Haggerty
Extract a new function from `do_for_each_ref()`. It will be useful elsewhere. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 15 +-- refs/refs-internal.h | 11 +++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/re

[PATCH 17/20] cache_ref_iterator_begin(): make function smarter

2017-03-20 Thread Michael Haggerty
ator_begin()` to be made more ignorant of the internals of `ref_cache`, and `find_containing_dir()` and `prime_ref_dir()` to be made private. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 44 +--- refs/ref-

[PATCH 09/20] refs: split `ref_cache` code into separate files

2017-03-20 Thread Michael Haggerty
, adds declarations, and changes the visibility of some functions, but doesn't change any code. The modules are still too tightly coupled, but the situation will be improved in subsequent commits. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- Makefile | 1 + refs

[PATCH 12/20] ref-cache: use a callback function to fill the cache

2017-03-20 Thread Michael Haggerty
` to supply a pointer to function `read_loose_refs` (renamed to `loose_fill_ref_dir`) when creating the ref cache for its loose refs. This means that we can generify the type of the back-pointer in `struct ref_cache` from `files_ref_store` to `ref_store`. Signed-off-by: Michael Haggerty <m

[PATCH 07/20] Rename `find_ref()` to `find_ref_entry()`

2017-03-20 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH 14/20] do_for_each_entry_in_dir(): eliminate `offset` argument

2017-03-20 Thread Michael Haggerty
It was never used. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 4 ++-- refs/ref-cache.c | 6 +++--- refs/ref-cache.h | 11 +-- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/refs/files-backend.c b/refs/files-bac

[PATCH 19/20] files_pack_refs(): use reference iteration

2017-03-20 Thread Michael Haggerty
Use reference iteration rather than do_for_each_entry_in_dir() in the definition of files_pack_refs(). Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 143 +-- 1 file changed, 60 insertions(+), 83 del

[PATCH 15/20] get_loose_ref_dir(): function renamed from get_loose_refs()

2017-03-20 Thread Michael Haggerty
The new name is more analogous to `get_packed_ref_dir()`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 0ff5df6b46..0a16f6196c

[PATCH 18/20] commit_packed_refs(): use reference iteration

2017-03-20 Thread Michael Haggerty
Use reference iteration rather than do_for_each_entry_in_dir() in the definition of commit_packed_refs(). Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 38 +- 1 file changed, 17 insertions(+), 21 deletions(-) diff

[PATCH 08/20] Rename `remove_entry()` to `remove_entry_from_dir()`

2017-03-20 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH 16/20] get_loose_ref_cache(): new function

2017-03-20 Thread Michael Haggerty
Extract a new function, `get_loose_ref_cache()`, from get_loose_ref_dir(). The function returns the `ref_cache` for the loose refs of a `files_ref_store`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 9 +++-- 1 file changed, 7 insertions(+), 2 del

[PATCH 20/20] do_for_each_entry_in_dir(): delete function

2017-03-20 Thread Michael Haggerty
Its only remaining caller was itself. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/ref-cache.c | 21 - refs/ref-cache.h | 11 --- 2 files changed, 32 deletions(-) diff --git a/refs/ref-cache.c b/refs/ref-cache.c index b3a30350d7..6059362f1d

[PATCH 11/20] refs: record the ref_store in ref_cache, not ref_dir

2017-03-20 Thread Michael Haggerty
the size of the individual entries. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- refs/ref-cache.c | 12 +++- refs/ref-cache.h | 9 ++--- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/refs/files-backend.c

[PATCH 10/20] ref-cache: introduce a new type, ref_cache

2017-03-20 Thread Michael Haggerty
For now, it just wraps a `ref_entry *` that points at the root of the tree. Soon it will hold more information. Add two new functions, `create_ref_cache()` and `free_ref_cache()`. Make `free_ref_entry()` private. Change files-backend to use this type to hold its caches. Signed-off-by: Michael

[PATCH 01/20] get_ref_dir(): don't call read_loose_refs() for "refs/bisect"

2017-03-20 Thread Michael Haggerty
nism, and this time the read was done correctly. This code has been broken since it was first introduced. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 42

[PATCH 13/20] refs: handle "refs/bisect/" in `loose_fill_ref_dir()`

2017-03-20 Thread Michael Haggerty
That "refs/bisect/" has to be handled specially when filling the ref_cache for loose references is a peculiarity of the files backend, and the ref-cache code shouldn't need to know about it. So move this code to the callback function, `loose_fill_ref_dir()`. Signed-off-by: Michael Hagg

[PATCH 00/20] Separate `ref_cache` into a separate module

2017-03-20 Thread Michael Haggerty
ox.org/git/20170318020337.22767-1-pclo...@gmail.com/ Michael Haggerty (20): get_ref_dir(): don't call read_loose_refs() for "refs/bisect" refs_read_raw_ref(): new function refs_ref_iterator_begin(): new function refs_verify_refname_available(): implement o

Re: [PATCH v6 17/27] refs: move submodule code out of files-backend.c

2017-03-20 Thread Michael Haggerty
On 03/20/2017 01:09 PM, Duy Nguyen wrote: > On Mon, Mar 20, 2017 at 4:05 AM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >>> [...] >>> diff --git a/refs/refs-internal.h b/refs/refs-internal.h >>> index f732473e1d..dfa1817929 100644 >>>

Re: [PATCH v3 2/4] refs: introduce get_worktree_ref_store()

2017-03-20 Thread Michael Haggerty
On 03/20/2017 01:01 PM, Duy Nguyen wrote: > On Mon, Mar 20, 2017 at 1:59 PM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> I guess I can hold my nose and accept storing worktree and submodule >> `ref_store`s together in a single hashmap > > Release you

Re: [PATCH v3 2/4] refs: introduce get_worktree_ref_store()

2017-03-20 Thread Michael Haggerty
On 03/18/2017 11:02 AM, Nguyễn Thái Ngọc Duy wrote: > files-backend at this point is still aware of the per-repo/worktree > separation in refs, so it can handle a linked worktree. > > Some refs operations are known not working when current files-backend is > used in a linked worktree (e.g.

Re: [PATCH v6 00/27] Remove submodule from files-backend.c

2017-03-19 Thread Michael Haggerty
optional comments, but nothing serious. With or without the suggested changes, I think it's ready to go. Thanks for working on this! Reviewed-by: Michael Haggerty <mhag...@alum.mit.edu> Michael

Re: [PATCH v6 26/27] t1406: new tests for submodule ref store

2017-03-19 Thread Michael Haggerty
On 03/18/2017 03:03 AM, Nguyễn Thái Ngọc Duy wrote: > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > t/t1406-submodule-ref-store.sh (new +x) | 95 > + > 1 file changed, 95 insertions(+) > create mode 100755 t/t1406-submodule-ref-store.sh > >

Re: [PATCH v6 18/27] files-backend: replace submodule_allowed check in files_downcast()

2017-03-19 Thread Michael Haggerty
On 03/18/2017 03:03 AM, Nguyễn Thái Ngọc Duy wrote: > files-backend.c is unlearning submodules. Instead of having a specific > check for submodules to see what operation is allowed, files backend > now takes a set of flags at init. Each operation will check if the > required flags is present

Re: [PATCH v6 17/27] refs: move submodule code out of files-backend.c

2017-03-19 Thread Michael Haggerty
On 03/18/2017 03:03 AM, Nguyễn Thái Ngọc Duy wrote: > files-backend is now initialized with a $GIT_DIR. Converting a submodule > path to where real submodule gitdir is located is done in get_ref_store(). > > This gives a slight performance improvement for submodules since we > don't convert

Re: [PATCH v6 16/27] path.c: move some code out of strbuf_git_path_submodule()

2017-03-19 Thread Michael Haggerty
On 03/18/2017 03:03 AM, Nguyễn Thái Ngọc Duy wrote: > refs is learning to avoid path rewriting that is done by > strbuf_git_path_submodule(). Factor out this code so it could be reused > by refs* Is the "*" on the previous line is a typo, or did you want to add a footnote, or ...? >

Re: [PATCH v6 11/27] refs.c: introduce get_main_ref_store()

2017-03-19 Thread Michael Haggerty
On 03/18/2017 03:03 AM, Nguyễn Thái Ngọc Duy wrote: > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > refs.c | 16 > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/refs.c b/refs.c > index e7606716dd..2637353b72 100644 > --- a/refs.c > +++

Re: [PATCH v6 09/27] files-backend: add and use files_refname_path()

2017-03-19 Thread Michael Haggerty
On 03/18/2017 03:03 AM, Nguyễn Thái Ngọc Duy wrote: > Keep repo-related path handling in one place. This will make it easier > to add submodule/multiworktree support later. > > This automatically adds the "if submodule then use the submodule version > of git_path" to other call sites too. But it

Re: [PATCH v6 03/27] files-backend: delete dead code in files_init_db()

2017-03-19 Thread Michael Haggerty
On 03/18/2017 03:03 AM, Nguyễn Thái Ngọc Duy wrote: > safe_create_dir() can do adjust_shared_perm() internally, and init-db > has always created 'refs' in shared mode since the beginning, > af6e277c5e (git-init-db: initialize shared repositories with --shared - > 2005-12-22). So this code looks

Re: [PATCH v6 02/27] files-backend: make files_log_ref_write() static

2017-03-19 Thread Michael Haggerty
On 03/18/2017 03:03 AM, Nguyễn Thái Ngọc Duy wrote: > Created in 5f3c3a4e6f (files_log_ref_write: new function - 2015-11-10) > but probably never used outside refs-internal.c > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > refs/files-backend.c | 3 +++ > refs/refs-internal.h

Re: Shared repositories no longer securable against privilege escalation

2017-03-17 Thread Michael Haggerty
soft failure. Alternatively, some form of stale > lockfile handling (currently there is none) could be made to work with > a writable HEAD.lock in a read-only bare repository. > > Obigatory HEAD.lock creation was introduced in the following commit: > > commit 92b1551b1d407

Re: [PATCH v5 07/24] files-backend: add and use files_refname_path()

2017-03-09 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > Keep repo-related path handling in one place. This will make it easier > to add submodule/multiworktree support later. > > This automatically adds the "if submodule then use the submodule version > of git_path" to other call sites too. But it

Re: [PATCH v5 24/24] t1406: new tests for submodule ref store

2017-03-03 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > t/t1406-submodule-ref-store.sh (new +x) | 95 > + > 1 file changed, 95 insertions(+) > create mode 100755 t/t1406-submodule-ref-store.sh I

Re: [PATCH v5 00/24] Remove submodule from files-backend.c

2017-03-03 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > v5 goes a bit longer than v4, basically: I've reviewed all of the patches and left a bunch of comments, mostly superficial. I'm very happy about the way it's going, and especially want to say how much I appreciate that you've done so much

Re: [PATCH v5 23/24] t1405: some basic tests on main ref store

2017-03-03 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > t/t1405-main-ref-store.sh (new +x) | 123 > + > 1 file changed, 123 insertions(+) > create mode 100755 t/t1405-main-ref-store.sh > > diff

Re: [PATCH v5 20/24] files-backend: avoid ref api targetting main ref store

2017-03-03 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > A small step towards making files-backend works as a non-main ref store > using the newly added store-aware API. > > For the record, `join` and `nm` on refs.o and files-backend.o tell me > that files-backend no longer uses functions that

Re: [PATCH v5 19/24] refs: new transaction related ref-store api

2017-03-03 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > The transaction struct now takes a ref store at creation and will > operate on that ref store alone. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > refs.c | 54 >

Re: [PATCH v5 16/24] files-backend: replace submodule_allowed check in files_downcast()

2017-03-03 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > files-backend.c is unlearning submodules. Instead of having a specific > check for submodules to see what operation is allowed, files backend > now takes a set of flags at init. Each operation will check if the > required flags is present

Re: [PATCH v5 15/24] refs: move submodule code out of files-backend.c

2017-03-03 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > files-backend is now initialized with a $GIT_DIR. Converting a submodule > path to where real submodule gitdir is located is done in get_ref_store(). > > This gives a slight performance improvement for submodules since we > don't convert

Re: [PATCH v5 24/24] t1406: new tests for submodule ref store

2017-03-02 Thread Michael Haggerty
On 03/02/2017 07:13 AM, Duy Nguyen wrote: > On Wed, Mar 1, 2017 at 10:11 PM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> By trial and error, I found that the test succeeds if I comment out the >> "for_each_reflog()" test. By having that test write its res

Re: [PATCH v5 24/24] t1406: new tests for submodule ref store

2017-03-01 Thread Michael Haggerty
On 03/01/2017 01:34 PM, Duy Nguyen wrote: > On Wed, Mar 1, 2017 at 12:34 AM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: >>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com> >>> --- &

Re: [PATCH v5 12/24] refs.c: kill register_ref_store(), add register_submodule_ref_store()

2017-03-01 Thread Michael Haggerty
On 03/01/2017 01:00 PM, Duy Nguyen wrote: > On Wed, Mar 1, 2017 at 1:03 AM, Michael Haggerty <mhag...@alum.mit.edu> wrote: >>> struct ref_store *get_ref_store(const char *submodule) >>> { >>> struct strbuf submodule_sb = STRBUF_INIT; >&g

Re: [PATCH v5 14/24] path.c: move some code out of strbuf_git_path_submodule()

2017-02-28 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > refs is learning to avoid path rewriting that is done by > strbuf_git_path_submodule(). Factor out this code so it could be reused > by refs* > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > path.c | 34

Re: [PATCH v5 00/24] Remove submodule from files-backend.c

2017-02-28 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > v5 goes a bit longer than v4, basically: I've read through patch 14/24 so far, and they all look good except for the mostly superficial comments that I have sent so far. I like the way this is heading! I'll try to continue tomorrow. Michael

Re: [PATCH v5 13/24] refs.c: make get_main_ref_store() public and use it

2017-02-28 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > get_ref_store() will soon be renamed to get_submodule_ref_store(). > Together with future get_worktree_ref_store(), the three functions > provide an appropriate ref store for different operation modes. New APIs > will be added to operate

Re: [PATCH v5 12/24] refs.c: kill register_ref_store(), add register_submodule_ref_store()

2017-02-28 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > This is the last function in this code (besides public API) that takes > submodule argument and handles both main/submodule cases. Break it down, > move main store registration in get_main_ref_store() and keep the rest > in

Re: [PATCH v5 08/24] files-backend: remove the use of git_path()

2017-02-28 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > Given $GIT_DIR and $GIT_COMMON_DIR, files-backend is now in charge of > deciding what goes where (*). The end goal is to pass $GIT_DIR only. A > refs "view" of a linked worktree is a logical ref store that combines > two files backends

Re: [PATCH v5 09/24] refs.c: introduce get_main_ref_store()

2017-02-28 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > refs.c | 16 > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/refs.c b/refs.c > index 81b64b4ed..dab1a21ac 100644 > --- a/refs.c > +++

Re: [PATCH v5 07/24] files-backend: add and use files_refname_path()

2017-02-28 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > Keep repo-related path handling in one place. This will make it easier > to add submodule/multiworktree support later. > > This automatically adds the "if submodule then use the submodule version > of git_path" to other call sites too. But it

Re: [PATCH v5 24/24] t1406: new tests for submodule ref store

2017-02-28 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > t/t1406-submodule-ref-store.sh (new +x) | 95 > + > 1 file changed, 95 insertions(+) > create mode 100755 t/t1406-submodule-ref-store.sh > >

Re: [PATCH v5 05/24] files-backend: move "logs/" out of TMP_RENAMED_LOG

2017-02-28 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > This makes reflog path building consistent, always in the form of > > strbuf_git_path(sb, "logs/%s", refname); > > It reduces the mental workload a bit in the next patch when that > function call is converted. > > Signed-off-by: Nguyễn

Re: [PATCH v5 04/24] files-backend: convert git_path() to strbuf_git_path()

2017-02-28 Thread Michael Haggerty
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > git_path() and friends are going to be killed in files-backend.c in near > future. And because there's a risk with overwriting buffer in > git_path(), let's convert them all to strbuf_git_path(). We'll have > easier time killing/converting

Re: [PATCH v4 00/15] Remove submodule from files-backend.c

2017-02-20 Thread Michael Haggerty
On 02/18/2017 02:32 PM, Nguyễn Thái Ngọc Duy wrote: > v4: I very much like the direction of this patch series. I reviewed the design pretty carefully and left some comments about it, and skimmed through the code changes. But I haven't yet reviewed the code in detail. I'll wait for your reaction

Re: [PATCH v4 11/15] refs.c: make get_main_ref_store() public and use it

2017-02-20 Thread Michael Haggerty
On 02/18/2017 02:32 PM, Nguyễn Thái Ngọc Duy wrote: > get_ref_store() will soon be renamed to get_submodule_ref_store(). > Together with future get_worktree_ref_store(), the three functions > provide an appropriate ref store for different operation modes. New APIs > will be added to operate

Re: [PATCH v4 14/15] files-backend: remove submodule_allowed from files_downcast()

2017-02-20 Thread Michael Haggerty
On 02/20/2017 01:33 PM, Duy Nguyen wrote: > On Mon, Feb 20, 2017 at 7:30 PM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> On 02/20/2017 01:21 PM, Duy Nguyen wrote: >>> On Mon, Feb 20, 2017 at 7:11 PM, Michael Haggerty <mhag...@alum.mit.edu> >>> w

Re: [PATCH v4 14/15] files-backend: remove submodule_allowed from files_downcast()

2017-02-20 Thread Michael Haggerty
On 02/20/2017 01:21 PM, Duy Nguyen wrote: > On Mon, Feb 20, 2017 at 7:11 PM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> On 02/18/2017 02:33 PM, Nguyễn Thái Ngọc Duy wrote: >>> Since submodule or not is irrelevant to files-backend after the last >>&

Re: [PATCH v4 14/15] files-backend: remove submodule_allowed from files_downcast()

2017-02-20 Thread Michael Haggerty
On 02/18/2017 02:33 PM, Nguyễn Thái Ngọc Duy wrote: > Since submodule or not is irrelevant to files-backend after the last > patch, remove the parameter from files_downcast() and kill > files_assert_main_repository(). > > PS. This implies that all ref operations are allowed for submodules. But >

Re: [PATCH v4 06/15] files-backend: remove the use of git_path()

2017-02-20 Thread Michael Haggerty
On 02/18/2017 02:32 PM, Nguyễn Thái Ngọc Duy wrote: > Given $GIT_DIR and $GIT_COMMON_DIR, files-backend is now in charge of > deciding what goes where. The end goal is to pass $GIT_DIR only. A > refs "view" of a linked worktree is a logical ref store that combines > two files backends together. >

Re: [PATCH v2 04/16] files-backend: replace *git_path*() with files_path()

2017-02-20 Thread Michael Haggerty
On 02/16/2017 12:48 PM, Nguyễn Thái Ngọc Duy wrote: > This centralizes all path rewriting of files-backend.c in one place so > we have easier time removing the path rewriting later. There could be > some hidden indirect git_path() though, I didn't audit the code to the > bottom. Almost all of the

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

2017-02-17 Thread Michael Haggerty
On 02/17/2017 09:07 AM, Jeff King wrote: > [...] > That's similar to what I wrote earlier, but if we don't mind overwriting > errno unconditionally, I think just: > > errno = EIO; /* covers ferror(), overwritten by failing fclose() */ > err |= ferror(fp); > err |= fclose(fp); > > does the

Re: [PATCH v2 00/19] object_id part 6

2017-02-17 Thread Michael Haggerty
On 02/14/2017 03:31 AM, brian m. carlson wrote: > This is another series in the continuing conversion to struct object_id. > > This series converts more of the builtin directory and some of the refs > code to use struct object_id. Additionally, it implements an > nth_packed_object_oid function

Re: [PATCH v2 15/19] refs: simplify parsing of reflog entries

2017-02-17 Thread Michael Haggerty
On 02/14/2017 03:31 AM, brian m. carlson wrote: > The current code for reflog entries uses a lot of hard-coded constants, > making it hard to read and modify. Use parse_oid_hex and two temporary > variables to simplify the code and reduce the use of magic constants. > > Signed-off-by: brian m.

Re: [PATCH v2 14/19] hex: introduce parse_oid_hex

2017-02-17 Thread Michael Haggerty
On 02/14/2017 03:31 AM, brian m. carlson wrote: > Introduce a function, parse_oid_hex, which parses a hexadecimal object > ID and if successful, sets a pointer to just beyond the last character. > This allows for simpler, more robust parsing without needing to > hard-code integer values throughout

Re: [PATCH v2 11/19] builtin/replace: convert to struct object_id

2017-02-17 Thread Michael Haggerty
On 02/14/2017 03:31 AM, brian m. carlson wrote: > Convert various uses of unsigned char [20] to struct object_id. Rename > replace_object_sha1 to rename_object_oid. Finally, specify a constant > in terms of GIT_SHA1_HEXSZ. The new name is not rename_object_oid but rather replace_object_oid. >

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

2017-02-17 Thread Michael Haggerty
On 02/16/2017 10:31 PM, Jeff King wrote: > On Thu, Feb 16, 2017 at 11:43:59AM -0500, Jeff King wrote: > >> On Thu, Feb 16, 2017 at 11:10:18AM +0100, Andreas Schwab wrote: >> > int xfclose(FILE *fp) > { > return ferror(fp) | fclose(fp); > } Yes, that's

Re: [PATCH v2 9/9] read_loose_refs(): read refs using resolve_ref_recursively()

2017-02-12 Thread Michael Haggerty
On 02/10/2017 08:22 PM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: >> [...] > > OK, but one thing puzzles me... > >> @@ -1390,27 +1390,6 @@ static struct ref_store *main_ref_store; >> static struct hashmap submodule_r

[PATCH v2 5/9] refs: store submodule ref stores in a hashmap

2017-02-10 Thread Michael Haggerty
Aside from scaling better, this means that the submodule name needn't be stored in the ref_store instance anymore (which will be changed in a moment). This, in turn, will help loosen the strict 1:1 relationship between ref_stores and submodules. Signed-off-by: Michael Haggerty <m

<    1   2   3   4   5   6   7   8   9   10   >