fix per-worktree ref iteration in fsck/reflog expire
cover fix per-worktree ref iteration in fsck/reflog expire
Sorry it took me so long to revisit these bugs, even though the first one was reported nearly a year ago. I guess I slept on it way longer than I should have.
This series opens up the currrent worktree's ref space, allowing one worktree to see refs from other worktrees. With this, it's possible to have less confusing error reports (e.g. "heh.. what HEAD are you talking about?"). And it's possible to peek one worktree from another, e.g. now I could do
git checkout main/HEAD
make test -j8
in one terminal and go back to the original terminal and continue hacking while tests are being run in the background.
With this issue out of the way, I could now continue to fix the "forget to look at all HEADs/reflogs" problem in fsck and "reflog expire". The fsck patches are mostly from Elijah with some code/test adaptation from me.
I also take this opportunity to try to standardize a common/private space in $GIT_DIR or ref hierarchy so we don't have to add more rules in the future.
One heads up. One remaining problem with "refs and worktrees" is the ability to completely separate ref space between worktrees (i.e. refs/heads/master on worktree A is completely different than one in worktree B). This is needed to make use worktrees in submodules. But the way I see it, per-worktree refs may have to be moved back to $GIT_COMMON_DIR/refs to be efficient.
This may be backward incompatible change. Haven't thought it through yet (and didn't see it coming because I largely ignored refs/bisect, which should have made me think about this much earlier)
Elijah Newren (2):
fsck: Move fsck_head_link() to get_default_heads() to avoid some globals
fsck: check HEAD and reflog from other worktrees
Nguyễn Thái Ngọc Duy (6):
refs.c: indent with tabs, not spaces
Add a place for (not) sharing stuff between worktrees
refs: new ref types to make per-worktree refs visible to all worktrees revision.c: correct a parameter name
revision.c: better error reporting on ref from different worktrees reflog expire: cover reflog from all worktrees
Documentation/git-reflog.txt | 7 ++- Documentation/gitrepository-layout.txt | 11 ++++- builtin/fsck.c | 68 ++++++++++++++++++-------- builtin/reflog.c | 22 +++++++-- path.c | 1 + refs.c | 23 ++++++++- refs.h | 8 +-- refs/files-backend.c | 42 ++++++++++++++-- revision.c | 22 ++++++--- t/t0060-path-utils.sh | 2 + t/t1415-worktree-refs.sh | 66 +++++++++++++++++++++++++ t/t1450-fsck.sh | 39 +++++++++++++++ worktree.c | 32 ++++++++++-- worktree.h | 14 ++++++ 14 files changed, 312 insertions(+), 45 deletions(-) create mode 100755 t/t1415-worktree-refs.sh
Signature · 1 line
2.19.0.647.gb9a6049235
PATCH 1/8 refs.c: indent with tabs, not spaces
--- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
refs.c +1 -1
@@ -646,7 +646,7 @@ enum ref_type ref_type(const char *refname)return REF_TYPE_PER_WORKTREE ;if (is_pseudoref_syntax (refname))return REF_TYPE_PSEUDOREF ; -return REF_TYPE_NORMAL ; +return REF_TYPE_NORMAL ; }long get_files_ref_lock_timeout_ms (void )
Signature · 1 line
2.19.0.647.gb9a6049235
PATCH 2/8 Add a place for (not) sharing stuff between worktrees
In reply to Nguyễn Thái Ngọc DuyWhen multiple worktrees are used, we need rules to determine if something belongs to one worktree or all of them. Instead of keeping adding rules when new stuff comes, have a generic rule:
- Inside $GIT_DIR, which is per-worktree by default, add $GIT_DIR/common which is always shared. New features that want to share stuff should put stuff under this directory.
- Inside refs/, which is shared by default except refs/bisect, add refs/local/ which is per-worktree. We may eventually move refs/bisect to this new location and remove the exception in refs code.
--- Documentation/gitrepository-layout.txt | 11 ++++++-- path.c | 1 + refs.c | 1 + refs/files-backend.c | 14 +++++++--- t/t0060-path-utils.sh | 2 ++ t/t1415-worktree-refs.sh | 36 ++++++++++++++++++++++++++ 6 files changed, 60 insertions(+), 5 deletions(-) create mode 100755 t/t1415-worktree-refs.sh
Documentation/gitrepository-layout.txt +9 -2
@@ -95,8 +95,10 @@ refs:: References are stored in subdirectories of this directory. The 'git prune' command knows to preserve objects reachable from refs found in this directory and - its subdirectories. This directory is ignored if $GIT_COMMON_DIR - is set and "$GIT_COMMON_DIR/refs" will be used instead. + its subdirectories. + This directory is ignored (except refs/bisect and refs/local) + if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/refs" will be + used instead. refs/heads/`name`:: records tip-of-the-tree commit objects of branch `name`
@@ -165,6 +167,11 @@ hooks:: each hook. This directory is ignored if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/hooks" will be used instead. +common:: + When multiple working trees are used, most of files in + $GIT_DIR are per-worktree with a few known exceptions. All + files under 'common' however will be shared between all + working trees. index:: The current index file for the repository. It is
path.c +1 -0
@@ -108,6 +108,7 @@ struct common_dir {static struct common_dir common_list[] = { {0 ,1 ,0 ,"branches" }, + {0 ,1 ,0 ,"common" }, {0 ,1 ,0 ,"hooks" }, {0 ,1 ,0 ,"info" }, {0 ,0 ,1 ,"info/sparse-checkout" },
refs.c +1 -0
@@ -624,6 +624,7 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)static int is_per_worktree_ref (const char *refname) {return !strcmp (refname,"HEAD" ) || +starts_with (refname,"refs/local/" ) ||starts_with (refname,"refs/bisect/" ) ||starts_with (refname,"refs/rewritten/" ); }
refs/files-backend.c +11 -3
@@ -269,9 +269,9 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,closedir (d);/* -* Manually add refs/bisect, which, being per-worktree, might -* not appear in the directory listing for refs/ in the main -* repo. +* Manually add refs/bisect and refs/local, which, being +* per-worktree, might not appear in the directory listing for +* refs/ in the main repo. */ if (!strcmp (dirname,"refs/" )) {int pos =search_ref_dir (dir,"refs/bisect/" ,12 );
@@ -281,6 +281,14 @@ static void loose_fill_ref_dir(struct ref_store *ref_store, dir->cache, "refs/bisect/", 12, 1);add_entry_to_dir (dir, child_entry); } + + pos =search_ref_dir (dir,"refs/local/" ,11 ); + +if (pos <0 ) { +struct ref_entry *child_entry =create_dir_entry ( + dir->cache,"refs/local/" ,11 ,1 ); +add_entry_to_dir (dir, child_entry); + } } }
t/t0060-path-utils.sh +2 -0
@@ -306,6 +306,8 @@ test_git_path GIT_COMMON_DIR=bar hooks/me bar/hooks/me test_git_path GIT_COMMON_DIR=bar config bar/config test_git_path GIT_COMMON_DIR=bar packed-refs bar/packed-refs test_git_path GIT_COMMON_DIR=bar shallow bar/shallow +test_git_path GIT_COMMON_DIR=bar common bar/common +test_git_path GIT_COMMON_DIR=bar common/file bar/common/file # In the tests below, $(pwd) must be used because it is a native path on # Windows and avoids MSYS's path mangling (which simplifies "foo/../bar" and
t/t1415-worktree-refs.sh new file +36 -0
@@ -0,0 +1,36 @@ +#!/bin/sh + +test_description='per-worktree refs' + +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit initial && + test_commit wt1 && + test_commit wt2 && + git worktree add wt1 wt1 && + git worktree add wt2 wt2 && + git checkout initial +' + +test_expect_success 'add refs/local' ' + git update-ref refs/local/foo HEAD && + git -C wt1 update-ref refs/local/foo HEAD && + git -C wt2 update-ref refs/local/foo HEAD +' + +test_expect_success 'refs/local must not be packed' ' + git pack-refs --all && + test_path_is_missing .git/refs/tags/wt1 && + test_path_is_file .git/refs/local/foo && + test_path_is_file .git/worktrees/wt1/refs/local/foo && + test_path_is_file .git/worktrees/wt2/refs/local/foo +' + +test_expect_success 'refs/local are per-worktree' ' + test_cmp_rev local/foo initial && + ( cd wt1 && test_cmp_rev local/foo wt1 ) && + ( cd wt2 && test_cmp_rev local/foo wt2 ) +' + +test_done
Signature · 1 line
2.19.0.647.gb9a6049235
PATCH 3/8 refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Nguyễn Thái Ngọc DuyOne of the problems with multiple worktree is accessing per-worktree refs of one worktree from another worktree. This was sort of solved by multiple ref store, where the code can open the ref store of another worktree and has access to the ref space of that worktree.
The problem with this is reporting. "HEAD" in another ref space is also called "HEAD" like in the current ref space. In order to differentiate them, all the code must somehow carry the ref store around and print something like "HEAD from this ref store".
But that is not feasible (or possible with a _lot_ of work). With the current design, we pass a reference around as a string (so called "refname"). Extending this design to pass a string _and_ a ref store is a nightmare, especially when handling extended SHA-1 syntax.
So we do it another way. Instead of entering a separate ref space, we make refs from other worktrees available in the current ref space. So "HEAD" is always HEAD of the current worktree, but then we can have "worktrees/blah/HEAD" to denote HEAD from a worktree named "blah". This syntax coincidentally matches the underlying directory structure which makes implementation a bit easier.
The main worktree has to be treated specially because well.. it's special from the beginning. So HEAD from the main worktree is acccessible via the name "main/HEAD" (we can't use "worktrees/main/HEAD" because "main" under "worktrees" is not reserved).
This patch also makes it possible to specify refs from one worktree in another one, e.g.
git log worktrees/foo/HEAD
--- refs.c | 20 ++++++++++++++++++++ refs.h | 8 +++++--- refs/files-backend.c | 28 ++++++++++++++++++++++++++++ t/t1415-worktree-refs.sh | 30 ++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 3 deletions(-)
refs.c +20 -0
@@ -641,12 +641,32 @@ static int is_pseudoref_syntax(const char *refname)return 1 ; } +static int is_main_pseudoref_syntax (const char *refname) +{ +return skip_prefix (refname,"main/" , &refname) && +is_pseudoref_syntax (refname); +} + +static int is_other_pseudoref_syntax (const char *refname) +{ +if (!skip_prefix (refname,"worktrees/" , &refname)) +return 0 ; + refname =strchr (refname,'/' ); +if (!refname) +return 0 ; +return is_pseudoref_syntax (refname +1 ); +} +enum ref_type ref_type (const char *refname) {if (is_per_worktree_ref (refname))return REF_TYPE_PER_WORKTREE ;if (is_pseudoref_syntax (refname))return REF_TYPE_PSEUDOREF ; +if (is_main_pseudoref_syntax (refname)) +return REF_TYPE_MAIN_PSEUDOREF ; +if (is_other_pseudoref_syntax (refname)) +return REF_TYPE_OTHER_PSEUDOREF ;return REF_TYPE_NORMAL ; }
refs.h +5 -3
@@ -704,9 +704,11 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);int ref_is_hidden (const char *,const char *);enum ref_type { -REF_TYPE_PER_WORKTREE , -REF_TYPE_PSEUDOREF , -REF_TYPE_NORMAL , +REF_TYPE_PER_WORKTREE ,/* refs inside refs/ but not shared */ +REF_TYPE_PSEUDOREF ,/* refs outside refs/ in current worktree */ +REF_TYPE_MAIN_PSEUDOREF ,/* pseudo refs from the main worktree */ +REF_TYPE_OTHER_PSEUDOREF ,/* pseudo refs from other worktrees */ +REF_TYPE_NORMAL ,/* normal/shared refs inside refs/ */ };enum ref_type ref_type (const char *refname);
refs/files-backend.c +28 -0
@@ -149,6 +149,23 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,return refs; } +static void files_reflog_path_other_worktrees (struct files_ref_store *refs, +struct strbuf *sb, +const char *refname) +{ +const char *real_ref; + +if (!skip_prefix (refname,"worktrees/" , &real_ref)) +BUG ("refname %s is not a other-worktree ref" , refname); + real_ref =strchr (real_ref,'/' ); +if (!real_ref) +BUG ("refname %s is not a other-worktree ref" , refname); + real_ref++; + +strbuf_addf (sb,"%s/%.*slogs/%s" , refs->gitcommondir, + (int )(real_ref - refname), refname, real_ref); +} +static void files_reflog_path (struct files_ref_store *refs,struct strbuf *sb,const char *refname)
@@ -158,6 +175,12 @@ static void files_reflog_path(struct files_ref_store *refs,case REF_TYPE_PSEUDOREF :strbuf_addf (sb,"%s/logs/%s" , refs->gitdir, refname);break ; +case REF_TYPE_OTHER_PSEUDOREF : +return files_reflog_path_other_worktrees (refs, sb, refname); +case REF_TYPE_MAIN_PSEUDOREF : +if (!skip_prefix (refname,"main/" , &refname)) +BUG ("ref %s is not a main pseudoref" , refname); +/* passthru */ case REF_TYPE_NORMAL :strbuf_addf (sb,"%s/logs/%s" , refs->gitcommondir, refname);break ;
@@ -176,6 +199,11 @@ static void files_ref_path(struct files_ref_store *refs,case REF_TYPE_PSEUDOREF :strbuf_addf (sb,"%s/%s" , refs->gitdir, refname);break ; +case REF_TYPE_MAIN_PSEUDOREF : +if (!skip_prefix (refname,"main/" , &refname)) +BUG ("ref %s is not a main pseudoref" , refname); +/* passthru */ +case REF_TYPE_OTHER_PSEUDOREF :case REF_TYPE_NORMAL :strbuf_addf (sb,"%s/%s" , refs->gitcommondir, refname);break ;
t/t1415-worktree-refs.sh +30 -0
@@ -33,4 +33,34 @@ test_expect_success 'refs/local are per-worktree' ' ( cd wt2 && test_cmp_rev local/foo wt2 ) ' +test_expect_success 'resolve main/HEAD' ' + test_cmp_rev main/HEAD initial && + ( cd wt1 && test_cmp_rev main/HEAD initial ) && + ( cd wt2 && test_cmp_rev main/HEAD initial ) +' + +test_expect_success 'resolve worktrees/xx/HEAD' ' + test_cmp_rev worktrees/wt1/HEAD wt1 && + ( cd wt1 && test_cmp_rev worktrees/wt1/HEAD wt1 ) && + ( cd wt2 && test_cmp_rev worktrees/wt1/HEAD wt1 ) +' + +test_expect_success 'reflog of main/HEAD' ' + git reflog HEAD | sed "s/HEAD/main\/HEAD/" >expected && + git reflog main/HEAD >actual && + test_cmp expected actual && + git -C wt1 reflog main/HEAD >actual.wt1 && + test_cmp expected actual.wt1 +' + +test_expect_success 'reflog of worktrees/xx/HEAD' ' + git -C wt2 reflog HEAD | sed "s/HEAD/worktrees\/wt2\/HEAD/" >expected && + git reflog worktrees/wt2/HEAD >actual && + test_cmp expected actual && + git -C wt1 reflog worktrees/wt2/HEAD >actual.wt1 && + test_cmp expected actual.wt1 && + git -C wt2 reflog worktrees/wt2/HEAD >actual.wt2 && + test_cmp expected actual.wt2 +' + test_done
Signature · 1 line
2.19.0.647.gb9a6049235
PATCH 4/8 revision.c: correct a parameter name
In reply to Nguyễn Thái Ngọc DuyThis function is a callback of for_each_reflog() which will pass a ref name as the first argument, not a path (to a reflog file).
--- revision.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
revision.c +4 -3
@@ -1277,13 +1277,14 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,return 0 ; } -static int handle_one_reflog (const char *path,const struct object_id *oid, +static int handle_one_reflog (const char *refname, +const struct object_id *oid,int flag,void *cb_data) {struct all_refs_cb *cb = cb_data; cb->warned_bad_reflog =0 ; - cb->name_for_errormsg = path; -refs_for_each_reflog_ent (cb->refs, path, + cb->name_for_errormsg = refname; +refs_for_each_reflog_ent (cb->refs, refname, handle_one_reflog_ent, cb_data);return 0 ; }
Signature · 1 line
2.19.0.647.gb9a6049235
PATCH 5/8 revision.c: better error reporting on ref from different worktrees
In reply to Nguyễn Thái Ngọc DuyMake use of the new ref aliases to pass refs from another worktree around and access them from the current ref store instead. This does not change any functionality, but when a problem shows up, we would report something like
fatal: bad object worktrees/ztemp/HEAD
warning: reflog of 'main/HEAD' references pruned commits
instead of
fatal: bad object HEAD
warning: reflog of 'HEAD' references pruned commits
which does not really tell where the refs are from.
--- revision.c | 21 +++++++++++++-------- worktree.c | 32 +++++++++++++++++++++++++++++--- worktree.h | 14 ++++++++++++++ 3 files changed, 56 insertions(+), 11 deletions(-)
revision.c +13 -8
@@ -1177,7 +1177,7 @@ struct all_refs_cb {int warned_bad_reflog;struct rev_info *all_revs;const char *name_for_errormsg; -struct ref_store *refs; +struct worktree *wt; };int ref_excluded (struct string_list *ref_excludes,const char *path)
@@ -1214,7 +1214,7 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs, cb->all_revs = revs; cb->all_flags = flags; revs->rev_input_given =1 ; - cb->refs =NULL ; + cb->wt =NULL ; }void clear_ref_exclusion (struct string_list **ref_excludes_p)
@@ -1277,15 +1277,20 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,return 0 ; } -static int handle_one_reflog (const char *refname, +static int handle_one_reflog (const char *refname_in_wt,const struct object_id *oid,int flag,void *cb_data) {struct all_refs_cb *cb = cb_data; +struct strbuf refname =STRBUF_INIT ; + cb->warned_bad_reflog =0 ; - cb->name_for_errormsg = refname; -refs_for_each_reflog_ent (cb->refs, refname, +strbuf_worktree_ref (cb->wt, &refname, refname_in_wt); + cb->name_for_errormsg = refname.buf; +refs_for_each_reflog_ent (get_main_ref_store (the_repository), + refname.buf, handle_one_reflog_ent, cb_data); +strbuf_release (&refname);return 0 ; }
@@ -1300,8 +1305,8 @@ static void add_other_reflogs_to_pending(struct all_refs_cb *cb)if (wt->is_current)continue ; - cb->refs =get_worktree_ref_store (wt); -refs_for_each_reflog (cb->refs, + cb->wt = wt; +refs_for_each_reflog (get_worktree_ref_store (wt), handle_one_reflog, cb); }
@@ -1314,7 +1319,7 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags) cb.all_revs = revs; cb.all_flags = flags; - cb.refs =get_main_ref_store (the_repository); + cb.wt =NULL ;for_each_reflog (handle_one_reflog, &cb);if (!revs->single_worktree)
worktree.c +29 -3
@@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path)return ret; } +void strbuf_worktree_ref (const struct worktree *wt, +struct strbuf *sb, +const char *refname) +{ +if (wt && !wt->is_current) { +if (is_main_worktree (wt)) +strbuf_addstr (sb,"main/" ); +else +strbuf_addf (sb,"worktrees/%s/" , wt->id); + } +strbuf_addstr (sb, refname); +} + +const char *worktree_ref (const struct worktree *wt,const char *refname) +{ +static struct strbuf sb =STRBUF_INIT ; + +strbuf_reset (&sb); +strbuf_worktree_ref (wt, &sb, refname); +return sb.buf; +} +int other_head_refs (each_ref_fn fn,void *cb_data) {struct worktree **worktrees, **p;
@@ -495,13 +517,17 @@ int other_head_refs(each_ref_fn fn, void *cb_data) worktrees =get_worktrees (0 );for (p = worktrees; *p; p++) {struct worktree *wt = *p; -struct ref_store *refs; +struct object_id oid; +int flag;if (wt->is_current)continue ; - refs =get_worktree_ref_store (wt); - ret =refs_head_ref (refs, fn, cb_data); +if (!refs_read_ref_full (get_main_ref_store (the_repository), +worktree_ref (wt,"HEAD" ), +RESOLVE_REF_READING , + &oid, &flag)) + ret =fn (worktree_ref (wt,"HEAD" ), &oid, flag, cb_data);if (ret)break ; }
worktree.h +14 -0
@@ -108,4 +108,18 @@ extern const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...) __attribute__((format (printf, 2, 3))); +/* +* Return a refname suitable for access from the current ref store. +*/ +void strbuf_worktree_ref (const struct worktree *wt, +struct strbuf *sb, +const char *refname); + +/* +* Return a refname suitable for access from the current ref +* store. The result may be destroyed at the next call. +*/ +const char *worktree_ref (const struct worktree *wt, +const char *refname); +#endif
Signature · 1 line
2.19.0.647.gb9a6049235
PATCH 7/8 fsck: check HEAD and reflog from other worktrees
In reply to Nguyễn Thái Ngọc DuyFrom: Elijah Newren <[email protected]>
fsck is a repo-wide operation and should check all references no matter which worktree they are associated to.
--- builtin/fsck.c | 55 ++++++++++++++++++++++++++++++++++--------------- t/t1450-fsck.sh | 39 +++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 17 deletions(-)
builtin/fsck.c +38 -17
@@ -19,6 +19,7 @@#include "packfile.h" #include "object-store.h" #include "run-command.h" +#include "worktree.h" #define REACHABLE 0x0001#define SEEN 0x0002
@@ -444,7 +445,11 @@ static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noidstatic int fsck_handle_reflog (const char *logname,const struct object_id *oid,int flag,void *cb_data) { -for_each_reflog_ent (logname, fsck_handle_reflog_ent, (void *)logname); +struct strbuf refname =STRBUF_INIT ; + +strbuf_worktree_ref (cb_data, &refname, logname); +for_each_reflog_ent (refname.buf, fsck_handle_reflog_ent, refname.buf); +strbuf_release (&refname);return 0 ; }
@@ -482,20 +487,34 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,return 0 ; } -static int fsck_head_link (const char **head_points_at, +static int fsck_head_link (const char *head_ref_name, +const char **head_points_at,struct object_id *head_oid);static void get_default_heads (void ) { +struct worktree **worktrees, **p;const char *head_points_at;struct object_id head_oid; -fsck_head_link (&head_points_at, &head_oid); -if (head_points_at && !is_null_oid (&head_oid)) -fsck_handle_ref ("HEAD" , &head_oid,0 ,NULL );for_each_rawref (fsck_handle_ref,NULL ); -if (include_reflogs) -for_each_reflog (fsck_handle_reflog,NULL ); + + worktrees =get_worktrees (0 ); +for (p = worktrees; *p; p++) { +struct worktree *wt = *p; +struct strbuf ref =STRBUF_INIT ; + +strbuf_worktree_ref (wt, &ref,"HEAD" ); +fsck_head_link (ref.buf, &head_points_at, &head_oid); +if (head_points_at && !is_null_oid (&head_oid)) +fsck_handle_ref (ref.buf, &head_oid,0 ,NULL ); +strbuf_release (&ref); + +if (include_reflogs) +refs_for_each_reflog (get_worktree_ref_store (wt), + fsck_handle_reflog, wt); + } +free_worktrees (worktrees);/* * Not having any default heads isn't really fatal, but
@@ -584,34 +603,36 @@ static void fsck_object_dir(const char *path)stop_progress (&progress); } -static int fsck_head_link (const char **head_points_at, +static int fsck_head_link (const char *head_ref_name, +const char **head_points_at,struct object_id *head_oid) {int null_is_error =0 ;if (verbose) -fprintf (stderr,"Checking HEAD link\n" ); +fprintf (stderr,"Checking %s link\n" , head_ref_name); - *head_points_at =resolve_ref_unsafe ("HEAD" ,0 , head_oid,NULL ); + *head_points_at =resolve_ref_unsafe (head_ref_name,0 , head_oid,NULL );if (!*head_points_at) { errors_found |=ERROR_REFS ; -return error ("Invalid HEAD" ); +return error ("Invalid %s" , head_ref_name); } -if (!strcmp (*head_points_at,"HEAD" )) +if (!strcmp (*head_points_at, head_ref_name))/* detached HEAD */ null_is_error =1 ;else if (!starts_with (*head_points_at,"refs/heads/" )) { errors_found |=ERROR_REFS ; -return error ("HEAD points to something strange (%s)" , - *head_points_at); +return error ("%s points to something strange (%s)" , + head_ref_name, *head_points_at); }if (is_null_oid (head_oid)) {if (null_is_error) { errors_found |=ERROR_REFS ; -return error ("HEAD: detached HEAD points at nothing" ); +return error ("%s: detached HEAD points at nothing" , + head_ref_name); } -fprintf (stderr,"notice: HEAD points to an unborn branch (%s)\n" , - *head_points_at +11 ); +fprintf (stderr,"notice: %s points to an unborn branch (%s)\n" , + head_ref_name, *head_points_at +11 ); }return 0 ; }
t/t1450-fsck.sh +39 -0
@@ -101,6 +101,45 @@ test_expect_success 'HEAD link pointing at a funny place' ' grep "HEAD points to something strange" out ' +test_expect_success 'HEAD link pointing at a funny object (from different wt)' ' + test_when_finished "mv .git/SAVED_HEAD .git/HEAD" && + test_when_finished "rm -rf .git/worktrees wt" && + git worktree add wt && + mv .git/HEAD .git/SAVED_HEAD && + echo 0000000000000000000000000000000000000000 >.git/HEAD && + # avoid corrupt/broken HEAD from interfering with repo discovery + test_must_fail git -C wt fsck 2>out && + cat out && + grep "main/HEAD: detached HEAD points" out +' + +test_expect_success 'other worktree HEAD link pointing at a funny object' ' + test_when_finished "rm -rf .git/worktrees other" && + git worktree add other && + echo 0000000000000000000000000000000000000000 >.git/worktrees/other/HEAD && + test_must_fail git fsck 2>out && + cat out && + grep "worktrees/other/HEAD: detached HEAD points" out +' + +test_expect_success 'other worktree HEAD link pointing at missing object' ' + test_when_finished "rm -rf .git/worktrees other" && + git worktree add other && + echo "Contents missing from repo" | git hash-object --stdin >.git/worktrees/other/HEAD && + test_must_fail git fsck 2>out && + cat out && + grep "worktrees/other/HEAD: invalid sha1 pointer" out +' + +test_expect_success 'other worktree HEAD link pointing at a funny place' ' + test_when_finished "rm -rf .git/worktrees other" && + git worktree add other && + echo "ref: refs/funny/place" >.git/worktrees/other/HEAD && + test_must_fail git fsck 2>out && + cat out && + grep "worktrees/other/HEAD points to something strange" out +' + test_expect_success 'email without @ is okay' ' git cat-file commit HEAD >basis && sed "s/@/AT/" basis >okay &&
Signature · 1 line
2.19.0.647.gb9a6049235
PATCH 8/8 reflog expire: cover reflog from all worktrees
In reply to Nguyễn Thái Ngọc Duy--- Documentation/git-reflog.txt | 7 ++++++- builtin/reflog.c | 22 +++++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-)
Documentation/git-reflog.txt +6 -1
@@ -20,7 +20,7 @@ depending on the subcommand: 'git reflog' ['show'] [log-options] [<ref>] 'git reflog expire' [--expire=<time>] [--expire-unreachable=<time>] [--rewrite] [--updateref] [--stale-fix] - [--dry-run | -n] [--verbose] [--all | <refs>...] + [--dry-run | -n] [--verbose] [--all [--single-worktree] | <refs>...] 'git reflog delete' [--rewrite] [--updateref] [--dry-run | -n] [--verbose] ref@\{specifier\}... 'git reflog exists' <ref>
@@ -72,6 +72,11 @@ Options for `expire` --all:: Process the reflogs of all references. +--single-worktree:: + By default when `--all` is specified, reflogs from all working + trees are processed. This option limits the processing to reflogs + from the current working tree only. + --expire=<time>:: Prune entries older than the specified time. If this option is not specified, the expiration time is taken from the
builtin/reflog.c +19 -3
@@ -10,6 +10,7 @@#include "diff.h" #include "revision.h" #include "reachable.h" +#include "worktree.h" /* NEEDSWORK: switch to using parse_options */ static const char reflog_expire_usage[] =
@@ -52,6 +53,7 @@ struct collect_reflog_cb {struct collected_reflog **e;int alloc;int nr; +struct worktree *wt; };/* Remember to update object flag allocation in object.h */
@@ -388,8 +390,12 @@ static int collect_reflog(const char *ref, const struct object_id *oid, int unus {struct collected_reflog *e;struct collect_reflog_cb *cb = cb_data; +struct strbuf newref =STRBUF_INIT ; + +strbuf_worktree_ref (cb->wt, &newref, ref); +FLEX_ALLOC_STR (e, reflog, newref.buf); +strbuf_release (&newref); -FLEX_ALLOC_STR (e, reflog, ref);oidcpy (&e->oid, oid);ALLOC_GROW (cb->e, cb->nr +1 , cb->alloc); cb->e[cb->nr++] = e;
@@ -512,7 +518,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) {struct expire_reflog_policy_cb cb;timestamp_t now =time (NULL ); -int i, status, do_all; +int i, status, do_all, all_worktrees =1 ;int explicit_expiry =0 ;unsigned int flags =0 ;
@@ -549,6 +555,8 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) flags |=EXPIRE_REFLOGS_UPDATE_REF ; else if (!strcmp(arg, "--all")) do_all =1 ; + else if (!strcmp(arg, "--single-worktree")) + all_worktrees =0 ; else if (!strcmp(arg, "--verbose")) flags |=EXPIRE_REFLOGS_VERBOSE ; else if (!strcmp(arg, "--")) {
@@ -577,10 +585,18 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)if (do_all) {struct collect_reflog_cb collected; +struct worktree **worktrees, **p;int i;memset (&collected,0 ,sizeof (collected)); -for_each_reflog (collect_reflog, &collected); + worktrees =get_worktrees (0 ); +for (p = worktrees; *p; p++) { +if (!all_worktrees && !(*p)->is_current) +continue ; + collected.wt = *p; +for_each_reflog (collect_reflog, &collected); + } +free_worktrees (worktrees);for (i =0 ; i < collected.nr; i++) {struct collected_reflog *e = collected.e[i];set_reflog_expiry_param (&cb.cmd, explicit_expiry, e->reflog);
Signature · 1 line
2.19.0.647.gb9a6049235
PATCH 6/8 fsck: Move fsck_head_link() to get_default_heads() to avoid some globals
In reply to Nguyễn Thái Ngọc DuyFrom: Elijah Newren <[email protected]>
This will make it easier to check the HEAD of other worktrees from fsck.
--- builtin/fsck.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-)
builtin/fsck.c +16 -11
@@ -36,8 +36,6 @@ static int check_strict;static int keep_cache_objects;static struct fsck_options fsck_walk_options =FSCK_OPTIONS_DEFAULT ;static struct fsck_options fsck_obj_options =FSCK_OPTIONS_DEFAULT ; -static struct object_id head_oid; -static const char *head_points_at;static int errors_found;static int write_lost_and_found;static int verbose;
@@ -484,8 +482,15 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,return 0 ; } +static int fsck_head_link (const char **head_points_at, +struct object_id *head_oid); +static void get_default_heads (void ) { +const char *head_points_at; +struct object_id head_oid; + +fsck_head_link (&head_points_at, &head_oid);if (head_points_at && !is_null_oid (&head_oid))fsck_handle_ref ("HEAD" , &head_oid,0 ,NULL );for_each_rawref (fsck_handle_ref,NULL );
@@ -579,33 +584,34 @@ static void fsck_object_dir(const char *path)stop_progress (&progress); } -static int fsck_head_link (void ) +static int fsck_head_link (const char **head_points_at, +struct object_id *head_oid) {int null_is_error =0 ;if (verbose)fprintf (stderr,"Checking HEAD link\n" ); - head_points_at =resolve_ref_unsafe ("HEAD" ,0 , &head_oid,NULL ); -if (!head_points_at) { + *head_points_at =resolve_ref_unsafe ("HEAD" ,0 , head_oid,NULL ); +if (!*head_points_at) { errors_found |=ERROR_REFS ;return error ("Invalid HEAD" ); } -if (!strcmp (head_points_at,"HEAD" )) +if (!strcmp (*head_points_at,"HEAD" ))/* detached HEAD */ null_is_error =1 ; -else if (!starts_with (head_points_at,"refs/heads/" )) { +else if (!starts_with (*head_points_at,"refs/heads/" )) { errors_found |=ERROR_REFS ;return error ("HEAD points to something strange (%s)" , - head_points_at); + *head_points_at); } -if (is_null_oid (&head_oid)) { +if (is_null_oid (head_oid)) {if (null_is_error) { errors_found |=ERROR_REFS ;return error ("HEAD: detached HEAD points at nothing" ); }fprintf (stderr,"notice: HEAD points to an unborn branch (%s)\n" , - head_points_at +11 ); + *head_points_at +11 ); }return 0 ; }
@@ -720,7 +726,6 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)git_config (fsck_config,NULL ); -fsck_head_link ();if (connectivity_only) {for_each_loose_object (mark_loose_for_connectivity,NULL ,0 );for_each_packed_object (mark_packed_for_connectivity,NULL ,0 );
Signature · 1 line
2.19.0.647.gb9a6049235
Re: [PATCH 2/8] Add a place for (not) sharing stuff between worktrees
In reply to Nguyễn Thái Ngọc DuyQuoted 12 lines · Nguyễn Thái Ngọc Duy
When multiple worktrees are used, we need rules to determine if something belongs to one worktree or all of them. Instead of keeping adding rules when new stuff comes, have a generic rule:
[...]Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
t/t1415-worktree-refs.sh +4
-0@@ -0,0 +1,36 @@ +test_expect_success 'setup' ' + test_commit initial && + test_commit wt1 && + test_commit wt2 &&
t/t1415-worktree-refs.sh line 14
Quoted 10 lines
t/t1415-worktree-refs.sh +10
-0+ git worktree add wt1 wt1 && + git worktree add wt2 wt2 && + git checkout initial +' + +test_expect_success 'add refs/local' ' + git update-ref refs/local/foo HEAD && + git -C wt1 update-ref refs/local/foo HEAD && + git -C wt2 update-ref refs/local/foo HEAD +'
Not at all worth a re-roll, but the "add refs/local" test seems like just more setup, thus could be rolled into the "setup" test (unless it will be growing in some non-setup way in later patches).
Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Nguyễn Thái Ngọc DuyQuoted 6 lines · Nguyễn Thái Ngọc Duy
[...]
The main worktree has to be treated specially because well.. it's special from the beginning. So HEAD from the main worktree is acccessible via the name "main/HEAD" (we can't use "worktrees/main/HEAD" because "main" under "worktrees" is not reserved).
Bikeshedding: I wonder if this would be more intuitive if called simply "/HEAD" rather than "main/HEAD".
Quoted 9 lines
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
refs.c +5
-0@@ -641,12 +641,32 @@ static int is_pseudoref_syntax(const char *refname) +static int is_main_pseudoref_syntax (const char *refname) +{ +return skip_prefix (refname,"main/" , &refname) && +is_pseudoref_syntax (refname); +}
refs.c static int is_pseudoref_syntax(const char *refname) · line 655
Quoted 10 lines
refs.c +10
-0+ +static int is_other_pseudoref_syntax (const char *refname) +{ +if (!skip_prefix (refname,"worktrees/" , &refname)) +return 0 ; + refname =strchr (refname,'/' ); +if (!refname) +return 0 ; +return is_pseudoref_syntax (refname +1 ); +}
If the input is "worktrees/refs/" (nothing following the trailing '/'), then an empty string will be passed to is_pseudoref_syntax(), which will return true. Does that result in correct behavior? (Same question about "main/" being passed to is_main_pseudoref_syntax().)
Re: [PATCH 5/8] revision.c: better error reporting on ref from different worktrees
In reply to Nguyễn Thái Ngọc DuyQuoted 4 lines · Nguyễn Thái Ngọc Duy
Make use of the new ref aliases to pass refs from another worktree around and access them from the current ref store instead. This does not change any functionality, but when a problem shows up, we would report something like
From this description, I had a hard time grasping that the first example output is the desired one. Not necessarily worth a re-roll, but had it said instead something like this:
...but when a problem arises, we would like the reported messages to mention full ref aliases, like this:
it would have been more obvious.
More below...
Quoted 16 lines
fatal: bad object worktrees/ztemp/HEAD
warning: reflog of 'main/HEAD' references pruned commitsinstead of
fatal: bad object HEAD
warning: reflog of 'HEAD' references pruned commitswhich does not really tell where the refs are from.
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
worktree.c +2
-0@@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path) +void strbuf_worktree_ref(const struct worktree *wt, + struct strbuf *sb,
worktree.c int submodule_uses_worktrees(const char *path) · line 498
Quoted 10 lines
worktree.c +10
-0+ const char *refname) +{ +if (wt && !wt->is_current) { +if (is_main_worktree (wt)) +strbuf_addstr (sb,"main/" ); +else +strbuf_addf (sb,"worktrees/%s/" , wt->id); + } +strbuf_addstr (sb, refname); +}
Seeing this use worktree->id to construct "worktrees/<id>/<refname>" makes me wonder how the user is going to know the ID of a worktree in order to specify such refs in the first place. For example, how is the user going to know the <id> in "git rev-parse worktrees/<id>/HEAD"? I don't think we print the worktree ID anywhere, do we? Certainly, "git worktree list" doesn't show it, and "git worktree add" stopped showing it as of 2c27002a0a (worktree: improve message when creating a new worktree, 2018-04-24).
worktree.h extern const char *worktree_git_path(const struct worktree *wt, · line 111
Quoted 6 lines
worktree.h +4
-0@@ -108,4 +108,18 @@ extern const char *worktree_git_path(const struct worktree *wt, +/* +* Return a refname suitable for access from the current ref +* store. The result may be destroyed at the next call. +*/
If you re-roll, perhaps: s/may/will/
Re: [PATCH 7/8] fsck: check HEAD and reflog from other worktrees
In reply to Nguyễn Thái Ngọc DuyQuoted 9 lines · Nguyễn Thái Ngọc Duy
fsck is a repo-wide operation and should check all references no matter which worktree they are associated to.
Reported-by Jeff King <[email protected]>Helped-by Elijah Newren <[email protected]>Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
t/t1450-fsck.sh test_expect_success 'HEAD link pointing at a funny place' ' · line 106
Quoted 6 lines
t/t1450-fsck.sh +6
-0@@ -101,6 +101,45 @@ test_expect_success 'HEAD link pointing at a funny place' ' +test_expect_success 'HEAD link pointing at a funny object (from different wt)' ' + test_when_finished "mv .git/SAVED_HEAD .git/HEAD" && + test_when_finished "rm -rf .git/worktrees wt" && + git worktree add wt && + mv .git/HEAD .git/SAVED_HEAD && + echo 0000000000000000000000000000000000000000 >.git/HEAD &&
Perhaps use $ZERO_OID here instead of hardcoded "0..." string to play friendly with brian's bc/hash-independent-tests (in 'next'). Same for following test.
t/t1450-fsck.sh test_expect_success 'HEAD link pointing at a funny place' ' · line 112
Quoted 3 lines
t/t1450-fsck.sh +3
-0+ # avoid corrupt/broken HEAD from interfering with repo discovery + test_must_fail git -C wt fsck 2>out && + cat out &&
Unneeded 'cat'. Debugging cruft? Same for other tests.
t/t1450-fsck.sh test_expect_success 'HEAD link pointing at a funny place' ' · line 113
Quoted 1 line
t/t1450-fsck.sh +1
-0+ grep "main/HEAD: detached HEAD points" out
This message doesn't get localized, so no need for test_i18ngrep(). Okay.
Quoted 1 line
t/t1450-fsck.sh +1
-0+'
Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Eric SunshineQuoted 11 lines · Eric Sunshine
Quoted 6 lines · Nguyễn Thái Ngọc Duy
[...]
The main worktree has to be treated specially because well.. it's special from the beginning. So HEAD from the main worktree is acccessible via the name "main/HEAD" (we can't use "worktrees/main/HEAD" because "main" under "worktrees" is not reserved).Bikeshedding: I wonder if this would be more intuitive if called simply "/HEAD" rather than "main/HEAD".
A ref name cannot start with '/'. I'm open to a different name than "main" though, it just felt a better name than "main-worktree".
Signature · 1 line
Duy
Re: [PATCH 5/8] revision.c: better error reporting on ref from different worktrees
In reply to Eric SunshineQuoted 14 lines · Eric Sunshine
worktree.c int submodule_uses_worktrees(const char *path) · line 498
Quoted 13 lines
worktree.c +12
-0@@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path) +void strbuf_worktree_ref (const struct worktree *wt, +struct strbuf *sb, +const char *refname) +{ +if (wt && !wt->is_current) { +if (is_main_worktree (wt)) +strbuf_addstr (sb,"main/" ); +else +strbuf_addf (sb,"worktrees/%s/" , wt->id); + } +strbuf_addstr (sb, refname); +}
Quoted 8 lines
Seeing this use
worktree->idto construct "worktrees/<id>/<refname>" makes me wonder how the user is going to know the ID of a worktree in order to specify such refs in the first place. For example, how is the user going to know the <id> in "git rev-parse worktrees/<id>/HEAD"? I don't think we print the worktree ID anywhere, do we? Certainly, "git worktree list" doesn't show it, and "git worktree add" stopped showing it as of2c27002a0a(worktree: improve message when creating a new worktree, 2018-04-24).
Oh yes I forgot to point this out. With this approach we have to have an id, and the directory name inside $GIT_DIR/worktrees seems a natural choice. I was hoping to go with extended ref syntax instead
[1] which gives us more flexibility in identifying a worktree. But I don't think that's going to happen. "git worktree" would need some improvements to show this id, specify it at "git worktree add" and potentially rename it even.
[1] https://public-inbox.org/git/CACsJy8BOvU_z-uLrFmzFyryMXHNDfc_FUN_4i4NnVXWoShaBLQ@mail.gmail.com/
Signature · 1 line
Duy
Re: [PATCH 2/8] Add a place for (not) sharing stuff between worktrees
In reply to Nguyễn Thái Ngọc DuyQuoted 8 lines · Nguyễn Thái Ngọc Duy
When multiple worktrees are used, we need rules to determine if something belongs to one worktree or all of them. Instead of keeping adding rules when new stuff comes, have a generic rule:
- Inside $GIT_DIR, which is per-worktree by default, add $GIT_DIR/common which is always shared. New features that want to share stuff should put stuff under this directory.
So that /common is a directory and you have to use it specifically in new code? That would be easy to overlook when coming up with $GIT_DIR/foo for implementing the git-foo.
Quoted 5 lines
- Inside refs/, which is shared by default except refs/bisect, add refs/local/ which is per-worktree. We may eventually move refs/bisect to this new location and remove the exception in refs code.
That sounds dangerous to me. There is already a concept of local and remote-tracking branches. So I would think that local may soon become an overused word, (just like "index" today or "recursive" to a lesser extend).
Could this special area be more explicit? (refs/worktree-local/ ? or after peeking at the docs below refs/un-common/ ?)
Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Nguyễn Thái Ngọc DuyQuoted 29 lines · Nguyễn Thái Ngọc Duy
One of the problems with multiple worktree is accessing per-worktree refs of one worktree from another worktree. This was sort of solved by multiple ref store, where the code can open the ref store of another worktree and has access to the ref space of that worktree.
The problem with this is reporting. "HEAD" in another ref space is also called "HEAD" like in the current ref space. In order to differentiate them, all the code must somehow carry the ref store around and print something like "HEAD from this ref store".
But that is not feasible (or possible with a _lot_ of work). With the current design, we pass a reference around as a string (so called "refname"). Extending this design to pass a string _and_ a ref store is a nightmare, especially when handling extended SHA-1 syntax.
So we do it another way. Instead of entering a separate ref space, we make refs from other worktrees available in the current ref space. So "HEAD" is always HEAD of the current worktree, but then we can have "worktrees/blah/HEAD" to denote HEAD from a worktree named "blah". This syntax coincidentally matches the underlying directory structure which makes implementation a bit easier.
The main worktree has to be treated specially because well.. it's special from the beginning. So HEAD from the main worktree is acccessible via the name "main/HEAD" (we can't use "worktrees/main/HEAD" because "main" under "worktrees" is not reserved).
Quoted 4 lines
This patch also makes it possible to specify refs from one worktree in another one, e.g.
git log worktrees/foo/HEAD
This has strong similarities to remote refs:
Locally I may have a branch master, whose (stale local copy of its distributed) counterpart is named origin/master.
It is also possible to have a working tree named origin (just I like to name my worktree "git", when working on git.git), how do we differentiate between the neighbor-worktree "origin/master" and the remote-tracking branch "origin/master" ?
As the remote tracking branches are shared between all worktree there is no need to differentiate between a local-worktree remote tracking branch and a neighbor-worktree remote tracking branch.
Now that you introduce the magic main working tree, we also need to disallow working trees to be named "main", i.e.
$ git worktree add main HEAD
produces
$ ls .git/worktrees/ main
How do we deal with that?
Re: [PATCH 2/8] Add a place for (not) sharing stuff between worktrees
In reply to Stefan BellerQuoted 7 lines · Stefan Beller
Quoted 5 lines · Nguyễn Thái Ngọc Duy
When multiple worktrees are used, we need rules to determine if something belongs to one worktree or all of them. Instead of keeping adding rules when new stuff comes, have a generic rule:
Quoted 7 lines
Quoted 3 lines
- Inside $GIT_DIR, which is per-worktree by default, add $GIT_DIR/common which is always shared. New features that want to share stuff should put stuff under this directory.
So that /common is a directory and you have to use it specifically in new code? That would be easy to overlook when coming up with $GIT_DIR/foo for implementing the git-foo.
There's no easy way out. I have to do _something_ if you want to share $GIT_DIR/foo to all worktrees. Either we have to update path.c and add "foo" which is not even an option for external commands, or we put "foo" in a common place, e.g. $GIT_DIR/common/foo.
Quoted 5 lines
Quoted 4 lines
- Inside refs/, which is shared by default except refs/bisect, add refs/local/ which is per-worktree. We may eventually move refs/bisect to this new location and remove the exception in refs code.
Quoted 8 lines
That sounds dangerous to me. There is already a concept of local and remote-tracking branches. So I would think that local may soon become an overused word, (just like "index" today or "recursive" to a lesser extend).
Could this special area be more explicit? (refs/worktree-local/ ? or after peeking at the docs below refs/un-common/ ?)
refs/un-common sounds really "uncommon" :D. If refs/local is bad, I guess we could go with either refs/worktree-local, refs/worktree, refs/private, refs/per-worktree... My vote is on refs/worktree. I think as long as the word "worktree" is in there, people would notice the difference.
Signature · 1 line
Duy
Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Stefan BellerQuoted 8 lines · Stefan Beller
Quoted 4 lines
This patch also makes it possible to specify refs from one worktree in another one, e.g.
git log worktrees/foo/HEAD
This has strong similarities to remote refs:
Locally I may have a branch master, whose (stale local copy of its distributed) counterpart is named origin/master.
If you think of each worktree as independent clones (which is more or less true, the fact that they share ODB is more like an implementation detail) then yes it's almost like remotes.
Quoted 4 lines
It is also possible to have a working tree named origin (just I like to name my worktree "git", when working on git.git), how do we differentiate between the neighbor-worktree "origin/master" and the remote-tracking branch "origin/master" ?
Hmm.. I think you're thinking that origin/master could either mean refs/worktrees/origin/master or refs/remotes/origin/master. I do not think we're going to support expanding origin/master to refs/worktrees/origin/master. This part about ref resolution did cross my mind but I didn't see a good reason to support it.
Even if we do support it, this is not even a new problem. If you have refs/heads/origin/master and refs/remotes/origin/master now, we have ref ambiguity anyway and a solution for this should handle refs/worktrees/origin/master well if it comes into the picture.
Quoted 8 lines
As the remote tracking branches are shared between all worktree there is no need to differentiate between a local-worktree remote tracking branch and a neighbor-worktree remote tracking branch.
Now that you introduce the magic main working tree, we also need to disallow working trees to be named "main", i.e.
Quoted 8 lines
$ git worktree add main HEADproduces
$ ls .git/worktrees/ mainHow do we deal with that?
main is accessed via worktrees/main/HEAD while the main worktree's HEAD is accessed via main/HEAD (which is _not_ automatically expanded to refs/worktrees/main/HEAD). But if it is, yes we need to detect ambiguity and tell the user to specify full ref name, either refs/main/HEAD or refs/worktrees/main/HEAD.
Signature · 1 line
Duy
Re: [PATCH 2/8] Add a place for (not) sharing stuff between worktrees
In reply to Duy NguyenQuoted 32 lines · Duy Nguyen
Quoted 7 lines · Stefan Beller
Quoted 5 lines · Nguyễn Thái Ngọc Duy
When multiple worktrees are used, we need rules to determine if something belongs to one worktree or all of them. Instead of keeping adding rules when new stuff comes, have a generic rule:
Quoted 7 lines
Quoted 3 lines
- Inside $GIT_DIR, which is per-worktree by default, add $GIT_DIR/common which is always shared. New features that want to share stuff should put stuff under this directory.
So that /common is a directory and you have to use it specifically in new code? That would be easy to overlook when coming up with $GIT_DIR/foo for implementing the git-foo.
There's no easy way out. I have to do _something_ if you want to share $GIT_DIR/foo to all worktrees. Either we have to update
path.cand add "foo" which is not even an option for external commands, or we put "foo" in a common place, e.g. $GIT_DIR/common/foo.Quoted 10 lines
Quoted 4 lines
- Inside refs/, which is shared by default except refs/bisect, add refs/local/ which is per-worktree. We may eventually move refs/bisect to this new location and remove the exception in refs code.
That sounds dangerous to me. There is already a concept of local and remote-tracking branches. So I would think that local may soon become an overused word, (just like "index" today or "recursive" to a lesser extend).
Quoted 7 lines
Quoted 3 lines
Could this special area be more explicit? (refs/worktree-local/ ? or after peeking at the docs below refs/un-common/ ?)
refs/un-common sounds really "uncommon" :D. If refs/local is bad, I guess we could go with either refs/worktree-local, refs/worktree, refs/private, refs/per-worktree... My vote is on refs/worktree. I
refs/worktree sounds good to me (I do not object), but I am not overly enthused either, as when I think further worktrees and submodules are both features with a very similar nature in that they touch a lot of core concepts in Git, but seem to be a niche feature for the masses for now.
For example I could think of submodules following this addressing mode as well: submodule/<path>/master sounds similar to the originally proposed worktree/<name>/<branch> convention. For now it is not quite clear to me why you would want to have access to the submodule refs in the superproject, but maybe the use case will come later.
And with that said, I wonder if the "local" part should be feature agnostic, or if we want to be "local" for worktrees, "local" for remotes, "local" for submodules (i.e. our own refs vs submodule refs).
Quoted 2 lines
think as long as the word "worktree" is in there, people would notice the difference.
That makes sense. But is refs/worktree shared or local? It's not quite obvious to me, as I could have refs/worktree/<worktree-name>/master instead when it is shared, so I tend to favor refs/local-worktree/ a bit more, but that is more typing. :/
==
As we grow the worktree feature, do we ever expect the need to reference the current worktree?
For example when there is a ref "test" that could be unique per repo and in the common area, so refs/heads/test would describe it and "test" would get there in DWIM mode.
But then I could also delete the common ref and recreate a "test" ref in worktree A, in worktree B however DWIMming "test" could still refer to A's "test" as it is unique (so far) in the repository. And maybe I would want to check if test exists locally, so I'd want to ask for "self/test" (with "self" == "B" as that is my cwd).
Stefan
Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Duy NguyenQuoted 7 lines · Duy Nguyen
Quoted 5 lines · Stefan Beller
Quoted 4 lines
This patch also makes it possible to specify refs from one worktree in another one, e.g.
git log worktrees/foo/HEAD
Quoted 7 lines
Quoted 3 lines
This has strong similarities to remote refs:
Locally I may have a branch master, whose (stale local copy of its distributed) counterpart is named origin/master.If you think of each worktree as independent clones (which is more or less true, the fact that they share ODB is more like an implementation detail) then yes it's almost like remotes.
Apart from the ODB and the refs subsystem, there is also the config space, which is shared (but you have sent out patches to have local config as well).
So I would think worktrees are better than having two clones not just due to the shared ODB, but also due to the common config as then I have to setup my repo only once and can add/remove worktrees cheaply (in terms of "how much time do I need to spend to configure it as I need").
Quoted 11 lines
Quoted 4 lines
It is also possible to have a working tree named origin (just I like to name my worktree "git", when working on git.git), how do we differentiate between the neighbor-worktree "origin/master" and the remote-tracking branch "origin/master" ?
Hmm.. I think you're thinking that origin/master could either mean refs/worktrees/origin/master or refs/remotes/origin/master. I do not think we're going to support expanding origin/master to refs/worktrees/origin/master. This part about ref resolution did cross my mind but I didn't see a good reason to support it.
Quoted 4 lines
Even if we do support it, this is not even a new problem. If you have refs/heads/origin/master and refs/remotes/origin/master now, we have ref ambiguity anyway and a solution for this should handle refs/worktrees/origin/master well if it comes into the picture.
So once origin/master is overloaded, I would have to spell out refs/worktrees/origin/master and refs/remotes/origin/master to avoid confusing the DWIM machinery. Makes sense.
Quoted 7 lines
Quoted 1 line
How do we deal with that?
main is accessed via worktrees/main/HEAD while the main worktree's HEAD is accessed via main/HEAD (which is _not_ automatically expanded to refs/worktrees/main/HEAD). But if it is, yes we need to detect ambiguity and tell the user to specify full ref name, either refs/main/HEAD or refs/worktrees/main/HEAD.
Ah, I see. Now I actually understand the last paragraph of the commit message. Thanks for explaining!
Stefan
Re: [PATCH 2/8] Add a place for (not) sharing stuff between worktrees
In reply to Stefan BellerQuoted 13 lines · Stefan Beller
Quoted 5 lines
Quoted 5 lines
That sounds dangerous to me. There is already a concept of local and remote-tracking branches. So I would think that local may soon become an overused word, (just like "index" today or "recursive" to a lesser extend).
Quoted 7 lines
Quoted 3 lines
Could this special area be more explicit? (refs/worktree-local/ ? or after peeking at the docs below refs/un-common/ ?)
refs/un-common sounds really "uncommon" :D. If refs/local is bad, I guess we could go with either refs/worktree-local, refs/worktree, refs/private, refs/per-worktree... My vote is on refs/worktree. I
Quoted 5 lines
refs/worktree sounds good to me (I do not object), but I am not overly enthused either, as when I think further worktrees and submodules are both features with a very similar nature in that they touch a lot of core concepts in Git, but seem to be a niche feature for the masses for now.
I think the similarity is partly because submodule feature also has to manage worktrees. My view is at some point, this "git worktree" would be good enough that it can handle submodules as well (for the worktree part only of course)
Quoted 6 lines
For example I could think of submodules following this addressing mode as well: submodule/<path>/master sounds similar to the originally proposed worktree/<name>/<branch> convention. For now it is not quite clear to me why you would want to have access to the submodule refs in the superproject, but maybe the use case will come later.
Yeah. In theory we could "mount" the submodule ref store to a superproject's ref store. I think it may be needed just for the same reason it's needed for worktree: error reporting. If you peek into a submodule and say "HEAD has an error", the user will get confused whether it's superproject's HEAD or a submodule's HEAD.
Quoted 3 lines
And with that said, I wonder if the "local" part should be feature agnostic, or if we want to be "local" for worktrees, "local" for remotes, "local" for submodules (i.e. our own refs vs submodule refs).
You lost me here.
Quoted 8 lines
Quoted 3 lines
think as long as the word "worktree" is in there, people would notice the difference.
That makes sense. But is refs/worktree shared or local? It's not quite obvious to me, as I could have refs/worktree/<worktree-name>/master instead when it is shared, so I tend to favor refs/local-worktree/ a bit more, but that is more typing. :/
OK I think mixing the two patches will different purposes messes you (or me) up ;-)
refs/worktrees/xxx (and refs/main/xxx) are about visibility from other worktrees. Or like Eric put it, they are simply aliases. These refs are not shared because if they are, you can already see them without new "ref mount points" like this.
refs/worktree (previously refs/local) is also per-worktree but it's specifically because you can't have per-worktree inside "refs/" (the only exception so far is refs/bisect which is hard coded). You can have refs outside "refs/" (like HEAD or FETCH_HEAD) and they will not be shared, but they cannot be iterated while those inside refs/ can be. This is more about deciding what to share and I believe is really worktree-specific and only matters to _current_ worktree.
Since refs/worktree is per-worktree, you can also view them from a different worktree via refs/worktrees/. E.g. if you have refs/worktree/foo then another worktree can see it via refs/worktrees/xxx/refs/worktree/foo (besides pseudo refs like refs/worktrees/xxx/HEAD)
Quoted 7 lines
As we grow the worktree feature, do we ever expect the need to reference the current worktree?
For example when there is a ref "test" that could be unique per repo and in the common area, so refs/heads/test would describe it and "test" would get there in DWIM mode.
Quoted 5 lines
But then I could also delete the common ref and recreate a "test" ref in worktree A, in worktree B however DWIMming "test" could still refer to A's "test" as it is unique (so far) in the repository. And maybe I would want to check if test exists locally, so I'd want to ask for "self/test" (with "self" == "B" as that is my cwd).
You probably lost me again. In theory we must be able to detect ambiguity and stop DWIMing. If you want to be ambiguity-free, you specify full ref name, starting with "refs/" which should function like "self/" because worktree design so far is always about the current worktree's view.
Signature · 1 line
Duy
Re: [PATCH 2/8] Add a place for (not) sharing stuff between worktrees
Quoted 5 lines · Duy Nguyen
Quoted 3 lines
And with that said, I wonder if the "local" part should be feature agnostic, or if we want to be "local" for worktrees, "local" for remotes, "local" for submodules (i.e. our own refs vs submodule refs).
You lost me here.
Yeah, me too after rereading. :P
I think the "local" part always implies that there is a part that is not local and depending on the feature you call it remote or other worktree.
When writing this comment I briefly wondered if we want to combine the local aspects of the various features.
However the "local" part really depends on the feature
(e.g. a ref on a different worktree is still local from the here/remote perspective or from the superproject/submodule perspective), so I think I was misguided.
Quoted 10 lines
Quoted 7 lines
Quoted 2 lines
think as long as the word "worktree" is in there, people would notice the difference.
That makes sense. But is refs/worktree shared or local? It's not quite obvious to me, as I could have refs/worktree/<worktree-name>/master instead when it is shared, so I tend to favor refs/local-worktree/ a bit more, but that is more typing. :/
OK I think mixing the two patches will different purposes messes you (or me) up ;-)
possible.
Quoted 14 lines
refs/worktrees/xxx (and refs/main/xxx) are about visibility from other worktrees. Or like Eric put it, they are simply aliases. These refs are not shared because if they are, you can already see them without new "ref mount points" like this.
refs/worktree (previously refs/local) is also per-worktree but it's specifically because you can't have per-worktree inside "refs/" (the only exception so far is refs/bisect which is hard coded). You can have refs outside "refs/" (like HEAD or
FETCH_HEAD) and they will not be shared, but they cannot be iterated while those inside refs/ can be. This is more about deciding what to share and I believe is really worktree-specific and only matters to _current_ worktree.
Quoted 5 lines
Since refs/worktree is per-worktree, you can also view them from a different worktree via refs/worktrees/. E.g. if you have refs/worktree/foo then another worktree can see it via refs/worktrees/xxx/refs/worktree/foo (besides pseudo refs like refs/worktrees/xxx/HEAD)
Ah. now I seem to understand, thanks for explaining.
Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Nguyễn Thái Ngọc DuyQuoted 5 lines · Nguyễn Thái Ngọc Duy
The main worktree has to be treated specially because well.. it's special from the beginning. So HEAD from the main worktree is acccessible via the name "main/HEAD" (we can't use "worktrees/main/HEAD" because "main" under "worktrees" is not reserved).
I do not quite follow. So with this, both refs/heads/master and main/refs/heads/master are good names for the master branch (even though the local branch names are not per worktree), because in the main worktree, refs/bisect/bad and main/refs/bisect/bad ought to mean the same thing.
side note: Or is this only for pseudo-refs
(i.e. $GIT_DIR/$name where $name consists of all caps or underscore and typically ends with HEAD)? Even if that were the case, I do not think it essentially changes the issue around disambiguation that much.
The disambiguation rule has always been: if you have a confusingly named ref, you can spell it out fully to avoid any ambiguity, e.g. refs/heads/refs/heads/foo can be given to "git rev-parse" and will mean the tip of the branch whose name is "refs/heads/foo", even when another branch whose name is "foo" exists.
Would we have a reasonable disambiguation rules that work well with the main/ and worktrees/* prefixes? When somebody has main/HEAD branch and writes "git rev-parse main/HEAD", does it find refs/heads/main/HEAD or $GIT_DIR/HEAD, if the user is in the main worktree?
It could be simply that the design is underdocumented in this patch set (in which case I would have appreciated 'RFC' near 'PATCH'), but I have a feeling that the code came way too early before such design issues are fleshed out.
Quoted 111 lines
refs.h +5
-3@@ -704,9 +704,11 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);int ref_is_hidden (const char *,const char *);enum ref_type { -REF_TYPE_PER_WORKTREE , -REF_TYPE_PSEUDOREF , -REF_TYPE_NORMAL , +REF_TYPE_PER_WORKTREE ,/* refs inside refs/ but not shared */ +REF_TYPE_PSEUDOREF ,/* refs outside refs/ in current worktree */ +REF_TYPE_MAIN_PSEUDOREF ,/* pseudo refs from the main worktree */ +REF_TYPE_OTHER_PSEUDOREF ,/* pseudo refs from other worktrees */ +REF_TYPE_NORMAL ,/* normal/shared refs inside refs/ */ };enum ref_type ref_type (const char *refname);refs/files-backend.c +28
-0@@ -149,6 +149,23 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,return refs; } +static void files_reflog_path_other_worktrees (struct files_ref_store *refs, +struct strbuf *sb, +const char *refname) +{ +const char *real_ref; + +if (!skip_prefix (refname,"worktrees/" , &real_ref)) +BUG ("refname %s is not a other-worktree ref" , refname); + real_ref =strchr (real_ref,'/' ); +if (!real_ref) +BUG ("refname %s is not a other-worktree ref" , refname); + real_ref++; + +strbuf_addf (sb,"%s/%.*slogs/%s" , refs->gitcommondir, + (int )(real_ref - refname), refname, real_ref); +} +static void files_reflog_path (struct files_ref_store *refs,struct strbuf *sb,const char *refname)@@ -158,6 +175,12 @@ static void files_reflog_path(struct files_ref_store *refs,case REF_TYPE_PSEUDOREF :strbuf_addf (sb,"%s/logs/%s" , refs->gitdir, refname);break ; +case REF_TYPE_OTHER_PSEUDOREF : +return files_reflog_path_other_worktrees (refs, sb, refname); +case REF_TYPE_MAIN_PSEUDOREF : +if (!skip_prefix (refname,"main/" , &refname)) +BUG ("ref %s is not a main pseudoref" , refname); +/* passthru */ case REF_TYPE_NORMAL :strbuf_addf (sb,"%s/logs/%s" , refs->gitcommondir, refname);break ;@@ -176,6 +199,11 @@ static void files_ref_path(struct files_ref_store *refs,case REF_TYPE_PSEUDOREF :strbuf_addf (sb,"%s/%s" , refs->gitdir, refname);break ; +case REF_TYPE_MAIN_PSEUDOREF : +if (!skip_prefix (refname,"main/" , &refname)) +BUG ("ref %s is not a main pseudoref" , refname); +/* passthru */ +case REF_TYPE_OTHER_PSEUDOREF :case REF_TYPE_NORMAL :strbuf_addf (sb,"%s/%s" , refs->gitcommondir, refname);break ;t/t1415-worktree-refs.sh +30
-0@@ -33,4 +33,34 @@ test_expect_success 'refs/local are per-worktree' ' ( cd wt2 && test_cmp_rev local/foo wt2 ) ' +test_expect_success 'resolve main/HEAD' ' + test_cmp_rev main/HEAD initial && + ( cd wt1 && test_cmp_rev main/HEAD initial ) && + ( cd wt2 && test_cmp_rev main/HEAD initial ) +' + +test_expect_success 'resolve worktrees/xx/HEAD' ' + test_cmp_rev worktrees/wt1/HEAD wt1 && + ( cd wt1 && test_cmp_rev worktrees/wt1/HEAD wt1 ) && + ( cd wt2 && test_cmp_rev worktrees/wt1/HEAD wt1 ) +' + +test_expect_success 'reflog of main/HEAD' ' + git reflog HEAD | sed "s/HEAD/main\/HEAD/" >expected && + git reflog main/HEAD >actual && + test_cmp expected actual && + git -C wt1 reflog main/HEAD >actual.wt1 && + test_cmp expected actual.wt1 +' + +test_expect_success 'reflog of worktrees/xx/HEAD' ' + git -C wt2 reflog HEAD | sed "s/HEAD/worktrees\/wt2\/HEAD/" >expected && + git reflog worktrees/wt2/HEAD >actual && + test_cmp expected actual && + git -C wt1 reflog worktrees/wt2/HEAD >actual.wt1 && + test_cmp expected actual.wt1 && + git -C wt2 reflog worktrees/wt2/HEAD >actual.wt2 && + test_cmp expected actual.wt2 +' + test_done
Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
Quoted 9 lines · Junio C Hamano
Quoted 5 lines · Nguyễn Thái Ngọc Duy
The main worktree has to be treated specially because well.. it's special from the beginning. So HEAD from the main worktree is acccessible via the name "main/HEAD" (we can't use "worktrees/main/HEAD" because "main" under "worktrees" is not reserved).
Quoted 5 lines
I do not quite follow. So with this, both refs/heads/master and main/refs/heads/master are good names for the master branch (even though the local branch names are not per worktree), because in the main worktree, refs/bisect/bad and main/refs/bisect/bad ought to mean the same thing.
True. I think the ambiguation here is about the main worktree versus a secondary worktree that is accidentally named "main". Then suddenly we have to worktrees of the same name, and accessing them both via worktrees/<id>/HEAD will not work, and there is no other way to disambiguate them.
Quoted 3 lines
side note: Or is this only for pseudo-refs (i.e. $GIT_DIR/$name where $name consists of all caps or underscore and typically ends with HEAD)?
Right now, due to implementation limitations, only pseudo refs (or loose refs in the case of refs/bisect) are accessible. But I don't see why main/refs/heads/master should not work.
Quoted 10 lines
The disambiguation rule has always been: if you have a confusingly named ref, you can spell it out fully to avoid any ambiguity, e.g. refs/heads/refs/heads/foo can be given to "git rev-parse" and will mean the tip of the branch whose name is "refs/heads/foo", even when another branch whose name is "foo" exists.
Would we have a reasonable disambiguation rules that work well with the main/ and worktrees/* prefixes? When somebody has main/HEAD branch and writes "git rev-parse main/HEAD", does it find refs/heads/main/HEAD or $GIT_DIR/HEAD, if the user is in the main worktree?
The rules are not touched. But it looks like everything still works as expected (I'm adding tests to verify this)
Signature · 1 line
Duy
Re: [PATCH 7/8] fsck: check HEAD and reflog from other worktrees
In reply to Eric SunshineQuoted 4 lines · Eric Sunshine
Quoted 1 line
+ grep "main/HEAD: detached HEAD points" out
This message doesn't get localized, so no need for
test_i18ngrep(). Okay.
Don't remind me. I started to mark all strings in fsck for translation and faced the reality that I would need to change a lot to test_i18ngrep :(
--
Duy
v2 cover fix per-worktree ref iteration in fsck/reflog expire
In reply to Nguyễn Thái Ngọc Duyv2 changes
- more documentation
- main/ prefix is renamed to main-worktree/ to reduce ambiguation chances and make it clearer
- refs/local is renamed to refs/worktree
- bug fix in is_main_pseudoref_syntax() and is_other_pseudoref_syntax()
Interdiff
Documentation/git-worktree.txt +31 -1
@@ -204,6 +204,35 @@ working trees, it can be used to identify worktrees. For example if you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg", then "ghi" or "def/ghi" is enough to point to the former working tree. +REFS +---- +In multiple working trees, some refs may be shared between all working +trees, some refs are local. One example is HEAD is different for all +working trees. This section is about the sharing rules and how to access +refs of one working tree from another. + +In general, all pseudo refs are per working tree and all refs starting +with "refs/" are shared. Pseudo refs are ones like HEAD which are +directly under GIT_DIR instead of inside GIT_DIR/refs. There are one +exception to this: refs inside refs/bisect and refs/worktree is not +shared. + +Refs that are per working tree can still be accessed from another +working tree via two special paths main-worktree and worktrees. The +former gives access to per-worktree refs of the main working tree, +while the former to all linked working trees. + +For example, main-worktree/HEAD or main-worktree/refs/bisect/good +resolve to the same value as the main working tree's HEAD and +refs/bisect/good respectively. Similarly, worktrees/foo/HEAD or +worktrees/bar/refs/bisect/bad are the same as +GIT_COMMON_DIR/worktrees/foo/HEAD and +GIT_COMMON_DIR/worktrees/bar/refs/bisect/bad. + +To access refs, it's best not to look inside GIT_DIR directly. Instead +use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1] +which will handle refs correctly. + DETAILS ------- Each linked working tree has a private sub-directory in the repository's
@@ -228,7 +257,8 @@ linked working tree `git rev-parse --git-path HEAD` returns `/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git rev-parse --git-path refs/heads/master` uses $GIT_COMMON_DIR and returns `/path/main/.git/refs/heads/master`, -since refs are shared across all working trees. +since refs are shared across all working trees, except refs/bisect and +refs/worktree. See linkgit:gitrepository-layout[5] for more information. The rule of thumb is do not make any assumption about whether a path belongs to
Documentation/gitrepository-layout.txt +3 -3
@@ -96,9 +96,9 @@ refs:: directory. The 'git prune' command knows to preserve objects reachable from refs found in this directory and its subdirectories. - This directory is ignored (except refs/bisect and refs/local) - if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/refs" will be - used instead. + This directory is ignored (except refs/bisect and + refs/worktree) if $GIT_COMMON_DIR is set and + "$GIT_COMMON_DIR/refs" will be used instead. refs/heads/`name`:: records tip-of-the-tree commit objects of branch `name`
path.c +1 -0
@@ -119,6 +119,7 @@ static struct common_dir common_list[] = { { 0, 1, 0, "objects" }, { 0, 1, 0, "refs" }, { 0, 1, 1, "refs/bisect" }, + { 0, 1, 1, "refs/worktree" }, { 0, 1, 0, "remotes" }, { 0, 1, 0, "worktrees" }, { 0, 1, 0, "rr-cache" },
refs.c +4 -3
@@ -624,7 +624,7 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)static int is_per_worktree_ref (const char *refname) {return !strcmp (refname,"HEAD" ) || -starts_with (refname,"refs/local/" ) || +starts_with (refname,"refs/worktree/" ) ||starts_with (refname,"refs/bisect/" ) ||starts_with (refname,"refs/rewritten/" ); }
@@ -643,7 +643,8 @@ static int is_pseudoref_syntax(const char *refname)static int is_main_pseudoref_syntax (const char *refname) { -return skip_prefix (refname,"main/" , &refname) && +return skip_prefix (refname,"main-worktree/" , &refname) && + *refname &&is_pseudoref_syntax (refname); }
@@ -652,7 +653,7 @@ static int is_other_pseudoref_syntax(const char *refname)if (!skip_prefix (refname,"worktrees/" , &refname))return 0 ; refname =strchr (refname,'/' ); -if (!refname) +if (!refname || !refname[1 ])return 0 ;return is_pseudoref_syntax (refname +1 ); }
refs/files-backend.c +5 -5
@@ -178,7 +178,7 @@ static void files_reflog_path(struct files_ref_store *refs,case REF_TYPE_OTHER_PSEUDOREF :return files_reflog_path_other_worktrees (refs, sb, refname);case REF_TYPE_MAIN_PSEUDOREF : -if (!skip_prefix (refname,"main/" , &refname)) +if (!skip_prefix (refname,"main-worktree/" , &refname))BUG ("ref %s is not a main pseudoref" , refname);/* passthru */ case REF_TYPE_NORMAL :
@@ -200,7 +200,7 @@ static void files_ref_path(struct files_ref_store *refs,strbuf_addf (sb,"%s/%s" , refs->gitdir, refname);break ;case REF_TYPE_MAIN_PSEUDOREF : -if (!skip_prefix (refname,"main/" , &refname)) +if (!skip_prefix (refname,"main-worktree/" , &refname))BUG ("ref %s is not a main pseudoref" , refname);/* passthru */ case REF_TYPE_OTHER_PSEUDOREF :
@@ -297,7 +297,7 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,closedir (d);/* -* Manually add refs/bisect and refs/local, which, being +* Manually add refs/bisect and refs/worktree, which, being * per-worktree, might not appear in the directory listing for * refs/ in the main repo. */
@@ -310,11 +310,11 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,add_entry_to_dir (dir, child_entry); } - pos =search_ref_dir (dir,"refs/local/" ,11 ); + pos =search_ref_dir (dir,"refs/worktree/" ,11 );if (pos <0 ) {struct ref_entry *child_entry =create_dir_entry ( - dir->cache,"refs/local/" ,11 ,1 ); + dir->cache,"refs/worktree/" ,11 ,1 );add_entry_to_dir (dir, child_entry); } }
t/t1415-worktree-refs.sh +36 -23
80 lines not on this page · raw message
t/t1450-fsck.sh +2 -6
@@ -106,19 +106,17 @@ test_expect_success 'HEAD link pointing at a funny object (from different wt)' ' test_when_finished "rm -rf .git/worktrees wt" && git worktree add wt && mv .git/HEAD .git/SAVED_HEAD && - echo 0000000000000000000000000000000000000000 >.git/HEAD && + echo $ZERO_OID >.git/HEAD && # avoid corrupt/broken HEAD from interfering with repo discovery test_must_fail git -C wt fsck 2>out && - cat out && grep "main/HEAD: detached HEAD points" out ' test_expect_success 'other worktree HEAD link pointing at a funny object' ' test_when_finished "rm -rf .git/worktrees other" && git worktree add other && - echo 0000000000000000000000000000000000000000 >.git/worktrees/other/HEAD && + echo $ZERO_OID >.git/worktrees/other/HEAD && test_must_fail git fsck 2>out && - cat out && grep "worktrees/other/HEAD: detached HEAD points" out '
@@ -127,7 +125,6 @@ test_expect_success 'other worktree HEAD link pointing at missing object' ' git worktree add other && echo "Contents missing from repo" | git hash-object --stdin >.git/worktrees/other/HEAD && test_must_fail git fsck 2>out && - cat out && grep "worktrees/other/HEAD: invalid sha1 pointer" out '
@@ -136,7 +133,6 @@ test_expect_success 'other worktree HEAD link pointing at a funny place' ' git worktree add other && echo "ref: refs/funny/place" >.git/worktrees/other/HEAD && test_must_fail git fsck 2>out && - cat out && grep "worktrees/other/HEAD points to something strange" out '
Elijah Newren (1):
fsck: Move fsck_head_link() to get_default_heads() to avoid some
globals
Nguyễn Thái Ngọc Duy (7):
refs.c: indent with tabs, not spaces
Add a place for (not) sharing stuff between worktrees
refs: new ref types to make per-worktree refs visible to all worktrees revision.c: correct a parameter name
revision.c: better error reporting on ref from different worktrees
fsck: check HEAD and reflog from other worktrees
reflog expire: cover reflog from all worktrees
Documentation/git-reflog.txt | 7 ++- Documentation/git-worktree.txt | 32 ++++++++++- Documentation/gitrepository-layout.txt | 11 +++- builtin/fsck.c | 68 +++++++++++++++------- builtin/reflog.c | 22 ++++++- path.c | 2 + refs.c | 24 +++++++- refs.h | 8 ++- refs/files-backend.c | 42 +++++++++++++- revision.c | 22 ++++--- t/t0060-path-utils.sh | 2 + t/t1415-worktree-refs.sh | 79 ++++++++++++++++++++++++++ t/t1450-fsck.sh | 35 ++++++++++++ worktree.c | 32 ++++++++++- worktree.h | 14 +++++ 15 files changed, 354 insertions(+), 46 deletions(-) create mode 100755 t/t1415-worktree-refs.sh
Signature · 1 line
2.19.0.341.g3acb95d729
PATCH v2 1/8 refs.c: indent with tabs, not spaces
--- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
refs.c +1 -1
@@ -646,7 +646,7 @@ enum ref_type ref_type(const char *refname)return REF_TYPE_PER_WORKTREE ;if (is_pseudoref_syntax (refname))return REF_TYPE_PSEUDOREF ; -return REF_TYPE_NORMAL ; +return REF_TYPE_NORMAL ; }long get_files_ref_lock_timeout_ms (void )
Signature · 1 line
2.19.0.341.g3acb95d729
PATCH v2 2/8 Add a place for (not) sharing stuff between worktrees
In reply to Nguyễn Thái Ngọc DuyWhen multiple worktrees are used, we need rules to determine if something belongs to one worktree or all of them. Instead of keeping adding rules when new stuff comes (*), have a generic rule:
- Inside $GIT_DIR, which is per-worktree by default, add $GIT_DIR/common which is always shared. New features that want to share stuff should put stuff under this directory.
- Inside refs/, which is shared by default except refs/bisect, add refs/worktree/ which is per-worktree. We may eventually move refs/bisect to this new location and remove the exception in refs code.
(*) And it may also include stuff from external commands which will
have no way to modify common/per-worktree rules.
--- Documentation/git-worktree.txt | 19 ++++++++++++++- Documentation/gitrepository-layout.txt | 11 +++++++-- path.c | 2 ++ refs.c | 1 + refs/files-backend.c | 14 ++++++++--- t/t0060-path-utils.sh | 2 ++ t/t1415-worktree-refs.sh | 33 ++++++++++++++++++++++++++ 7 files changed, 76 insertions(+), 6 deletions(-) create mode 100755 t/t1415-worktree-refs.sh
Documentation/git-worktree.txt +18 -1
@@ -204,6 +204,22 @@ working trees, it can be used to identify worktrees. For example if you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg", then "ghi" or "def/ghi" is enough to point to the former working tree. +REFS +---- +In multiple working trees, some refs may be shared between all working +trees, some refs are local. One example is HEAD is different for all +working trees. This section is about the sharing rules. + +In general, all pseudo refs are per working tree and all refs starting +with "refs/" are shared. Pseudo refs are ones like HEAD which are +directly under GIT_DIR instead of inside GIT_DIR/refs. There are one +exception to this: refs inside refs/bisect and refs/worktree is not +shared. + +To access refs, it's best not to look inside GIT_DIR directly. Instead +use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1] +which will handle refs correctly. + DETAILS ------- Each linked working tree has a private sub-directory in the repository's
@@ -228,7 +244,8 @@ linked working tree `git rev-parse --git-path HEAD` returns `/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git rev-parse --git-path refs/heads/master` uses $GIT_COMMON_DIR and returns `/path/main/.git/refs/heads/master`, -since refs are shared across all working trees. +since refs are shared across all working trees, except refs/bisect and +refs/worktree. See linkgit:gitrepository-layout[5] for more information. The rule of thumb is do not make any assumption about whether a path belongs to
Documentation/gitrepository-layout.txt +9 -2
@@ -95,8 +95,10 @@ refs:: References are stored in subdirectories of this directory. The 'git prune' command knows to preserve objects reachable from refs found in this directory and - its subdirectories. This directory is ignored if $GIT_COMMON_DIR - is set and "$GIT_COMMON_DIR/refs" will be used instead. + its subdirectories. + This directory is ignored (except refs/bisect and + refs/worktree) if $GIT_COMMON_DIR is set and + "$GIT_COMMON_DIR/refs" will be used instead. refs/heads/`name`:: records tip-of-the-tree commit objects of branch `name`
@@ -165,6 +167,11 @@ hooks:: each hook. This directory is ignored if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/hooks" will be used instead. +common:: + When multiple working trees are used, most of files in + $GIT_DIR are per-worktree with a few known exceptions. All + files under 'common' however will be shared between all + working trees. index:: The current index file for the repository. It is
path.c +2 -0
@@ -108,6 +108,7 @@ struct common_dir {static struct common_dir common_list[] = { {0 ,1 ,0 ,"branches" }, + {0 ,1 ,0 ,"common" }, {0 ,1 ,0 ,"hooks" }, {0 ,1 ,0 ,"info" }, {0 ,0 ,1 ,"info/sparse-checkout" },
@@ -118,6 +119,7 @@ static struct common_dir common_list[] = { { 0, 1, 0, "objects" }, { 0, 1, 0, "refs" }, { 0, 1, 1, "refs/bisect" }, + { 0, 1, 1, "refs/worktree" }, { 0, 1, 0, "remotes" }, { 0, 1, 0, "worktrees" }, { 0, 1, 0, "rr-cache" },
refs.c +1 -0
@@ -624,6 +624,7 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)static int is_per_worktree_ref (const char *refname) {return !strcmp (refname,"HEAD" ) || +starts_with (refname,"refs/worktree/" ) ||starts_with (refname,"refs/bisect/" ) ||starts_with (refname,"refs/rewritten/" ); }
refs/files-backend.c +11 -3
@@ -269,9 +269,9 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,closedir (d);/* -* Manually add refs/bisect, which, being per-worktree, might -* not appear in the directory listing for refs/ in the main -* repo. +* Manually add refs/bisect and refs/worktree, which, being +* per-worktree, might not appear in the directory listing for +* refs/ in the main repo. */ if (!strcmp (dirname,"refs/" )) {int pos =search_ref_dir (dir,"refs/bisect/" ,12 );
@@ -281,6 +281,14 @@ static void loose_fill_ref_dir(struct ref_store *ref_store, dir->cache, "refs/bisect/", 12, 1);add_entry_to_dir (dir, child_entry); } + + pos =search_ref_dir (dir,"refs/worktree/" ,11 ); + +if (pos <0 ) { +struct ref_entry *child_entry =create_dir_entry ( + dir->cache,"refs/worktree/" ,11 ,1 ); +add_entry_to_dir (dir, child_entry); + } } }
t/t0060-path-utils.sh +2 -0
@@ -306,6 +306,8 @@ test_git_path GIT_COMMON_DIR=bar hooks/me bar/hooks/me test_git_path GIT_COMMON_DIR=bar config bar/config test_git_path GIT_COMMON_DIR=bar packed-refs bar/packed-refs test_git_path GIT_COMMON_DIR=bar shallow bar/shallow +test_git_path GIT_COMMON_DIR=bar common bar/common +test_git_path GIT_COMMON_DIR=bar common/file bar/common/file # In the tests below, $(pwd) must be used because it is a native path on # Windows and avoids MSYS's path mangling (which simplifies "foo/../bar" and
t/t1415-worktree-refs.sh new file +33 -0
@@ -0,0 +1,33 @@ +#!/bin/sh + +test_description='per-worktree refs' + +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit initial && + test_commit wt1 && + test_commit wt2 && + git worktree add wt1 wt1 && + git worktree add wt2 wt2 && + git checkout initial && + git update-ref refs/worktree/foo HEAD && + git -C wt1 update-ref refs/worktree/foo HEAD && + git -C wt2 update-ref refs/worktree/foo HEAD +' + +test_expect_success 'refs/worktree must not be packed' ' + git pack-refs --all && + test_path_is_missing .git/refs/tags/wt1 && + test_path_is_file .git/refs/worktree/foo && + test_path_is_file .git/worktrees/wt1/refs/worktree/foo && + test_path_is_file .git/worktrees/wt2/refs/worktree/foo +' + +test_expect_success 'refs/worktree are per-worktree' ' + test_cmp_rev worktree/foo initial && + ( cd wt1 && test_cmp_rev worktree/foo wt1 ) && + ( cd wt2 && test_cmp_rev worktree/foo wt2 ) +' + +test_done
Signature · 1 line
2.19.0.341.g3acb95d729
PATCH v2 3/8 refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Nguyễn Thái Ngọc DuyOne of the problems with multiple worktree is accessing per-worktree refs of one worktree from another worktree. This was sort of solved by multiple ref store, where the code can open the ref store of another worktree and has access to the ref space of that worktree.
The problem with this is reporting. "HEAD" in another ref space is also called "HEAD" like in the current ref space. In order to differentiate them, all the code must somehow carry the ref store around and print something like "HEAD from this ref store".
But that is not feasible (or possible with a _lot_ of work). With the current design, we pass a reference around as a string (so called "refname"). Extending this design to pass a string _and_ a ref store is a nightmare, especially when handling extended SHA-1 syntax.
So we do it another way. Instead of entering a separate ref space, we make refs from other worktrees available in the current ref space. So "HEAD" is always HEAD of the current worktree, but then we can have "worktrees/blah/HEAD" to denote HEAD from a worktree named "blah". This syntax coincidentally matches the underlying directory structure which makes implementation a bit easier.
The main worktree has to be treated specially because well.. it's special from the beginning. So HEAD from the main worktree is acccessible via the name "main-worktree/HEAD" instead of "worktrees/main/HEAD" because "main" could be just another secondary worktree.
This patch also makes it possible to specify refs from one worktree in another one, e.g.
git log worktrees/foo/HEAD
--- Documentation/git-worktree.txt | 15 ++++++++++- refs.c | 21 ++++++++++++++++ refs.h | 8 +++--- refs/files-backend.c | 28 +++++++++++++++++++++ t/t1415-worktree-refs.sh | 46 ++++++++++++++++++++++++++++++++++ 5 files changed, 114 insertions(+), 4 deletions(-)
Documentation/git-worktree.txt +14 -1
@@ -208,7 +208,8 @@ REFS ---- In multiple working trees, some refs may be shared between all working trees, some refs are local. One example is HEAD is different for all -working trees. This section is about the sharing rules. +working trees. This section is about the sharing rules and how to access +refs of one working tree from another. In general, all pseudo refs are per working tree and all refs starting with "refs/" are shared. Pseudo refs are ones like HEAD which are
@@ -216,6 +217,18 @@ directly under GIT_DIR instead of inside GIT_DIR/refs. There are one exception to this: refs inside refs/bisect and refs/worktree is not shared. +Refs that are per working tree can still be accessed from another +working tree via two special paths main-worktree and worktrees. The +former gives access to per-worktree refs of the main working tree, +while the former to all linked working trees. + +For example, main-worktree/HEAD or main-worktree/refs/bisect/good +resolve to the same value as the main working tree's HEAD and +refs/bisect/good respectively. Similarly, worktrees/foo/HEAD or +worktrees/bar/refs/bisect/bad are the same as +GIT_COMMON_DIR/worktrees/foo/HEAD and +GIT_COMMON_DIR/worktrees/bar/refs/bisect/bad. + To access refs, it's best not to look inside GIT_DIR directly. Instead use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1] which will handle refs correctly.
refs.c +21 -0
@@ -641,12 +641,33 @@ static int is_pseudoref_syntax(const char *refname)return 1 ; } +static int is_main_pseudoref_syntax (const char *refname) +{ +return skip_prefix (refname,"main-worktree/" , &refname) && + *refname && +is_pseudoref_syntax (refname); +} + +static int is_other_pseudoref_syntax (const char *refname) +{ +if (!skip_prefix (refname,"worktrees/" , &refname)) +return 0 ; + refname =strchr (refname,'/' ); +if (!refname || !refname[1 ]) +return 0 ; +return is_pseudoref_syntax (refname +1 ); +} +enum ref_type ref_type (const char *refname) {if (is_per_worktree_ref (refname))return REF_TYPE_PER_WORKTREE ;if (is_pseudoref_syntax (refname))return REF_TYPE_PSEUDOREF ; +if (is_main_pseudoref_syntax (refname)) +return REF_TYPE_MAIN_PSEUDOREF ; +if (is_other_pseudoref_syntax (refname)) +return REF_TYPE_OTHER_PSEUDOREF ;return REF_TYPE_NORMAL ; }
refs.h +5 -3
@@ -704,9 +704,11 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);int ref_is_hidden (const char *,const char *);enum ref_type { -REF_TYPE_PER_WORKTREE , -REF_TYPE_PSEUDOREF , -REF_TYPE_NORMAL , +REF_TYPE_PER_WORKTREE ,/* refs inside refs/ but not shared */ +REF_TYPE_PSEUDOREF ,/* refs outside refs/ in current worktree */ +REF_TYPE_MAIN_PSEUDOREF ,/* pseudo refs from the main worktree */ +REF_TYPE_OTHER_PSEUDOREF ,/* pseudo refs from other worktrees */ +REF_TYPE_NORMAL ,/* normal/shared refs inside refs/ */ };enum ref_type ref_type (const char *refname);
refs/files-backend.c +28 -0
@@ -149,6 +149,23 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,return refs; } +static void files_reflog_path_other_worktrees (struct files_ref_store *refs, +struct strbuf *sb, +const char *refname) +{ +const char *real_ref; + +if (!skip_prefix (refname,"worktrees/" , &real_ref)) +BUG ("refname %s is not a other-worktree ref" , refname); + real_ref =strchr (real_ref,'/' ); +if (!real_ref) +BUG ("refname %s is not a other-worktree ref" , refname); + real_ref++; + +strbuf_addf (sb,"%s/%.*slogs/%s" , refs->gitcommondir, + (int )(real_ref - refname), refname, real_ref); +} +static void files_reflog_path (struct files_ref_store *refs,struct strbuf *sb,const char *refname)
@@ -158,6 +175,12 @@ static void files_reflog_path(struct files_ref_store *refs,case REF_TYPE_PSEUDOREF :strbuf_addf (sb,"%s/logs/%s" , refs->gitdir, refname);break ; +case REF_TYPE_OTHER_PSEUDOREF : +return files_reflog_path_other_worktrees (refs, sb, refname); +case REF_TYPE_MAIN_PSEUDOREF : +if (!skip_prefix (refname,"main-worktree/" , &refname)) +BUG ("ref %s is not a main pseudoref" , refname); +/* passthru */ case REF_TYPE_NORMAL :strbuf_addf (sb,"%s/logs/%s" , refs->gitcommondir, refname);break ;
@@ -176,6 +199,11 @@ static void files_ref_path(struct files_ref_store *refs,case REF_TYPE_PSEUDOREF :strbuf_addf (sb,"%s/%s" , refs->gitdir, refname);break ; +case REF_TYPE_MAIN_PSEUDOREF : +if (!skip_prefix (refname,"main-worktree/" , &refname)) +BUG ("ref %s is not a main pseudoref" , refname); +/* passthru */ +case REF_TYPE_OTHER_PSEUDOREF :case REF_TYPE_NORMAL :strbuf_addf (sb,"%s/%s" , refs->gitcommondir, refname);break ;
t/t1415-worktree-refs.sh +46 -0
50 lines not on this page · raw message
Signature · 1 line
2.19.0.341.g3acb95d729
PATCH v2 4/8 revision.c: correct a parameter name
In reply to Nguyễn Thái Ngọc DuyThis function is a callback of for_each_reflog() which will pass a ref name as the first argument, not a path (to a reflog file).
--- revision.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
revision.c +4 -3
@@ -1277,13 +1277,14 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,return 0 ; } -static int handle_one_reflog (const char *path,const struct object_id *oid, +static int handle_one_reflog (const char *refname, +const struct object_id *oid,int flag,void *cb_data) {struct all_refs_cb *cb = cb_data; cb->warned_bad_reflog =0 ; - cb->name_for_errormsg = path; -refs_for_each_reflog_ent (cb->refs, path, + cb->name_for_errormsg = refname; +refs_for_each_reflog_ent (cb->refs, refname, handle_one_reflog_ent, cb_data);return 0 ; }
Signature · 1 line
2.19.0.341.g3acb95d729
PATCH v2 5/8 revision.c: better error reporting on ref from different worktrees
In reply to Nguyễn Thái Ngọc DuyMake use of the new ref aliases to pass refs from another worktree around and access them from the current ref store instead. This does not change any functionality, but when a problem arises, we would like the reported messages to mention full ref aliases, like this:
fatal: bad object worktrees/ztemp/HEAD
warning: reflog of 'main-worktree/HEAD' references pruned commits
instead of
fatal: bad object HEAD
warning: reflog of 'HEAD' references pruned commits
which does not really tell where the refs are from.
--- revision.c | 21 +++++++++++++-------- worktree.c | 32 +++++++++++++++++++++++++++++--- worktree.h | 14 ++++++++++++++ 3 files changed, 56 insertions(+), 11 deletions(-)
revision.c +13 -8
@@ -1177,7 +1177,7 @@ struct all_refs_cb {int warned_bad_reflog;struct rev_info *all_revs;const char *name_for_errormsg; -struct ref_store *refs; +struct worktree *wt; };int ref_excluded (struct string_list *ref_excludes,const char *path)
@@ -1214,7 +1214,7 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs, cb->all_revs = revs; cb->all_flags = flags; revs->rev_input_given =1 ; - cb->refs =NULL ; + cb->wt =NULL ; }void clear_ref_exclusion (struct string_list **ref_excludes_p)
@@ -1277,15 +1277,20 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,return 0 ; } -static int handle_one_reflog (const char *refname, +static int handle_one_reflog (const char *refname_in_wt,const struct object_id *oid,int flag,void *cb_data) {struct all_refs_cb *cb = cb_data; +struct strbuf refname =STRBUF_INIT ; + cb->warned_bad_reflog =0 ; - cb->name_for_errormsg = refname; -refs_for_each_reflog_ent (cb->refs, refname, +strbuf_worktree_ref (cb->wt, &refname, refname_in_wt); + cb->name_for_errormsg = refname.buf; +refs_for_each_reflog_ent (get_main_ref_store (the_repository), + refname.buf, handle_one_reflog_ent, cb_data); +strbuf_release (&refname);return 0 ; }
@@ -1300,8 +1305,8 @@ static void add_other_reflogs_to_pending(struct all_refs_cb *cb)if (wt->is_current)continue ; - cb->refs =get_worktree_ref_store (wt); -refs_for_each_reflog (cb->refs, + cb->wt = wt; +refs_for_each_reflog (get_worktree_ref_store (wt), handle_one_reflog, cb); }
@@ -1314,7 +1319,7 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags) cb.all_revs = revs; cb.all_flags = flags; - cb.refs =get_main_ref_store (the_repository); + cb.wt =NULL ;for_each_reflog (handle_one_reflog, &cb);if (!revs->single_worktree)
worktree.c +29 -3
@@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path)return ret; } +void strbuf_worktree_ref (const struct worktree *wt, +struct strbuf *sb, +const char *refname) +{ +if (wt && !wt->is_current) { +if (is_main_worktree (wt)) +strbuf_addstr (sb,"main/" ); +else +strbuf_addf (sb,"worktrees/%s/" , wt->id); + } +strbuf_addstr (sb, refname); +} + +const char *worktree_ref (const struct worktree *wt,const char *refname) +{ +static struct strbuf sb =STRBUF_INIT ; + +strbuf_reset (&sb); +strbuf_worktree_ref (wt, &sb, refname); +return sb.buf; +} +int other_head_refs (each_ref_fn fn,void *cb_data) {struct worktree **worktrees, **p;
@@ -495,13 +517,17 @@ int other_head_refs(each_ref_fn fn, void *cb_data) worktrees =get_worktrees (0 );for (p = worktrees; *p; p++) {struct worktree *wt = *p; -struct ref_store *refs; +struct object_id oid; +int flag;if (wt->is_current)continue ; - refs =get_worktree_ref_store (wt); - ret =refs_head_ref (refs, fn, cb_data); +if (!refs_read_ref_full (get_main_ref_store (the_repository), +worktree_ref (wt,"HEAD" ), +RESOLVE_REF_READING , + &oid, &flag)) + ret =fn (worktree_ref (wt,"HEAD" ), &oid, flag, cb_data);if (ret)break ; }
worktree.h +14 -0
@@ -108,4 +108,18 @@ extern const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...) __attribute__((format (printf, 2, 3))); +/* +* Return a refname suitable for access from the current ref store. +*/ +void strbuf_worktree_ref (const struct worktree *wt, +struct strbuf *sb, +const char *refname); + +/* +* Return a refname suitable for access from the current ref +* store. The result may be destroyed at the next call. +*/ +const char *worktree_ref (const struct worktree *wt, +const char *refname); +#endif
Signature · 1 line
2.19.0.341.g3acb95d729
PATCH v2 6/8 fsck: Move fsck_head_link() to get_default_heads() to avoid some globals
In reply to Nguyễn Thái Ngọc DuyFrom: Elijah Newren <[email protected]>
This will make it easier to check the HEAD of other worktrees from fsck.
--- builtin/fsck.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-)
builtin/fsck.c +16 -11
@@ -36,8 +36,6 @@ static int check_strict;static int keep_cache_objects;static struct fsck_options fsck_walk_options =FSCK_OPTIONS_DEFAULT ;static struct fsck_options fsck_obj_options =FSCK_OPTIONS_DEFAULT ; -static struct object_id head_oid; -static const char *head_points_at;static int errors_found;static int write_lost_and_found;static int verbose;
@@ -484,8 +482,15 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,return 0 ; } +static int fsck_head_link (const char **head_points_at, +struct object_id *head_oid); +static void get_default_heads (void ) { +const char *head_points_at; +struct object_id head_oid; + +fsck_head_link (&head_points_at, &head_oid);if (head_points_at && !is_null_oid (&head_oid))fsck_handle_ref ("HEAD" , &head_oid,0 ,NULL );for_each_rawref (fsck_handle_ref,NULL );
@@ -579,33 +584,34 @@ static void fsck_object_dir(const char *path)stop_progress (&progress); } -static int fsck_head_link (void ) +static int fsck_head_link (const char **head_points_at, +struct object_id *head_oid) {int null_is_error =0 ;if (verbose)fprintf (stderr,"Checking HEAD link\n" ); - head_points_at =resolve_ref_unsafe ("HEAD" ,0 , &head_oid,NULL ); -if (!head_points_at) { + *head_points_at =resolve_ref_unsafe ("HEAD" ,0 , head_oid,NULL ); +if (!*head_points_at) { errors_found |=ERROR_REFS ;return error ("Invalid HEAD" ); } -if (!strcmp (head_points_at,"HEAD" )) +if (!strcmp (*head_points_at,"HEAD" ))/* detached HEAD */ null_is_error =1 ; -else if (!starts_with (head_points_at,"refs/heads/" )) { +else if (!starts_with (*head_points_at,"refs/heads/" )) { errors_found |=ERROR_REFS ;return error ("HEAD points to something strange (%s)" , - head_points_at); + *head_points_at); } -if (is_null_oid (&head_oid)) { +if (is_null_oid (head_oid)) {if (null_is_error) { errors_found |=ERROR_REFS ;return error ("HEAD: detached HEAD points at nothing" ); }fprintf (stderr,"notice: HEAD points to an unborn branch (%s)\n" , - head_points_at +11 ); + *head_points_at +11 ); }return 0 ; }
@@ -720,7 +726,6 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)git_config (fsck_config,NULL ); -fsck_head_link ();if (connectivity_only) {for_each_loose_object (mark_loose_for_connectivity,NULL ,0 );for_each_packed_object (mark_packed_for_connectivity,NULL ,0 );
Signature · 1 line
2.19.0.341.g3acb95d729
PATCH v2 7/8 fsck: check HEAD and reflog from other worktrees
In reply to Nguyễn Thái Ngọc Duyfsck is a repo-wide operation and should check all references no matter which worktree they are associated to.
--- builtin/fsck.c | 55 ++++++++++++++++++++++++++++++++++--------------- t/t1450-fsck.sh | 35 +++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 17 deletions(-)
builtin/fsck.c +38 -17
@@ -19,6 +19,7 @@#include "packfile.h" #include "object-store.h" #include "run-command.h" +#include "worktree.h" #define REACHABLE 0x0001#define SEEN 0x0002
@@ -444,7 +445,11 @@ static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noidstatic int fsck_handle_reflog (const char *logname,const struct object_id *oid,int flag,void *cb_data) { -for_each_reflog_ent (logname, fsck_handle_reflog_ent, (void *)logname); +struct strbuf refname =STRBUF_INIT ; + +strbuf_worktree_ref (cb_data, &refname, logname); +for_each_reflog_ent (refname.buf, fsck_handle_reflog_ent, refname.buf); +strbuf_release (&refname);return 0 ; }
@@ -482,20 +487,34 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,return 0 ; } -static int fsck_head_link (const char **head_points_at, +static int fsck_head_link (const char *head_ref_name, +const char **head_points_at,struct object_id *head_oid);static void get_default_heads (void ) { +struct worktree **worktrees, **p;const char *head_points_at;struct object_id head_oid; -fsck_head_link (&head_points_at, &head_oid); -if (head_points_at && !is_null_oid (&head_oid)) -fsck_handle_ref ("HEAD" , &head_oid,0 ,NULL );for_each_rawref (fsck_handle_ref,NULL ); -if (include_reflogs) -for_each_reflog (fsck_handle_reflog,NULL ); + + worktrees =get_worktrees (0 ); +for (p = worktrees; *p; p++) { +struct worktree *wt = *p; +struct strbuf ref =STRBUF_INIT ; + +strbuf_worktree_ref (wt, &ref,"HEAD" ); +fsck_head_link (ref.buf, &head_points_at, &head_oid); +if (head_points_at && !is_null_oid (&head_oid)) +fsck_handle_ref (ref.buf, &head_oid,0 ,NULL ); +strbuf_release (&ref); + +if (include_reflogs) +refs_for_each_reflog (get_worktree_ref_store (wt), + fsck_handle_reflog, wt); + } +free_worktrees (worktrees);/* * Not having any default heads isn't really fatal, but
@@ -584,34 +603,36 @@ static void fsck_object_dir(const char *path)stop_progress (&progress); } -static int fsck_head_link (const char **head_points_at, +static int fsck_head_link (const char *head_ref_name, +const char **head_points_at,struct object_id *head_oid) {int null_is_error =0 ;if (verbose) -fprintf (stderr,"Checking HEAD link\n" ); +fprintf (stderr,"Checking %s link\n" , head_ref_name); - *head_points_at =resolve_ref_unsafe ("HEAD" ,0 , head_oid,NULL ); + *head_points_at =resolve_ref_unsafe (head_ref_name,0 , head_oid,NULL );if (!*head_points_at) { errors_found |=ERROR_REFS ; -return error ("Invalid HEAD" ); +return error ("Invalid %s" , head_ref_name); } -if (!strcmp (*head_points_at,"HEAD" )) +if (!strcmp (*head_points_at, head_ref_name))/* detached HEAD */ null_is_error =1 ;else if (!starts_with (*head_points_at,"refs/heads/" )) { errors_found |=ERROR_REFS ; -return error ("HEAD points to something strange (%s)" , - *head_points_at); +return error ("%s points to something strange (%s)" , + head_ref_name, *head_points_at); }if (is_null_oid (head_oid)) {if (null_is_error) { errors_found |=ERROR_REFS ; -return error ("HEAD: detached HEAD points at nothing" ); +return error ("%s: detached HEAD points at nothing" , + head_ref_name); } -fprintf (stderr,"notice: HEAD points to an unborn branch (%s)\n" , - *head_points_at +11 ); +fprintf (stderr,"notice: %s points to an unborn branch (%s)\n" , + head_ref_name, *head_points_at +11 ); }return 0 ; }
t/t1450-fsck.sh +35 -0
@@ -101,6 +101,41 @@ test_expect_success 'HEAD link pointing at a funny place' ' grep "HEAD points to something strange" out ' +test_expect_success 'HEAD link pointing at a funny object (from different wt)' ' + test_when_finished "mv .git/SAVED_HEAD .git/HEAD" && + test_when_finished "rm -rf .git/worktrees wt" && + git worktree add wt && + mv .git/HEAD .git/SAVED_HEAD && + echo $ZERO_OID >.git/HEAD && + # avoid corrupt/broken HEAD from interfering with repo discovery + test_must_fail git -C wt fsck 2>out && + grep "main/HEAD: detached HEAD points" out +' + +test_expect_success 'other worktree HEAD link pointing at a funny object' ' + test_when_finished "rm -rf .git/worktrees other" && + git worktree add other && + echo $ZERO_OID >.git/worktrees/other/HEAD && + test_must_fail git fsck 2>out && + grep "worktrees/other/HEAD: detached HEAD points" out +' + +test_expect_success 'other worktree HEAD link pointing at missing object' ' + test_when_finished "rm -rf .git/worktrees other" && + git worktree add other && + echo "Contents missing from repo" | git hash-object --stdin >.git/worktrees/other/HEAD && + test_must_fail git fsck 2>out && + grep "worktrees/other/HEAD: invalid sha1 pointer" out +' + +test_expect_success 'other worktree HEAD link pointing at a funny place' ' + test_when_finished "rm -rf .git/worktrees other" && + git worktree add other && + echo "ref: refs/funny/place" >.git/worktrees/other/HEAD && + test_must_fail git fsck 2>out && + grep "worktrees/other/HEAD points to something strange" out +' + test_expect_success 'email without @ is okay' ' git cat-file commit HEAD >basis && sed "s/@/AT/" basis >okay &&
Signature · 1 line
2.19.0.341.g3acb95d729
PATCH v2 8/8 reflog expire: cover reflog from all worktrees
In reply to Nguyễn Thái Ngọc Duy--- Documentation/git-reflog.txt | 7 ++++++- builtin/reflog.c | 22 +++++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-)
Documentation/git-reflog.txt +6 -1
@@ -20,7 +20,7 @@ depending on the subcommand: 'git reflog' ['show'] [log-options] [<ref>] 'git reflog expire' [--expire=<time>] [--expire-unreachable=<time>] [--rewrite] [--updateref] [--stale-fix] - [--dry-run | -n] [--verbose] [--all | <refs>...] + [--dry-run | -n] [--verbose] [--all [--single-worktree] | <refs>...] 'git reflog delete' [--rewrite] [--updateref] [--dry-run | -n] [--verbose] ref@\{specifier\}... 'git reflog exists' <ref>
@@ -72,6 +72,11 @@ Options for `expire` --all:: Process the reflogs of all references. +--single-worktree:: + By default when `--all` is specified, reflogs from all working + trees are processed. This option limits the processing to reflogs + from the current working tree only. + --expire=<time>:: Prune entries older than the specified time. If this option is not specified, the expiration time is taken from the
builtin/reflog.c +19 -3
@@ -10,6 +10,7 @@#include "diff.h" #include "revision.h" #include "reachable.h" +#include "worktree.h" /* NEEDSWORK: switch to using parse_options */ static const char reflog_expire_usage[] =
@@ -52,6 +53,7 @@ struct collect_reflog_cb {struct collected_reflog **e;int alloc;int nr; +struct worktree *wt; };/* Remember to update object flag allocation in object.h */
@@ -388,8 +390,12 @@ static int collect_reflog(const char *ref, const struct object_id *oid, int unus {struct collected_reflog *e;struct collect_reflog_cb *cb = cb_data; +struct strbuf newref =STRBUF_INIT ; + +strbuf_worktree_ref (cb->wt, &newref, ref); +FLEX_ALLOC_STR (e, reflog, newref.buf); +strbuf_release (&newref); -FLEX_ALLOC_STR (e, reflog, ref);oidcpy (&e->oid, oid);ALLOC_GROW (cb->e, cb->nr +1 , cb->alloc); cb->e[cb->nr++] = e;
@@ -512,7 +518,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) {struct expire_reflog_policy_cb cb;timestamp_t now =time (NULL ); -int i, status, do_all; +int i, status, do_all, all_worktrees =1 ;int explicit_expiry =0 ;unsigned int flags =0 ;
@@ -549,6 +555,8 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) flags |=EXPIRE_REFLOGS_UPDATE_REF ; else if (!strcmp(arg, "--all")) do_all =1 ; + else if (!strcmp(arg, "--single-worktree")) + all_worktrees =0 ; else if (!strcmp(arg, "--verbose")) flags |=EXPIRE_REFLOGS_VERBOSE ; else if (!strcmp(arg, "--")) {
@@ -577,10 +585,18 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)if (do_all) {struct collect_reflog_cb collected; +struct worktree **worktrees, **p;int i;memset (&collected,0 ,sizeof (collected)); -for_each_reflog (collect_reflog, &collected); + worktrees =get_worktrees (0 ); +for (p = worktrees; *p; p++) { +if (!all_worktrees && !(*p)->is_current) +continue ; + collected.wt = *p; +for_each_reflog (collect_reflog, &collected); + } +free_worktrees (worktrees);for (i =0 ; i < collected.nr; i++) {struct collected_reflog *e = collected.e[i];set_reflog_expiry_param (&cb.cmd, explicit_expiry, e->reflog);
Signature · 1 line
2.19.0.341.g3acb95d729
Re: [PATCH v2 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Nguyễn Thái Ngọc DuyQuoted 1 line · Nguyễn Thái Ngọc Duy
The main worktree has to be treated specially because well.. it's
Nit: s/well../well.../
Documentation/git-worktree.txt directly under GIT_DIR instead of inside GIT_DIR/refs. There are one · line 218
Quoted 11 lines
special from the beginning. So HEAD from the main worktree is acccessible via the name "main-worktree/HEAD" instead of "worktrees/main/HEAD" because "main" could be just another secondary worktree.
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
Documentation/git-worktree.txt +2
-0@@ -216,6 +217,18 @@ directly under GIT_DIR instead of inside GIT_DIR/refs. There are one +Refs that are per working tree can still be accessed from another +working tree via two special paths main-worktree and worktrees. The
s/paths/paths,/
Documentation/git-worktree.txt directly under GIT_DIR instead of inside GIT_DIR/refs. There are one · line 223
Quoted 2 lines
Documentation/git-worktree.txt +2
-0+former gives access to per-worktree refs of the main working tree, +while the former to all linked working trees.
s/former/latter/
t/t1415-worktree-refs.sh test_expect_success 'refs/worktree are per-worktree' ' · line 33
Quoted 6 lines
t/t1415-worktree-refs.sh +4
-0@@ -30,4 +30,50 @@ test_expect_success 'refs/worktree are per-worktree' ' +test_expect_success 'ambiguous main-worktree/HEAD' ' + mkdir -p .git/refs/heads/main-worktree && + test_when_finished rm .git/refs/heads/main-worktree/HEAD && + cp .git/HEAD .git/refs/heads/main-worktree/HEAD &&
Better to use "rm -f" for cleanup in case this 'cp' fails for some reason.
t/t1415-worktree-refs.sh test_expect_success 'refs/worktree are per-worktree' ' · line 43
Quoted 1 line
t/t1415-worktree-refs.sh +1
-0+ git rev-parse main-worktree/HEAD 2>warn >/dev/null &&
You could probably omit the /dev/null redirect.
t/t1415-worktree-refs.sh test_expect_success 'refs/worktree are per-worktree' ' · line 55
Quoted 6 lines
t/t1415-worktree-refs.sh +6
-0+ grep "main-worktree/HEAD.*ambiguous" warn +' + +test_expect_success 'ambiguous worktrees/xx/HEAD' ' + mkdir -p .git/refs/heads/worktrees/wt1 && + test_when_finished rm .git/refs/heads/worktrees/wt1/HEAD &&
Ditto "rm -f".
t/t1415-worktree-refs.sh test_expect_success 'refs/worktree are per-worktree' ' · line 57
Quoted 2 lines
t/t1415-worktree-refs.sh +2
-0+ cp .git/HEAD .git/refs/heads/worktrees/wt1/HEAD && + git rev-parse worktrees/wt1/HEAD 2>warn >/dev/null &&
Ditto /dev/null.
Quoted 2 lines
t/t1415-worktree-refs.sh +2
-0+ grep "worktrees/wt1/HEAD.*ambiguous" warn +'
Re: [PATCH v2 5/8] revision.c: better error reporting on ref from different worktrees
In reply to Nguyễn Thái Ngọc DuyQuoted 9 lines · Nguyễn Thái Ngọc Duy
Make use of the new ref aliases to pass refs from another worktree around and access them from the current ref store instead. This does not change any functionality, but when a problem arises, we would like the reported messages to mention full ref aliases, like this:
[...]Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
worktree.c int submodule_uses_worktrees(const char *path) · line 493
Quoted 7 lines
worktree.c +7
-0@@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path) +void strbuf_worktree_ref (const struct worktree *wt, +struct strbuf *sb, +const char *refname) +{ +if (wt && !wt->is_current) { +if (is_main_worktree (wt)) +strbuf_addstr (sb,"main/" );
I think this needs to be "main-worktree/", doesn't it?
worktree.h extern const char *worktree_git_path(const struct worktree *wt, · line 110
Quoted 10 lines
worktree.c +5
-0+else +strbuf_addf (sb,"worktrees/%s/" , wt->id); + } +strbuf_addstr (sb, refname); +}worktree.h +3
-0@@ -108,4 +108,18 @@ extern const char *worktree_git_path(const struct worktree *wt, +/* +* Return a refname suitable for access from the current ref +* store. The result may be destroyed at the next call.
s/may/will/
Re: [PATCH v2 8/8] reflog expire: cover reflog from all worktrees
In reply to Nguyễn Thái Ngọc DuyDocumentation/git-reflog.txt Options for `expire` · line 75
Quoted 8 lines · Nguyễn Thái Ngọc Duy
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
Documentation/git-reflog.txt +4
-0@@ -72,6 +72,11 @@ Options for `expire` +--single-worktree:: + By default when `--all` is specified, reflogs from all working + trees are processed. This option limits the processing to reflogs + from the current working tree only.
Bikeshedding: I wonder if this should be named "--this-worktree" or "--this-worktree-only" or if it should somehow be orthogonal to --all rather than modifying it. (Genuine questions. I don't have the answers.)
Quoted 7 lines
builtin/reflog.c +1
-0@@ -577,10 +585,18 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)if (do_all) {struct collect_reflog_cb collected; +struct worktree **worktrees, **p;int i;
builtin/reflog.c static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) · line 598
Quoted 10 lines
builtin/reflog.c +8
-1memset (&collected,0 ,sizeof (collected)); -for_each_reflog (collect_reflog, &collected); + worktrees =get_worktrees (0 ); +for (p = worktrees; *p; p++) { +if (!all_worktrees && !(*p)->is_current) +continue ; + collected.wt = *p; +for_each_reflog (collect_reflog, &collected); + } +free_worktrees (worktrees);
Should this have a test in the test suite?
Re: [PATCH v2 8/8] reflog expire: cover reflog from all worktrees
Quoted 15 lines · Eric Sunshine
Documentation/git-reflog.txt Options for `expire` · line 75
Quoted 8 lines · Nguyễn Thái Ngọc Duy
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
Documentation/git-reflog.txt +4
-0@@ -72,6 +72,11 @@ Options for `expire` +--single-worktree:: + By default when `--all` is specified, reflogs from all working + trees are processed. This option limits the processing to reflogs + from the current working tree only.Bikeshedding: I wonder if this should be named "--this-worktree" or "--this-worktree-only" or if it should somehow be orthogonal to --all rather than modifying it. (Genuine questions. I don't have the answers.)
It follows a precedent (made by me :p) which is rev-list --single-worktree. I doubt that option is widely used though so we could still rename it if there's a better name. I made --single-worktree to contrast "all worktrees" by default. Even if it's "this/current worktree" it still has to somehow say "everything in this worktree" so I felt modifying --all was a good idea.
Quoted 7 lines
Quoted 7 lines
builtin/reflog.c +1
-0@@ -577,10 +585,18 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)if (do_all) {struct collect_reflog_cb collected; +struct worktree **worktrees, **p;int i;
Quoted 12 lines
builtin/reflog.c static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) · line 599
Quoted 10 lines
builtin/reflog.c +8
-1memset (&collected,0 ,sizeof (collected)); -for_each_reflog (collect_reflog, &collected); + worktrees =get_worktrees (0 ); +for (p = worktrees; *p; p++) { +if (!all_worktrees && !(*p)->is_current) +continue ; + collected.wt = *p; +for_each_reflog (collect_reflog, &collected); + } +free_worktrees (worktrees);Should this have a test in the test suite?
Of course. I was partly lazy/tired near the end, and anticipated more comments anyway so I did not do it :D
Signature · 1 line
Duy
Re: [PATCH v2 8/8] reflog expire: cover reflog from all worktrees
Quoted 12 lines · Duy Nguyen
Quoted 10 lines · Eric Sunshine
Documentation/git-reflog.txt Options for `expire` · line 78
Quoted 4 lines · Nguyễn Thái Ngọc Duy
+--single-worktree::
Documentation/git-reflog.txt +3
-0+ By default when `--all` is specified, reflogs from all working + trees are processed. This option limits the processing to reflogs + from the current working tree only.Bikeshedding: I wonder if this should be named "--this-worktree" or "--this-worktree-only" or if it should somehow be orthogonal to --all rather than modifying it. (Genuine questions. I don't have the answers.)
Quoted 6 lines
It follows a precedent (made by me :p) which is rev-list --single-worktree. I doubt that option is widely used though so we could still rename it if there's a better name. I made --single-worktree to contrast "all worktrees" by default. Even if it's "this/current worktree" it still has to somehow say "everything in this worktree" so I felt modifying --all was a good idea.
Precedent overrides bikeshedding, so leaving it as-is is fine.
Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Duy NguyenQuoted 12 lines · Duy Nguyen
Quoted 11 lines
Quoted 5 lines
The main worktree has to be treated specially because well.. it's special from the beginning. So HEAD from the main worktree is acccessible via the name "main/HEAD" (we can't use "worktrees/main/HEAD" because "main" under "worktrees" is not reserved).
I do not quite follow. So with this, both refs/heads/master and main/refs/heads/master are good names for the master branch (even though the local branch names are not per worktree), because in the main worktree, refs/bisect/bad and main/refs/bisect/bad ought to mean the same thing.
Quoted 5 lines
True. I think the ambiguation here is about the main worktree versus a secondary worktree that is accidentally named "main". Then suddenly we have to worktrees of the same name, and accessing them both via worktrees/<id>/HEAD will not work, and there is no other way to disambiguate them.
So those who have happily been referring 'refs/heads/main/foo' as 'main/foo' now suddenly have to say 'refs/heads/main/foo' instead?
Quoted 2 lines
The rules are not touched. But it looks like everything still works as expected (I'm adding tests to verify this)
What I am worried about is that _your_ expectation may not coincide with the expectations of users, especially with ones with existing refs that overlap with the namespaces this series suddenly starts carving out and squatting on. As long as that won't be a problem, I think it is OK, even with 'main' not renamed to 'worktree-main' or somesuch.
Re: [PATCH v2 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Nguyễn Thái Ngọc DuyQuoted 4 lines · Nguyễn Thái Ngọc Duy
refs/files-backend.c +0
-0
refs/files-backend.c static void files_reflog_path(struct files_ref_store *refs, · line 183
Quoted 10 lines
refs/files-backend.c +6
-0case REF_TYPE_PSEUDOREF :strbuf_addf (sb,"%s/logs/%s" , refs->gitdir, refname);break ; +case REF_TYPE_OTHER_PSEUDOREF : +return files_reflog_path_other_worktrees (refs, sb, refname); +case REF_TYPE_MAIN_PSEUDOREF : +if (!skip_prefix (refname,"main-worktree/" , &refname)) +BUG ("ref %s is not a main pseudoref" , refname); +/* passthru */
Correct spelling of the colloquial phrase is fallthru, but see 1cf01a34 ("consistently use "fallthrough" comments in switches", 2017-09-21) that encourages use of "fallthrough" fully spelled out.
Otherwise you'd see something like this.
CC refs/files-backend.orefs/files-backend.c: In function 'files_ref_path':refs/files-backend.c:203:6: error: this statement may fall through [-Werror=implicit-fallthrough=]
if (!skip_prefix(refname, "main-worktree/", &refname))
^
refs/files-backend.c:206:2: note: here
case REF_TYPE_OTHER_PSEUDOREF:
^~~~refs/files-backend.c: In function 'files_reflog_path':refs/files-backend.c:181:6: error: this statement may fall through [-Werror=implicit-fallthrough=]
if (!skip_prefix(refname, "main-worktree/", &refname))
^
refs/files-backend.c:184:2: note: here
case REF_TYPE_NORMAL:
^~~~
cc1: all warnings being treated as errors Makefile:2289: recipe for target 'refs/files-backend.o' failed
make: *** [refs/files-backend.o] Error 1
v3 cover fix per-worktree ref iteration in fsck/reflog expire
In reply to Nguyễn Thái Ngọc Duyv3 changes
- fix incorrect ref reporting (it reported main/HEAD instead of main-worktree/HEAD)
- other document typos
- fix strbuf_worktree_ref() producing refs that cannot be handled by files-backend.c, e.g. worktrees/foo/refs/heads/master (one day the ref store will, but not now)
- make sure the "HEAD" special case in reflog expire still works with main-worktree/HEAD and worktrees/xxx/HEAD
- tests added for reflog
Elijah Newren (1):
fsck: Move fsck_head_link() to get_default_heads() to avoid some
globals
Nguyễn Thái Ngọc Duy (7):
refs.c: indent with tabs, not spaces
Add a place for (not) sharing stuff between worktrees
refs: new ref types to make per-worktree refs visible to all worktrees revision.c: correct a parameter name
revision.c: better error reporting on ref from different worktrees
fsck: check HEAD and reflog from other worktrees
reflog expire: cover reflog from all worktrees
Documentation/git-reflog.txt | 7 ++- Documentation/git-worktree.txt | 32 ++++++++++- Documentation/gitrepository-layout.txt | 11 +++- builtin/fsck.c | 68 +++++++++++++++------- builtin/reflog.c | 46 +++++++++++++-- path.c | 2 + refs.c | 24 +++++++- refs.h | 8 ++- refs/files-backend.c | 42 +++++++++++++- revision.c | 22 ++++--- t/t0060-path-utils.sh | 2 + t/t1410-reflog.sh | 15 +++++ t/t1415-worktree-refs.sh | 79 ++++++++++++++++++++++++++ t/t1450-fsck.sh | 35 ++++++++++++ worktree.c | 79 +++++++++++++++++++++++++- worktree.h | 24 ++++++++ 16 files changed, 449 insertions(+), 47 deletions(-) create mode 100755 t/t1415-worktree-refs.sh
Range-diff against v2:
1: 328a4d1263 ! 1: fff4cfcc93 refs: new ref types to make per-worktree refs visible to all worktrees
@@ -24,7 +24,7 @@
"blah". This syntax coincidentally matches the underlying directory
structure which makes implementation a bit easier.
- The main worktree has to be treated specially because well.. it's
+ The main worktree has to be treated specially because well... it's
special from the beginning. So HEAD from the main worktree is
acccessible via the name "main-worktree/HEAD" instead of
"worktrees/main/HEAD" because "main" could be just another secondary
@@ -53,9 +53,9 @@
shared.
+Refs that are per working tree can still be accessed from another
-+working tree via two special paths main-worktree and worktrees. The
++working tree via two special paths, main-worktree and worktrees. The
+former gives access to per-worktree refs of the main working tree,
-+while the former to all linked working trees.
++while the latter to all linked working trees.
+
+For example, main-worktree/HEAD or main-worktree/refs/bisect/good
+resolve to the same value as the main working tree's HEAD and
@@ -128,6 +128,14 @@
diff --git a/refs/files-backend.c b/refs/files-backend.c
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
+@@
+ #include "../object.h"
+ #include "../dir.h"
+ #include "../chdir-notify.h"
++#include "worktree.h"
+
+ /*
+ * This backend uses the following flags in `ref_update::flags` for
@@
return refs;
}
@@ -137,16 +145,18 @@
+ const char *refname)
+{
+ const char *real_ref;
++ const char *worktree_name;
++ int length;
+
-+ if (!skip_prefix(refname, "worktrees/", &real_ref))
-+ BUG("refname %s is not a other-worktree ref", refname);
-+ real_ref = strchr(real_ref, '/');
-+ if (!real_ref)
++ if (parse_worktree_ref(refname, &worktree_name, &length, &real_ref))
+ BUG("refname %s is not a other-worktree ref", refname);
-+ real_ref++;
+
-+ strbuf_addf(sb, "%s/%.*slogs/%s", refs->gitcommondir,
-+ (int)(real_ref - refname), refname, real_ref);
++ if (worktree_name)
++ strbuf_addf(sb, "%s/worktrees/%.*s/logs/%s", refs->gitcommondir,
++ length, worktree_name, real_ref);
++ else
++ strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir,
++ real_ref);
+}
+
static void files_reflog_path(struct files_ref_store *refs,
@@ -157,11 +167,8 @@
strbuf_addf(sb, "%s/logs/%s", refs->gitdir, refname);
break;
+ case REF_TYPE_OTHER_PSEUDOREF:
-+ return files_reflog_path_other_worktrees(refs, sb, refname);
+ case REF_TYPE_MAIN_PSEUDOREF:
-+ if (!skip_prefix(refname, "main-worktree/", &refname))
-+ BUG("ref %s is not a main pseudoref", refname);
-+ /* passthru */
++ return files_reflog_path_other_worktrees(refs, sb, refname);
case REF_TYPE_NORMAL:
strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname);
break;
@@ -172,7 +179,7 @@
+ case REF_TYPE_MAIN_PSEUDOREF:
+ if (!skip_prefix(refname, "main-worktree/", &refname))
+ BUG("ref %s is not a main pseudoref", refname);
-+ /* passthru */
++ /* fallthrough */
+ case REF_TYPE_OTHER_PSEUDOREF:
case REF_TYPE_NORMAL:
strbuf_addf(sb, "%s/%s", refs->gitcommondir, refname);
@@ -193,9 +200,9 @@
+
+test_expect_success 'ambiguous main-worktree/HEAD' '
+ mkdir -p .git/refs/heads/main-worktree &&
-+ test_when_finished rm .git/refs/heads/main-worktree/HEAD &&
++ test_when_finished rm -f .git/refs/heads/main-worktree/HEAD &&
+ cp .git/HEAD .git/refs/heads/main-worktree/HEAD &&
-+ git rev-parse main-worktree/HEAD 2>warn >/dev/null &&
++ git rev-parse main-worktree/HEAD 2>warn &&
+ grep "main-worktree/HEAD.*ambiguous" warn
+'
+
@@ -207,9 +214,9 @@
+
+test_expect_success 'ambiguous worktrees/xx/HEAD' '
+ mkdir -p .git/refs/heads/worktrees/wt1 &&
-+ test_when_finished rm .git/refs/heads/worktrees/wt1/HEAD &&
++ test_when_finished rm -f .git/refs/heads/worktrees/wt1/HEAD &&
+ cp .git/HEAD .git/refs/heads/worktrees/wt1/HEAD &&
-+ git rev-parse worktrees/wt1/HEAD 2>warn >/dev/null &&
++ git rev-parse worktrees/wt1/HEAD 2>warn &&
+ grep "worktrees/wt1/HEAD.*ambiguous" warn
+'
+
@@ -232,3 +239,62 @@
+'
+
test_done
+
+ diff --git a/worktree.c b/worktree.c
+ --- a/worktree.c
+ +++ b/worktree.c
+@@
+ return ret;
+ }
+
++int parse_worktree_ref(const char *worktree_ref, const char **name,
++ int *name_length, const char **ref)
++{
++ if (skip_prefix(worktree_ref, "main-worktree/", &worktree_ref)) {
++ if (!*worktree_ref)
++ return -1;
++ if (name)
++ *name = NULL;
++ if (name_length)
++ *name_length = 0;
++ if (ref)
++ *ref = worktree_ref;
++ return 0;
++ }
++ if (skip_prefix(worktree_ref, "worktrees/", &worktree_ref)) {
++ const char *slash = strchr(worktree_ref, '/');
++
++ if (!slash || slash == worktree_ref || !slash[1])
++ return -1;
++ if (name)
++ *name = worktree_ref;
++ if (name_length)
++ *name_length = slash - worktree_ref;
++ if (ref)
++ *ref = slash + 1;
++ return 0;
++ }
++ return -1;
++}
++
+ int other_head_refs(each_ref_fn fn, void *cb_data)
+ {
+ struct worktree **worktrees, **p;
+
+ diff --git a/worktree.h b/worktree.h
+ --- a/worktree.h
+ +++ b/worktree.h
+@@
+ const char *fmt, ...)
+ __attribute__((format (printf, 2, 3)));
+
++/*
++ * Parse a worktree ref (i.e. with prefix main-worktree/ or
++ * worktrees/) and return the position of the worktree's name and
++ * length (or NULL and zero if it's main worktree), and ref.
++ *
++ * All name, name_length and ref arguments could be NULL.
++ */
++int parse_worktree_ref(const char *worktree_ref, const char **name,
++ int *name_length, const char **ref);
+ #endif
2: ffdd30f7fc = 2: e936a0af1e revision.c: correct a parameter name
3: 6809bab191 ! 3: 382c645d73 revision.c: better error reporting on ref from different worktrees
@@ -87,18 +87,35 @@
--- a/worktree.c
+++ b/worktree.c
@@
- return ret;
+ return -1;
}
+void strbuf_worktree_ref(const struct worktree *wt,
+ struct strbuf *sb,
+ const char *refname)
+{
-+ if (wt && !wt->is_current) {
-+ if (is_main_worktree(wt))
-+ strbuf_addstr(sb, "main/");
-+ else
-+ strbuf_addf(sb, "worktrees/%s/", wt->id);
++ switch (ref_type(refname)) {
++ case REF_TYPE_PSEUDOREF:
++ case REF_TYPE_PER_WORKTREE:
++ if (wt && !wt->is_current) {
++ if (is_main_worktree(wt))
++ strbuf_addstr(sb, "main-worktree/");
++ else
++ strbuf_addf(sb, "worktrees/%s/", wt->id);
++ }
++ break;
++
++ case REF_TYPE_MAIN_PSEUDOREF:
++ case REF_TYPE_OTHER_PSEUDOREF:
++ break;
++
++ case REF_TYPE_NORMAL:
++ /*
++ * For shared refs, don't prefix worktrees/ or
++ * main-worktree/. It's not necessary and
++ * files-backend.c can't handle it anyway.
++ */
++ break;
+ }
+ strbuf_addstr(sb, refname);
+}
@@ -141,9 +158,10 @@
--- a/worktree.h
+++ b/worktree.h
@@
- const char *fmt, ...)
- __attribute__((format (printf, 2, 3)));
-
+ */
+ int parse_worktree_ref(const char *worktree_ref, const char **name,
+ int *name_length, const char **ref);
++
+/*
+ * Return a refname suitable for access from the current ref store.
+ */
@@ -153,7 +171,7 @@
+
+/*
+ * Return a refname suitable for access from the current ref
-+ * store. The result may be destroyed at the next call.
++ * store. The result will be destroyed at the next call.
+ */
+const char *worktree_ref(const struct worktree *wt,
+ const char *refname);
4: 2e13fa8361 = 4: e2b99ef955 fsck: Move fsck_head_link() to get_default_heads() to avoid some globals
5: 65f1547df3 ! 5: 8fbff370c3 fsck: check HEAD and reflog from other worktrees
@@ -136,7 +136,7 @@
+ echo $ZERO_OID >.git/HEAD &&
+ # avoid corrupt/broken HEAD from interfering with repo discovery
+ test_must_fail git -C wt fsck 2>out &&
-+ grep "main/HEAD: detached HEAD points" out
++ grep "main-worktree/HEAD: detached HEAD points" out
+'
+
+test_expect_success 'other worktree HEAD link pointing at a funny object' '
6: 86326b44b5 ! 6: 2cd501f2ce reflog expire: cover reflog from all worktrees
@@ -48,12 +48,48 @@
};
/* Remember to update object flag allocation in object.h */
+@@
+ return 0;
+ }
+
++static int is_head(const char *refname)
++{
++ switch (ref_type(refname)) {
++ case REF_TYPE_OTHER_PSEUDOREF:
++ case REF_TYPE_MAIN_PSEUDOREF:
++ if (parse_worktree_ref(refname, NULL, NULL, &refname))
++ BUG("not a worktree ref: %s", refname);
++ break;
++ default:
++ break;
++ }
++ return !strcmp(refname, "HEAD");
++}
++
+ static void reflog_expiry_prepare(const char *refname,
+ const struct object_id *oid,
+ void *cb_data)
+ {
+ struct expire_reflog_policy_cb *cb = cb_data;
+
+- if (!cb->cmd.expire_unreachable || !strcmp(refname, "HEAD")) {
++ if (!cb->cmd.expire_unreachable || is_head(refname)) {
+ cb->tip_commit = NULL;
+ cb->unreachable_expire_kind = UE_HEAD;
+ } else {
@@
{
struct collected_reflog *e;
struct collect_reflog_cb *cb = cb_data;
+ struct strbuf newref = STRBUF_INIT;
+
++ /*
++ * Avoid collecting the same shared ref multiple times because
++ * they are available via all worktrees.
++ */
++ if (!cb->wt->is_current && ref_type(ref) == REF_TYPE_NORMAL)
++ return 0;
++
+ strbuf_worktree_ref(cb->wt, &newref, ref);
+ FLEX_ALLOC_STR(e, reflog, newref.buf);
+ strbuf_release(&newref);
@@ -94,9 +130,34 @@
+ if (!all_worktrees && !(*p)->is_current)
+ continue;
+ collected.wt = *p;
-+ for_each_reflog(collect_reflog, &collected);
++ refs_for_each_reflog(get_worktree_ref_store(*p),
++ collect_reflog, &collected);
+ }
+ free_worktrees(worktrees);
for (i = 0; i < collected.nr; i++) {
struct collected_reflog *e = collected.e[i];
set_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);
+
+ diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
+ --- a/t/t1410-reflog.sh
+ +++ b/t/t1410-reflog.sh
+@@
+ )
+ '
+
++test_expect_success 'expire with multiple worktrees' '
++ git init main-wt &&
++ (
++ cd main-wt &&
++ test_tick &&
++ test_commit foo &&
++ git worktree add link-wt &&
++ test_tick &&
++ test_commit -C link-wt foobar &&
++ test_tick &&
++ git reflog expire --verbose --all --expire=$test_tick &&
++ test_must_be_empty .git/worktrees/link-wt/logs/HEAD
++ )
++'
++
+ test_doneSignature · 1 line
2.19.1.647.g708186aaf9
PATCH v3 1/8 refs.c: indent with tabs, not spaces
--- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
refs.c +1 -1
@@ -646,7 +646,7 @@ enum ref_type ref_type(const char *refname)return REF_TYPE_PER_WORKTREE ;if (is_pseudoref_syntax (refname))return REF_TYPE_PSEUDOREF ; -return REF_TYPE_NORMAL ; +return REF_TYPE_NORMAL ; }long get_files_ref_lock_timeout_ms (void )
Signature · 1 line
2.19.1.647.g708186aaf9
PATCH v3 2/8 Add a place for (not) sharing stuff between worktrees
In reply to Nguyễn Thái Ngọc DuyWhen multiple worktrees are used, we need rules to determine if something belongs to one worktree or all of them. Instead of keeping adding rules when new stuff comes (*), have a generic rule:
- Inside $GIT_DIR, which is per-worktree by default, add $GIT_DIR/common which is always shared. New features that want to share stuff should put stuff under this directory.
- Inside refs/, which is shared by default except refs/bisect, add refs/worktree/ which is per-worktree. We may eventually move refs/bisect to this new location and remove the exception in refs code.
(*) And it may also include stuff from external commands which will
have no way to modify common/per-worktree rules.
--- Documentation/git-worktree.txt | 19 ++++++++++++++- Documentation/gitrepository-layout.txt | 11 +++++++-- path.c | 2 ++ refs.c | 1 + refs/files-backend.c | 14 ++++++++--- t/t0060-path-utils.sh | 2 ++ t/t1415-worktree-refs.sh | 33 ++++++++++++++++++++++++++ 7 files changed, 76 insertions(+), 6 deletions(-) create mode 100755 t/t1415-worktree-refs.sh
Documentation/git-worktree.txt +18 -1
@@ -204,6 +204,22 @@ working trees, it can be used to identify worktrees. For example if you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg", then "ghi" or "def/ghi" is enough to point to the former working tree. +REFS +---- +In multiple working trees, some refs may be shared between all working +trees, some refs are local. One example is HEAD is different for all +working trees. This section is about the sharing rules. + +In general, all pseudo refs are per working tree and all refs starting +with "refs/" are shared. Pseudo refs are ones like HEAD which are +directly under GIT_DIR instead of inside GIT_DIR/refs. There are one +exception to this: refs inside refs/bisect and refs/worktree is not +shared. + +To access refs, it's best not to look inside GIT_DIR directly. Instead +use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1] +which will handle refs correctly. + DETAILS ------- Each linked working tree has a private sub-directory in the repository's
@@ -228,7 +244,8 @@ linked working tree `git rev-parse --git-path HEAD` returns `/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git rev-parse --git-path refs/heads/master` uses $GIT_COMMON_DIR and returns `/path/main/.git/refs/heads/master`, -since refs are shared across all working trees. +since refs are shared across all working trees, except refs/bisect and +refs/worktree. See linkgit:gitrepository-layout[5] for more information. The rule of thumb is do not make any assumption about whether a path belongs to
Documentation/gitrepository-layout.txt +9 -2
@@ -95,8 +95,10 @@ refs:: References are stored in subdirectories of this directory. The 'git prune' command knows to preserve objects reachable from refs found in this directory and - its subdirectories. This directory is ignored if $GIT_COMMON_DIR - is set and "$GIT_COMMON_DIR/refs" will be used instead. + its subdirectories. + This directory is ignored (except refs/bisect and + refs/worktree) if $GIT_COMMON_DIR is set and + "$GIT_COMMON_DIR/refs" will be used instead. refs/heads/`name`:: records tip-of-the-tree commit objects of branch `name`
@@ -165,6 +167,11 @@ hooks:: each hook. This directory is ignored if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/hooks" will be used instead. +common:: + When multiple working trees are used, most of files in + $GIT_DIR are per-worktree with a few known exceptions. All + files under 'common' however will be shared between all + working trees. index:: The current index file for the repository. It is
path.c +2 -0
@@ -108,6 +108,7 @@ struct common_dir {static struct common_dir common_list[] = { {0 ,1 ,0 ,"branches" }, + {0 ,1 ,0 ,"common" }, {0 ,1 ,0 ,"hooks" }, {0 ,1 ,0 ,"info" }, {0 ,0 ,1 ,"info/sparse-checkout" },
@@ -118,6 +119,7 @@ static struct common_dir common_list[] = { { 0, 1, 0, "objects" }, { 0, 1, 0, "refs" }, { 0, 1, 1, "refs/bisect" }, + { 0, 1, 1, "refs/worktree" }, { 0, 1, 0, "remotes" }, { 0, 1, 0, "worktrees" }, { 0, 1, 0, "rr-cache" },
refs.c +1 -0
@@ -624,6 +624,7 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)static int is_per_worktree_ref (const char *refname) {return !strcmp (refname,"HEAD" ) || +starts_with (refname,"refs/worktree/" ) ||starts_with (refname,"refs/bisect/" ) ||starts_with (refname,"refs/rewritten/" ); }
refs/files-backend.c +11 -3
@@ -269,9 +269,9 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,closedir (d);/* -* Manually add refs/bisect, which, being per-worktree, might -* not appear in the directory listing for refs/ in the main -* repo. +* Manually add refs/bisect and refs/worktree, which, being +* per-worktree, might not appear in the directory listing for +* refs/ in the main repo. */ if (!strcmp (dirname,"refs/" )) {int pos =search_ref_dir (dir,"refs/bisect/" ,12 );
@@ -281,6 +281,14 @@ static void loose_fill_ref_dir(struct ref_store *ref_store, dir->cache, "refs/bisect/", 12, 1);add_entry_to_dir (dir, child_entry); } + + pos =search_ref_dir (dir,"refs/worktree/" ,11 ); + +if (pos <0 ) { +struct ref_entry *child_entry =create_dir_entry ( + dir->cache,"refs/worktree/" ,11 ,1 ); +add_entry_to_dir (dir, child_entry); + } } }
t/t0060-path-utils.sh +2 -0
@@ -306,6 +306,8 @@ test_git_path GIT_COMMON_DIR=bar hooks/me bar/hooks/me test_git_path GIT_COMMON_DIR=bar config bar/config test_git_path GIT_COMMON_DIR=bar packed-refs bar/packed-refs test_git_path GIT_COMMON_DIR=bar shallow bar/shallow +test_git_path GIT_COMMON_DIR=bar common bar/common +test_git_path GIT_COMMON_DIR=bar common/file bar/common/file # In the tests below, $(pwd) must be used because it is a native path on # Windows and avoids MSYS's path mangling (which simplifies "foo/../bar" and
t/t1415-worktree-refs.sh new file +33 -0
@@ -0,0 +1,33 @@ +#!/bin/sh + +test_description='per-worktree refs' + +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit initial && + test_commit wt1 && + test_commit wt2 && + git worktree add wt1 wt1 && + git worktree add wt2 wt2 && + git checkout initial && + git update-ref refs/worktree/foo HEAD && + git -C wt1 update-ref refs/worktree/foo HEAD && + git -C wt2 update-ref refs/worktree/foo HEAD +' + +test_expect_success 'refs/worktree must not be packed' ' + git pack-refs --all && + test_path_is_missing .git/refs/tags/wt1 && + test_path_is_file .git/refs/worktree/foo && + test_path_is_file .git/worktrees/wt1/refs/worktree/foo && + test_path_is_file .git/worktrees/wt2/refs/worktree/foo +' + +test_expect_success 'refs/worktree are per-worktree' ' + test_cmp_rev worktree/foo initial && + ( cd wt1 && test_cmp_rev worktree/foo wt1 ) && + ( cd wt2 && test_cmp_rev worktree/foo wt2 ) +' + +test_done
Signature · 1 line
2.19.1.647.g708186aaf9
PATCH v3 3/8 refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Nguyễn Thái Ngọc DuyOne of the problems with multiple worktree is accessing per-worktree refs of one worktree from another worktree. This was sort of solved by multiple ref store, where the code can open the ref store of another worktree and has access to the ref space of that worktree.
The problem with this is reporting. "HEAD" in another ref space is also called "HEAD" like in the current ref space. In order to differentiate them, all the code must somehow carry the ref store around and print something like "HEAD from this ref store".
But that is not feasible (or possible with a _lot_ of work). With the current design, we pass a reference around as a string (so called "refname"). Extending this design to pass a string _and_ a ref store is a nightmare, especially when handling extended SHA-1 syntax.
So we do it another way. Instead of entering a separate ref space, we make refs from other worktrees available in the current ref space. So "HEAD" is always HEAD of the current worktree, but then we can have "worktrees/blah/HEAD" to denote HEAD from a worktree named "blah". This syntax coincidentally matches the underlying directory structure which makes implementation a bit easier.
The main worktree has to be treated specially because well... it's special from the beginning. So HEAD from the main worktree is acccessible via the name "main-worktree/HEAD" instead of "worktrees/main/HEAD" because "main" could be just another secondary worktree.
This patch also makes it possible to specify refs from one worktree in another one, e.g.
git log worktrees/foo/HEAD
--- Documentation/git-worktree.txt | 15 ++++++++++- refs.c | 21 ++++++++++++++++ refs.h | 8 +++--- refs/files-backend.c | 28 +++++++++++++++++++++ t/t1415-worktree-refs.sh | 46 ++++++++++++++++++++++++++++++++++ worktree.c | 30 ++++++++++++++++++++++ worktree.h | 9 +++++++ 7 files changed, 153 insertions(+), 4 deletions(-)
Documentation/git-worktree.txt +14 -1
@@ -208,7 +208,8 @@ REFS ---- In multiple working trees, some refs may be shared between all working trees, some refs are local. One example is HEAD is different for all -working trees. This section is about the sharing rules. +working trees. This section is about the sharing rules and how to access +refs of one working tree from another. In general, all pseudo refs are per working tree and all refs starting with "refs/" are shared. Pseudo refs are ones like HEAD which are
@@ -216,6 +217,18 @@ directly under GIT_DIR instead of inside GIT_DIR/refs. There are one exception to this: refs inside refs/bisect and refs/worktree is not shared. +Refs that are per working tree can still be accessed from another +working tree via two special paths, main-worktree and worktrees. The +former gives access to per-worktree refs of the main working tree, +while the latter to all linked working trees. + +For example, main-worktree/HEAD or main-worktree/refs/bisect/good +resolve to the same value as the main working tree's HEAD and +refs/bisect/good respectively. Similarly, worktrees/foo/HEAD or +worktrees/bar/refs/bisect/bad are the same as +GIT_COMMON_DIR/worktrees/foo/HEAD and +GIT_COMMON_DIR/worktrees/bar/refs/bisect/bad. + To access refs, it's best not to look inside GIT_DIR directly. Instead use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1] which will handle refs correctly.
refs.c +21 -0
@@ -641,12 +641,33 @@ static int is_pseudoref_syntax(const char *refname)return 1 ; } +static int is_main_pseudoref_syntax (const char *refname) +{ +return skip_prefix (refname,"main-worktree/" , &refname) && + *refname && +is_pseudoref_syntax (refname); +} + +static int is_other_pseudoref_syntax (const char *refname) +{ +if (!skip_prefix (refname,"worktrees/" , &refname)) +return 0 ; + refname =strchr (refname,'/' ); +if (!refname || !refname[1 ]) +return 0 ; +return is_pseudoref_syntax (refname +1 ); +} +enum ref_type ref_type (const char *refname) {if (is_per_worktree_ref (refname))return REF_TYPE_PER_WORKTREE ;if (is_pseudoref_syntax (refname))return REF_TYPE_PSEUDOREF ; +if (is_main_pseudoref_syntax (refname)) +return REF_TYPE_MAIN_PSEUDOREF ; +if (is_other_pseudoref_syntax (refname)) +return REF_TYPE_OTHER_PSEUDOREF ;return REF_TYPE_NORMAL ; }
refs.h +5 -3
@@ -714,9 +714,11 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);int ref_is_hidden (const char *,const char *);enum ref_type { -REF_TYPE_PER_WORKTREE , -REF_TYPE_PSEUDOREF , -REF_TYPE_NORMAL , +REF_TYPE_PER_WORKTREE ,/* refs inside refs/ but not shared */ +REF_TYPE_PSEUDOREF ,/* refs outside refs/ in current worktree */ +REF_TYPE_MAIN_PSEUDOREF ,/* pseudo refs from the main worktree */ +REF_TYPE_OTHER_PSEUDOREF ,/* pseudo refs from other worktrees */ +REF_TYPE_NORMAL ,/* normal/shared refs inside refs/ */ };enum ref_type ref_type (const char *refname);
refs/files-backend.c +28 -0
@@ -10,6 +10,7 @@#include "../object.h" #include "../dir.h" #include "../chdir-notify.h" +#include "worktree.h" /* * This backend uses the following flags in `ref_update::flags` for
@@ -149,6 +150,25 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,return refs; } +static void files_reflog_path_other_worktrees (struct files_ref_store *refs, +struct strbuf *sb, +const char *refname) +{ +const char *real_ref; +const char *worktree_name; +int length; + +if (parse_worktree_ref (refname, &worktree_name, &length, &real_ref)) +BUG ("refname %s is not a other-worktree ref" , refname); + +if (worktree_name) +strbuf_addf (sb,"%s/worktrees/%.*s/logs/%s" , refs->gitcommondir, + length, worktree_name, real_ref); +else +strbuf_addf (sb,"%s/logs/%s" , refs->gitcommondir, + real_ref); +} +static void files_reflog_path (struct files_ref_store *refs,struct strbuf *sb,const char *refname)
@@ -158,6 +178,9 @@ static void files_reflog_path(struct files_ref_store *refs,case REF_TYPE_PSEUDOREF :strbuf_addf (sb,"%s/logs/%s" , refs->gitdir, refname);break ; +case REF_TYPE_OTHER_PSEUDOREF : +case REF_TYPE_MAIN_PSEUDOREF : +return files_reflog_path_other_worktrees (refs, sb, refname);case REF_TYPE_NORMAL :strbuf_addf (sb,"%s/logs/%s" , refs->gitcommondir, refname);break ;
@@ -176,6 +199,11 @@ static void files_ref_path(struct files_ref_store *refs,case REF_TYPE_PSEUDOREF :strbuf_addf (sb,"%s/%s" , refs->gitdir, refname);break ; +case REF_TYPE_MAIN_PSEUDOREF : +if (!skip_prefix (refname,"main-worktree/" , &refname)) +BUG ("ref %s is not a main pseudoref" , refname); +/* fallthrough */ +case REF_TYPE_OTHER_PSEUDOREF :case REF_TYPE_NORMAL :strbuf_addf (sb,"%s/%s" , refs->gitcommondir, refname);break ;
t/t1415-worktree-refs.sh +46 -0
50 lines not on this page · raw message
worktree.c +30 -0
@@ -487,6 +487,36 @@ int submodule_uses_worktrees(const char *path)return ret; } +int parse_worktree_ref (const char *worktree_ref,const char **name, +int *name_length,const char **ref) +{ +if (skip_prefix (worktree_ref,"main-worktree/" , &worktree_ref)) { +if (!*worktree_ref) +return -1 ; +if (name) + *name =NULL ; +if (name_length) + *name_length =0 ; +if (ref) + *ref = worktree_ref; +return 0 ; + } +if (skip_prefix (worktree_ref,"worktrees/" , &worktree_ref)) { +const char *slash =strchr (worktree_ref,'/' ); + +if (!slash || slash == worktree_ref || !slash[1 ]) +return -1 ; +if (name) + *name = worktree_ref; +if (name_length) + *name_length = slash - worktree_ref; +if (ref) + *ref = slash +1 ; +return 0 ; + } +return -1 ; +} +int other_head_refs (each_ref_fn fn,void *cb_data) {struct worktree **worktrees, **p;
worktree.h +9 -0
@@ -108,4 +108,13 @@ extern const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...) __attribute__((format (printf, 2, 3))); +/* +* Parse a worktree ref (i.e. with prefix main-worktree/ or +* worktrees/) and return the position of the worktree's name and +* length (or NULL and zero if it's main worktree), and ref. +* +* All name, name_length and ref arguments could be NULL. +*/ +int parse_worktree_ref (const char *worktree_ref,const char **name, +int *name_length,const char **ref);#endif
Signature · 1 line
2.19.1.647.g708186aaf9
PATCH v3 4/8 revision.c: correct a parameter name
In reply to Nguyễn Thái Ngọc DuyThis function is a callback of for_each_reflog() which will pass a ref name as the first argument, not a path (to a reflog file).
--- revision.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
revision.c +4 -3
@@ -1278,13 +1278,14 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,return 0 ; } -static int handle_one_reflog (const char *path,const struct object_id *oid, +static int handle_one_reflog (const char *refname, +const struct object_id *oid,int flag,void *cb_data) {struct all_refs_cb *cb = cb_data; cb->warned_bad_reflog =0 ; - cb->name_for_errormsg = path; -refs_for_each_reflog_ent (cb->refs, path, + cb->name_for_errormsg = refname; +refs_for_each_reflog_ent (cb->refs, refname, handle_one_reflog_ent, cb_data);return 0 ; }
Signature · 1 line
2.19.1.647.g708186aaf9
PATCH v3 5/8 revision.c: better error reporting on ref from different worktrees
In reply to Nguyễn Thái Ngọc DuyMake use of the new ref aliases to pass refs from another worktree around and access them from the current ref store instead. This does not change any functionality, but when a problem arises, we would like the reported messages to mention full ref aliases, like this:
fatal: bad object worktrees/ztemp/HEAD
warning: reflog of 'main-worktree/HEAD' references pruned commits
instead of
fatal: bad object HEAD
warning: reflog of 'HEAD' references pruned commits
which does not really tell where the refs are from.
--- revision.c | 21 +++++++++++++-------- worktree.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++--- worktree.h | 15 +++++++++++++++ 3 files changed, 74 insertions(+), 11 deletions(-)
revision.c +13 -8
@@ -1178,7 +1178,7 @@ struct all_refs_cb {int warned_bad_reflog;struct rev_info *all_revs;const char *name_for_errormsg; -struct ref_store *refs; +struct worktree *wt; };int ref_excluded (struct string_list *ref_excludes,const char *path)
@@ -1215,7 +1215,7 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs, cb->all_revs = revs; cb->all_flags = flags; revs->rev_input_given =1 ; - cb->refs =NULL ; + cb->wt =NULL ; }void clear_ref_exclusion (struct string_list **ref_excludes_p)
@@ -1278,15 +1278,20 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,return 0 ; } -static int handle_one_reflog (const char *refname, +static int handle_one_reflog (const char *refname_in_wt,const struct object_id *oid,int flag,void *cb_data) {struct all_refs_cb *cb = cb_data; +struct strbuf refname =STRBUF_INIT ; + cb->warned_bad_reflog =0 ; - cb->name_for_errormsg = refname; -refs_for_each_reflog_ent (cb->refs, refname, +strbuf_worktree_ref (cb->wt, &refname, refname_in_wt); + cb->name_for_errormsg = refname.buf; +refs_for_each_reflog_ent (get_main_ref_store (the_repository), + refname.buf, handle_one_reflog_ent, cb_data); +strbuf_release (&refname);return 0 ; }
@@ -1301,8 +1306,8 @@ static void add_other_reflogs_to_pending(struct all_refs_cb *cb)if (wt->is_current)continue ; - cb->refs =get_worktree_ref_store (wt); -refs_for_each_reflog (cb->refs, + cb->wt = wt; +refs_for_each_reflog (get_worktree_ref_store (wt), handle_one_reflog, cb); }
@@ -1315,7 +1320,7 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags) cb.all_revs = revs; cb.all_flags = flags; - cb.refs =get_main_ref_store (revs->repo); + cb.wt =NULL ;for_each_reflog (handle_one_reflog, &cb);if (!revs->single_worktree)
worktree.c +46 -3
65 lines not on this page · raw message
worktree.h +15 -0
@@ -117,4 +117,19 @@ extern const char *worktree_git_path(const struct worktree *wt,*/ int parse_worktree_ref (const char *worktree_ref,const char **name,int *name_length,const char **ref); + +/* +* Return a refname suitable for access from the current ref store. +*/ +void strbuf_worktree_ref (const struct worktree *wt, +struct strbuf *sb, +const char *refname); + +/* +* Return a refname suitable for access from the current ref +* store. The result will be destroyed at the next call. +*/ +const char *worktree_ref (const struct worktree *wt, +const char *refname); +#endif
Signature · 1 line
2.19.1.647.g708186aaf9
PATCH v3 6/8 fsck: Move fsck_head_link() to get_default_heads() to avoid some globals
In reply to Nguyễn Thái Ngọc DuyFrom: Elijah Newren <[email protected]>
This will make it easier to check the HEAD of other worktrees from fsck.
--- builtin/fsck.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-)
builtin/fsck.c +16 -11
@@ -36,8 +36,6 @@ static int check_strict;static int keep_cache_objects;static struct fsck_options fsck_walk_options =FSCK_OPTIONS_DEFAULT ;static struct fsck_options fsck_obj_options =FSCK_OPTIONS_DEFAULT ; -static struct object_id head_oid; -static const char *head_points_at;static int errors_found;static int write_lost_and_found;static int verbose;
@@ -484,8 +482,15 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,return 0 ; } +static int fsck_head_link (const char **head_points_at, +struct object_id *head_oid); +static void get_default_heads (void ) { +const char *head_points_at; +struct object_id head_oid; + +fsck_head_link (&head_points_at, &head_oid);if (head_points_at && !is_null_oid (&head_oid))fsck_handle_ref ("HEAD" , &head_oid,0 ,NULL );for_each_rawref (fsck_handle_ref,NULL );
@@ -579,33 +584,34 @@ static void fsck_object_dir(const char *path)stop_progress (&progress); } -static int fsck_head_link (void ) +static int fsck_head_link (const char **head_points_at, +struct object_id *head_oid) {int null_is_error =0 ;if (verbose)fprintf (stderr,"Checking HEAD link\n" ); - head_points_at =resolve_ref_unsafe ("HEAD" ,0 , &head_oid,NULL ); -if (!head_points_at) { + *head_points_at =resolve_ref_unsafe ("HEAD" ,0 , head_oid,NULL ); +if (!*head_points_at) { errors_found |=ERROR_REFS ;return error ("Invalid HEAD" ); } -if (!strcmp (head_points_at,"HEAD" )) +if (!strcmp (*head_points_at,"HEAD" ))/* detached HEAD */ null_is_error =1 ; -else if (!starts_with (head_points_at,"refs/heads/" )) { +else if (!starts_with (*head_points_at,"refs/heads/" )) { errors_found |=ERROR_REFS ;return error ("HEAD points to something strange (%s)" , - head_points_at); + *head_points_at); } -if (is_null_oid (&head_oid)) { +if (is_null_oid (head_oid)) {if (null_is_error) { errors_found |=ERROR_REFS ;return error ("HEAD: detached HEAD points at nothing" ); }fprintf (stderr,"notice: HEAD points to an unborn branch (%s)\n" , - head_points_at +11 ); + *head_points_at +11 ); }return 0 ; }
@@ -720,7 +726,6 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)git_config (fsck_config,NULL ); -fsck_head_link ();if (connectivity_only) {for_each_loose_object (mark_loose_for_connectivity,NULL ,0 );for_each_packed_object (mark_packed_for_connectivity,NULL ,0 );
Signature · 1 line
2.19.1.647.g708186aaf9
PATCH v3 7/8 fsck: check HEAD and reflog from other worktrees
In reply to Nguyễn Thái Ngọc Duyfsck is a repo-wide operation and should check all references no matter which worktree they are associated to.
--- builtin/fsck.c | 55 ++++++++++++++++++++++++++++++++++--------------- t/t1450-fsck.sh | 35 +++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 17 deletions(-)
builtin/fsck.c +38 -17
@@ -19,6 +19,7 @@#include "packfile.h" #include "object-store.h" #include "run-command.h" +#include "worktree.h" #define REACHABLE 0x0001#define SEEN 0x0002
@@ -444,7 +445,11 @@ static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noidstatic int fsck_handle_reflog (const char *logname,const struct object_id *oid,int flag,void *cb_data) { -for_each_reflog_ent (logname, fsck_handle_reflog_ent, (void *)logname); +struct strbuf refname =STRBUF_INIT ; + +strbuf_worktree_ref (cb_data, &refname, logname); +for_each_reflog_ent (refname.buf, fsck_handle_reflog_ent, refname.buf); +strbuf_release (&refname);return 0 ; }
@@ -482,20 +487,34 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,return 0 ; } -static int fsck_head_link (const char **head_points_at, +static int fsck_head_link (const char *head_ref_name, +const char **head_points_at,struct object_id *head_oid);static void get_default_heads (void ) { +struct worktree **worktrees, **p;const char *head_points_at;struct object_id head_oid; -fsck_head_link (&head_points_at, &head_oid); -if (head_points_at && !is_null_oid (&head_oid)) -fsck_handle_ref ("HEAD" , &head_oid,0 ,NULL );for_each_rawref (fsck_handle_ref,NULL ); -if (include_reflogs) -for_each_reflog (fsck_handle_reflog,NULL ); + + worktrees =get_worktrees (0 ); +for (p = worktrees; *p; p++) { +struct worktree *wt = *p; +struct strbuf ref =STRBUF_INIT ; + +strbuf_worktree_ref (wt, &ref,"HEAD" ); +fsck_head_link (ref.buf, &head_points_at, &head_oid); +if (head_points_at && !is_null_oid (&head_oid)) +fsck_handle_ref (ref.buf, &head_oid,0 ,NULL ); +strbuf_release (&ref); + +if (include_reflogs) +refs_for_each_reflog (get_worktree_ref_store (wt), + fsck_handle_reflog, wt); + } +free_worktrees (worktrees);/* * Not having any default heads isn't really fatal, but
@@ -584,34 +603,36 @@ static void fsck_object_dir(const char *path)stop_progress (&progress); } -static int fsck_head_link (const char **head_points_at, +static int fsck_head_link (const char *head_ref_name, +const char **head_points_at,struct object_id *head_oid) {int null_is_error =0 ;if (verbose) -fprintf (stderr,"Checking HEAD link\n" ); +fprintf (stderr,"Checking %s link\n" , head_ref_name); - *head_points_at =resolve_ref_unsafe ("HEAD" ,0 , head_oid,NULL ); + *head_points_at =resolve_ref_unsafe (head_ref_name,0 , head_oid,NULL );if (!*head_points_at) { errors_found |=ERROR_REFS ; -return error ("Invalid HEAD" ); +return error ("Invalid %s" , head_ref_name); } -if (!strcmp (*head_points_at,"HEAD" )) +if (!strcmp (*head_points_at, head_ref_name))/* detached HEAD */ null_is_error =1 ;else if (!starts_with (*head_points_at,"refs/heads/" )) { errors_found |=ERROR_REFS ; -return error ("HEAD points to something strange (%s)" , - *head_points_at); +return error ("%s points to something strange (%s)" , + head_ref_name, *head_points_at); }if (is_null_oid (head_oid)) {if (null_is_error) { errors_found |=ERROR_REFS ; -return error ("HEAD: detached HEAD points at nothing" ); +return error ("%s: detached HEAD points at nothing" , + head_ref_name); } -fprintf (stderr,"notice: HEAD points to an unborn branch (%s)\n" , - *head_points_at +11 ); +fprintf (stderr,"notice: %s points to an unborn branch (%s)\n" , + head_ref_name, *head_points_at +11 ); }return 0 ; }
t/t1450-fsck.sh +35 -0
@@ -101,6 +101,41 @@ test_expect_success 'HEAD link pointing at a funny place' ' grep "HEAD points to something strange" out ' +test_expect_success 'HEAD link pointing at a funny object (from different wt)' ' + test_when_finished "mv .git/SAVED_HEAD .git/HEAD" && + test_when_finished "rm -rf .git/worktrees wt" && + git worktree add wt && + mv .git/HEAD .git/SAVED_HEAD && + echo $ZERO_OID >.git/HEAD && + # avoid corrupt/broken HEAD from interfering with repo discovery + test_must_fail git -C wt fsck 2>out && + grep "main-worktree/HEAD: detached HEAD points" out +' + +test_expect_success 'other worktree HEAD link pointing at a funny object' ' + test_when_finished "rm -rf .git/worktrees other" && + git worktree add other && + echo $ZERO_OID >.git/worktrees/other/HEAD && + test_must_fail git fsck 2>out && + grep "worktrees/other/HEAD: detached HEAD points" out +' + +test_expect_success 'other worktree HEAD link pointing at missing object' ' + test_when_finished "rm -rf .git/worktrees other" && + git worktree add other && + echo "Contents missing from repo" | git hash-object --stdin >.git/worktrees/other/HEAD && + test_must_fail git fsck 2>out && + grep "worktrees/other/HEAD: invalid sha1 pointer" out +' + +test_expect_success 'other worktree HEAD link pointing at a funny place' ' + test_when_finished "rm -rf .git/worktrees other" && + git worktree add other && + echo "ref: refs/funny/place" >.git/worktrees/other/HEAD && + test_must_fail git fsck 2>out && + grep "worktrees/other/HEAD points to something strange" out +' + test_expect_success 'email without @ is okay' ' git cat-file commit HEAD >basis && sed "s/@/AT/" basis >okay &&
Signature · 1 line
2.19.1.647.g708186aaf9
PATCH v3 8/8 reflog expire: cover reflog from all worktrees
In reply to Nguyễn Thái Ngọc Duy--- Documentation/git-reflog.txt | 7 +++++- builtin/reflog.c | 46 ++++++++++++++++++++++++++++++++---- t/t1410-reflog.sh | 15 ++++++++++++ 3 files changed, 63 insertions(+), 5 deletions(-)
Documentation/git-reflog.txt +6 -1
@@ -20,7 +20,7 @@ depending on the subcommand: 'git reflog' ['show'] [log-options] [<ref>] 'git reflog expire' [--expire=<time>] [--expire-unreachable=<time>] [--rewrite] [--updateref] [--stale-fix] - [--dry-run | -n] [--verbose] [--all | <refs>...] + [--dry-run | -n] [--verbose] [--all [--single-worktree] | <refs>...] 'git reflog delete' [--rewrite] [--updateref] [--dry-run | -n] [--verbose] ref@\{specifier\}... 'git reflog exists' <ref>
@@ -72,6 +72,11 @@ Options for `expire` --all:: Process the reflogs of all references. +--single-worktree:: + By default when `--all` is specified, reflogs from all working + trees are processed. This option limits the processing to reflogs + from the current working tree only. + --expire=<time>:: Prune entries older than the specified time. If this option is not specified, the expiration time is taken from the
builtin/reflog.c +42 -4
98 lines not on this page · raw message
t/t1410-reflog.sh +15 -0
@@ -368,4 +368,19 @@ test_expect_success 'continue walking past root commits' ' ) ' +test_expect_success 'expire with multiple worktrees' ' + git init main-wt && + ( + cd main-wt && + test_tick && + test_commit foo && + git worktree add link-wt && + test_tick && + test_commit -C link-wt foobar && + test_tick && + git reflog expire --verbose --all --expire=$test_tick && + test_must_be_empty .git/worktrees/link-wt/logs/HEAD + ) +' + test_done
Signature · 1 line
2.19.1.647.g708186aaf9
Re: [PATCH v3 2/8] Add a place for (not) sharing stuff between worktrees
In reply to Nguyễn Thái Ngọc DuyQuoted 1 line · Nguyễn Thái Ngọc Duy
Subject: Re: [PATCH v3 2/8] Add a place for (not) sharing stuff between worktrees
"a place"? Missing "in $GIR_DIR" in the descrition made me read the above three times before getting what it wanted to say.
My attempt to improve it, which admittedly is not great, came up with:
worktree: convention to make per-worktree things identifiable in $GIT_DIR
Quoted 8 lines
When multiple worktrees are used, we need rules to determine if something belongs to one worktree or all of them. Instead of keeping adding rules when new stuff comes (*), have a generic rule:
- Inside $GIT_DIR, which is per-worktree by default, add $GIT_DIR/common which is always shared. New features that want to share stuff should put stuff under this directory.
Quoted 7 lines
- Inside refs/, which is shared by default except refs/bisect, add refs/worktree/ which is per-worktree. We may eventually move refs/bisect to this new location and remove the exception in refs code.
(*) And it may also include stuff from external commands which will
have no way to modify common/per-worktree rules.
OK. Establishing such a convention is a good role for the core-git should play to help third-party tools.
Should this play well with the per-worktree configuration as well? Is it better to carve out a configuration variable namespace so that certain keys are never read from common ones (or per-worktree ones), so that people can tell which ones are what? I know your current design says "this is just another new layer, and the users can hang themselves with this new rope". I am wondering if there is a need to do something a bit more structured.
Re: [PATCH v3 2/8] Add a place for (not) sharing stuff between worktrees
In reply to Nguyễn Thái Ngọc DuyQuoted 12 lines · Nguyễn Thái Ngọc Duy
Documentation/git-worktree.txt +4
-0@@ -204,6 +204,22 @@ working trees, it can be used to identify worktrees. For example if you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg", then "ghi" or "def/ghi" is enough to point to the former working tree. +REFS +---- +In multiple working trees, some refs may be shared between all working +trees, some refs are local. One example is HEAD is different for all
Documentation/git-worktree.txt working trees, it can be used to identify worktrees. For example if · line 220
Quoted 10 lines
Documentation/git-worktree.txt +10
-0+working trees. This section is about the sharing rules. + +In general, all pseudo refs are per working tree and all refs starting +with "refs/" are shared. Pseudo refs are ones like HEAD which are +directly under GIT_DIR instead of inside GIT_DIR/refs. There are one +exception to this: refs inside refs/bisect and refs/worktree is not +shared. + +To access refs, it's best not to look inside GIT_DIR directly. Instead +use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1]
s/revparse/rev-parse/
Quoted 2 lines
Documentation/git-worktree.txt +2
-0+which will handle refs correctly. +
Re: [PATCH v3 2/8] Add a place for (not) sharing stuff between worktrees
In reply to Junio C HamanoQuoted 19 lines · Junio C Hamano
Quoted 8 lines
When multiple worktrees are used, we need rules to determine if something belongs to one worktree or all of them. Instead of keeping adding rules when new stuff comes (*), have a generic rule:
- Inside $GIT_DIR, which is per-worktree by default, add $GIT_DIR/common which is always shared. New features that want to share stuff should put stuff under this directory.
Quoted 7 lines
- Inside refs/, which is shared by default except refs/bisect, add refs/worktree/ which is per-worktree. We may eventually move refs/bisect to this new location and remove the exception in refs code.
(*) And it may also include stuff from external commands which will
have no way to modify common/per-worktree rules.OK. Establishing such a convention is a good role for the core-git should play to help third-party tools.
Quoted 7 lines
Should this play well with the per-worktree configuration as well? Is it better to carve out a configuration variable namespace so that certain keys are never read from common ones (or per-worktree ones), so that people can tell which ones are what? I know your current design says "this is just another new layer, and the users can hang themselves with this new rope". I am wondering if there is a need to do something a bit more structured.
I actually find the layered design of config files more elegant. Even before config.worktree is added, the user has system/per-user/per-repo places to put stuff in. "git config" gives control to read or write from a specific layer. We have to add layers to $GIT_DIR/refs to handle multiple worktrees, but since the "API" for those don't have the concept of layers at all, we need a single view where items from different worktrees are accessible via the same path/ref name.
Adding variable namespace in config files does not look easy either. Suppose we have perWorktree.* section just for per-workree stuff, what do we do when the user just do "git config -f not-per-worktree-file perWorktree.something foo" (or --global instead of -f)? What we could do (and already do for some config keys) is ignore perWorktree.* unless they come from config.worktree. Which does help a bit, but not really perfect.
And a namespace like this means the user cannot have subsections anymore (e.g. perWorktree.group.<name>.var is not supported if I remember correctly). So yeah... perhaps we should wait a bit and see what the user (or third party tool makers) comes up with.
Signature · 1 line
Duy
Re: [PATCH v3 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
In reply to Nguyễn Thái Ngọc DuyOn Sun, Oct 21 2018, Nguyễn Thái Ngọc Duy wrote:
This change has a regression in 2.20:
Quoted 5 lines
[...]
static void files_reflog_path(structfiles_ref_store*refs,refs/files-backend.c +0
-0struct strbuf *sb, const char *refname)
refs/files-backend.c static void files_reflog_path(struct files_ref_store *refs, · line 186
Quoted 9 lines
refs/files-backend.c +3
-0@@ -158,6 +178,9 @@ static void files_reflog_path(struct files_ref_store *refs,case REF_TYPE_PSEUDOREF :strbuf_addf (sb,"%s/logs/%s" , refs->gitdir, refname);break ; +case REF_TYPE_OTHER_PSEUDOREF : +case REF_TYPE_MAIN_PSEUDOREF : +return files_reflog_path_other_worktrees (refs, sb, refname);case REF_TYPE_NORMAL :strbuf_addf (sb,"%s/logs/%s" , refs->gitcommondir, refname);break ;
SunCC on Solaris hard errors on this:
"refs/files-backend.c", line 183: void function cannot return value
Needs to be files...(); break; instead.
Re: [PATCH v3 3/8] refs: new ref types to make per-worktree refs visible to all worktrees
Quoted 21 lines · Ævar Arnfjörð Bjarmason
On Sun, Oct 21 2018, Nguyễn Thái Ngọc Duy wrote:
This change has a regression in 2.20:
Quoted 5 lines
[...]
static void files_reflog_path(structfiles_ref_store*refs,refs/files-backend.c +0
-0struct strbuf *sb, const char *refname)refs/files-backend.c static void files_reflog_path(struct files_ref_store *refs, · line 186
Quoted 9 lines
refs/files-backend.c +3
-0@@ -158,6 +178,9 @@ static void files_reflog_path(struct files_ref_store *refs,case REF_TYPE_PSEUDOREF :strbuf_addf (sb,"%s/logs/%s" , refs->gitdir, refname);break ; +case REF_TYPE_OTHER_PSEUDOREF : +case REF_TYPE_MAIN_PSEUDOREF : +return files_reflog_path_other_worktrees (refs, sb, refname);case REF_TYPE_NORMAL :strbuf_addf (sb,"%s/logs/%s" , refs->gitcommondir, refname);break ;SunCC on Solaris hard errors on this:
Quoted 3 lines
"
refs/files-backend.c", line 183: void function cannot return valueNeeds to be files...(); break; instead.
True.
The caller itself returns "void", so it would be nice if this were a mere warning() from practical usabliity's point of view, though ;-)
PATCH 1/1 files-backend.c: fix build error on Solaris
In reply to Ævar Arnfjörð BjarmasonThis function files_reflog_path returns void, which usually means "return;" not returning "void value" from another function.
--- refs/files-backend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
refs/files-backend.c +2 -1
@@ -180,7 +180,8 @@ static void files_reflog_path(struct files_ref_store *refs,break ;case REF_TYPE_OTHER_PSEUDOREF :case REF_TYPE_MAIN_PSEUDOREF : -return files_reflog_path_other_worktrees (refs, sb, refname); +files_reflog_path_other_worktrees (refs, sb, refname); +break ;case REF_TYPE_NORMAL :strbuf_addf (sb,"%s/logs/%s" , refs->gitcommondir, refname);break ;
Signature · 1 line
2.19.1.1327.g328c130451.dirty
Re: [PATCH] files-backend.c: fix build error on Solaris
clang with -Wpedantic also catch this (at least with Apple LLVM version 10.0.0); recent versions of gcc also include that flag and at least 8.2.0 shows a warning for it, so it might be worth adding it to developer mode (maybe under the pedantic DEVOPTS), would this be something I should be adding?, what are the expectations around warnings and compilers?
Carlo
Re: [PATCH] files-backend.c: fix build error on Solaris
Quoted 9 lines · Carlo Arenas
Signed-off-by Carlo Marcelo Arenas Belón <[email protected]>clang with -Wpedantic also catch this (at least with Apple LLVM version 10.0.0); recent versions of gcc also include that flag and at least 8.2.0 shows a warning for it, so it might be worth adding it to developer mode (maybe under the pedantic DEVOPTS), would this be something I should be adding?, what are the expectations around warnings and compilers?
make DEVELOPER=1 DEVOPTS=pedantic (with either gcc 7.3.0 or 8.2.0) can already catch this. I have no comment if this pedantic should be part of default DEVELOPER=1. But at least in controlled environments like Travis, we could turn it on to catch more.
Signature · 1 line
Duy
Re: [PATCH] files-backend.c: fix build error on Solaris
In reply to Carlo ArenasQuoted 1 line · Carlo Arenas
Signed-off-by Carlo Marcelo Arenas Belón <[email protected]>
Do you mean Tested-by: (meaning, you actually saw the breakage with SunCC without the patch and also saw the patch fixed the breakage)?
Quoted 4 lines
clang with -Wpedantic also catch this (at least with Apple LLVM version 10.0.0); recent versions of gcc also include that flag and at least 8.2.0 shows a warning for it, so it might be worth adding it to developer mode (maybe under the pedantic DEVOPTS).
Nice to know.
Quoted 2 lines
..., would this be something I should be adding?, what are the expectations around warnings and compilers?
It may be a worthwhile thing to discuss, and as a discussion starter, a patch would help ;-).