{"thread":{"id":"57028","subject":"[PATCH 0/2] ns/tmp-objdir: add support for temporary writable databases","startedAt":"2021-12-04T02:41:03Z","lastAt":"2021-12-08T16:41:35Z","messageCount":18,"participants":["Neeraj K. Singh via GitGitGadget","Neeraj Singh via GitGitGadget","Junio C Hamano","Neeraj Singh","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"443053","messageId":"pull.1091.git.1638585658.gitgitgadget@gmail.com","threadId":"57028","inReplyTo":null,"subject":"[PATCH 0/2] ns/tmp-objdir: add support for temporary writable databases","fromName":"Neeraj K. Singh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-04T02:40:56Z","receivedAt":"2021-12-04T02:41:03Z","isPatch":true,"sender":{"key":"name:Neeraj K. Singh","avatar":null},"body":"New interface into the tmp-objdir API to help in-core use of the quarantine\nfeature.\n\nThis patch series was formerly part of the ns/batched-fsync topic [1]. It's\nnow split out into its own gitgitgadget PR and discussion thread since it is\nthe base for en/remerge-diff as well.\n\nThe most recent feedback was in [2]. I removed printing from prune_subdir\nand simplified the strbuf handling in prune_tmp_file.\n\nReferences: [1]\nhttps://lore.kernel.org/git/pull.1076.v9.git.git.1637020263.gitgitgadget@gmail.com/\n[2]\nhttps://lore.kernel.org/git/CABPp-BH6m4q_EoX77bqLcpCN1HRfJ_XayeCV2O0sRybX53rPrw@mail.gmail.com/\n\nNeeraj Singh (2):\n  tmp-objdir: new API for creating temporary writable databases\n  tmp-objdir: disable ref updates when replacing the primary odb\n\n builtin/prune.c        | 20 ++++++++++++---\n builtin/receive-pack.c |  2 +-\n environment.c          |  9 +++++++\n object-file.c          | 50 ++++++++++++++++++++++++++++++++++++--\n object-store.h         | 26 ++++++++++++++++++++\n object.c               |  2 +-\n refs.c                 |  2 +-\n repository.c           |  2 ++\n repository.h           |  1 +\n tmp-objdir.c           | 55 +++++++++++++++++++++++++++++++++++++++---\n tmp-objdir.h           | 29 +++++++++++++++++++---\n 11 files changed, 183 insertions(+), 15 deletions(-)\n\n\nbase-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1091%2Fneerajsi-msft%2Fns%2Ftmp-objdir-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1091/neerajsi-msft/ns/tmp-objdir-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1091\n-- \ngitgitgadget\n"},{"id":"443054","messageId":"4ae4303595eaa225ffece900ea21b418a1796069.1638585658.git.gitgitgadget@gmail.com","threadId":"57028","inReplyTo":"pull.1091.git.1638585658.gitgitgadget@gmail.com","subject":"[PATCH 1/2] tmp-objdir: new API for creating temporary writable databases","fromName":"Neeraj Singh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-04T02:40:57Z","receivedAt":"2021-12-04T02:41:05Z","isPatch":true,"sender":{"key":"nksingh85@gmail.com","avatar":null},"body":"From: Neeraj Singh <neerajsi@microsoft.com>\n\nThe tmp_objdir API provides the ability to create temporary object\ndirectories, but was designed with the goal of having subprocesses\naccess these object stores, followed by the main process migrating\nobjects from it to the main object store or just deleting it.  The\nsubprocesses would view it as their primary datastore and write to it.\n\nHere we add the tmp_objdir_replace_primary_odb function that replaces\nthe current process's writable \"main\" object directory with the\nspecified one. The previous main object directory is restored in either\ntmp_objdir_migrate or tmp_objdir_destroy.\n\nFor the --remerge-diff usecase, add a new `will_destroy` flag in `struct\nobject_database` to mark ephemeral object databases that do not require\nfsync durability.\n\nAdd 'git prune' support for removing temporary object databases, and\nmake sure that they have a name starting with tmp_ and containing an\noperation-specific name.\n\nBased-on-patch-by: Elijah Newren <newren@gmail.com>\n\nSigned-off-by: Neeraj Singh <neerajsi@microsoft.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/prune.c        | 20 ++++++++++++---\n builtin/receive-pack.c |  2 +-\n environment.c          |  5 ++++\n object-file.c          | 44 +++++++++++++++++++++++++++++++--\n object-store.h         | 19 +++++++++++++++\n object.c               |  2 +-\n tmp-objdir.c           | 55 +++++++++++++++++++++++++++++++++++++++---\n tmp-objdir.h           | 29 +++++++++++++++++++---\n 8 files changed, 162 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/prune.c b/builtin/prune.c\nindex 485c9a3c56f..c2bcdc07db4 100644\n--- a/builtin/prune.c\n+++ b/builtin/prune.c\n@@ -26,10 +26,22 @@ static int prune_tmp_file(const char *fullpath)\n \t\treturn error(\"Could not stat '%s'\", fullpath);\n \tif (st.st_mtime > expire)\n \t\treturn 0;\n-\tif (show_only || verbose)\n-\t\tprintf(\"Removing stale temporary file %s\\n\", fullpath);\n-\tif (!show_only)\n-\t\tunlink_or_warn(fullpath);\n+\tif (S_ISDIR(st.st_mode)) {\n+\t\tif (show_only || verbose)\n+\t\t\tprintf(\"Removing stale temporary directory %s\\n\", fullpath);\n+\t\tif (!show_only) {\n+\t\t\tstruct strbuf remove_dir_buf = STRBUF_INIT;\n+\n+\t\t\tstrbuf_addstr(&remove_dir_buf, fullpath);\n+\t\t\tremove_dir_recursively(&remove_dir_buf, 0);\n+\t\t\tstrbuf_release(&remove_dir_buf);\n+\t\t}\n+\t} else {\n+\t\tif (show_only || verbose)\n+\t\t\tprintf(\"Removing stale temporary file %s\\n\", fullpath);\n+\t\tif (!show_only)\n+\t\t\tunlink_or_warn(fullpath);\n+\t}\n \treturn 0;\n }\n \ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 49b846d9605..8815e24cde5 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -2213,7 +2213,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)\n \t\tstrvec_push(&child.args, alt_shallow_file);\n \t}\n \n-\ttmp_objdir = tmp_objdir_create();\n+\ttmp_objdir = tmp_objdir_create(\"incoming\");\n \tif (!tmp_objdir) {\n \t\tif (err_fd > 0)\n \t\t\tclose(err_fd);\ndiff --git a/environment.c b/environment.c\nindex 9da7f3c1a19..342400fcaad 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -17,6 +17,7 @@\n #include \"commit.h\"\n #include \"strvec.h\"\n #include \"object-store.h\"\n+#include \"tmp-objdir.h\"\n #include \"chdir-notify.h\"\n #include \"shallow.h\"\n \n@@ -331,10 +332,14 @@ static void update_relative_gitdir(const char *name,\n \t\t\t\t   void *data)\n {\n \tchar *path = reparent_relative_path(old_cwd, new_cwd, get_git_dir());\n+\tstruct tmp_objdir *tmp_objdir = tmp_objdir_unapply_primary_odb();\n \ttrace_printf_key(&trace_setup_key,\n \t\t\t \"setup: move $GIT_DIR to '%s'\",\n \t\t\t path);\n+\n \tset_git_dir_1(path);\n+\tif (tmp_objdir)\n+\t\ttmp_objdir_reapply_primary_odb(tmp_objdir, old_cwd, new_cwd);\n \tfree(path);\n }\n \ndiff --git a/object-file.c b/object-file.c\nindex c3d866a287e..0b6a61aeaff 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -683,6 +683,43 @@ void add_to_alternates_memory(const char *reference)\n \t\t\t     '\\n', NULL, 0);\n }\n \n+struct object_directory *set_temporary_primary_odb(const char *dir, int will_destroy)\n+{\n+\tstruct object_directory *new_odb;\n+\n+\t/*\n+\t * Make sure alternates are initialized, or else our entry may be\n+\t * overwritten when they are.\n+\t */\n+\tprepare_alt_odb(the_repository);\n+\n+\t/*\n+\t * Make a new primary odb and link the old primary ODB in as an\n+\t * alternate\n+\t */\n+\tnew_odb = xcalloc(1, sizeof(*new_odb));\n+\tnew_odb->path = xstrdup(dir);\n+\tnew_odb->will_destroy = will_destroy;\n+\tnew_odb->next = the_repository->objects->odb;\n+\tthe_repository->objects->odb = new_odb;\n+\treturn new_odb->next;\n+}\n+\n+void restore_primary_odb(struct object_directory *restore_odb, const char *old_path)\n+{\n+\tstruct object_directory *cur_odb = the_repository->objects->odb;\n+\n+\tif (strcmp(old_path, cur_odb->path))\n+\t\tBUG(\"expected %s as primary object store; found %s\",\n+\t\t    old_path, cur_odb->path);\n+\n+\tif (cur_odb->next != restore_odb)\n+\t\tBUG(\"we expect the old primary object store to be the first alternate\");\n+\n+\tthe_repository->objects->odb = restore_odb;\n+\tfree_object_directory(cur_odb);\n+}\n+\n /*\n  * Compute the exact path an alternate is at and returns it. In case of\n  * error NULL is returned and the human readable error is added to `err`\n@@ -1809,8 +1846,11 @@ int hash_object_file(const struct git_hash_algo *algo, const void *buf,\n /* Finalize a file on disk, and close it. */\n static void close_loose_object(int fd)\n {\n-\tif (fsync_object_files)\n-\t\tfsync_or_die(fd, \"loose object file\");\n+\tif (!the_repository->objects->odb->will_destroy) {\n+\t\tif (fsync_object_files)\n+\t\t\tfsync_or_die(fd, \"loose object file\");\n+\t}\n+\n \tif (close(fd) != 0)\n \t\tdie_errno(_(\"error when closing loose object file\"));\n }\ndiff --git a/object-store.h b/object-store.h\nindex 952efb6a4be..cb173e69392 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -27,6 +27,11 @@ struct object_directory {\n \tuint32_t loose_objects_subdir_seen[8]; /* 256 bits */\n \tstruct oidtree *loose_objects_cache;\n \n+\t/*\n+\t * This object store is ephemeral, so there is no need to fsync.\n+\t */\n+\tint will_destroy;\n+\n \t/*\n \t * Path to the alternative object store. If this is a relative path,\n \t * it is relative to the current working directory.\n@@ -58,6 +63,17 @@ void add_to_alternates_file(const char *dir);\n  */\n void add_to_alternates_memory(const char *dir);\n \n+/*\n+ * Replace the current writable object directory with the specified temporary\n+ * object directory; returns the former primary object directory.\n+ */\n+struct object_directory *set_temporary_primary_odb(const char *dir, int will_destroy);\n+\n+/*\n+ * Restore a previous ODB replaced by set_temporary_main_odb.\n+ */\n+void restore_primary_odb(struct object_directory *restore_odb, const char *old_path);\n+\n /*\n  * Populate and return the loose object cache array corresponding to the\n  * given object ID.\n@@ -68,6 +84,9 @@ struct oidtree *odb_loose_cache(struct object_directory *odb,\n /* Empty the loose object cache for the specified object directory. */\n void odb_clear_loose_cache(struct object_directory *odb);\n \n+/* Clear and free the specified object directory */\n+void free_object_directory(struct object_directory *odb);\n+\n struct packed_git {\n \tstruct hashmap_entry packmap_ent;\n \tstruct packed_git *next;\ndiff --git a/object.c b/object.c\nindex 23a24e678a8..048f96a260e 100644\n--- a/object.c\n+++ b/object.c\n@@ -513,7 +513,7 @@ struct raw_object_store *raw_object_store_new(void)\n \treturn o;\n }\n \n-static void free_object_directory(struct object_directory *odb)\n+void free_object_directory(struct object_directory *odb)\n {\n \tfree(odb->path);\n \todb_clear_loose_cache(odb);\ndiff --git a/tmp-objdir.c b/tmp-objdir.c\nindex b8d880e3626..3d38eeab66b 100644\n--- a/tmp-objdir.c\n+++ b/tmp-objdir.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"tmp-objdir.h\"\n+#include \"chdir-notify.h\"\n #include \"dir.h\"\n #include \"sigchain.h\"\n #include \"string-list.h\"\n@@ -11,6 +12,8 @@\n struct tmp_objdir {\n \tstruct strbuf path;\n \tstruct strvec env;\n+\tstruct object_directory *prev_odb;\n+\tint will_destroy;\n };\n \n /*\n@@ -38,6 +41,9 @@ static int tmp_objdir_destroy_1(struct tmp_objdir *t, int on_signal)\n \tif (t == the_tmp_objdir)\n \t\tthe_tmp_objdir = NULL;\n \n+\tif (!on_signal && t->prev_odb)\n+\t\trestore_primary_odb(t->prev_odb, t->path.buf);\n+\n \t/*\n \t * This may use malloc via strbuf_grow(), but we should\n \t * have pre-grown t->path sufficiently so that this\n@@ -52,6 +58,7 @@ static int tmp_objdir_destroy_1(struct tmp_objdir *t, int on_signal)\n \t */\n \tif (!on_signal)\n \t\ttmp_objdir_free(t);\n+\n \treturn err;\n }\n \n@@ -121,7 +128,7 @@ static int setup_tmp_objdir(const char *root)\n \treturn ret;\n }\n \n-struct tmp_objdir *tmp_objdir_create(void)\n+struct tmp_objdir *tmp_objdir_create(const char *prefix)\n {\n \tstatic int installed_handlers;\n \tstruct tmp_objdir *t;\n@@ -129,11 +136,16 @@ struct tmp_objdir *tmp_objdir_create(void)\n \tif (the_tmp_objdir)\n \t\tBUG(\"only one tmp_objdir can be used at a time\");\n \n-\tt = xmalloc(sizeof(*t));\n+\tt = xcalloc(1, sizeof(*t));\n \tstrbuf_init(&t->path, 0);\n \tstrvec_init(&t->env);\n \n-\tstrbuf_addf(&t->path, \"%s/incoming-XXXXXX\", get_object_directory());\n+\t/*\n+\t * Use a string starting with tmp_ so that the builtin/prune.c code\n+\t * can recognize any stale objdirs left behind by a crash and delete\n+\t * them.\n+\t */\n+\tstrbuf_addf(&t->path, \"%s/tmp_objdir-%s-XXXXXX\", get_object_directory(), prefix);\n \n \t/*\n \t * Grow the strbuf beyond any filename we expect to be placed in it.\n@@ -269,6 +281,13 @@ int tmp_objdir_migrate(struct tmp_objdir *t)\n \tif (!t)\n \t\treturn 0;\n \n+\tif (t->prev_odb) {\n+\t\tif (the_repository->objects->odb->will_destroy)\n+\t\t\tBUG(\"migrating an ODB that was marked for destruction\");\n+\t\trestore_primary_odb(t->prev_odb, t->path.buf);\n+\t\tt->prev_odb = NULL;\n+\t}\n+\n \tstrbuf_addbuf(&src, &t->path);\n \tstrbuf_addstr(&dst, get_object_directory());\n \n@@ -292,3 +311,33 @@ void tmp_objdir_add_as_alternate(const struct tmp_objdir *t)\n {\n \tadd_to_alternates_memory(t->path.buf);\n }\n+\n+void tmp_objdir_replace_primary_odb(struct tmp_objdir *t, int will_destroy)\n+{\n+\tif (t->prev_odb)\n+\t\tBUG(\"the primary object database is already replaced\");\n+\tt->prev_odb = set_temporary_primary_odb(t->path.buf, will_destroy);\n+\tt->will_destroy = will_destroy;\n+}\n+\n+struct tmp_objdir *tmp_objdir_unapply_primary_odb(void)\n+{\n+\tif (!the_tmp_objdir || !the_tmp_objdir->prev_odb)\n+\t\treturn NULL;\n+\n+\trestore_primary_odb(the_tmp_objdir->prev_odb, the_tmp_objdir->path.buf);\n+\tthe_tmp_objdir->prev_odb = NULL;\n+\treturn the_tmp_objdir;\n+}\n+\n+void tmp_objdir_reapply_primary_odb(struct tmp_objdir *t, const char *old_cwd,\n+\t\tconst char *new_cwd)\n+{\n+\tchar *path;\n+\n+\tpath = reparent_relative_path(old_cwd, new_cwd, t->path.buf);\n+\tstrbuf_reset(&t->path);\n+\tstrbuf_addstr(&t->path, path);\n+\tfree(path);\n+\ttmp_objdir_replace_primary_odb(t, t->will_destroy);\n+}\ndiff --git a/tmp-objdir.h b/tmp-objdir.h\nindex b1e45b4c75d..a3145051f25 100644\n--- a/tmp-objdir.h\n+++ b/tmp-objdir.h\n@@ -10,7 +10,7 @@\n  *\n  * Example:\n  *\n- *\tstruct tmp_objdir *t = tmp_objdir_create();\n+ *\tstruct tmp_objdir *t = tmp_objdir_create(\"incoming\");\n  *\tif (!run_command_v_opt_cd_env(cmd, 0, NULL, tmp_objdir_env(t)) &&\n  *\t    !tmp_objdir_migrate(t))\n  *\t\tprintf(\"success!\\n\");\n@@ -22,9 +22,10 @@\n struct tmp_objdir;\n \n /*\n- * Create a new temporary object directory; returns NULL on failure.\n+ * Create a new temporary object directory with the specified prefix;\n+ * returns NULL on failure.\n  */\n-struct tmp_objdir *tmp_objdir_create(void);\n+struct tmp_objdir *tmp_objdir_create(const char *prefix);\n \n /*\n  * Return a list of environment strings, suitable for use with\n@@ -51,4 +52,26 @@ int tmp_objdir_destroy(struct tmp_objdir *);\n  */\n void tmp_objdir_add_as_alternate(const struct tmp_objdir *);\n \n+/*\n+ * Replaces the main object store in the current process with the temporary\n+ * object directory and makes the former main object store an alternate.\n+ * If will_destroy is nonzero, the object directory may not be migrated.\n+ */\n+void tmp_objdir_replace_primary_odb(struct tmp_objdir *, int will_destroy);\n+\n+/*\n+ * If the primary object database was replaced by a temporary object directory,\n+ * restore it to its original value while keeping the directory contents around.\n+ * Returns NULL if the primary object database was not replaced.\n+ */\n+struct tmp_objdir *tmp_objdir_unapply_primary_odb(void);\n+\n+/*\n+ * Reapplies the former primary temporary object database, after protentially\n+ * changing its relative path.\n+ */\n+void tmp_objdir_reapply_primary_odb(struct tmp_objdir *, const char *old_cwd,\n+\t\tconst char *new_cwd);\n+\n+\n #endif /* TMP_OBJDIR_H */\n-- \ngitgitgadget\n\n"},{"id":"443055","messageId":"d8ae001500c788cdabf4e6918da0a7ce89a48fc6.1638585658.git.gitgitgadget@gmail.com","threadId":"57028","inReplyTo":"pull.1091.git.1638585658.gitgitgadget@gmail.com","subject":"[PATCH 2/2] tmp-objdir: disable ref updates when replacing the primary odb","fromName":"Neeraj Singh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-04T02:40:58Z","receivedAt":"2021-12-04T02:41:05Z","isPatch":true,"sender":{"key":"nksingh85@gmail.com","avatar":null},"body":"From: Neeraj Singh <neerajsi@microsoft.com>\n\nWhen creating a subprocess with a temporary ODB, we set the\nGIT_QUARANTINE_ENVIRONMENT env var to tell child Git processes not\nto update refs, since the tmp-objdir may go away.\n\nIntroduce a similar mechanism for in-process temporary ODBs when\nwe call tmp_objdir_replace_primary_odb. Now both mechanisms set\nthe disable_ref_updates flag on the odb, which is queried by\nthe ref_transaction_prepare function.\n\nNote: This change adds an assumption that the state of\nthe_repository is relevant for any ref transaction that might\nbe initiated. Unwinding this assumption should be straightforward\nby saving the relevant repository to query in the transaction or\nthe ref_store.\n\nPeff's test case was invoking ref updates via the cachetextconv\nsetting. That particular code silently does nothing when a ref\nupdate is forbidden. See the call to notes_cache_put in\nfill_textconv where errors are ignored.\n\nReported-by: Jeff King <peff@peff.net>\n\nSigned-off-by: Neeraj Singh <neerajsi@microsoft.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n environment.c  | 4 ++++\n object-file.c  | 6 ++++++\n object-store.h | 9 ++++++++-\n refs.c         | 2 +-\n repository.c   | 2 ++\n repository.h   | 1 +\n 6 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/environment.c b/environment.c\nindex 342400fcaad..2701dfeeec8 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -169,6 +169,10 @@ void setup_git_env(const char *git_dir)\n \targs.graft_file = getenv_safe(&to_free, GRAFT_ENVIRONMENT);\n \targs.index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT);\n \targs.alternate_db = getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT);\n+\tif (getenv(GIT_QUARANTINE_ENVIRONMENT)) {\n+\t\targs.disable_ref_updates = 1;\n+\t}\n+\n \trepo_set_gitdir(the_repository, git_dir, &args);\n \tstrvec_clear(&to_free);\n \ndiff --git a/object-file.c b/object-file.c\nindex 0b6a61aeaff..659ef7623ff 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -699,6 +699,12 @@ struct object_directory *set_temporary_primary_odb(const char *dir, int will_des\n \t */\n \tnew_odb = xcalloc(1, sizeof(*new_odb));\n \tnew_odb->path = xstrdup(dir);\n+\n+\t/*\n+\t * Disable ref updates while a temporary odb is active, since\n+\t * the objects in the database may roll back.\n+\t */\n+\tnew_odb->disable_ref_updates = 1;\n \tnew_odb->will_destroy = will_destroy;\n \tnew_odb->next = the_repository->objects->odb;\n \tthe_repository->objects->odb = new_odb;\ndiff --git a/object-store.h b/object-store.h\nindex cb173e69392..9ae9262c340 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -27,10 +27,17 @@ struct object_directory {\n \tuint32_t loose_objects_subdir_seen[8]; /* 256 bits */\n \tstruct oidtree *loose_objects_cache;\n \n+\t/*\n+\t * This is a temporary object store created by the tmp_objdir\n+\t * facility. Disable ref updates since the objects in the store\n+\t * might be discarded on rollback.\n+\t */\n+\tunsigned int disable_ref_updates : 1;\n+\n \t/*\n \t * This object store is ephemeral, so there is no need to fsync.\n \t */\n-\tint will_destroy;\n+\tunsigned int will_destroy : 1;\n \n \t/*\n \t * Path to the alternative object store. If this is a relative path,\ndiff --git a/refs.c b/refs.c\nindex d7cc0a23a3b..27ec7d1fc64 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2137,7 +2137,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,\n \t\tbreak;\n \t}\n \n-\tif (getenv(GIT_QUARANTINE_ENVIRONMENT)) {\n+\tif (the_repository->objects->odb->disable_ref_updates) {\n \t\tstrbuf_addstr(err,\n \t\t\t      _(\"ref updates forbidden inside quarantine environment\"));\n \t\treturn -1;\ndiff --git a/repository.c b/repository.c\nindex c5b90ba93ea..dce8e35ac20 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -80,6 +80,8 @@ void repo_set_gitdir(struct repository *repo,\n \texpand_base_dir(&repo->objects->odb->path, o->object_dir,\n \t\t\trepo->commondir, \"objects\");\n \n+\trepo->objects->odb->disable_ref_updates = o->disable_ref_updates;\n+\n \tfree(repo->objects->alternate_db);\n \trepo->objects->alternate_db = xstrdup_or_null(o->alternate_db);\n \texpand_base_dir(&repo->graft_file, o->graft_file,\ndiff --git a/repository.h b/repository.h\nindex a057653981c..7c04e99ac5c 100644\n--- a/repository.h\n+++ b/repository.h\n@@ -158,6 +158,7 @@ struct set_gitdir_args {\n \tconst char *graft_file;\n \tconst char *index_file;\n \tconst char *alternate_db;\n+\tint disable_ref_updates;\n };\n \n void repo_set_gitdir(struct repository *repo, const char *root,\n-- \ngitgitgadget\n"},{"id":"443109","messageId":"xmqqtufmlxmb.fsf@gitster.g","threadId":"57028","inReplyTo":"d8ae001500c788cdabf4e6918da0a7ce89a48fc6.1638585658.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] tmp-objdir: disable ref updates when replacing the primary odb","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-05T18:23:08Z","receivedAt":"2021-12-05T18:23:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Neeraj Singh via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n>  \t/*\n>  \t * This object store is ephemeral, so there is no need to fsync.\n>  \t */\n> -\tint will_destroy;\n> +\tunsigned int will_destroy : 1;\n\n<CANQDOddCC7+gGUy1VBxxwvN7ieP+N8mQhbxK2xx6ySqZc6U7-g@mail.gmail.com>\n?\n\n(https://github.com/git/git/pull/1076#discussion_r750645345)\n"},{"id":"443121","messageId":"20211205234408.GA26229@neerajsi-x1.localdomain","threadId":"57028","inReplyTo":"xmqqtufmlxmb.fsf@gitster.g","subject":"Re: [PATCH 2/2] tmp-objdir: disable ref updates when replacing the primary odb","fromName":"Neeraj Singh","fromEmail":"nksingh85@gmail.com","sentAt":"2021-12-05T23:44:08Z","receivedAt":"2021-12-05T23:44:13Z","isPatch":true,"sender":{"key":"nksingh85@gmail.com","avatar":null},"body":"On Sun, Dec 05, 2021 at 10:23:08AM -0800, Junio C Hamano wrote:\n> \"Neeraj Singh via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n> >  \t/*\n> >  \t * This object store is ephemeral, so there is no need to fsync.\n> >  \t */\n> > -\tint will_destroy;\n> > +\tunsigned int will_destroy : 1;\n> \n> <CANQDOddCC7+gGUy1VBxxwvN7ieP+N8mQhbxK2xx6ySqZc6U7-g@mail.gmail.com>\n> ?\n> \n> (https://github.com/git/git/pull/1076#discussion_r750645345)\n\nThanks for noticing this! I also lost one other change\nwhile splitting this out: we are referencing\nthe_repository from the refs code, but as of 34224e14d we\nshould be picking it up from the ref_store. I'll submit\nan updated series as soon as it passes CI.\n\nThanks,\nNeeraj\n"},{"id":"443122","messageId":"xmqqsfv6ip1y.fsf@gitster.g","threadId":"57028","inReplyTo":"20211205234408.GA26229@neerajsi-x1.localdomain","subject":"Re: [PATCH 2/2] tmp-objdir: disable ref updates when replacing the primary odb","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-05T23:56:25Z","receivedAt":"2021-12-05T23:56:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Neeraj Singh <nksingh85@gmail.com> writes:\n\n> On Sun, Dec 05, 2021 at 10:23:08AM -0800, Junio C Hamano wrote:\n>> \"Neeraj Singh via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>> \n>> >  \t/*\n>> >  \t * This object store is ephemeral, so there is no need to fsync.\n>> >  \t */\n>> > -\tint will_destroy;\n>> > +\tunsigned int will_destroy : 1;\n>> \n>> <CANQDOddCC7+gGUy1VBxxwvN7ieP+N8mQhbxK2xx6ySqZc6U7-g@mail.gmail.com>\n>> ?\n>> \n>> (https://github.com/git/git/pull/1076#discussion_r750645345)\n>\n> Thanks for noticing this! I also lost one other change\n> while splitting this out: we are referencing\n> the_repository from the refs code, but as of 34224e14d we\n> should be picking it up from the ref_store. I'll submit\n> an updated series as soon as it passes CI.\n\nNo rush.\n\nReviewers and other project participants would appreciate you more\nif you took a deep breath, after seeing a CI success, and gave a\nfinal re-reading of the patches with a critical pair of eyes, before\nyou send the updated series out.\n\nThanks.\n"},{"id":"443123","messageId":"pull.1091.v2.git.1638750965.gitgitgadget@gmail.com","threadId":"57028","inReplyTo":"pull.1091.git.1638585658.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] ns/tmp-objdir: add support for temporary writable databases","fromName":"Neeraj K. Singh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-06T00:36:03Z","receivedAt":"2021-12-06T00:36:10Z","isPatch":true,"sender":{"key":"name:Neeraj K. Singh","avatar":null},"body":"V2 changes: I lost a couple changes in the shuffle while splitting these\nchanges out\n\n * Make the will-destroy boolean a single bit field of type unsigned int so\n   that it doesn't change twice in this small patch series.\n * Remove a the_repository reference in the disable ref updates code. Now\n   the repository is taken from the ref_store.\n\nNew interface into the tmp-objdir API to help in-core use of the quarantine\nfeature.\n\nThis patch series was formerly part of the ns/batched-fsync topic [1]. It's\nnow split out into its own gitgitgadget PR and discussion thread since it is\nthe base for en/remerge-diff as well.\n\nThe most recent feedback was in [2]. I removed printing from prune_subdir\nand simplified the strbuf handling in prune_tmp_file.\n\nReferences: [1]\nhttps://lore.kernel.org/git/pull.1076.v9.git.git.1637020263.gitgitgadget@gmail.com/\n[2]\nhttps://lore.kernel.org/git/CABPp-BH6m4q_EoX77bqLcpCN1HRfJ_XayeCV2O0sRybX53rPrw@mail.gmail.com/\n\nNeeraj Singh (2):\n  tmp-objdir: new API for creating temporary writable databases\n  tmp-objdir: disable ref updates when replacing the primary odb\n\n builtin/prune.c        | 20 ++++++++++++---\n builtin/receive-pack.c |  2 +-\n environment.c          |  9 +++++++\n object-file.c          | 50 ++++++++++++++++++++++++++++++++++++--\n object-store.h         | 26 ++++++++++++++++++++\n object.c               |  2 +-\n refs.c                 |  2 +-\n repository.c           |  2 ++\n repository.h           |  1 +\n tmp-objdir.c           | 55 +++++++++++++++++++++++++++++++++++++++---\n tmp-objdir.h           | 29 +++++++++++++++++++---\n 11 files changed, 183 insertions(+), 15 deletions(-)\n\n\nbase-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1091%2Fneerajsi-msft%2Fns%2Ftmp-objdir-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1091/neerajsi-msft/ns/tmp-objdir-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1091\n\nRange-diff vs v1:\n\n 1:  4ae4303595e ! 1:  36c00613d9a tmp-objdir: new API for creating temporary writable databases\n     @@ object-store.h: struct object_directory {\n      +\t/*\n      +\t * This object store is ephemeral, so there is no need to fsync.\n      +\t */\n     -+\tint will_destroy;\n     ++\tunsigned int will_destroy : 1;\n      +\n       \t/*\n       \t * Path to the alternative object store. If this is a relative path,\n 2:  d8ae001500c ! 2:  f667cbcc47d tmp-objdir: disable ref updates when replacing the primary odb\n     @@ object-store.h: struct object_directory {\n       \t/*\n       \t * This object store is ephemeral, so there is no need to fsync.\n       \t */\n     --\tint will_destroy;\n     -+\tunsigned int will_destroy : 1;\n     - \n     - \t/*\n     - \t * Path to the alternative object store. If this is a relative path,\n      \n       ## refs.c ##\n      @@ refs.c: int ref_transaction_prepare(struct ref_transaction *transaction,\n     @@ refs.c: int ref_transaction_prepare(struct ref_transaction *transaction,\n       \t}\n       \n      -\tif (getenv(GIT_QUARANTINE_ENVIRONMENT)) {\n     -+\tif (the_repository->objects->odb->disable_ref_updates) {\n     ++\tif (refs->repo->objects->odb->disable_ref_updates) {\n       \t\tstrbuf_addstr(err,\n       \t\t\t      _(\"ref updates forbidden inside quarantine environment\"));\n       \t\treturn -1;\n\n-- \ngitgitgadget\n"},{"id":"443125","messageId":"36c00613d9a6ad4fc768e15b9ec23f9af520338a.1638750965.git.gitgitgadget@gmail.com","threadId":"57028","inReplyTo":"pull.1091.v2.git.1638750965.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] tmp-objdir: new API for creating temporary writable databases","fromName":"Neeraj Singh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-06T00:36:04Z","receivedAt":"2021-12-06T00:36:15Z","isPatch":true,"sender":{"key":"nksingh85@gmail.com","avatar":null},"body":"From: Neeraj Singh <neerajsi@microsoft.com>\n\nThe tmp_objdir API provides the ability to create temporary object\ndirectories, but was designed with the goal of having subprocesses\naccess these object stores, followed by the main process migrating\nobjects from it to the main object store or just deleting it.  The\nsubprocesses would view it as their primary datastore and write to it.\n\nHere we add the tmp_objdir_replace_primary_odb function that replaces\nthe current process's writable \"main\" object directory with the\nspecified one. The previous main object directory is restored in either\ntmp_objdir_migrate or tmp_objdir_destroy.\n\nFor the --remerge-diff usecase, add a new `will_destroy` flag in `struct\nobject_database` to mark ephemeral object databases that do not require\nfsync durability.\n\nAdd 'git prune' support for removing temporary object databases, and\nmake sure that they have a name starting with tmp_ and containing an\noperation-specific name.\n\nBased-on-patch-by: Elijah Newren <newren@gmail.com>\n\nSigned-off-by: Neeraj Singh <neerajsi@microsoft.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/prune.c        | 20 ++++++++++++---\n builtin/receive-pack.c |  2 +-\n environment.c          |  5 ++++\n object-file.c          | 44 +++++++++++++++++++++++++++++++--\n object-store.h         | 19 +++++++++++++++\n object.c               |  2 +-\n tmp-objdir.c           | 55 +++++++++++++++++++++++++++++++++++++++---\n tmp-objdir.h           | 29 +++++++++++++++++++---\n 8 files changed, 162 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/prune.c b/builtin/prune.c\nindex 485c9a3c56f..c2bcdc07db4 100644\n--- a/builtin/prune.c\n+++ b/builtin/prune.c\n@@ -26,10 +26,22 @@ static int prune_tmp_file(const char *fullpath)\n \t\treturn error(\"Could not stat '%s'\", fullpath);\n \tif (st.st_mtime > expire)\n \t\treturn 0;\n-\tif (show_only || verbose)\n-\t\tprintf(\"Removing stale temporary file %s\\n\", fullpath);\n-\tif (!show_only)\n-\t\tunlink_or_warn(fullpath);\n+\tif (S_ISDIR(st.st_mode)) {\n+\t\tif (show_only || verbose)\n+\t\t\tprintf(\"Removing stale temporary directory %s\\n\", fullpath);\n+\t\tif (!show_only) {\n+\t\t\tstruct strbuf remove_dir_buf = STRBUF_INIT;\n+\n+\t\t\tstrbuf_addstr(&remove_dir_buf, fullpath);\n+\t\t\tremove_dir_recursively(&remove_dir_buf, 0);\n+\t\t\tstrbuf_release(&remove_dir_buf);\n+\t\t}\n+\t} else {\n+\t\tif (show_only || verbose)\n+\t\t\tprintf(\"Removing stale temporary file %s\\n\", fullpath);\n+\t\tif (!show_only)\n+\t\t\tunlink_or_warn(fullpath);\n+\t}\n \treturn 0;\n }\n \ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 49b846d9605..8815e24cde5 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -2213,7 +2213,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)\n \t\tstrvec_push(&child.args, alt_shallow_file);\n \t}\n \n-\ttmp_objdir = tmp_objdir_create();\n+\ttmp_objdir = tmp_objdir_create(\"incoming\");\n \tif (!tmp_objdir) {\n \t\tif (err_fd > 0)\n \t\t\tclose(err_fd);\ndiff --git a/environment.c b/environment.c\nindex 9da7f3c1a19..342400fcaad 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -17,6 +17,7 @@\n #include \"commit.h\"\n #include \"strvec.h\"\n #include \"object-store.h\"\n+#include \"tmp-objdir.h\"\n #include \"chdir-notify.h\"\n #include \"shallow.h\"\n \n@@ -331,10 +332,14 @@ static void update_relative_gitdir(const char *name,\n \t\t\t\t   void *data)\n {\n \tchar *path = reparent_relative_path(old_cwd, new_cwd, get_git_dir());\n+\tstruct tmp_objdir *tmp_objdir = tmp_objdir_unapply_primary_odb();\n \ttrace_printf_key(&trace_setup_key,\n \t\t\t \"setup: move $GIT_DIR to '%s'\",\n \t\t\t path);\n+\n \tset_git_dir_1(path);\n+\tif (tmp_objdir)\n+\t\ttmp_objdir_reapply_primary_odb(tmp_objdir, old_cwd, new_cwd);\n \tfree(path);\n }\n \ndiff --git a/object-file.c b/object-file.c\nindex c3d866a287e..0b6a61aeaff 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -683,6 +683,43 @@ void add_to_alternates_memory(const char *reference)\n \t\t\t     '\\n', NULL, 0);\n }\n \n+struct object_directory *set_temporary_primary_odb(const char *dir, int will_destroy)\n+{\n+\tstruct object_directory *new_odb;\n+\n+\t/*\n+\t * Make sure alternates are initialized, or else our entry may be\n+\t * overwritten when they are.\n+\t */\n+\tprepare_alt_odb(the_repository);\n+\n+\t/*\n+\t * Make a new primary odb and link the old primary ODB in as an\n+\t * alternate\n+\t */\n+\tnew_odb = xcalloc(1, sizeof(*new_odb));\n+\tnew_odb->path = xstrdup(dir);\n+\tnew_odb->will_destroy = will_destroy;\n+\tnew_odb->next = the_repository->objects->odb;\n+\tthe_repository->objects->odb = new_odb;\n+\treturn new_odb->next;\n+}\n+\n+void restore_primary_odb(struct object_directory *restore_odb, const char *old_path)\n+{\n+\tstruct object_directory *cur_odb = the_repository->objects->odb;\n+\n+\tif (strcmp(old_path, cur_odb->path))\n+\t\tBUG(\"expected %s as primary object store; found %s\",\n+\t\t    old_path, cur_odb->path);\n+\n+\tif (cur_odb->next != restore_odb)\n+\t\tBUG(\"we expect the old primary object store to be the first alternate\");\n+\n+\tthe_repository->objects->odb = restore_odb;\n+\tfree_object_directory(cur_odb);\n+}\n+\n /*\n  * Compute the exact path an alternate is at and returns it. In case of\n  * error NULL is returned and the human readable error is added to `err`\n@@ -1809,8 +1846,11 @@ int hash_object_file(const struct git_hash_algo *algo, const void *buf,\n /* Finalize a file on disk, and close it. */\n static void close_loose_object(int fd)\n {\n-\tif (fsync_object_files)\n-\t\tfsync_or_die(fd, \"loose object file\");\n+\tif (!the_repository->objects->odb->will_destroy) {\n+\t\tif (fsync_object_files)\n+\t\t\tfsync_or_die(fd, \"loose object file\");\n+\t}\n+\n \tif (close(fd) != 0)\n \t\tdie_errno(_(\"error when closing loose object file\"));\n }\ndiff --git a/object-store.h b/object-store.h\nindex 952efb6a4be..82cf13f1054 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -27,6 +27,11 @@ struct object_directory {\n \tuint32_t loose_objects_subdir_seen[8]; /* 256 bits */\n \tstruct oidtree *loose_objects_cache;\n \n+\t/*\n+\t * This object store is ephemeral, so there is no need to fsync.\n+\t */\n+\tunsigned int will_destroy : 1;\n+\n \t/*\n \t * Path to the alternative object store. If this is a relative path,\n \t * it is relative to the current working directory.\n@@ -58,6 +63,17 @@ void add_to_alternates_file(const char *dir);\n  */\n void add_to_alternates_memory(const char *dir);\n \n+/*\n+ * Replace the current writable object directory with the specified temporary\n+ * object directory; returns the former primary object directory.\n+ */\n+struct object_directory *set_temporary_primary_odb(const char *dir, int will_destroy);\n+\n+/*\n+ * Restore a previous ODB replaced by set_temporary_main_odb.\n+ */\n+void restore_primary_odb(struct object_directory *restore_odb, const char *old_path);\n+\n /*\n  * Populate and return the loose object cache array corresponding to the\n  * given object ID.\n@@ -68,6 +84,9 @@ struct oidtree *odb_loose_cache(struct object_directory *odb,\n /* Empty the loose object cache for the specified object directory. */\n void odb_clear_loose_cache(struct object_directory *odb);\n \n+/* Clear and free the specified object directory */\n+void free_object_directory(struct object_directory *odb);\n+\n struct packed_git {\n \tstruct hashmap_entry packmap_ent;\n \tstruct packed_git *next;\ndiff --git a/object.c b/object.c\nindex 23a24e678a8..048f96a260e 100644\n--- a/object.c\n+++ b/object.c\n@@ -513,7 +513,7 @@ struct raw_object_store *raw_object_store_new(void)\n \treturn o;\n }\n \n-static void free_object_directory(struct object_directory *odb)\n+void free_object_directory(struct object_directory *odb)\n {\n \tfree(odb->path);\n \todb_clear_loose_cache(odb);\ndiff --git a/tmp-objdir.c b/tmp-objdir.c\nindex b8d880e3626..3d38eeab66b 100644\n--- a/tmp-objdir.c\n+++ b/tmp-objdir.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"tmp-objdir.h\"\n+#include \"chdir-notify.h\"\n #include \"dir.h\"\n #include \"sigchain.h\"\n #include \"string-list.h\"\n@@ -11,6 +12,8 @@\n struct tmp_objdir {\n \tstruct strbuf path;\n \tstruct strvec env;\n+\tstruct object_directory *prev_odb;\n+\tint will_destroy;\n };\n \n /*\n@@ -38,6 +41,9 @@ static int tmp_objdir_destroy_1(struct tmp_objdir *t, int on_signal)\n \tif (t == the_tmp_objdir)\n \t\tthe_tmp_objdir = NULL;\n \n+\tif (!on_signal && t->prev_odb)\n+\t\trestore_primary_odb(t->prev_odb, t->path.buf);\n+\n \t/*\n \t * This may use malloc via strbuf_grow(), but we should\n \t * have pre-grown t->path sufficiently so that this\n@@ -52,6 +58,7 @@ static int tmp_objdir_destroy_1(struct tmp_objdir *t, int on_signal)\n \t */\n \tif (!on_signal)\n \t\ttmp_objdir_free(t);\n+\n \treturn err;\n }\n \n@@ -121,7 +128,7 @@ static int setup_tmp_objdir(const char *root)\n \treturn ret;\n }\n \n-struct tmp_objdir *tmp_objdir_create(void)\n+struct tmp_objdir *tmp_objdir_create(const char *prefix)\n {\n \tstatic int installed_handlers;\n \tstruct tmp_objdir *t;\n@@ -129,11 +136,16 @@ struct tmp_objdir *tmp_objdir_create(void)\n \tif (the_tmp_objdir)\n \t\tBUG(\"only one tmp_objdir can be used at a time\");\n \n-\tt = xmalloc(sizeof(*t));\n+\tt = xcalloc(1, sizeof(*t));\n \tstrbuf_init(&t->path, 0);\n \tstrvec_init(&t->env);\n \n-\tstrbuf_addf(&t->path, \"%s/incoming-XXXXXX\", get_object_directory());\n+\t/*\n+\t * Use a string starting with tmp_ so that the builtin/prune.c code\n+\t * can recognize any stale objdirs left behind by a crash and delete\n+\t * them.\n+\t */\n+\tstrbuf_addf(&t->path, \"%s/tmp_objdir-%s-XXXXXX\", get_object_directory(), prefix);\n \n \t/*\n \t * Grow the strbuf beyond any filename we expect to be placed in it.\n@@ -269,6 +281,13 @@ int tmp_objdir_migrate(struct tmp_objdir *t)\n \tif (!t)\n \t\treturn 0;\n \n+\tif (t->prev_odb) {\n+\t\tif (the_repository->objects->odb->will_destroy)\n+\t\t\tBUG(\"migrating an ODB that was marked for destruction\");\n+\t\trestore_primary_odb(t->prev_odb, t->path.buf);\n+\t\tt->prev_odb = NULL;\n+\t}\n+\n \tstrbuf_addbuf(&src, &t->path);\n \tstrbuf_addstr(&dst, get_object_directory());\n \n@@ -292,3 +311,33 @@ void tmp_objdir_add_as_alternate(const struct tmp_objdir *t)\n {\n \tadd_to_alternates_memory(t->path.buf);\n }\n+\n+void tmp_objdir_replace_primary_odb(struct tmp_objdir *t, int will_destroy)\n+{\n+\tif (t->prev_odb)\n+\t\tBUG(\"the primary object database is already replaced\");\n+\tt->prev_odb = set_temporary_primary_odb(t->path.buf, will_destroy);\n+\tt->will_destroy = will_destroy;\n+}\n+\n+struct tmp_objdir *tmp_objdir_unapply_primary_odb(void)\n+{\n+\tif (!the_tmp_objdir || !the_tmp_objdir->prev_odb)\n+\t\treturn NULL;\n+\n+\trestore_primary_odb(the_tmp_objdir->prev_odb, the_tmp_objdir->path.buf);\n+\tthe_tmp_objdir->prev_odb = NULL;\n+\treturn the_tmp_objdir;\n+}\n+\n+void tmp_objdir_reapply_primary_odb(struct tmp_objdir *t, const char *old_cwd,\n+\t\tconst char *new_cwd)\n+{\n+\tchar *path;\n+\n+\tpath = reparent_relative_path(old_cwd, new_cwd, t->path.buf);\n+\tstrbuf_reset(&t->path);\n+\tstrbuf_addstr(&t->path, path);\n+\tfree(path);\n+\ttmp_objdir_replace_primary_odb(t, t->will_destroy);\n+}\ndiff --git a/tmp-objdir.h b/tmp-objdir.h\nindex b1e45b4c75d..a3145051f25 100644\n--- a/tmp-objdir.h\n+++ b/tmp-objdir.h\n@@ -10,7 +10,7 @@\n  *\n  * Example:\n  *\n- *\tstruct tmp_objdir *t = tmp_objdir_create();\n+ *\tstruct tmp_objdir *t = tmp_objdir_create(\"incoming\");\n  *\tif (!run_command_v_opt_cd_env(cmd, 0, NULL, tmp_objdir_env(t)) &&\n  *\t    !tmp_objdir_migrate(t))\n  *\t\tprintf(\"success!\\n\");\n@@ -22,9 +22,10 @@\n struct tmp_objdir;\n \n /*\n- * Create a new temporary object directory; returns NULL on failure.\n+ * Create a new temporary object directory with the specified prefix;\n+ * returns NULL on failure.\n  */\n-struct tmp_objdir *tmp_objdir_create(void);\n+struct tmp_objdir *tmp_objdir_create(const char *prefix);\n \n /*\n  * Return a list of environment strings, suitable for use with\n@@ -51,4 +52,26 @@ int tmp_objdir_destroy(struct tmp_objdir *);\n  */\n void tmp_objdir_add_as_alternate(const struct tmp_objdir *);\n \n+/*\n+ * Replaces the main object store in the current process with the temporary\n+ * object directory and makes the former main object store an alternate.\n+ * If will_destroy is nonzero, the object directory may not be migrated.\n+ */\n+void tmp_objdir_replace_primary_odb(struct tmp_objdir *, int will_destroy);\n+\n+/*\n+ * If the primary object database was replaced by a temporary object directory,\n+ * restore it to its original value while keeping the directory contents around.\n+ * Returns NULL if the primary object database was not replaced.\n+ */\n+struct tmp_objdir *tmp_objdir_unapply_primary_odb(void);\n+\n+/*\n+ * Reapplies the former primary temporary object database, after protentially\n+ * changing its relative path.\n+ */\n+void tmp_objdir_reapply_primary_odb(struct tmp_objdir *, const char *old_cwd,\n+\t\tconst char *new_cwd);\n+\n+\n #endif /* TMP_OBJDIR_H */\n-- \ngitgitgadget\n\n"},{"id":"443124","messageId":"f667cbcc47dd59d029f8712464f0551898a70b15.1638750965.git.gitgitgadget@gmail.com","threadId":"57028","inReplyTo":"pull.1091.v2.git.1638750965.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] tmp-objdir: disable ref updates when replacing the primary odb","fromName":"Neeraj Singh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-06T00:36:05Z","receivedAt":"2021-12-06T00:36:16Z","isPatch":true,"sender":{"key":"nksingh85@gmail.com","avatar":null},"body":"From: Neeraj Singh <neerajsi@microsoft.com>\n\nWhen creating a subprocess with a temporary ODB, we set the\nGIT_QUARANTINE_ENVIRONMENT env var to tell child Git processes not\nto update refs, since the tmp-objdir may go away.\n\nIntroduce a similar mechanism for in-process temporary ODBs when\nwe call tmp_objdir_replace_primary_odb. Now both mechanisms set\nthe disable_ref_updates flag on the odb, which is queried by\nthe ref_transaction_prepare function.\n\nNote: This change adds an assumption that the state of\nthe_repository is relevant for any ref transaction that might\nbe initiated. Unwinding this assumption should be straightforward\nby saving the relevant repository to query in the transaction or\nthe ref_store.\n\nPeff's test case was invoking ref updates via the cachetextconv\nsetting. That particular code silently does nothing when a ref\nupdate is forbidden. See the call to notes_cache_put in\nfill_textconv where errors are ignored.\n\nReported-by: Jeff King <peff@peff.net>\n\nSigned-off-by: Neeraj Singh <neerajsi@microsoft.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n environment.c  | 4 ++++\n object-file.c  | 6 ++++++\n object-store.h | 7 +++++++\n refs.c         | 2 +-\n repository.c   | 2 ++\n repository.h   | 1 +\n 6 files changed, 21 insertions(+), 1 deletion(-)\n\ndiff --git a/environment.c b/environment.c\nindex 342400fcaad..2701dfeeec8 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -169,6 +169,10 @@ void setup_git_env(const char *git_dir)\n \targs.graft_file = getenv_safe(&to_free, GRAFT_ENVIRONMENT);\n \targs.index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT);\n \targs.alternate_db = getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT);\n+\tif (getenv(GIT_QUARANTINE_ENVIRONMENT)) {\n+\t\targs.disable_ref_updates = 1;\n+\t}\n+\n \trepo_set_gitdir(the_repository, git_dir, &args);\n \tstrvec_clear(&to_free);\n \ndiff --git a/object-file.c b/object-file.c\nindex 0b6a61aeaff..659ef7623ff 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -699,6 +699,12 @@ struct object_directory *set_temporary_primary_odb(const char *dir, int will_des\n \t */\n \tnew_odb = xcalloc(1, sizeof(*new_odb));\n \tnew_odb->path = xstrdup(dir);\n+\n+\t/*\n+\t * Disable ref updates while a temporary odb is active, since\n+\t * the objects in the database may roll back.\n+\t */\n+\tnew_odb->disable_ref_updates = 1;\n \tnew_odb->will_destroy = will_destroy;\n \tnew_odb->next = the_repository->objects->odb;\n \tthe_repository->objects->odb = new_odb;\ndiff --git a/object-store.h b/object-store.h\nindex 82cf13f1054..9ae9262c340 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -27,6 +27,13 @@ struct object_directory {\n \tuint32_t loose_objects_subdir_seen[8]; /* 256 bits */\n \tstruct oidtree *loose_objects_cache;\n \n+\t/*\n+\t * This is a temporary object store created by the tmp_objdir\n+\t * facility. Disable ref updates since the objects in the store\n+\t * might be discarded on rollback.\n+\t */\n+\tunsigned int disable_ref_updates : 1;\n+\n \t/*\n \t * This object store is ephemeral, so there is no need to fsync.\n \t */\ndiff --git a/refs.c b/refs.c\nindex d7cc0a23a3b..ac744e85f5f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2137,7 +2137,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,\n \t\tbreak;\n \t}\n \n-\tif (getenv(GIT_QUARANTINE_ENVIRONMENT)) {\n+\tif (refs->repo->objects->odb->disable_ref_updates) {\n \t\tstrbuf_addstr(err,\n \t\t\t      _(\"ref updates forbidden inside quarantine environment\"));\n \t\treturn -1;\ndiff --git a/repository.c b/repository.c\nindex c5b90ba93ea..dce8e35ac20 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -80,6 +80,8 @@ void repo_set_gitdir(struct repository *repo,\n \texpand_base_dir(&repo->objects->odb->path, o->object_dir,\n \t\t\trepo->commondir, \"objects\");\n \n+\trepo->objects->odb->disable_ref_updates = o->disable_ref_updates;\n+\n \tfree(repo->objects->alternate_db);\n \trepo->objects->alternate_db = xstrdup_or_null(o->alternate_db);\n \texpand_base_dir(&repo->graft_file, o->graft_file,\ndiff --git a/repository.h b/repository.h\nindex a057653981c..7c04e99ac5c 100644\n--- a/repository.h\n+++ b/repository.h\n@@ -158,6 +158,7 @@ struct set_gitdir_args {\n \tconst char *graft_file;\n \tconst char *index_file;\n \tconst char *alternate_db;\n+\tint disable_ref_updates;\n };\n \n void repo_set_gitdir(struct repository *repo, const char *root,\n-- \ngitgitgadget\n"},{"id":"443132","messageId":"CANQDOdfyoEM0pELKWzoK5ZUrDqwWnQLtbAycESzCqRRdWyWUSA@mail.gmail.com","threadId":"57028","inReplyTo":"xmqqsfv6ip1y.fsf@gitster.g","subject":"Re: [PATCH 2/2] tmp-objdir: disable ref updates when replacing the primary odb","fromName":"Neeraj Singh","fromEmail":"nksingh85@gmail.com","sentAt":"2021-12-06T03:10:04Z","receivedAt":"2021-12-06T03:10:18Z","isPatch":true,"sender":{"key":"nksingh85@gmail.com","avatar":null},"body":"On Sun, Dec 5, 2021 at 3:56 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Neeraj Singh <nksingh85@gmail.com> writes:\n>\n> > On Sun, Dec 05, 2021 at 10:23:08AM -0800, Junio C Hamano wrote:\n> >> \"Neeraj Singh via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> >>\n> >> >    /*\n> >> >     * This object store is ephemeral, so there is no need to fsync.\n> >> >     */\n> >> > -  int will_destroy;\n> >> > +  unsigned int will_destroy : 1;\n> >>\n> >> <CANQDOddCC7+gGUy1VBxxwvN7ieP+N8mQhbxK2xx6ySqZc6U7-g@mail.gmail.com>\n> >> ?\n> >>\n> >> (https://github.com/git/git/pull/1076#discussion_r750645345)\n> >\n> > Thanks for noticing this! I also lost one other change\n> > while splitting this out: we are referencing\n> > the_repository from the refs code, but as of 34224e14d we\n> > should be picking it up from the ref_store. I'll submit\n> > an updated series as soon as it passes CI.\n>\n> No rush.\n>\n> Reviewers and other project participants would appreciate you more\n> if you took a deep breath, after seeing a CI success, and gave a\n> final re-reading of the patches with a critical pair of eyes, before\n> you send the updated series out.\n>\n> Thanks.\n\nFair enough.  Of course I didn't see your email before I resubmitted.\nThanks for the feedback.\n"},{"id":"443133","messageId":"CANQDOdfCHfJa4dFMhAKSJ2Ppp9KKYc9DnAe_VBWdZMd-bPgY6Q@mail.gmail.com","threadId":"57028","inReplyTo":"f667cbcc47dd59d029f8712464f0551898a70b15.1638750965.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] tmp-objdir: disable ref updates when replacing the primary odb","fromName":"Neeraj Singh","fromEmail":"nksingh85@gmail.com","sentAt":"2021-12-06T03:12:03Z","receivedAt":"2021-12-06T03:12:18Z","isPatch":true,"sender":{"key":"nksingh85@gmail.com","avatar":null},"body":"On Sun, Dec 5, 2021 at 4:36 PM Neeraj Singh via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> From: Neeraj Singh <neerajsi@microsoft.com>\n>\n> Note: This change adds an assumption that the state of\n> the_repository is relevant for any ref transaction that might\n> be initiated. Unwinding this assumption should be straightforward\n> by saving the relevant repository to query in the transaction or\n> the ref_store.\n>\n\nThis part of the commit description needs to be deleted, since it no\nlonger applies.  We pick up the repo from the ref_store now.\n"},{"id":"443139","messageId":"xmqq4k7mi3g4.fsf@gitster.g","threadId":"57028","inReplyTo":"36c00613d9a6ad4fc768e15b9ec23f9af520338a.1638750965.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/2] tmp-objdir: new API for creating temporary writable databases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-06T07:43:07Z","receivedAt":"2021-12-06T07:43:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Neeraj Singh via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> diff --git a/builtin/prune.c b/builtin/prune.c\n> index 485c9a3c56f..c2bcdc07db4 100644\n> --- a/builtin/prune.c\n> +++ b/builtin/prune.c\n> @@ -26,10 +26,22 @@ static int prune_tmp_file(const char *fullpath)\n>  \t\treturn error(\"Could not stat '%s'\", fullpath);\n>  \tif (st.st_mtime > expire)\n>  \t\treturn 0;\n> -\tif (show_only || verbose)\n> -\t\tprintf(\"Removing stale temporary file %s\\n\", fullpath);\n> -\tif (!show_only)\n> -\t\tunlink_or_warn(fullpath);\n> +\tif (S_ISDIR(st.st_mode)) {\n\nBecause the updated tmp_objdir_create() always uses \"tmp_objdir-\" as\nthe common prefix (instead of \"incoming-\" that we used to use,\nprune_cruft() will call this function not just for temporary files\nfor loose objects, but also for directories.  So a new code to do an\nequivalent of \"rm -fr\" is added here.  OK.\n\n> +\t\tif (show_only || verbose)\n> +\t\t\tprintf(\"Removing stale temporary directory %s\\n\", fullpath);\n> +\t\tif (!show_only) {\n> +\t\t\tstruct strbuf remove_dir_buf = STRBUF_INIT;\n> +\n> +\t\t\tstrbuf_addstr(&remove_dir_buf, fullpath);\n> +\t\t\tremove_dir_recursively(&remove_dir_buf, 0);\n> +\t\t\tstrbuf_release(&remove_dir_buf);\n> +\t\t}\n> +\t} else {\n> +\t\tif (show_only || verbose)\n> +\t\t\tprintf(\"Removing stale temporary file %s\\n\", fullpath);\n> +\t\tif (!show_only)\n> +\t\t\tunlink_or_warn(fullpath);\n> +\t}\n>  \treturn 0;\n>  }\n>  \n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index 49b846d9605..8815e24cde5 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -2213,7 +2213,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)\n>  \t\tstrvec_push(&child.args, alt_shallow_file);\n>  \t}\n>  \n> -\ttmp_objdir = tmp_objdir_create();\n> +\ttmp_objdir = tmp_objdir_create(\"incoming\");\n>  \tif (!tmp_objdir) {\n>  \t\tif (err_fd > 0)\n>  \t\t\tclose(err_fd);\n> diff --git a/environment.c b/environment.c\n> index 9da7f3c1a19..342400fcaad 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -17,6 +17,7 @@\n>  #include \"commit.h\"\n>  #include \"strvec.h\"\n>  #include \"object-store.h\"\n> +#include \"tmp-objdir.h\"\n>  #include \"chdir-notify.h\"\n>  #include \"shallow.h\"\n>  \n> @@ -331,10 +332,14 @@ static void update_relative_gitdir(const char *name,\n>  \t\t\t\t   void *data)\n>  {\n>  \tchar *path = reparent_relative_path(old_cwd, new_cwd, get_git_dir());\n> +\tstruct tmp_objdir *tmp_objdir = tmp_objdir_unapply_primary_odb();\n>  \ttrace_printf_key(&trace_setup_key,\n>  \t\t\t \"setup: move $GIT_DIR to '%s'\",\n>  \t\t\t path);\n> +\n>  \tset_git_dir_1(path);\n\nIf a blank line needs to be added, have it between the variable\ndeclarations and the first statement (i.e. before the above call to\n\"trace_printf_key()\").\n\n> +\tif (tmp_objdir)\n> +\t\ttmp_objdir_reapply_primary_odb(tmp_objdir, old_cwd, new_cwd);\n>  \tfree(path);\n>  }\n\nThis is called during set_git_dir(), which happens fairly early in\nthe set-up sequence.  I wonder if there is a real use case that\ncreates a tmp-objdir that early in the process to require this\nunapply-reapply sequence.\n\n> @@ -1809,8 +1846,11 @@ int hash_object_file(const struct git_hash_algo *algo, const void *buf,\n>  /* Finalize a file on disk, and close it. */\n>  static void close_loose_object(int fd)\n>  {\n> -\tif (fsync_object_files)\n> -\t\tfsync_or_die(fd, \"loose object file\");\n> +\tif (!the_repository->objects->odb->will_destroy) {\n> +\t\tif (fsync_object_files)\n> +\t\t\tfsync_or_die(fd, \"loose object file\");\n\nOK, so we omit fsync because these newly created loose objects may\nnot survive and instead get discarded.  Presumably when we migrate\nthem to the real object store, we'll make sure they hit the disk\nplatter in some other way?\n\n\t... goes and cheats by reading ahead ...\n\nAhh, ok, new objects created in a temporary object store that is\nmarked with the will_destroy bit is not allowed to migrate to the\nreal object store, so there is no point to fsync them.\n\nset_temporary_primary_odb() and tmp_objdir_replace_primary_odb() can\nmark the temporary one to be throw-away, but unfortunately there is\nno caller in this step, so it is a bit hard to see when a throw-away\nobject store is useful.  I guess remerge-diff wants to do tentative\nmerges that create new objects in a throw-away object directory,\nbecause it is logically a read-only operation.\n\n> diff --git a/tmp-objdir.c b/tmp-objdir.c\n> index b8d880e3626..3d38eeab66b 100644\n> --- a/tmp-objdir.c\n> +++ b/tmp-objdir.c\n> @@ -1,5 +1,6 @@\n>  #include \"cache.h\"\n>  #include \"tmp-objdir.h\"\n> +#include \"chdir-notify.h\"\n>  #include \"dir.h\"\n>  #include \"sigchain.h\"\n>  #include \"string-list.h\"\n> @@ -11,6 +12,8 @@\n>  struct tmp_objdir {\n>  \tstruct strbuf path;\n>  \tstruct strvec env;\n> +\tstruct object_directory *prev_odb;\n> +\tint will_destroy;\n\nThe other one was a one-bit unsigned bitfield, but this is a full\ninteger.  I somehow think that the other one can and should be a\nfull integer, too---it's not like there are tons of bits need to be\nstored in the structure or we will have tons of instances of the\nstructure that storing many bits compactly matters.\n\nThanks.\n"},{"id":"443143","messageId":"20211206085300.GA26699@neerajsi-x1.localdomain","threadId":"57028","inReplyTo":"xmqq4k7mi3g4.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] tmp-objdir: new API for creating temporary writable databases","fromName":"Neeraj Singh","fromEmail":"nksingh85@gmail.com","sentAt":"2021-12-06T08:53:00Z","receivedAt":"2021-12-06T08:53:04Z","isPatch":true,"sender":{"key":"nksingh85@gmail.com","avatar":null},"body":"On Sun, Dec 05, 2021 at 11:43:07PM -0800, Junio C Hamano wrote:\n> \"Neeraj Singh via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n> > @@ -331,10 +332,14 @@ static void update_relative_gitdir(const char *name,\n> >  \t\t\t\t   void *data)\n> >  {\n> >  \tchar *path = reparent_relative_path(old_cwd, new_cwd, get_git_dir());\n> > +\tstruct tmp_objdir *tmp_objdir = tmp_objdir_unapply_primary_odb();\n> >  \ttrace_printf_key(&trace_setup_key,\n> >  \t\t\t \"setup: move $GIT_DIR to '%s'\",\n> >  \t\t\t path);\n> > +\n> >  \tset_git_dir_1(path);\n> \n> If a blank line needs to be added, have it between the variable\n> declarations and the first statement (i.e. before the above call to\n> \"trace_printf_key()\").\n> \n\nWill fix.\n\n> > +\tif (tmp_objdir)\n> > +\t\ttmp_objdir_reapply_primary_odb(tmp_objdir, old_cwd, new_cwd);\n> >  \tfree(path);\n> >  }\n> \n> This is called during set_git_dir(), which happens fairly early in\n> the set-up sequence.  I wonder if there is a real use case that\n> creates a tmp-objdir that early in the process to require this\n> unapply-reapply sequence.\n> \n\nThe lack of this code was causing a failure, I believe in\nt2107-update-index-basic.sh: \"--refresh triggers late setup_work_tree\".\n\nThis problem came up after applying: https://lore.kernel.org/git/4a40fd4a29a468b9ce320bc7b22f19e5a526fad6.1637020263.git.gitgitgadget@gmail.com/\n\nI thought it would be best to fix this in the tmp-objdir code so that\ncallers could plug/unplug bulk checkin without any subtle surprises.\n\n> > @@ -1809,8 +1846,11 @@ int hash_object_file(const struct git_hash_algo *algo, const void *buf,\n> >  /* Finalize a file on disk, and close it. */\n> >  static void close_loose_object(int fd)\n> >  {\n> > -\tif (fsync_object_files)\n> > -\t\tfsync_or_die(fd, \"loose object file\");\n> > +\tif (!the_repository->objects->odb->will_destroy) {\n> > +\t\tif (fsync_object_files)\n> > +\t\t\tfsync_or_die(fd, \"loose object file\");\n> \n> OK, so we omit fsync because these newly created loose objects may\n> not survive and instead get discarded.  Presumably when we migrate\n> them to the real object store, we'll make sure they hit the disk\n> platter in some other way?\n> \n> \t... goes and cheats by reading ahead ...\n> \n> Ahh, ok, new objects created in a temporary object store that is\n> marked with the will_destroy bit is not allowed to migrate to the\n> real object store, so there is no point to fsync them.\n> \n> set_temporary_primary_odb() and tmp_objdir_replace_primary_odb() can\n> mark the temporary one to be throw-away, but unfortunately there is\n> no caller in this step, so it is a bit hard to see when a throw-away\n> object store is useful.  I guess remerge-diff wants to do tentative\n> merges that create new objects in a throw-away object directory,\n> because it is logically a read-only operation.\n> \n\nYes, this code is there exactly for remerge-diff and anyone doing something\nsimilar in the future.\n\n> > diff --git a/tmp-objdir.c b/tmp-objdir.c\n> > index b8d880e3626..3d38eeab66b 100644\n> > --- a/tmp-objdir.c\n> > +++ b/tmp-objdir.c\n> > @@ -1,5 +1,6 @@\n> >  #include \"cache.h\"\n> >  #include \"tmp-objdir.h\"\n> > +#include \"chdir-notify.h\"\n> >  #include \"dir.h\"\n> >  #include \"sigchain.h\"\n> >  #include \"string-list.h\"\n> > @@ -11,6 +12,8 @@\n> >  struct tmp_objdir {\n> >  \tstruct strbuf path;\n> >  \tstruct strvec env;\n> > +\tstruct object_directory *prev_odb;\n> > +\tint will_destroy;\n> \n> The other one was a one-bit unsigned bitfield, but this is a full\n> integer.  I somehow think that the other one can and should be a\n> full integer, too---it's not like there are tons of bits need to be\n> stored in the structure or we will have tons of instances of the\n> structure that storing many bits compactly matters.\n> \n\nThe principle I was trying to follow here is that the only flag in a\nstructure might as well be a full integer, but when we have two or more\nit might be worth combining them into a single machine word.  Given that\nthese are not highly replicated structures, you're right that's it's not\na big benefit.\n\nI'll switch everything to an int and call it good.\n\nGiven that this patch series introduces functions with no users, are you\ngoing to hold off on putting this into 'next' until another next-worthy\npatch series is ready?  I've already reworked the batch mode stuff on Github,\nbut I'll need to do a lot more testing before sending it to the list.\n\nThanks,\nNeeraj\n"},{"id":"443182","messageId":"xmqqr1aphbue.fsf@gitster.g","threadId":"57028","inReplyTo":"20211206085300.GA26699@neerajsi-x1.localdomain","subject":"Re: [PATCH v2 1/2] tmp-objdir: new API for creating temporary writable databases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-06T17:39:21Z","receivedAt":"2021-12-06T17:39:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Neeraj Singh <nksingh85@gmail.com> writes:\n\n>> > +\tif (tmp_objdir)\n>> > +\t\ttmp_objdir_reapply_primary_odb(tmp_objdir, old_cwd, new_cwd);\n>> >  \tfree(path);\n>> >  }\n>> \n>> This is called during set_git_dir(), which happens fairly early in\n>> the set-up sequence.  I wonder if there is a real use case that\n>> creates a tmp-objdir that early in the process to require this\n>> unapply-reapply sequence.\n>> \n>\n> The lack of this code was causing a failure, I believe in\n> t2107-update-index-basic.sh: \"--refresh triggers late setup_work_tree\".\n>\n> This problem came up after applying: https://lore.kernel.org/git/4a40fd4a29a468b9ce320bc7b22f19e5a526fad6.1637020263.git.gitgitgadget@gmail.com/\n>\n> I thought it would be best to fix this in the tmp-objdir code so that\n> callers could plug/unplug bulk checkin without any subtle surprises.\n\nOK, I think that is fine.\n\nAs a slightly-related tangent that is outside the topic, I think we\nshould revisit \"update-index\", which is one of the oldest plumbing\ncommands with its own quirks.  I do not offhand see why it needs to\nsprinkle this many setup_work_tree() calls everywhere.  Having an\nindex to work on means we must have a working tree to update and/or\nrefresh from.  We should be able to get away with the NEED_WORK_TREE\nbit in the git.c::commands[] table for this command.  If this were a\nmore recent command, I may suspect that there were valid reasons\nlike \"in this particular mode, update-index must work inside a bare\nrepository\" to force us to take this unusual program structure, but\nbecause this is probably a lot older than NEED_WORK_TREE bit, I\nwould not be surprised if the answer were \"nobody noticed the\nugliness so far\".\n\n> Given that this patch series introduces functions with no users, are you\n> going to hold off on putting this into 'next' until another next-worthy\n> patch series is ready?\n\nEven without any existing callers, as long as we see Reviewed-by: by\nElijah, who we know will have to build on top of this series, I\nthink this can and should go to 'next'.\n\nThanks.\n\n"},{"id":"443206","messageId":"pull.1091.v3.git.1638828305.gitgitgadget@gmail.com","threadId":"57028","inReplyTo":"pull.1091.v2.git.1638750965.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] ns/tmp-objdir: add support for temporary writable databases","fromName":"Neeraj K. Singh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-06T22:05:03Z","receivedAt":"2021-12-06T22:05:09Z","isPatch":true,"sender":{"key":"name:Neeraj K. Singh","avatar":null},"body":"V3 (hopefully final):\n\n * Fix the commit description for patch [2/2] to reflect the fact that\n   disabling ref updates no longer depends on the_repository.\n * Add a link to Jeff King's test case in patch [2/2]. The test relies on\n   remerge-diff, so it can't be directly included here.\n * Adjust line spacing in update_relative_gitdir (gitster)\n * Switch struct object_directory to use full-width integers rather than\n   flags (gitster)\n * Fix typo s/protentially/potentially (neerajsi)\n\nV2 changes: I lost a couple changes in the shuffle while splitting these\nchanges out\n\n * Make the will-destroy boolean a single bit field of type unsigned int so\n   that it doesn't change twice in this small patch series.\n * Remove a the_repository reference in the disable ref updates code. Now\n   the repository is taken from the ref_store.\n\nNew interface into the tmp-objdir API to help in-core use of the quarantine\nfeature.\n\nThis patch series was formerly part of the ns/batched-fsync topic [1]. It's\nnow split out into its own gitgitgadget PR and discussion thread since it is\nthe base for en/remerge-diff as well.\n\nThe most recent feedback was in [2]. I removed printing from prune_subdir\nand simplified the strbuf handling in prune_tmp_file.\n\nReferences: [1]\nhttps://lore.kernel.org/git/pull.1076.v9.git.git.1637020263.gitgitgadget@gmail.com/\n[2]\nhttps://lore.kernel.org/git/CABPp-BH6m4q_EoX77bqLcpCN1HRfJ_XayeCV2O0sRybX53rPrw@mail.gmail.com/\n\nNeeraj Singh (2):\n  tmp-objdir: new API for creating temporary writable databases\n  tmp-objdir: disable ref updates when replacing the primary odb\n\n builtin/prune.c        | 20 ++++++++++++---\n builtin/receive-pack.c |  2 +-\n environment.c          |  9 +++++++\n object-file.c          | 50 ++++++++++++++++++++++++++++++++++++--\n object-store.h         | 26 ++++++++++++++++++++\n object.c               |  2 +-\n refs.c                 |  2 +-\n repository.c           |  2 ++\n repository.h           |  1 +\n tmp-objdir.c           | 55 +++++++++++++++++++++++++++++++++++++++---\n tmp-objdir.h           | 29 +++++++++++++++++++---\n 11 files changed, 183 insertions(+), 15 deletions(-)\n\n\nbase-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1091%2Fneerajsi-msft%2Fns%2Ftmp-objdir-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1091/neerajsi-msft/ns/tmp-objdir-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1091\n\nRange-diff vs v2:\n\n 1:  36c00613d9a ! 1:  cccb3888070 tmp-objdir: new API for creating temporary writable databases\n     @@ environment.c: static void update_relative_gitdir(const char *name,\n       {\n       \tchar *path = reparent_relative_path(old_cwd, new_cwd, get_git_dir());\n      +\tstruct tmp_objdir *tmp_objdir = tmp_objdir_unapply_primary_odb();\n     ++\n       \ttrace_printf_key(&trace_setup_key,\n       \t\t\t \"setup: move $GIT_DIR to '%s'\",\n       \t\t\t path);\n     -+\n       \tset_git_dir_1(path);\n      +\tif (tmp_objdir)\n      +\t\ttmp_objdir_reapply_primary_odb(tmp_objdir, old_cwd, new_cwd);\n     @@ object-store.h: struct object_directory {\n      +\t/*\n      +\t * This object store is ephemeral, so there is no need to fsync.\n      +\t */\n     -+\tunsigned int will_destroy : 1;\n     ++\tint will_destroy;\n      +\n       \t/*\n       \t * Path to the alternative object store. If this is a relative path,\n     @@ tmp-objdir.h: int tmp_objdir_destroy(struct tmp_objdir *);\n       void tmp_objdir_add_as_alternate(const struct tmp_objdir *);\n       \n      +/*\n     -+ * Replaces the main object store in the current process with the temporary\n     ++ * Replaces the writable object store in the current process with the temporary\n      + * object directory and makes the former main object store an alternate.\n      + * If will_destroy is nonzero, the object directory may not be migrated.\n      + */\n     @@ tmp-objdir.h: int tmp_objdir_destroy(struct tmp_objdir *);\n      +struct tmp_objdir *tmp_objdir_unapply_primary_odb(void);\n      +\n      +/*\n     -+ * Reapplies the former primary temporary object database, after protentially\n     ++ * Reapplies the former primary temporary object database, after potentially\n      + * changing its relative path.\n      + */\n      +void tmp_objdir_reapply_primary_odb(struct tmp_objdir *, const char *old_cwd,\n 2:  f667cbcc47d ! 2:  4e44121c2d7 tmp-objdir: disable ref updates when replacing the primary odb\n     @@ Commit message\n          the disable_ref_updates flag on the odb, which is queried by\n          the ref_transaction_prepare function.\n      \n     -    Note: This change adds an assumption that the state of\n     -    the_repository is relevant for any ref transaction that might\n     -    be initiated. Unwinding this assumption should be straightforward\n     -    by saving the relevant repository to query in the transaction or\n     -    the ref_store.\n     -\n     -    Peff's test case was invoking ref updates via the cachetextconv\n     +    Peff's test case [1] was invoking ref updates via the cachetextconv\n          setting. That particular code silently does nothing when a ref\n          update is forbidden. See the call to notes_cache_put in\n          fill_textconv where errors are ignored.\n      \n     +    [1] https://lore.kernel.org/git/YVOn3hDsb5pnxR53@coredump.intra.peff.net/\n     +\n          Reported-by: Jeff King <peff@peff.net>\n      \n          Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>\n     @@ object-store.h: struct object_directory {\n      +\t * facility. Disable ref updates since the objects in the store\n      +\t * might be discarded on rollback.\n      +\t */\n     -+\tunsigned int disable_ref_updates : 1;\n     ++\tint disable_ref_updates;\n      +\n       \t/*\n       \t * This object store is ephemeral, so there is no need to fsync.\n\n-- \ngitgitgadget\n"},{"id":"443207","messageId":"cccb388807002c844e0833aa7ab85e93fa4e4e50.1638828305.git.gitgitgadget@gmail.com","threadId":"57028","inReplyTo":"pull.1091.v3.git.1638828305.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] tmp-objdir: new API for creating temporary writable databases","fromName":"Neeraj Singh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-06T22:05:04Z","receivedAt":"2021-12-06T22:05:12Z","isPatch":true,"sender":{"key":"nksingh85@gmail.com","avatar":null},"body":"From: Neeraj Singh <neerajsi@microsoft.com>\n\nThe tmp_objdir API provides the ability to create temporary object\ndirectories, but was designed with the goal of having subprocesses\naccess these object stores, followed by the main process migrating\nobjects from it to the main object store or just deleting it.  The\nsubprocesses would view it as their primary datastore and write to it.\n\nHere we add the tmp_objdir_replace_primary_odb function that replaces\nthe current process's writable \"main\" object directory with the\nspecified one. The previous main object directory is restored in either\ntmp_objdir_migrate or tmp_objdir_destroy.\n\nFor the --remerge-diff usecase, add a new `will_destroy` flag in `struct\nobject_database` to mark ephemeral object databases that do not require\nfsync durability.\n\nAdd 'git prune' support for removing temporary object databases, and\nmake sure that they have a name starting with tmp_ and containing an\noperation-specific name.\n\nBased-on-patch-by: Elijah Newren <newren@gmail.com>\n\nSigned-off-by: Neeraj Singh <neerajsi@microsoft.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/prune.c        | 20 ++++++++++++---\n builtin/receive-pack.c |  2 +-\n environment.c          |  5 ++++\n object-file.c          | 44 +++++++++++++++++++++++++++++++--\n object-store.h         | 19 +++++++++++++++\n object.c               |  2 +-\n tmp-objdir.c           | 55 +++++++++++++++++++++++++++++++++++++++---\n tmp-objdir.h           | 29 +++++++++++++++++++---\n 8 files changed, 162 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/prune.c b/builtin/prune.c\nindex 485c9a3c56f..c2bcdc07db4 100644\n--- a/builtin/prune.c\n+++ b/builtin/prune.c\n@@ -26,10 +26,22 @@ static int prune_tmp_file(const char *fullpath)\n \t\treturn error(\"Could not stat '%s'\", fullpath);\n \tif (st.st_mtime > expire)\n \t\treturn 0;\n-\tif (show_only || verbose)\n-\t\tprintf(\"Removing stale temporary file %s\\n\", fullpath);\n-\tif (!show_only)\n-\t\tunlink_or_warn(fullpath);\n+\tif (S_ISDIR(st.st_mode)) {\n+\t\tif (show_only || verbose)\n+\t\t\tprintf(\"Removing stale temporary directory %s\\n\", fullpath);\n+\t\tif (!show_only) {\n+\t\t\tstruct strbuf remove_dir_buf = STRBUF_INIT;\n+\n+\t\t\tstrbuf_addstr(&remove_dir_buf, fullpath);\n+\t\t\tremove_dir_recursively(&remove_dir_buf, 0);\n+\t\t\tstrbuf_release(&remove_dir_buf);\n+\t\t}\n+\t} else {\n+\t\tif (show_only || verbose)\n+\t\t\tprintf(\"Removing stale temporary file %s\\n\", fullpath);\n+\t\tif (!show_only)\n+\t\t\tunlink_or_warn(fullpath);\n+\t}\n \treturn 0;\n }\n \ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 49b846d9605..8815e24cde5 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -2213,7 +2213,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)\n \t\tstrvec_push(&child.args, alt_shallow_file);\n \t}\n \n-\ttmp_objdir = tmp_objdir_create();\n+\ttmp_objdir = tmp_objdir_create(\"incoming\");\n \tif (!tmp_objdir) {\n \t\tif (err_fd > 0)\n \t\t\tclose(err_fd);\ndiff --git a/environment.c b/environment.c\nindex 9da7f3c1a19..fe51dfe24d4 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -17,6 +17,7 @@\n #include \"commit.h\"\n #include \"strvec.h\"\n #include \"object-store.h\"\n+#include \"tmp-objdir.h\"\n #include \"chdir-notify.h\"\n #include \"shallow.h\"\n \n@@ -331,10 +332,14 @@ static void update_relative_gitdir(const char *name,\n \t\t\t\t   void *data)\n {\n \tchar *path = reparent_relative_path(old_cwd, new_cwd, get_git_dir());\n+\tstruct tmp_objdir *tmp_objdir = tmp_objdir_unapply_primary_odb();\n+\n \ttrace_printf_key(&trace_setup_key,\n \t\t\t \"setup: move $GIT_DIR to '%s'\",\n \t\t\t path);\n \tset_git_dir_1(path);\n+\tif (tmp_objdir)\n+\t\ttmp_objdir_reapply_primary_odb(tmp_objdir, old_cwd, new_cwd);\n \tfree(path);\n }\n \ndiff --git a/object-file.c b/object-file.c\nindex c3d866a287e..0b6a61aeaff 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -683,6 +683,43 @@ void add_to_alternates_memory(const char *reference)\n \t\t\t     '\\n', NULL, 0);\n }\n \n+struct object_directory *set_temporary_primary_odb(const char *dir, int will_destroy)\n+{\n+\tstruct object_directory *new_odb;\n+\n+\t/*\n+\t * Make sure alternates are initialized, or else our entry may be\n+\t * overwritten when they are.\n+\t */\n+\tprepare_alt_odb(the_repository);\n+\n+\t/*\n+\t * Make a new primary odb and link the old primary ODB in as an\n+\t * alternate\n+\t */\n+\tnew_odb = xcalloc(1, sizeof(*new_odb));\n+\tnew_odb->path = xstrdup(dir);\n+\tnew_odb->will_destroy = will_destroy;\n+\tnew_odb->next = the_repository->objects->odb;\n+\tthe_repository->objects->odb = new_odb;\n+\treturn new_odb->next;\n+}\n+\n+void restore_primary_odb(struct object_directory *restore_odb, const char *old_path)\n+{\n+\tstruct object_directory *cur_odb = the_repository->objects->odb;\n+\n+\tif (strcmp(old_path, cur_odb->path))\n+\t\tBUG(\"expected %s as primary object store; found %s\",\n+\t\t    old_path, cur_odb->path);\n+\n+\tif (cur_odb->next != restore_odb)\n+\t\tBUG(\"we expect the old primary object store to be the first alternate\");\n+\n+\tthe_repository->objects->odb = restore_odb;\n+\tfree_object_directory(cur_odb);\n+}\n+\n /*\n  * Compute the exact path an alternate is at and returns it. In case of\n  * error NULL is returned and the human readable error is added to `err`\n@@ -1809,8 +1846,11 @@ int hash_object_file(const struct git_hash_algo *algo, const void *buf,\n /* Finalize a file on disk, and close it. */\n static void close_loose_object(int fd)\n {\n-\tif (fsync_object_files)\n-\t\tfsync_or_die(fd, \"loose object file\");\n+\tif (!the_repository->objects->odb->will_destroy) {\n+\t\tif (fsync_object_files)\n+\t\t\tfsync_or_die(fd, \"loose object file\");\n+\t}\n+\n \tif (close(fd) != 0)\n \t\tdie_errno(_(\"error when closing loose object file\"));\n }\ndiff --git a/object-store.h b/object-store.h\nindex 952efb6a4be..cb173e69392 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -27,6 +27,11 @@ struct object_directory {\n \tuint32_t loose_objects_subdir_seen[8]; /* 256 bits */\n \tstruct oidtree *loose_objects_cache;\n \n+\t/*\n+\t * This object store is ephemeral, so there is no need to fsync.\n+\t */\n+\tint will_destroy;\n+\n \t/*\n \t * Path to the alternative object store. If this is a relative path,\n \t * it is relative to the current working directory.\n@@ -58,6 +63,17 @@ void add_to_alternates_file(const char *dir);\n  */\n void add_to_alternates_memory(const char *dir);\n \n+/*\n+ * Replace the current writable object directory with the specified temporary\n+ * object directory; returns the former primary object directory.\n+ */\n+struct object_directory *set_temporary_primary_odb(const char *dir, int will_destroy);\n+\n+/*\n+ * Restore a previous ODB replaced by set_temporary_main_odb.\n+ */\n+void restore_primary_odb(struct object_directory *restore_odb, const char *old_path);\n+\n /*\n  * Populate and return the loose object cache array corresponding to the\n  * given object ID.\n@@ -68,6 +84,9 @@ struct oidtree *odb_loose_cache(struct object_directory *odb,\n /* Empty the loose object cache for the specified object directory. */\n void odb_clear_loose_cache(struct object_directory *odb);\n \n+/* Clear and free the specified object directory */\n+void free_object_directory(struct object_directory *odb);\n+\n struct packed_git {\n \tstruct hashmap_entry packmap_ent;\n \tstruct packed_git *next;\ndiff --git a/object.c b/object.c\nindex 23a24e678a8..048f96a260e 100644\n--- a/object.c\n+++ b/object.c\n@@ -513,7 +513,7 @@ struct raw_object_store *raw_object_store_new(void)\n \treturn o;\n }\n \n-static void free_object_directory(struct object_directory *odb)\n+void free_object_directory(struct object_directory *odb)\n {\n \tfree(odb->path);\n \todb_clear_loose_cache(odb);\ndiff --git a/tmp-objdir.c b/tmp-objdir.c\nindex b8d880e3626..3d38eeab66b 100644\n--- a/tmp-objdir.c\n+++ b/tmp-objdir.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"tmp-objdir.h\"\n+#include \"chdir-notify.h\"\n #include \"dir.h\"\n #include \"sigchain.h\"\n #include \"string-list.h\"\n@@ -11,6 +12,8 @@\n struct tmp_objdir {\n \tstruct strbuf path;\n \tstruct strvec env;\n+\tstruct object_directory *prev_odb;\n+\tint will_destroy;\n };\n \n /*\n@@ -38,6 +41,9 @@ static int tmp_objdir_destroy_1(struct tmp_objdir *t, int on_signal)\n \tif (t == the_tmp_objdir)\n \t\tthe_tmp_objdir = NULL;\n \n+\tif (!on_signal && t->prev_odb)\n+\t\trestore_primary_odb(t->prev_odb, t->path.buf);\n+\n \t/*\n \t * This may use malloc via strbuf_grow(), but we should\n \t * have pre-grown t->path sufficiently so that this\n@@ -52,6 +58,7 @@ static int tmp_objdir_destroy_1(struct tmp_objdir *t, int on_signal)\n \t */\n \tif (!on_signal)\n \t\ttmp_objdir_free(t);\n+\n \treturn err;\n }\n \n@@ -121,7 +128,7 @@ static int setup_tmp_objdir(const char *root)\n \treturn ret;\n }\n \n-struct tmp_objdir *tmp_objdir_create(void)\n+struct tmp_objdir *tmp_objdir_create(const char *prefix)\n {\n \tstatic int installed_handlers;\n \tstruct tmp_objdir *t;\n@@ -129,11 +136,16 @@ struct tmp_objdir *tmp_objdir_create(void)\n \tif (the_tmp_objdir)\n \t\tBUG(\"only one tmp_objdir can be used at a time\");\n \n-\tt = xmalloc(sizeof(*t));\n+\tt = xcalloc(1, sizeof(*t));\n \tstrbuf_init(&t->path, 0);\n \tstrvec_init(&t->env);\n \n-\tstrbuf_addf(&t->path, \"%s/incoming-XXXXXX\", get_object_directory());\n+\t/*\n+\t * Use a string starting with tmp_ so that the builtin/prune.c code\n+\t * can recognize any stale objdirs left behind by a crash and delete\n+\t * them.\n+\t */\n+\tstrbuf_addf(&t->path, \"%s/tmp_objdir-%s-XXXXXX\", get_object_directory(), prefix);\n \n \t/*\n \t * Grow the strbuf beyond any filename we expect to be placed in it.\n@@ -269,6 +281,13 @@ int tmp_objdir_migrate(struct tmp_objdir *t)\n \tif (!t)\n \t\treturn 0;\n \n+\tif (t->prev_odb) {\n+\t\tif (the_repository->objects->odb->will_destroy)\n+\t\t\tBUG(\"migrating an ODB that was marked for destruction\");\n+\t\trestore_primary_odb(t->prev_odb, t->path.buf);\n+\t\tt->prev_odb = NULL;\n+\t}\n+\n \tstrbuf_addbuf(&src, &t->path);\n \tstrbuf_addstr(&dst, get_object_directory());\n \n@@ -292,3 +311,33 @@ void tmp_objdir_add_as_alternate(const struct tmp_objdir *t)\n {\n \tadd_to_alternates_memory(t->path.buf);\n }\n+\n+void tmp_objdir_replace_primary_odb(struct tmp_objdir *t, int will_destroy)\n+{\n+\tif (t->prev_odb)\n+\t\tBUG(\"the primary object database is already replaced\");\n+\tt->prev_odb = set_temporary_primary_odb(t->path.buf, will_destroy);\n+\tt->will_destroy = will_destroy;\n+}\n+\n+struct tmp_objdir *tmp_objdir_unapply_primary_odb(void)\n+{\n+\tif (!the_tmp_objdir || !the_tmp_objdir->prev_odb)\n+\t\treturn NULL;\n+\n+\trestore_primary_odb(the_tmp_objdir->prev_odb, the_tmp_objdir->path.buf);\n+\tthe_tmp_objdir->prev_odb = NULL;\n+\treturn the_tmp_objdir;\n+}\n+\n+void tmp_objdir_reapply_primary_odb(struct tmp_objdir *t, const char *old_cwd,\n+\t\tconst char *new_cwd)\n+{\n+\tchar *path;\n+\n+\tpath = reparent_relative_path(old_cwd, new_cwd, t->path.buf);\n+\tstrbuf_reset(&t->path);\n+\tstrbuf_addstr(&t->path, path);\n+\tfree(path);\n+\ttmp_objdir_replace_primary_odb(t, t->will_destroy);\n+}\ndiff --git a/tmp-objdir.h b/tmp-objdir.h\nindex b1e45b4c75d..cda5ec76778 100644\n--- a/tmp-objdir.h\n+++ b/tmp-objdir.h\n@@ -10,7 +10,7 @@\n  *\n  * Example:\n  *\n- *\tstruct tmp_objdir *t = tmp_objdir_create();\n+ *\tstruct tmp_objdir *t = tmp_objdir_create(\"incoming\");\n  *\tif (!run_command_v_opt_cd_env(cmd, 0, NULL, tmp_objdir_env(t)) &&\n  *\t    !tmp_objdir_migrate(t))\n  *\t\tprintf(\"success!\\n\");\n@@ -22,9 +22,10 @@\n struct tmp_objdir;\n \n /*\n- * Create a new temporary object directory; returns NULL on failure.\n+ * Create a new temporary object directory with the specified prefix;\n+ * returns NULL on failure.\n  */\n-struct tmp_objdir *tmp_objdir_create(void);\n+struct tmp_objdir *tmp_objdir_create(const char *prefix);\n \n /*\n  * Return a list of environment strings, suitable for use with\n@@ -51,4 +52,26 @@ int tmp_objdir_destroy(struct tmp_objdir *);\n  */\n void tmp_objdir_add_as_alternate(const struct tmp_objdir *);\n \n+/*\n+ * Replaces the writable object store in the current process with the temporary\n+ * object directory and makes the former main object store an alternate.\n+ * If will_destroy is nonzero, the object directory may not be migrated.\n+ */\n+void tmp_objdir_replace_primary_odb(struct tmp_objdir *, int will_destroy);\n+\n+/*\n+ * If the primary object database was replaced by a temporary object directory,\n+ * restore it to its original value while keeping the directory contents around.\n+ * Returns NULL if the primary object database was not replaced.\n+ */\n+struct tmp_objdir *tmp_objdir_unapply_primary_odb(void);\n+\n+/*\n+ * Reapplies the former primary temporary object database, after potentially\n+ * changing its relative path.\n+ */\n+void tmp_objdir_reapply_primary_odb(struct tmp_objdir *, const char *old_cwd,\n+\t\tconst char *new_cwd);\n+\n+\n #endif /* TMP_OBJDIR_H */\n-- \ngitgitgadget\n\n"},{"id":"443208","messageId":"4e44121c2d7bced65e25eb7ec5156290132bec94.1638828305.git.gitgitgadget@gmail.com","threadId":"57028","inReplyTo":"pull.1091.v3.git.1638828305.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] tmp-objdir: disable ref updates when replacing the primary odb","fromName":"Neeraj Singh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-06T22:05:05Z","receivedAt":"2021-12-06T22:05:13Z","isPatch":true,"sender":{"key":"nksingh85@gmail.com","avatar":null},"body":"From: Neeraj Singh <neerajsi@microsoft.com>\n\nWhen creating a subprocess with a temporary ODB, we set the\nGIT_QUARANTINE_ENVIRONMENT env var to tell child Git processes not\nto update refs, since the tmp-objdir may go away.\n\nIntroduce a similar mechanism for in-process temporary ODBs when\nwe call tmp_objdir_replace_primary_odb. Now both mechanisms set\nthe disable_ref_updates flag on the odb, which is queried by\nthe ref_transaction_prepare function.\n\nPeff's test case [1] was invoking ref updates via the cachetextconv\nsetting. That particular code silently does nothing when a ref\nupdate is forbidden. See the call to notes_cache_put in\nfill_textconv where errors are ignored.\n\n[1] https://lore.kernel.org/git/YVOn3hDsb5pnxR53@coredump.intra.peff.net/\n\nReported-by: Jeff King <peff@peff.net>\n\nSigned-off-by: Neeraj Singh <neerajsi@microsoft.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n environment.c  | 4 ++++\n object-file.c  | 6 ++++++\n object-store.h | 7 +++++++\n refs.c         | 2 +-\n repository.c   | 2 ++\n repository.h   | 1 +\n 6 files changed, 21 insertions(+), 1 deletion(-)\n\ndiff --git a/environment.c b/environment.c\nindex fe51dfe24d4..a8b64f5194f 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -169,6 +169,10 @@ void setup_git_env(const char *git_dir)\n \targs.graft_file = getenv_safe(&to_free, GRAFT_ENVIRONMENT);\n \targs.index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT);\n \targs.alternate_db = getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT);\n+\tif (getenv(GIT_QUARANTINE_ENVIRONMENT)) {\n+\t\targs.disable_ref_updates = 1;\n+\t}\n+\n \trepo_set_gitdir(the_repository, git_dir, &args);\n \tstrvec_clear(&to_free);\n \ndiff --git a/object-file.c b/object-file.c\nindex 0b6a61aeaff..659ef7623ff 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -699,6 +699,12 @@ struct object_directory *set_temporary_primary_odb(const char *dir, int will_des\n \t */\n \tnew_odb = xcalloc(1, sizeof(*new_odb));\n \tnew_odb->path = xstrdup(dir);\n+\n+\t/*\n+\t * Disable ref updates while a temporary odb is active, since\n+\t * the objects in the database may roll back.\n+\t */\n+\tnew_odb->disable_ref_updates = 1;\n \tnew_odb->will_destroy = will_destroy;\n \tnew_odb->next = the_repository->objects->odb;\n \tthe_repository->objects->odb = new_odb;\ndiff --git a/object-store.h b/object-store.h\nindex cb173e69392..6f89482df03 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -27,6 +27,13 @@ struct object_directory {\n \tuint32_t loose_objects_subdir_seen[8]; /* 256 bits */\n \tstruct oidtree *loose_objects_cache;\n \n+\t/*\n+\t * This is a temporary object store created by the tmp_objdir\n+\t * facility. Disable ref updates since the objects in the store\n+\t * might be discarded on rollback.\n+\t */\n+\tint disable_ref_updates;\n+\n \t/*\n \t * This object store is ephemeral, so there is no need to fsync.\n \t */\ndiff --git a/refs.c b/refs.c\nindex d7cc0a23a3b..ac744e85f5f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2137,7 +2137,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,\n \t\tbreak;\n \t}\n \n-\tif (getenv(GIT_QUARANTINE_ENVIRONMENT)) {\n+\tif (refs->repo->objects->odb->disable_ref_updates) {\n \t\tstrbuf_addstr(err,\n \t\t\t      _(\"ref updates forbidden inside quarantine environment\"));\n \t\treturn -1;\ndiff --git a/repository.c b/repository.c\nindex c5b90ba93ea..dce8e35ac20 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -80,6 +80,8 @@ void repo_set_gitdir(struct repository *repo,\n \texpand_base_dir(&repo->objects->odb->path, o->object_dir,\n \t\t\trepo->commondir, \"objects\");\n \n+\trepo->objects->odb->disable_ref_updates = o->disable_ref_updates;\n+\n \tfree(repo->objects->alternate_db);\n \trepo->objects->alternate_db = xstrdup_or_null(o->alternate_db);\n \texpand_base_dir(&repo->graft_file, o->graft_file,\ndiff --git a/repository.h b/repository.h\nindex a057653981c..7c04e99ac5c 100644\n--- a/repository.h\n+++ b/repository.h\n@@ -158,6 +158,7 @@ struct set_gitdir_args {\n \tconst char *graft_file;\n \tconst char *index_file;\n \tconst char *alternate_db;\n+\tint disable_ref_updates;\n };\n \n void repo_set_gitdir(struct repository *repo, const char *root,\n-- \ngitgitgadget\n"},{"id":"443480","messageId":"CABPp-BEJbS=5i+d-Aa_fCH-WAjSOhX+nSZk5Q9Kb6RiizFg7ZQ@mail.gmail.com","threadId":"57028","inReplyTo":"pull.1091.v3.git.1638828305.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/2] ns/tmp-objdir: add support for temporary writable databases","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-12-08T16:41:21Z","receivedAt":"2021-12-08T16:41:35Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Dec 6, 2021 at 11:18 PM Neeraj K. Singh via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> V3 (hopefully final):\n>\n>  * Fix the commit description for patch [2/2] to reflect the fact that\n>    disabling ref updates no longer depends on the_repository.\n>  * Add a link to Jeff King's test case in patch [2/2]. The test relies on\n>    remerge-diff, so it can't be directly included here.\n>  * Adjust line spacing in update_relative_gitdir (gitster)\n>  * Switch struct object_directory to use full-width integers rather than\n>    flags (gitster)\n>  * Fix typo s/protentially/potentially (neerajsi)\n\nThis version looks good to me:\n\nReviewed-by: Elijah Newren <newren@gmail.com>\n\nFor future reference, when splitting one of your series apart, copying\nthe relevant subset of the cc lines (e.g. from the description at\nhttps://github.com/git/git/pull/1076 to the one at\nhttps://github.com/gitgitgadget/git/pull/1091) would be helpful.\n"}]}