| Message ID | pull.712.git.1598628333959.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 63c056736544c8273dc6dc4275c2f2857bbaee44 |
| Headers | show |
| Series | refs: move REF_LOG_ONLY to refs-internal.h | expand |
On Fri, Aug 28, 2020 at 5:25 PM Han-Wen Nienhuys via GitGitGadget <gitgitgadget@gmail.com> wrote: > REF_LOG_ONLY is used in the transaction preparation: if a symref is involved in > a transaction, the referent of the symref should be updated, and the symref > itself should only be updated in the reflog. Jun, are you waiting for me to do anything with this patch?
Han-Wen Nienhuys <hanwen@google.com> writes: > On Fri, Aug 28, 2020 at 5:25 PM Han-Wen Nienhuys via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> REF_LOG_ONLY is used in the transaction preparation: if a symref is involved in >> a transaction, the referent of the symref should be updated, and the symref >> itself should only be updated in the reflog. > > Jun, are you waiting for me to do anything with this patch? If I said somthing, perhaps I am, but since I do not remember, it is likely that the list traffic has exceeded my bandwidth and this slipped through the cracks, I think. Thanks for pinging to keep the thread refreshed. It helps.
"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Han-Wen Nienhuys <hanwen@google.com> > > REF_LOG_ONLY is used in the transaction preparation: if a symref is involved in > a transaction, the referent of the symref should be updated, and the symref > itself should only be updated in the reflog. > > Other ref backends will need to duplicate this logic too, so move it to a > central place. Hmph, I am not necessarily sure about "need to duplicate" [*1*], but I do agree with the patch text---the bit should not belong to a single "files-backend" backend. [Footnote] *1* obviously, a better alternative, if possible, would be to let the more generic layer do so without forcing the backends to duplicate. But even if such a change were possible and we decide to avoid duplication, it does not make sense to have this bit specifically defined for the files-backend and nobody else. > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > --- > refs: move REF_LOG_ONLY to refs-internal.h > > REF_LOG_ONLY is used in the transaction preparation: if a symref is > involved in a transaction, the referent of the symref should be updated, > and the symref itself should only be updated in the reflog. > > Other ref backends will need to duplicate this logic too, so move it to > a central place. > > Signed-off-by: Han-Wen Nienhuys hanwen@google.com [hanwen@google.com] > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-712%2Fhanwen%2Flog-only-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-712/hanwen/log-only-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/712 > > refs/files-backend.c | 7 ------- > refs/refs-internal.h | 7 +++++++ > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/refs/files-backend.c b/refs/files-backend.c > index 985631f33e..b1946dc583 100644 > --- a/refs/files-backend.c > +++ b/refs/files-backend.c > @@ -38,13 +38,6 @@ > */ > #define REF_NEEDS_COMMIT (1 << 6) > > -/* > - * Used as a flag in ref_update::flags when we want to log a ref > - * update but not actually perform it. This is used when a symbolic > - * ref update is split up. > - */ > -#define REF_LOG_ONLY (1 << 7) > - > /* > * Used as a flag in ref_update::flags when the ref_update was via an > * update to HEAD. > diff --git a/refs/refs-internal.h b/refs/refs-internal.h > index 357359a0be..1f92861aeb 100644 > --- a/refs/refs-internal.h > +++ b/refs/refs-internal.h > @@ -31,6 +31,13 @@ struct ref_transaction; > */ > #define REF_HAVE_OLD (1 << 3) > > +/* > + * Used as a flag in ref_update::flags when we want to log a ref > + * update but not actually perform it. This is used when a symbolic > + * ref update is split up. > + */ > +#define REF_LOG_ONLY (1 << 7) > + > /* > * Return the length of time to retry acquiring a loose reference lock > * before giving up, in milliseconds: > > base-commit: 675a4aaf3b226c0089108221b96559e0baae5de9
On Wed, Sep 9, 2020 at 12:50 AM Junio C Hamano <gitster@pobox.com> wrote: > Hmph, I am not necessarily sure about "need to duplicate" [*1*], but > I do agree with the patch text---the bit should not belong to a > single "files-backend" backend. > > [Footnote] > > *1* obviously, a better alternative, if possible, would be to let > the more generic layer do so without forcing the backends to > duplicate. But even if such a change were possible and we > decide to avoid duplication, it does not make sense to have this > bit specifically defined for the files-backend and nobody else. I'm not sure if it is possible. REFS_LOG_ONLY is used in the split_head_update() which is called from the middle of the files_transaction_prepare(). In particular, it can happen after a number of locks have already been taken. If this moves into the generic layer, the head split happens before the locks are taken, which could alter behavior.
diff --git a/refs/files-backend.c b/refs/files-backend.c index 985631f33e..b1946dc583 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -38,13 +38,6 @@ */ #define REF_NEEDS_COMMIT (1 << 6) -/* - * Used as a flag in ref_update::flags when we want to log a ref - * update but not actually perform it. This is used when a symbolic - * ref update is split up. - */ -#define REF_LOG_ONLY (1 << 7) - /* * Used as a flag in ref_update::flags when the ref_update was via an * update to HEAD. diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 357359a0be..1f92861aeb 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -31,6 +31,13 @@ struct ref_transaction; */ #define REF_HAVE_OLD (1 << 3) +/* + * Used as a flag in ref_update::flags when we want to log a ref + * update but not actually perform it. This is used when a symbolic + * ref update is split up. + */ +#define REF_LOG_ONLY (1 << 7) + /* * Return the length of time to retry acquiring a loose reference lock * before giving up, in milliseconds: