| Message ID | 20180922180500.4689-6-pclouds@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix per-worktree ref iteration in fsck/reflog expire | expand |
On Sat, Sep 22, 2018 at 2:05 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote: > 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... > 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. > > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > diff --git a/worktree.c b/worktree.c > @@ -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); > +} 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). > diff --git a/worktree.h b/worktree.h > @@ -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/
On Sun, Sep 23, 2018 at 10:25 AM Eric Sunshine <sunshine@sunshineco.com> wrote: > > @@ -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); > > +} > > 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). 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/
diff --git a/revision.c b/revision.c index 63aae722c1..8ce660e3b1 100644 --- a/revision.c +++ b/revision.c @@ -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) diff --git a/worktree.c b/worktree.c index b0d0b5426d..ec1a5bc511 100644 --- a/worktree.c +++ b/worktree.c @@ -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; } diff --git a/worktree.h b/worktree.h index df3fc30f73..0016eb9e88 100644 --- a/worktree.h +++ b/worktree.h @@ -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
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 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. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- revision.c | 21 +++++++++++++-------- worktree.c | 32 +++++++++++++++++++++++++++++--- worktree.h | 14 ++++++++++++++ 3 files changed, 56 insertions(+), 11 deletions(-)