{"thread":{"id":"60766","subject":"[PATCH 0/7] refs: convert special refs to become normal pseudo-refs","startedAt":"2024-01-19T10:39:59Z","lastAt":"2024-01-24T09:05:53Z","messageCount":23,"participants":["Patrick Steinhardt","Junio C Hamano","Karthik Nayak","Phillip Wood","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"487058","messageId":"cover.1705659748.git.ps@pks.im","threadId":"60766","inReplyTo":null,"subject":"[PATCH 0/7] refs: convert special refs to become normal pseudo-refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-19T10:39:53Z","receivedAt":"2024-01-19T10:39:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series is a follow-up to the one that introduced\n`is_special_ref()` [1]. Back then, we agreed that the list of special\nrefs ought to shrink over time [2] so that only the actually-special\nrefs FETCH_HEAD and MERGE_HEAD remain, which is exactly what I'm doing\nnow.\n\nPatrick\n\n[1]: https://public-inbox.org/git/cover.1701243201.git.ps@pks.im/\n[2]: https://public-inbox.org/git/ZXlfeWtDgr1GQFCL@tanuki/\n\nPatrick Steinhardt (7):\n  sequencer: clean up pseudo refs with REF_NO_DEREF\n  sequencer: delete REBASE_HEAD in correct repo when picking commits\n  refs: convert AUTO_MERGE to become a normal pseudo-ref\n  sequencer: introduce functions to handle autostashes via refs\n  refs: convert MERGE_AUTOSTASH to become a normal pseudo-ref\n  refs: redefine special refs\n  Documentation: add \"special refs\" to the glossary\n\n Documentation/glossary-content.txt | 14 +++++\n branch.c                           |  5 +-\n builtin/commit.c                   |  2 +-\n builtin/merge.c                    | 27 ++++-----\n builtin/rebase.c                   |  2 +-\n merge-ort.c                        | 19 +++---\n path.c                             |  2 -\n path.h                             |  2 -\n refs.c                             | 35 +++--------\n repository.c                       |  2 -\n repository.h                       |  2 -\n sequencer.c                        | 95 ++++++++++++++++++++++++------\n sequencer.h                        |  3 +\n 13 files changed, 132 insertions(+), 78 deletions(-)\n\n\nbase-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5\n-- \n2.43.GIT\n\n"},{"id":"487059","messageId":"48b95fe954c1dbdd080ce7a0cc871f4850bddeae.1705659748.git.ps@pks.im","threadId":"60766","inReplyTo":"cover.1705659748.git.ps@pks.im","subject":"[PATCH 1/7] sequencer: clean up pseudo refs with REF_NO_DEREF","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-19T10:39:59Z","receivedAt":"2024-01-19T10:40:02Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When cleaning up the state-tracking pseudorefs CHERRY_PICK_HEAD or\nREVERT_HEAD we do not set REF_NO_DEREF. In the unlikely case where those\nrefs are a symref we would thus end up deleting the symref targets, and\nnot the symrefs themselves.\n\nHarden the code to use REF_NO_DEREF to fix this.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n sequencer.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 3cc88d8a80..b9cbc290ea 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -474,7 +474,7 @@ static void print_advice(struct repository *r, int show_hint,\n \t\t * of the commit itself so remove CHERRY_PICK_HEAD\n \t\t */\n \t\trefs_delete_ref(get_main_ref_store(r), \"\", \"CHERRY_PICK_HEAD\",\n-\t\t\t\tNULL, 0);\n+\t\t\t\tNULL, REF_NO_DEREF);\n \t\treturn;\n \t}\n \n@@ -1667,7 +1667,7 @@ static int do_commit(struct repository *r,\n \t\tstrbuf_release(&sb);\n \t\tif (!res) {\n \t\t\trefs_delete_ref(get_main_ref_store(r), \"\",\n-\t\t\t\t\t\"CHERRY_PICK_HEAD\", NULL, 0);\n+\t\t\t\t\t\"CHERRY_PICK_HEAD\", NULL, REF_NO_DEREF);\n \t\t\tunlink(git_path_merge_msg(r));\n \t\t\tif (!is_rebase_i(opts))\n \t\t\t\tprint_commit_summary(r, NULL, &oid,\n@@ -2406,7 +2406,7 @@ static int do_pick_commit(struct repository *r,\n \t} else if (allow == 2) {\n \t\tdrop_commit = 1;\n \t\trefs_delete_ref(get_main_ref_store(r), \"\", \"CHERRY_PICK_HEAD\",\n-\t\t\t\tNULL, 0);\n+\t\t\t\tNULL, REF_NO_DEREF);\n \t\tunlink(git_path_merge_msg(r));\n \t\tunlink(git_path_auto_merge(r));\n \t\tfprintf(stderr,\n@@ -2802,7 +2802,7 @@ void sequencer_post_commit_cleanup(struct repository *r, int verbose)\n \n \tif (refs_ref_exists(get_main_ref_store(r), \"CHERRY_PICK_HEAD\")) {\n \t\tif (!refs_delete_ref(get_main_ref_store(r), \"\",\n-\t\t\t\t     \"CHERRY_PICK_HEAD\", NULL, 0) &&\n+\t\t\t\t     \"CHERRY_PICK_HEAD\", NULL, REF_NO_DEREF) &&\n \t\t    verbose)\n \t\t\twarning(_(\"cancelling a cherry picking in progress\"));\n \t\topts.action = REPLAY_PICK;\n@@ -2811,7 +2811,7 @@ void sequencer_post_commit_cleanup(struct repository *r, int verbose)\n \n \tif (refs_ref_exists(get_main_ref_store(r), \"REVERT_HEAD\")) {\n \t\tif (!refs_delete_ref(get_main_ref_store(r), \"\", \"REVERT_HEAD\",\n-\t\t\t\t     NULL, 0) &&\n+\t\t\t\t     NULL, REF_NO_DEREF) &&\n \t\t    verbose)\n \t\t\twarning(_(\"cancelling a revert in progress\"));\n \t\topts.action = REPLAY_REVERT;\n@@ -4116,7 +4116,7 @@ static int do_merge(struct repository *r,\n \n \t\tstrbuf_release(&ref_name);\n \t\trefs_delete_ref(get_main_ref_store(r), \"\", \"CHERRY_PICK_HEAD\",\n-\t\t\t\tNULL, 0);\n+\t\t\t\tNULL, REF_NO_DEREF);\n \t\trollback_lock_file(&lock);\n \n \t\tret = run_command(&cmd);\n@@ -5108,7 +5108,7 @@ static int commit_staged_changes(struct repository *r,\n \t\tif (refs_ref_exists(get_main_ref_store(r),\n \t\t\t\t    \"CHERRY_PICK_HEAD\") &&\n \t\t    refs_delete_ref(get_main_ref_store(r), \"\",\n-\t\t\t\t    \"CHERRY_PICK_HEAD\", NULL, 0))\n+\t\t\t\t    \"CHERRY_PICK_HEAD\", NULL, REF_NO_DEREF))\n \t\t\treturn error(_(\"could not remove CHERRY_PICK_HEAD\"));\n \t\tif (unlink(git_path_merge_msg(r)) && errno != ENOENT)\n \t\t\treturn error_errno(_(\"could not remove '%s'\"),\n-- \n2.43.GIT\n\n"},{"id":"487060","messageId":"6f827be3c20a9b06b933509f9b5e0c498a7c3dc9.1705659748.git.ps@pks.im","threadId":"60766","inReplyTo":"cover.1705659748.git.ps@pks.im","subject":"[PATCH 2/7] sequencer: delete REBASE_HEAD in correct repo when picking commits","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-19T10:40:04Z","receivedAt":"2024-01-19T10:40:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When picking commits, we delete some state before executing the next\nsequencer action on interactive rebases. But while we use the correct\nrepository to calculate paths to state files that need deletion, we use\nthe repo-less `delete_ref()` function to delete REBASE_HEAD. Thus, if\nthe sequencer ran in a different repository than `the_repository`, we\nwould end up deleting the ref in the wrong repository.\n\nFix this by using `refs_delete_ref()` instead.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n sequencer.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex b9cbc290ea..6f620f5717 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -4767,7 +4767,8 @@ static int pick_commits(struct repository *r,\n \t\t\tunlink(rebase_path_author_script());\n \t\t\tunlink(git_path_merge_head(r));\n \t\t\tunlink(git_path_auto_merge(r));\n-\t\t\tdelete_ref(NULL, \"REBASE_HEAD\", NULL, REF_NO_DEREF);\n+\t\t\trefs_delete_ref(get_main_ref_store(r), \"\", \"REBASE_HEAD\",\n+\t\t\t\t\tNULL, REF_NO_DEREF);\n \n \t\t\tif (item->command == TODO_BREAK) {\n \t\t\t\tif (!opts->verbose)\n-- \n2.43.GIT\n\n"},{"id":"487061","messageId":"ae2df6316f79e372c51d59666d685e59981d2f98.1705659748.git.ps@pks.im","threadId":"60766","inReplyTo":"cover.1705659748.git.ps@pks.im","subject":"[PATCH 3/7] refs: convert AUTO_MERGE to become a normal pseudo-ref","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-19T10:40:09Z","receivedAt":"2024-01-19T10:40:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In 70c70de616 (refs: complete list of special refs, 2023-12-14) we have\ninrtoduced a new `is_special_ref()` function that classifies some refs\nas being special. The rule is that special refs are exclusively read and\nwritten via the filesystem directly, whereas normal refs exclucsively go\nvia the refs API.\n\nThe intent of that commit was to record the status quo so that we know\nto route reads of such special refs consistently. Eventually, the list\nshould be reduced to its bare minimum of refs which really are special,\nnamely FETCH_HEAD and MERGE_HEAD.\n\nFollow up on this promise and convert the AUTO_MERGE ref to become a\nnormal pseudo-ref by using the refs API to both read and write it\ninstead of accessing the filesystem directly.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n branch.c         |  3 ++-\n builtin/rebase.c |  2 +-\n merge-ort.c      | 19 ++++++++++++-------\n path.c           |  1 -\n path.h           |  1 -\n refs.c           |  1 -\n repository.c     |  1 -\n repository.h     |  1 -\n sequencer.c      | 12 ++++++++----\n 9 files changed, 23 insertions(+), 18 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 534594f7f8..c8bd9519e6 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -817,7 +817,8 @@ void remove_merge_branch_state(struct repository *r)\n \tunlink(git_path_merge_rr(r));\n \tunlink(git_path_merge_msg(r));\n \tunlink(git_path_merge_mode(r));\n-\tunlink(git_path_auto_merge(r));\n+\trefs_delete_ref(get_main_ref_store(r), \"\", \"AUTO_MERGE\",\n+\t\t\tNULL, REF_NO_DEREF);\n \tsave_autostash(git_path_merge_autostash(r));\n }\n \ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 995818c28d..5b086f651a 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -515,7 +515,7 @@ static int finish_rebase(struct rebase_options *opts)\n \tint ret = 0;\n \n \tdelete_ref(NULL, \"REBASE_HEAD\", NULL, REF_NO_DEREF);\n-\tunlink(git_path_auto_merge(the_repository));\n+\tdelete_ref(NULL, \"AUTO_MERGE\", NULL, REF_NO_DEREF);\n \tapply_autostash(state_dir_path(\"autostash\", opts));\n \t/*\n \t * We ignore errors in 'git maintenance run --auto', since the\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 77ba7f3020..d72fd04f58 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -38,6 +38,7 @@\n #include \"path.h\"\n #include \"promisor-remote.h\"\n #include \"read-cache-ll.h\"\n+#include \"refs.h\"\n #include \"revision.h\"\n #include \"sparse-index.h\"\n #include \"strmap.h\"\n@@ -4659,9 +4660,6 @@ void merge_switch_to_result(struct merge_options *opt,\n {\n \tassert(opt->priv == NULL);\n \tif (result->clean >= 0 && update_worktree_and_index) {\n-\t\tconst char *filename;\n-\t\tFILE *fp;\n-\n \t\ttrace2_region_enter(\"merge\", \"checkout\", opt->repo);\n \t\tif (checkout(opt, head, result->tree)) {\n \t\t\t/* failure to function */\n@@ -4687,10 +4685,17 @@ void merge_switch_to_result(struct merge_options *opt,\n \t\ttrace2_region_leave(\"merge\", \"record_conflicted\", opt->repo);\n \n \t\ttrace2_region_enter(\"merge\", \"write_auto_merge\", opt->repo);\n-\t\tfilename = git_path_auto_merge(opt->repo);\n-\t\tfp = xfopen(filename, \"w\");\n-\t\tfprintf(fp, \"%s\\n\", oid_to_hex(&result->tree->object.oid));\n-\t\tfclose(fp);\n+\t\tif (refs_update_ref(get_main_ref_store(opt->repo), \"\", \"AUTO_MERGE\",\n+\t\t\t\t    &result->tree->object.oid, NULL, REF_NO_DEREF,\n+\t\t\t\t    UPDATE_REFS_MSG_ON_ERR)) {\n+\t\t\t/* failure to function */\n+\t\t\topt->priv = NULL;\n+\t\t\tresult->clean = -1;\n+\t\t\tmerge_finalize(opt, result);\n+\t\t\ttrace2_region_leave(\"merge\", \"write_auto_merge\",\n+\t\t\t\t\t    opt->repo);\n+\t\t\treturn;\n+\t\t}\n \t\ttrace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n \t}\n \tif (display_update_msgs)\ndiff --git a/path.c b/path.c\nindex 67e2690efe..f881c03171 100644\n--- a/path.c\n+++ b/path.c\n@@ -1589,6 +1589,5 @@ REPO_GIT_PATH_FUNC(merge_rr, \"MERGE_RR\")\n REPO_GIT_PATH_FUNC(merge_mode, \"MERGE_MODE\")\n REPO_GIT_PATH_FUNC(merge_head, \"MERGE_HEAD\")\n REPO_GIT_PATH_FUNC(merge_autostash, \"MERGE_AUTOSTASH\")\n-REPO_GIT_PATH_FUNC(auto_merge, \"AUTO_MERGE\")\n REPO_GIT_PATH_FUNC(fetch_head, \"FETCH_HEAD\")\n REPO_GIT_PATH_FUNC(shallow, \"shallow\")\ndiff --git a/path.h b/path.h\nindex 639372edd9..f387410f8c 100644\n--- a/path.h\n+++ b/path.h\n@@ -176,7 +176,6 @@ const char *git_path_merge_rr(struct repository *r);\n const char *git_path_merge_mode(struct repository *r);\n const char *git_path_merge_head(struct repository *r);\n const char *git_path_merge_autostash(struct repository *r);\n-const char *git_path_auto_merge(struct repository *r);\n const char *git_path_fetch_head(struct repository *r);\n const char *git_path_shallow(struct repository *r);\n \ndiff --git a/refs.c b/refs.c\nindex 20e8f1ff1f..906c7e5f27 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1874,7 +1874,6 @@ static int is_special_ref(const char *refname)\n \t * (normal ones).\n \t */\n \tstatic const char * const special_refs[] = {\n-\t\t\"AUTO_MERGE\",\n \t\t\"FETCH_HEAD\",\n \t\t\"MERGE_AUTOSTASH\",\n \t\t\"MERGE_HEAD\",\ndiff --git a/repository.c b/repository.c\nindex d7d24d416a..a931e3b1b3 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -263,7 +263,6 @@ static void repo_clear_path_cache(struct repo_path_cache *cache)\n \tFREE_AND_NULL(cache->merge_mode);\n \tFREE_AND_NULL(cache->merge_head);\n \tFREE_AND_NULL(cache->merge_autostash);\n-\tFREE_AND_NULL(cache->auto_merge);\n \tFREE_AND_NULL(cache->fetch_head);\n \tFREE_AND_NULL(cache->shallow);\n }\ndiff --git a/repository.h b/repository.h\nindex f5269b3730..47e7d20b59 100644\n--- a/repository.h\n+++ b/repository.h\n@@ -68,7 +68,6 @@ struct repo_path_cache {\n \tchar *merge_mode;\n \tchar *merge_head;\n \tchar *merge_autostash;\n-\tchar *auto_merge;\n \tchar *fetch_head;\n \tchar *shallow;\n };\ndiff --git a/sequencer.c b/sequencer.c\nindex 6f620f5717..47c8d17cbb 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2408,7 +2408,8 @@ static int do_pick_commit(struct repository *r,\n \t\trefs_delete_ref(get_main_ref_store(r), \"\", \"CHERRY_PICK_HEAD\",\n \t\t\t\tNULL, REF_NO_DEREF);\n \t\tunlink(git_path_merge_msg(r));\n-\t\tunlink(git_path_auto_merge(r));\n+\t\trefs_delete_ref(get_main_ref_store(r), \"\", \"AUTO_MERGE\",\n+\t\t\t\tNULL, REF_NO_DEREF);\n \t\tfprintf(stderr,\n \t\t\t_(\"dropping %s %s -- patch contents already upstream\\n\"),\n \t\t\toid_to_hex(&commit->object.oid), msg.subject);\n@@ -2818,7 +2819,8 @@ void sequencer_post_commit_cleanup(struct repository *r, int verbose)\n \t\tneed_cleanup = 1;\n \t}\n \n-\tunlink(git_path_auto_merge(r));\n+\trefs_delete_ref(get_main_ref_store(r), \"\", \"AUTO_MERGE\",\n+\t\t\tNULL, REF_NO_DEREF);\n \n \tif (!need_cleanup)\n \t\treturn;\n@@ -4766,7 +4768,8 @@ static int pick_commits(struct repository *r,\n \t\t\t}\n \t\t\tunlink(rebase_path_author_script());\n \t\t\tunlink(git_path_merge_head(r));\n-\t\t\tunlink(git_path_auto_merge(r));\n+\t\t\trefs_delete_ref(get_main_ref_store(r), \"\", \"AUTO_MERGE\",\n+\t\t\t\t\tNULL, REF_NO_DEREF);\n \t\t\trefs_delete_ref(get_main_ref_store(r), \"\", \"REBASE_HEAD\",\n \t\t\t\t\tNULL, REF_NO_DEREF);\n \n@@ -5123,7 +5126,8 @@ static int commit_staged_changes(struct repository *r,\n \t\treturn error(_(\"could not commit staged changes.\"));\n \tunlink(rebase_path_amend());\n \tunlink(git_path_merge_head(r));\n-\tunlink(git_path_auto_merge(r));\n+\trefs_delete_ref(get_main_ref_store(r), \"\", \"AUTO_MERGE\",\n+\t\t\tNULL, REF_NO_DEREF);\n \tif (final_fixup) {\n \t\tunlink(rebase_path_fixup_msg());\n \t\tunlink(rebase_path_squash_msg());\n-- \n2.43.GIT\n\n"},{"id":"487062","messageId":"abcf1f5cf428072d19639dc4209e0c1554f3eb53.1705659748.git.ps@pks.im","threadId":"60766","inReplyTo":"cover.1705659748.git.ps@pks.im","subject":"[PATCH 4/7] sequencer: introduce functions to handle autostashes via refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-19T10:40:15Z","receivedAt":"2024-01-19T10:40:18Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We're about to convert the MERGE_AUTOSTASH ref to become non-special,\nusing the refs API instead of direct filesystem access to both read and\nwrite the ref. The current interfaces to write autostashes is entirely\npath-based though, so we need to extend them to also support writes via\nthe refs API instead.\n\nIdeally, we would be able to fully replace the old set of path-based\ninterfaces. But the sequencer will continue to write state into\n\"rebase-merge/autostash\". This path is not considered to be a ref at all\nand will thus stay is-is for now, which requires us to keep both path-\nand refs-based interfaces to handle autostashes.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n sequencer.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++----\n sequencer.h |  3 +++\n 2 files changed, 64 insertions(+), 5 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 47c8d17cbb..91de546b32 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -4463,12 +4463,17 @@ static enum todo_command peek_command(struct todo_list *todo_list, int offset)\n \treturn -1;\n }\n \n-void create_autostash(struct repository *r, const char *path)\n+static void create_autostash_internal(struct repository *r,\n+\t\t\t\t      const char *path,\n+\t\t\t\t      const char *refname)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct lock_file lock_file = LOCK_INIT;\n \tint fd;\n \n+\tif (path && refname)\n+\t\tBUG(\"can only pass path or refname\");\n+\n \tfd = repo_hold_locked_index(r, &lock_file, 0);\n \trefresh_index(r->index, REFRESH_QUIET, NULL, NULL, NULL);\n \tif (0 <= fd)\n@@ -4495,10 +4500,16 @@ void create_autostash(struct repository *r, const char *path)\n \t\tstrbuf_reset(&buf);\n \t\tstrbuf_add_unique_abbrev(&buf, &oid, DEFAULT_ABBREV);\n \n-\t\tif (safe_create_leading_directories_const(path))\n-\t\t\tdie(_(\"Could not create directory for '%s'\"),\n-\t\t\t    path);\n-\t\twrite_file(path, \"%s\", oid_to_hex(&oid));\n+\t\tif (path) {\n+\t\t\tif (safe_create_leading_directories_const(path))\n+\t\t\t\tdie(_(\"Could not create directory for '%s'\"),\n+\t\t\t\t    path);\n+\t\t\twrite_file(path, \"%s\", oid_to_hex(&oid));\n+\t\t} else {\n+\t\t\trefs_update_ref(get_main_ref_store(r), \"\", refname,\n+\t\t\t\t\t&oid, null_oid(), 0, UPDATE_REFS_DIE_ON_ERR);\n+\t\t}\n+\n \t\tprintf(_(\"Created autostash: %s\\n\"), buf.buf);\n \t\tif (reset_head(r, &ropts) < 0)\n \t\t\tdie(_(\"could not reset --hard\"));\n@@ -4509,6 +4520,16 @@ void create_autostash(struct repository *r, const char *path)\n \tstrbuf_release(&buf);\n }\n \n+void create_autostash(struct repository *r, const char *path)\n+{\n+\tcreate_autostash_internal(r, path, NULL);\n+}\n+\n+void create_autostash_ref(struct repository *r, const char *refname)\n+{\n+\tcreate_autostash_internal(r, NULL, refname);\n+}\n+\n static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply)\n {\n \tstruct child_process child = CHILD_PROCESS_INIT;\n@@ -4586,6 +4607,41 @@ int apply_autostash_oid(const char *stash_oid)\n \treturn apply_save_autostash_oid(stash_oid, 1);\n }\n \n+static int apply_save_autostash_ref(struct repository *r, const char *refname,\n+\t\t\t\t    int attempt_apply)\n+{\n+\tstruct object_id stash_oid;\n+\tchar stash_oid_hex[GIT_MAX_HEXSZ + 1];\n+\tint flag, ret;\n+\n+\tif (!refs_ref_exists(get_main_ref_store(r), refname))\n+\t\treturn 0;\n+\n+\tif (!refs_resolve_ref_unsafe(get_main_ref_store(r), refname,\n+\t\t\t\t     RESOLVE_REF_READING, &stash_oid, &flag))\n+\t\treturn -1;\n+\tif (flag & REF_ISSYMREF)\n+\t\treturn error(_(\"autostash reference is a symref\"));\n+\n+\toid_to_hex_r(stash_oid_hex, &stash_oid);\n+\tret = apply_save_autostash_oid(stash_oid_hex, attempt_apply);\n+\n+\trefs_delete_ref(get_main_ref_store(r), \"\", refname,\n+\t\t\t&stash_oid, REF_NO_DEREF);\n+\n+\treturn ret;\n+}\n+\n+int save_autostash_ref(struct repository *r, const char *refname)\n+{\n+\treturn apply_save_autostash_ref(r, refname, 0);\n+}\n+\n+int apply_autostash_ref(struct repository *r, const char *refname)\n+{\n+\treturn apply_save_autostash_ref(r, refname, 1);\n+}\n+\n static int checkout_onto(struct repository *r, struct replay_opts *opts,\n \t\t\t const char *onto_name, const struct object_id *onto,\n \t\t\t const struct object_id *orig_head)\ndiff --git a/sequencer.h b/sequencer.h\nindex 913a0f652d..dcef7bb99c 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -225,9 +225,12 @@ void commit_post_rewrite(struct repository *r,\n \t\t\t const struct object_id *new_head);\n \n void create_autostash(struct repository *r, const char *path);\n+void create_autostash_ref(struct repository *r, const char *refname);\n int save_autostash(const char *path);\n+int save_autostash_ref(struct repository *r, const char *refname);\n int apply_autostash(const char *path);\n int apply_autostash_oid(const char *stash_oid);\n+int apply_autostash_ref(struct repository *r, const char *refname);\n \n #define SUMMARY_INITIAL_COMMIT   (1 << 0)\n #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)\n-- \n2.43.GIT\n\n"},{"id":"487063","messageId":"c29576d2d7e0247be118dd9cd9ae3fac3c7b33c7.1705659748.git.ps@pks.im","threadId":"60766","inReplyTo":"cover.1705659748.git.ps@pks.im","subject":"[PATCH 5/7] refs: convert MERGE_AUTOSTASH to become a normal pseudo-ref","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-19T10:40:19Z","receivedAt":"2024-01-19T10:40:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Similar to the preceding conversion of the AUTO_MERGE pseudo-ref, let's\nconvert the MERGE_AUTOSTASH ref to become a normal pseudo-ref as well.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n branch.c         |  2 +-\n builtin/commit.c |  2 +-\n builtin/merge.c  | 27 +++++++++++++--------------\n path.c           |  1 -\n path.h           |  1 -\n refs.c           |  1 -\n repository.c     |  1 -\n repository.h     |  1 -\n 8 files changed, 15 insertions(+), 21 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex c8bd9519e6..6719a181bd 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -819,7 +819,7 @@ void remove_merge_branch_state(struct repository *r)\n \tunlink(git_path_merge_mode(r));\n \trefs_delete_ref(get_main_ref_store(r), \"\", \"AUTO_MERGE\",\n \t\t\tNULL, REF_NO_DEREF);\n-\tsave_autostash(git_path_merge_autostash(r));\n+\tsave_autostash_ref(r, \"MERGE_AUTOSTASH\");\n }\n \n void remove_branch_state(struct repository *r, int verbose)\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 65196a2827..6d1fa71676 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1877,7 +1877,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t\t\t     &oid, flags);\n \t}\n \n-\tapply_autostash(git_path_merge_autostash(the_repository));\n+\tapply_autostash_ref(the_repository, \"MERGE_AUTOSTASH\");\n \n cleanup:\n \tstrbuf_release(&author_ident);\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex ebbe05033e..8f819781cc 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -476,7 +476,7 @@ static void finish(struct commit *head_commit,\n \trun_hooks_l(\"post-merge\", squash ? \"1\" : \"0\", NULL);\n \n \tif (new_head)\n-\t\tapply_autostash(git_path_merge_autostash(the_repository));\n+\t\tapply_autostash_ref(the_repository, \"MERGE_AUTOSTASH\");\n \tstrbuf_release(&reflog_message);\n }\n \n@@ -1315,7 +1315,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tif (abort_current_merge) {\n \t\tint nargc = 2;\n \t\tconst char *nargv[] = {\"reset\", \"--merge\", NULL};\n-\t\tstruct strbuf stash_oid = STRBUF_INIT;\n+\t\tchar stash_oid_hex[GIT_MAX_HEXSZ + 1];\n+\t\tstruct object_id stash_oid = {0};\n \n \t\tif (orig_argc != 2)\n \t\t\tusage_msg_opt(_(\"--abort expects no arguments\"),\n@@ -1324,17 +1325,17 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tif (!file_exists(git_path_merge_head(the_repository)))\n \t\t\tdie(_(\"There is no merge to abort (MERGE_HEAD missing).\"));\n \n-\t\tif (read_oneliner(&stash_oid, git_path_merge_autostash(the_repository),\n-\t\t    READ_ONELINER_SKIP_IF_EMPTY))\n-\t\t\tunlink(git_path_merge_autostash(the_repository));\n+\t\tif (!read_ref(\"MERGE_AUTOSTASH\", &stash_oid))\n+\t\t\tdelete_ref(\"\", \"MERGE_AUTOSTASH\", &stash_oid, REF_NO_DEREF);\n \n \t\t/* Invoke 'git reset --merge' */\n \t\tret = cmd_reset(nargc, nargv, prefix);\n \n-\t\tif (stash_oid.len)\n-\t\t\tapply_autostash_oid(stash_oid.buf);\n+\t\tif (!is_null_oid(&stash_oid)) {\n+\t\t\toid_to_hex_r(stash_oid_hex, &stash_oid);\n+\t\t\tapply_autostash_oid(stash_oid_hex);\n+\t\t}\n \n-\t\tstrbuf_release(&stash_oid);\n \t\tgoto done;\n \t}\n \n@@ -1563,13 +1564,12 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\tif (autostash)\n-\t\t\tcreate_autostash(the_repository,\n-\t\t\t\t\t git_path_merge_autostash(the_repository));\n+\t\t\tcreate_autostash_ref(the_repository, \"MERGE_AUTOSTASH\");\n \t\tif (checkout_fast_forward(the_repository,\n \t\t\t\t\t  &head_commit->object.oid,\n \t\t\t\t\t  &commit->object.oid,\n \t\t\t\t\t  overwrite_ignore)) {\n-\t\t\tapply_autostash(git_path_merge_autostash(the_repository));\n+\t\t\tapply_autostash_ref(the_repository, \"MERGE_AUTOSTASH\");\n \t\t\tret = 1;\n \t\t\tgoto done;\n \t\t}\n@@ -1655,8 +1655,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tdie_ff_impossible();\n \n \tif (autostash)\n-\t\tcreate_autostash(the_repository,\n-\t\t\t\t git_path_merge_autostash(the_repository));\n+\t\tcreate_autostash_ref(the_repository, \"MERGE_AUTOSTASH\");\n \n \t/* We are going to make a new commit. */\n \tgit_committer_info(IDENT_STRICT);\n@@ -1741,7 +1740,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\telse\n \t\t\tfprintf(stderr, _(\"Merge with strategy %s failed.\\n\"),\n \t\t\t\tuse_strategies[0]->name);\n-\t\tapply_autostash(git_path_merge_autostash(the_repository));\n+\t\tapply_autostash_ref(the_repository, \"MERGE_AUTOSTASH\");\n \t\tret = 2;\n \t\tgoto done;\n \t} else if (best_strategy == wt_strategy)\ndiff --git a/path.c b/path.c\nindex f881c03171..0fb527918b 100644\n--- a/path.c\n+++ b/path.c\n@@ -1588,6 +1588,5 @@ REPO_GIT_PATH_FUNC(merge_msg, \"MERGE_MSG\")\n REPO_GIT_PATH_FUNC(merge_rr, \"MERGE_RR\")\n REPO_GIT_PATH_FUNC(merge_mode, \"MERGE_MODE\")\n REPO_GIT_PATH_FUNC(merge_head, \"MERGE_HEAD\")\n-REPO_GIT_PATH_FUNC(merge_autostash, \"MERGE_AUTOSTASH\")\n REPO_GIT_PATH_FUNC(fetch_head, \"FETCH_HEAD\")\n REPO_GIT_PATH_FUNC(shallow, \"shallow\")\ndiff --git a/path.h b/path.h\nindex f387410f8c..b3233c51fa 100644\n--- a/path.h\n+++ b/path.h\n@@ -175,7 +175,6 @@ const char *git_path_merge_msg(struct repository *r);\n const char *git_path_merge_rr(struct repository *r);\n const char *git_path_merge_mode(struct repository *r);\n const char *git_path_merge_head(struct repository *r);\n-const char *git_path_merge_autostash(struct repository *r);\n const char *git_path_fetch_head(struct repository *r);\n const char *git_path_shallow(struct repository *r);\n \ndiff --git a/refs.c b/refs.c\nindex 906c7e5f27..047c81b1c1 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1875,7 +1875,6 @@ static int is_special_ref(const char *refname)\n \t */\n \tstatic const char * const special_refs[] = {\n \t\t\"FETCH_HEAD\",\n-\t\t\"MERGE_AUTOSTASH\",\n \t\t\"MERGE_HEAD\",\n \t};\n \tsize_t i;\ndiff --git a/repository.c b/repository.c\nindex a931e3b1b3..7aacb51b65 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -262,7 +262,6 @@ static void repo_clear_path_cache(struct repo_path_cache *cache)\n \tFREE_AND_NULL(cache->merge_rr);\n \tFREE_AND_NULL(cache->merge_mode);\n \tFREE_AND_NULL(cache->merge_head);\n-\tFREE_AND_NULL(cache->merge_autostash);\n \tFREE_AND_NULL(cache->fetch_head);\n \tFREE_AND_NULL(cache->shallow);\n }\ndiff --git a/repository.h b/repository.h\nindex 47e7d20b59..7a250a6605 100644\n--- a/repository.h\n+++ b/repository.h\n@@ -67,7 +67,6 @@ struct repo_path_cache {\n \tchar *merge_rr;\n \tchar *merge_mode;\n \tchar *merge_head;\n-\tchar *merge_autostash;\n \tchar *fetch_head;\n \tchar *shallow;\n };\n-- \n2.43.GIT\n\n"},{"id":"487064","messageId":"a4b4dd51f81fdf65f79b9afc3bd85109817ea128.1705659748.git.ps@pks.im","threadId":"60766","inReplyTo":"cover.1705659748.git.ps@pks.im","subject":"[PATCH 6/7] refs: redefine special refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-19T10:40:24Z","receivedAt":"2024-01-19T10:40:28Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Now that our list of special refs really only contains refs which have\nactually-special semantics, let's redefine what makes a special ref.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n refs.c | 33 +++++++--------------------------\n 1 file changed, 7 insertions(+), 26 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 047c81b1c1..08a900a047 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1839,13 +1839,10 @@ static int refs_read_special_head(struct ref_store *ref_store,\n static int is_special_ref(const char *refname)\n {\n \t/*\n-\t * Special references get written and read directly via the filesystem\n-\t * by the subsystems that create them. Thus, they must not go through\n-\t * the reference backend but must instead be read directly. It is\n-\t * arguable whether this behaviour is sensible, or whether it's simply\n-\t * a leaky abstraction enabled by us only having a single reference\n-\t * backend implementation. But at least for a subset of references it\n-\t * indeed does make sense to treat them specially:\n+\t * Special references are refs that have different semantics compared\n+\t * to \"normal\" refs. These refs can thus not be stored in the ref\n+\t * backend, but must always be accessed via the filesystem. The\n+\t * following refs are special:\n \t *\n \t * - FETCH_HEAD may contain multiple object IDs, and each one of them\n \t *   carries additional metadata like where it came from.\n@@ -1853,25 +1850,9 @@ static int is_special_ref(const char *refname)\n \t * - MERGE_HEAD may contain multiple object IDs when merging multiple\n \t *   heads.\n \t *\n-\t * There are some exceptions that you might expect to see on this list\n-\t * but which are handled exclusively via the reference backend:\n-\t *\n-\t * - BISECT_EXPECTED_REV\n-\t *\n-\t * - CHERRY_PICK_HEAD\n-\t *\n-\t * - HEAD\n-\t *\n-\t * - ORIG_HEAD\n-\t *\n-\t * - \"rebase-apply/\" and \"rebase-merge/\" contain all of the state for\n-\t *   rebases, including some reference-like files. These are\n-\t *   exclusively read and written via the filesystem and never go\n-\t *   through the refdb.\n-\t *\n-\t * Writing or deleting references must consistently go either through\n-\t * the filesystem (special refs) or through the reference backend\n-\t * (normal ones).\n+\t * Reading, writing or deleting references must consistently go either\n+\t * through the filesystem (special refs) or through the reference\n+\t * backend (normal ones).\n \t */\n \tstatic const char * const special_refs[] = {\n \t\t\"FETCH_HEAD\",\n-- \n2.43.GIT\n\n"},{"id":"487065","messageId":"2a0943a78d0db0f489962520536594845970e0b0.1705659748.git.ps@pks.im","threadId":"60766","inReplyTo":"cover.1705659748.git.ps@pks.im","subject":"[PATCH 7/7] Documentation: add \"special refs\" to the glossary","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-19T10:40:29Z","receivedAt":"2024-01-19T10:40:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Add the \"special refs\" term to our glossary.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/glossary-content.txt | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt\nindex f7d98c11e3..d71b199955 100644\n--- a/Documentation/glossary-content.txt\n+++ b/Documentation/glossary-content.txt\n@@ -638,6 +638,20 @@ The most notable example is `HEAD`.\n \tAn <<def_object,object>> used to temporarily store the contents of a\n \t<<def_dirty,dirty>> working directory and the index for future reuse.\n \n+[[def_special_ref]]special ref::\n+\tA ref that has different semantics than normal refs. These refs can be\n+\taccessed via normal Git commands but may not behave the same as a\n+\tnormal ref in some cases.\n++\n+The following special refs are known to Git:\n+\n+ - \"`FETCH_HEAD`\" is written by linkgit:git-fetch[1] or linkgit:git-pull[1]. It\n+   may refer to multiple object IDs. Each object ID is annotated with metadata\n+   indicating where it was fetched from and its fetch status.\n+\n+ - \"`MERGE_HEAD`\" is written by linkgit:git-merge[1] when resolving merge\n+   conflicts. It contains all commit IDs which are being merged.\n+\n [[def_submodule]]submodule::\n \tA <<def_repository,repository>> that holds the history of a\n \tseparate project inside another repository (the latter of\n-- \n2.43.GIT\n\n"},{"id":"487086","messageId":"xmqqttn9jfta.fsf@gitster.g","threadId":"60766","inReplyTo":"48b95fe954c1dbdd080ce7a0cc871f4850bddeae.1705659748.git.ps@pks.im","subject":"Re: [PATCH 1/7] sequencer: clean up pseudo refs with REF_NO_DEREF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-19T19:14:57Z","receivedAt":"2024-01-19T19:15:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> When cleaning up the state-tracking pseudorefs CHERRY_PICK_HEAD or\n> REVERT_HEAD we do not set REF_NO_DEREF. In the unlikely case where those\n> refs are a symref we would thus end up deleting the symref targets, and\n> not the symrefs themselves.\n>\n> Harden the code to use REF_NO_DEREF to fix this.\n\nThis level of attention to detail is very much appreciated.  I\nsuspect that this was inspired by the other topic we discussed the\nother day to tighten accesses to {MERGE,CHERRY_PICK,REVERT}_HEAD\nthat is done via repo_get_oid() to the one based on read_ref_full()?\n"},{"id":"487087","messageId":"xmqqjzo5jf79.fsf@gitster.g","threadId":"60766","inReplyTo":"ae2df6316f79e372c51d59666d685e59981d2f98.1705659748.git.ps@pks.im","subject":"Re: [PATCH 3/7] refs: convert AUTO_MERGE to become a normal pseudo-ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-19T19:28:10Z","receivedAt":"2024-01-19T19:28:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> In 70c70de616 (refs: complete list of special refs, 2023-12-14) we have\n> inrtoduced a new `is_special_ref()` function that classifies some refs\n\n\"introduced\"\n\n> @@ -4687,10 +4685,17 @@ void merge_switch_to_result(struct merge_options *opt,\n>  \t\ttrace2_region_leave(\"merge\", \"record_conflicted\", opt->repo);\n>  \n>  \t\ttrace2_region_enter(\"merge\", \"write_auto_merge\", opt->repo);\n> -\t\tfilename = git_path_auto_merge(opt->repo);\n> -\t\tfp = xfopen(filename, \"w\");\n> -\t\tfprintf(fp, \"%s\\n\", oid_to_hex(&result->tree->object.oid));\n> -\t\tfclose(fp);\n> +\t\tif (refs_update_ref(get_main_ref_store(opt->repo), \"\", \"AUTO_MERGE\",\n> +\t\t\t\t    &result->tree->object.oid, NULL, REF_NO_DEREF,\n> +\t\t\t\t    UPDATE_REFS_MSG_ON_ERR)) {\n> +\t\t\t/* failure to function */\n> +\t\t\topt->priv = NULL;\n> +\t\t\tresult->clean = -1;\n> +\t\t\tmerge_finalize(opt, result);\n> +\t\t\ttrace2_region_leave(\"merge\", \"write_auto_merge\",\n> +\t\t\t\t\t    opt->repo);\n> +\t\t\treturn;\n> +\t\t}\n>  \t\ttrace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n>  \t}\n>  \tif (display_update_msgs)\n\nWe used to ignore errors while updating AUTO_MERGE, implying that it\nis an optional nicety that does not have to block the merge.  Now we\nexplicitly mark the resulting index unclean.  While my gut feeling\nsays that it should not matter all that much (as such a failure\nwould be rare enough that the user may want to inspect and double\ncheck the situation before going forward), I am not 100% sure if the\nchange is behaviour is acceptable by everybody (cc'ed Elijah for\nsecond opinion).\n\nEverything else in this patch looked good.\n\nThanks.\n"},{"id":"487089","messageId":"xmqqbk9hjdai.fsf@gitster.g","threadId":"60766","inReplyTo":"abcf1f5cf428072d19639dc4209e0c1554f3eb53.1705659748.git.ps@pks.im","subject":"Re: [PATCH 4/7] sequencer: introduce functions to handle autostashes via refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-19T20:09:25Z","receivedAt":"2024-01-19T20:09:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> We're about to convert the MERGE_AUTOSTASH ref to become non-special,\n> using the refs API instead of direct filesystem access to both read and\n> write the ref. The current interfaces to write autostashes is entirely\n> path-based though, so we need to extend them to also support writes via\n> the refs API instead.\n>\n> Ideally, we would be able to fully replace the old set of path-based\n> interfaces. But the sequencer will continue to write state into\n> \"rebase-merge/autostash\". This path is not considered to be a ref at all\n> and will thus stay is-is for now, which requires us to keep both path-\n\n\"is-is\"???\n\n> and refs-based interfaces to handle autostashes.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  sequencer.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++----\n>  sequencer.h |  3 +++\n>  2 files changed, 64 insertions(+), 5 deletions(-)\n\nThe conversion (rather, the introduction to allow refs API to be\nused to access them) look correct, but offhand I do not know what\nthe implication of leaving the file based interface would be.\n\nThanks.\n"},{"id":"487103","messageId":"xmqq7ck5jceq.fsf@gitster.g","threadId":"60766","inReplyTo":"a4b4dd51f81fdf65f79b9afc3bd85109817ea128.1705659748.git.ps@pks.im","subject":"Re: [PATCH 6/7] refs: redefine special refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-19T20:28:29Z","receivedAt":"2024-01-19T20:28:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Now that our list of special refs really only contains refs which have\n> actually-special semantics, let's redefine what makes a special ref.\n\nYay.\n\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  refs.c | 33 +++++++--------------------------\n>  1 file changed, 7 insertions(+), 26 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index 047c81b1c1..08a900a047 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1839,13 +1839,10 @@ static int refs_read_special_head(struct ref_store *ref_store,\n>  static int is_special_ref(const char *refname)\n>  {\n>  \t/*\n> -\t * Special references get written and read directly via the filesystem\n> -\t * by the subsystems that create them. Thus, they must not go through\n> -\t * the reference backend but must instead be read directly. It is\n> -\t * arguable whether this behaviour is sensible, or whether it's simply\n> -\t * a leaky abstraction enabled by us only having a single reference\n> -\t * backend implementation. But at least for a subset of references it\n> -\t * indeed does make sense to treat them specially:\n> +\t * Special references are refs that have different semantics compared\n> +\t * to \"normal\" refs. These refs can thus not be stored in the ref\n> +\t * backend, but must always be accessed via the filesystem. The\n> +\t * following refs are special:\n>  \t *\n>  \t * - FETCH_HEAD may contain multiple object IDs, and each one of them\n>  \t *   carries additional metadata like where it came from.\n> @@ -1853,25 +1850,9 @@ static int is_special_ref(const char *refname)\n>  \t * - MERGE_HEAD may contain multiple object IDs when merging multiple\n>  \t *   heads.\n>  \t *\n> -\t * There are some exceptions that you might expect to see on this list\n> -\t * but which are handled exclusively via the reference backend:\n> -\t *\n> -\t * - BISECT_EXPECTED_REV\n> -\t *\n> -\t * - CHERRY_PICK_HEAD\n> -\t *\n> -\t * - HEAD\n> -\t *\n> -\t * - ORIG_HEAD\n> -\t *\n> -\t * - \"rebase-apply/\" and \"rebase-merge/\" contain all of the state for\n> -\t *   rebases, including some reference-like files. These are\n> -\t *   exclusively read and written via the filesystem and never go\n> -\t *   through the refdb.\n> -\t *\n> -\t * Writing or deleting references must consistently go either through\n> -\t * the filesystem (special refs) or through the reference backend\n> -\t * (normal ones).\n> +\t * Reading, writing or deleting references must consistently go either\n> +\t * through the filesystem (special refs) or through the reference\n> +\t * backend (normal ones).\n>  \t */\n>  \tstatic const char * const special_refs[] = {\n>  \t\t\"FETCH_HEAD\",\n"},{"id":"487193","messageId":"Za5FEJOrUGTm620N@tanuki","threadId":"60766","inReplyTo":"xmqqttn9jfta.fsf@gitster.g","subject":"Re: [PATCH 1/7] sequencer: clean up pseudo refs with REF_NO_DEREF","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-22T10:36:00Z","receivedAt":"2024-01-22T10:36:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Jan 19, 2024 at 11:14:57AM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > When cleaning up the state-tracking pseudorefs CHERRY_PICK_HEAD or\n> > REVERT_HEAD we do not set REF_NO_DEREF. In the unlikely case where those\n> > refs are a symref we would thus end up deleting the symref targets, and\n> > not the symrefs themselves.\n> >\n> > Harden the code to use REF_NO_DEREF to fix this.\n> \n> This level of attention to detail is very much appreciated.  I\n> suspect that this was inspired by the other topic we discussed the\n> other day to tighten accesses to {MERGE,CHERRY_PICK,REVERT}_HEAD\n> that is done via repo_get_oid() to the one based on read_ref_full()?\n\nAt least not directly, no. I was adding new calls to `delete_ref()` and\nwas reflexively adding REF_NO_DEREF, which made me wonder why the other\ncases in the vicinity didn't use it yet. So it's more inspired by the\nfirst patch series that introduced the \"special refs\" classification,\nwhere I already added ref deletions with `REF_NO_DEREF`.\n\nPatrick\n"},{"id":"487195","messageId":"Za5HOkWM3IQIiKDJ@tanuki","threadId":"60766","inReplyTo":"xmqqjzo5jf79.fsf@gitster.g","subject":"Re: [PATCH 3/7] refs: convert AUTO_MERGE to become a normal pseudo-ref","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-22T10:45:14Z","receivedAt":"2024-01-22T10:45:20Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Jan 19, 2024 at 11:28:10AM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > In 70c70de616 (refs: complete list of special refs, 2023-12-14) we have\n> > inrtoduced a new `is_special_ref()` function that classifies some refs\n> \n> \"introduced\"\n> \n> > @@ -4687,10 +4685,17 @@ void merge_switch_to_result(struct merge_options *opt,\n> >  \t\ttrace2_region_leave(\"merge\", \"record_conflicted\", opt->repo);\n> >  \n> >  \t\ttrace2_region_enter(\"merge\", \"write_auto_merge\", opt->repo);\n> > -\t\tfilename = git_path_auto_merge(opt->repo);\n> > -\t\tfp = xfopen(filename, \"w\");\n> > -\t\tfprintf(fp, \"%s\\n\", oid_to_hex(&result->tree->object.oid));\n> > -\t\tfclose(fp);\n> > +\t\tif (refs_update_ref(get_main_ref_store(opt->repo), \"\", \"AUTO_MERGE\",\n> > +\t\t\t\t    &result->tree->object.oid, NULL, REF_NO_DEREF,\n> > +\t\t\t\t    UPDATE_REFS_MSG_ON_ERR)) {\n> > +\t\t\t/* failure to function */\n> > +\t\t\topt->priv = NULL;\n> > +\t\t\tresult->clean = -1;\n> > +\t\t\tmerge_finalize(opt, result);\n> > +\t\t\ttrace2_region_leave(\"merge\", \"write_auto_merge\",\n> > +\t\t\t\t\t    opt->repo);\n> > +\t\t\treturn;\n> > +\t\t}\n> >  \t\ttrace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n> >  \t}\n> >  \tif (display_update_msgs)\n> \n> We used to ignore errors while updating AUTO_MERGE, implying that it\n> is an optional nicety that does not have to block the merge.  Now we\n> explicitly mark the resulting index unclean.  While my gut feeling\n> says that it should not matter all that much (as such a failure\n> would be rare enough that the user may want to inspect and double\n> check the situation before going forward), I am not 100% sure if the\n> change is behaviour is acceptable by everybody (cc'ed Elijah for\n> second opinion).\n\nWe only ignored _some_ errors when updating AUTO_MERGE. Most notably we\ndie when we fail to create the file, but we succeed in case its contents\naren't written. This does not make much sense to me -- my expectation\nwould be that we should verify either the complete operation or nothing\nof it and ignore all failures. Gracefully leaving an empty file behind\nis a weird in-between state, so I'd claim it's more or less an oversight\nthat we did not perform proper error checking here.\n\nPatrick\n"},{"id":"487196","messageId":"Za5IoEjs0q7cf354@tanuki","threadId":"60766","inReplyTo":"xmqqbk9hjdai.fsf@gitster.g","subject":"Re: [PATCH 4/7] sequencer: introduce functions to handle autostashes via refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-22T10:51:12Z","receivedAt":"2024-01-22T10:51:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Jan 19, 2024 at 12:09:25PM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > We're about to convert the MERGE_AUTOSTASH ref to become non-special,\n> > using the refs API instead of direct filesystem access to both read and\n> > write the ref. The current interfaces to write autostashes is entirely\n> > path-based though, so we need to extend them to also support writes via\n> > the refs API instead.\n> >\n> > Ideally, we would be able to fully replace the old set of path-based\n> > interfaces. But the sequencer will continue to write state into\n> > \"rebase-merge/autostash\". This path is not considered to be a ref at all\n> > and will thus stay is-is for now, which requires us to keep both path-\n> \n> \"is-is\"???\n\nOops, \"as-is\" :)\n\n> > and refs-based interfaces to handle autostashes.\n> >\n> > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> > ---\n> >  sequencer.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++----\n> >  sequencer.h |  3 +++\n> >  2 files changed, 64 insertions(+), 5 deletions(-)\n> \n> The conversion (rather, the introduction to allow refs API to be\n> used to access them) look correct, but offhand I do not know what\n> the implication of leaving the file based interface would be.\n\nWe have said in past discussions that the sequencer state should remain\nself contained, and that requires us to keep the files-based interface.\nRefactoring it would likely be a larger undertaking, as we have also\nsaid that refs must either have pseudo-ref syntax or start with \"refs/\".\n\nThe sequencer with its \"rebase-merge/autostash\" files doesn't conform to\neither of those requirements, so we would also have to rename those\nreflike files. I doubt that this is really worth it, so I would keep\nthose around as internal implementation details of how the sequencer\nworks. These refs are not supposed to be accessed by the user in the\nfirst place, and we do not document their existence to the best of my\nknowledge. Also, `git rev-parse rebase-merge/autostash` would fail.\n\nSo overall I think it's fine to leave this internal sequencer state as\nself-contained as it is.\n\nPatrick\n"},{"id":"487200","messageId":"CAOLa=ZQakWpRgFZdLnALq9f+acAR3hBwgO_kxJ6gb_yv+vrX=g@mail.gmail.com","threadId":"60766","inReplyTo":"48b95fe954c1dbdd080ce7a0cc871f4850bddeae.1705659748.git.ps@pks.im","subject":"Re: [PATCH 1/7] sequencer: clean up pseudo refs with REF_NO_DEREF","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-01-22T11:49:37Z","receivedAt":"2024-01-22T11:49:39Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> When cleaning up the state-tracking pseudorefs CHERRY_PICK_HEAD or\n> REVERT_HEAD we do not set REF_NO_DEREF. In the unlikely case where those\n> refs are a symref we would thus end up deleting the symref targets, and\n> not the symrefs themselves.\n>\n> Harden the code to use REF_NO_DEREF to fix this.\n>\n\nJust a question for my understanding. Is that that currently\nCHERRY_PICK_HEAD or REVERT_HEAD are never symrefs, so we don't really\nworry about this, but adding this change makes it safer?\n"},{"id":"487201","messageId":"CAOLa=ZRXS823vHFaBWx_T1159Ex3Y+sxBGXQVa8v+fq_q546mg@mail.gmail.com","threadId":"60766","inReplyTo":"ae2df6316f79e372c51d59666d685e59981d2f98.1705659748.git.ps@pks.im","subject":"Re: [PATCH 3/7] refs: convert AUTO_MERGE to become a normal pseudo-ref","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-01-22T12:02:02Z","receivedAt":"2024-01-22T12:02:04Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> In 70c70de616 (refs: complete list of special refs, 2023-12-14) we have\n> inrtoduced a new `is_special_ref()` function that classifies some refs\n> as being special. The rule is that special refs are exclusively read and\n> written via the filesystem directly, whereas normal refs exclucsively go\n> via the refs API.\n>\n\ns/exclucsively/exclusively\n"},{"id":"487202","messageId":"Za5fVWZ8oUEpA04F@tanuki","threadId":"60766","inReplyTo":"CAOLa=ZQakWpRgFZdLnALq9f+acAR3hBwgO_kxJ6gb_yv+vrX=g@mail.gmail.com","subject":"Re: [PATCH 1/7] sequencer: clean up pseudo refs with REF_NO_DEREF","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-22T12:28:05Z","receivedAt":"2024-01-22T12:28:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Jan 22, 2024 at 03:49:37AM -0800, Karthik Nayak wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > When cleaning up the state-tracking pseudorefs CHERRY_PICK_HEAD or\n> > REVERT_HEAD we do not set REF_NO_DEREF. In the unlikely case where those\n> > refs are a symref we would thus end up deleting the symref targets, and\n> > not the symrefs themselves.\n> >\n> > Harden the code to use REF_NO_DEREF to fix this.\n> >\n> \n> Just a question for my understanding. Is that that currently\n> CHERRY_PICK_HEAD or REVERT_HEAD are never symrefs, so we don't really\n> worry about this, but adding this change makes it safer?\n\nThey shouldn't ever be, at least Git would never end up writing symrefs\nthere. So for those refs to be symrefs the user would need to manually\ncreate them as such, and in that case it would certainly be unexpected\nif we deleted the symref targets when cleaning up state after a revert\nor cherry-pick.\n\nSo yes, it's about hardening the code for edge cases that shouldn't\nreally happen in the first place.\n\nPatrick\n"},{"id":"487215","messageId":"a7712d26-7c63-4c77-b339-126d05be6b0d@gmail.com","threadId":"60766","inReplyTo":"Za5IoEjs0q7cf354@tanuki","subject":"Re: [PATCH 4/7] sequencer: introduce functions to handle autostashes via refs","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-01-22T19:54:58Z","receivedAt":"2024-01-22T19:55:00Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 22/01/2024 10:51, Patrick Steinhardt wrote:\n> On Fri, Jan 19, 2024 at 12:09:25PM -0800, Junio C Hamano wrote:\n>> Patrick Steinhardt <ps@pks.im> writes:\n>> The conversion (rather, the introduction to allow refs API to be\n>> used to access them) look correct, but offhand I do not know what\n>> the implication of leaving the file based interface would be.\n> \n> We have said in past discussions that the sequencer state should remain\n> self contained, and that requires us to keep the files-based interface.\n> Refactoring it would likely be a larger undertaking, as we have also\n> said that refs must either have pseudo-ref syntax or start with \"refs/\".\n> \n> The sequencer with its \"rebase-merge/autostash\" files doesn't conform to\n> either of those requirements, so we would also have to rename those\n> reflike files. I doubt that this is really worth it, so I would keep\n> those around as internal implementation details of how the sequencer\n> works. These refs are not supposed to be accessed by the user in the\n> first place, and we do not document their existence to the best of my\n> knowledge. Also, `git rev-parse rebase-merge/autostash` would fail.\n\nExactly \"rebase-merge/autostash\" is a detail of the rebase \nimplementation, it is not a ref that users should be using.\n\n> So overall I think it's fine to leave this internal sequencer state as\n> self-contained as it is.\n\nThat's my feeling too\n\nBest Wishes\n\nPhillip\n"},{"id":"487218","messageId":"xmqqy1chrumk.fsf@gitster.g","threadId":"60766","inReplyTo":"a7712d26-7c63-4c77-b339-126d05be6b0d@gmail.com","subject":"Re: [PATCH 4/7] sequencer: introduce functions to handle autostashes via refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-22T20:16:51Z","receivedAt":"2024-01-22T20:16:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n>> ... These refs are not supposed to be accessed by the user in the\n>> first place, and we do not document their existence to the best of my\n>> knowledge. Also, `git rev-parse rebase-merge/autostash` would fail.\n>\n> Exactly \"rebase-merge/autostash\" is a detail of the rebase\n> implementation, it is not a ref that users should be using.\n>\n>> So overall I think it's fine to leave this internal sequencer state as\n>> self-contained as it is.\n>\n> That's my feeling too\n\nGood.  As long as \"git rev-parse rebase-merge/autostash\" fails, and\nregardless of the ref backend in use, we always do \"read one line from\nthe on-disk file and run get_oid_hex() on it\", I would be happy with\nthat direction.\n\nThanks.\n\n[Footnote]\n\n * We seem to overuse strbuf_read_file() even when we expect that\n   the file is a single-liner.  We have read_line_from_git_path() in\n   wt-status.c that only reads one line and we may want to split it\n   into two: one that takes a filename and calls the other with\n   git_path(\"%s\", filename), and the other that takes a path for any\n   on-disk file, reads a single line, and returns the string that\n   was read, or something like that.  We might later want to update\n   the \"other\" function so that it errors out if there are extra\n   trailing lines (i.e. we expect it is a single-liner, but that\n   expectation is violated---now what?).\n"},{"id":"487266","messageId":"4f60dd83-913d-4c66-989d-282ec1845f4b@gmail.com","threadId":"60766","inReplyTo":"2a0943a78d0db0f489962520536594845970e0b0.1705659748.git.ps@pks.im","subject":"Re: [PATCH 7/7] Documentation: add \"special refs\" to the glossary","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-01-23T16:27:20Z","receivedAt":"2024-01-23T16:27:22Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Patrick\n\nOn 19/01/2024 10:40, Patrick Steinhardt wrote:\n> Add the \"special refs\" term to our glossary.\n\nRelated to this the glossary entry for pseudorefs says\n\n         Pseudorefs are a class of files under `$GIT_DIR` which behave\n         like refs for the purposes of rev-parse, but which are treated\n         specially by git.  Pseudorefs both have names that are all-caps,\n         and always start with a line consisting of a\n         <<def_SHA1,SHA-1>> followed by whitespace.  So, HEAD is not a\n         pseudoref, because it is sometimes a symbolic ref.  They might\n         optionally contain some additional data.  `MERGE_HEAD` and\n         `CHERRY_PICK_HEAD` are examples.  Unlike\n         <<def_per_worktree_ref,per-worktree refs>>, these files cannot\n         be symbolic refs, and never have reflogs.  They also cannot be\n         updated through the normal ref update machinery.  Instead,\n         they are updated by directly writing to the files.  However,\n         they can be read as if they were refs, so `git rev-parse\n         MERGE_HEAD` will work.\n\nwhich is very file-centric. We should probably update that as we're \nmoving away from filesystem access except for special refs.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>   Documentation/glossary-content.txt | 14 ++++++++++++++\n>   1 file changed, 14 insertions(+)\n> \n> diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt\n> index f7d98c11e3..d71b199955 100644\n> --- a/Documentation/glossary-content.txt\n> +++ b/Documentation/glossary-content.txt\n> @@ -638,6 +638,20 @@ The most notable example is `HEAD`.\n>   \tAn <<def_object,object>> used to temporarily store the contents of a\n>   \t<<def_dirty,dirty>> working directory and the index for future reuse.\n>   \n> +[[def_special_ref]]special ref::\n> +\tA ref that has different semantics than normal refs. These refs can be\n> +\taccessed via normal Git commands but may not behave the same as a\n> +\tnormal ref in some cases.\n> ++\n> +The following special refs are known to Git:\n> +\n> + - \"`FETCH_HEAD`\" is written by linkgit:git-fetch[1] or linkgit:git-pull[1]. It\n> +   may refer to multiple object IDs. Each object ID is annotated with metadata\n> +   indicating where it was fetched from and its fetch status.\n> +\n> + - \"`MERGE_HEAD`\" is written by linkgit:git-merge[1] when resolving merge\n> +   conflicts. It contains all commit IDs which are being merged.\n> +\n>   [[def_submodule]]submodule::\n>   \tA <<def_repository,repository>> that holds the history of a\n>   \tseparate project inside another repository (the latter of\n"},{"id":"487293","messageId":"CABPp-BG1vA7B9iqLMAq8HnwgA_J6XsbSCa0dEH0PAkWQajP3Uw@mail.gmail.com","threadId":"60766","inReplyTo":"Za5HOkWM3IQIiKDJ@tanuki","subject":"Re: [PATCH 3/7] refs: convert AUTO_MERGE to become a normal pseudo-ref","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2024-01-24T03:19:00Z","receivedAt":"2024-01-24T03:20:27Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Jan 22, 2024 at 2:45 AM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> On Fri, Jan 19, 2024 at 11:28:10AM -0800, Junio C Hamano wrote:\n> > Patrick Steinhardt <ps@pks.im> writes:\n> >\n> > > In 70c70de616 (refs: complete list of special refs, 2023-12-14) we have\n> > > inrtoduced a new `is_special_ref()` function that classifies some refs\n> >\n> > \"introduced\"\n> >\n> > > @@ -4687,10 +4685,17 @@ void merge_switch_to_result(struct merge_options *opt,\n> > >             trace2_region_leave(\"merge\", \"record_conflicted\", opt->repo);\n> > >\n> > >             trace2_region_enter(\"merge\", \"write_auto_merge\", opt->repo);\n> > > -           filename = git_path_auto_merge(opt->repo);\n> > > -           fp = xfopen(filename, \"w\");\n> > > -           fprintf(fp, \"%s\\n\", oid_to_hex(&result->tree->object.oid));\n> > > -           fclose(fp);\n> > > +           if (refs_update_ref(get_main_ref_store(opt->repo), \"\", \"AUTO_MERGE\",\n> > > +                               &result->tree->object.oid, NULL, REF_NO_DEREF,\n> > > +                               UPDATE_REFS_MSG_ON_ERR)) {\n> > > +                   /* failure to function */\n> > > +                   opt->priv = NULL;\n> > > +                   result->clean = -1;\n> > > +                   merge_finalize(opt, result);\n> > > +                   trace2_region_leave(\"merge\", \"write_auto_merge\",\n> > > +                                       opt->repo);\n> > > +                   return;\n> > > +           }\n> > >             trace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n> > >     }\n> > >     if (display_update_msgs)\n> >\n> > We used to ignore errors while updating AUTO_MERGE, implying that it\n> > is an optional nicety that does not have to block the merge.  Now we\n> > explicitly mark the resulting index unclean.  While my gut feeling\n> > says that it should not matter all that much (as such a failure\n> > would be rare enough that the user may want to inspect and double\n> > check the situation before going forward), I am not 100% sure if the\n> > change is behaviour is acceptable by everybody (cc'ed Elijah for\n> > second opinion).\n>\n> We only ignored _some_ errors when updating AUTO_MERGE. Most notably we\n> die when we fail to create the file, but we succeed in case its contents\n> aren't written. This does not make much sense to me -- my expectation\n> would be that we should verify either the complete operation or nothing\n> of it and ignore all failures. Gracefully leaving an empty file behind\n> is a weird in-between state, so I'd claim it's more or less an oversight\n> that we did not perform proper error checking here.\n\nI can confirm it was indeed just an oversight.  I like your change to\nmake this code more careful.\n"},{"id":"487321","messageId":"ZbDS6B5mjLeQ8pIy@tanuki","threadId":"60766","inReplyTo":"4f60dd83-913d-4c66-989d-282ec1845f4b@gmail.com","subject":"Re: [PATCH 7/7] Documentation: add \"special refs\" to the glossary","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-24T09:05:44Z","receivedAt":"2024-01-24T09:05:53Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Jan 23, 2024 at 04:27:20PM +0000, Phillip Wood wrote:\n> Hi Patrick\n> \n> On 19/01/2024 10:40, Patrick Steinhardt wrote:\n> > Add the \"special refs\" term to our glossary.\n> \n> Related to this the glossary entry for pseudorefs says\n> \n>         Pseudorefs are a class of files under `$GIT_DIR` which behave\n>         like refs for the purposes of rev-parse, but which are treated\n>         specially by git.  Pseudorefs both have names that are all-caps,\n>         and always start with a line consisting of a\n>         <<def_SHA1,SHA-1>> followed by whitespace.  So, HEAD is not a\n>         pseudoref, because it is sometimes a symbolic ref.  They might\n>         optionally contain some additional data.  `MERGE_HEAD` and\n>         `CHERRY_PICK_HEAD` are examples.  Unlike\n>         <<def_per_worktree_ref,per-worktree refs>>, these files cannot\n>         be symbolic refs, and never have reflogs.  They also cannot be\n>         updated through the normal ref update machinery.  Instead,\n>         they are updated by directly writing to the files.  However,\n>         they can be read as if they were refs, so `git rev-parse\n>         MERGE_HEAD` will work.\n> \n> which is very file-centric. We should probably update that as we're moving\n> away from filesystem access except for special refs.\n\nGood point indeed, thanks for the hint! Will update in a future patch\nseries.\n\nPatrick\n"}]}