| Message ID | cover.1702560829.git.ps@pks.im (mailing list archive) |
|---|---|
| Headers | show |
| Series | refs: improve handling of special refs | expand |
Patrick Steinhardt <ps@pks.im> writes: > Patrick Steinhardt (4): > wt-status: read HEAD and ORIG_HEAD via the refdb > refs: propagate errno when reading special refs fails > refs: complete list of special refs > bisect: consistently write BISECT_EXPECTED_REV via the refdb With the clear understanding that we plan to make those other than FETCH_HEAD and MERGE_HEAD in the is_special_ref().special_refs[] eventually not special at all, this round looked quite sensible to me. Let's merge it down to 'next'. Thanks.
On Wed, Dec 20, 2023 at 11:28:51AM -0800, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > Patrick Steinhardt (4): > > wt-status: read HEAD and ORIG_HEAD via the refdb > > refs: propagate errno when reading special refs fails > > refs: complete list of special refs > > bisect: consistently write BISECT_EXPECTED_REV via the refdb > > With the clear understanding that we plan to make those other than > FETCH_HEAD and MERGE_HEAD in the is_special_ref().special_refs[] > eventually not special at all, this round looked quite sensible to > me. > > Let's merge it down to 'next'. Thanks. We (myself or a fellow team member at GitLab) will work on converting the remaining not-so-special refs once this topic lands on the `master` branch. Unless of course somebody else picks it up before we do. Patrick
Hi, this is the third version of my patch series to improve handling of special refs. Changes combpared to v2: - Patch 1: We're now more careful around missing and symbolic refs. - Patch 3: Document in the commit message that the extended list of special refs is not intended to stay like this forever. Thanks for your reviews and the discussion! Patrick Patrick Steinhardt (4): wt-status: read HEAD and ORIG_HEAD via the refdb refs: propagate errno when reading special refs fails refs: complete list of special refs bisect: consistently write BISECT_EXPECTED_REV via the refdb bisect.c | 25 +++------------- builtin/bisect.c | 8 ++--- refs.c | 59 +++++++++++++++++++++++++++++++++++-- t/t1403-show-ref.sh | 10 +++++++ t/t6030-bisect-porcelain.sh | 2 +- wt-status.c | 22 +++++++++----- 6 files changed, 87 insertions(+), 39 deletions(-) Range-diff against v2: 1: 1db3eb3945 ! 1: aea9410bd9 wt-status: read HEAD and ORIG_HEAD via the refdb @@ Commit message via the refdb layer so that we'll also be compatible with alternate reference backends. - Note that we pass `RESOLVE_REF_NO_RECURSE` to `read_ref_full()`. This is - because we didn't resolve symrefs before either, and in practice none of - the refs in "rebase-merge/" would be symbolic. We thus don't want to - resolve symrefs with the new code either to retain the old behaviour. + There are some subtleties involved here: + + - We pass `RESOLVE_REF_READING` so that a missing ref will cause + `read_ref_full()` to return an error. + + - We pass `RESOLVE_REF_NO_RECURSE` so that we do not try to resolve + symrefs. The old code didn't resolve symrefs either, and we only + ever write object IDs into the refs in "rebase-merge/". + + - In the same spirit we verify that successfully-read refs are not + symbolic refs. Signed-off-by: Patrick Steinhardt <ps@pks.im> @@ wt-status.c: static char *read_line_from_git_path(const char *filename) - char *head, *orig_head, *rebase_amend, *rebase_orig_head; + struct object_id head_oid, orig_head_oid; + char *rebase_amend, *rebase_orig_head; ++ int head_flags, orig_head_flags; if ((!s->amend && !s->nowarn && !s->workdir_dirty) || !s->branch || strcmp(s->branch, "HEAD")) @@ wt-status.c: static char *read_line_from_git_path(const char *filename) - head = read_line_from_git_path("HEAD"); - orig_head = read_line_from_git_path("ORIG_HEAD"); -+ if (read_ref_full("HEAD", RESOLVE_REF_NO_RECURSE, &head_oid, NULL) || -+ read_ref_full("ORIG_HEAD", RESOLVE_REF_NO_RECURSE, &orig_head_oid, NULL)) ++ if (read_ref_full("HEAD", RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, ++ &head_oid, &head_flags) || ++ read_ref_full("ORIG_HEAD", RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, ++ &orig_head_oid, &orig_head_flags)) ++ return 0; ++ if (head_flags & REF_ISSYMREF || orig_head_flags & REF_ISSYMREF) + return 0; + rebase_amend = read_line_from_git_path("rebase-merge/amend"); 2: 24032a62e5 = 2: 455ab69177 refs: propagate errno when reading special refs fails 3: 3dd9089fd5 ! 3: 81ac092281 refs: complete list of special refs @@ Commit message We're already mostly good with regard to the first item, except for `BISECT_EXPECTED_REV` which will be addressed in a subsequent commit. - But the current list of special refs is missing a lot of refs that - really should be treated specially. Right now, we only treat - `FETCH_HEAD` and `MERGE_HEAD` specially here. + But the current list of special refs is missing some refs that really + should be treated specially. Right now, we only treat `FETCH_HEAD` and + `MERGE_HEAD` specially here. Introduce a new function `is_special_ref()` that contains all current instances of special refs to fix the reading path. + Note that this is only a temporary measure where we record and rectify + the current state. Ideally, the list of special refs should in the end + only contain `FETCH_HEAD` and `MERGE_HEAD` again because they both may + reference multiple objects and can contain annotations, so they indeed + are special. + Based-on-patch-by: Han-Wen Nienhuys <hanwenn@gmail.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> 4: 4a4447a3f5 = 4: 3244678161 bisect: consistently write BISECT_EXPECTED_REV via the refdb base-commit: 1a87c842ece327d03d08096395969aca5e0a6996