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

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

[Han-Wen Nienhuys via GitGitGadget]

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(-)

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

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

[Han-Wen Nienhuys]

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

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

[Junio C Hamano]

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.

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

[Junio C Hamano]

"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

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

[Han-Wen Nienhuys]

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.

-- 
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