{"thread":{"id":"53801","subject":"[PATCH 0/2] Remove special casing for PSEUDOREF updates","startedAt":"2020-07-06T17:29:38Z","lastAt":"2020-08-19T15:53:00Z","messageCount":39,"participants":["Han-Wen Nienhuys via GitGitGadget","Junio C Hamano","Han-Wen Nienhuys","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"401044","messageId":"pull.673.git.1594056572.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":null,"subject":"[PATCH 0/2] Remove special casing for PSEUDOREF updates","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-06T17:29:30Z","receivedAt":"2020-07-06T17:29:38Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"This gets rid of the special casing code for pseudorefs in refs.c\n\nThis is in preparation for reftable.\n\nHan-Wen Nienhuys (2):\n Modify pseudo refs through ref backend storage\n Make HEAD a PSEUDOREF rather than PER_WORKTREE.\n\n refs.c | 127 ++++--------------------------------------\n refs/files-backend.c | 11 ++--\n t/t1400-update-ref.sh | 6 +-\n 3 files changed, 20 insertions(+), 124 deletions(-)\n\n\nbase-commit: a08a83db2bf27f015bec9a435f6d73e223c21c5e\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-673%2Fhanwen%2Fpseudoref-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-673/hanwen/pseudoref-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/673\n-- \ngitgitgadget\n"},{"id":"401045","messageId":"6821f57bdf326f161f152a8af0e47b54513c77b1.1594056572.git.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.git.1594056572.gitgitgadget@gmail.com","subject":"[PATCH 1/2] Modify pseudo refs through ref backend storage","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-06T17:29:31Z","receivedAt":"2020-07-06T17:29:38Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe previous behavior was introduced in commit 74ec19d4be\n(\"pseudorefs: create and use pseudoref update and delete functions\",\nJul 31, 2015), with the justification \"alternate ref backends still\nneed to store pseudorefs in GIT_DIR\".\n\nRefs such as REBASE_HEAD are read through the ref backend. This can\nonly work consistently if they are written through the ref backend as\nwell. Tooling that works directly on files under .git should be\nupdated to use git commands to read refs instead.\n\nThis needs the following fixes\n\n* Only write log for HEAD pseudo ref. Fixes t1400\n 'core.logAllRefUpdates=always creates no reflog for ORIG_HEAD'\n\n* t1400: change hard coded error messages to check for.\n\n* don't deref non-HEAD pseudoref symrefs. This fixes t1405. Without\n this, a deleting a FOO symref pointing to refs/heads/master will remove\n master too.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n refs.c | 120 +++---------------------------------------\n refs/files-backend.c | 11 ++--\n t/t1400-update-ref.sh | 6 +--\n 3 files changed, 17 insertions(+), 120 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 224ff66c7b..a2fd42364f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -739,102 +739,6 @@ long get_files_ref_lock_timeout_ms(void)\n \treturn timeout_ms;\n }\n \n-static int write_pseudoref(const char *pseudoref, const struct object_id *oid,\n-\t\t\t const struct object_id *old_oid, struct strbuf *err)\n-{\n-\tconst char *filename;\n-\tint fd;\n-\tstruct lock_file lock = LOCK_INIT;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tint ret = -1;\n-\n-\tif (!oid)\n-\t\treturn 0;\n-\n-\tstrbuf_addf(&buf, \"%s\\n\", oid_to_hex(oid));\n-\n-\tfilename = git_path(\"%s\", pseudoref);\n-\tfd = hold_lock_file_for_update_timeout(&lock, filename, 0,\n-\t\t\t\t\t get_files_ref_lock_timeout_ms());\n-\tif (fd < 0) {\n-\t\tstrbuf_addf(err, _(\"could not open '%s' for writing: %s\"),\n-\t\t\t filename, strerror(errno));\n-\t\tgoto done;\n-\t}\n-\n-\tif (old_oid) {\n-\t\tstruct object_id actual_old_oid;\n-\n-\t\tif (read_ref(pseudoref, &actual_old_oid)) {\n-\t\t\tif (!is_null_oid(old_oid)) {\n-\t\t\t\tstrbuf_addf(err, _(\"could not read ref '%s'\"),\n-\t\t\t\t\t pseudoref);\n-\t\t\t\trollback_lock_file(&lock);\n-\t\t\t\tgoto done;\n-\t\t\t}\n-\t\t} else if (is_null_oid(old_oid)) {\n-\t\t\tstrbuf_addf(err, _(\"ref '%s' already exists\"),\n-\t\t\t\t pseudoref);\n-\t\t\trollback_lock_file(&lock);\n-\t\t\tgoto done;\n-\t\t} else if (!oideq(&actual_old_oid, old_oid)) {\n-\t\t\tstrbuf_addf(err, _(\"unexpected object ID when writing '%s'\"),\n-\t\t\t\t pseudoref);\n-\t\t\trollback_lock_file(&lock);\n-\t\t\tgoto done;\n-\t\t}\n-\t}\n-\n-\tif (write_in_full(fd, buf.buf, buf.len) < 0) {\n-\t\tstrbuf_addf(err, _(\"could not write to '%s'\"), filename);\n-\t\trollback_lock_file(&lock);\n-\t\tgoto done;\n-\t}\n-\n-\tcommit_lock_file(&lock);\n-\tret = 0;\n-done:\n-\tstrbuf_release(&buf);\n-\treturn ret;\n-}\n-\n-static int delete_pseudoref(const char *pseudoref, const struct object_id *old_oid)\n-{\n-\tconst char *filename;\n-\n-\tfilename = git_path(\"%s\", pseudoref);\n-\n-\tif (old_oid && !is_null_oid(old_oid)) {\n-\t\tstruct lock_file lock = LOCK_INIT;\n-\t\tint fd;\n-\t\tstruct object_id actual_old_oid;\n-\n-\t\tfd = hold_lock_file_for_update_timeout(\n-\t\t\t\t&lock, filename, 0,\n-\t\t\t\tget_files_ref_lock_timeout_ms());\n-\t\tif (fd < 0) {\n-\t\t\terror_errno(_(\"could not open '%s' for writing\"),\n-\t\t\t\t filename);\n-\t\t\treturn -1;\n-\t\t}\n-\t\tif (read_ref(pseudoref, &actual_old_oid))\n-\t\t\tdie(_(\"could not read ref '%s'\"), pseudoref);\n-\t\tif (!oideq(&actual_old_oid, old_oid)) {\n-\t\t\terror(_(\"unexpected object ID when deleting '%s'\"),\n-\t\t\t pseudoref);\n-\t\t\trollback_lock_file(&lock);\n-\t\t\treturn -1;\n-\t\t}\n-\n-\t\tunlink(filename);\n-\t\trollback_lock_file(&lock);\n-\t} else {\n-\t\tunlink(filename);\n-\t}\n-\n-\treturn 0;\n-}\n-\n int refs_delete_ref(struct ref_store *refs, const char *msg,\n \t\t const char *refname,\n \t\t const struct object_id *old_oid,\n@@ -843,11 +747,6 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n-\tif (ref_type(refname) == REF_TYPE_PSEUDOREF) {\n-\t\tassert(refs == get_main_ref_store(the_repository));\n-\t\treturn delete_pseudoref(refname, old_oid);\n-\t}\n-\n \ttransaction = ref_store_transaction_begin(refs, &err);\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, old_oid,\n@@ -1170,18 +1069,13 @@ int refs_update_ref(struct ref_store *refs, const char *msg,\n \tstruct strbuf err = STRBUF_INIT;\n \tint ret = 0;\n \n-\tif (ref_type(refname) == REF_TYPE_PSEUDOREF) {\n-\t\tassert(refs == get_main_ref_store(the_repository));\n-\t\tret = write_pseudoref(refname, new_oid, old_oid, &err);\n-\t} else {\n-\t\tt = ref_store_transaction_begin(refs, &err);\n-\t\tif (!t ||\n-\t\t ref_transaction_update(t, refname, new_oid, old_oid,\n-\t\t\t\t\t flags, msg, &err) ||\n-\t\t ref_transaction_commit(t, &err)) {\n-\t\t\tret = 1;\n-\t\t\tref_transaction_free(t);\n-\t\t}\n+\tt = ref_store_transaction_begin(refs, &err);\n+\tif (!t ||\n+\t ref_transaction_update(t, refname, new_oid, old_oid, flags, msg,\n+\t\t\t\t &err) ||\n+\t ref_transaction_commit(t, &err)) {\n+\t\tret = 1;\n+\t\tref_transaction_free(t);\n \t}\n \tif (ret) {\n \t\tconst char *str = _(\"update_ref failed for ref '%s': %s\");\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 6516c7bc8c..9951c2e403 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1228,7 +1228,6 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,\n \n \tfor (i = 0; i < refnames->nr; i++) {\n \t\tconst char *refname = refnames->items[i].string;\n-\n \t\tif (refs_delete_ref(&refs->base, msg, refname, NULL, flags))\n \t\t\tresult |= error(_(\"could not remove reference %s\"), refname);\n \t}\n@@ -2436,7 +2435,9 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n \tupdate->backend_data = lock;\n \n \tif (update->type & REF_ISSYMREF) {\n-\t\tif (update->flags & REF_NO_DEREF) {\n+\t\tif (update->flags & REF_NO_DEREF ||\n+\t\t (ref_type(update->refname) == REF_TYPE_PSEUDOREF &&\n+\t\t strcmp(update->refname, \"HEAD\"))) {\n \t\t\t/*\n \t\t\t * We won't be reading the referent as part of\n \t\t\t * the transaction, so we have to read it here\n@@ -2782,8 +2783,10 @@ static int files_transaction_finish(struct ref_store *ref_store,\n \t\tstruct ref_update *update = transaction->updates[i];\n \t\tstruct ref_lock *lock = update->backend_data;\n \n-\t\tif (update->flags & REF_NEEDS_COMMIT ||\n-\t\t update->flags & REF_LOG_ONLY) {\n+\t\tif ((ref_type(lock->ref_name) != REF_TYPE_PSEUDOREF ||\n+\t\t !strcmp(lock->ref_name, \"HEAD\")) &&\n+\t\t (update->flags & REF_NEEDS_COMMIT ||\n+\t\t update->flags & REF_LOG_ONLY)) {\n \t\t\tif (files_log_ref_write(refs,\n \t\t\t\t\t\tlock->ref_name,\n \t\t\t\t\t\t&lock->old_oid,\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 27171f8261..6b8030e8fe 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -476,7 +476,7 @@ test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER\n test_expect_success 'given old value for missing pseudoref, do not create' '\n \ttest_must_fail git update-ref PSEUDOREF $A $B 2>err &&\n \ttest_path_is_missing .git/PSEUDOREF &&\n-\ttest_i18ngrep \"could not read ref\" err\n+\ttest_i18ngrep \"unable to resolve reference\" err\n '\n \n test_expect_success 'create pseudoref' '\n@@ -497,7 +497,7 @@ test_expect_success 'overwrite pseudoref with correct old value' '\n test_expect_success 'do not overwrite pseudoref with wrong old value' '\n \ttest_must_fail git update-ref PSEUDOREF $D $E 2>err &&\n \ttest $C = $(cat .git/PSEUDOREF) &&\n-\ttest_i18ngrep \"unexpected object ID\" err\n+\ttest_i18ngrep \"cannot lock ref.*expected\" err\n '\n \n test_expect_success 'delete pseudoref' '\n@@ -509,7 +509,7 @@ test_expect_success 'do not delete pseudoref with wrong old value' '\n \tgit update-ref PSEUDOREF $A &&\n \ttest_must_fail git update-ref -d PSEUDOREF $B 2>err &&\n \ttest $A = $(cat .git/PSEUDOREF) &&\n-\ttest_i18ngrep \"unexpected object ID\" err\n+\ttest_i18ngrep \"cannot lock ref.*expected\" err\n '\n \n test_expect_success 'delete pseudoref with correct old value' '\n-- \ngitgitgadget\n\n"},{"id":"401046","messageId":"470821dc6d4b72c6414b3458883277ad36952892.1594056572.git.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.git.1594056572.gitgitgadget@gmail.com","subject":"[PATCH 2/2] Make HEAD a PSEUDOREF rather than PER_WORKTREE.","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-06T17:29:32Z","receivedAt":"2020-07-06T17:29:39Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThis is consistent with the definition of REF_TYPE_PSEUDOREF\n(uppercase in the root ref namespace).\n\nSigned-off-by: Han-Wen Nienhuys \n---\n refs.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex a2fd42364f..265767a234 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -676,10 +676,9 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)\n \n static int is_per_worktree_ref(const char *refname)\n {\n-\treturn !strcmp(refname, \"HEAD\") ||\n-\t\tstarts_with(refname, \"refs/worktree/\") ||\n-\t\tstarts_with(refname, \"refs/bisect/\") ||\n-\t\tstarts_with(refname, \"refs/rewritten/\");\n+\treturn starts_with(refname, \"refs/worktree/\") ||\n+\t starts_with(refname, \"refs/bisect/\") ||\n+\t starts_with(refname, \"refs/rewritten/\");\n }\n \n static int is_pseudoref_syntax(const char *refname)\n-- \ngitgitgadget\n"},{"id":"401050","messageId":"xmqqimf09ypc.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"6821f57bdf326f161f152a8af0e47b54513c77b1.1594056572.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] Modify pseudo refs through ref backend storage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-06T20:26:55Z","receivedAt":"2020-07-06T20:27:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" writes:\n\n> Subject: Re: [PATCH 1/2] Modify pseudo refs through ref backend storage\n> From: Han-Wen Nienhuys \n\nWith what definition of \"pseudo refs\" has this change been made?\nThose things like HEAD, CHERRY_PICK_HEAD, FETCH_HEAD etc. that have\ntraditionally been written as a plain text file in $GIT_DIR and are\nused to name objects by having a full object name in it? \n\nOr the entities that behave like refs and stored in ref backends,\nwith all-uppercase-names but do not sit inside refs/ hierarchy?\n\nI think it is OK (and possibly a good move in the longer term, but\nthat is just my gut feeling) to make ref backends resopnsible for\nenumerating, reading and writing them (i.e. I think we want to use\nthe latter definition for the longer term health of the project).\nAnd we would want to ...\n\n> The previous behavior was introduced in commit 74ec19d4be\n> (\"pseudorefs: create and use pseudoref update and delete functions\",\n> Jul 31, 2015), with the justification \"alternate ref backends still\n> need to store pseudorefs in GIT_DIR\".\n\n... declare that justification invalid for that purpose.\n\nIs that what is going on? I just want to make sure I am following\nyour flow of thought.\n\n> Refs such as REBASE_HEAD are read through the ref backend. This can\n> only work consistently if they are written through the ref backend as\n> well. Tooling that works directly on files under .git should be\n> updated to use git commands to read refs instead.\n\nOK.\n\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 6516c7bc8c..9951c2e403 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -1228,7 +1228,6 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,\n> \n> \tfor (i = 0; i < refnames->nr; i++) {\n> \t\tconst char *refname = refnames->items[i].string;\n> -\n> \t\tif (refs_delete_ref(&refs->base, msg, refname, NULL, flags))\n> \t\t\tresult |= error(_(\"could not remove reference %s\"), refname);\n> \t}\n\nUnreleated change?\n\n> @@ -2436,7 +2435,9 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n> \tupdate->backend_data = lock;\n> \n> \tif (update->type & REF_ISSYMREF) {\n> -\t\tif (update->flags & REF_NO_DEREF) {\n> +\t\tif (update->flags & REF_NO_DEREF ||\n> +\t\t (ref_type(update->refname) == REF_TYPE_PSEUDOREF &&\n> +\t\t strcmp(update->refname, \"HEAD\"))) {\n> \t\t\t/*\n> \t\t\t * We won't be reading the referent as part of\n> \t\t\t * the transaction, so we have to read it here\n\nThe old \"if we are not dereferencing\" condition in if() exactly\nmatched the comment, but the condition in if() after this change is\nnot \"if we are not dereferencing\". Even if we are dereferencing,\nunder some new condition, we would still drop into this block and do\nnot follow the \"else\" side that creates a new update for the\nreferent. Is this part of \"modify pseudo refs via backend\" topic,\nor should it be a separate modification? Why is this change needed?\n\nIt seems that no matter where in the refs/ hierarchy (or even\noutside) a symbolic ref resides, the way to update itself (not the\nreferent through the symbolic ref) should be the same, which is what\nthe original says, and we want to change that reasoning here, but it\nis not quite clear to me why.\n\n> @@ -2782,8 +2783,10 @@ static int files_transaction_finish(struct ref_store *ref_store,\n> \t\tstruct ref_update *update = transaction->updates[i];\n> \t\tstruct ref_lock *lock = update->backend_data;\n> \n> -\t\tif (update->flags & REF_NEEDS_COMMIT ||\n> -\t\t update->flags & REF_LOG_ONLY) {\n> +\t\tif ((ref_type(lock->ref_name) != REF_TYPE_PSEUDOREF ||\n> +\t\t !strcmp(lock->ref_name, \"HEAD\")) &&\n> +\t\t (update->flags & REF_NEEDS_COMMIT ||\n> +\t\t update->flags & REF_LOG_ONLY)) {\n\nAnd this one stops the files backend from touching all pseudorefs\nother than HEAD with this codepath. That somehow feels totally\nopposite from what the log message explained above---weren't we\nupdating the code to write these pseudorefs through the individual\nbackends, which the files backend is one example of? Isn't this\nchange stopping the backend from writing the pseudorefs other than\nHEAD instead?\n\nPuzzled.\n\n> \t\t\tif (files_log_ref_write(refs,\n> \t\t\t\t\t\tlock->ref_name,\n> \t\t\t\t\t\t&lock->old_oid,\n\n\n> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n> index 27171f8261..6b8030e8fe 100755\n> --- a/t/t1400-update-ref.sh\n> +++ b/t/t1400-update-ref.sh\n> @@ -476,7 +476,7 @@ test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER\n> test_expect_success 'given old value for missing pseudoref, do not create' '\n> \ttest_must_fail git update-ref PSEUDOREF $A $B 2>err &&\n> \ttest_path_is_missing .git/PSEUDOREF &&\n\nThe reason why I asked what this patch thinks the definition of\npseudoref is is because of this thing. Shouldn't this line be fixed\nnot to depend on the files backend? Likewise for $(cat .git/PSEUDOREF)\nin the remaining two hunks.\n\n> -\ttest_i18ngrep \"could not read ref\" err\n> +\ttest_i18ngrep \"unable to resolve reference\" err\n> '\n> \n> test_expect_success 'create pseudoref' '\n> @@ -497,7 +497,7 @@ test_expect_success 'overwrite pseudoref with correct old value' '\n> test_expect_success 'do not overwrite pseudoref with wrong old value' '\n> \ttest_must_fail git update-ref PSEUDOREF $D $E 2>err &&\n> \ttest $C = $(cat .git/PSEUDOREF) &&\n> -\ttest_i18ngrep \"unexpected object ID\" err\n> +\ttest_i18ngrep \"cannot lock ref.*expected\" err\n> '\n> \n> test_expect_success 'delete pseudoref' '\n> @@ -509,7 +509,7 @@ test_expect_success 'do not delete pseudoref with wrong old value' '\n> \tgit update-ref PSEUDOREF $A &&\n> \ttest_must_fail git update-ref -d PSEUDOREF $B 2>err &&\n> \ttest $A = $(cat .git/PSEUDOREF) &&\n> -\ttest_i18ngrep \"unexpected object ID\" err\n> +\ttest_i18ngrep \"cannot lock ref.*expected\" err\n> '\n> \n> test_expect_success 'delete pseudoref with correct old value' '\n"},{"id":"401051","messageId":"xmqqeepo9yjb.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"470821dc6d4b72c6414b3458883277ad36952892.1594056572.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] Make HEAD a PSEUDOREF rather than PER_WORKTREE.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-06T20:30:32Z","receivedAt":"2020-07-06T20:30:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" writes:\n\n> From: Han-Wen Nienhuys \n>\n> This is consistent with the definition of REF_TYPE_PSEUDOREF\n> (uppercase in the root ref namespace).\n\nI wonder if some special casing we saw for \"HEAD\" in 1/2 (e.g. we no\nlonger do this to pseudorefs in this codepath but HEAD is different\nand will continue to be dealt with in the codepath) needs adjustment\nafter this change? HEAD still needs to be able to have a different\nvalue (either detached or pointing to another ref) per worktree, but\nit is unclear where that knowledge now resides in the new code. Are\nwe declaring that all pseudorefs are per worktree (which I think is\nnot wrong)?\n\n> Signed-off-by: Han-Wen Nienhuys \n> ---\n> refs.c | 7 +++----\n> 1 file changed, 3 insertions(+), 4 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index a2fd42364f..265767a234 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -676,10 +676,9 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)\n> \n> static int is_per_worktree_ref(const char *refname)\n> {\n> -\treturn !strcmp(refname, \"HEAD\") ||\n> -\t\tstarts_with(refname, \"refs/worktree/\") ||\n> -\t\tstarts_with(refname, \"refs/bisect/\") ||\n> -\t\tstarts_with(refname, \"refs/rewritten/\");\n> +\treturn starts_with(refname, \"refs/worktree/\") ||\n> +\t starts_with(refname, \"refs/bisect/\") ||\n> +\t starts_with(refname, \"refs/rewritten/\");\n> }\n> \n> static int is_pseudoref_syntax(const char *refname)\n"},{"id":"401086","messageId":"CAFQ2z_Nvieyfy4AWz8=sRjsbuXgv1HLVosH78p4Axj5vce5yNQ@mail.gmail.com","threadId":"53801","inReplyTo":"xmqqeepo9yjb.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/2] Make HEAD a PSEUDOREF rather than PER_WORKTREE.","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2020-07-07T09:24:08Z","receivedAt":"2020-07-07T09:24:23Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Mon, Jul 6, 2020 at 10:30 PM Junio C Hamano wrote:\n> > This is consistent with the definition of REF_TYPE_PSEUDOREF\n> > (uppercase in the root ref namespace).\n>\n> I wonder if some special casing we saw for \"HEAD\" in 1/2 (e.g. we no\n> longer do this to pseudorefs in this codepath but HEAD is different\n> and will continue to be dealt with in the codepath) needs adjustment\n> after this change? HEAD still needs to be able to have a different\n> value (either detached or pointing to another ref) per worktree, but\n> it is unclear where that knowledge now resides in the new code. Are\n> we declaring that all pseudorefs are per worktree (which I think is\n> not wrong)?\n\nthe source code already says\n\nenum ref_type {\n REF_TYPE_PER_WORKTREE, /* refs inside refs/ but not shared */\n REF_TYPE_PSEUDOREF, /* refs outside refs/ in current worktree */\n\nbut considers HEAD a REF_TYPE_PER_WORKTREE, counter to the comment.\n\nIIRC, this change actually has almost no effect, because most code\ntreats both enum values as equivalent. (see eg. builtin/reflog.c),\nexcept for the code that the preceding commit changes.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\nRegistergericht und -nummer: Hamburg, HRB 86891\nSitz der Gesellschaft: Hamburg\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"401097","messageId":"CAFQ2z_OPbcX9FGsFnmraAUpTu2eKuf2PZ+DO-CTWvaBZD6WQQQ@mail.gmail.com","threadId":"53801","inReplyTo":"xmqqimf09ypc.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 1/2] Modify pseudo refs through ref backend storage","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2020-07-07T13:56:38Z","receivedAt":"2020-07-07T13:56:53Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Mon, Jul 6, 2020 at 10:27 PM Junio C Hamano wrote:\n>\n> \"Han-Wen Nienhuys via GitGitGadget\" writes:\n>\n> > Subject: Re: [PATCH 1/2] Modify pseudo refs through ref backend storage\n> > From: Han-Wen Nienhuys \n>\n> With what definition of \"pseudo refs\" has this change been made?\n> Those things like HEAD, CHERRY_PICK_HEAD, FETCH_HEAD etc. that have\n> traditionally been written as a plain text file in $GIT_DIR and are\n> used to name objects by having a full object name in it?\n>\n> Or the entities that behave like refs and stored in ref backends,\n> with all-uppercase-names but do not sit inside refs/ hierarchy?\n\nI thought these were the same? - see below :)\n\n> I think it is OK (and possibly a good move in the longer term, but\n> that is just my gut feeling) to make ref backends resopnsible for\n> enumerating, reading and writing them (i.e. I think we want to use\n> the latter definition for the longer term health of the project).\n> And we would want to ...\n>\n> > The previous behavior was introduced in commit 74ec19d4be\n> > (\"pseudorefs: create and use pseudoref update and delete functions\",\n> > Jul 31, 2015), with the justification \"alternate ref backends still\n> > need to store pseudorefs in GIT_DIR\".\n>\n> ... declare that justification invalid for that purpose.\n>\n> Is that what is going on? I just want to make sure I am following\n> your flow of thought.\n\nyep.\n\n> > const char *refname = refnames->items[i].string;\n> > -\n> > if (refs_delete_ref(&refs->base, msg, refname, NULL, flags))\n> > result |= error(_(\"could not remove reference %s\"), refname);\n> > }\n>\n> Unreleated change?\n\noops.\n\n> > @@ -2436,7 +2435,9 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n> > update->backend_data = lock;\n> >\n> > if (update->type & REF_ISSYMREF) {\n> > - if (update->flags & REF_NO_DEREF) {\n> > + if (update->flags & REF_NO_DEREF ||\n> > + (ref_type(update->refname) == REF_TYPE_PSEUDOREF &&\n> > + strcmp(update->refname, \"HEAD\"))) {\n> > /*\n> > * We won't be reading the referent as part of\n> > * the transaction, so we have to read it here\n>\n> The old \"if we are not dereferencing\" condition in if() exactly\n> matched the comment, but the condition in if() after this change is\n> not \"if we are not dereferencing\". Even if we are dereferencing,\n> under some new condition, we would still drop into this block and do\n> not follow the \"else\" side that creates a new update for the\n> referent. Is this part of \"modify pseudo refs via backend\" topic,\n> or should it be a separate modification? Why is this change needed?\n\nThere is a test (mentioned in the commit message), t1405, which does\n\n$RUN create-symref FOO refs/heads/master nothing &&\n# calls refs_create_symref()\n..\n$RUN delete-refs 0 nothing FOO refs/tags/new-tag &&\n# calls refs_delete_refs, which calls delete_pseudoref\n..\ngit rev-parse master >expected &&\n\nPreviously, the delete_pseudoref code would delete just .git/FOO file.\nWithout the change here, going through the ref backend ends up in the\n!REF_NO_DEREF case, deleting the master branch.\n\nI don't know what the correct semantics are (are symrefs used for\nanything except HEAD ?), so\n\n\n> It seems that no matter where in the refs/ hierarchy (or even\n> outside) a symbolic ref resides, the way to update itself (not the\n> referent through the symbolic ref) should be the same, which is what\n> the original says, and we want to change that reasoning here, but it\n> is not quite clear to me why.\n>\n> > @@ -2782,8 +2783,10 @@ static int files_transaction_finish(struct ref_store *ref_store,\n> > struct ref_update *update = transaction->updates[i];\n> > struct ref_lock *lock = update->backend_data;\n> >\n> > - if (update->flags & REF_NEEDS_COMMIT ||\n> > - update->flags & REF_LOG_ONLY) {\n> > + if ((ref_type(lock->ref_name) != REF_TYPE_PSEUDOREF ||\n> > + !strcmp(lock->ref_name, \"HEAD\")) &&\n> > + (update->flags & REF_NEEDS_COMMIT ||\n> > + update->flags & REF_LOG_ONLY)) {\n>\n> And this one stops the files backend from touching all pseudorefs\n> other than HEAD with this codepath. That somehow feels totally\n> opposite from what the log message explained above---weren't we\n> updating the code to write these pseudorefs through the individual\n> backends, which the files backend is one example of? Isn't this\n> change stopping the backend from writing the pseudorefs other than\n> HEAD instead?\n\nThere is a test\n\n 'core.logAllRefUpdates=always creates no reflog for ORIG_HEAD'\n\nthat wants there to not be reflog updates for ORIG_HEAD. I don't see\nwhy that behavior exists, and would be happy to change it, but I'm too\nmuch of a newbie to decide what is right here.\n\n> > --- a/t/t1400-update-ref.sh\n> > +++ b/t/t1400-update-ref.sh\n> > @@ -476,7 +476,7 @@ test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER\n> > test_expect_success 'given old value for missing pseudoref, do not create' '\n> > test_must_fail git update-ref PSEUDOREF $A $B 2>err &&\n> > test_path_is_missing .git/PSEUDOREF &&\n>\n> The reason why I asked what this patch thinks the definition of\n> pseudoref is is because of this thing. Shouldn't this line be fixed\n> not to depend on the files backend? Likewise for $(cat .git/PSEUDOREF)\n> in the remaining two hunks.\n\nThis patch doesn't introduce reftable yet, so both definitions are\nequivalent for the sake of this patch.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\nRegistergericht und -nummer: Hamburg, HRB 86891\nSitz der Gesellschaft: Hamburg\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"401128","messageId":"xmqqv9iz5p2v.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"CAFQ2z_OPbcX9FGsFnmraAUpTu2eKuf2PZ+DO-CTWvaBZD6WQQQ@mail.gmail.com","subject":"Re: [PATCH 1/2] Modify pseudo refs through ref backend storage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-07T15:20:40Z","receivedAt":"2020-07-07T15:33:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n>> Or the entities that behave like refs and stored in ref backends,\n>> with all-uppercase-names but do not sit inside refs/ hierarchy?\n>\n> I thought these were the same? - see below :)\n\n>> > --- a/t/t1400-update-ref.sh\n>> > +++ b/t/t1400-update-ref.sh\n>> > @@ -476,7 +476,7 @@ test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER\n>> > test_expect_success 'given old value for missing pseudoref, do not create' '\n>> > test_must_fail git update-ref PSEUDOREF $A $B 2>err &&\n>> > test_path_is_missing .git/PSEUDOREF &&\n>>\n>> The reason why I asked what this patch thinks the definition of\n>> pseudoref is is because of this thing. Shouldn't this line be fixed\n>> not to depend on the files backend? Likewise for $(cat .git/PSEUDOREF)\n>> in the remaining two hunks.\n>\n> This patch doesn't introduce reftable yet, so both definitions are\n> equivalent for the sake of this patch.\n\nBut at the end of the tunnel, we do want to see people stop\nexpecting that .git/PSEUDOREF file is what PSEUDOREF ref is, no? I\nthink that is what \"pseudo refs are not just files directly under\n$GIT_DIR, but managed by the backend\" means, and that in turn is a\nvalid justification for other changes introduced in this patch.\nExpecting the effect of modifying pseudo refs _after_ the code is\nchanged that such modifications are properly done through the ref\nAPI to appear on the filesystem feels like it goes against the\nreason why we are making this change, compared to checking to see if\nthe pseudoref really got updated as desired via the ref API, at\nleast to me.\n"},{"id":"401135","messageId":"CAFQ2z_P37j32O71Z+NxNNU7-xLfYf-Rz-MvbYakcpaysiep9Zw@mail.gmail.com","threadId":"53801","inReplyTo":"xmqqv9iz5p2v.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 1/2] Modify pseudo refs through ref backend storage","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2020-07-07T17:15:32Z","receivedAt":"2020-07-07T17:15:46Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Tue, Jul 7, 2020 at 5:20 PM Junio C Hamano wrote:\n\n> >> The reason why I asked what this patch thinks the definition of\n> >> pseudoref is is because of this thing. Shouldn't this line be fixed\n> >> not to depend on the files backend? Likewise for $(cat .git/PSEUDOREF)\n> >> in the remaining two hunks.\n> >\n> > This patch doesn't introduce reftable yet, so both definitions are\n> > equivalent for the sake of this patch.\n>\n> But at the end of the tunnel, we do want to see people stop\n> expecting that .git/PSEUDOREF file is what PSEUDOREF ref is, no? I\n> think that is what \"pseudo refs are not just files directly under\n> $GIT_DIR, but managed by the backend\" means, and that in turn is a\n> valid justification for other changes introduced in this patch.\n> Expecting the effect of modifying pseudo refs _after_ the code is\n> changed that such modifications are properly done through the ref\n> API to appear on the filesystem feels like it goes against the\n> reason why we are making this change, compared to checking to see if\n> the pseudoref really got updated as desired via the ref API, at\n> least to me.\n\nI can fix this specific instance here (which is what I think you\nwant), for this commit to practice what it preaches. At the same time\nthere are probably about 100 or so other places where the tests check\nthe file system directly for ref(log) existence, so it would never be\ntotally consistent.\n\nThe only way to systematically find the offending places is to\nintroduce a new ref backend and then fix all the tests, and I think\nthat goes outside the scope of this small series.\n\n--\nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"401144","messageId":"xmqq5zaz5h0y.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"CAFQ2z_P37j32O71Z+NxNNU7-xLfYf-Rz-MvbYakcpaysiep9Zw@mail.gmail.com","subject":"Re: [PATCH 1/2] Modify pseudo refs through ref backend storage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-07T18:14:37Z","receivedAt":"2020-07-07T18:14:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n> I can fix this specific instance here (which is what I think you\n> want), for this commit to practice what it preaches. At the same time\n> there are probably about 100 or so other places where the tests check\n> the file system directly for ref(log) existence, so it would never be\n> totally consistent.\n>\n> The only way to systematically find the offending places is to\n> introduce a new ref backend and then fix all the tests, and I think\n> that goes outside the scope of this small series.\n\nGood. I think we are on the same page. I suggested to make the\npatch internally consistent, nothing more. And fixing everything in\nthe world is outside the scope of these two patches.\n\nThanks.\n"},{"id":"401266","messageId":"pull.673.v2.git.1594329108.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.git.1594056572.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] Remove special casing for PSEUDOREF updates","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-09T21:11:45Z","receivedAt":"2020-07-09T21:11:53Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"This gets rid of the special casing code for pseudorefs in refs.c\n\nThis is in preparation for reftable.\n\nv2\n\n * remove special casing of non-HEAD pseudorefs; update t1400 and t1405\n accordingly\n * open question: should git-update-ref.txt be updated, when it talks about\n logAllRefUpdates?\n\nHan-Wen Nienhuys (3):\n t1400: use git rev-parse for testing PSEUDOREF existence\n Modify pseudo refs through ref backend storage\n Make HEAD a PSEUDOREF rather than PER_WORKTREE.\n\n refs.c | 127 +++-----------------------------------\n t/t1400-update-ref.sh | 30 ++++-----\n t/t1405-main-ref-store.sh | 5 +-\n 3 files changed, 29 insertions(+), 133 deletions(-)\n\n\nbase-commit: 4a0fcf9f760c9774be77f51e1e88a7499b53d2e2\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-673%2Fhanwen%2Fpseudoref-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-673/hanwen/pseudoref-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/673\n\nRange-diff vs v1:\n\n -: ---------- > 1: 9c3dc4b2cb t1400: use git rev-parse for testing PSEUDOREF existence\n 1: 6821f57bdf ! 2: 871b411517 Modify pseudo refs through ref backend storage\n @@ Commit message\n well. Tooling that works directly on files under .git should be\n updated to use git commands to read refs instead.\n \n - This needs the following fixes\n + The following behaviors change:\n \n - * Only write log for HEAD pseudo ref. Fixes t1400\n - 'core.logAllRefUpdates=always creates no reflog for ORIG_HEAD'\n + * Updates to pseudorefs (eg. ORIG_HEAD) with\n + core.logAllRefUpdates=always will create reflogs for the pseudoref.\n \n - * t1400: change hard coded error messages to check for.\n -\n - * don't deref non-HEAD pseudoref symrefs. This fixes t1405. Without\n - this, a deleting a FOO symref pointing to refs/heads/master will remove\n - master too.\n + * non-HEAD pseudoref symrefs are also dereferenced on deletion. Update\n + t1405 accordingly.\n \n Signed-off-by: Han-Wen Nienhuys \n \n @@ refs.c: int refs_update_ref(struct ref_store *refs, const char *msg,\n \tif (ret) {\n \t\tconst char *str = _(\"update_ref failed for ref '%s': %s\");\n \n - ## refs/files-backend.c ##\n -@@ refs/files-backend.c: static int files_delete_refs(struct ref_store *ref_store, const char *msg,\n - \n - \tfor (i = 0; i < refnames->nr; i++) {\n - \t\tconst char *refname = refnames->items[i].string;\n --\n - \t\tif (refs_delete_ref(&refs->base, msg, refname, NULL, flags))\n - \t\t\tresult |= error(_(\"could not remove reference %s\"), refname);\n - \t}\n -@@ refs/files-backend.c: static int lock_ref_for_update(struct files_ref_store *refs,\n - \tupdate->backend_data = lock;\n + ## t/t1400-update-ref.sh ##\n +@@ t/t1400-update-ref.sh: test_expect_success 'core.logAllRefUpdates=always creates reflog by default' '\n + \tgit reflog exists $outside\n + '\n \n - \tif (update->type & REF_ISSYMREF) {\n --\t\tif (update->flags & REF_NO_DEREF) {\n -+\t\tif (update->flags & REF_NO_DEREF ||\n -+\t\t (ref_type(update->refname) == REF_TYPE_PSEUDOREF &&\n -+\t\t strcmp(update->refname, \"HEAD\"))) {\n - \t\t\t/*\n - \t\t\t * We won't be reading the referent as part of\n - \t\t\t * the transaction, so we have to read it here\n -@@ refs/files-backend.c: static int files_transaction_finish(struct ref_store *ref_store,\n - \t\tstruct ref_update *update = transaction->updates[i];\n - \t\tstruct ref_lock *lock = update->backend_data;\n +-test_expect_success 'core.logAllRefUpdates=always creates no reflog for ORIG_HEAD' '\n ++test_expect_success 'core.logAllRefUpdates=always creates reflog for ORIG_HEAD' '\n + \ttest_config core.logAllRefUpdates always &&\n + \tgit update-ref ORIG_HEAD $A &&\n +-\ttest_must_fail git reflog exists ORIG_HEAD\n ++\tgit reflog exists ORIG_HEAD\n + '\n \n --\t\tif (update->flags & REF_NEEDS_COMMIT ||\n --\t\t update->flags & REF_LOG_ONLY) {\n -+\t\tif ((ref_type(lock->ref_name) != REF_TYPE_PSEUDOREF ||\n -+\t\t !strcmp(lock->ref_name, \"HEAD\")) &&\n -+\t\t (update->flags & REF_NEEDS_COMMIT ||\n -+\t\t update->flags & REF_LOG_ONLY)) {\n - \t\t\tif (files_log_ref_write(refs,\n - \t\t\t\t\t\tlock->ref_name,\n - \t\t\t\t\t\t&lock->old_oid,\n -\n - ## t/t1400-update-ref.sh ##\n + test_expect_success '--no-create-reflog overrides core.logAllRefUpdates=always' '\n @@ t/t1400-update-ref.sh: test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER\n test_expect_success 'given old value for missing pseudoref, do not create' '\n \ttest_must_fail git update-ref PSEUDOREF $A $B 2>err &&\n - \ttest_path_is_missing .git/PSEUDOREF &&\n + \ttest_must_fail git rev-parse PSEUDOREF &&\n -\ttest_i18ngrep \"could not read ref\" err\n +\ttest_i18ngrep \"unable to resolve reference\" err\n '\n @@ t/t1400-update-ref.sh: test_expect_success 'git cat-file blob master@{2005-05-26\n @@ t/t1400-update-ref.sh: test_expect_success 'overwrite pseudoref with correct old value' '\n test_expect_success 'do not overwrite pseudoref with wrong old value' '\n \ttest_must_fail git update-ref PSEUDOREF $D $E 2>err &&\n - \ttest $C = $(cat .git/PSEUDOREF) &&\n + \ttest $C = $(git rev-parse PSEUDOREF) &&\n -\ttest_i18ngrep \"unexpected object ID\" err\n +\ttest_i18ngrep \"cannot lock ref.*expected\" err\n '\n @@ t/t1400-update-ref.sh: test_expect_success 'overwrite pseudoref with correct old\n @@ t/t1400-update-ref.sh: test_expect_success 'do not delete pseudoref with wrong old value' '\n \tgit update-ref PSEUDOREF $A &&\n \ttest_must_fail git update-ref -d PSEUDOREF $B 2>err &&\n - \ttest $A = $(cat .git/PSEUDOREF) &&\n + \ttest $A = $(git rev-parse PSEUDOREF) &&\n -\ttest_i18ngrep \"unexpected object ID\" err\n +\ttest_i18ngrep \"cannot lock ref.*expected\" err\n '\n \n test_expect_success 'delete pseudoref with correct old value' '\n +\n + ## t/t1405-main-ref-store.sh ##\n +@@ t/t1405-main-ref-store.sh: test_expect_success 'create_symref(FOO, refs/heads/master)' '\n + test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '\n + \tgit rev-parse FOO -- &&\n + \tgit rev-parse refs/tags/new-tag -- &&\n ++\tm=$(git rev-parse master) &&\n + \t$RUN delete-refs 0 nothing FOO refs/tags/new-tag &&\n + \ttest_must_fail git rev-parse FOO -- &&\n +-\ttest_must_fail git rev-parse refs/tags/new-tag --\n ++\ttest_must_fail git rev-parse refs/tags/new-tag --&&\n ++\ttest_must_fail git rev-parse master -- &&\n ++\tgit update-ref refs/heads/master $m\n + '\n + \n + test_expect_success 'rename_refs(master, new-master)' '\n 2: 470821dc6d = 3: 1c2b9d5f17 Make HEAD a PSEUDOREF rather than PER_WORKTREE.\n\n-- \ngitgitgadget\n"},{"id":"401267","messageId":"871b411517eaee006ed30e6dfa8b1133b15ecea6.1594329108.git.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.v2.git.1594329108.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] Modify pseudo refs through ref backend storage","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-09T21:11:47Z","receivedAt":"2020-07-09T21:11:55Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe previous behavior was introduced in commit 74ec19d4be\n(\"pseudorefs: create and use pseudoref update and delete functions\",\nJul 31, 2015), with the justification \"alternate ref backends still\nneed to store pseudorefs in GIT_DIR\".\n\nRefs such as REBASE_HEAD are read through the ref backend. This can\nonly work consistently if they are written through the ref backend as\nwell. Tooling that works directly on files under .git should be\nupdated to use git commands to read refs instead.\n\nThe following behaviors change:\n\n* Updates to pseudorefs (eg. ORIG_HEAD) with\n core.logAllRefUpdates=always will create reflogs for the pseudoref.\n\n* non-HEAD pseudoref symrefs are also dereferenced on deletion. Update\n t1405 accordingly.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n refs.c | 120 +++-----------------------------------\n t/t1400-update-ref.sh | 10 ++--\n t/t1405-main-ref-store.sh | 5 +-\n 3 files changed, 16 insertions(+), 119 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 639cba93b4..d676acb1f4 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -771,102 +771,6 @@ long get_files_ref_lock_timeout_ms(void)\n \treturn timeout_ms;\n }\n \n-static int write_pseudoref(const char *pseudoref, const struct object_id *oid,\n-\t\t\t const struct object_id *old_oid, struct strbuf *err)\n-{\n-\tconst char *filename;\n-\tint fd;\n-\tstruct lock_file lock = LOCK_INIT;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tint ret = -1;\n-\n-\tif (!oid)\n-\t\treturn 0;\n-\n-\tstrbuf_addf(&buf, \"%s\\n\", oid_to_hex(oid));\n-\n-\tfilename = git_path(\"%s\", pseudoref);\n-\tfd = hold_lock_file_for_update_timeout(&lock, filename, 0,\n-\t\t\t\t\t get_files_ref_lock_timeout_ms());\n-\tif (fd < 0) {\n-\t\tstrbuf_addf(err, _(\"could not open '%s' for writing: %s\"),\n-\t\t\t filename, strerror(errno));\n-\t\tgoto done;\n-\t}\n-\n-\tif (old_oid) {\n-\t\tstruct object_id actual_old_oid;\n-\n-\t\tif (read_ref(pseudoref, &actual_old_oid)) {\n-\t\t\tif (!is_null_oid(old_oid)) {\n-\t\t\t\tstrbuf_addf(err, _(\"could not read ref '%s'\"),\n-\t\t\t\t\t pseudoref);\n-\t\t\t\trollback_lock_file(&lock);\n-\t\t\t\tgoto done;\n-\t\t\t}\n-\t\t} else if (is_null_oid(old_oid)) {\n-\t\t\tstrbuf_addf(err, _(\"ref '%s' already exists\"),\n-\t\t\t\t pseudoref);\n-\t\t\trollback_lock_file(&lock);\n-\t\t\tgoto done;\n-\t\t} else if (!oideq(&actual_old_oid, old_oid)) {\n-\t\t\tstrbuf_addf(err, _(\"unexpected object ID when writing '%s'\"),\n-\t\t\t\t pseudoref);\n-\t\t\trollback_lock_file(&lock);\n-\t\t\tgoto done;\n-\t\t}\n-\t}\n-\n-\tif (write_in_full(fd, buf.buf, buf.len) < 0) {\n-\t\tstrbuf_addf(err, _(\"could not write to '%s'\"), filename);\n-\t\trollback_lock_file(&lock);\n-\t\tgoto done;\n-\t}\n-\n-\tcommit_lock_file(&lock);\n-\tret = 0;\n-done:\n-\tstrbuf_release(&buf);\n-\treturn ret;\n-}\n-\n-static int delete_pseudoref(const char *pseudoref, const struct object_id *old_oid)\n-{\n-\tconst char *filename;\n-\n-\tfilename = git_path(\"%s\", pseudoref);\n-\n-\tif (old_oid && !is_null_oid(old_oid)) {\n-\t\tstruct lock_file lock = LOCK_INIT;\n-\t\tint fd;\n-\t\tstruct object_id actual_old_oid;\n-\n-\t\tfd = hold_lock_file_for_update_timeout(\n-\t\t\t\t&lock, filename, 0,\n-\t\t\t\tget_files_ref_lock_timeout_ms());\n-\t\tif (fd < 0) {\n-\t\t\terror_errno(_(\"could not open '%s' for writing\"),\n-\t\t\t\t filename);\n-\t\t\treturn -1;\n-\t\t}\n-\t\tif (read_ref(pseudoref, &actual_old_oid))\n-\t\t\tdie(_(\"could not read ref '%s'\"), pseudoref);\n-\t\tif (!oideq(&actual_old_oid, old_oid)) {\n-\t\t\terror(_(\"unexpected object ID when deleting '%s'\"),\n-\t\t\t pseudoref);\n-\t\t\trollback_lock_file(&lock);\n-\t\t\treturn -1;\n-\t\t}\n-\n-\t\tunlink(filename);\n-\t\trollback_lock_file(&lock);\n-\t} else {\n-\t\tunlink(filename);\n-\t}\n-\n-\treturn 0;\n-}\n-\n int refs_delete_ref(struct ref_store *refs, const char *msg,\n \t\t const char *refname,\n \t\t const struct object_id *old_oid,\n@@ -875,11 +779,6 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n-\tif (ref_type(refname) == REF_TYPE_PSEUDOREF) {\n-\t\tassert(refs == get_main_ref_store(the_repository));\n-\t\treturn delete_pseudoref(refname, old_oid);\n-\t}\n-\n \ttransaction = ref_store_transaction_begin(refs, &err);\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, old_oid,\n@@ -1202,18 +1101,13 @@ int refs_update_ref(struct ref_store *refs, const char *msg,\n \tstruct strbuf err = STRBUF_INIT;\n \tint ret = 0;\n \n-\tif (ref_type(refname) == REF_TYPE_PSEUDOREF) {\n-\t\tassert(refs == get_main_ref_store(the_repository));\n-\t\tret = write_pseudoref(refname, new_oid, old_oid, &err);\n-\t} else {\n-\t\tt = ref_store_transaction_begin(refs, &err);\n-\t\tif (!t ||\n-\t\t ref_transaction_update(t, refname, new_oid, old_oid,\n-\t\t\t\t\t flags, msg, &err) ||\n-\t\t ref_transaction_commit(t, &err)) {\n-\t\t\tret = 1;\n-\t\t\tref_transaction_free(t);\n-\t\t}\n+\tt = ref_store_transaction_begin(refs, &err);\n+\tif (!t ||\n+\t ref_transaction_update(t, refname, new_oid, old_oid, flags, msg,\n+\t\t\t\t &err) ||\n+\t ref_transaction_commit(t, &err)) {\n+\t\tret = 1;\n+\t\tref_transaction_free(t);\n \t}\n \tif (ret) {\n \t\tconst char *str = _(\"update_ref failed for ref '%s': %s\");\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 7414b898f8..65d349fb33 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -160,10 +160,10 @@ test_expect_success 'core.logAllRefUpdates=always creates reflog by default' '\n \tgit reflog exists $outside\n '\n \n-test_expect_success 'core.logAllRefUpdates=always creates no reflog for ORIG_HEAD' '\n+test_expect_success 'core.logAllRefUpdates=always creates reflog for ORIG_HEAD' '\n \ttest_config core.logAllRefUpdates always &&\n \tgit update-ref ORIG_HEAD $A &&\n-\ttest_must_fail git reflog exists ORIG_HEAD\n+\tgit reflog exists ORIG_HEAD\n '\n \n test_expect_success '--no-create-reflog overrides core.logAllRefUpdates=always' '\n@@ -476,7 +476,7 @@ test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER\n test_expect_success 'given old value for missing pseudoref, do not create' '\n \ttest_must_fail git update-ref PSEUDOREF $A $B 2>err &&\n \ttest_must_fail git rev-parse PSEUDOREF &&\n-\ttest_i18ngrep \"could not read ref\" err\n+\ttest_i18ngrep \"unable to resolve reference\" err\n '\n \n test_expect_success 'create pseudoref' '\n@@ -497,7 +497,7 @@ test_expect_success 'overwrite pseudoref with correct old value' '\n test_expect_success 'do not overwrite pseudoref with wrong old value' '\n \ttest_must_fail git update-ref PSEUDOREF $D $E 2>err &&\n \ttest $C = $(git rev-parse PSEUDOREF) &&\n-\ttest_i18ngrep \"unexpected object ID\" err\n+\ttest_i18ngrep \"cannot lock ref.*expected\" err\n '\n \n test_expect_success 'delete pseudoref' '\n@@ -509,7 +509,7 @@ test_expect_success 'do not delete pseudoref with wrong old value' '\n \tgit update-ref PSEUDOREF $A &&\n \ttest_must_fail git update-ref -d PSEUDOREF $B 2>err &&\n \ttest $A = $(git rev-parse PSEUDOREF) &&\n-\ttest_i18ngrep \"unexpected object ID\" err\n+\ttest_i18ngrep \"cannot lock ref.*expected\" err\n '\n \n test_expect_success 'delete pseudoref with correct old value' '\ndiff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh\nindex 331899ddc4..a8d695cd4f 100755\n--- a/t/t1405-main-ref-store.sh\n+++ b/t/t1405-main-ref-store.sh\n@@ -31,9 +31,12 @@ test_expect_success 'create_symref(FOO, refs/heads/master)' '\n test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '\n \tgit rev-parse FOO -- &&\n \tgit rev-parse refs/tags/new-tag -- &&\n+\tm=$(git rev-parse master) &&\n \t$RUN delete-refs 0 nothing FOO refs/tags/new-tag &&\n \ttest_must_fail git rev-parse FOO -- &&\n-\ttest_must_fail git rev-parse refs/tags/new-tag --\n+\ttest_must_fail git rev-parse refs/tags/new-tag --&&\n+\ttest_must_fail git rev-parse master -- &&\n+\tgit update-ref refs/heads/master $m\n '\n \n test_expect_success 'rename_refs(master, new-master)' '\n-- \ngitgitgadget\n\n"},{"id":"401269","messageId":"1c2b9d5f177ab5ebf4d45ce77ebe8e6cec78c3a6.1594329108.git.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.v2.git.1594329108.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] Make HEAD a PSEUDOREF rather than PER_WORKTREE.","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-09T21:11:48Z","receivedAt":"2020-07-09T21:11:56Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThis is consistent with the definition of REF_TYPE_PSEUDOREF\n(uppercase in the root ref namespace).\n\nSigned-off-by: Han-Wen Nienhuys \n---\n refs.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex d676acb1f4..5cd71f711f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -708,10 +708,9 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)\n \n static int is_per_worktree_ref(const char *refname)\n {\n-\treturn !strcmp(refname, \"HEAD\") ||\n-\t\tstarts_with(refname, \"refs/worktree/\") ||\n-\t\tstarts_with(refname, \"refs/bisect/\") ||\n-\t\tstarts_with(refname, \"refs/rewritten/\");\n+\treturn starts_with(refname, \"refs/worktree/\") ||\n+\t starts_with(refname, \"refs/bisect/\") ||\n+\t starts_with(refname, \"refs/rewritten/\");\n }\n \n static int is_pseudoref_syntax(const char *refname)\n-- \ngitgitgadget\n"},{"id":"401268","messageId":"9c3dc4b2cb115de0725f79e27878103756de58ce.1594329108.git.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.v2.git.1594329108.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] t1400: use git rev-parse for testing PSEUDOREF existence","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-09T21:11:46Z","receivedAt":"2020-07-09T21:11:57Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThis will allow these tests to run with alternative ref backends\n\nSigned-off-by: Han-Wen Nienhuys \n---\n t/t1400-update-ref.sh | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 27171f8261..7414b898f8 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -475,57 +475,57 @@ test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER\n \n test_expect_success 'given old value for missing pseudoref, do not create' '\n \ttest_must_fail git update-ref PSEUDOREF $A $B 2>err &&\n-\ttest_path_is_missing .git/PSEUDOREF &&\n+\ttest_must_fail git rev-parse PSEUDOREF &&\n \ttest_i18ngrep \"could not read ref\" err\n '\n \n test_expect_success 'create pseudoref' '\n \tgit update-ref PSEUDOREF $A &&\n-\ttest $A = $(cat .git/PSEUDOREF)\n+\ttest $A = $(git rev-parse PSEUDOREF)\n '\n \n test_expect_success 'overwrite pseudoref with no old value given' '\n \tgit update-ref PSEUDOREF $B &&\n-\ttest $B = $(cat .git/PSEUDOREF)\n+\ttest $B = $(git rev-parse PSEUDOREF)\n '\n \n test_expect_success 'overwrite pseudoref with correct old value' '\n \tgit update-ref PSEUDOREF $C $B &&\n-\ttest $C = $(cat .git/PSEUDOREF)\n+\ttest $C = $(git rev-parse PSEUDOREF)\n '\n \n test_expect_success 'do not overwrite pseudoref with wrong old value' '\n \ttest_must_fail git update-ref PSEUDOREF $D $E 2>err &&\n-\ttest $C = $(cat .git/PSEUDOREF) &&\n+\ttest $C = $(git rev-parse PSEUDOREF) &&\n \ttest_i18ngrep \"unexpected object ID\" err\n '\n \n test_expect_success 'delete pseudoref' '\n \tgit update-ref -d PSEUDOREF &&\n-\ttest_path_is_missing .git/PSEUDOREF\n+\ttest_must_fail git rev-parse PSEUDOREF\n '\n \n test_expect_success 'do not delete pseudoref with wrong old value' '\n \tgit update-ref PSEUDOREF $A &&\n \ttest_must_fail git update-ref -d PSEUDOREF $B 2>err &&\n-\ttest $A = $(cat .git/PSEUDOREF) &&\n+\ttest $A = $(git rev-parse PSEUDOREF) &&\n \ttest_i18ngrep \"unexpected object ID\" err\n '\n \n test_expect_success 'delete pseudoref with correct old value' '\n \tgit update-ref -d PSEUDOREF $A &&\n-\ttest_path_is_missing .git/PSEUDOREF\n+\ttest_must_fail git rev-parse PSEUDOREF\n '\n \n test_expect_success 'create pseudoref with old OID zero' '\n \tgit update-ref PSEUDOREF $A $Z &&\n-\ttest $A = $(cat .git/PSEUDOREF)\n+\ttest $A = $(git rev-parse PSEUDOREF)\n '\n \n test_expect_success 'do not overwrite pseudoref with old OID zero' '\n \ttest_when_finished git update-ref -d PSEUDOREF &&\n \ttest_must_fail git update-ref PSEUDOREF $B $Z 2>err &&\n-\ttest $A = $(cat .git/PSEUDOREF) &&\n+\ttest $A = $(git rev-parse PSEUDOREF) &&\n \ttest_i18ngrep \"already exists\" err\n '\n \n-- \ngitgitgadget\n\n"},{"id":"401367","messageId":"xmqqwo3btboo.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"pull.673.v2.git.1594329108.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/3] Remove special casing for PSEUDOREF updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-10T19:25:11Z","receivedAt":"2020-07-10T19:25:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" writes:\n\n> This gets rid of the special casing code for pseudorefs in refs.c\n>\n> This is in preparation for reftable.\n>\n> v2\n>\n> * remove special casing of non-HEAD pseudorefs; update t1400 and t1405\n> accordingly\n> * open question: should git-update-ref.txt be updated, when it talks about\n> logAllRefUpdates?\n\nThis does make writing ORIG_HEAD and friends a lot more heavyweight\nthat just \"write a 40-byte text file on the filesystem to remember\nit for a short while\", and it becomes even so with the whole reflog\nthing, but I think it is a good longer term direction to go. \n\nOthers may disagree, though.\n\nIn any case, if we are changing the behaviour with these patches, we\nshould update the documentation.\n\nThanks.\n\n\n"},{"id":"401659","messageId":"pull.673.v3.git.1594925141.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.v2.git.1594329108.gitgitgadget@gmail.com","subject":"[PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-16T18:45:38Z","receivedAt":"2020-07-16T18:45:46Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"This gets rid of the special casing code for pseudorefs in refs.c\n\nThis is in preparation for reftable.\n\nv3\n\n * tweak git-update-ref.txt description for logAllRefUpdates\n\nHan-Wen Nienhuys (3):\n t1400: use git rev-parse for testing PSEUDOREF existence\n Modify pseudo refs through ref backend storage\n Make HEAD a PSEUDOREF rather than PER_WORKTREE.\n\n Documentation/git-update-ref.txt | 13 ++--\n refs.c | 127 +++----------------------------\n t/t1400-update-ref.sh | 30 ++++----\n t/t1405-main-ref-store.sh | 5 +-\n 4 files changed, 36 insertions(+), 139 deletions(-)\n\n\nbase-commit: b6a658bd00c9c29e07f833cabfc0ef12224e277a\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-673%2Fhanwen%2Fpseudoref-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-673/hanwen/pseudoref-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/673\n\nRange-diff vs v2:\n\n 1: 9c3dc4b2cb = 1: 28bd3534d0 t1400: use git rev-parse for testing PSEUDOREF existence\n 2: 871b411517 ! 2: 79cd5dd480 Modify pseudo refs through ref backend storage\n @@ Commit message\n \n Signed-off-by: Han-Wen Nienhuys \n \n + ## Documentation/git-update-ref.txt ##\n +@@ Documentation/git-update-ref.txt: still see a subset of the modifications.\n + \n + LOGGING UPDATES\n + ---------------\n +-If config parameter \"core.logAllRefUpdates\" is true and the ref is one under\n +-\"refs/heads/\", \"refs/remotes/\", \"refs/notes/\", or the symbolic ref HEAD; or\n +-the file \"$GIT_DIR/logs/\" exists then `git update-ref` will append\n +-a line to the log file \"$GIT_DIR/logs/\" (dereferencing all\n +-symbolic refs before creating the log name) describing the change\n +-in ref value. Log lines are formatted as:\n ++If config parameter \"core.logAllRefUpdates\" is true and the ref is one\n ++under \"refs/heads/\", \"refs/remotes/\", \"refs/notes/\", or a pseudoref\n ++like HEAD or ORIG_HEAD; or the file \"$GIT_DIR/logs/\" exists then\n ++`git update-ref` will append a line to the log file\n ++\"$GIT_DIR/logs/\" (dereferencing all symbolic refs before creating\n ++the log name) describing the change in ref value. Log lines are\n ++formatted as:\n + \n + oldsha1 SP newsha1 SP committer LF\n + \n +\n ## refs.c ##\n @@ refs.c: long get_files_ref_lock_timeout_ms(void)\n \treturn timeout_ms;\n 3: 1c2b9d5f17 = 3: 3ab9f2f04e Make HEAD a PSEUDOREF rather than PER_WORKTREE.\n\n-- \ngitgitgadget\n"},{"id":"401660","messageId":"28bd3534d02b7384daf87fb89eb56909883ebec7.1594925141.git.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.v3.git.1594925141.gitgitgadget@gmail.com","subject":"[PATCH v3 1/3] t1400: use git rev-parse for testing PSEUDOREF existence","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-16T18:45:39Z","receivedAt":"2020-07-16T18:45:47Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThis will allow these tests to run with alternative ref backends\n\nSigned-off-by: Han-Wen Nienhuys \n---\n t/t1400-update-ref.sh | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 27171f8261..7414b898f8 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -475,57 +475,57 @@ test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER\n \n test_expect_success 'given old value for missing pseudoref, do not create' '\n \ttest_must_fail git update-ref PSEUDOREF $A $B 2>err &&\n-\ttest_path_is_missing .git/PSEUDOREF &&\n+\ttest_must_fail git rev-parse PSEUDOREF &&\n \ttest_i18ngrep \"could not read ref\" err\n '\n \n test_expect_success 'create pseudoref' '\n \tgit update-ref PSEUDOREF $A &&\n-\ttest $A = $(cat .git/PSEUDOREF)\n+\ttest $A = $(git rev-parse PSEUDOREF)\n '\n \n test_expect_success 'overwrite pseudoref with no old value given' '\n \tgit update-ref PSEUDOREF $B &&\n-\ttest $B = $(cat .git/PSEUDOREF)\n+\ttest $B = $(git rev-parse PSEUDOREF)\n '\n \n test_expect_success 'overwrite pseudoref with correct old value' '\n \tgit update-ref PSEUDOREF $C $B &&\n-\ttest $C = $(cat .git/PSEUDOREF)\n+\ttest $C = $(git rev-parse PSEUDOREF)\n '\n \n test_expect_success 'do not overwrite pseudoref with wrong old value' '\n \ttest_must_fail git update-ref PSEUDOREF $D $E 2>err &&\n-\ttest $C = $(cat .git/PSEUDOREF) &&\n+\ttest $C = $(git rev-parse PSEUDOREF) &&\n \ttest_i18ngrep \"unexpected object ID\" err\n '\n \n test_expect_success 'delete pseudoref' '\n \tgit update-ref -d PSEUDOREF &&\n-\ttest_path_is_missing .git/PSEUDOREF\n+\ttest_must_fail git rev-parse PSEUDOREF\n '\n \n test_expect_success 'do not delete pseudoref with wrong old value' '\n \tgit update-ref PSEUDOREF $A &&\n \ttest_must_fail git update-ref -d PSEUDOREF $B 2>err &&\n-\ttest $A = $(cat .git/PSEUDOREF) &&\n+\ttest $A = $(git rev-parse PSEUDOREF) &&\n \ttest_i18ngrep \"unexpected object ID\" err\n '\n \n test_expect_success 'delete pseudoref with correct old value' '\n \tgit update-ref -d PSEUDOREF $A &&\n-\ttest_path_is_missing .git/PSEUDOREF\n+\ttest_must_fail git rev-parse PSEUDOREF\n '\n \n test_expect_success 'create pseudoref with old OID zero' '\n \tgit update-ref PSEUDOREF $A $Z &&\n-\ttest $A = $(cat .git/PSEUDOREF)\n+\ttest $A = $(git rev-parse PSEUDOREF)\n '\n \n test_expect_success 'do not overwrite pseudoref with old OID zero' '\n \ttest_when_finished git update-ref -d PSEUDOREF &&\n \ttest_must_fail git update-ref PSEUDOREF $B $Z 2>err &&\n-\ttest $A = $(cat .git/PSEUDOREF) &&\n+\ttest $A = $(git rev-parse PSEUDOREF) &&\n \ttest_i18ngrep \"already exists\" err\n '\n \n-- \ngitgitgadget\n\n"},{"id":"401661","messageId":"3ab9f2f04ef35956600a738004780dc0c53d1faa.1594925141.git.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.v3.git.1594925141.gitgitgadget@gmail.com","subject":"[PATCH v3 3/3] Make HEAD a PSEUDOREF rather than PER_WORKTREE.","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-16T18:45:41Z","receivedAt":"2020-07-16T18:45:50Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThis is consistent with the definition of REF_TYPE_PSEUDOREF\n(uppercase in the root ref namespace).\n\nSigned-off-by: Han-Wen Nienhuys \n---\n refs.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex d676acb1f4..5cd71f711f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -708,10 +708,9 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)\n \n static int is_per_worktree_ref(const char *refname)\n {\n-\treturn !strcmp(refname, \"HEAD\") ||\n-\t\tstarts_with(refname, \"refs/worktree/\") ||\n-\t\tstarts_with(refname, \"refs/bisect/\") ||\n-\t\tstarts_with(refname, \"refs/rewritten/\");\n+\treturn starts_with(refname, \"refs/worktree/\") ||\n+\t starts_with(refname, \"refs/bisect/\") ||\n+\t starts_with(refname, \"refs/rewritten/\");\n }\n \n static int is_pseudoref_syntax(const char *refname)\n-- \ngitgitgadget\n"},{"id":"401662","messageId":"79cd5dd480a4218d4563120c03046375594cc23e.1594925141.git.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.v3.git.1594925141.gitgitgadget@gmail.com","subject":"[PATCH v3 2/3] Modify pseudo refs through ref backend storage","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-16T18:45:40Z","receivedAt":"2020-07-16T18:45:50Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe previous behavior was introduced in commit 74ec19d4be\n(\"pseudorefs: create and use pseudoref update and delete functions\",\nJul 31, 2015), with the justification \"alternate ref backends still\nneed to store pseudorefs in GIT_DIR\".\n\nRefs such as REBASE_HEAD are read through the ref backend. This can\nonly work consistently if they are written through the ref backend as\nwell. Tooling that works directly on files under .git should be\nupdated to use git commands to read refs instead.\n\nThe following behaviors change:\n\n* Updates to pseudorefs (eg. ORIG_HEAD) with\n core.logAllRefUpdates=always will create reflogs for the pseudoref.\n\n* non-HEAD pseudoref symrefs are also dereferenced on deletion. Update\n t1405 accordingly.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n Documentation/git-update-ref.txt | 13 ++--\n refs.c | 120 ++-----------------------------\n t/t1400-update-ref.sh | 10 +--\n t/t1405-main-ref-store.sh | 5 +-\n 4 files changed, 23 insertions(+), 125 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex 3e737c2360..d401234b03 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -148,12 +148,13 @@ still see a subset of the modifications.\n \n LOGGING UPDATES\n ---------------\n-If config parameter \"core.logAllRefUpdates\" is true and the ref is one under\n-\"refs/heads/\", \"refs/remotes/\", \"refs/notes/\", or the symbolic ref HEAD; or\n-the file \"$GIT_DIR/logs/\" exists then `git update-ref` will append\n-a line to the log file \"$GIT_DIR/logs/\" (dereferencing all\n-symbolic refs before creating the log name) describing the change\n-in ref value. Log lines are formatted as:\n+If config parameter \"core.logAllRefUpdates\" is true and the ref is one\n+under \"refs/heads/\", \"refs/remotes/\", \"refs/notes/\", or a pseudoref\n+like HEAD or ORIG_HEAD; or the file \"$GIT_DIR/logs/\" exists then\n+`git update-ref` will append a line to the log file\n+\"$GIT_DIR/logs/\" (dereferencing all symbolic refs before creating\n+the log name) describing the change in ref value. Log lines are\n+formatted as:\n \n oldsha1 SP newsha1 SP committer LF\n \ndiff --git a/refs.c b/refs.c\nindex 639cba93b4..d676acb1f4 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -771,102 +771,6 @@ long get_files_ref_lock_timeout_ms(void)\n \treturn timeout_ms;\n }\n \n-static int write_pseudoref(const char *pseudoref, const struct object_id *oid,\n-\t\t\t const struct object_id *old_oid, struct strbuf *err)\n-{\n-\tconst char *filename;\n-\tint fd;\n-\tstruct lock_file lock = LOCK_INIT;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tint ret = -1;\n-\n-\tif (!oid)\n-\t\treturn 0;\n-\n-\tstrbuf_addf(&buf, \"%s\\n\", oid_to_hex(oid));\n-\n-\tfilename = git_path(\"%s\", pseudoref);\n-\tfd = hold_lock_file_for_update_timeout(&lock, filename, 0,\n-\t\t\t\t\t get_files_ref_lock_timeout_ms());\n-\tif (fd < 0) {\n-\t\tstrbuf_addf(err, _(\"could not open '%s' for writing: %s\"),\n-\t\t\t filename, strerror(errno));\n-\t\tgoto done;\n-\t}\n-\n-\tif (old_oid) {\n-\t\tstruct object_id actual_old_oid;\n-\n-\t\tif (read_ref(pseudoref, &actual_old_oid)) {\n-\t\t\tif (!is_null_oid(old_oid)) {\n-\t\t\t\tstrbuf_addf(err, _(\"could not read ref '%s'\"),\n-\t\t\t\t\t pseudoref);\n-\t\t\t\trollback_lock_file(&lock);\n-\t\t\t\tgoto done;\n-\t\t\t}\n-\t\t} else if (is_null_oid(old_oid)) {\n-\t\t\tstrbuf_addf(err, _(\"ref '%s' already exists\"),\n-\t\t\t\t pseudoref);\n-\t\t\trollback_lock_file(&lock);\n-\t\t\tgoto done;\n-\t\t} else if (!oideq(&actual_old_oid, old_oid)) {\n-\t\t\tstrbuf_addf(err, _(\"unexpected object ID when writing '%s'\"),\n-\t\t\t\t pseudoref);\n-\t\t\trollback_lock_file(&lock);\n-\t\t\tgoto done;\n-\t\t}\n-\t}\n-\n-\tif (write_in_full(fd, buf.buf, buf.len) < 0) {\n-\t\tstrbuf_addf(err, _(\"could not write to '%s'\"), filename);\n-\t\trollback_lock_file(&lock);\n-\t\tgoto done;\n-\t}\n-\n-\tcommit_lock_file(&lock);\n-\tret = 0;\n-done:\n-\tstrbuf_release(&buf);\n-\treturn ret;\n-}\n-\n-static int delete_pseudoref(const char *pseudoref, const struct object_id *old_oid)\n-{\n-\tconst char *filename;\n-\n-\tfilename = git_path(\"%s\", pseudoref);\n-\n-\tif (old_oid && !is_null_oid(old_oid)) {\n-\t\tstruct lock_file lock = LOCK_INIT;\n-\t\tint fd;\n-\t\tstruct object_id actual_old_oid;\n-\n-\t\tfd = hold_lock_file_for_update_timeout(\n-\t\t\t\t&lock, filename, 0,\n-\t\t\t\tget_files_ref_lock_timeout_ms());\n-\t\tif (fd < 0) {\n-\t\t\terror_errno(_(\"could not open '%s' for writing\"),\n-\t\t\t\t filename);\n-\t\t\treturn -1;\n-\t\t}\n-\t\tif (read_ref(pseudoref, &actual_old_oid))\n-\t\t\tdie(_(\"could not read ref '%s'\"), pseudoref);\n-\t\tif (!oideq(&actual_old_oid, old_oid)) {\n-\t\t\terror(_(\"unexpected object ID when deleting '%s'\"),\n-\t\t\t pseudoref);\n-\t\t\trollback_lock_file(&lock);\n-\t\t\treturn -1;\n-\t\t}\n-\n-\t\tunlink(filename);\n-\t\trollback_lock_file(&lock);\n-\t} else {\n-\t\tunlink(filename);\n-\t}\n-\n-\treturn 0;\n-}\n-\n int refs_delete_ref(struct ref_store *refs, const char *msg,\n \t\t const char *refname,\n \t\t const struct object_id *old_oid,\n@@ -875,11 +779,6 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n-\tif (ref_type(refname) == REF_TYPE_PSEUDOREF) {\n-\t\tassert(refs == get_main_ref_store(the_repository));\n-\t\treturn delete_pseudoref(refname, old_oid);\n-\t}\n-\n \ttransaction = ref_store_transaction_begin(refs, &err);\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, old_oid,\n@@ -1202,18 +1101,13 @@ int refs_update_ref(struct ref_store *refs, const char *msg,\n \tstruct strbuf err = STRBUF_INIT;\n \tint ret = 0;\n \n-\tif (ref_type(refname) == REF_TYPE_PSEUDOREF) {\n-\t\tassert(refs == get_main_ref_store(the_repository));\n-\t\tret = write_pseudoref(refname, new_oid, old_oid, &err);\n-\t} else {\n-\t\tt = ref_store_transaction_begin(refs, &err);\n-\t\tif (!t ||\n-\t\t ref_transaction_update(t, refname, new_oid, old_oid,\n-\t\t\t\t\t flags, msg, &err) ||\n-\t\t ref_transaction_commit(t, &err)) {\n-\t\t\tret = 1;\n-\t\t\tref_transaction_free(t);\n-\t\t}\n+\tt = ref_store_transaction_begin(refs, &err);\n+\tif (!t ||\n+\t ref_transaction_update(t, refname, new_oid, old_oid, flags, msg,\n+\t\t\t\t &err) ||\n+\t ref_transaction_commit(t, &err)) {\n+\t\tret = 1;\n+\t\tref_transaction_free(t);\n \t}\n \tif (ret) {\n \t\tconst char *str = _(\"update_ref failed for ref '%s': %s\");\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 7414b898f8..65d349fb33 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -160,10 +160,10 @@ test_expect_success 'core.logAllRefUpdates=always creates reflog by default' '\n \tgit reflog exists $outside\n '\n \n-test_expect_success 'core.logAllRefUpdates=always creates no reflog for ORIG_HEAD' '\n+test_expect_success 'core.logAllRefUpdates=always creates reflog for ORIG_HEAD' '\n \ttest_config core.logAllRefUpdates always &&\n \tgit update-ref ORIG_HEAD $A &&\n-\ttest_must_fail git reflog exists ORIG_HEAD\n+\tgit reflog exists ORIG_HEAD\n '\n \n test_expect_success '--no-create-reflog overrides core.logAllRefUpdates=always' '\n@@ -476,7 +476,7 @@ test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER\n test_expect_success 'given old value for missing pseudoref, do not create' '\n \ttest_must_fail git update-ref PSEUDOREF $A $B 2>err &&\n \ttest_must_fail git rev-parse PSEUDOREF &&\n-\ttest_i18ngrep \"could not read ref\" err\n+\ttest_i18ngrep \"unable to resolve reference\" err\n '\n \n test_expect_success 'create pseudoref' '\n@@ -497,7 +497,7 @@ test_expect_success 'overwrite pseudoref with correct old value' '\n test_expect_success 'do not overwrite pseudoref with wrong old value' '\n \ttest_must_fail git update-ref PSEUDOREF $D $E 2>err &&\n \ttest $C = $(git rev-parse PSEUDOREF) &&\n-\ttest_i18ngrep \"unexpected object ID\" err\n+\ttest_i18ngrep \"cannot lock ref.*expected\" err\n '\n \n test_expect_success 'delete pseudoref' '\n@@ -509,7 +509,7 @@ test_expect_success 'do not delete pseudoref with wrong old value' '\n \tgit update-ref PSEUDOREF $A &&\n \ttest_must_fail git update-ref -d PSEUDOREF $B 2>err &&\n \ttest $A = $(git rev-parse PSEUDOREF) &&\n-\ttest_i18ngrep \"unexpected object ID\" err\n+\ttest_i18ngrep \"cannot lock ref.*expected\" err\n '\n \n test_expect_success 'delete pseudoref with correct old value' '\ndiff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh\nindex 331899ddc4..a8d695cd4f 100755\n--- a/t/t1405-main-ref-store.sh\n+++ b/t/t1405-main-ref-store.sh\n@@ -31,9 +31,12 @@ test_expect_success 'create_symref(FOO, refs/heads/master)' '\n test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '\n \tgit rev-parse FOO -- &&\n \tgit rev-parse refs/tags/new-tag -- &&\n+\tm=$(git rev-parse master) &&\n \t$RUN delete-refs 0 nothing FOO refs/tags/new-tag &&\n \ttest_must_fail git rev-parse FOO -- &&\n-\ttest_must_fail git rev-parse refs/tags/new-tag --\n+\ttest_must_fail git rev-parse refs/tags/new-tag --&&\n+\ttest_must_fail git rev-parse master -- &&\n+\tgit update-ref refs/heads/master $m\n '\n \n test_expect_success 'rename_refs(master, new-master)' '\n-- \ngitgitgadget\n\n"},{"id":"401665","messageId":"xmqqwo33f6xb.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"pull.673.v3.git.1594925141.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-16T22:09:52Z","receivedAt":"2020-07-16T22:10:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" writes:\n\n> This gets rid of the special casing code for pseudorefs in refs.c\n>\n> This is in preparation for reftable.\n>\n> v3\n>\n> * tweak git-update-ref.txt description for logAllRefUpdates\n>\n> Han-Wen Nienhuys (3):\n> t1400: use git rev-parse for testing PSEUDOREF existence\n> Modify pseudo refs through ref backend storage\n> Make HEAD a PSEUDOREF rather than PER_WORKTREE.\n\nI reviewed some codepaths that deal with FETCH_HEAD recently. \n\nAs the file is quite different from all the other pseudo references\nin that it needs to store more than one object name and in that each\nref in it needs more than just the object name, I doubt that it\nmakes much sense to enhance the refs API so that its requirements\ncan be covered.\n\nIt would probably make sense for FETCH_HEAD to stay \"a text file\ndirectly underneath $GIT_DIR\", but get_oid() needs to be able to\nread it and return the first object name recorded in the file.\n"},{"id":"401680","messageId":"nycvar.QRO.7.76.6.2007171042461.54@tvgsbejvaqbjf.bet","threadId":"53801","inReplyTo":"79cd5dd480a4218d4563120c03046375594cc23e.1594925141.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 2/3] Modify pseudo refs through ref backend storage","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-07-17T08:52:06Z","receivedAt":"2020-07-17T10:18:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Han-Wen,\n\nOn Thu, 16 Jul 2020, Han-Wen Nienhuys via GitGitGadget wrote:\n\n> From: Han-Wen Nienhuys \n>\n> The previous behavior was introduced in commit 74ec19d4be\n> (\"pseudorefs: create and use pseudoref update and delete functions\",\n> Jul 31, 2015), with the justification \"alternate ref backends still\n> need to store pseudorefs in GIT_DIR\".\n>\n> Refs such as REBASE_HEAD are read through the ref backend. This can\n> only work consistently if they are written through the ref backend as\n> well. Tooling that works directly on files under .git should be\n> updated to use git commands to read refs instead.\n>\n> The following behaviors change:\n>\n> * Updates to pseudorefs (eg. ORIG_HEAD) with\n> core.logAllRefUpdates=always will create reflogs for the pseudoref.\n>\n> * non-HEAD pseudoref symrefs are also dereferenced on deletion. Update\n> t1405 accordingly.\n\nUnfortunately, there is still a breakage in t1405, although it only really\nshows up on Windows and macOS because of case-insensitive filesystems.\nhttps://github.com/gitgitgadget/git/runs/879772942?check_suite_focus=true\nshows the concrete problem: t1405.17 'delete_ref(refs/heads/foo)' will\nfail thusly:\n\n\t++ test-tool ref-store main delete-ref msg refs/heads/foo c90e4dc5e12224a428dedfbd45ba11e5531706a2 0\n\terror: cannot lock ref 'refs/heads/foo': is at 1e995a94b5a60488b6ebaf78ec779d85a55ea700 but expected c90e4dc5e12224a428dedfbd45ba11e5531706a2\n\terror: last command exited with $?=1\n\nThe reason is that there exists a `refs/heads/foo` _and_ the symref `FOO`\n(which points to `refs/heads/master`, which is at 1e995a).\n\nAn earlier symptom of this bug is visible on Linux, too, though: when\nyou run t1405.4 'delete_refs(FOO, refs/tags/new-tag)', it shows this:\n\n\t+ git rev-parse FOO --\n\twarning: ignoring dangling symref FOO\n\tfatal: bad revision 'FOO'\n\n(Seeing this warning suggests to me that this should probably be caught\n_by the regression test_, i.e. stderr should be redirected, and it should\nbe verified via `test_i18ngrep` that that warning is not shown.)\n\nThe reason for this warning is that the symref `.git/FOO` is no longer\nremoved as part of t1405.4 'delete_refs(FOO, refs/tags/new-tag)'. I've\nused this patch to bisect the breakage down to this here patch:\n\n-- snip --\ndiff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh\nindex a8d695cd4f1..779906d607f 100755\n--- a/t/t1405-main-ref-store.sh\n+++ b/t/t1405-main-ref-store.sh\n@@ -34,6 +34,7 @@ test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '\n \tm=$(git rev-parse master) &&\n \t$RUN delete-refs 0 nothing FOO refs/tags/new-tag &&\n \ttest_must_fail git rev-parse FOO -- &&\n+\ttest_path_is_missing .git/FOO &&\n \ttest_must_fail git rev-parse refs/tags/new-tag --&&\n \ttest_must_fail git rev-parse master -- &&\n \tgit update-ref refs/heads/master $m\n-- snap --\n\nHopefully you can figure out what is going wrong.\n\nCiao,\nDscho\n"},{"id":"402127","messageId":"CAFQ2z_P+L5ystTA8MjSjnUJQyEoH5Q3QtnMd0ezJpKPv_ntrgA@mail.gmail.com","threadId":"53801","inReplyTo":"xmqqwo33f6xb.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2020-07-27T08:50:50Z","receivedAt":"2020-07-27T08:51:04Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Fri, Jul 17, 2020 at 12:10 AM Junio C Hamano wrote:\n> I reviewed some codepaths that deal with FETCH_HEAD recently.\n>\n> As the file is quite different from all the other pseudo references\n> in that it needs to store more than one object name and in that each\n> ref in it needs more than just the object name, I doubt that it\n> makes much sense to enhance the refs API so that its requirements\n> can be covered.\n\n\n\nI agree. Do we ever pretend that FETCH_HEAD is a ref today?\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"402135","messageId":"CAFQ2z_M-OcJA_a3QZ2QfzREM2JHN6G89HKPuccUUSwKWbp_Fdw@mail.gmail.com","threadId":"53801","inReplyTo":"nycvar.QRO.7.76.6.2007171042461.54@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v3 2/3] Modify pseudo refs through ref backend storage","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2020-07-27T15:41:14Z","receivedAt":"2020-07-27T15:41:29Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Fri, Jul 17, 2020 at 12:18 PM Johannes Schindelin\n wrote:\n> The reason is that there exists a `refs/heads/foo` _and_ the symref `FOO`\n> (which points to `refs/heads/master`, which is at 1e995a).\n>\n> An earlier symptom of this bug is visible on Linux, too, though: when\n> you run t1405.4 'delete_refs(FOO, refs/tags/new-tag)', it shows this:\n>\n> + git rev-parse FOO --\n> warning: ignoring dangling symref FOO\n> fatal: bad revision 'FOO'\n\nThanks, this was very helpful for debugging.\n\nThe reason is that we do split_symref_update(), where a transaction\nthat modifies SYMREF (pointing to REFERENT) without REF_NO_DEREF is\nchanged to a transaction that modifies REFERENT and a reflog-only\nupdate for SYMREF. In this case, that leaves the FOO symref in place.\n\nThis seems very specific to the test (is there any use of symrefs\nbeyond HEAD?) , and I fixed it by passing REF_NO_DEREF to the\ndelete-refs call.\n\nIt would also be nice if this behavior was lifted out of the files\nbackend and into the generic layer, but it's not yet clear to me if\nthis is possible.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\nRegistergericht und -nummer: Hamburg, HRB 86891\nSitz der Gesellschaft: Hamburg\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"402138","messageId":"xmqqpn8hgca7.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"CAFQ2z_P+L5ystTA8MjSjnUJQyEoH5Q3QtnMd0ezJpKPv_ntrgA@mail.gmail.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-27T16:20:32Z","receivedAt":"2020-07-27T16:20:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n> On Fri, Jul 17, 2020 at 12:10 AM Junio C Hamano wrote:\n>> I reviewed some codepaths that deal with FETCH_HEAD recently.\n>>\n>> As the file is quite different from all the other pseudo references\n>> in that it needs to store more than one object name and in that each\n>> ref in it needs more than just the object name, I doubt that it\n>> makes much sense to enhance the refs API so that its requirements\n>> can be covered.\n>\n> I agree. Do we ever pretend that FETCH_HEAD is a ref today?\n\n\"git rev-parse FETCH_HEAD\", \"git show FETCH_HEAD\" etc. all should keep\nworking, so in that sense, it is treated as a ref. It does not\nprotect the history leading to the objects listed in it from being\ncollected, though.\n\n\"git merge FETCH_HEAD\" is an interesting case---I haven't thought it\nthrough.\n\nJung fubhyq unccra nsgre \"tvg chyy bevtva sbb one\" nggrzcgf gb teno\ngjb oenapurf naq znxr na bpgbchf zretr vagb gur oenapu pheeragyl\npurpxrq bhg, naq gura \"tvg erfrg --uneq && tvg zretr SRGPU_URNQ\" vf\ntvira?\n"},{"id":"402139","messageId":"pull.673.v4.git.1595867147.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.v3.git.1594925141.gitgitgadget@gmail.com","subject":"[PATCH v4 0/3] Remove special casing for PSEUDOREF updates","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-27T16:25:44Z","receivedAt":"2020-07-27T16:25:52Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"This gets rid of the special casing code for pseudorefs in refs.c\n\nThis is in preparation for reftable.\n\nv4\n\n * Address problems on case-insensitive file systems reported by Dscho.\n\nHan-Wen Nienhuys (3):\n t1400: use git rev-parse for testing PSEUDOREF existence\n Modify pseudo refs through ref backend storage\n Make HEAD a PSEUDOREF rather than PER_WORKTREE.\n\n Documentation/git-update-ref.txt | 13 ++--\n refs.c | 127 +++----------------------------\n t/t1400-update-ref.sh | 30 ++++----\n t/t1405-main-ref-store.sh | 5 +-\n 4 files changed, 36 insertions(+), 139 deletions(-)\n\n\nbase-commit: 5c06d60fc55d2213c089f63c282468080f812686\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-673%2Fhanwen%2Fpseudoref-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-673/hanwen/pseudoref-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/673\n\nRange-diff vs v3:\n\n 1: 28bd3534d0 = 1: b5274d9053 t1400: use git rev-parse for testing PSEUDOREF existence\n 2: 79cd5dd480 ! 2: ba5f1b4d26 Modify pseudo refs through ref backend storage\n @@ t/t1405-main-ref-store.sh: test_expect_success 'create_symref(FOO, refs/heads/ma\n test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '\n \tgit rev-parse FOO -- &&\n \tgit rev-parse refs/tags/new-tag -- &&\n +-\t$RUN delete-refs 0 nothing FOO refs/tags/new-tag &&\n +\tm=$(git rev-parse master) &&\n - \t$RUN delete-refs 0 nothing FOO refs/tags/new-tag &&\n ++\tREF_NO_DEREF=1 &&\n ++\t$RUN delete-refs $REF_NO_DEREF nothing FOO refs/tags/new-tag &&\n ++\ttest_must_fail git rev-parse --symbolic-full-name FOO &&\n \ttest_must_fail git rev-parse FOO -- &&\n --\ttest_must_fail git rev-parse refs/tags/new-tag --\n -+\ttest_must_fail git rev-parse refs/tags/new-tag --&&\n -+\ttest_must_fail git rev-parse master -- &&\n -+\tgit update-ref refs/heads/master $m\n + \ttest_must_fail git rev-parse refs/tags/new-tag --\n '\n - \n - test_expect_success 'rename_refs(master, new-master)' '\n 3: 3ab9f2f04e = 3: 358d34df07 Make HEAD a PSEUDOREF rather than PER_WORKTREE.\n\n-- \ngitgitgadget\n"},{"id":"402140","messageId":"b5274d9053ffa9a56923cc9ba3e10e3bd1d07f0f.1595867147.git.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.v4.git.1595867147.gitgitgadget@gmail.com","subject":"[PATCH v4 1/3] t1400: use git rev-parse for testing PSEUDOREF existence","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-27T16:25:45Z","receivedAt":"2020-07-27T16:25:53Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThis will allow these tests to run with alternative ref backends\n\nSigned-off-by: Han-Wen Nienhuys \n---\n t/t1400-update-ref.sh | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 27171f8261..7414b898f8 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -475,57 +475,57 @@ test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER\n \n test_expect_success 'given old value for missing pseudoref, do not create' '\n \ttest_must_fail git update-ref PSEUDOREF $A $B 2>err &&\n-\ttest_path_is_missing .git/PSEUDOREF &&\n+\ttest_must_fail git rev-parse PSEUDOREF &&\n \ttest_i18ngrep \"could not read ref\" err\n '\n \n test_expect_success 'create pseudoref' '\n \tgit update-ref PSEUDOREF $A &&\n-\ttest $A = $(cat .git/PSEUDOREF)\n+\ttest $A = $(git rev-parse PSEUDOREF)\n '\n \n test_expect_success 'overwrite pseudoref with no old value given' '\n \tgit update-ref PSEUDOREF $B &&\n-\ttest $B = $(cat .git/PSEUDOREF)\n+\ttest $B = $(git rev-parse PSEUDOREF)\n '\n \n test_expect_success 'overwrite pseudoref with correct old value' '\n \tgit update-ref PSEUDOREF $C $B &&\n-\ttest $C = $(cat .git/PSEUDOREF)\n+\ttest $C = $(git rev-parse PSEUDOREF)\n '\n \n test_expect_success 'do not overwrite pseudoref with wrong old value' '\n \ttest_must_fail git update-ref PSEUDOREF $D $E 2>err &&\n-\ttest $C = $(cat .git/PSEUDOREF) &&\n+\ttest $C = $(git rev-parse PSEUDOREF) &&\n \ttest_i18ngrep \"unexpected object ID\" err\n '\n \n test_expect_success 'delete pseudoref' '\n \tgit update-ref -d PSEUDOREF &&\n-\ttest_path_is_missing .git/PSEUDOREF\n+\ttest_must_fail git rev-parse PSEUDOREF\n '\n \n test_expect_success 'do not delete pseudoref with wrong old value' '\n \tgit update-ref PSEUDOREF $A &&\n \ttest_must_fail git update-ref -d PSEUDOREF $B 2>err &&\n-\ttest $A = $(cat .git/PSEUDOREF) &&\n+\ttest $A = $(git rev-parse PSEUDOREF) &&\n \ttest_i18ngrep \"unexpected object ID\" err\n '\n \n test_expect_success 'delete pseudoref with correct old value' '\n \tgit update-ref -d PSEUDOREF $A &&\n-\ttest_path_is_missing .git/PSEUDOREF\n+\ttest_must_fail git rev-parse PSEUDOREF\n '\n \n test_expect_success 'create pseudoref with old OID zero' '\n \tgit update-ref PSEUDOREF $A $Z &&\n-\ttest $A = $(cat .git/PSEUDOREF)\n+\ttest $A = $(git rev-parse PSEUDOREF)\n '\n \n test_expect_success 'do not overwrite pseudoref with old OID zero' '\n \ttest_when_finished git update-ref -d PSEUDOREF &&\n \ttest_must_fail git update-ref PSEUDOREF $B $Z 2>err &&\n-\ttest $A = $(cat .git/PSEUDOREF) &&\n+\ttest $A = $(git rev-parse PSEUDOREF) &&\n \ttest_i18ngrep \"already exists\" err\n '\n \n-- \ngitgitgadget\n\n"},{"id":"402142","messageId":"ba5f1b4d260bbe1804c54bac7ecf83acbd219a7e.1595867147.git.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.v4.git.1595867147.gitgitgadget@gmail.com","subject":"[PATCH v4 2/3] Modify pseudo refs through ref backend storage","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-27T16:25:46Z","receivedAt":"2020-07-27T16:25:54Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe previous behavior was introduced in commit 74ec19d4be\n(\"pseudorefs: create and use pseudoref update and delete functions\",\nJul 31, 2015), with the justification \"alternate ref backends still\nneed to store pseudorefs in GIT_DIR\".\n\nRefs such as REBASE_HEAD are read through the ref backend. This can\nonly work consistently if they are written through the ref backend as\nwell. Tooling that works directly on files under .git should be\nupdated to use git commands to read refs instead.\n\nThe following behaviors change:\n\n* Updates to pseudorefs (eg. ORIG_HEAD) with\n core.logAllRefUpdates=always will create reflogs for the pseudoref.\n\n* non-HEAD pseudoref symrefs are also dereferenced on deletion. Update\n t1405 accordingly.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n Documentation/git-update-ref.txt | 13 ++--\n refs.c | 120 ++-----------------------------\n t/t1400-update-ref.sh | 10 +--\n t/t1405-main-ref-store.sh | 5 +-\n 4 files changed, 23 insertions(+), 125 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex 3e737c2360..d401234b03 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -148,12 +148,13 @@ still see a subset of the modifications.\n \n LOGGING UPDATES\n ---------------\n-If config parameter \"core.logAllRefUpdates\" is true and the ref is one under\n-\"refs/heads/\", \"refs/remotes/\", \"refs/notes/\", or the symbolic ref HEAD; or\n-the file \"$GIT_DIR/logs/\" exists then `git update-ref` will append\n-a line to the log file \"$GIT_DIR/logs/\" (dereferencing all\n-symbolic refs before creating the log name) describing the change\n-in ref value. Log lines are formatted as:\n+If config parameter \"core.logAllRefUpdates\" is true and the ref is one\n+under \"refs/heads/\", \"refs/remotes/\", \"refs/notes/\", or a pseudoref\n+like HEAD or ORIG_HEAD; or the file \"$GIT_DIR/logs/\" exists then\n+`git update-ref` will append a line to the log file\n+\"$GIT_DIR/logs/\" (dereferencing all symbolic refs before creating\n+the log name) describing the change in ref value. Log lines are\n+formatted as:\n \n oldsha1 SP newsha1 SP committer LF\n \ndiff --git a/refs.c b/refs.c\nindex 639cba93b4..d676acb1f4 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -771,102 +771,6 @@ long get_files_ref_lock_timeout_ms(void)\n \treturn timeout_ms;\n }\n \n-static int write_pseudoref(const char *pseudoref, const struct object_id *oid,\n-\t\t\t const struct object_id *old_oid, struct strbuf *err)\n-{\n-\tconst char *filename;\n-\tint fd;\n-\tstruct lock_file lock = LOCK_INIT;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tint ret = -1;\n-\n-\tif (!oid)\n-\t\treturn 0;\n-\n-\tstrbuf_addf(&buf, \"%s\\n\", oid_to_hex(oid));\n-\n-\tfilename = git_path(\"%s\", pseudoref);\n-\tfd = hold_lock_file_for_update_timeout(&lock, filename, 0,\n-\t\t\t\t\t get_files_ref_lock_timeout_ms());\n-\tif (fd < 0) {\n-\t\tstrbuf_addf(err, _(\"could not open '%s' for writing: %s\"),\n-\t\t\t filename, strerror(errno));\n-\t\tgoto done;\n-\t}\n-\n-\tif (old_oid) {\n-\t\tstruct object_id actual_old_oid;\n-\n-\t\tif (read_ref(pseudoref, &actual_old_oid)) {\n-\t\t\tif (!is_null_oid(old_oid)) {\n-\t\t\t\tstrbuf_addf(err, _(\"could not read ref '%s'\"),\n-\t\t\t\t\t pseudoref);\n-\t\t\t\trollback_lock_file(&lock);\n-\t\t\t\tgoto done;\n-\t\t\t}\n-\t\t} else if (is_null_oid(old_oid)) {\n-\t\t\tstrbuf_addf(err, _(\"ref '%s' already exists\"),\n-\t\t\t\t pseudoref);\n-\t\t\trollback_lock_file(&lock);\n-\t\t\tgoto done;\n-\t\t} else if (!oideq(&actual_old_oid, old_oid)) {\n-\t\t\tstrbuf_addf(err, _(\"unexpected object ID when writing '%s'\"),\n-\t\t\t\t pseudoref);\n-\t\t\trollback_lock_file(&lock);\n-\t\t\tgoto done;\n-\t\t}\n-\t}\n-\n-\tif (write_in_full(fd, buf.buf, buf.len) < 0) {\n-\t\tstrbuf_addf(err, _(\"could not write to '%s'\"), filename);\n-\t\trollback_lock_file(&lock);\n-\t\tgoto done;\n-\t}\n-\n-\tcommit_lock_file(&lock);\n-\tret = 0;\n-done:\n-\tstrbuf_release(&buf);\n-\treturn ret;\n-}\n-\n-static int delete_pseudoref(const char *pseudoref, const struct object_id *old_oid)\n-{\n-\tconst char *filename;\n-\n-\tfilename = git_path(\"%s\", pseudoref);\n-\n-\tif (old_oid && !is_null_oid(old_oid)) {\n-\t\tstruct lock_file lock = LOCK_INIT;\n-\t\tint fd;\n-\t\tstruct object_id actual_old_oid;\n-\n-\t\tfd = hold_lock_file_for_update_timeout(\n-\t\t\t\t&lock, filename, 0,\n-\t\t\t\tget_files_ref_lock_timeout_ms());\n-\t\tif (fd < 0) {\n-\t\t\terror_errno(_(\"could not open '%s' for writing\"),\n-\t\t\t\t filename);\n-\t\t\treturn -1;\n-\t\t}\n-\t\tif (read_ref(pseudoref, &actual_old_oid))\n-\t\t\tdie(_(\"could not read ref '%s'\"), pseudoref);\n-\t\tif (!oideq(&actual_old_oid, old_oid)) {\n-\t\t\terror(_(\"unexpected object ID when deleting '%s'\"),\n-\t\t\t pseudoref);\n-\t\t\trollback_lock_file(&lock);\n-\t\t\treturn -1;\n-\t\t}\n-\n-\t\tunlink(filename);\n-\t\trollback_lock_file(&lock);\n-\t} else {\n-\t\tunlink(filename);\n-\t}\n-\n-\treturn 0;\n-}\n-\n int refs_delete_ref(struct ref_store *refs, const char *msg,\n \t\t const char *refname,\n \t\t const struct object_id *old_oid,\n@@ -875,11 +779,6 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n-\tif (ref_type(refname) == REF_TYPE_PSEUDOREF) {\n-\t\tassert(refs == get_main_ref_store(the_repository));\n-\t\treturn delete_pseudoref(refname, old_oid);\n-\t}\n-\n \ttransaction = ref_store_transaction_begin(refs, &err);\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, old_oid,\n@@ -1202,18 +1101,13 @@ int refs_update_ref(struct ref_store *refs, const char *msg,\n \tstruct strbuf err = STRBUF_INIT;\n \tint ret = 0;\n \n-\tif (ref_type(refname) == REF_TYPE_PSEUDOREF) {\n-\t\tassert(refs == get_main_ref_store(the_repository));\n-\t\tret = write_pseudoref(refname, new_oid, old_oid, &err);\n-\t} else {\n-\t\tt = ref_store_transaction_begin(refs, &err);\n-\t\tif (!t ||\n-\t\t ref_transaction_update(t, refname, new_oid, old_oid,\n-\t\t\t\t\t flags, msg, &err) ||\n-\t\t ref_transaction_commit(t, &err)) {\n-\t\t\tret = 1;\n-\t\t\tref_transaction_free(t);\n-\t\t}\n+\tt = ref_store_transaction_begin(refs, &err);\n+\tif (!t ||\n+\t ref_transaction_update(t, refname, new_oid, old_oid, flags, msg,\n+\t\t\t\t &err) ||\n+\t ref_transaction_commit(t, &err)) {\n+\t\tret = 1;\n+\t\tref_transaction_free(t);\n \t}\n \tif (ret) {\n \t\tconst char *str = _(\"update_ref failed for ref '%s': %s\");\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 7414b898f8..65d349fb33 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -160,10 +160,10 @@ test_expect_success 'core.logAllRefUpdates=always creates reflog by default' '\n \tgit reflog exists $outside\n '\n \n-test_expect_success 'core.logAllRefUpdates=always creates no reflog for ORIG_HEAD' '\n+test_expect_success 'core.logAllRefUpdates=always creates reflog for ORIG_HEAD' '\n \ttest_config core.logAllRefUpdates always &&\n \tgit update-ref ORIG_HEAD $A &&\n-\ttest_must_fail git reflog exists ORIG_HEAD\n+\tgit reflog exists ORIG_HEAD\n '\n \n test_expect_success '--no-create-reflog overrides core.logAllRefUpdates=always' '\n@@ -476,7 +476,7 @@ test_expect_success 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER\n test_expect_success 'given old value for missing pseudoref, do not create' '\n \ttest_must_fail git update-ref PSEUDOREF $A $B 2>err &&\n \ttest_must_fail git rev-parse PSEUDOREF &&\n-\ttest_i18ngrep \"could not read ref\" err\n+\ttest_i18ngrep \"unable to resolve reference\" err\n '\n \n test_expect_success 'create pseudoref' '\n@@ -497,7 +497,7 @@ test_expect_success 'overwrite pseudoref with correct old value' '\n test_expect_success 'do not overwrite pseudoref with wrong old value' '\n \ttest_must_fail git update-ref PSEUDOREF $D $E 2>err &&\n \ttest $C = $(git rev-parse PSEUDOREF) &&\n-\ttest_i18ngrep \"unexpected object ID\" err\n+\ttest_i18ngrep \"cannot lock ref.*expected\" err\n '\n \n test_expect_success 'delete pseudoref' '\n@@ -509,7 +509,7 @@ test_expect_success 'do not delete pseudoref with wrong old value' '\n \tgit update-ref PSEUDOREF $A &&\n \ttest_must_fail git update-ref -d PSEUDOREF $B 2>err &&\n \ttest $A = $(git rev-parse PSEUDOREF) &&\n-\ttest_i18ngrep \"unexpected object ID\" err\n+\ttest_i18ngrep \"cannot lock ref.*expected\" err\n '\n \n test_expect_success 'delete pseudoref with correct old value' '\ndiff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh\nindex 331899ddc4..74af927fba 100755\n--- a/t/t1405-main-ref-store.sh\n+++ b/t/t1405-main-ref-store.sh\n@@ -31,7 +31,10 @@ test_expect_success 'create_symref(FOO, refs/heads/master)' '\n test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '\n \tgit rev-parse FOO -- &&\n \tgit rev-parse refs/tags/new-tag -- &&\n-\t$RUN delete-refs 0 nothing FOO refs/tags/new-tag &&\n+\tm=$(git rev-parse master) &&\n+\tREF_NO_DEREF=1 &&\n+\t$RUN delete-refs $REF_NO_DEREF nothing FOO refs/tags/new-tag &&\n+\ttest_must_fail git rev-parse --symbolic-full-name FOO &&\n \ttest_must_fail git rev-parse FOO -- &&\n \ttest_must_fail git rev-parse refs/tags/new-tag --\n '\n-- \ngitgitgadget\n\n"},{"id":"402141","messageId":"358d34df0769eff54bc293f473d6e4dedd912563.1595867147.git.gitgitgadget@gmail.com","threadId":"53801","inReplyTo":"pull.673.v4.git.1595867147.gitgitgadget@gmail.com","subject":"[PATCH v4 3/3] Make HEAD a PSEUDOREF rather than PER_WORKTREE.","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-07-27T16:25:47Z","receivedAt":"2020-07-27T16:25:56Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThis is consistent with the definition of REF_TYPE_PSEUDOREF\n(uppercase in the root ref namespace).\n\nSigned-off-by: Han-Wen Nienhuys \n---\n refs.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex d676acb1f4..5cd71f711f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -708,10 +708,9 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)\n \n static int is_per_worktree_ref(const char *refname)\n {\n-\treturn !strcmp(refname, \"HEAD\") ||\n-\t\tstarts_with(refname, \"refs/worktree/\") ||\n-\t\tstarts_with(refname, \"refs/bisect/\") ||\n-\t\tstarts_with(refname, \"refs/rewritten/\");\n+\treturn starts_with(refname, \"refs/worktree/\") ||\n+\t starts_with(refname, \"refs/bisect/\") ||\n+\t starts_with(refname, \"refs/rewritten/\");\n }\n \n static int is_pseudoref_syntax(const char *refname)\n-- \ngitgitgadget\n"},{"id":"402739","messageId":"CAFQ2z_O1sRm-_SNP=-GvgNLqB+qgf6k9YVfbF1XCAmFWdeX6Ew@mail.gmail.com","threadId":"53801","inReplyTo":"xmqqpn8hgca7.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2020-08-03T19:07:18Z","receivedAt":"2020-08-03T19:07:33Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Mon, Jul 27, 2020 at 6:20 PM Junio C Hamano wrote:\n> > On Fri, Jul 17, 2020 at 12:10 AM Junio C Hamano wrote:\n> >> I reviewed some codepaths that deal with FETCH_HEAD recently.\n> >>\n> >> As the file is quite different from all the other pseudo references\n> >> in that it needs to store more than one object name and in that each\n> >> ref in it needs more than just the object name, I doubt that it\n> >> makes much sense to enhance the refs API so that its requirements\n> >> can be covered.\n> >\n> > I agree. Do we ever pretend that FETCH_HEAD is a ref today?\n>\n> \"git rev-parse FETCH_HEAD\", \"git show FETCH_HEAD\" etc. all should keep\n> working, so in that sense, it is treated as a ref.\n\nI added this to the last version of the full reftable patch series\nthat I posted, as patches\n\"Split off reading loose ref data in separate function\" and \"Read\nFETCH_HEAD as loose ref\".\n\nWhich other refs that aren't really refs should also be supported? The\nJGit source code suggests that MERGE_HEAD should also be special\ncased?\n\nIt's not pretty, though. The entry point is read_raw_ref(), which gets\na ref_store as argument. ref_store doesn't have generic API to get at\nthe repository directory, so the implementation of reading the\nFETCH_HEAD file has to be pushed down to the ref backend (which does\nknow the directory).\n\n> It does not\n> protect the history leading to the objects listed in it from being\n> collected, though.\n\nIs there documented agreement of how pseudo refs and GC should interact?\n\n> \"git merge FETCH_HEAD\" is an interesting case---I haven't thought it\n> through.\n>\n> What should happen after \"git pull origin foo bar\" attempts to grab\n> two branches and make an octopus merge into the branch currently\n> checked out, and then \"git reset --hard && git merge FETCH_HEAD\" is\n> given?\n\nI don't understand this question.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\nRegistergericht und -nummer: Hamburg, HRB 86891\nSitz der Gesellschaft: Hamburg\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"402747","messageId":"xmqqh7tjv6hb.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"CAFQ2z_O1sRm-_SNP=-GvgNLqB+qgf6k9YVfbF1XCAmFWdeX6Ew@mail.gmail.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-03T20:07:12Z","receivedAt":"2020-08-03T20:07:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n>> >> As the file is quite different from all the other pseudo references\n>> >> in that it needs to store more than one object name and in that each\n>> >> ref in it needs more than just the object name, I doubt that it\n>> >> makes much sense to enhance the refs API so that its requirements\n>> >> can be covered.\n>> >\n>> > I agree. Do we ever pretend that FETCH_HEAD is a ref today?\n>>\n>> \"git rev-parse FETCH_HEAD\", \"git show FETCH_HEAD\" etc. all should keep\n>> working, so in that sense, it is treated as a ref.\n>\n> I added this to the last version of the full reftable patch series\n> that I posted, as patches\n> \"Split off reading loose ref data in separate function\" and \"Read\n> FETCH_HEAD as loose ref\".\n>\n> Which other refs that aren't really refs should also be supported? The\n> JGit source code suggests that MERGE_HEAD should also be special\n> cased?\n\nI'd think all .git/${SOMETHING}_HEAD are of transitory nature that\ncan be left as simple on-disk files that are read (and preferrably\nwritten---except for FETCH_HEAD for obvious reasons) as if they are\nloose refs handled by files backend. It probably makes sense not to\neven write reflog entries for them---it is not like the MERGE_HEAD\nI see now in .git/ directory is an updated version of MERGE_HEAD I\nhad there yesterday. \"git log -g MERGE_HEAD\" gives no interesting\ninformation.\n\n>> \"git merge FETCH_HEAD\" is an interesting case---I haven't thought it\n>> through.\n>>\n>> What should happen after \"git pull origin foo bar\" attempts to grab\n>> two branches and make an octopus merge into the branch currently\n>> checked out, and then \"git reset --hard && git merge FETCH_HEAD\" is\n>> given?\n>\n> I don't understand this question.\n\nThe request \"git pull origin foo bar\" is \"grab the tip of 'foo' and\n'bar' branches from remote whose name is 'origin'; merge these two\ncommits into the commit that I have checked out\".\n\nTo fulfill the request, \"git pull\" runs \"git fetch\", and the latter\nleaves two lines of interest in .git/FETCH_HEAD file. Each line\nlists the name of the object at the ref, an optional \"not-for-merge\"\ntoken (which in this case does not exist, as both records are for\nmerging), and piece of human-readable text to describe where that\nobject came from to help later step that computes the default\nmessage for the resulting merge commit.\n\nIf the octopus merge does not finish correctly (e.g. due to\nconflicts), with \"git reset --hard\", we can recover to the original\nstate and re-attempt the opeation with \"git merge FETCH_HEAD\". Such\na merge using FETCH_HEAD will produce an octopus merge.\n\nWhich means that at least \"git merge\", FETCH_HEAD is not just a\nregular ref where you can ask what object it points at and it gives\nyou a single object name back.\n\nBut to other commands like \"git log master..FETCH_HEAD\", it acts as\nif there is only one object recorded. That makes it an interesting\ncase. Making it act as if \"git log ^master origin/foo origin/bar\"\nwere given might be a \"bugfix\" to make it behave more \"correctly\",\nbut I do not know how large a fallout such a change brings in.\n\n\n\n"},{"id":"402891","messageId":"xmqq1rklrhle.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"xmqqh7tjv6hb.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-05T01:45:17Z","receivedAt":"2020-08-05T01:45:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> Which other refs that aren't really refs should also be supported? The\n>> JGit source code suggests that MERGE_HEAD should also be special\n>> cased?\n>\n> I'd think all .git/${SOMETHING}_HEAD are of transitory nature that\n> can be left as simple on-disk files that are read (and preferrably\n> written---except for FETCH_HEAD for obvious reasons) as if they are\n> loose refs handled by files backend.\n\nSorry for flipping and flopping. The above goes directly against\nthe spirit of 09743417 (Modify pseudo refs through ref backend\nstorage, 2020-07-27). I still think .git/${SOMETHING}_HEAD except\nfor FETCH_HEAD should be written and read via the ref subsystem, but\nI was wrong to say that it should always be done via the files\nbackend. There is no reason to insist on the use of files backend\nhere.\n\n> It probably makes sense not to\n> even write reflog entries for them---it is not like the MERGE_HEAD\n> I see now in .git/ directory is an updated version of MERGE_HEAD I\n> had there yesterday. \"git log -g MERGE_HEAD\" gives no interesting\n> information.\n\nThis still is true, but that is pretty much orthogonal to which\nbackend is used.\n\n> If the octopus merge does not finish correctly (e.g. due to\n> conflicts), with \"git reset --hard\", we can recover to the original\n> state and re-attempt the opeation with \"git merge FETCH_HEAD\". Such\n> a merge using FETCH_HEAD will produce an octopus merge.\n>\n> Which means that at least \"git merge\", FETCH_HEAD is not just a\n> regular ref where you can ask what object it points at and it gives\n> you a single object name back.\n>\n> But to other commands like \"git log master..FETCH_HEAD\", it acts as\n> if there is only one object recorded.\n\nAll of which means FETCH_HEAD is special and we may not want to\nburden the special casing of it to newer backends.\n"},{"id":"402907","messageId":"CAFQ2z_NU4=WcUDvVY9PNs+p-WU4u5=QY+=DHonGyK29numhWoQ@mail.gmail.com","threadId":"53801","inReplyTo":"xmqq1rklrhle.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2020-08-05T10:48:21Z","receivedAt":"2020-08-05T11:23:12Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Wed, Aug 5, 2020 at 3:45 AM Junio C Hamano wrote:\n> > I'd think all .git/${SOMETHING}_HEAD are of transitory nature that\n> > can be left as simple on-disk files that are read (and preferrably\n> > written---except for FETCH_HEAD for obvious reasons) as if they are\n> > loose refs handled by files backend.\n>\n> Sorry for flipping and flopping. The above goes directly against\n> the spirit of 09743417 (Modify pseudo refs through ref backend\n> storage, 2020-07-27). I still think .git/${SOMETHING}_HEAD except\n> for FETCH_HEAD should be written and read via the ref subsystem, but\n> I was wrong to say that it should always be done via the files\n> backend. There is no reason to insist on the use of files backend\n> here.\n\nYes, that confused me. Thanks for setting that straight.\n\n> > If the octopus merge does not finish correctly (e.g. due to\n> > conflicts), with \"git reset --hard\", we can recover to the original\n> > state and re-attempt the opeation with \"git merge FETCH_HEAD\". Such\n> > a merge using FETCH_HEAD will produce an octopus merge.\n> >\n> > Which means that at least \"git merge\", FETCH_HEAD is not just a\n> > regular ref where you can ask what object it points at and it gives\n> > you a single object name back.\n> >\n> > But to other commands like \"git log master..FETCH_HEAD\", it acts as\n> > if there is only one object recorded.\n>\n> All of which means FETCH_HEAD is special and we may not want to\n> burden the special casing of it to newer backends.\n\nCan you confirm that FETCH_HEAD is the only thing that can store more\nthan just a symref / SHA1 ? Based on the name, and a comment in the\nJGit source, I thought that MERGE_HEAD might contain more than one\nSHA1 at a time.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"402952","messageId":"xmqqk0ydozqe.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"CAFQ2z_NU4=WcUDvVY9PNs+p-WU4u5=QY+=DHonGyK29numhWoQ@mail.gmail.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-05T15:54:01Z","receivedAt":"2020-08-05T20:07:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n>> All of which means FETCH_HEAD is special and we may not want to\n>> burden the special casing of it to newer backends.\n>\n> Can you confirm that FETCH_HEAD is the only thing that can store more\n> than just a symref / SHA1 ? Based on the name, and a comment in the\n> JGit source, I thought that MERGE_HEAD might contain more than one\n> SHA1 at a time.\n\nNo I cannot. I do not think MERGE_HEAD is something I added with as\ndeep a thought as I did with FETCH_HEAD. If it mimics FETCH_HEAD,\nthen perhaps it does, but I somehow doubt it.\n\nAs can be seen in builtin/merge.c::collect_parents(), we do special\ncase FETCH_HEAD when grabbing what commit*S* to merge, but all\nothers are read with get_merge_parent() that makes a single call to\nget_oid(), i.e. anything other than FETCH_HEAD cannot have more than\none object recorded.\n"},{"id":"403264","messageId":"CAFQ2z_OgNQKZ23gNB9=LqPn2M=WKL2vPQQpxm3drRWeN1sxH_A@mail.gmail.com","threadId":"53801","inReplyTo":"xmqqk0ydozqe.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2020-08-10T14:27:08Z","receivedAt":"2020-08-10T14:27:26Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Wed, Aug 5, 2020 at 5:54 PM Junio C Hamano wrote:\n> >> All of which means FETCH_HEAD is special and we may not want to\n> >> burden the special casing of it to newer backends.\n> >\n> > Can you confirm that FETCH_HEAD is the only thing that can store more\n> > than just a symref / SHA1 ? Based on the name, and a comment in the\n> > JGit source, I thought that MERGE_HEAD might contain more than one\n> > SHA1 at a time.\n>\n> No I cannot. I do not think MERGE_HEAD is something I added with as\n> deep a thought as I did with FETCH_HEAD. If it mimics FETCH_HEAD,\n> then perhaps it does, but I somehow doubt it.\n>\n> As can be seen in builtin/merge.c::collect_parents(), we do special\n> case FETCH_HEAD when grabbing what commit*S* to merge, but all\n> others are read with get_merge_parent() that makes a single call to\n> get_oid(), i.e. anything other than FETCH_HEAD cannot have more than\n> one object recorded.\n\nUnderstood. Is there anything you'd like me to do with this patch\nseries so it can be merged?\n\nDealing with FETCH_HEAD generically isn't possible unless we extend\nthe API of the ref backend: the generic ref_store instance doesn't\noffer a way to get at the path that corresponds to FETCH_HEAD, so we\ncan't handle it in refs_read_raw_ref(). In the current reftable\nseries, FETCH_HEAD is dealt with in the backends separately.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"403271","messageId":"xmqqk0y6cwst.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"CAFQ2z_OgNQKZ23gNB9=LqPn2M=WKL2vPQQpxm3drRWeN1sxH_A@mail.gmail.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-10T16:04:02Z","receivedAt":"2020-08-10T16:04:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n> Dealing with FETCH_HEAD generically isn't possible unless we extend\n> the API of the ref backend: the generic ref_store instance doesn't\n> offer a way to get at the path that corresponds to FETCH_HEAD, so we\n> can't handle it in refs_read_raw_ref(). In the current reftable\n> series, FETCH_HEAD is dealt with in the backends separately.\n\nI am not sure what the best way would be, but I do not think any\nexisting code writes into it using the refs API at all, even though\nit may be read only for the first object name using the refs API.\n\nAnd I am not sure if we want to extend the write side API so that\nthe callers can express the full flexibility of that single file.\n\nSo perhaps the best way forward would be to ensure that anybody who\ntries to read from FETCH_HEAD using the ref API reads the first\nobject name in it from $GIT_DIR/FETCH_HEAD file as we've always done\nsince the beginning of time, regardless of what ref backend is used,\nthat anybody who tries to write FETCH_HEAD using the ref API gets an\nerror, and letting the writers write into it directly?\n\n\n\n"},{"id":"403344","messageId":"CAFQ2z_NMH75mewQyvN=NnjYR059ut3jfJLH_GPuiMA0ehyzBrQ@mail.gmail.com","threadId":"53801","inReplyTo":"xmqqk0y6cwst.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2020-08-11T10:49:59Z","receivedAt":"2020-08-11T10:50:15Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Mon, Aug 10, 2020 at 6:04 PM Junio C Hamano wrote:\n>\n> Han-Wen Nienhuys writes:\n>\n> > Dealing with FETCH_HEAD generically isn't possible unless we extend\n> > the API of the ref backend: the generic ref_store instance doesn't\n> > offer a way to get at the path that corresponds to FETCH_HEAD, so we\n> > can't handle it in refs_read_raw_ref(). In the current reftable\n> > series, FETCH_HEAD is dealt with in the backends separately.\n>\n> I am not sure what the best way would be, but I do not think any\n> existing code writes into it using the refs API at all, even though\n> it may be read only for the first object name using the refs API.\n>\n> And I am not sure if we want to extend the write side API so that\n> the callers can express the full flexibility of that single file.\n\nThat's not what I am getting at. I am just interested in how to handle\nFETCH_HEAD in refs_read_raw_ref.\n\n> So perhaps the best way forward would be to ensure that anybody who\n> tries to read from FETCH_HEAD using the ref API reads the first\n> object name in it from $GIT_DIR/FETCH_HEAD file as we've always done\n> since the beginning of time, regardless of what ref backend is used,\n\nRight, but how do we get at the value of $GIT_DIR given a struct\nref_store? We can either push that out to the ref store backend,\nbecause each backend knows what $GIT_DIR is, or we can make $GIT_DIR a\nproperty of the generic ref_store.\n\nI suppose it's cleaner to make the latter API extension.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"403373","messageId":"xmqq5z9pav01.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"CAFQ2z_NMH75mewQyvN=NnjYR059ut3jfJLH_GPuiMA0ehyzBrQ@mail.gmail.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-11T18:38:06Z","receivedAt":"2020-08-11T18:38:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n> On Mon, Aug 10, 2020 at 6:04 PM Junio C Hamano wrote:\n>>\n>> Han-Wen Nienhuys writes:\n>>\n>> > Dealing with FETCH_HEAD generically isn't possible unless we extend\n>> > the API of the ref backend: the generic ref_store instance doesn't\n>> > offer a way to get at the path that corresponds to FETCH_HEAD, so we\n>> > can't handle it in refs_read_raw_ref(). In the current reftable\n>> > series, FETCH_HEAD is dealt with in the backends separately.\n>>\n>> I am not sure what the best way would be, but I do not think any\n>> existing code writes into it using the refs API at all, even though\n>> it may be read only for the first object name using the refs API.\n>>\n>> And I am not sure if we want to extend the write side API so that\n>> the callers can express the full flexibility of that single file.\n>\n> That's not what I am getting at. I am just interested in how to handle\n> FETCH_HEAD in refs_read_raw_ref.\n>\n>> So perhaps the best way forward would be to ensure that anybody who\n>> tries to read from FETCH_HEAD using the ref API reads the first\n>> object name in it from $GIT_DIR/FETCH_HEAD file as we've always done\n>> since the beginning of time, regardless of what ref backend is used,\n>\n> Right, but how do we get at the value of $GIT_DIR given a struct\n> ref_store? We can either push that out to the ref store backend,\n> because each backend knows what $GIT_DIR is, or we can make $GIT_DIR a\n> property of the generic ref_store.\n>\n> I suppose it's cleaner to make the latter API extension.\n\nYup. Sounds like the right direction to go.\n\nThanks.\n"},{"id":"404006","messageId":"CAFQ2z_No3QPJju7N9EqnT+knOioJpxxi61oFf9HyRGgf-fuwpA@mail.gmail.com","threadId":"53801","inReplyTo":"xmqqk0ydozqe.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2020-08-19T13:19:51Z","receivedAt":"2020-08-19T13:22:17Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Wed, Aug 5, 2020 at 5:54 PM Junio C Hamano wrote:\n> >> All of which means FETCH_HEAD is special and we may not want to\n> >> burden the special casing of it to newer backends.\n> >\n> > Can you confirm that FETCH_HEAD is the only thing that can store more\n> > than just a symref / SHA1 ? Based on the name, and a comment in the\n> > JGit source, I thought that MERGE_HEAD might contain more than one\n> > SHA1 at a time.\n>\n> No I cannot. I do not think MERGE_HEAD is something I added with as\n> deep a thought as I did with FETCH_HEAD. If it mimics FETCH_HEAD,\n> then perhaps it does, but I somehow doubt it.\n\nblame.c has an append_merge_parents() which reads multiple lines from\nMERGE_HEAD, and cmd_commit does so too. It's populated from\nwrite_merge_heads(). So the special casing should be extended to\nMERGE_HEAD.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\nRegistergericht und -nummer: Hamburg, HRB 86891\nSitz der Gesellschaft: Hamburg\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"404029","messageId":"xmqqmu2qvdik.fsf@gitster.c.googlers.com","threadId":"53801","inReplyTo":"CAFQ2z_No3QPJju7N9EqnT+knOioJpxxi61oFf9HyRGgf-fuwpA@mail.gmail.com","subject":"Re: [PATCH v3 0/3] Remove special casing for PSEUDOREF updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-19T15:52:51Z","receivedAt":"2020-08-19T15:53:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n> blame.c has an append_merge_parents() which reads multiple lines from\n> MERGE_HEAD, and cmd_commit does so too. It's populated from\n> write_merge_heads(). So the special casing should be extended to\n> MERGE_HEAD.\n\nAhh. That makes sense.\n"}]}