ns/tmp-objdir: add support for temporary writable databases
cover ns/tmp-objdir: add support for temporary writable databases
New interface into the tmp-objdir API to help in-core use of the quarantine feature.
This patch series was formerly part of the ns/batched-fsync topic [1]. It's now split out into its own gitgitgadget PR and discussion thread since it is the base for en/remerge-diff as well.
The most recent feedback was in [2]. I removed printing from prune_subdir and simplified the strbuf handling in prune_tmp_file.
References: [1]
https://lore.kernel.org/git/[email protected]/
[2]
https://lore.kernel.org/git/CABPp-BH6m4q_EoX77bqLcpCN1HRfJ_XayeCV2O0sRybX53rPrw@mail.gmail.com/
Neeraj Singh (2):
tmp-objdir: new API for creating temporary writable databases
tmp-objdir: disable ref updates when replacing the primary odb
builtin/prune.c | 20 ++++++++++++--- builtin/receive-pack.c | 2 +- environment.c | 9 +++++++ object-file.c | 50 ++++++++++++++++++++++++++++++++++++-- object-store.h | 26 ++++++++++++++++++++ object.c | 2 +- refs.c | 2 +- repository.c | 2 ++ repository.h | 1 + tmp-objdir.c | 55 +++++++++++++++++++++++++++++++++++++++--- tmp-objdir.h | 29 +++++++++++++++++++--- 11 files changed, 183 insertions(+), 15 deletions(-)
base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1091%2Fneerajsi-msft%2Fns%2Ftmp-objdir-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1091/neerajsi-msft/ns/tmp-objdir-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1091
Signature · 1 line
gitgitgadget
PATCH 1/2 tmp-objdir: new API for creating temporary writable databases
From: Neeraj Singh <[email protected]>
The tmp_objdir API provides the ability to create temporary object directories, but was designed with the goal of having subprocesses access these object stores, followed by the main process migrating objects from it to the main object store or just deleting it. The subprocesses would view it as their primary datastore and write to it.
Here we add the tmp_objdir_replace_primary_odb function that replaces the current process's writable "main" object directory with the specified one. The previous main object directory is restored in either tmp_objdir_migrate or tmp_objdir_destroy.
For the --remerge-diff usecase, add a new will_destroy flag in struct object_database to mark ephemeral object databases that do not require fsync durability.
Add 'git prune' support for removing temporary object databases, and make sure that they have a name starting with tmp_ and containing an operation-specific name.
--- builtin/prune.c | 20 ++++++++++++--- builtin/receive-pack.c | 2 +- environment.c | 5 ++++ object-file.c | 44 +++++++++++++++++++++++++++++++-- object-store.h | 19 +++++++++++++++ object.c | 2 +- tmp-objdir.c | 55 +++++++++++++++++++++++++++++++++++++++--- tmp-objdir.h | 29 +++++++++++++++++++--- 8 files changed, 162 insertions(+), 14 deletions(-)
builtin/prune.c +16 -4
@@ -26,10 +26,22 @@ static int prune_tmp_file(const char *fullpath)return error ("Could not stat '%s'" , fullpath);if (st.st_mtime > expire)return 0 ; -if (show_only || verbose) -printf ("Removing stale temporary file %s\n" , fullpath); -if (!show_only) -unlink_or_warn (fullpath); +if (S_ISDIR (st.st_mode)) { +if (show_only || verbose) +printf ("Removing stale temporary directory %s\n" , fullpath); +if (!show_only) { +struct strbuf remove_dir_buf =STRBUF_INIT ; + +strbuf_addstr (&remove_dir_buf, fullpath); +remove_dir_recursively (&remove_dir_buf,0 ); +strbuf_release (&remove_dir_buf); + } + }else { +if (show_only || verbose) +printf ("Removing stale temporary file %s\n" , fullpath); +if (!show_only) +unlink_or_warn (fullpath); + }return 0 ; }
builtin/receive-pack.c +1 -1
@@ -2213,7 +2213,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)strvec_push (&child.args, alt_shallow_file); } - tmp_objdir =tmp_objdir_create (); + tmp_objdir =tmp_objdir_create ("incoming" );if (!tmp_objdir) {if (err_fd >0 )close (err_fd);
environment.c +5 -0
@@ -17,6 +17,7 @@#include "commit.h" #include "strvec.h" #include "object-store.h" +#include "tmp-objdir.h" #include "chdir-notify.h" #include "shallow.h"
@@ -331,10 +332,14 @@ static void update_relative_gitdir(const char *name, void *data) {char *path =reparent_relative_path (old_cwd, new_cwd,get_git_dir ()); +struct tmp_objdir *tmp_objdir =tmp_objdir_unapply_primary_odb ();trace_printf_key (&trace_setup_key,"setup: move $GIT_DIR to '%s'" , path); +set_git_dir_1 (path); +if (tmp_objdir) +tmp_objdir_reapply_primary_odb (tmp_objdir, old_cwd, new_cwd);free (path); }
object-file.c +42 -2
56 lines not on this page · raw message
object-store.h +19 -0
@@ -27,6 +27,11 @@ struct object_directory {uint32_t loose_objects_subdir_seen[8 ];/* 256 bits */ struct oidtree *loose_objects_cache; +/* +* This object store is ephemeral, so there is no need to fsync. +*/ +int will_destroy; +/* * Path to the alternative object store. If this is a relative path, * it is relative to the current working directory.
@@ -58,6 +63,17 @@ void add_to_alternates_file(const char *dir);*/ void add_to_alternates_memory (const char *dir); +/* +* Replace the current writable object directory with the specified temporary +* object directory; returns the former primary object directory. +*/ +struct object_directory *set_temporary_primary_odb (const char *dir,int will_destroy); + +/* +* Restore a previous ODB replaced by set_temporary_main_odb. +*/ +void restore_primary_odb (struct object_directory *restore_odb,const char *old_path); +/* * Populate and return the loose object cache array corresponding to the * given object ID.
@@ -68,6 +84,9 @@ struct oidtree *odb_loose_cache(struct object_directory *odb,/* Empty the loose object cache for the specified object directory. */ void odb_clear_loose_cache (struct object_directory *odb); +/* Clear and free the specified object directory */ +void free_object_directory (struct object_directory *odb); +struct packed_git {struct hashmap_entry packmap_ent;struct packed_git *next;
object.c +1 -1
@@ -513,7 +513,7 @@ struct raw_object_store *raw_object_store_new(void)return o; } -static void free_object_directory (struct object_directory *odb) +void free_object_directory (struct object_directory *odb) {free (odb->path);odb_clear_loose_cache (odb);
tmp-objdir.c +52 -3
102 lines not on this page · raw message
tmp-objdir.h +26 -3
@@ -10,7 +10,7 @@* * Example: * -* struct tmp_objdir *t = tmp_objdir_create(); +* struct tmp_objdir *t = tmp_objdir_create("incoming"); * if (!run_command_v_opt_cd_env(cmd, 0, NULL, tmp_objdir_env(t)) && * !tmp_objdir_migrate(t)) * printf("success!\n");
@@ -22,9 +22,10 @@struct tmp_objdir ;/* -* Create a new temporary object directory; returns NULL on failure. +* Create a new temporary object directory with the specified prefix; +* returns NULL on failure. */ -struct tmp_objdir *tmp_objdir_create (void ); +struct tmp_objdir *tmp_objdir_create (const char *prefix);/* * Return a list of environment strings, suitable for use with
@@ -51,4 +52,26 @@ int tmp_objdir_destroy(struct tmp_objdir *);*/ void tmp_objdir_add_as_alternate (const struct tmp_objdir *); +/* +* Replaces the main object store in the current process with the temporary +* object directory and makes the former main object store an alternate. +* If will_destroy is nonzero, the object directory may not be migrated. +*/ +void tmp_objdir_replace_primary_odb (struct tmp_objdir *,int will_destroy); + +/* +* If the primary object database was replaced by a temporary object directory, +* restore it to its original value while keeping the directory contents around. +* Returns NULL if the primary object database was not replaced. +*/ +struct tmp_objdir *tmp_objdir_unapply_primary_odb (void ); + +/* +* Reapplies the former primary temporary object database, after protentially +* changing its relative path. +*/ +void tmp_objdir_reapply_primary_odb (struct tmp_objdir *,const char *old_cwd, +const char *new_cwd); + +#endif /* TMP_OBJDIR_H */
Signature · 1 line
gitgitgadget
PATCH 2/2 tmp-objdir: disable ref updates when replacing the primary odb
In reply to Neeraj K. Singh via GitGitGadgetFrom: Neeraj Singh <[email protected]>
When creating a subprocess with a temporary ODB, we set the GIT_QUARANTINE_ENVIRONMENT env var to tell child Git processes not to update refs, since the tmp-objdir may go away.
Introduce a similar mechanism for in-process temporary ODBs when we call tmp_objdir_replace_primary_odb. Now both mechanisms set the disable_ref_updates flag on the odb, which is queried by the ref_transaction_prepare function.
Note: This change adds an assumption that the state of the_repository is relevant for any ref transaction that might be initiated. Unwinding this assumption should be straightforward by saving the relevant repository to query in the transaction or the ref_store.
Peff's test case was invoking ref updates via the cachetextconv setting. That particular code silently does nothing when a ref update is forbidden. See the call to notes_cache_put in fill_textconv where errors are ignored.
--- environment.c | 4 ++++ object-file.c | 6 ++++++ object-store.h | 9 ++++++++- refs.c | 2 +- repository.c | 2 ++ repository.h | 1 + 6 files changed, 22 insertions(+), 2 deletions(-)
environment.c +4 -0
@@ -169,6 +169,10 @@ void setup_git_env(const char *git_dir) args.graft_file =getenv_safe (&to_free,GRAFT_ENVIRONMENT ); args.index_file =getenv_safe (&to_free,INDEX_ENVIRONMENT ); args.alternate_db =getenv_safe (&to_free,ALTERNATE_DB_ENVIRONMENT ); +if (getenv (GIT_QUARANTINE_ENVIRONMENT )) { + args.disable_ref_updates =1 ; + } +repo_set_gitdir (the_repository, git_dir, &args);strvec_clear (&to_free);
object-file.c +6 -0
@@ -699,6 +699,12 @@ struct object_directory *set_temporary_primary_odb(const char *dir, int will_des*/ new_odb =xcalloc (1 ,sizeof (*new_odb)); new_odb->path =xstrdup (dir); + +/* +* Disable ref updates while a temporary odb is active, since +* the objects in the database may roll back. +*/ + new_odb->disable_ref_updates =1 ; new_odb->will_destroy = will_destroy; new_odb->next = the_repository->objects->odb; the_repository->objects->odb = new_odb;
object-store.h +8 -1
@@ -27,10 +27,17 @@ struct object_directory {uint32_t loose_objects_subdir_seen[8 ];/* 256 bits */ struct oidtree *loose_objects_cache; +/* +* This is a temporary object store created by the tmp_objdir +* facility. Disable ref updates since the objects in the store +* might be discarded on rollback. +*/ + unsigned int disable_ref_updates : 1; +/* * This object store is ephemeral, so there is no need to fsync. */ -int will_destroy; + unsigned int will_destroy : 1;/* * Path to the alternative object store. If this is a relative path,
refs.c +1 -1
@@ -2137,7 +2137,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,break ; } -if (getenv (GIT_QUARANTINE_ENVIRONMENT )) { +if (the_repository->objects->odb->disable_ref_updates) {strbuf_addstr (err,_ ("ref updates forbidden inside quarantine environment" ));return -1 ;
repository.c +2 -0
@@ -80,6 +80,8 @@ void repo_set_gitdir(struct repository *repo,expand_base_dir (&repo->objects->odb->path, o->object_dir, repo->commondir,"objects" ); + repo->objects->odb->disable_ref_updates = o->disable_ref_updates; +free (repo->objects->alternate_db); repo->objects->alternate_db =xstrdup_or_null (o->alternate_db); expand_base_dir(&repo->graft_file, o->graft_file,
repository.h +1 -0
@@ -158,6 +158,7 @@ struct set_gitdir_args {const char *graft_file;const char *index_file;const char *alternate_db; +int disable_ref_updates; }; void repo_set_gitdir(struct repository *repo, const char *root,
Signature · 1 line
gitgitgadget
Re: [PATCH 2/2] tmp-objdir: disable ref updates when replacing the primary odb
object-store.h struct object_directory { · line 40
Quoted 5 lines · Neeraj Singh via GitGitGadget
object-store.h +1
-1/* * This object store is ephemeral, so there is no need to fsync. */ -int will_destroy; + unsigned int will_destroy : 1;
<CANQDOddCC7+gGUy1VBxxwvN7ieP+N8mQhbxK2xx6ySqZc6U7-g@mail.gmail.com> ?
(https://github.com/git/git/pull/1076#discussion_r750645345)
Re: [PATCH 2/2] tmp-objdir: disable ref updates when replacing the primary odb
Quoted 11 lines · Junio C Hamano
object-store.h struct object_directory { · line 40
Quoted 5 lines · Neeraj Singh via GitGitGadget
object-store.h +1
-1/* * This object store is ephemeral, so there is no need to fsync. */ -int will_destroy; + unsigned int will_destroy : 1;<CANQDOddCC7+gGUy1VBxxwvN7ieP+N8mQhbxK2xx6ySqZc6U7-g@mail.gmail.com> ?
Quoted 1 line
(https://github.com/git/git/pull/1076#discussion_r750645345)
Thanks for noticing this! I also lost one other change while splitting this out: we are referencing the_repository from the refs code, but as of 34224e14d we should be picking it up from the ref_store. I'll submit an updated series as soon as it passes CI.
Thanks,
Neeraj
Re: [PATCH 2/2] tmp-objdir: disable ref updates when replacing the primary odb
Quoted 12 lines · Neeraj Singh
Quoted 11 lines · Junio C Hamano
object-store.h struct object_directory { · line 40
Quoted 5 lines · Neeraj Singh via GitGitGadget
object-store.h +1
-1/* * This object store is ephemeral, so there is no need to fsync. */ -int will_destroy; + unsigned int will_destroy : 1;<CANQDOddCC7+gGUy1VBxxwvN7ieP+N8mQhbxK2xx6ySqZc6U7-g@mail.gmail.com> ?
Quoted 7 lines
Quoted 1 line
(https://github.com/git/git/pull/1076#discussion_r750645345)
Thanks for noticing this! I also lost one other change while splitting this out: we are referencing
the_repositoryfrom the refs code, but as of34224e14dwe should be picking it up from theref_store. I'll submit an updated series as soon as it passes CI.
No rush.
Reviewers and other project participants would appreciate you more if you took a deep breath, after seeing a CI success, and gave a final re-reading of the patches with a critical pair of eyes, before you send the updated series out.
Thanks.
v2 cover ns/tmp-objdir: add support for temporary writable databases
In reply to Neeraj K. Singh via GitGitGadgetV2 changes: I lost a couple changes in the shuffle while splitting these changes out
* Make the will-destroy boolean a single bit field of type unsigned int so that it doesn't change twice in this small patch series.
* Remove a the_repository reference in the disable ref updates code. Now the repository is taken from the ref_store.
New interface into the tmp-objdir API to help in-core use of the quarantine feature.
This patch series was formerly part of the ns/batched-fsync topic [1]. It's now split out into its own gitgitgadget PR and discussion thread since it is the base for en/remerge-diff as well.
The most recent feedback was in [2]. I removed printing from prune_subdir and simplified the strbuf handling in prune_tmp_file.
References: [1]
https://lore.kernel.org/git/[email protected]/
[2]
https://lore.kernel.org/git/CABPp-BH6m4q_EoX77bqLcpCN1HRfJ_XayeCV2O0sRybX53rPrw@mail.gmail.com/
Neeraj Singh (2):
tmp-objdir: new API for creating temporary writable databases
tmp-objdir: disable ref updates when replacing the primary odb
builtin/prune.c | 20 ++++++++++++--- builtin/receive-pack.c | 2 +- environment.c | 9 +++++++ object-file.c | 50 ++++++++++++++++++++++++++++++++++++-- object-store.h | 26 ++++++++++++++++++++ object.c | 2 +- refs.c | 2 +- repository.c | 2 ++ repository.h | 1 + tmp-objdir.c | 55 +++++++++++++++++++++++++++++++++++++++--- tmp-objdir.h | 29 +++++++++++++++++++--- 11 files changed, 183 insertions(+), 15 deletions(-)
base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1091%2Fneerajsi-msft%2Fns%2Ftmp-objdir-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1091/neerajsi-msft/ns/tmp-objdir-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1091
Range-diff vs v1:
1: 4ae4303595e ! 1: 36c00613d9a tmp-objdir: new API for creating temporary writable databases
@@ object-store.h: struct object_directory {
+ /*
+ * This object store is ephemeral, so there is no need to fsync.
+ */
-+ int will_destroy;
++ unsigned int will_destroy : 1;
+
/*
* Path to the alternative object store. If this is a relative path,
2: d8ae001500c ! 2: f667cbcc47d tmp-objdir: disable ref updates when replacing the primary odb
@@ object-store.h: struct object_directory {
/*
* This object store is ephemeral, so there is no need to fsync.
*/
-- int will_destroy;
-+ unsigned int will_destroy : 1;
-
- /*
- * Path to the alternative object store. If this is a relative path,
## refs.c ##
@@ refs.c: int ref_transaction_prepare(struct ref_transaction *transaction,
@@ refs.c: int ref_transaction_prepare(struct ref_transaction *transaction,
}
- if (getenv(GIT_QUARANTINE_ENVIRONMENT)) {
-+ if (the_repository->objects->odb->disable_ref_updates) {
++ if (refs->repo->objects->odb->disable_ref_updates) {
strbuf_addstr(err,
_("ref updates forbidden inside quarantine environment"));
return -1;Signature · 1 line
gitgitgadget
PATCH v2 1/2 tmp-objdir: new API for creating temporary writable databases
From: Neeraj Singh <[email protected]>
The tmp_objdir API provides the ability to create temporary object directories, but was designed with the goal of having subprocesses access these object stores, followed by the main process migrating objects from it to the main object store or just deleting it. The subprocesses would view it as their primary datastore and write to it.
Here we add the tmp_objdir_replace_primary_odb function that replaces the current process's writable "main" object directory with the specified one. The previous main object directory is restored in either tmp_objdir_migrate or tmp_objdir_destroy.
For the --remerge-diff usecase, add a new will_destroy flag in struct object_database to mark ephemeral object databases that do not require fsync durability.
Add 'git prune' support for removing temporary object databases, and make sure that they have a name starting with tmp_ and containing an operation-specific name.
--- builtin/prune.c | 20 ++++++++++++--- builtin/receive-pack.c | 2 +- environment.c | 5 ++++ object-file.c | 44 +++++++++++++++++++++++++++++++-- object-store.h | 19 +++++++++++++++ object.c | 2 +- tmp-objdir.c | 55 +++++++++++++++++++++++++++++++++++++++--- tmp-objdir.h | 29 +++++++++++++++++++--- 8 files changed, 162 insertions(+), 14 deletions(-)
builtin/prune.c +16 -4
@@ -26,10 +26,22 @@ static int prune_tmp_file(const char *fullpath)return error ("Could not stat '%s'" , fullpath);if (st.st_mtime > expire)return 0 ; -if (show_only || verbose) -printf ("Removing stale temporary file %s\n" , fullpath); -if (!show_only) -unlink_or_warn (fullpath); +if (S_ISDIR (st.st_mode)) { +if (show_only || verbose) +printf ("Removing stale temporary directory %s\n" , fullpath); +if (!show_only) { +struct strbuf remove_dir_buf =STRBUF_INIT ; + +strbuf_addstr (&remove_dir_buf, fullpath); +remove_dir_recursively (&remove_dir_buf,0 ); +strbuf_release (&remove_dir_buf); + } + }else { +if (show_only || verbose) +printf ("Removing stale temporary file %s\n" , fullpath); +if (!show_only) +unlink_or_warn (fullpath); + }return 0 ; }
builtin/receive-pack.c +1 -1
@@ -2213,7 +2213,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)strvec_push (&child.args, alt_shallow_file); } - tmp_objdir =tmp_objdir_create (); + tmp_objdir =tmp_objdir_create ("incoming" );if (!tmp_objdir) {if (err_fd >0 )close (err_fd);
environment.c +5 -0
@@ -17,6 +17,7 @@#include "commit.h" #include "strvec.h" #include "object-store.h" +#include "tmp-objdir.h" #include "chdir-notify.h" #include "shallow.h"
@@ -331,10 +332,14 @@ static void update_relative_gitdir(const char *name, void *data) {char *path =reparent_relative_path (old_cwd, new_cwd,get_git_dir ()); +struct tmp_objdir *tmp_objdir =tmp_objdir_unapply_primary_odb ();trace_printf_key (&trace_setup_key,"setup: move $GIT_DIR to '%s'" , path); +set_git_dir_1 (path); +if (tmp_objdir) +tmp_objdir_reapply_primary_odb (tmp_objdir, old_cwd, new_cwd);free (path); }
object-file.c +42 -2
56 lines not on this page · raw message
object-store.h +19 -0
@@ -27,6 +27,11 @@ struct object_directory {uint32_t loose_objects_subdir_seen[8 ];/* 256 bits */ struct oidtree *loose_objects_cache; +/* +* This object store is ephemeral, so there is no need to fsync. +*/ + unsigned int will_destroy : 1; +/* * Path to the alternative object store. If this is a relative path, * it is relative to the current working directory.
@@ -58,6 +63,17 @@ void add_to_alternates_file(const char *dir);*/ void add_to_alternates_memory (const char *dir); +/* +* Replace the current writable object directory with the specified temporary +* object directory; returns the former primary object directory. +*/ +struct object_directory *set_temporary_primary_odb (const char *dir,int will_destroy); + +/* +* Restore a previous ODB replaced by set_temporary_main_odb. +*/ +void restore_primary_odb (struct object_directory *restore_odb,const char *old_path); +/* * Populate and return the loose object cache array corresponding to the * given object ID.
@@ -68,6 +84,9 @@ struct oidtree *odb_loose_cache(struct object_directory *odb,/* Empty the loose object cache for the specified object directory. */ void odb_clear_loose_cache (struct object_directory *odb); +/* Clear and free the specified object directory */ +void free_object_directory (struct object_directory *odb); +struct packed_git {struct hashmap_entry packmap_ent;struct packed_git *next;
object.c +1 -1
@@ -513,7 +513,7 @@ struct raw_object_store *raw_object_store_new(void)return o; } -static void free_object_directory (struct object_directory *odb) +void free_object_directory (struct object_directory *odb) {free (odb->path);odb_clear_loose_cache (odb);
tmp-objdir.c +52 -3
102 lines not on this page · raw message
tmp-objdir.h +26 -3
@@ -10,7 +10,7 @@* * Example: * -* struct tmp_objdir *t = tmp_objdir_create(); +* struct tmp_objdir *t = tmp_objdir_create("incoming"); * if (!run_command_v_opt_cd_env(cmd, 0, NULL, tmp_objdir_env(t)) && * !tmp_objdir_migrate(t)) * printf("success!\n");
@@ -22,9 +22,10 @@struct tmp_objdir ;/* -* Create a new temporary object directory; returns NULL on failure. +* Create a new temporary object directory with the specified prefix; +* returns NULL on failure. */ -struct tmp_objdir *tmp_objdir_create (void ); +struct tmp_objdir *tmp_objdir_create (const char *prefix);/* * Return a list of environment strings, suitable for use with
@@ -51,4 +52,26 @@ int tmp_objdir_destroy(struct tmp_objdir *);*/ void tmp_objdir_add_as_alternate (const struct tmp_objdir *); +/* +* Replaces the main object store in the current process with the temporary +* object directory and makes the former main object store an alternate. +* If will_destroy is nonzero, the object directory may not be migrated. +*/ +void tmp_objdir_replace_primary_odb (struct tmp_objdir *,int will_destroy); + +/* +* If the primary object database was replaced by a temporary object directory, +* restore it to its original value while keeping the directory contents around. +* Returns NULL if the primary object database was not replaced. +*/ +struct tmp_objdir *tmp_objdir_unapply_primary_odb (void ); + +/* +* Reapplies the former primary temporary object database, after protentially +* changing its relative path. +*/ +void tmp_objdir_reapply_primary_odb (struct tmp_objdir *,const char *old_cwd, +const char *new_cwd); + +#endif /* TMP_OBJDIR_H */
Signature · 1 line
gitgitgadget
PATCH v2 2/2 tmp-objdir: disable ref updates when replacing the primary odb
In reply to Neeraj K. Singh via GitGitGadgetFrom: Neeraj Singh <[email protected]>
When creating a subprocess with a temporary ODB, we set the GIT_QUARANTINE_ENVIRONMENT env var to tell child Git processes not to update refs, since the tmp-objdir may go away.
Introduce a similar mechanism for in-process temporary ODBs when we call tmp_objdir_replace_primary_odb. Now both mechanisms set the disable_ref_updates flag on the odb, which is queried by the ref_transaction_prepare function.
Note: This change adds an assumption that the state of the_repository is relevant for any ref transaction that might be initiated. Unwinding this assumption should be straightforward by saving the relevant repository to query in the transaction or the ref_store.
Peff's test case was invoking ref updates via the cachetextconv setting. That particular code silently does nothing when a ref update is forbidden. See the call to notes_cache_put in fill_textconv where errors are ignored.
--- environment.c | 4 ++++ object-file.c | 6 ++++++ object-store.h | 7 +++++++ refs.c | 2 +- repository.c | 2 ++ repository.h | 1 + 6 files changed, 21 insertions(+), 1 deletion(-)
environment.c +4 -0
@@ -169,6 +169,10 @@ void setup_git_env(const char *git_dir) args.graft_file =getenv_safe (&to_free,GRAFT_ENVIRONMENT ); args.index_file =getenv_safe (&to_free,INDEX_ENVIRONMENT ); args.alternate_db =getenv_safe (&to_free,ALTERNATE_DB_ENVIRONMENT ); +if (getenv (GIT_QUARANTINE_ENVIRONMENT )) { + args.disable_ref_updates =1 ; + } +repo_set_gitdir (the_repository, git_dir, &args);strvec_clear (&to_free);
object-file.c +6 -0
@@ -699,6 +699,12 @@ struct object_directory *set_temporary_primary_odb(const char *dir, int will_des*/ new_odb =xcalloc (1 ,sizeof (*new_odb)); new_odb->path =xstrdup (dir); + +/* +* Disable ref updates while a temporary odb is active, since +* the objects in the database may roll back. +*/ + new_odb->disable_ref_updates =1 ; new_odb->will_destroy = will_destroy; new_odb->next = the_repository->objects->odb; the_repository->objects->odb = new_odb;
object-store.h +7 -0
@@ -27,6 +27,13 @@ struct object_directory {uint32_t loose_objects_subdir_seen[8 ];/* 256 bits */ struct oidtree *loose_objects_cache; +/* +* This is a temporary object store created by the tmp_objdir +* facility. Disable ref updates since the objects in the store +* might be discarded on rollback. +*/ + unsigned int disable_ref_updates : 1; +/* * This object store is ephemeral, so there is no need to fsync. */
refs.c +1 -1
@@ -2137,7 +2137,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,break ; } -if (getenv (GIT_QUARANTINE_ENVIRONMENT )) { +if (refs->repo->objects->odb->disable_ref_updates) {strbuf_addstr (err,_ ("ref updates forbidden inside quarantine environment" ));return -1 ;
repository.c +2 -0
@@ -80,6 +80,8 @@ void repo_set_gitdir(struct repository *repo,expand_base_dir (&repo->objects->odb->path, o->object_dir, repo->commondir,"objects" ); + repo->objects->odb->disable_ref_updates = o->disable_ref_updates; +free (repo->objects->alternate_db); repo->objects->alternate_db =xstrdup_or_null (o->alternate_db); expand_base_dir(&repo->graft_file, o->graft_file,
repository.h +1 -0
@@ -158,6 +158,7 @@ struct set_gitdir_args {const char *graft_file;const char *index_file;const char *alternate_db; +int disable_ref_updates; }; void repo_set_gitdir(struct repository *repo, const char *root,
Signature · 1 line
gitgitgadget
Re: [PATCH 2/2] tmp-objdir: disable ref updates when replacing the primary odb
In reply to Junio C HamanoQuoted 23 lines · Junio C Hamano
Quoted 12 lines · Neeraj Singh
Quoted 11 lines · Junio C Hamano
object-store.h struct object_directory { · line 40
Quoted 5 lines · Neeraj Singh via GitGitGadget
/* * This object store is ephemeral, so there is no need to fsync. */object-store.h +1
-1-int will_destroy; + unsigned int will_destroy : 1;<CANQDOddCC7+gGUy1VBxxwvN7ieP+N8mQhbxK2xx6ySqZc6U7-g@mail.gmail.com> ?
Quoted 7 lines
Quoted 1 line
(https://github.com/git/git/pull/1076#discussion_r750645345)
Thanks for noticing this! I also lost one other change while splitting this out: we are referencing
the_repositoryfrom the refs code, but as of34224e14dwe should be picking it up from theref_store. I'll submit an updated series as soon as it passes CI.
Quoted 8 lines
No rush.
Reviewers and other project participants would appreciate you more if you took a deep breath, after seeing a CI success, and gave a final re-reading of the patches with a critical pair of eyes, before you send the updated series out.
Thanks.
Fair enough. Of course I didn't see your email before I resubmitted. Thanks for the feedback.
Re: [PATCH v2 2/2] tmp-objdir: disable ref updates when replacing the primary odb
In reply to Neeraj Singh via GitGitGadgetQuoted 9 lines · Neeraj Singh via GitGitGadget
From: Neeraj Singh <[email protected]>
Note: This change adds an assumption that the state of
the_repositoryis relevant for any ref transaction that might be initiated. Unwinding this assumption should be straightforward by saving the relevant repository to query in the transaction or theref_store.
This part of the commit description needs to be deleted, since it no longer applies. We pick up the repo from the ref_store now.
Re: [PATCH v2 1/2] tmp-objdir: new API for creating temporary writable databases
In reply to Neeraj Singh via GitGitGadgetbuiltin/prune.c static int prune_tmp_file(const char *fullpath) · line 29
Quoted 13 lines · Neeraj Singh via GitGitGadget
builtin/prune.c +1
-4@@ -26,10 +26,22 @@ static int prune_tmp_file(const char *fullpath)return error ("Could not stat '%s'" , fullpath);if (st.st_mtime > expire)return 0 ; -if (show_only || verbose) -printf ("Removing stale temporary file %s\n" , fullpath); -if (!show_only) -unlink_or_warn (fullpath); +if (S_ISDIR (st.st_mode)) {
Because the updated tmp_objdir_create() always uses "tmp_objdir-" as the common prefix (instead of "incoming-" that we used to use, prune_cruft() will call this function not just for temporary files for loose objects, but also for directories. So a new code to do an equivalent of "rm -fr" is added here. OK.
Quoted 44 lines
builtin/prune.c +15
-0+if (show_only || verbose) +printf ("Removing stale temporary directory %s\n" , fullpath); +if (!show_only) { +struct strbuf remove_dir_buf =STRBUF_INIT ; + +strbuf_addstr (&remove_dir_buf, fullpath); +remove_dir_recursively (&remove_dir_buf,0 ); +strbuf_release (&remove_dir_buf); + } + }else { +if (show_only || verbose) +printf ("Removing stale temporary file %s\n" , fullpath); +if (!show_only) +unlink_or_warn (fullpath); + }return 0 ; }builtin/receive-pack.c +1
-1@@ -2213,7 +2213,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)strvec_push (&child.args, alt_shallow_file); } - tmp_objdir =tmp_objdir_create (); + tmp_objdir =tmp_objdir_create ("incoming" );if (!tmp_objdir) {if (err_fd >0 )close (err_fd);environment.c +1
-0@@ -17,6 +17,7 @@#include "commit.h" #include "strvec.h" #include "object-store.h" +#include "tmp-objdir.h" #include "chdir-notify.h" #include "shallow.h"
environment.c static void update_relative_gitdir(const char *name, · line 340
Quoted 9 lines
environment.c +2
-0@@ -331,10 +332,14 @@ static void update_relative_gitdir(const char *name, void *data) {char *path =reparent_relative_path (old_cwd, new_cwd,get_git_dir ()); +struct tmp_objdir *tmp_objdir =tmp_objdir_unapply_primary_odb ();trace_printf_key (&trace_setup_key,"setup: move $GIT_DIR to '%s'" , path); +set_git_dir_1 (path);
If a blank line needs to be added, have it between the variable declarations and the first statement (i.e. before the above call to "trace_printf_key()").
environment.c static void update_relative_gitdir(const char *name, · line 344
Quoted 4 lines
environment.c +2
-0+if (tmp_objdir) +tmp_objdir_reapply_primary_odb (tmp_objdir, old_cwd, new_cwd);free (path); }
This is called during set_git_dir(), which happens fairly early in the set-up sequence. I wonder if there is a real use case that creates a tmp-objdir that early in the process to require this unapply-reapply sequence.
environment.c int hash_object_file(const struct git_hash_algo *algo, const void *buf, · line 1851
Quoted 9 lines
environment.c +3
-2@@ -1809,8 +1846,11 @@ int hash_object_file(const struct git_hash_algo *algo, const void *buf,/* Finalize a file on disk, and close it. */ static void close_loose_object (int fd) { -if (fsync_object_files) -fsync_or_die (fd,"loose object file" ); +if (!the_repository->objects->odb->will_destroy) { +if (fsync_object_files) +fsync_or_die (fd,"loose object file" );
OK, so we omit fsync because these newly created loose objects may not survive and instead get discarded. Presumably when we migrate them to the real object store, we'll make sure they hit the disk platter in some other way?
... goes and cheats by reading ahead ...
Ahh, ok, new objects created in a temporary object store that is marked with the will_destroy bit is not allowed to migrate to the real object store, so there is no point to fsync them.
set_temporary_primary_odb() and tmp_objdir_replace_primary_odb() can mark the temporary one to be throw-away, but unfortunately there is no caller in this step, so it is a bit hard to see when a throw-away object store is useful. I guess remerge-diff wants to do tentative merges that create new objects in a throw-away object directory, because it is logically a read-only operation.
Quoted 12 lines
tmp-objdir.c +1
-0@@ -1,5 +1,6 @@#include "cache.h" #include "tmp-objdir.h" +#include "chdir-notify.h" #include "dir.h" #include "sigchain.h" #include "string-list.h"
tmp-objdir.c line 16
Quoted 5 lines
tmp-objdir.c +2
-0@@ -11,6 +12,8 @@struct tmp_objdir {struct strbuf path;struct strvec env; +struct object_directory *prev_odb; +int will_destroy;
The other one was a one-bit unsigned bitfield, but this is a full integer. I somehow think that the other one can and should be a full integer, too---it's not like there are tons of bits need to be stored in the structure or we will have tons of instances of the structure that storing many bits compactly matters.
Thanks.
Re: [PATCH v2 1/2] tmp-objdir: new API for creating temporary writable databases
Quoted 17 lines · Junio C Hamano
environment.c static void update_relative_gitdir(const char *name, · line 340
Quoted 10 lines · Neeraj Singh via GitGitGadget
environment.c +2
-0@@ -331,10 +332,14 @@ static void update_relative_gitdir(const char *name, void *data) {char *path =reparent_relative_path (old_cwd, new_cwd,get_git_dir ()); +struct tmp_objdir *tmp_objdir =tmp_objdir_unapply_primary_odb ();trace_printf_key (&trace_setup_key,"setup: move $GIT_DIR to '%s'" , path); +set_git_dir_1 (path);If a blank line needs to be added, have it between the variable declarations and the first statement (i.e. before the above call to "
trace_printf_key()").
Will fix.
Quoted 10 lines
environment.c static void update_relative_gitdir(const char *name, · line 344
Quoted 4 lines
environment.c +2
-0+if (tmp_objdir) +tmp_objdir_reapply_primary_odb (tmp_objdir, old_cwd, new_cwd);free (path); }This is called during
set_git_dir(), which happens fairly early in the set-up sequence. I wonder if there is a real use case that creates a tmp-objdir that early in the process to require this unapply-reapply sequence.
The lack of this code was causing a failure, I believe in t2107-update-index-basic.sh: "--refresh triggers late setup_work_tree".
This problem came up after applying: https://lore.kernel.org/git/4a40fd4a29a468b9ce320bc7b22f19e5a526fad6.1637020263.git.gitgitgadget@gmail.com/
I thought it would be best to fix this in the tmp-objdir code so that callers could plug/unplug bulk checkin without any subtle surprises.
Quoted 21 lines
object-file.c int hash_object_file(const struct git_hash_algo *algo, const void *buf, · line 1851
Quoted 9 lines
object-file.c +3
-2@@ -1809,8 +1846,11 @@ int hash_object_file(const struct git_hash_algo *algo, const void *buf,/* Finalize a file on disk, and close it. */ static void close_loose_object (int fd) { -if (fsync_object_files) -fsync_or_die (fd,"loose object file" ); +if (!the_repository->objects->odb->will_destroy) { +if (fsync_object_files) +fsync_or_die (fd,"loose object file" );OK, so we omit fsync because these newly created loose objects may not survive and instead get discarded. Presumably when we migrate them to the real object store, we'll make sure they hit the disk platter in some other way?
... goes and cheats by reading ahead ...
Ahh, ok, new objects created in a temporary object store that is marked with the
will_destroybit is not allowed to migrate to the real object store, so there is no point to fsync them.
Quoted 7 lines
set_temporary_primary_odb()andtmp_objdir_replace_primary_odb()can mark the temporary one to be throw-away, but unfortunately there is no caller in this step, so it is a bit hard to see when a throw-away object store is useful. I guess remerge-diff wants to do tentative merges that create new objects in a throw-away object directory, because it is logically a read-only operation.
Yes, this code is there exactly for remerge-diff and anyone doing something similar in the future.
Quoted 15 lines
Quoted 15 lines
tmp-objdir.c +1
-0@@ -1,5 +1,6 @@#include "cache.h" #include "tmp-objdir.h" +#include "chdir-notify.h" #include "dir.h" #include "sigchain.h" #include "string-list.h" @@ -11,6 +12,8 @@struct tmp_objdir {struct strbuf path;struct strvec env;
Quoted 9 lines
tmp-objdir.c line 16
Quoted 2 lines
tmp-objdir.c +2
-0+struct object_directory *prev_odb; +int will_destroy;The other one was a one-bit unsigned bitfield, but this is a full integer. I somehow think that the other one can and should be a full integer, too---it's not like there are tons of bits need to be stored in the structure or we will have tons of instances of the structure that storing many bits compactly matters.
The principle I was trying to follow here is that the only flag in a structure might as well be a full integer, but when we have two or more it might be worth combining them into a single machine word. Given that these are not highly replicated structures, you're right that's it's not a big benefit.
I'll switch everything to an int and call it good.
Given that this patch series introduces functions with no users, are you going to hold off on putting this into 'next' until another next-worthy patch series is ready? I've already reworked the batch mode stuff on Github, but I'll need to do a lot more testing before sending it to the list.
Thanks,
Neeraj
Re: [PATCH v2 1/2] tmp-objdir: new API for creating temporary writable databases
Quoted 11 lines · Neeraj Singh
Quoted 10 lines
environment.c static void update_relative_gitdir(const char *name, · line 344
Quoted 4 lines
environment.c +2
-0+if (tmp_objdir) +tmp_objdir_reapply_primary_odb (tmp_objdir, old_cwd, new_cwd);free (path); }This is called during
set_git_dir(), which happens fairly early in the set-up sequence. I wonder if there is a real use case that creates a tmp-objdir that early in the process to require this unapply-reapply sequence.
Quoted 7 lines
The lack of this code was causing a failure, I believe in
t2107-update-index-basic.sh: "--refresh triggers latesetup_work_tree".This problem came up after applying: https://lore.kernel.org/git/4a40fd4a29a468b9ce320bc7b22f19e5a526fad6.1637020263.git.gitgitgadget@gmail.com/
I thought it would be best to fix this in the tmp-objdir code so that callers could plug/unplug bulk checkin without any subtle surprises.
OK, I think that is fine.
As a slightly-related tangent that is outside the topic, I think we should revisit "update-index", which is one of the oldest plumbing commands with its own quirks. I do not offhand see why it needs to sprinkle this many setup_work_tree() calls everywhere. Having an index to work on means we must have a working tree to update and/or refresh from. We should be able to get away with the NEED_WORK_TREE bit in the git.c::commands[] table for this command. If this were a more recent command, I may suspect that there were valid reasons like "in this particular mode, update-index must work inside a bare repository" to force us to take this unusual program structure, but because this is probably a lot older than NEED_WORK_TREE bit, I would not be surprised if the answer were "nobody noticed the ugliness so far".
Quoted 3 lines
Given that this patch series introduces functions with no users, are you going to hold off on putting this into 'next' until another next-worthy patch series is ready?
Even without any existing callers, as long as we see Reviewed-by: by Elijah, who we know will have to build on top of this series, I think this can and should go to 'next'.
Thanks.
v3 cover ns/tmp-objdir: add support for temporary writable databases
In reply to Neeraj K. Singh via GitGitGadgetV3 (hopefully final):
* Fix the commit description for patch [2/2] to reflect the fact that disabling ref updates no longer depends on the_repository.
* Add a link to Jeff King's test case in patch [2/2]. The test relies on remerge-diff, so it can't be directly included here.
* Adjust line spacing in update_relative_gitdir (gitster)
* Switch struct object_directory to use full-width integers rather than flags (gitster)
* Fix typo s/protentially/potentially (neerajsi)
V2 changes: I lost a couple changes in the shuffle while splitting these changes out
* Make the will-destroy boolean a single bit field of type unsigned int so that it doesn't change twice in this small patch series.
* Remove a the_repository reference in the disable ref updates code. Now the repository is taken from the ref_store.
New interface into the tmp-objdir API to help in-core use of the quarantine feature.
This patch series was formerly part of the ns/batched-fsync topic [1]. It's now split out into its own gitgitgadget PR and discussion thread since it is the base for en/remerge-diff as well.
The most recent feedback was in [2]. I removed printing from prune_subdir and simplified the strbuf handling in prune_tmp_file.
References: [1]
https://lore.kernel.org/git/[email protected]/
[2]
https://lore.kernel.org/git/CABPp-BH6m4q_EoX77bqLcpCN1HRfJ_XayeCV2O0sRybX53rPrw@mail.gmail.com/
Neeraj Singh (2):
tmp-objdir: new API for creating temporary writable databases
tmp-objdir: disable ref updates when replacing the primary odb
builtin/prune.c | 20 ++++++++++++--- builtin/receive-pack.c | 2 +- environment.c | 9 +++++++ object-file.c | 50 ++++++++++++++++++++++++++++++++++++-- object-store.h | 26 ++++++++++++++++++++ object.c | 2 +- refs.c | 2 +- repository.c | 2 ++ repository.h | 1 + tmp-objdir.c | 55 +++++++++++++++++++++++++++++++++++++++--- tmp-objdir.h | 29 +++++++++++++++++++--- 11 files changed, 183 insertions(+), 15 deletions(-)
base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1091%2Fneerajsi-msft%2Fns%2Ftmp-objdir-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1091/neerajsi-msft/ns/tmp-objdir-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1091
Range-diff vs v2:
1: 36c00613d9a ! 1: cccb3888070 tmp-objdir: new API for creating temporary writable databases
@@ environment.c: static void update_relative_gitdir(const char *name,
{
char *path = reparent_relative_path(old_cwd, new_cwd, get_git_dir());
+ struct tmp_objdir *tmp_objdir = tmp_objdir_unapply_primary_odb();
++
trace_printf_key(&trace_setup_key,
"setup: move $GIT_DIR to '%s'",
path);
-+
set_git_dir_1(path);
+ if (tmp_objdir)
+ tmp_objdir_reapply_primary_odb(tmp_objdir, old_cwd, new_cwd);
@@ object-store.h: struct object_directory {
+ /*
+ * This object store is ephemeral, so there is no need to fsync.
+ */
-+ unsigned int will_destroy : 1;
++ int will_destroy;
+
/*
* Path to the alternative object store. If this is a relative path,
@@ tmp-objdir.h: int tmp_objdir_destroy(struct tmp_objdir *);
void tmp_objdir_add_as_alternate(const struct tmp_objdir *);
+/*
-+ * Replaces the main object store in the current process with the temporary
++ * Replaces the writable object store in the current process with the temporary
+ * object directory and makes the former main object store an alternate.
+ * If will_destroy is nonzero, the object directory may not be migrated.
+ */
@@ tmp-objdir.h: int tmp_objdir_destroy(struct tmp_objdir *);
+struct tmp_objdir *tmp_objdir_unapply_primary_odb(void);
+
+/*
-+ * Reapplies the former primary temporary object database, after protentially
++ * Reapplies the former primary temporary object database, after potentially
+ * changing its relative path.
+ */
+void tmp_objdir_reapply_primary_odb(struct tmp_objdir *, const char *old_cwd, 2: f667cbcc47d ! 2: 4e44121c2d7 tmp-objdir: disable ref updates when replacing the primary odb
@@ Commit message
the disable_ref_updates flag on the odb, which is queried by
the ref_transaction_prepare function.
- Note: This change adds an assumption that the state of
- the_repository is relevant for any ref transaction that might
- be initiated. Unwinding this assumption should be straightforward
- by saving the relevant repository to query in the transaction or
- the ref_store.
-
- Peff's test case was invoking ref updates via the cachetextconv
+ Peff's test case [1] was invoking ref updates via the cachetextconv
setting. That particular code silently does nothing when a ref update is forbidden. See the call to notes_cache_put in fill_textconv where errors are ignored.
+ [1] https://lore.kernel.org/git/[email protected]/
+
@@ object-store.h: struct object_directory {
+ * facility. Disable ref updates since the objects in the store
+ * might be discarded on rollback.
+ */
-+ unsigned int disable_ref_updates : 1;
++ int disable_ref_updates;
+
/*
* This object store is ephemeral, so there is no need to fsync.
Signature · 1 line
gitgitgadget
PATCH v3 1/2 tmp-objdir: new API for creating temporary writable databases
From: Neeraj Singh <[email protected]>
The tmp_objdir API provides the ability to create temporary object directories, but was designed with the goal of having subprocesses access these object stores, followed by the main process migrating objects from it to the main object store or just deleting it. The subprocesses would view it as their primary datastore and write to it.
Here we add the tmp_objdir_replace_primary_odb function that replaces the current process's writable "main" object directory with the specified one. The previous main object directory is restored in either tmp_objdir_migrate or tmp_objdir_destroy.
For the --remerge-diff usecase, add a new will_destroy flag in struct object_database to mark ephemeral object databases that do not require fsync durability.
Add 'git prune' support for removing temporary object databases, and make sure that they have a name starting with tmp_ and containing an operation-specific name.
--- builtin/prune.c | 20 ++++++++++++--- builtin/receive-pack.c | 2 +- environment.c | 5 ++++ object-file.c | 44 +++++++++++++++++++++++++++++++-- object-store.h | 19 +++++++++++++++ object.c | 2 +- tmp-objdir.c | 55 +++++++++++++++++++++++++++++++++++++++--- tmp-objdir.h | 29 +++++++++++++++++++--- 8 files changed, 162 insertions(+), 14 deletions(-)
builtin/prune.c +16 -4
@@ -26,10 +26,22 @@ static int prune_tmp_file(const char *fullpath)return error ("Could not stat '%s'" , fullpath);if (st.st_mtime > expire)return 0 ; -if (show_only || verbose) -printf ("Removing stale temporary file %s\n" , fullpath); -if (!show_only) -unlink_or_warn (fullpath); +if (S_ISDIR (st.st_mode)) { +if (show_only || verbose) +printf ("Removing stale temporary directory %s\n" , fullpath); +if (!show_only) { +struct strbuf remove_dir_buf =STRBUF_INIT ; + +strbuf_addstr (&remove_dir_buf, fullpath); +remove_dir_recursively (&remove_dir_buf,0 ); +strbuf_release (&remove_dir_buf); + } + }else { +if (show_only || verbose) +printf ("Removing stale temporary file %s\n" , fullpath); +if (!show_only) +unlink_or_warn (fullpath); + }return 0 ; }
builtin/receive-pack.c +1 -1
@@ -2213,7 +2213,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)strvec_push (&child.args, alt_shallow_file); } - tmp_objdir =tmp_objdir_create (); + tmp_objdir =tmp_objdir_create ("incoming" );if (!tmp_objdir) {if (err_fd >0 )close (err_fd);
environment.c +5 -0
@@ -17,6 +17,7 @@#include "commit.h" #include "strvec.h" #include "object-store.h" +#include "tmp-objdir.h" #include "chdir-notify.h" #include "shallow.h"
@@ -331,10 +332,14 @@ static void update_relative_gitdir(const char *name, void *data) {char *path =reparent_relative_path (old_cwd, new_cwd,get_git_dir ()); +struct tmp_objdir *tmp_objdir =tmp_objdir_unapply_primary_odb (); +trace_printf_key (&trace_setup_key,"setup: move $GIT_DIR to '%s'" , path);set_git_dir_1 (path); +if (tmp_objdir) +tmp_objdir_reapply_primary_odb (tmp_objdir, old_cwd, new_cwd);free (path); }
object-file.c +42 -2
56 lines not on this page · raw message
object-store.h +19 -0
@@ -27,6 +27,11 @@ struct object_directory {uint32_t loose_objects_subdir_seen[8 ];/* 256 bits */ struct oidtree *loose_objects_cache; +/* +* This object store is ephemeral, so there is no need to fsync. +*/ +int will_destroy; +/* * Path to the alternative object store. If this is a relative path, * it is relative to the current working directory.
@@ -58,6 +63,17 @@ void add_to_alternates_file(const char *dir);*/ void add_to_alternates_memory (const char *dir); +/* +* Replace the current writable object directory with the specified temporary +* object directory; returns the former primary object directory. +*/ +struct object_directory *set_temporary_primary_odb (const char *dir,int will_destroy); + +/* +* Restore a previous ODB replaced by set_temporary_main_odb. +*/ +void restore_primary_odb (struct object_directory *restore_odb,const char *old_path); +/* * Populate and return the loose object cache array corresponding to the * given object ID.
@@ -68,6 +84,9 @@ struct oidtree *odb_loose_cache(struct object_directory *odb,/* Empty the loose object cache for the specified object directory. */ void odb_clear_loose_cache (struct object_directory *odb); +/* Clear and free the specified object directory */ +void free_object_directory (struct object_directory *odb); +struct packed_git {struct hashmap_entry packmap_ent;struct packed_git *next;
object.c +1 -1
@@ -513,7 +513,7 @@ struct raw_object_store *raw_object_store_new(void)return o; } -static void free_object_directory (struct object_directory *odb) +void free_object_directory (struct object_directory *odb) {free (odb->path);odb_clear_loose_cache (odb);
tmp-objdir.c +52 -3
102 lines not on this page · raw message
tmp-objdir.h +26 -3
@@ -10,7 +10,7 @@* * Example: * -* struct tmp_objdir *t = tmp_objdir_create(); +* struct tmp_objdir *t = tmp_objdir_create("incoming"); * if (!run_command_v_opt_cd_env(cmd, 0, NULL, tmp_objdir_env(t)) && * !tmp_objdir_migrate(t)) * printf("success!\n");
@@ -22,9 +22,10 @@struct tmp_objdir ;/* -* Create a new temporary object directory; returns NULL on failure. +* Create a new temporary object directory with the specified prefix; +* returns NULL on failure. */ -struct tmp_objdir *tmp_objdir_create (void ); +struct tmp_objdir *tmp_objdir_create (const char *prefix);/* * Return a list of environment strings, suitable for use with
@@ -51,4 +52,26 @@ int tmp_objdir_destroy(struct tmp_objdir *);*/ void tmp_objdir_add_as_alternate (const struct tmp_objdir *); +/* +* Replaces the writable object store in the current process with the temporary +* object directory and makes the former main object store an alternate. +* If will_destroy is nonzero, the object directory may not be migrated. +*/ +void tmp_objdir_replace_primary_odb (struct tmp_objdir *,int will_destroy); + +/* +* If the primary object database was replaced by a temporary object directory, +* restore it to its original value while keeping the directory contents around. +* Returns NULL if the primary object database was not replaced. +*/ +struct tmp_objdir *tmp_objdir_unapply_primary_odb (void ); + +/* +* Reapplies the former primary temporary object database, after potentially +* changing its relative path. +*/ +void tmp_objdir_reapply_primary_odb (struct tmp_objdir *,const char *old_cwd, +const char *new_cwd); + +#endif /* TMP_OBJDIR_H */
Signature · 1 line
gitgitgadget
PATCH v3 2/2 tmp-objdir: disable ref updates when replacing the primary odb
In reply to Neeraj K. Singh via GitGitGadgetFrom: Neeraj Singh <[email protected]>
When creating a subprocess with a temporary ODB, we set the GIT_QUARANTINE_ENVIRONMENT env var to tell child Git processes not to update refs, since the tmp-objdir may go away.
Introduce a similar mechanism for in-process temporary ODBs when we call tmp_objdir_replace_primary_odb. Now both mechanisms set the disable_ref_updates flag on the odb, which is queried by the ref_transaction_prepare function.
Peff's test case [1] was invoking ref updates via the cachetextconv setting. That particular code silently does nothing when a ref update is forbidden. See the call to notes_cache_put in fill_textconv where errors are ignored.
[1] https://lore.kernel.org/git/[email protected]/
--- environment.c | 4 ++++ object-file.c | 6 ++++++ object-store.h | 7 +++++++ refs.c | 2 +- repository.c | 2 ++ repository.h | 1 + 6 files changed, 21 insertions(+), 1 deletion(-)
environment.c +4 -0
@@ -169,6 +169,10 @@ void setup_git_env(const char *git_dir) args.graft_file =getenv_safe (&to_free,GRAFT_ENVIRONMENT ); args.index_file =getenv_safe (&to_free,INDEX_ENVIRONMENT ); args.alternate_db =getenv_safe (&to_free,ALTERNATE_DB_ENVIRONMENT ); +if (getenv (GIT_QUARANTINE_ENVIRONMENT )) { + args.disable_ref_updates =1 ; + } +repo_set_gitdir (the_repository, git_dir, &args);strvec_clear (&to_free);
object-file.c +6 -0
@@ -699,6 +699,12 @@ struct object_directory *set_temporary_primary_odb(const char *dir, int will_des*/ new_odb =xcalloc (1 ,sizeof (*new_odb)); new_odb->path =xstrdup (dir); + +/* +* Disable ref updates while a temporary odb is active, since +* the objects in the database may roll back. +*/ + new_odb->disable_ref_updates =1 ; new_odb->will_destroy = will_destroy; new_odb->next = the_repository->objects->odb; the_repository->objects->odb = new_odb;
object-store.h +7 -0
@@ -27,6 +27,13 @@ struct object_directory {uint32_t loose_objects_subdir_seen[8 ];/* 256 bits */ struct oidtree *loose_objects_cache; +/* +* This is a temporary object store created by the tmp_objdir +* facility. Disable ref updates since the objects in the store +* might be discarded on rollback. +*/ +int disable_ref_updates; +/* * This object store is ephemeral, so there is no need to fsync. */
refs.c +1 -1
@@ -2137,7 +2137,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,break ; } -if (getenv (GIT_QUARANTINE_ENVIRONMENT )) { +if (refs->repo->objects->odb->disable_ref_updates) {strbuf_addstr (err,_ ("ref updates forbidden inside quarantine environment" ));return -1 ;
repository.c +2 -0
@@ -80,6 +80,8 @@ void repo_set_gitdir(struct repository *repo,expand_base_dir (&repo->objects->odb->path, o->object_dir, repo->commondir,"objects" ); + repo->objects->odb->disable_ref_updates = o->disable_ref_updates; +free (repo->objects->alternate_db); repo->objects->alternate_db =xstrdup_or_null (o->alternate_db); expand_base_dir(&repo->graft_file, o->graft_file,
repository.h +1 -0
@@ -158,6 +158,7 @@ struct set_gitdir_args {const char *graft_file;const char *index_file;const char *alternate_db; +int disable_ref_updates; }; void repo_set_gitdir(struct repository *repo, const char *root,
Signature · 1 line
gitgitgadget
Quoted 11 lines · Neeraj K. Singh via GitGitGadget
V3 (hopefully final):
* Fix the commit description for patch [2/2] to reflect the fact that disabling ref updates no longer depends on
the_repository.
* Add a link to Jeff King's test case in patch [2/2]. The test relies on remerge-diff, so it can't be directly included here.
* Adjust line spacing inupdate_relative_gitdir(gitster)
* Switch structobject_directoryto use full-width integers rather than flags (gitster)
* Fix typo s/protentially/potentially (neerajsi)
This version looks good to me:
For future reference, when splitting one of your series apart, copying the relevant subset of the cc lines (e.g. from the description at
https://github.com/git/git/pull/1076 to the one at
https://github.com/gitgitgadget/git/pull/1091) would be helpful.