threads / patch / 54151

patchrefs: move REF_LOG_ONLY to refs-internal.h

Subject: [PATCH] refs: move REF_LOG_ONLY to refs-internal.h

## tl;dr

5 messages between Aug 28, 2020 and Sep 9, 2020. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Han-Wen Nienhuys via GitGitGadget· Aug 28, 2020, 15:25 UTC · lore
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.

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(-)
Show changes to 2 files +7 −7

refs/files-backend.c, refs/refs-internal.h

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
-- 
gitgitgadget
Han-Wen Nienhuys· Sep 8, 2020, 18:17 UTC · re: Han-Wen Nienhuys via GitGitGadget · lore

Re: [PATCH] refs: move REF_LOG_ONLY to refs-internal.h

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 - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Junio C Hamano· Sep 8, 2020, 21:04 UTC · re: Han-Wen Nienhuys · lore

Re: [PATCH] refs: move REF_LOG_ONLY to refs-internal.h

Han-Wen Nienhuys <hanwen@google.com> writes:
Show 7 quoted lines
> 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.
Junio C Hamano· Sep 8, 2020, 22:50 UTC · re: Han-Wen Nienhuys via GitGitGadget · lore

Re: [PATCH] refs: move REF_LOG_ONLY to refs-internal.h

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 8 quoted lines
> 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.
Show 59 quoted lines
> 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
Han-Wen Nienhuys· Sep 9, 2020, 09:35 UTC · re: Junio C Hamano · lore

Re: [PATCH] refs: move REF_LOG_ONLY to refs-internal.h

On Wed, Sep 9, 2020 at 12:50 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 11 quoted lines
> 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.

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

← back to recent threads