object-store refactoring 3 (replace objects, main ref store)
Subsystems (2)
cover object-store refactoring 3 (replace objects, main ref store)
This applies on top of 464416a2eaadf84d2bfdf795007863d03b222b7c (sb/packfiles-in-repository).
It is also available at https://github.com/stefanbeller/git/tree/object-store-3
This series will bring the replacement mechanism (git replace) into the object store.
The first patches are cleaning up a bit, and patches 6-19 are converting one function at a time using the tick-tock pattern with the #define trick. See cfc62fc98c (sha1_file: add repository argument to link_alt_odb_entry, 2018-03-23) for explanation.
Thanks,
Stefan
Stefan Beller (19):
replace_object.c: rename to use dash in file name
replace-object: move replace_object to object store
object-store: move lookup_replace_object to replace-object.h
replace-object: move replace objects prepared flag to object store
replace-object: check_replace_refs is safe in multi repo environment
refs: add repository argument to get_main_ref_store
refs: add repository argument to for_each_replace_ref
replace-object: add repository argument to replace_object_pos
replace-object: add repository argument to register_replace_object
replace-object: add repository argument to prepare_replace_object
replace-object: add repository argument to do_lookup_replace_object
replace-object: add repository argument to lookup_replace_object
refs: store the main ref store inside the repository struct
refs: allow for_each_replace_ref to handle arbitrary repositories
replace-object: allow replace_object_pos to handle arbitrary
repositories
replace-object: allow register_replace_object to handle arbitrary
repositories
replace-object: allow prepare_replace_object to handle arbitrary
repositories
replace-object: allow do_lookup_replace_object to handle arbitrary
repositories
replace-object: allow lookup_replace_object to handle arbitrary
repositories
Makefile | 2 +- builtin/mktag.c | 3 +- builtin/pack-refs.c | 3 +- builtin/replace.c | 4 +- cache.h | 19 ------- environment.c | 2 +- object-store.h | 16 ++++++ object.c | 3 +- refs.c | 80 ++++++++++++++-------------- refs.h | 4 +- replace_object.c => replace-object.c | 66 +++++++++++------------ replace-object.h | 35 ++++++++++++ repository.h | 5 ++ revision.c | 5 +- sha1_file.c | 7 +-- streaming.c | 3 +- t/helper/test-ref-store.c | 3 +- 17 files changed, 149 insertions(+), 111 deletions(-) rename replace_object.c => replace-object.c (56%) create mode 100644 replace-object.h
$ git diff 464416a2eaadf84d2bfdf795007863d03b222b7c..HEAD -- object-store.h repository.h
object-store.h +16 -0
@@ -93,6 +93,22 @@ struct raw_object_store {struct alternate_object_database *alt_odb_list;struct alternate_object_database **alt_odb_tail; +/* +* Objects that should be substituted by other objects +* (see git-replace(1)). +*/ +struct replace_objects { +/* +* An array of replacements. The array is kept sorted by the original +* sha1. +*/ +struct replace_object **items; + +int alloc, nr; + +unsigned prepared :1 ; + } replacements; +/* * private data *
repository.h +5 -0
@@ -26,6 +26,11 @@ struct repository {*/ struct raw_object_store *objects; +/* +* The store in which the refs are hold. +*/ +struct ref_store *main_ref_store; +/* * Path to the repository's graft file. * Cannot be NULL after initialization.
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 1/19 replace_object.c: rename to use dash in file name
This is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names.
Noticed while adding a header corresponding to this file.
--- Makefile | 2 +- replace_object.c => replace-object.c | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename replace_object.c => replace-object.c (100%)
Makefile +1 -1
@@ -871,7 +871,7 @@ LIB_OBJS += refs/packed-backend.o LIB_OBJS += refs/ref-cache.o LIB_OBJS += ref-filter.o LIB_OBJS += remote.o -LIB_OBJS += replace_object.o +LIB_OBJS += replace-object.o LIB_OBJS += repository.o LIB_OBJS += rerere.o LIB_OBJS += resolve-undo.o
replace-object.c renamed from replace_object.c +0 -0
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 2/19 replace-object: move replace_object to object store
In reply to Stefan BellerRefs belong to particular repositories, so the replacements defined by them should belong to a particular repository as well.
Move the definition of a single object replacement to a new header "replace-object.h". While at it replace the hardcoded 20 by GIT_MAX_RAWSZ.
--- object-store.h | 14 ++++++++++++++ replace-object.c | 40 ++++++++++++++++++---------------------- replace-object.h | 9 +++++++++ 3 files changed, 41 insertions(+), 22 deletions(-) create mode 100644 replace-object.h
object-store.h +14 -0
@@ -93,6 +93,20 @@ struct raw_object_store {struct alternate_object_database *alt_odb_list;struct alternate_object_database **alt_odb_tail; +/* +* Objects that should be substituted by other objects +* (see git-replace(1)). +*/ +struct replace_objects { +/* +* An array of replacements. The array is kept sorted by the original +* sha1. +*/ +struct replace_object **items; + +int alloc, nr; + } replacements; +/* * private data *
replace-object.c +18 -22
@@ -1,19 +1,11 @@#include "cache.h" +#include "replace-object.h" +#include "object-store.h" #include "sha1-lookup.h" #include "refs.h" +#include "repository.h" #include "commit.h" -/* -* An array of replacements. The array is kept sorted by the original -* sha1. -*/ -static struct replace_object { -unsigned char original[20 ]; -unsigned char replacement[20 ]; -} **replace_object; - -static int replace_object_alloc, replace_object_nr; -static const unsigned char *replace_sha1_access (size_t index,void *table) {struct replace_object **replace = table;
@@ -22,7 +14,8 @@ static const unsigned char *replace_sha1_access(size_t index, void *table)static int replace_object_pos (const unsigned char *sha1) { -return sha1_pos (sha1, replace_object, replace_object_nr, +return sha1_pos (sha1, the_repository->objects->replacements.items, + the_repository->objects->replacements.nr, replace_sha1_access); }
@@ -35,18 +28,21 @@ static int register_replace_object(struct replace_object *replace,if (ignore_dups)free (replace);else { -free (replace_object[pos]); - replace_object[pos] = replace; +free ( the_repository->objects->replacements.items[pos]); + the_repository->objects->replacements.items[pos] = replace; }return 1 ; } pos = -pos -1 ; -ALLOC_GROW (replace_object, replace_object_nr +1 , replace_object_alloc); - replace_object_nr++; -if (pos < replace_object_nr) -MOVE_ARRAY (replace_object + pos +1 , replace_object + pos, - replace_object_nr - pos -1 ); - replace_object[pos] = replace; +ALLOC_GROW ( the_repository->objects->replacements.items, + the_repository->objects->replacements.nr +1 , + the_repository->objects->replacements.alloc); + the_repository->objects->replacements.nr++; +if (pos < the_repository->objects->replacements.nr) +MOVE_ARRAY ( the_repository->objects->replacements.items + pos +1 , + the_repository->objects->replacements.items + pos, + the_repository->objects->replacements.nr - pos -1 ); + the_repository->objects->replacements.items[pos] = replace;return 0 ; }
@@ -84,7 +80,7 @@ static void prepare_replace_object(void)for_each_replace_ref (register_replace_ref,NULL ); replace_object_prepared =1 ; -if (!replace_object_nr) +if (!the_repository->objects->replacements.nr) check_replace_refs =0 ; }
@@ -113,7 +109,7 @@ const unsigned char *do_lookup_replace_object(const unsigned char *sha1) pos =replace_object_pos (cur);if (0 <= pos) - cur = replace_object[pos]->replacement; + cur = the_repository->objects->replacements.items[pos]->replacement; }while (0 <= pos);return cur;
replace-object.h new file +9 -0
@@ -0,0 +1,9 @@ +#ifndef REPLACE_OBJECT_H +#define REPLACE_OBJECT_H + +struct replace_object { +unsigned char original[GIT_MAX_RAWSZ ]; +unsigned char replacement[GIT_MAX_RAWSZ ]; +}; + +#endif /* REPLACE_OBJECT_H */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Re: [PATCH 02/19] replace-object: move replace_object to object store
Quoted 6 lines · Stefan Beller
Refs belong to particular repositories, so the replacements defined by them should belong to a particular repository as well.
Move the definition of a single object replacement to a new header "
replace-object.h". While at it replace the hardcoded 20 byGIT_MAX_RAWSZ.
Quoted 8 lines
Signed-off-by Jonathan Nieder <[email protected]>Signed-off-by Stefan Beller <[email protected]>--- object-store.h | 14 ++++++++++++++ replace-object.c | 40 ++++++++++++++++++---------------------- replace-object.h | 9 +++++++++ 3 files changed, 41 insertions(+), 22 deletions(-)create mode 100644
replace-object.h
Throughout this commit, there appears to be an extra space inserted before 'the_repository'. Some are more obvious than others (such as a 'free( the_repository->...)' but others are after the indentation.
Quoted 125 lines
object-store.h +14
-0@@ -93,6 +93,20 @@ struct raw_object_store {struct alternate_object_database *alt_odb_list;struct alternate_object_database **alt_odb_tail; +/* +* Objects that should be substituted by other objects +* (see git-replace(1)). +*/ +struct replace_objects { +/* +* An array of replacements. The array is kept sorted by the original +* sha1. +*/ +struct replace_object **items; + +int alloc, nr; + } replacements; +/* * private data * replace-object.c +18
-22@@ -1,19 +1,11 @@#include "cache.h" +#include "replace-object.h" +#include "object-store.h" #include "sha1-lookup.h" #include "refs.h" +#include "repository.h" #include "commit.h" -/* -* An array of replacements. The array is kept sorted by the original -* sha1. -*/ -static struct replace_object { -unsigned char original[20 ]; -unsigned char replacement[20 ]; -} **replace_object; - -static int replace_object_alloc, replace_object_nr; -static const unsigned char *replace_sha1_access (size_t index,void *table) {struct replace_object **replace = table;@@ -22,7 +14,8 @@ static const unsigned char *replace_sha1_access(size_t index, void *table)static int replace_object_pos (const unsigned char *sha1) { -return sha1_pos (sha1, replace_object, replace_object_nr, +return sha1_pos (sha1, the_repository->objects->replacements.items, + the_repository->objects->replacements.nr, replace_sha1_access); }@@ -35,18 +28,21 @@ static int register_replace_object(struct replace_object *replace,if (ignore_dups)free (replace);else { -free (replace_object[pos]); - replace_object[pos] = replace; +free ( the_repository->objects->replacements.items[pos]); + the_repository->objects->replacements.items[pos] = replace; }return 1 ; } pos = -pos -1 ; -ALLOC_GROW (replace_object, replace_object_nr +1 , replace_object_alloc); - replace_object_nr++; -if (pos < replace_object_nr) -MOVE_ARRAY (replace_object + pos +1 , replace_object + pos, - replace_object_nr - pos -1 ); - replace_object[pos] = replace; +ALLOC_GROW ( the_repository->objects->replacements.items, + the_repository->objects->replacements.nr +1 , + the_repository->objects->replacements.alloc); + the_repository->objects->replacements.nr++; +if (pos < the_repository->objects->replacements.nr) +MOVE_ARRAY ( the_repository->objects->replacements.items + pos +1 , + the_repository->objects->replacements.items + pos, + the_repository->objects->replacements.nr - pos -1 ); + the_repository->objects->replacements.items[pos] = replace;return 0 ; }@@ -84,7 +80,7 @@ static void prepare_replace_object(void)for_each_replace_ref (register_replace_ref,NULL ); replace_object_prepared =1 ; -if (!replace_object_nr) +if (!the_repository->objects->replacements.nr) check_replace_refs =0 ; }@@ -113,7 +109,7 @@ const unsigned char *do_lookup_replace_object(const unsigned char *sha1) pos =replace_object_pos (cur);if (0 <= pos) - cur = replace_object[pos]->replacement; + cur = the_repository->objects->replacements.items[pos]->replacement; }while (0 <= pos);return cur;replace-object.h new file +9
-0@@ -0,0 +1,9 @@ +#ifndef REPLACE_OBJECT_H +#define REPLACE_OBJECT_H + +struct replace_object { +unsigned char original[GIT_MAX_RAWSZ ]; +unsigned char replacement[GIT_MAX_RAWSZ ]; +}; + +#endif /* REPLACE_OBJECT_H */
PATCH 3/19 object-store: move lookup_replace_object to replace-object.h
In reply to Stefan Bellerlookup_replace_object is a low-level function that most users of the object store do not need to use directly.
Move it to replace-object.h to avoid a dependency loop in an upcoming change to its inline definition that will make use of repository.h.
--- builtin/mktag.c | 1 + cache.h | 19 ------------------- object.c | 1 + replace-object.h | 22 ++++++++++++++++++++++ sha1_file.c | 1 + streaming.c | 1 + 6 files changed, 26 insertions(+), 19 deletions(-)
builtin/mktag.c +1 -0
@@ -1,5 +1,6 @@#include "builtin.h" #include "tag.h" +#include "replace-object.h" /* * A signature file has a very simple fixed format: four lines
cache.h +0 -19
@@ -1214,25 +1214,6 @@ static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *return read_sha1_file_extended (sha1, type, size,1 ); } -/* -* This internal function is only declared here for the benefit of -* lookup_replace_object(). Please do not call it directly. -*/ -extern const unsigned char *do_lookup_replace_object (const unsigned char *sha1); - -/* -* If object sha1 should be replaced, return the replacement object's -* name (replaced recursively, if necessary). The return value is -* either sha1 or a pointer to a permanently-allocated value. When -* object replacement is suppressed, always return sha1. -*/ -static inline const unsigned char *lookup_replace_object (const unsigned char *sha1) -{ -if (!check_replace_refs) -return sha1; -return do_lookup_replace_object (sha1); -} -/* Read and unpack a sha1 file into memory, write memory to a sha1 file */ extern int sha1_object_info (const unsigned char *,unsigned long *);extern int hash_sha1_file (const void *buf,unsigned long len,const char *type,unsigned char *sha1);
object.c +1 -0
@@ -1,5 +1,6 @@#include "cache.h" #include "object.h" +#include "replace-object.h" #include "blob.h" #include "tree.h" #include "commit.h"
replace-object.h +22 -0
@@ -1,9 +1,31 @@#ifndef REPLACE_OBJECT_H #define REPLACE_OBJECT_H +#include "cache.h" +#include "repository.h" +struct replace_object {unsigned char original[GIT_MAX_RAWSZ ];unsigned char replacement[GIT_MAX_RAWSZ ]; }; +/* +* This internal function is only declared here for the benefit of +* lookup_replace_object(). Please do not call it directly. +*/ +extern const unsigned char *do_lookup_replace_object (const unsigned char *sha1); + +/* +* If object sha1 should be replaced, return the replacement object's +* name (replaced recursively, if necessary). The return value is +* either sha1 or a pointer to a permanently-allocated value. When +* object replacement is suppressed, always return sha1. +*/ +static inline const unsigned char *lookup_replace_object (const unsigned char *sha1) +{ +if (!check_replace_refs) +return sha1; +return do_lookup_replace_object (sha1); +} +#endif /* REPLACE_OBJECT_H */
sha1_file.c +1 -0
@@ -23,6 +23,7 @@#include "sha1-lookup.h" #include "bulk-checkin.h" #include "repository.h" +#include "replace-object.h" #include "streaming.h" #include "dir.h" #include "list.h"
streaming.c +1 -0
@@ -5,6 +5,7 @@#include "streaming.h" #include "repository.h" #include "object-store.h" +#include "replace-object.h" #include "packfile.h" enum input_source {
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 4/19 replace-object: move replace objects prepared flag to object store
In reply to Stefan BellerRemove another global variable on the way.
--- object-store.h | 2 ++ replace-object.c | 6 ++---- 2 files changed, 4 insertions(+), 4 deletions(-)
object-store.h +2 -0
@@ -105,6 +105,8 @@ struct raw_object_store {struct replace_object **items;int alloc, nr; + + unsigned prepared : 1; } replacements;/*
replace-object.c +2 -4
@@ -73,13 +73,11 @@ static int register_replace_ref(const char *refname,static void prepare_replace_object (void ) { -static int replace_object_prepared; - -if (replace_object_prepared) +if (the_repository->objects->replacements.prepared)return ;for_each_replace_ref (register_replace_ref,NULL ); - replace_object_prepared =1 ; + the_repository->objects->replacements.prepared =1 ;if (!the_repository->objects->replacements.nr) check_replace_refs =0 ; }
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 5/19 replace-object: check_replace_refs is safe in multi repo environment
In reply to Stefan BellerIn e1111cef23 (inline lookup_replace_object() calls, 2011-05-15) a shortcut for checking the object replacement was added by setting check_replace_refs to 0 once the replacements were evaluated to not exist. This works fine in with the assumption of only one repository in existence.
The assumption won't hold true any more when we work on multiple instances of a repository structs (e.g. one struct per submodule), as the first repository to be inspected may have no replacements and would set the global variable. Other repositories would then completely omit their evaluation of replacements.
This reverts back the meaning of the flag check_replace_refs of "Do we need to check with the lookup table?" to "Do we need to read the replacement definition?", adding the bypassing logic to lookup_replace_object after the replacement definition was read. As with the original patch, delay the renaming of the global variable
--- environment.c | 2 +- replace-object.c | 2 -- replace-object.h | 5 ++++- 3 files changed, 5 insertions(+), 4 deletions(-)
environment.c +1 -1
@@ -50,7 +50,7 @@ const char *editor_program;const char *askpass_program;const char *excludes_file;enum auto_crlf auto_crlf =AUTO_CRLF_FALSE ; -int check_replace_refs =1 ; +int check_replace_refs =1 ;/* NEEDSWORK: rename to read_replace_refs */ char *git_replace_ref_base;enum eol core_eol =EOL_UNSET ;int global_conv_flags_eol =CONV_EOL_RNDTRP_WARN ;
replace-object.c +0 -2
@@ -78,8 +78,6 @@ static void prepare_replace_object(void)for_each_replace_ref (register_replace_ref,NULL ); the_repository->objects->replacements.prepared =1 ; -if (!the_repository->objects->replacements.nr) - check_replace_refs =0 ; }/* We allow "recursive" replacement. Only within reason, though */
replace-object.h +4 -1
@@ -3,6 +3,7 @@#include "cache.h" #include "repository.h" +#include "object-store.h" struct replace_object {unsigned char original[GIT_MAX_RAWSZ ];
@@ -23,7 +24,9 @@ extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1);*/ static inline const unsigned char *lookup_replace_object (const unsigned char *sha1) { -if (!check_replace_refs) +if (!check_replace_refs || + (the_repository->objects->replacements.prepared && + the_repository->objects->replacements.nr ==0 ))return sha1;return do_lookup_replace_object (sha1); }
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 6/19 refs: add repository argument to get_main_ref_store
In reply to Stefan BellerAdd a repository argument to allow the get_main_ref_store caller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
# Conflicts:
# t/helper/test-ref-store.c
--- builtin/pack-refs.c | 3 +- refs.c | 67 ++++++++++++++++++++------------------- refs.h | 4 ++- revision.c | 5 +-- t/helper/test-ref-store.c | 3 +- 5 files changed, 44 insertions(+), 38 deletions(-)
builtin/pack-refs.c +2 -1
@@ -1,6 +1,7 @@#include "builtin.h" #include "parse-options.h" #include "refs.h" +#include "repository.h" static char const *const pack_refs_usage[] = {N_ ("git pack-refs [<options>]" ),
@@ -17,5 +18,5 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix) };if (parse_options (argc, argv, prefix, opts, pack_refs_usage,0 ))usage_with_options (pack_refs_usage, opts); -return refs_pack_refs (get_main_ref_store (), flags); +return refs_pack_refs (get_main_ref_store (the_repository), flags); }
refs.c +34 -33
269 lines not on this page · raw message
refs.h +3 -1
@@ -758,7 +758,9 @@ int reflog_expire(const char *refname, const struct object_id *oid,int ref_storage_backend_exists (const char *name); -struct ref_store *get_main_ref_store (void ); +#define get_main_ref_store (r) \ + get_main_ref_store_##r() +struct ref_store *get_main_ref_store_the_repository (void );/* * Return the ref_store instance for the specified submodule. For the * main repository, use submodule==NULL; such a call cannot fail. For
revision.c +3 -2
@@ -6,6 +6,7 @@#include "diff.h" #include "refs.h" #include "revision.h" +#include "repository.h" #include "graph.h" #include "grep.h" #include "reflog-walk.h"
@@ -1289,7 +1290,7 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags) cb.all_revs = revs; cb.all_flags = flags; - cb.refs =get_main_ref_store (); + cb.refs =get_main_ref_store (the_repository);for_each_reflog (handle_one_reflog, &cb);if (!revs->single_worktree)
@@ -2182,7 +2183,7 @@ static int handle_revision_pseudo_opt(const char *submodule,die ("BUG: --single-worktree cannot be used together with submodule" ); refs =get_submodule_ref_store (submodule); }else - refs =get_main_ref_store (); + refs =get_main_ref_store (the_repository);/* * NOTE!
t/helper/test-ref-store.c +2 -1
@@ -2,6 +2,7 @@#include "refs.h" #include "worktree.h" #include "object-store.h" +#include "repository.h" static const char *notnull (const char *arg,const char *name) {
@@ -22,7 +23,7 @@ static const char **get_store(const char **argv, struct ref_store **refs)if (!argv[0 ]) {die ("ref store required" ); }else if (!strcmp (argv[0 ],"main" )) { - *refs =get_main_ref_store (); + *refs =get_main_ref_store (the_repository); }else if (skip_prefix (argv[0 ],"submodule:" , &gitdir)) {struct strbuf sb =STRBUF_INIT ;int ret;
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Re: [PATCH 06/19] refs: add repository argument to get_main_ref_store
Quoted 5 lines · Stefan Beller
Add a repository argument to allow the
get_main_ref_storecaller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other thanthe_repositoryyet.
Quoted 8 lines
As with the previous commits, use a macro to catch callers passing a repository other than
the_repositoryat compile time.Signed-off-by Jonathan Nieder <[email protected]>Signed-off-by Stefan Beller <[email protected]># Conflicts:
#t/helper/test-ref-store.c
Meh.
Re: [PATCH 06/19] refs: add repository argument to get_main_ref_store
Hi Eric,
Quoted 5 lines · Eric Sunshine
Quoted 3 lines
# Conflicts:
#t/helper/test-ref-store.cMeh.
Fixed in a reroll.
PATCH 7/19 refs: add repository argument to for_each_replace_ref
In reply to Stefan BellerAdd a repository argument to allow for_each_replace_ref callers to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- builtin/replace.c | 4 +++- refs.c | 2 +- refs.h | 4 +++- replace-object.c | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-)
builtin/replace.c +3 -1
@@ -14,6 +14,8 @@#include "refs.h" #include "parse-options.h" #include "run-command.h" +#include "object-store.h" +#include "repository.h" #include "tag.h" static const char *const git_replace_usage[] = {
@@ -83,7 +85,7 @@ static int list_replace_refs(const char *pattern, const char *format)"valid formats are 'short', 'medium' and 'long'\n" , format); -for_each_replace_ref (show_reference, (void *)&data); +for_each_replace_ref (the_repository, show_reference, (void *)&data);return 0 ; }
refs.c +1 -1
@@ -1415,7 +1415,7 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,return do_for_each_ref (refs, prefix, fn,0 , flag, cb_data); } -int for_each_replace_ref (each_ref_fn fn,void *cb_data) +int for_each_replace_ref_the_repository (each_ref_fn fn,void *cb_data) { return do_for_each_ref(get_main_ref_store(the_repository), git_replace_ref_base, fn,
refs.h +3 -1
@@ -300,7 +300,9 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,int for_each_tag_ref (each_ref_fn fn,void *cb_data);int for_each_branch_ref (each_ref_fn fn,void *cb_data);int for_each_remote_ref (each_ref_fn fn,void *cb_data); -int for_each_replace_ref (each_ref_fn fn,void *cb_data); +#define for_each_replace_ref (r, fn, cb) \ + for_each_replace_ref_##r(fn, cb) +int for_each_replace_ref_the_repository (each_ref_fn fn,void *cb_data);int for_each_glob_ref (each_ref_fn fn,const char *pattern,void *cb_data);int for_each_glob_ref_in (each_ref_fn fn,const char *pattern,const char *prefix,void *cb_data);
replace-object.c +1 -1
@@ -76,7 +76,7 @@ static void prepare_replace_object(void)if (the_repository->objects->replacements.prepared)return ; -for_each_replace_ref (register_replace_ref,NULL ); +for_each_replace_ref (the_repository, register_replace_ref,NULL ); the_repository->objects->replacements.prepared =1 ; }
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 8/19 replace-object: add repository argument to replace_object_pos
In reply to Stefan BellerAdd a repository argument to allow the replace_object_pos caller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- replace-object.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
replace-object.c +5 -3
@@ -12,7 +12,9 @@ static const unsigned char *replace_sha1_access(size_t index, void *table)return replace[index]->original; } -static int replace_object_pos (const unsigned char *sha1) +#define replace_object_pos (r, s) \ + replace_object_pos_##r(s) +static int replace_object_pos_the_repository (const unsigned char *sha1) { return sha1_pos(sha1, the_repository->objects->replacements.items, the_repository->objects->replacements.nr,
@@ -22,7 +24,7 @@ static int replace_object_pos(const unsigned char *sha1)static int register_replace_object (struct replace_object *replace,int ignore_dups) { -int pos =replace_object_pos (replace->original); +int pos =replace_object_pos (the_repository, replace->original);if (0 <= pos) {if (ignore_dups)
@@ -103,7 +105,7 @@ const unsigned char *do_lookup_replace_object(const unsigned char *sha1)die ("replace depth too high for object %s" ,sha1_to_hex (sha1)); - pos =replace_object_pos (cur); + pos =replace_object_pos (the_repository, cur);if (0 <= pos) cur = the_repository->objects->replacements.items[pos]->replacement; }while (0 <= pos);
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 9/19 replace-object: add repository argument to register_replace_object
In reply to Stefan BellerAdd a repository argument to allow the register_replace_object caller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- replace-object.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
replace-object.c +5 -3
@@ -21,8 +21,10 @@ static int replace_object_pos_the_repository(const unsigned char *sha1) replace_sha1_access); } -static int register_replace_object (struct replace_object *replace, -int ignore_dups) +#define register_replace_object (r, rp, i) \ + register_replace_object_##r(rp, i) +static int register_replace_object_the_repository (struct replace_object *replace, +int ignore_dups) {int pos =replace_object_pos (the_repository, replace->original);
@@ -67,7 +69,7 @@ static int register_replace_ref(const char *refname,hashcpy (repl_obj->replacement, oid->hash);/* Register new object */ -if (register_replace_object (repl_obj,1 )) +if (register_replace_object (the_repository, repl_obj,1 ))die ("duplicate replace ref: %s" , refname);return 0 ;
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 10/19 replace-object: add repository argument to prepare_replace_object
In reply to Stefan BellerAdd a repository argument to allow the prepare_replace_object caller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- replace-object.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
replace-object.c +4 -2
@@ -75,7 +75,9 @@ static int register_replace_ref(const char *refname,return 0 ; } -static void prepare_replace_object (void ) +#define prepare_replace_object (r) \ + prepare_replace_object_##r() +static void prepare_replace_object_the_repository (void ) {if (the_repository->objects->replacements.prepared)return ;
@@ -99,7 +101,7 @@ const unsigned char *do_lookup_replace_object(const unsigned char *sha1)int pos, depth =MAXREPLACEDEPTH ;const unsigned char *cur = sha1; -prepare_replace_object (); +prepare_replace_object (the_repository);/* Try to recursively replace the object */ do {
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 11/19 replace-object: add repository argument to do_lookup_replace_object
In reply to Stefan BellerAdd a repository argument to allow the do_lookup_replace_object caller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- replace-object.c | 2 +- replace-object.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-)
replace-object.c +1 -1
@@ -96,7 +96,7 @@ static void prepare_replace_object_the_repository(void)* permanently-allocated value. This function always respects replace * references, regardless of the value of check_replace_refs. */ -const unsigned char *do_lookup_replace_object (const unsigned char *sha1) +const unsigned char *do_lookup_replace_object_the_repository (const unsigned char *sha1) {int pos, depth =MAXREPLACEDEPTH ;const unsigned char *cur = sha1;
replace-object.h +3 -2
@@ -14,7 +14,8 @@ struct replace_object {* This internal function is only declared here for the benefit of * lookup_replace_object(). Please do not call it directly. */ -extern const unsigned char *do_lookup_replace_object (const unsigned char *sha1); +#define do_lookup_replace_object (r, s) do_lookup_replace_object_##r(s) +extern const unsigned char *do_lookup_replace_object_the_repository (const unsigned char *sha1);/* * If object sha1 should be replaced, return the replacement object's
@@ -28,7 +29,7 @@ static inline const unsigned char *lookup_replace_object(const unsigned char *sh (the_repository->objects->replacements.prepared && the_repository->objects->replacements.nr == 0))return sha1; - return do_lookup_replace_object(sha1); + return do_lookup_replace_object(the_repository, sha1); }#endif /* REPLACE_OBJECT_H */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 12/19 replace-object: add repository argument to lookup_replace_object
In reply to Stefan BellerAdd a repository argument to allow callers of lookup_replace_object to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- builtin/mktag.c | 2 +- object.c | 2 +- replace-object.h | 3 ++- sha1_file.c | 6 +++--- streaming.c | 2 +- 5 files changed, 8 insertions(+), 7 deletions(-)
builtin/mktag.c +1 -1
@@ -25,7 +25,7 @@ static int verify_object(const unsigned char *sha1, const char *expected_type)enum object_type type;unsigned long size;void *buffer =read_sha1_file (sha1, &type, &size); -const unsigned char *repl =lookup_replace_object (sha1); +const unsigned char *repl =lookup_replace_object (the_repository, sha1);if (buffer) {if (type ==type_from_string (expected_type))
object.c +1 -1
@@ -247,7 +247,7 @@ struct object *parse_object(const struct object_id *oid)unsigned long size;enum object_type type;int eaten; -const unsigned char *repl =lookup_replace_object (oid->hash); +const unsigned char *repl =lookup_replace_object (the_repository, oid->hash);void *buffer;struct object *obj;
replace-object.h +2 -1
@@ -23,7 +23,8 @@ extern const unsigned char *do_lookup_replace_object_the_repository(const unsign* either sha1 or a pointer to a permanently-allocated value. When * object replacement is suppressed, always return sha1. */ -static inline const unsigned char *lookup_replace_object (const unsigned char *sha1) +#define lookup_replace_object (r, s) lookup_replace_object_##r(s) +static inline const unsigned char *lookup_replace_object_the_repository (const unsigned char *sha1) { if (!check_replace_refs || (the_repository->objects->replacements.prepared &&
sha1_file.c +3 -3
@@ -1236,7 +1236,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,struct pack_entry e;int rtype;const unsigned char *real = (flags &OBJECT_INFO_LOOKUP_REPLACE ) ? -lookup_replace_object (sha1) : +lookup_replace_object (the_repository, sha1) : sha1;int already_retried =0 ;
@@ -1374,8 +1374,8 @@ void *read_sha1_file_extended(const unsigned char *sha1,const struct packed_git *p;const char *path;struct stat st; -const unsigned char *repl = lookup_replace ?lookup_replace_object (sha1) - : sha1; +const unsigned char *repl = lookup_replace ? +lookup_replace_object (the_repository, sha1) : sha1; errno =0 ; data =read_object (repl, type, size);
streaming.c +1 -1
@@ -140,7 +140,7 @@ struct git_istream *open_istream(const unsigned char *sha1, {struct git_istream *st;struct object_info oi =OBJECT_INFO_INIT ; -const unsigned char *real =lookup_replace_object (sha1); +const unsigned char *real =lookup_replace_object (the_repository, sha1);enum input_source src =istream_source (real, type, &oi);if (src <0 )
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 13/19 refs: store the main ref store inside the repository struct
In reply to Stefan Beller--- refs.c | 13 +++++-------- refs.h | 4 +--- repository.h | 5 +++++ 3 files changed, 11 insertions(+), 11 deletions(-)
refs.c +5 -8
@@ -1608,9 +1608,6 @@ static struct ref_store_hash_entry *alloc_ref_store_hash_entry(return entry; } -/* A pointer to the ref_store for the main repository: */ -static struct ref_store *main_ref_store; -/* A hashmap of ref_stores, stored by submodule name: */ static struct hashmap submodule_ref_stores;
@@ -1652,13 +1649,13 @@ static struct ref_store *ref_store_init(const char *gitdir,return refs; } -struct ref_store *get_main_ref_store_the_repository (void ) +struct ref_store *get_main_ref_store (struct repository *r) { -if (main_ref_store) -return main_ref_store; +if (r->main_ref_store) +return r->main_ref_store; - main_ref_store =ref_store_init (get_git_dir (),REF_STORE_ALL_CAPS ); -return main_ref_store; + r->main_ref_store =ref_store_init (r->gitdir,REF_STORE_ALL_CAPS ); +return r->main_ref_store; }/*
refs.h +1 -3
@@ -760,9 +760,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,int ref_storage_backend_exists (const char *name); -#define get_main_ref_store (r) \ - get_main_ref_store_##r() -struct ref_store *get_main_ref_store_the_repository (void ); +struct ref_store *get_main_ref_store (struct repository *r);/* * Return the ref_store instance for the specified submodule. For the * main repository, use submodule==NULL; such a call cannot fail. For
repository.h +5 -0
@@ -26,6 +26,11 @@ struct repository {*/ struct raw_object_store *objects; +/* +* The store in which the refs are hold. +*/ +struct ref_store *main_ref_store; +/* * Path to the repository's graft file. * Cannot be NULL after initialization.
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Re: [PATCH 13/19] refs: store the main ref store inside the repository struct
repository.h struct repository { · line 28
Quoted 5 lines · Stefan Beller
repository.h +3
-0@@ -26,6 +26,11 @@ struct repository { +/* +* The store in which the refs are hold. +*/
s/hold/held/
Also, this comment is short enough to fit on one line: /* ... */
Quoted 1 line
repository.h +1
-0+struct ref_store *main_ref_store;
PATCH 14/19 refs: allow for_each_replace_ref to handle arbitrary repositories
In reply to Stefan Beller--- refs.c | 4 ++-- refs.h | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-)
refs.c +2 -2
@@ -1415,9 +1415,9 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,return do_for_each_ref (refs, prefix, fn,0 , flag, cb_data); } -int for_each_replace_ref_the_repository (each_ref_fn fn,void *cb_data) +int for_each_replace_ref (struct repository *r,each_ref_fn fn,void *cb_data) { -return do_for_each_ref (get_main_ref_store (the_repository), +return do_for_each_ref (get_main_ref_store (r), git_replace_ref_base, fn,strlen (git_replace_ref_base),DO_FOR_EACH_INCLUDE_BROKEN , cb_data);
refs.h +1 -3
@@ -300,9 +300,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,int for_each_tag_ref (each_ref_fn fn,void *cb_data);int for_each_branch_ref (each_ref_fn fn,void *cb_data);int for_each_remote_ref (each_ref_fn fn,void *cb_data); -#define for_each_replace_ref (r, fn, cb) \ - for_each_replace_ref_##r(fn, cb) -int for_each_replace_ref_the_repository (each_ref_fn fn,void *cb_data); +int for_each_replace_ref (struct repository *r,each_ref_fn fn,void *cb_data);int for_each_glob_ref (each_ref_fn fn,const char *pattern,void *cb_data);int for_each_glob_ref_in (each_ref_fn fn,const char *pattern,const char *prefix,void *cb_data);
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 15/19 replace-object: allow replace_object_pos to handle arbitrary repositories
In reply to Stefan Beller--- replace-object.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
replace-object.c +3 -5
@@ -12,12 +12,10 @@ static const unsigned char *replace_sha1_access(size_t index, void *table)return replace[index]->original; } -#define replace_object_pos (r, s) \ - replace_object_pos_##r(s) -static int replace_object_pos_the_repository (const unsigned char *sha1) +static int replace_object_pos (struct repository *r,const unsigned char *sha1) { -return sha1_pos (sha1, the_repository->objects->replacements.items, - the_repository->objects->replacements.nr, +return sha1_pos (sha1, r->objects->replacements.items, + r->objects->replacements.nr, replace_sha1_access); }
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 16/19 replace-object: allow register_replace_object to handle arbitrary repositories
In reply to Stefan Beller--- replace-object.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)
replace-object.c +15 -16
@@ -19,32 +19,31 @@ static int replace_object_pos(struct repository *r, const unsigned char *sha1) replace_sha1_access); } -#define register_replace_object (r, rp, i) \ - register_replace_object_##r(rp, i) -static int register_replace_object_the_repository (struct replace_object *replace, -int ignore_dups) +static int register_replace_object (struct repository *r, +struct replace_object *replace, +int ignore_dups) { -int pos =replace_object_pos (the_repository, replace->original); +int pos =replace_object_pos (r, replace->original);if (0 <= pos) {if (ignore_dups)free (replace);else { -free ( the_repository->objects->replacements.items[pos]); - the_repository->objects->replacements.items[pos] = replace; +free (r->objects->replacements.items[pos]); + r->objects->replacements.items[pos] = replace; }return 1 ; } pos = -pos -1 ; -ALLOC_GROW ( the_repository->objects->replacements.items, - the_repository->objects->replacements.nr +1 , - the_repository->objects->replacements.alloc); - the_repository->objects->replacements.nr++; -if (pos < the_repository->objects->replacements.nr) -MOVE_ARRAY ( the_repository->objects->replacements.items + pos +1 , - the_repository->objects->replacements.items + pos, - the_repository->objects->replacements.nr - pos -1 ); - the_repository->objects->replacements.items[pos] = replace; +ALLOC_GROW (r->objects->replacements.items, + r->objects->replacements.nr +1 , + r->objects->replacements.alloc); + r->objects->replacements.nr++; +if (pos < r->objects->replacements.nr) +MOVE_ARRAY (r->objects->replacements.items + pos +1 , + r->objects->replacements.items + pos, + r->objects->replacements.nr - pos -1 ); + r->objects->replacements.items[pos] = replace;return 0 ; }
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 17/19 replace-object: allow prepare_replace_object to handle arbitrary repositories
In reply to Stefan Beller--- replace-object.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
replace-object.c +4 -6
@@ -72,15 +72,13 @@ static int register_replace_ref(const char *refname,return 0 ; } -#define prepare_replace_object (r) \ - prepare_replace_object_##r() -static void prepare_replace_object_the_repository (void ) +static void prepare_replace_object (struct repository *r) { -if (the_repository->objects->replacements.prepared) +if (r->objects->replacements.prepared)return ; -for_each_replace_ref (the_repository, register_replace_ref,NULL ); - the_repository->objects->replacements.prepared =1 ; +for_each_replace_ref (r, register_replace_ref, r); + r->objects->replacements.prepared =1 ; }/* We allow "recursive" replacement. Only within reason, though */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 18/19 replace-object: allow do_lookup_replace_object to handle arbitrary repositories
In reply to Stefan Beller--- replace-object.c | 9 +++++---- replace-object.h | 3 +-- 2 files changed, 6 insertions(+), 6 deletions(-)
replace-object.c +5 -4
@@ -91,12 +91,13 @@ static void prepare_replace_object(struct repository *r)* permanently-allocated value. This function always respects replace * references, regardless of the value of check_replace_refs. */ -const unsigned char *do_lookup_replace_object_the_repository (const unsigned char *sha1) +const unsigned char *do_lookup_replace_object (struct repository *r, +const unsigned char *sha1) {int pos, depth =MAXREPLACEDEPTH ;const unsigned char *cur = sha1; -prepare_replace_object (the_repository); +prepare_replace_object (r);/* Try to recursively replace the object */ do {
@@ -104,9 +105,9 @@ const unsigned char *do_lookup_replace_object_the_repository(const unsigned chardie ("replace depth too high for object %s" ,sha1_to_hex (sha1)); - pos =replace_object_pos (the_repository, cur); + pos =replace_object_pos (r, cur);if (0 <= pos) - cur = the_repository->objects->replacements.items[pos]->replacement; + cur = r->objects->replacements.items[pos]->replacement; }while (0 <= pos);return cur;
replace-object.h +1 -2
@@ -14,8 +14,7 @@ struct replace_object {* This internal function is only declared here for the benefit of * lookup_replace_object(). Please do not call it directly. */ -#define do_lookup_replace_object (r, s) do_lookup_replace_object_##r(s) -extern const unsigned char *do_lookup_replace_object_the_repository (const unsigned char *sha1); +extern const unsigned char *do_lookup_replace_object (struct repository *r,const unsigned char *sha1);/* * If object sha1 should be replaced, return the replacement object's
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 19/19 replace-object: allow lookup_replace_object to handle arbitrary repositories
In reply to Stefan Beller--- replace-object.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
replace-object.h +5 -5
@@ -22,14 +22,14 @@ extern const unsigned char *do_lookup_replace_object(struct repository *r, const* either sha1 or a pointer to a permanently-allocated value. When * object replacement is suppressed, always return sha1. */ -#define lookup_replace_object (r, s) lookup_replace_object_##r(s) -static inline const unsigned char *lookup_replace_object_the_repository (const unsigned char *sha1) +static inline const unsigned char *lookup_replace_object (struct repository *r, +const unsigned char *sha1) {if (!check_replace_refs || - (the_repository->objects->replacements.prepared && - the_repository->objects->replacements.nr ==0 )) + (r->objects->replacements.prepared && + r->objects->replacements.nr ==0 ))return sha1; -return do_lookup_replace_object (the_repository, sha1); +return do_lookup_replace_object (r, sha1); }#endif /* REPLACE_OBJECT_H */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Am 07.04.2018 um 01:21 schrieb Stefan Beller:
Quoted 3 lines
This applies on top of
464416a2eaadf84d2bfdf795007863d03b222b7c(sb/packfiles-in-repository).
It is also available at https://github.com/stefanbeller/git/tree/object-store-3
This series conflicts with 1731a1e239 (replace_object: convert struct replace_object to object_id) and b383a13cc0 (Convert lookup_replace_object to struct object_id), which are in next.
Quoted 2 lines
This series will bring the replacement mechanism (git replace) into the object store.
Good idea.
Quoted 14 lines
$ git diff 464416a2eaadf84d2bfdf795007863d03b222b7c..HEAD -- object-store.h repository.hobject-store.h +5
-0@@ -93,6 +93,22 @@ struct raw_object_store {struct alternate_object_database *alt_odb_list;struct alternate_object_database **alt_odb_tail; +/* +* Objects that should be substituted by other objects +* (see git-replace(1)). +*/ +struct replace_objects {
object-store.h struct raw_object_store { · line 110
Quoted 10 lines
object-store.h +10
-0+/* +* An array of replacements. The array is kept sorted by the original +* sha1. +*/ +struct replace_object **items; + +int alloc, nr; + + unsigned prepared : 1; + } replacements;
An oidmap would be a better fit -- lookups should be quicker and memory consumption not much worse. I meant to submit something like this eventually after Brian's series lands:
-- >8 --
Subject: [PATCH] replace_object: use oidmap
Load the replace objects into an oidmap to allow for easy lookups in constant time.
---
This is on top of next.
replace_object.c | 76 ++++++++++-------------------------------------- 1 file changed, 16 insertions(+), 60 deletions(-)
replace_object.c +16 -60
@@ -1,54 +1,14 @@#include "cache.h" -#include "sha1-lookup.h" +#include "oidmap.h" #include "refs.h" #include "commit.h" -/* -* An array of replacements. The array is kept sorted by the original -* sha1. -*/ -static struct replace_object { -struct object_id original; +struct replace_object { +struct oidmap_entry original;struct object_id replacement; -} **replace_object; - -static int replace_object_alloc, replace_object_nr; +}; -static const unsigned char *replace_sha1_access (size_t index,void *table) -{ -struct replace_object **replace = table; -return replace[index]->original.hash; -} - -static int replace_object_pos (const unsigned char *sha1) -{ -return sha1_pos (sha1, replace_object, replace_object_nr, - replace_sha1_access); -} - -static int register_replace_object (struct replace_object *replace, -int ignore_dups) -{ -int pos =replace_object_pos (replace->original.hash); - -if (0 <= pos) { -if (ignore_dups) -free (replace); -else { -free (replace_object[pos]); - replace_object[pos] = replace; - } -return 1 ; - } - pos = -pos -1 ; -ALLOC_GROW (replace_object, replace_object_nr +1 , replace_object_alloc); - replace_object_nr++; -if (pos < replace_object_nr) -MOVE_ARRAY (replace_object + pos +1 , replace_object + pos, - replace_object_nr - pos -1 ); - replace_object[pos] = replace; -return 0 ; -} +static struct oidmap replace_map =OIDMAP_INIT ; static int register_replace_ref(const char *refname, const struct object_id *oid,
@@ -59,7 +19,7 @@ static int register_replace_ref(const char *refname,const char *hash = slash ? slash +1 : refname;struct replace_object *repl_obj =xmalloc (sizeof (*repl_obj)); -if (get_oid_hex (hash, &repl_obj->original)) { +if (get_oid_hex (hash, &repl_obj->original.oid)) {free (repl_obj);warning ("bad replace ref name: %s" , refname);return 0 ;
@@ -69,7 +29,7 @@ static int register_replace_ref(const char *refname,oidcpy (&repl_obj->replacement, oid);/* Register new object */ -if (register_replace_object (repl_obj,1 )) +if (oidmap_put (&replace_map, repl_obj))die ("duplicate replace ref: %s" , refname);return 0 ;
@@ -84,7 +44,7 @@ static void prepare_replace_object(void)for_each_replace_ref (register_replace_ref,NULL ); replace_object_prepared =1 ; -if (!replace_object_nr) +if (!replace_map.map.tablesize) check_replace_refs =0 ; }
@@ -100,21 +60,17 @@ static void prepare_replace_object(void)*/ const struct object_id *do_lookup_replace_object (const struct object_id *oid) { -int pos, depth =MAXREPLACEDEPTH ; +int depth =MAXREPLACEDEPTH ;const struct object_id *cur = oid;prepare_replace_object ();/* Try to recursively replace the object */ -do { -if (--depth <0 ) -die ("replace depth too high for object %s" , -oid_to_hex (oid)); - - pos =replace_object_pos (cur->hash); -if (0 <= pos) - cur = &replace_object[pos]->replacement; - }while (0 <= pos); - -return cur; +while (depth-- >0 ) { +struct replace_object *repl_obj =oidmap_get (&replace_map, cur); +if (!repl_obj) +return cur; + cur = &repl_obj->replacement; + } +die ("replace depth too high for object %s" ,oid_to_hex (oid)); }
Signature · 1 line
2.17.0
Hi René,
Quoted 8 lines · René Scharfe
Am 07.04.2018 um 01:21 schrieb Stefan Beller:
Quoted 3 lines
This applies on top of
464416a2eaadf84d2bfdf795007863d03b222b7c(sb/packfiles-in-repository).
It is also available at https://github.com/stefanbeller/git/tree/object-store-3This series conflicts with
1731a1e239(replace_object: convert structreplace_objecttoobject_id) andb383a13cc0(Convertlookup_replace_objectto structobject_id), which are in next.
ok, I'll investigate. Maybe the reroll will be on top of a merge of brians series and sb/packfiles-in-repository, both of which go to master quickly now that 2.17 is out?
Quoted 5 lines
Quoted 3 lines
This series will bring the replacement mechanism (git replace) into the object store.
Good idea.
heh, thanks. I think this is the smallest unit in which the community is willing to digest it. (The largest coherent patch series was the demo of "everything in struct repo"[1])
[1] https://public-inbox.org/git/[email protected]/
Quoted 33 lines
Quoted 15 lines
$ git diff 464416a2eaadf84d2bfdf795007863d03b222b7c..HEAD -- object-store.h repository.hobject-store.h +5
-0@@ -93,6 +93,22 @@ struct raw_object_store {struct alternate_object_database *alt_odb_list;struct alternate_object_database **alt_odb_tail; +/* +* Objects that should be substituted by other objects +* (see git-replace(1)). +*/ +struct replace_objects {object-store.h struct raw_object_store { · line 110
Quoted 10 lines
object-store.h +10
-0+/* +* An array of replacements. The array is kept sorted by the original +* sha1. +*/ +struct replace_object **items; + +int alloc, nr; + + unsigned prepared : 1; + } replacements;An oidmap would be a better fit -- lookups should be quicker and memory consumption not much worse. I meant to submit something like this eventually after Brian's series lands:
-- >8 --
Subject: [PATCH]replace_object: use oidmap
Quoted 6 lines
Load the replace objects into an oidmap to allow for easy lookups in constant time.
Signed-off-by Rene Scharfe <[email protected]>---
This is on top of next.
So this is on top of brians series (modulo other next-ish stuff) which this series ought to merge with?
The patch looks good -- just from the diff stat alone.
Thanks,
Stefan
Quoted 127 lines
replace_object.c | 76 ++++++++++-------------------------------------- 1 file changed, 16 insertions(+), 60 deletions(-)replace_object.c +16
-61@@ -1,54 +1,14 @@#include "cache.h" -#include "sha1-lookup.h" +#include "oidmap.h" #include "refs.h" #include "commit.h" -/* -* An array of replacements. The array is kept sorted by the original -* sha1. -*/ -static struct replace_object { -struct object_id original; +struct replace_object { +struct oidmap_entry original;struct object_id replacement; -} **replace_object; - -static int replace_object_alloc, replace_object_nr; +}; -static const unsigned char *replace_sha1_access (size_t index,void *table) -{ -struct replace_object **replace = table; -return replace[index]->original.hash; -} - -static int replace_object_pos (const unsigned char *sha1) -{ -return sha1_pos (sha1, replace_object, replace_object_nr, - replace_sha1_access); -} - -static int register_replace_object (struct replace_object *replace, -int ignore_dups) -{ -int pos =replace_object_pos (replace->original.hash); - -if (0 <= pos) { -if (ignore_dups) -free (replace); -else { -free (replace_object[pos]); - replace_object[pos] = replace; - } -return 1 ; - } - pos = -pos -1 ; -ALLOC_GROW (replace_object, replace_object_nr +1 , replace_object_alloc); - replace_object_nr++; -if (pos < replace_object_nr) -MOVE_ARRAY (replace_object + pos +1 , replace_object + pos, - replace_object_nr - pos -1 ); - replace_object[pos] = replace; -return 0 ; -} +static struct oidmap replace_map =OIDMAP_INIT ; static int register_replace_ref(const char *refname, const struct object_id *oid,@@ -59,7 +19,7 @@ static int register_replace_ref(const char *refname,const char *hash = slash ? slash +1 : refname;struct replace_object *repl_obj =xmalloc (sizeof (*repl_obj)); -if (get_oid_hex (hash, &repl_obj->original)) { +if (get_oid_hex (hash, &repl_obj->original.oid)) {free (repl_obj);warning ("bad replace ref name: %s" , refname);return 0 ;@@ -69,7 +29,7 @@ static int register_replace_ref(const char *refname,oidcpy (&repl_obj->replacement, oid);/* Register new object */ -if (register_replace_object (repl_obj,1 )) +if (oidmap_put (&replace_map, repl_obj))die ("duplicate replace ref: %s" , refname);return 0 ;@@ -84,7 +44,7 @@ static void prepare_replace_object(void)for_each_replace_ref (register_replace_ref,NULL ); replace_object_prepared =1 ; -if (!replace_object_nr) +if (!replace_map.map.tablesize) check_replace_refs =0 ; }@@ -100,21 +60,17 @@ static void prepare_replace_object(void)*/ const struct object_id *do_lookup_replace_object (const struct object_id *oid) { -int pos, depth =MAXREPLACEDEPTH ; +int depth =MAXREPLACEDEPTH ;const struct object_id *cur = oid;prepare_replace_object ();/* Try to recursively replace the object */ -do { -if (--depth <0 ) -die ("replace depth too high for object %s" , -oid_to_hex (oid)); - - pos =replace_object_pos (cur->hash); -if (0 <= pos) - cur = &replace_object[pos]->replacement; - }while (0 <= pos); - -return cur; +while (depth-- >0 ) { +struct replace_object *repl_obj =oidmap_get (&replace_map, cur); +if (!repl_obj) +return cur; + cur = &repl_obj->replacement; + } +die ("replace depth too high for object %s" ,oid_to_hex (oid)); } --2.17.0
On Sat, Apr 7, 2018 at 1:21 AM, Stefan Beller <[email protected]> wrote: *
repository.h struct repository { · line 33
Quoted 13 lines
repository.h +5
-0@@ -26,6 +26,11 @@ struct repository {*/ struct raw_object_store *objects; +/* +* The store in which the refs are hold. +*/ +struct ref_store *main_ref_store; +
We probably should drop the main_ prefix here because this could also be a submodule ref store (when the repository is about a submodule). worktree ref store is a different story and I don't know how to best present it here yet (I'm still thinking a separate struct repository). But we can worry about that when struct repository supports multiple worktree.
Signature · 1 line
Duy
Quoted 9 lines · Duy Nguyen
On Sat, Apr 7, 2018 at 1:21 AM, Stefan Beller <[email protected]> wrote: *
Quoted 8 lines
repository.h +0
-0@@ -26,6 +26,11 @@ struct repository {*/ struct raw_object_store *objects;
Quoted 8 lines
repository.h struct repository { · line 33
Quoted 5 lines
repository.h +5
-0+/* +* The store in which the refs are hold. +*/ +struct ref_store *main_ref_store; +We probably should drop the
main_prefix here because this could also be a submodule ref store (when the repository is about a submodule).
it's still the main ref store of the submodule, then. :) But yes, I agree we should rename it. Now or eventually later?
Quoted 2 lines
worktree ref store is a different story and I don't know how to best present it here yet (I'm still thinking a separate struct repository).
I imagine that we'd rather want to have arrays of config/worktree path/refs when needed and the worktree is just an index into all of these things?
Quoted 2 lines
But we can worry about that when struct repository supports multiple worktree.
ok. Thanks for bringing this to my attention.
Thanks,
Stefan
Quoted 7 lines · Stefan Beller
This applies on top of
464416a2eaadf84d2bfdf795007863d03b222b7c(sb/packfiles-in-repository).
It is also available at https://github.com/stefanbeller/git/tree/object-store-3This series will bring the replacement mechanism (git replace) into the object store.
Quoted 7 lines
The first patches are cleaning up a bit, and patches 6-19 are converting one function at a time using the tick-tock pattern with the #define trick. See
cfc62fc98c(sha1_file: add repository argument tolink_alt_odb_entry, 2018-03-23) for explanation.Thanks,
Stefan
I looked through these patches and only found one set of whitespace errors. Compiles and tests fine on my machine.
A message that is not in the archive (sent off-list or not yet received): nycvar.QRO.7.76.6.1804091038430.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz
+cc list
Quoted 8 lines · Johannes Schindelin
Hi Stefan,
Quoted 2 lines · Stefan Beller
See
cfc62fc98c(sha1_file: add repository argument tolink_alt_odb_entry, 2018-03-23) for explanation."See ... for explanation." ... are you going full Russian on us? ;-)
That commit is supposed to merge to master now, so we don't lose it. I just dislike to repeat myself, and there we explained that trick already. As commit messages don't link well together in email clients, I will repeat the important part.
Thanks,
Stefan
Quoted 3 lines
Ciao,
Dscho
v2 cover object-store refactoring 3 (replace objects, main ref store)
In reply to Stefan Bellerv2:
This applies on top of a merge of
origin/bc/object-id and origin/sb/packfiles-in-repository, both of which are pending merge to master. It is also available at
https://github.com/stefanbeller/git/tree/object-store-3
* removed whitespaces as noted by Stolee
* incorporated Renes patch as the first patch of this series (It may go independently if this series takes too long)
* Adressed Erics concern regarding sloppy commit messages (removed #Conflict markers), typo in comment
* I did not drop the main_ from the ref store, yet, as asked by Duy.
Thanks,
Stefan
v1:
This applies on top of 464416a2eaadf84d2bfdf795007863d03b222b7c (sb/packfiles-in-repository).
It is also available at https://github.com/stefanbeller/git/tree/object-store-3
This series will bring the replacement mechanism (git replace) into the object store.
The first patches are cleaning up a bit, and patches 6-19 are converting one function at a time using the tick-tock pattern with the #define trick. See cfc62fc98c (sha1_file: add repository argument to link_alt_odb_entry, 2018-03-23) for explanation.
Thanks,
Stefan
René Scharfe (1):
replace_object: use oidmap
Stefan Beller (15):
replace_object.c: rename to use dash in file name
replace-object: move replace_map to object store
object-store: move lookup_replace_object to replace-object.h
replace-object: eliminate replace objects prepared flag
replace-object: check_replace_refs is safe in multi repo environment
refs: add repository argument to get_main_ref_store
refs: add repository argument to for_each_replace_ref
replace-object: add repository argument to prepare_replace_object
replace-object: add repository argument to do_lookup_replace_object
replace-object: add repository argument to lookup_replace_object
refs: store the main ref store inside the repository struct
refs: allow for_each_replace_ref to handle arbitrary repositories
replace-object: allow prepare_replace_object to handle arbitrary
repositories
replace-object: allow do_lookup_replace_object to handle arbitrary
repositories
replace-object: allow lookup_replace_object to handle arbitrary
repositories
Makefile | 2 +- builtin/mktag.c | 3 +- builtin/pack-refs.c | 3 +- builtin/replace.c | 4 +- cache.h | 19 ------ environment.c | 2 +- object-store.h | 8 +++ object.c | 3 +- refs.c | 80 +++++++++++++------------ refs.h | 4 +- replace-object.c | 73 +++++++++++++++++++++++ replace-object.h | 36 ++++++++++++ replace_object.c | 120 -------------------------------------- repository.h | 3 + revision.c | 5 +- sha1_file.c | 7 ++- streaming.c | 3 +- t/helper/test-ref-store.c | 3 +- 18 files changed, 183 insertions(+), 195 deletions(-) create mode 100644 replace-object.c create mode 100644 replace-object.h delete mode 100644 replace_object.c
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v2 1/16 replace_object: use oidmap
From: René Scharfe <[email protected]>
Load the replace objects into an oidmap to allow for easy lookups in constant time.
--- replace_object.c | 76 ++++++++++-------------------------------------- 1 file changed, 16 insertions(+), 60 deletions(-)
replace_object.c +16 -60
@@ -1,54 +1,14 @@#include "cache.h" -#include "sha1-lookup.h" +#include "oidmap.h" #include "refs.h" #include "commit.h" -/* -* An array of replacements. The array is kept sorted by the original -* sha1. -*/ -static struct replace_object { -struct object_id original; +struct replace_object { +struct oidmap_entry original;struct object_id replacement; -} **replace_object; - -static int replace_object_alloc, replace_object_nr; +}; -static const unsigned char *replace_sha1_access (size_t index,void *table) -{ -struct replace_object **replace = table; -return replace[index]->original.hash; -} - -static int replace_object_pos (const unsigned char *sha1) -{ -return sha1_pos (sha1, replace_object, replace_object_nr, - replace_sha1_access); -} - -static int register_replace_object (struct replace_object *replace, -int ignore_dups) -{ -int pos =replace_object_pos (replace->original.hash); - -if (0 <= pos) { -if (ignore_dups) -free (replace); -else { -free (replace_object[pos]); - replace_object[pos] = replace; - } -return 1 ; - } - pos = -pos -1 ; -ALLOC_GROW (replace_object, replace_object_nr +1 , replace_object_alloc); - replace_object_nr++; -if (pos < replace_object_nr) -MOVE_ARRAY (replace_object + pos +1 , replace_object + pos, - replace_object_nr - pos -1 ); - replace_object[pos] = replace; -return 0 ; -} +static struct oidmap replace_map =OIDMAP_INIT ; static int register_replace_ref(const char *refname, const struct object_id *oid,
@@ -59,7 +19,7 @@ static int register_replace_ref(const char *refname,const char *hash = slash ? slash +1 : refname;struct replace_object *repl_obj =xmalloc (sizeof (*repl_obj)); -if (get_oid_hex (hash, &repl_obj->original)) { +if (get_oid_hex (hash, &repl_obj->original.oid)) {free (repl_obj);warning ("bad replace ref name: %s" , refname);return 0 ;
@@ -69,7 +29,7 @@ static int register_replace_ref(const char *refname,oidcpy (&repl_obj->replacement, oid);/* Register new object */ -if (register_replace_object (repl_obj,1 )) +if (oidmap_put (&replace_map, repl_obj))die ("duplicate replace ref: %s" , refname);return 0 ;
@@ -84,7 +44,7 @@ static void prepare_replace_object(void)for_each_replace_ref (register_replace_ref,NULL ); replace_object_prepared =1 ; -if (!replace_object_nr) +if (!replace_map.map.tablesize) check_replace_refs =0 ; }
@@ -100,21 +60,17 @@ static void prepare_replace_object(void)*/ const struct object_id *do_lookup_replace_object (const struct object_id *oid) { -int pos, depth =MAXREPLACEDEPTH ; +int depth =MAXREPLACEDEPTH ;const struct object_id *cur = oid;prepare_replace_object ();/* Try to recursively replace the object */ -do { -if (--depth <0 ) -die ("replace depth too high for object %s" , -oid_to_hex (oid)); - - pos =replace_object_pos (cur->hash); -if (0 <= pos) - cur = &replace_object[pos]->replacement; - }while (0 <= pos); - -return cur; +while (depth-- >0 ) { +struct replace_object *repl_obj =oidmap_get (&replace_map, cur); +if (!repl_obj) +return cur; + cur = &repl_obj->replacement; + } +die ("replace depth too high for object %s" ,oid_to_hex (oid)); }
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Re: [PATCH 01/16] replace_object: use oidmap
replace_object.c line 9
Quoted 9 lines · Stefan Beller
replace_object.c +3
-5-static struct replace_object { -struct object_id original; +struct replace_object { +struct oidmap_entry original;struct object_id replacement; -} **replace_object; - -static int replace_object_alloc, replace_object_nr; +};
The oidmap key can also serve as the original side, which is good ;-)
replace_object.c static void prepare_replace_object(void) · line 48
Quoted 7 lines
replace_object.c +1
-1@@ -84,7 +44,7 @@ static void prepare_replace_object(void)for_each_replace_ref (register_replace_ref,NULL ); replace_object_prepared =1 ; -if (!replace_object_nr) +if (!replace_map.map.tablesize) check_replace_refs =0 ;
The original checks _nr (i.e. how many do we actually have) as opposed to _alloc; on the other hand, .tablesize is not about the number of contents in the table.
But before or after the patch, what the code really wants to check is if the table is _initialized_, so from that point of view, checking .tablesize is good enough.
So, I guess this step is quite right ;-)
PATCH v2 2/16 replace_object.c: rename to use dash in file name
In reply to Stefan BellerThis is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names.
Noticed while adding a header corresponding to this file.
--- Makefile | 2 +- replace_object.c => replace-object.c | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename replace_object.c => replace-object.c (100%)
Makefile +1 -1
@@ -871,7 +871,7 @@ LIB_OBJS += refs/packed-backend.o LIB_OBJS += refs/ref-cache.o LIB_OBJS += ref-filter.o LIB_OBJS += remote.o -LIB_OBJS += replace_object.o +LIB_OBJS += replace-object.o LIB_OBJS += repository.o LIB_OBJS += rerere.o LIB_OBJS += resolve-undo.o
replace-object.c renamed from replace_object.c +0 -0
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Re: [PATCH 02/16] replace_object.c: rename to use dash in file name
Quoted 9 lines · Stefan Beller
This is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names.
Noticed while adding a header corresponding to this file.
Signed-off-by Jonathan Nieder <[email protected]>Signed-off-by Stefan Beller <[email protected]>---
Hmph, is this authored by Jonathan?
There are sha1_{file,name}.c, exec_cmd.[ch], and unicode_width.h remaining, though ;-)
Quoted 22 lines
Makefile | 2 +- replace_object.c => replace-object.c | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename replace_object.c => replace-object.c (100%)Makefile +1
-1@@ -871,7 +871,7 @@ LIB_OBJS += refs/packed-backend.o LIB_OBJS += refs/ref-cache.o LIB_OBJS += ref-filter.o LIB_OBJS += remote.o -LIB_OBJS += replace_object.o +LIB_OBJS += replace-object.o LIB_OBJS += repository.o LIB_OBJS += rerere.o LIB_OBJS += resolve-undo.oreplace-object.c renamed from replace_object.c +0
-0
Re: [PATCH 02/16] replace_object.c: rename to use dash in file name
Hi Junio,
Quoted 6 lines · Junio C Hamano
Quoted 4 lines · Stefan Beller
This is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names.
Quoted 7 lines
Quoted 5 lines
Noticed while adding a header corresponding to this file.
Signed-off-by Jonathan Nieder <[email protected]>Signed-off-by Stefan Beller <[email protected]>---
Hmph, is this authored by Jonathan?
This one (as well as other patches that are also signed off by Jonathan) was cherry-picked from the long series[1], which was done partially in a pair programming session or by passing patches back and forth.
Most of the mechanical changes were done by one author and we just added the others sign off to have the whole series look like pair programming.
This patch is [2], as-is, so I did not mess up the original authorship.
We decided to not use another trailer, such as co-authored-by or such as we'd have to sign off anyway.
[1] https://public-inbox.org/git/[email protected]/
[2] https://public-inbox.org/git/[email protected]/
Quoted 3 lines
There are sha1_{file,name}.c, exec_cmd.[ch], and
unicode_width.hremaining, though ;-)
Noted.
Thanks,
Stefan
cover Rename files to use dashes instead of underscores
In reply to Junio C HamanoThis is the followup for
https://public-inbox.org/git/[email protected]/
We have no files left with underscores in their names.
Thanks,
Stefan
Stefan Beller (6):
write_or_die.c: rename to use dashes in file name unicode_width.h: rename to use dash in file name
exec_cmd: rename to use dash in file name sha1_name.c: rename to use dash in file name sha1_file.c: rename to use dash in file name replace_object.c: rename to use dash in file name
Documentation/technical/api-object-access.txt | 2 +- Makefile | 14 +++++++------- attr.c | 2 +- builtin/add.c | 2 +- builtin/am.c | 2 +- builtin/describe.c | 2 +- builtin/difftool.c | 2 +- builtin/hash-object.c | 2 +- builtin/help.c | 2 +- builtin/index-pack.c | 4 ++-- builtin/init-db.c | 2 +- builtin/merge-tree.c | 2 +- builtin/notes.c | 2 +- builtin/pull.c | 2 +- builtin/receive-pack.c | 2 +- common-main.c | 2 +- config.c | 2 +- contrib/update-unicode/README | 6 +++--- contrib/update-unicode/update_unicode.sh | 2 +- exec_cmd.c => exec-cmd.c | 2 +- exec_cmd.h => exec-cmd.h | 0 fetch-pack.c | 2 +- git.c | 2 +- help.c | 2 +- http-backend.c | 2 +- http-fetch.c | 2 +- http-push.c | 2 +- imap-send.c | 2 +- list-objects-filter.c | 2 +- object.h | 2 +- remote-curl.c | 2 +- remote-testsvn.c | 2 +- replace_object.c => replace-object.c | 0 run-command.c | 2 +- sequencer.c | 2 +- sha1_file.c => sha1-file.c | 0 sha1_name.c => sha1-name.c | 0 shell.c | 2 +- unicode_width.h => unicode-width.h | 0 upload-pack.c | 2 +- utf8.c | 2 +- write_or_die.c => write-or-die.c | 0 42 files changed, 45 insertions(+), 45 deletions(-) rename exec_cmd.c => exec-cmd.c (99%) rename exec_cmd.h => exec-cmd.h (100%) rename replace_object.c => replace-object.c (100%) rename sha1_file.c => sha1-file.c (100%) rename sha1_name.c => sha1-name.c (100%) rename unicode_width.h => unicode-width.h (100%) rename write_or_die.c => write-or-die.c (100%)
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 1/6 write_or_die.c: rename to use dashes in file name
This is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names.
--- Makefile | 2 +- write_or_die.c => write-or-die.c | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename write_or_die.c => write-or-die.c (100%)
Makefile +1 -1
@@ -933,7 +933,7 @@ LIB_OBJS += walker.o LIB_OBJS += wildmatch.o LIB_OBJS += worktree.o LIB_OBJS += wrapper.o -LIB_OBJS += write_or_die.o +LIB_OBJS += write-or-die.o LIB_OBJS += ws.o LIB_OBJS += wt-status.o LIB_OBJS += xdiff-interface.o
write-or-die.c renamed from write_or_die.c +0 -0
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 2/6 unicode_width.h: rename to use dash in file name
In reply to Stefan BellerThis is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names.
Also adjust contrib/update-unicode as well.
--- contrib/update-unicode/README | 6 +++--- contrib/update-unicode/update_unicode.sh | 2 +- unicode_width.h => unicode-width.h | 0 utf8.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) rename unicode_width.h => unicode-width.h (100%)
contrib/update-unicode/README +3 -3
@@ -1,10 +1,10 @@ TL;DR: Run update_unicode.sh after the publication of a new Unicode -standard and commit the resulting unicode_widths.h file. +standard and commit the resulting unicode-widths.h file. The long version ================ -The Git source code ships the file unicode_widths.h which contains +The Git source code ships the file unicode-widths.h which contains tables of zero and double width Unicode code points, respectively. These tables are generated using update_unicode.sh in this directory. update_unicode.sh itself uses a third-party tool, uniset, to query two
@@ -16,5 +16,5 @@ This requires a current-ish version of autoconf (2.69 works per December On each run, update_unicode.sh checks whether more recent Unicode data files are available from the Unicode consortium, and rebuilds the header -unicode_widths.h with the new data. The new header can then be +unicode-widths.h with the new data. The new header can then be committed.
contrib/update-unicode/update_unicode.sh +1 -1
@@ -6,7 +6,7 @@ #Cf Format a format control character # cd "$(dirname "$0")" -UNICODEWIDTH_H=$(git rev-parse --show-toplevel)/unicode_width.h +UNICODEWIDTH_H=$(git rev-parse --show-toplevel)/unicode-width.h wget -N http://www.unicode.org/Public/UCD/latest/ucd/UnicodeData.txt \ http://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt &&
unicode-width.h renamed from unicode_width.h +0 -0
utf8.c +1 -1
@@ -81,7 +81,7 @@ static int git_wcwidth(ucs_char_t ch)/* * Sorted list of non-overlapping intervals of non-spacing characters, */ -#include "unicode_width.h" +#include "unicode-width.h" /* test for 8-bit control characters */ if (ch ==0 )
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 3/6 exec_cmd: rename to use dash in file name
In reply to Stefan BellerThis is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names.
--- Makefile | 6 +++--- attr.c | 2 +- builtin/add.c | 2 +- builtin/am.c | 2 +- builtin/describe.c | 2 +- builtin/difftool.c | 2 +- builtin/hash-object.c | 2 +- builtin/help.c | 2 +- builtin/index-pack.c | 2 +- builtin/init-db.c | 2 +- builtin/merge-tree.c | 2 +- builtin/notes.c | 2 +- builtin/pull.c | 2 +- builtin/receive-pack.c | 2 +- common-main.c | 2 +- config.c | 2 +- exec_cmd.c => exec-cmd.c | 2 +- exec_cmd.h => exec-cmd.h | 0 fetch-pack.c | 2 +- git.c | 2 +- help.c | 2 +- http-backend.c | 2 +- http-fetch.c | 2 +- http-push.c | 2 +- imap-send.c | 2 +- remote-curl.c | 2 +- remote-testsvn.c | 2 +- run-command.c | 2 +- sequencer.c | 2 +- shell.c | 2 +- upload-pack.c | 2 +- 31 files changed, 32 insertions(+), 32 deletions(-) rename exec_cmd.c => exec-cmd.c (99%) rename exec_cmd.h => exec-cmd.h (100%)
Makefile +3 -3
@@ -815,7 +815,7 @@ LIB_OBJS += ewah/bitmap.o LIB_OBJS += ewah/ewah_bitmap.o LIB_OBJS += ewah/ewah_io.o LIB_OBJS += ewah/ewah_rlw.o -LIB_OBJS += exec_cmd.o +LIB_OBJS += exec-cmd.o LIB_OBJS += fetch-object.o LIB_OBJS += fetch-pack.o LIB_OBJS += fsck.o
@@ -2152,8 +2152,8 @@ else $(OBJECTS): $(LIB_H) endif -exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX -exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \ +exec-cmd.sp exec-cmd.s exec-cmd.o: GIT-PREFIX +exec-cmd.sp exec-cmd.s exec-cmd.o: EXTRA_CPPFLAGS = \ '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \ '-DBINDIR="$(bindir_relative_SQ)"' \ '-DPREFIX="$(prefix_SQ)"'
attr.c +1 -1
@@ -10,7 +10,7 @@#define NO_THE_INDEX_COMPATIBILITY_MACROS #include "cache.h" #include "config.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "attr.h" #include "dir.h" #include "utf8.h"
builtin/add.c +1 -1
@@ -9,7 +9,7 @@#include "lockfile.h" #include "dir.h" #include "pathspec.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "cache-tree.h" #include "run-command.h" #include "parse-options.h"
builtin/am.c +1 -1
@@ -6,7 +6,7 @@#include "cache.h" #include "config.h" #include "builtin.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "parse-options.h" #include "dir.h" #include "run-command.h"
builtin/describe.c +1 -1
@@ -6,7 +6,7 @@#include "blob.h" #include "refs.h" #include "builtin.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "parse-options.h" #include "revision.h" #include "diff.h"
builtin/difftool.c +1 -1
@@ -15,7 +15,7 @@#include "config.h" #include "builtin.h" #include "run-command.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "parse-options.h" #include "argv-array.h" #include "strbuf.h"
builtin/hash-object.c +1 -1
@@ -9,7 +9,7 @@#include "blob.h" #include "quote.h" #include "parse-options.h" -#include "exec_cmd.h" +#include "exec-cmd.h" /* * This is to create corrupt objects for debugging and as such it
builtin/help.c +1 -1
@@ -4,7 +4,7 @@#include "cache.h" #include "config.h" #include "builtin.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "parse-options.h" #include "run-command.h" #include "column.h"
builtin/index-pack.c +1 -1
@@ -9,7 +9,7 @@#include "tree.h" #include "progress.h" #include "fsck.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "streaming.h" #include "thread-utils.h" #include "packfile.h"
builtin/init-db.c +1 -1
@@ -7,7 +7,7 @@#include "config.h" #include "refs.h" #include "builtin.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "parse-options.h" #ifndef DEFAULT_GIT_TEMPLATE_DIR
builtin/merge-tree.c +1 -1
@@ -2,7 +2,7 @@#include "tree-walk.h" #include "xdiff-interface.h" #include "blob.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "merge-blobs.h" static const char merge_tree_usage[] ="git merge-tree <base-tree> <branch1> <branch2>" ;
builtin/notes.c +1 -1
@@ -14,7 +14,7 @@#include "blob.h" #include "pretty.h" #include "refs.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "run-command.h" #include "parse-options.h" #include "string-list.h"
builtin/pull.c +1 -1
@@ -9,7 +9,7 @@#include "config.h" #include "builtin.h" #include "parse-options.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "run-command.h" #include "sha1-array.h" #include "remote.h"
builtin/receive-pack.c +1 -1
@@ -6,7 +6,7 @@#include "pkt-line.h" #include "sideband.h" #include "run-command.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "commit.h" #include "object.h" #include "remote.h"
common-main.c +1 -1
@@ -1,5 +1,5 @@#include "cache.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "attr.h" /*
config.c +1 -1
@@ -9,7 +9,7 @@#include "config.h" #include "repository.h" #include "lockfile.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "strbuf.h" #include "quote.h" #include "hashmap.h"
exec-cmd.c renamed from exec_cmd.c +1 -1
@@ -1,5 +1,5 @@#include "cache.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "quote.h" #include "argv-array.h" #define MAX_ARGS 32
exec-cmd.h renamed from exec_cmd.h +0 -0
fetch-pack.c +1 -1
@@ -5,7 +5,7 @@#include "pkt-line.h" #include "commit.h" #include "tag.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "pack.h" #include "sideband.h" #include "fetch-pack.h"
git.c +1 -1
@@ -1,6 +1,6 @@#include "builtin.h" #include "config.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "help.h" #include "run-command.h"
help.c +1 -1
@@ -1,7 +1,7 @@#include "cache.h" #include "config.h" #include "builtin.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "run-command.h" #include "levenshtein.h" #include "help.h"
http-backend.c +1 -1
@@ -4,7 +4,7 @@#include "pkt-line.h" #include "object.h" #include "tag.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "run-command.h" #include "string-list.h" #include "url.h"
http-fetch.c +1 -1
@@ -1,6 +1,6 @@#include "cache.h" #include "config.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "http.h" #include "walker.h"
http-push.c +1 -1
@@ -6,7 +6,7 @@#include "refs.h" #include "diff.h" #include "revision.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "remote.h" #include "list-objects.h" #include "sigchain.h"
imap-send.c +1 -1
@@ -24,7 +24,7 @@#include "cache.h" #include "config.h" #include "credential.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "run-command.h" #include "parse-options.h" #ifdef NO_OPENSSL
remote-curl.c +1 -1
@@ -4,7 +4,7 @@#include "strbuf.h" #include "walker.h" #include "http.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "run-command.h" #include "pkt-line.h" #include "string-list.h"
remote-testsvn.c +1 -1
@@ -3,7 +3,7 @@#include "remote.h" #include "strbuf.h" #include "url.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "run-command.h" #include "vcs-svn/svndump.h" #include "notes.h"
run-command.c +1 -1
@@ -1,6 +1,6 @@#include "cache.h" #include "run-command.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "sigchain.h" #include "argv-array.h" #include "thread-utils.h"
sequencer.c +1 -1
@@ -7,7 +7,7 @@#include "sequencer.h" #include "tag.h" #include "run-command.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "utf8.h" #include "cache-tree.h" #include "diff.h"
shell.c +1 -1
@@ -1,6 +1,6 @@#include "cache.h" #include "quote.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "strbuf.h" #include "run-command.h"
upload-pack.c +1 -1
@@ -6,7 +6,7 @@#include "tag.h" #include "object.h" #include "commit.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "diff.h" #include "revision.h" #include "list-objects.h"
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 4/6 sha1_name.c: rename to use dash in file name
In reply to Stefan BellerThis is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names.
--- Makefile | 2 +- list-objects-filter.c | 2 +- object.h | 2 +- sha1_name.c => sha1-name.c | 0 4 files changed, 3 insertions(+), 3 deletions(-) rename sha1_name.c => sha1-name.c (100%)
Makefile +1 -1
@@ -898,7 +898,7 @@ LIB_OBJS += setup.o LIB_OBJS += sha1-array.o LIB_OBJS += sha1-lookup.o LIB_OBJS += sha1_file.o -LIB_OBJS += sha1_name.o +LIB_OBJS += sha1-name.o LIB_OBJS += shallow.o LIB_OBJS += sideband.o LIB_OBJS += sigchain.o
list-objects-filter.c +1 -1
@@ -19,7 +19,7 @@* in the traversal (until we mark it SEEN). This is a way to * let us silently de-dup calls to show() in the caller. This * is subtly different from the "revision.h:SHOWN" and the -* "sha1_name.c:ONELINE_SEEN" bits. And also different from +* "sha1-name.c:ONELINE_SEEN" bits. And also different from * the non-de-dup usage in pack-bitmap.c */ #define FILTER_SHOWN_BUT_REVISIT (1<<21)
object.h +1 -1
@@ -37,7 +37,7 @@ struct object_array {* bundle.c: 16 * http-push.c: 16-----19 * commit.c: 16-----19 -* sha1_name.c: 20 +* sha1-name.c: 20 * list-objects-filter.c: 21 * builtin/fsck.c: 0--3 * builtin/index-pack.c: 2021
sha1-name.c renamed from sha1_name.c +0 -0
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 5/6 sha1_file.c: rename to use dash in file name
In reply to Stefan BellerThis is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names.
--- Documentation/technical/api-object-access.txt | 2 +- Makefile | 2 +- builtin/index-pack.c | 2 +- sha1_file.c => sha1-file.c | 0 4 files changed, 3 insertions(+), 3 deletions(-) rename sha1_file.c => sha1-file.c (100%)
Documentation/technical/api-object-access.txt +1 -1
@@ -1,7 +1,7 @@ object access API ================= -Talk about <sha1_file.c> and <object.h> family, things like +Talk about <sha1-file.c> and <object.h> family, things like * read_sha1_file() * read_object_with_reference()
Makefile +1 -1
@@ -897,7 +897,7 @@ LIB_OBJS += server-info.o LIB_OBJS += setup.o LIB_OBJS += sha1-array.o LIB_OBJS += sha1-lookup.o -LIB_OBJS += sha1_file.o +LIB_OBJS += sha1-file.o LIB_OBJS += sha1-name.o LIB_OBJS += shallow.o LIB_OBJS += sideband.o
builtin/index-pack.c +1 -1
@@ -1592,7 +1592,7 @@ static void read_idx_option(struct pack_idx_option *opts, const char *pack_name)/* * Get rid of the idx file as we do not need it anymore. * NEEDSWORK: extract this bit from free_pack_by_name() in -* sha1_file.c, perhaps? It shouldn't matter very much as we +* sha1-file.c, perhaps? It shouldn't matter very much as we * know we haven't installed this pack (hence we never have * read anything from it). */
sha1-file.c renamed from sha1_file.c +0 -0
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH 6/6 replace_object.c: rename to use dash in file name
In reply to Stefan BellerThis is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names.
Noticed while adding a header corresponding to this file.
--- Makefile | 2 +- replace_object.c => replace-object.c | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename replace_object.c => replace-object.c (100%)
Makefile +1 -1
@@ -885,7 +885,7 @@ LIB_OBJS += refs/packed-backend.o LIB_OBJS += refs/ref-cache.o LIB_OBJS += ref-filter.o LIB_OBJS += remote.o -LIB_OBJS += replace_object.o +LIB_OBJS += replace-object.o LIB_OBJS += repository.o LIB_OBJS += rerere.o LIB_OBJS += resolve-undo.o
replace-object.c renamed from replace_object.c +0 -0
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Re: [PATCH 0/6] Rename files to use dashes instead of underscores
In reply to Stefan BellerQuoted 5 lines · Stefan Beller
This is the followup for
https://public-inbox.org/git/[email protected]/We have no files left with underscores in their names.
It applies on master, and also contains that patch for replace_objects, which I would intend to drop from that other series.
Re: [PATCH 0/6] Rename files to use dashes instead of underscores
In reply to Stefan BellerHi Stefan,
Quoted 4 lines · Stefan Beller
This is the followup for
https://public-inbox.org/git/[email protected]/We have no files left with underscores in their names.
Yaaay!
Quoted 7 lines
Stefan Beller (6):
write_or_die.c: rename to use dashes in file nameunicode_width.h: rename to use dash in file name
exec_cmd: rename to use dash in file namesha1_name.c: rename to use dash in file namesha1_file.c: rename to use dash in file namereplace_object.c: rename to use dash in file name
These are all obviously correct (I did not apply the series and used git grep to verify that nothing underscored is left there, but I trust you to have done that already).
Ciao,
Dscho
Re: [PATCH 0/6] Rename files to use dashes instead of underscores
Hi Johannes,
Quoted 19 lines · Johannes Schindelin
Hi Stefan,
Quoted 4 lines · Stefan Beller
This is the followup for
https://public-inbox.org/git/[email protected]/We have no files left with underscores in their names.
Yaaay!
Quoted 7 lines
Stefan Beller (6):
write_or_die.c: rename to use dashes in file nameunicode_width.h: rename to use dash in file name
exec_cmd: rename to use dash in file namesha1_name.c: rename to use dash in file namesha1_file.c: rename to use dash in file namereplace_object.c: rename to use dash in file name
Quoted 3 lines
These are all obviously correct (I did not apply the series and used
git grepto verify that nothing underscored is left there, but I trust you to have done that already).
Yes and I did not tell the full story.
There is still 'check_bindir' as compile output, such that I ignored it.
And there was sha1dc_git.{h, c} which I also ignored as it is unclear to me how our relationship with the DC library is.
I think that could also be converted as it seems to be a shallow wrapper around that lib.
Re: [PATCH 0/6] Rename files to use dashes instead of underscores
In reply to Stefan BellerQuoted 4 lines · Stefan Beller
This is the followup for
https://public-inbox.org/git/[email protected]/We have no files left with underscores in their names.
This series looked good to me. It's a nice change.
Signature · 2 lines
brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204
PATCH v2 3/16 replace-object: move replace_map to object store
In reply to Stefan Beller--- object-store.h | 8 ++++++++ replace-object.c | 17 +++++++---------- replace-object.h | 9 +++++++++ 3 files changed, 24 insertions(+), 10 deletions(-) create mode 100644 replace-object.h
object-store.h +8 -0
@@ -1,6 +1,8 @@ #ifndef OBJECT_STORE_H #define OBJECT_STORE_H +#include "oidmap.h" + struct alternate_object_database { struct alternate_object_database *next;
@@ -93,6 +95,12 @@ struct raw_object_store {struct alternate_object_database *alt_odb_list;struct alternate_object_database **alt_odb_tail; +/* +* Objects that should be substituted by other objects +* (see git-replace(1)). +*/ +struct oidmap replace_map; +/* * private data *
replace-object.c +7 -10
@@ -1,15 +1,11 @@#include "cache.h" #include "oidmap.h" +#include "object-store.h" +#include "replace-object.h" #include "refs.h" +#include "repository.h" #include "commit.h" -struct replace_object { -struct oidmap_entry original; -struct object_id replacement; -}; - -static struct oidmap replace_map =OIDMAP_INIT ; -static int register_replace_ref (const char *refname,const struct object_id *oid,int flag,void *cb_data)
@@ -29,7 +25,7 @@ static int register_replace_ref(const char *refname,oidcpy (&repl_obj->replacement, oid);/* Register new object */ -if (oidmap_put (&replace_map, repl_obj)) +if (oidmap_put (&the_repository->objects->replace_map, repl_obj))die ("duplicate replace ref: %s" , refname);return 0 ;
@@ -44,7 +40,7 @@ static void prepare_replace_object(void)for_each_replace_ref (register_replace_ref,NULL ); replace_object_prepared =1 ; -if (!replace_map.map.tablesize) +if (!the_repository->objects->replace_map.map.tablesize) check_replace_refs =0 ; }
@@ -67,7 +63,8 @@ const struct object_id *do_lookup_replace_object(const struct object_id *oid)/* Try to recursively replace the object */ while (depth-- >0 ) { -struct replace_object *repl_obj =oidmap_get (&replace_map, cur); +struct replace_object *repl_obj = +oidmap_get (&the_repository->objects->replace_map, cur);if (!repl_obj)return cur; cur = &repl_obj->replacement;
replace-object.h new file +9 -0
@@ -0,0 +1,9 @@ +#ifndef REPLACE_OBJECT_H +#define REPLACE_OBJECT_H + +struct replace_object { +struct oidmap_entry original; +struct object_id replacement; +}; + +#endif /* REPLACE_OBJECT_H */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Re: [PATCH 03/16] replace-object: move replace_map to object store
Quoted 7 lines · Stefan Beller
Signed-off-by Stefan Beller <[email protected]>--- object-store.h | 8 ++++++++ replace-object.c | 17 +++++++---------- replace-object.h | 9 +++++++++ 3 files changed, 24 insertions(+), 10 deletions(-) create mode 100644 replace-object.h
With this, the relationship between an object X and another object Y that replaces the object X is defined only within the scope of a single repository.
The exception in reachability rule around these replacement objects is also local to a repository (i.e. if traversal from refs reaches X, then both X and Y are reachable and need to be kept from gc), so I think this is a reasonable way to arrange things.
PATCH v2 4/16 object-store: move lookup_replace_object to replace-object.h
In reply to Stefan Bellerlookup_replace_object is a low-level function that most users of the object store do not need to use directly.
Move it to replace-object.h to avoid a dependency loop in an upcoming change to its inline definition that will make use of repository.h.
--- builtin/mktag.c | 1 + cache.h | 19 ------------------- object.c | 1 + replace-object.h | 22 ++++++++++++++++++++++ sha1_file.c | 1 + streaming.c | 1 + 6 files changed, 26 insertions(+), 19 deletions(-)
builtin/mktag.c +1 -0
@@ -1,5 +1,6 @@#include "builtin.h" #include "tag.h" +#include "replace-object.h" /* * A signature file has a very simple fixed format: four lines
cache.h +0 -19
@@ -1187,25 +1187,6 @@ static inline void *read_object_file(const struct object_id *oid, enum object_tyreturn read_object_file_extended (oid, type, size,1 ); } -/* -* This internal function is only declared here for the benefit of -* lookup_replace_object(). Please do not call it directly. -*/ -extern const struct object_id *do_lookup_replace_object (const struct object_id *oid); - -/* -* If object sha1 should be replaced, return the replacement object's -* name (replaced recursively, if necessary). The return value is -* either sha1 or a pointer to a permanently-allocated value. When -* object replacement is suppressed, always return sha1. -*/ -static inline const struct object_id *lookup_replace_object (const struct object_id *oid) -{ -if (!check_replace_refs) -return oid; -return do_lookup_replace_object (oid); -} -/* Read and unpack an object file into memory, write memory to an object file */ extern int oid_object_info (const struct object_id *,unsigned long *);
object.c +1 -0
@@ -1,5 +1,6 @@#include "cache.h" #include "object.h" +#include "replace-object.h" #include "blob.h" #include "tree.h" #include "commit.h"
replace-object.h +22 -0
@@ -1,9 +1,31 @@#ifndef REPLACE_OBJECT_H #define REPLACE_OBJECT_H +#include "oidmap.h" +#include "repository.h" +struct replace_object {struct oidmap_entry original;struct object_id replacement; }; +/* +* This internal function is only declared here for the benefit of +* lookup_replace_object(). Please do not call it directly. +*/ +extern const struct object_id *do_lookup_replace_object (const struct object_id *oid); + +/* +* If object sha1 should be replaced, return the replacement object's +* name (replaced recursively, if necessary). The return value is +* either sha1 or a pointer to a permanently-allocated value. When +* object replacement is suppressed, always return sha1. +*/ +static inline const struct object_id *lookup_replace_object (const struct object_id *oid) +{ +if (!check_replace_refs) +return oid; +return do_lookup_replace_object (oid); +} +#endif /* REPLACE_OBJECT_H */
sha1_file.c +1 -0
@@ -23,6 +23,7 @@#include "sha1-lookup.h" #include "bulk-checkin.h" #include "repository.h" +#include "replace-object.h" #include "streaming.h" #include "dir.h" #include "list.h"
streaming.c +1 -0
@@ -5,6 +5,7 @@#include "streaming.h" #include "repository.h" #include "object-store.h" +#include "replace-object.h" #include "packfile.h" enum input_source {
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v2 5/16 replace-object: eliminate replace objects prepared flag
In reply to Stefan BellerBy making the oidmap a pointer, we eliminate the need for the global boolean variable 'replace_object_prepared'.
--- object-store.h | 2 +- replace-object.c | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-)
object-store.h +1 -1
@@ -99,7 +99,7 @@ struct raw_object_store {* Objects that should be substituted by other objects * (see git-replace(1)). */ -struct oidmap replace_map; +struct oidmap *replace_map;/* * private data
replace-object.c +9 -7
@@ -25,7 +25,7 @@ static int register_replace_ref(const char *refname,oidcpy (&repl_obj->replacement, oid);/* Register new object */ -if (oidmap_put (&the_repository->objects->replace_map, repl_obj)) +if (oidmap_put (the_repository->objects->replace_map, repl_obj))die ("duplicate replace ref: %s" , refname);return 0 ;
@@ -33,14 +33,16 @@ static int register_replace_ref(const char *refname,static void prepare_replace_object (void ) { -static int replace_object_prepared; - -if (replace_object_prepared) +if (the_repository->objects->replace_map)return ; + the_repository->objects->replace_map = +xmalloc (sizeof (*the_repository->objects->replace_map)); +oidmap_init (the_repository->objects->replace_map,0 ); +for_each_replace_ref (register_replace_ref,NULL ); - replace_object_prepared =1 ; -if (!the_repository->objects->replace_map.map.tablesize) + +if (!the_repository->objects->replace_map->map.tablesize) check_replace_refs =0 ; }
@@ -64,7 +66,7 @@ const struct object_id *do_lookup_replace_object(const struct object_id *oid)/* Try to recursively replace the object */ while (depth-- >0 ) {struct replace_object *repl_obj = -oidmap_get (&the_repository->objects->replace_map, cur); +oidmap_get (the_repository->objects->replace_map, cur);if (!repl_obj)return cur; cur = &repl_obj->replacement;
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Re: [PATCH 05/16] replace-object: eliminate replace objects prepared flag
Quoted 2 lines · Stefan Beller
By making the oidmap a pointer, we eliminate the need for the global boolean variable '
replace_object_prepared'.
That is not quite a justification for this change, as making it a pointer (and paying for the malloc(3) overhead) is not the only way to remove the variable (i.e. the "has this been initialized?" bit can be moved to "struct raw_object_store").
One possible advantage of this approach, I guess, is that we would more quickly catch code that tries to access replace-map without initializing it.
Re: [PATCH 05/16] replace-object: eliminate replace objects prepared flag
In reply to Stefan BellerAm 10.04.2018 um 00:45 schrieb Stefan Beller:
Quoted 14 lines
By making the oidmap a pointer, we eliminate the need for the global boolean variable '
replace_object_prepared'.Signed-off-by Stefan Beller <[email protected]>--- object-store.h | 2 +- replace-object.c | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-)object-store.h +0
-0
object-store.h struct raw_object_store { · line 102
Quoted 5 lines
object-store.h +1
-1@@ -99,7 +99,7 @@ struct raw_object_store {* Objects that should be substituted by other objects * (see git-replace(1)). */ -struct oidmap replace_map; +struct oidmap *replace_map;
This also allows the '#include "oidmap.h"' introduced in patch 3 to be replaced by 'struct oidmap;' (forward declaration instead of include). Keeping the type opaque discourages circumventing accessor functions; not dragging in other headers avoids some compile time overhead.
René
PATCH v2 6/16 replace-object: check_replace_refs is safe in multi repo environment
In reply to Stefan BellerIn e1111cef23 (inline lookup_replace_object() calls, 2011-05-15) a shortcut for checking the object replacement was added by setting check_replace_refs to 0 once the replacements were evaluated to not exist. This works fine in with the assumption of only one repository in existence.
The assumption won't hold true any more when we work on multiple instances of a repository structs (e.g. one struct per submodule), as the first repository to be inspected may have no replacements and would set the global variable. Other repositories would then completely omit their evaluation of replacements.
This reverts back the meaning of the flag check_replace_refs of "Do we need to check with the lookup table?" to "Do we need to read the replacement definition?", adding the bypassing logic to lookup_replace_object after the replacement definition was read. As with the original patch, delay the renaming of the global variable
--- environment.c | 2 +- replace-object.c | 3 --- replace-object.h | 5 ++++- 3 files changed, 5 insertions(+), 5 deletions(-)
environment.c +1 -1
@@ -50,7 +50,7 @@ const char *editor_program;const char *askpass_program;const char *excludes_file;enum auto_crlf auto_crlf =AUTO_CRLF_FALSE ; -int check_replace_refs =1 ; +int check_replace_refs =1 ;/* NEEDSWORK: rename to read_replace_refs */ char *git_replace_ref_base;enum eol core_eol =EOL_UNSET ;int global_conv_flags_eol =CONV_EOL_RNDTRP_WARN ;
replace-object.c +0 -3
@@ -41,9 +41,6 @@ static void prepare_replace_object(void)oidmap_init (the_repository->objects->replace_map,0 );for_each_replace_ref (register_replace_ref,NULL ); - -if (!the_repository->objects->replace_map->map.tablesize) - check_replace_refs =0 ; }/* We allow "recursive" replacement. Only within reason, though */
replace-object.h +4 -1
@@ -3,6 +3,7 @@#include "oidmap.h" #include "repository.h" +#include "object-store.h" struct replace_object {struct oidmap_entry original;
@@ -23,7 +24,9 @@ extern const struct object_id *do_lookup_replace_object(const struct object_id **/ static inline const struct object_id *lookup_replace_object (const struct object_id *oid) { -if (!check_replace_refs) +if (!check_replace_refs || + (the_repository->objects->replace_map && + the_repository->objects->replace_map->map.tablesize ==0 ))return oid;return do_lookup_replace_object (oid); }
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Re: [PATCH 06/16] replace-object: check_replace_refs is safe in multi repo environment
Quoted 5 lines · Stefan Beller
In
e1111cef23(inlinelookup_replace_object()calls, 2011-05-15) a shortcut for checking the object replacement was added by settingcheck_replace_refsto 0 once the replacements were evaluated to not exist. This works fine in with the assumption of only one repository in existence.
"works fine in with the..."? I guess s/ in with/ with/?
Quoted 11 lines
The assumption won't hold true any more when we work on multiple instances of a repository structs (e.g. one struct per submodule), as the first repository to be inspected may have no replacements and would set the global variable. Other repositories would then completely omit their evaluation of replacements.
This reverts back the meaning of the flag
check_replace_refsof "Do we need to check with the lookup table?" to "Do we need to read the replacement definition?", adding the bypassing logic tolookup_replace_objectafter the replacement definition was read. As with the original patch, delay the renaming of the global variable
Hmph, if we decided that replace database is per repository instance, shouldn't this variable also become per repository, instead of staying to be a system-wide global?
Perhaps that will happpen in a later stage of the series that I haven't seen yet, I guess. And until that happens, we disable the optimization and always call into do_lookup_replace_object() when lookup_replace_object() is called, which is OK.
replace-object.h extern const struct object_id *do_lookup_replace_object(const struct object_id * · line 29
Quoted 6 lines
static inline const struct
object_id*lookup_replace_object(const structobject_id*oid) {replace-object.h +3
-1-if (!check_replace_refs) +if (!check_replace_refs || + (the_repository->objects->replace_map && + the_repository->objects->replace_map->map.tablesize ==0 ))
Ah, we still have the same optimization, so this looks alright. The variable's name and semantics do need to be updated--I didn't check but if this variable is exposed to the end users in any way, such a fundamental sematic change may be hard to transition, though.
This series looks good so far.
PATCH v2 7/16 refs: add repository argument to get_main_ref_store
In reply to Stefan BellerAdd a repository argument to allow the get_main_ref_store caller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- builtin/pack-refs.c | 3 +- refs.c | 67 ++++++++++++++++++++------------------- refs.h | 4 ++- revision.c | 5 +-- t/helper/test-ref-store.c | 3 +- 5 files changed, 44 insertions(+), 38 deletions(-)
builtin/pack-refs.c +2 -1
@@ -1,6 +1,7 @@#include "builtin.h" #include "parse-options.h" #include "refs.h" +#include "repository.h" static char const *const pack_refs_usage[] = {N_ ("git pack-refs [<options>]" ),
@@ -17,5 +18,5 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix) };if (parse_options (argc, argv, prefix, opts, pack_refs_usage,0 ))usage_with_options (pack_refs_usage, opts); -return refs_pack_refs (get_main_ref_store (), flags); +return refs_pack_refs (get_main_ref_store (the_repository), flags); }
refs.c +34 -33
269 lines not on this page · raw message
refs.h +3 -1
@@ -758,7 +758,9 @@ int reflog_expire(const char *refname, const struct object_id *oid,int ref_storage_backend_exists (const char *name); -struct ref_store *get_main_ref_store (void ); +#define get_main_ref_store (r) \ + get_main_ref_store_##r() +struct ref_store *get_main_ref_store_the_repository (void );/* * Return the ref_store instance for the specified submodule. For the * main repository, use submodule==NULL; such a call cannot fail. For
revision.c +3 -2
@@ -6,6 +6,7 @@#include "diff.h" #include "refs.h" #include "revision.h" +#include "repository.h" #include "graph.h" #include "grep.h" #include "reflog-walk.h"
@@ -1285,7 +1286,7 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags) cb.all_revs = revs; cb.all_flags = flags; - cb.refs =get_main_ref_store (); + cb.refs =get_main_ref_store (the_repository);for_each_reflog (handle_one_reflog, &cb);if (!revs->single_worktree)
@@ -2176,7 +2177,7 @@ static int handle_revision_pseudo_opt(const char *submodule,die ("BUG: --single-worktree cannot be used together with submodule" ); refs =get_submodule_ref_store (submodule); }else - refs =get_main_ref_store (); + refs =get_main_ref_store (the_repository);/* * NOTE!
t/helper/test-ref-store.c +2 -1
@@ -2,6 +2,7 @@#include "refs.h" #include "worktree.h" #include "object-store.h" +#include "repository.h" static const char *notnull (const char *arg,const char *name) {
@@ -22,7 +23,7 @@ static const char **get_store(const char **argv, struct ref_store **refs)if (!argv[0 ]) {die ("ref store required" ); }else if (!strcmp (argv[0 ],"main" )) { - *refs =get_main_ref_store (); + *refs =get_main_ref_store (the_repository); }else if (skip_prefix (argv[0 ],"submodule:" , &gitdir)) {struct strbuf sb =STRBUF_INIT ;int ret;
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Re: [PATCH 07/16] refs: add repository argument to get_main_ref_store
Quoted 7 lines · Stefan Beller
Add a repository argument to allow the
get_main_ref_storecaller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other thanthe_repositoryyet.As with the previous commits, use a macro to catch callers passing a repository other than
the_repositoryat compile time.
This seems OK to me from a refs perspective.
The macro trick is surprising. I guess it gets you a compile-time check, under the assumption that nothing else is called the_repository. But why actually commit the macro, as opposed to compiling once locally to check for correctness, then maybe add something like assert(r == the_repository) for the actual commit?
But I don't care either way, since the macro disappears again soon.
Michael
Re: [PATCH 07/16] refs: add repository argument to get_main_ref_store
Hi Michael,
Quoted 6 lines · Michael Haggerty
Quoted 5 lines · Stefan Beller
Add a repository argument to allow the
get_main_ref_storecaller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other thanthe_repositoryyet.
Quoted 7 lines
Quoted 2 lines
As with the previous commits, use a macro to catch callers passing a repository other than
the_repositoryat compile time.This seems OK to me from a refs perspective.
The macro trick is surprising. I guess it gets you a compile-time check, under the assumption that nothing else is called
the_repository.
Yes. Credit goes to Jonathan Tan for this trick.
Quoted 4 lines
But
why actually commit the macro, as opposed to compiling once locally to check for correctness, then maybe add something likeassert(r == the_repository)for the actual commit?
The eternal struggle of contributing patches that are easy to review. ;)
With the assert we'll have a run time check, which is not desirable compared to a compile time check. And from a reviewers point of view running a "rebase -x make" on the series that Junio queued is easier than to reason about the "assert(r = the_repository)" IMHO.
Quoted 1 line
But I don't care either way, since the macro disappears again soon.
Glad you're ok with this approach.
Thanks for looking at the refs specific code,
Stefan
PATCH v2 8/16 refs: add repository argument to for_each_replace_ref
In reply to Stefan BellerAdd a repository argument to allow for_each_replace_ref callers to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- builtin/replace.c | 4 +++- refs.c | 2 +- refs.h | 4 +++- replace-object.c | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-)
builtin/replace.c +3 -1
@@ -14,6 +14,8 @@#include "refs.h" #include "parse-options.h" #include "run-command.h" +#include "object-store.h" +#include "repository.h" #include "tag.h" static const char *const git_replace_usage[] = {
@@ -83,7 +85,7 @@ static int list_replace_refs(const char *pattern, const char *format)"valid formats are 'short', 'medium' and 'long'\n" , format); -for_each_replace_ref (show_reference, (void *)&data); +for_each_replace_ref (the_repository, show_reference, (void *)&data);return 0 ; }
refs.c +1 -1
@@ -1415,7 +1415,7 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,return do_for_each_ref (refs, prefix, fn,0 , flag, cb_data); } -int for_each_replace_ref (each_ref_fn fn,void *cb_data) +int for_each_replace_ref_the_repository (each_ref_fn fn,void *cb_data) { return do_for_each_ref(get_main_ref_store(the_repository), git_replace_ref_base, fn,
refs.h +3 -1
@@ -300,7 +300,9 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,int for_each_tag_ref (each_ref_fn fn,void *cb_data);int for_each_branch_ref (each_ref_fn fn,void *cb_data);int for_each_remote_ref (each_ref_fn fn,void *cb_data); -int for_each_replace_ref (each_ref_fn fn,void *cb_data); +#define for_each_replace_ref (r, fn, cb) \ + for_each_replace_ref_##r(fn, cb) +int for_each_replace_ref_the_repository (each_ref_fn fn,void *cb_data);int for_each_glob_ref (each_ref_fn fn,const char *pattern,void *cb_data);int for_each_glob_ref_in (each_ref_fn fn,const char *pattern,const char *prefix,void *cb_data);
replace-object.c +1 -1
@@ -40,7 +40,7 @@ static void prepare_replace_object(void)xmalloc (sizeof (*the_repository->objects->replace_map));oidmap_init (the_repository->objects->replace_map,0 ); -for_each_replace_ref (register_replace_ref,NULL ); +for_each_replace_ref (the_repository, register_replace_ref,NULL ); }/* We allow "recursive" replacement. Only within reason, though */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v2 9/16 replace-object: add repository argument to prepare_replace_object
In reply to Stefan BellerAdd a repository argument to allow the prepare_replace_object caller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- replace-object.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
replace-object.c +4 -2
@@ -31,7 +31,9 @@ static int register_replace_ref(const char *refname,return 0 ; } -static void prepare_replace_object (void ) +#define prepare_replace_object (r) \ + prepare_replace_object_##r() +static void prepare_replace_object_the_repository (void ) {if (the_repository->objects->replace_map)return ;
@@ -58,7 +60,7 @@ const struct object_id *do_lookup_replace_object(const struct object_id *oid)int depth =MAXREPLACEDEPTH ;const struct object_id *cur = oid; -prepare_replace_object (); +prepare_replace_object (the_repository);/* Try to recursively replace the object */ while (depth-- >0 ) {
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v2 10/16 replace-object: add repository argument to do_lookup_replace_object
In reply to Stefan BellerAdd a repository argument to allow the do_lookup_replace_object caller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- replace-object.c | 2 +- replace-object.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-)
replace-object.c +1 -1
@@ -55,7 +55,7 @@ static void prepare_replace_object_the_repository(void)* permanently-allocated value. This function always respects replace * references, regardless of the value of check_replace_refs. */ -const struct object_id *do_lookup_replace_object (const struct object_id *oid) +const struct object_id *do_lookup_replace_object_the_repository (const struct object_id *oid) {int depth =MAXREPLACEDEPTH ;const struct object_id *cur = oid;
replace-object.h +3 -2
@@ -14,7 +14,8 @@ struct replace_object {* This internal function is only declared here for the benefit of * lookup_replace_object(). Please do not call it directly. */ -extern const struct object_id *do_lookup_replace_object (const struct object_id *oid); +#define do_lookup_replace_object (r, s) do_lookup_replace_object_##r(s) +extern const struct object_id *do_lookup_replace_object_the_repository (const struct object_id *oid);/* * If object sha1 should be replaced, return the replacement object's
@@ -28,7 +29,7 @@ static inline const struct object_id *lookup_replace_object(const struct object_ (the_repository->objects->replace_map && the_repository->objects->replace_map->map.tablesize == 0))return oid; - return do_lookup_replace_object(oid); + return do_lookup_replace_object(the_repository, oid); }#endif /* REPLACE_OBJECT_H */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v2 11/16 replace-object: add repository argument to lookup_replace_object
In reply to Stefan BellerAdd a repository argument to allow callers of lookup_replace_object to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- builtin/mktag.c | 2 +- object.c | 2 +- replace-object.h | 3 ++- sha1_file.c | 6 +++--- streaming.c | 2 +- 5 files changed, 8 insertions(+), 7 deletions(-)
builtin/mktag.c +1 -1
@@ -25,7 +25,7 @@ static int verify_object(const struct object_id *oid, const char *expected_type)enum object_type type;unsigned long size;void *buffer =read_object_file (oid, &type, &size); -const struct object_id *repl =lookup_replace_object (oid); +const struct object_id *repl =lookup_replace_object (the_repository, oid);if (buffer) {if (type ==type_from_string (expected_type))
object.c +1 -1
@@ -247,7 +247,7 @@ struct object *parse_object(const struct object_id *oid)unsigned long size;enum object_type type;int eaten; -const struct object_id *repl =lookup_replace_object (oid); +const struct object_id *repl =lookup_replace_object (the_repository, oid);void *buffer;struct object *obj;
replace-object.h +2 -1
@@ -23,7 +23,8 @@ extern const struct object_id *do_lookup_replace_object_the_repository(const str* either sha1 or a pointer to a permanently-allocated value. When * object replacement is suppressed, always return sha1. */ -static inline const struct object_id *lookup_replace_object (const struct object_id *oid) +#define lookup_replace_object (r, s) lookup_replace_object_##r(s) +static inline const struct object_id *lookup_replace_object_the_repository (const struct object_id *oid) { if (!check_replace_refs || (the_repository->objects->replace_map &&
sha1_file.c +3 -3
@@ -1240,7 +1240,7 @@ int oid_object_info_extended(const struct object_id *oid, struct object_info *oiint already_retried =0 ;if (flags &OBJECT_INFO_LOOKUP_REPLACE ) - real =lookup_replace_object (oid); + real =lookup_replace_object (the_repository, oid);if (is_null_oid (real))return -1 ;
@@ -1379,8 +1379,8 @@ void *read_object_file_extended(const struct object_id *oid,const struct packed_git *p;const char *path;struct stat st; -const struct object_id *repl = lookup_replace ?lookup_replace_object (oid) - : oid; +const struct object_id *repl = lookup_replace ? +lookup_replace_object (the_repository, oid) : oid; errno =0 ; data =read_object (repl->hash, type, size);
streaming.c +1 -1
@@ -140,7 +140,7 @@ struct git_istream *open_istream(const struct object_id *oid, {struct git_istream *st;struct object_info oi =OBJECT_INFO_INIT ; -const struct object_id *real =lookup_replace_object (oid); +const struct object_id *real =lookup_replace_object (the_repository, oid);enum input_source src =istream_source (real, type, &oi);if (src <0 )
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v2 12/16 refs: store the main ref store inside the repository struct
In reply to Stefan Beller--- refs.c | 13 +++++-------- refs.h | 4 +--- repository.h | 3 +++ 3 files changed, 9 insertions(+), 11 deletions(-)
refs.c +5 -8
@@ -1608,9 +1608,6 @@ static struct ref_store_hash_entry *alloc_ref_store_hash_entry(return entry; } -/* A pointer to the ref_store for the main repository: */ -static struct ref_store *main_ref_store; -/* A hashmap of ref_stores, stored by submodule name: */ static struct hashmap submodule_ref_stores;
@@ -1652,13 +1649,13 @@ static struct ref_store *ref_store_init(const char *gitdir,return refs; } -struct ref_store *get_main_ref_store_the_repository (void ) +struct ref_store *get_main_ref_store (struct repository *r) { -if (main_ref_store) -return main_ref_store; +if (r->main_ref_store) +return r->main_ref_store; - main_ref_store =ref_store_init (get_git_dir (),REF_STORE_ALL_CAPS ); -return main_ref_store; + r->main_ref_store =ref_store_init (r->gitdir,REF_STORE_ALL_CAPS ); +return r->main_ref_store; }/*
refs.h +1 -3
@@ -760,9 +760,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,int ref_storage_backend_exists (const char *name); -#define get_main_ref_store (r) \ - get_main_ref_store_##r() -struct ref_store *get_main_ref_store_the_repository (void ); +struct ref_store *get_main_ref_store (struct repository *r);/* * Return the ref_store instance for the specified submodule. For the * main repository, use submodule==NULL; such a call cannot fail. For
repository.h +3 -0
@@ -26,6 +26,9 @@ struct repository {*/ struct raw_object_store *objects; +/* The store in which the refs are held. */ +struct ref_store *main_ref_store; +/* * Path to the repository's graft file. * Cannot be NULL after initialization.
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Re: [PATCH 12/16] refs: store the main ref store inside the repository struct
Quoted 27 lines · Stefan Beller
Signed-off-by Stefan Beller <[email protected]>--- refs.c | 13 +++++-------- refs.h | 4 +--- repository.h | 3 +++ 3 files changed, 9 insertions(+), 11 deletions(-)refs.c +1
-4@@ -1608,9 +1608,6 @@ static struct ref_store_hash_entry *alloc_ref_store_hash_entry(return entry; } -/* A pointer to the ref_store for the main repository: */ -static struct ref_store *main_ref_store; -/* A hashmap of ref_stores, stored by submodule name: */ static struct hashmap submodule_ref_stores;@@ -1652,13 +1649,13 @@ static struct ref_store *ref_store_init(const char *gitdir,return refs; } -struct ref_store *get_main_ref_store_the_repository (void ) +struct ref_store *get_main_ref_store (struct repository *r)
refs.c static struct ref_store *ref_store_init(const char *gitdir, · line 1658
Quoted 10 lines
refs.c +4
-4{ -if (main_ref_store) -return main_ref_store; +if (r->main_ref_store) +return r->main_ref_store; - main_ref_store =ref_store_init (get_git_dir (),REF_STORE_ALL_CAPS ); -return main_ref_store; + r->main_ref_store =ref_store_init (r->gitdir,REF_STORE_ALL_CAPS ); +return r->main_ref_store;
I assume that since this takes in a git dir as a parameter that the ref-store is in a good enough place to be embedded in a repository struct (as in this would work with an arbitrary repo)?
Quoted 35 lines
refs.c +0
-0}/* refs.h +1
-3@@ -760,9 +760,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,int ref_storage_backend_exists (const char *name); -#define get_main_ref_store (r) \ - get_main_ref_store_##r() -struct ref_store *get_main_ref_store_the_repository (void ); +struct ref_store *get_main_ref_store (struct repository *r);/* * Return the ref_store instance for the specified submodule. For the * main repository, use submodule==NULL; such a call cannot fail. For repository.h +3
-0@@ -26,6 +26,9 @@ struct repository {*/ struct raw_object_store *objects; +/* The store in which the refs are held. */ +struct ref_store *main_ref_store; +/* * Path to the repository's graft file. * Cannot be NULL after initialization. Signature · 1 line
2.17.0.484.g0c8726318c-goog
Signature · 1 line
Brandon Williams
Re: [PATCH 12/16] refs: store the main ref store inside the repository struct
Hi Brandon,
Quoted 8 lines · Brandon Williams
refs.c static struct ref_store *ref_store_init(const char *gitdir, · line 1658
Quoted 4 lines
refs.c +2
-2- main_ref_store =ref_store_init (get_git_dir (),REF_STORE_ALL_CAPS ); -return main_ref_store; + r->main_ref_store =ref_store_init (r->gitdir,REF_STORE_ALL_CAPS ); +return r->main_ref_store;I assume that since this takes in a git dir as a parameter that the ref-store is in a good enough place to be embedded in a repository struct (as in this would work with an arbitrary repo)?
That is my current understanding.
As the refs code can also take a path into a submodule and construct a submodule ref store for the caller, we'd want to resolve the tension between the ref store and the repository struct who is responsible for the submodule ref store eventually by removing the submodule functionality from the ref store and only relying on the ref stores created by the repository struct.
Re: [PATCH 12/16] refs: store the main ref store inside the repository struct
Quoted 13 lines · Stefan Beller
Hi Brandon,
Quoted 8 lines · Brandon Williams
refs.c static struct ref_store *ref_store_init(const char *gitdir, · line 1658
Quoted 4 lines
refs.c +2
-2- main_ref_store =ref_store_init (get_git_dir (),REF_STORE_ALL_CAPS ); -return main_ref_store; + r->main_ref_store =ref_store_init (r->gitdir,REF_STORE_ALL_CAPS ); +return r->main_ref_store;I assume that since this takes in a git dir as a parameter that the ref-store is in a good enough place to be embedded in a repository struct (as in this would work with an arbitrary repo)?
Quoted 8 lines
That is my current understanding.
As the refs code can also take a path into a submodule and construct a submodule ref store for the caller, we'd want to resolve the tension between the ref store and the repository struct who is responsible for the submodule ref store eventually by removing the submodule functionality from the ref store and only relying on the ref stores created by the repository struct.
oh right, assuming it can already handle submodule ref-stores it should work as-is then.
Signature · 1 line
Brandon Williams
Re: [PATCH 12/16] refs: store the main ref store inside the repository struct
In reply to Stefan BellerQuoted 61 lines · Stefan Beller
Signed-off-by Stefan Beller <[email protected]>--- refs.c | 13 +++++-------- refs.h | 4 +--- repository.h | 3 +++ 3 files changed, 9 insertions(+), 11 deletions(-)refs.c +5
-8@@ -1608,9 +1608,6 @@ static struct ref_store_hash_entry *alloc_ref_store_hash_entry(return entry; } -/* A pointer to the ref_store for the main repository: */ -static struct ref_store *main_ref_store; -/* A hashmap of ref_stores, stored by submodule name: */ static struct hashmap submodule_ref_stores;@@ -1652,13 +1649,13 @@ static struct ref_store *ref_store_init(const char *gitdir,return refs; } -struct ref_store *get_main_ref_store_the_repository (void ) +struct ref_store *get_main_ref_store (struct repository *r) { -if (main_ref_store) -return main_ref_store; +if (r->main_ref_store) +return r->main_ref_store; - main_ref_store =ref_store_init (get_git_dir (),REF_STORE_ALL_CAPS ); -return main_ref_store; + r->main_ref_store =ref_store_init (r->gitdir,REF_STORE_ALL_CAPS ); +return r->main_ref_store; }/* refs.h +1
-3@@ -760,9 +760,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,int ref_storage_backend_exists (const char *name); -#define get_main_ref_store (r) \ - get_main_ref_store_##r() -struct ref_store *get_main_ref_store_the_repository (void ); +struct ref_store *get_main_ref_store (struct repository *r);/* * Return the ref_store instance for the specified submodule. For the * main repository, use submodule==NULL; such a call cannot fail. For repository.h +0
-0
repository.h struct repository { · line 34
Quoted 9 lines
repository.h +3
-0@@ -26,6 +26,9 @@ struct repository {*/ struct raw_object_store *objects; +/* The store in which the refs are held. */ +struct ref_store *main_ref_store; +/* * Path to the repository's graft file. * Cannot be NULL after initialization.
This also makes sense to me, as far as it goes. I have a few comments and questions:
Why do you call the new member main_ref_store? Is there expected to be some other ref_store associated with a repository?
I think the origin of the name main_ref_store was to distinguish it from submodule ref stores. But presumably those will soon become the "main" ref stores for their respective submodule repository objects, right? So maybe calling things repository.ref_store and get_ref_store(repository) would be appropriate.
There are some places in the reference code that only work with the main repository. The ones that I can think of are:
* ref_resolves_to_object() depends on an object store.
* peel_object() and ref_iterator_peel() also have to look up objects (in this case, tag objects).
* Anything that calls files_assert_main_repository() or depends on REF_STORE_MAIN isn't implemented for other reference stores (usually, I think, these are functions that depend on the object store).
Some of these things might be easy to generalize to non-main repositories, but I didn't bother because AFAIK currently only the main repository store is ever mutated.
You can move a now-obsolete comment above the definition of struct files_ref_store if you haven't in some other patch (search for "libification").
Hope that helps,
Michael
Re: [PATCH 12/16] refs: store the main ref store inside the repository struct
Hi Michael,
Quoted 5 lines · Michael Haggerty
This also makes sense to me, as far as it goes. I have a few comments and questions:
Why do you call the new member
main_ref_store? Is there expected to be some otherref_storeassociated with a repository?
I'll rename it in a reroll.
Quoted 5 lines
I think the origin of the name
main_ref_storewas to distinguish it from submodule ref stores. But presumably those will soon become the "main" ref stores for their respective submodule repository objects, right?
I hope so.
Quoted 2 lines
So maybe calling things
repository.ref_storeandget_ref_store(repository)would be appropriate.
ok.
Quoted 13 lines
There are some places in the reference code that only work with the main repository. The ones that I can think of are:
*
ref_resolves_to_object()depends on an object store.*
peel_object()andref_iterator_peel()also have to look up objects (in this case, tag objects).* Anything that calls
files_assert_main_repository()or depends onREF_STORE_MAINisn't implemented for other reference stores (usually, I think, these are functions that depend on the object store).
Quoted 7 lines
Some of these things might be easy to generalize to non-main repositories, but I didn't bother because AFAIK currently only the main repository store is ever mutated.
You can move a now-obsolete comment above the definition of
struct files_ref_storeif you haven't in some other patch (search for "libification").
ok, I'll have a look at that.
My plan was to remove the submodule accessors from the refs API, and mandate the access via
repo_submodule_init(&submodule_repo, superproject_repo, path);
sub_ref_store = get_ref_store(submodule_repo);instead of also having
sub_ref_store = get_submodule_ref_store(path);
as that would ease the refs API (and its internals potentially) as well as avoiding errors with mixing up repositories. As the construction of a submodule repository struct requires its superproject repo, it helps avoiding pitfalls with nested submodules IMO.
Thanks for the comments,
Stefan
Quoted 3 lines
Hope that helps,
Michael
PATCH v2 13/16 refs: allow for_each_replace_ref to handle arbitrary repositories
In reply to Stefan Beller--- refs.c | 4 ++-- refs.h | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-)
refs.c +2 -2
@@ -1415,9 +1415,9 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,return do_for_each_ref (refs, prefix, fn,0 , flag, cb_data); } -int for_each_replace_ref_the_repository (each_ref_fn fn,void *cb_data) +int for_each_replace_ref (struct repository *r,each_ref_fn fn,void *cb_data) { -return do_for_each_ref (get_main_ref_store (the_repository), +return do_for_each_ref (get_main_ref_store (r), git_replace_ref_base, fn,strlen (git_replace_ref_base),DO_FOR_EACH_INCLUDE_BROKEN , cb_data);
refs.h +1 -3
@@ -300,9 +300,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,int for_each_tag_ref (each_ref_fn fn,void *cb_data);int for_each_branch_ref (each_ref_fn fn,void *cb_data);int for_each_remote_ref (each_ref_fn fn,void *cb_data); -#define for_each_replace_ref (r, fn, cb) \ - for_each_replace_ref_##r(fn, cb) -int for_each_replace_ref_the_repository (each_ref_fn fn,void *cb_data); +int for_each_replace_ref (struct repository *r,each_ref_fn fn,void *cb_data);int for_each_glob_ref (each_ref_fn fn,const char *pattern,void *cb_data);int for_each_glob_ref_in (each_ref_fn fn,const char *pattern,const char *prefix,void *cb_data);
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v2 14/16 replace-object: allow prepare_replace_object to handle arbitrary repositories
In reply to Stefan Beller--- replace-object.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
replace-object.c +5 -7
@@ -31,18 +31,16 @@ static int register_replace_ref(const char *refname,return 0 ; } -#define prepare_replace_object (r) \ - prepare_replace_object_##r() -static void prepare_replace_object_the_repository (void ) +static void prepare_replace_object (struct repository *r) { -if (the_repository->objects->replace_map) +if (r->objects->replace_map)return ; - the_repository->objects->replace_map = + r->objects->replace_map =xmalloc (sizeof (*the_repository->objects->replace_map)); -oidmap_init (the_repository->objects->replace_map,0 ); +oidmap_init (r->objects->replace_map,0 ); -for_each_replace_ref (the_repository, register_replace_ref,NULL ); +for_each_replace_ref (r, register_replace_ref,NULL ); }/* We allow "recursive" replacement. Only within reason, though */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v2 15/16 replace-object: allow do_lookup_replace_object to handle arbitrary repositories
In reply to Stefan Beller--- replace-object.c | 7 ++++--- replace-object.h | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-)
replace-object.c +4 -3
@@ -53,17 +53,18 @@ static void prepare_replace_object(struct repository *r)* permanently-allocated value. This function always respects replace * references, regardless of the value of check_replace_refs. */ -const struct object_id *do_lookup_replace_object_the_repository (const struct object_id *oid) +const struct object_id *do_lookup_replace_object (struct repository *r, +const struct object_id *oid) {int depth =MAXREPLACEDEPTH ;const struct object_id *cur = oid; -prepare_replace_object (the_repository); +prepare_replace_object (r);/* Try to recursively replace the object */ while (depth-- >0 ) {struct replace_object *repl_obj = -oidmap_get (the_repository->objects->replace_map, cur); +oidmap_get (r->objects->replace_map, cur);if (!repl_obj)return cur; cur = &repl_obj->replacement;
replace-object.h +2 -2
@@ -14,8 +14,8 @@ struct replace_object {* This internal function is only declared here for the benefit of * lookup_replace_object(). Please do not call it directly. */ -#define do_lookup_replace_object (r, s) do_lookup_replace_object_##r(s) -extern const struct object_id *do_lookup_replace_object_the_repository (const struct object_id *oid); +extern const struct object_id *do_lookup_replace_object (struct repository *r, +const struct object_id *oid);/* * If object sha1 should be replaced, return the replacement object's
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v2 16/16 replace-object: allow lookup_replace_object to handle arbitrary repositories
In reply to Stefan Beller--- replace-object.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
replace-object.h +5 -5
@@ -23,14 +23,14 @@ extern const struct object_id *do_lookup_replace_object(struct repository *r,* either sha1 or a pointer to a permanently-allocated value. When * object replacement is suppressed, always return sha1. */ -#define lookup_replace_object (r, s) lookup_replace_object_##r(s) -static inline const struct object_id *lookup_replace_object_the_repository (const struct object_id *oid) +static inline const struct object_id *lookup_replace_object (struct repository *r, +const struct object_id *oid) {if (!check_replace_refs || - (the_repository->objects->replace_map && - the_repository->objects->replace_map->map.tablesize ==0 )) + (r->objects->replace_map && + r->objects->replace_map->map.tablesize ==0 ))return oid; -return do_lookup_replace_object (the_repository, oid); +return do_lookup_replace_object (r, oid); }#endif /* REPLACE_OBJECT_H */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
On 04/09, Stefan Beller wrote:
I've looked through the series and it looks good. My only concern is making sure that the ref-store is in a good place to be embedded into the repository struct.
Quoted 81 lines
v2:
This applies on top of a merge of
origin/bc/object-id and origin/sb/packfiles-in-repository, both of which are pending merge to master. It is also available at
https://github.com/stefanbeller/git/tree/object-store-3* removed whitespaces as noted by Stolee
* incorporated Renes patch as the first patch of this series (It may go independently if this series takes too long)
* Adressed Erics concern regarding sloppy commit messages (removed #Conflict markers), typo in comment
* I did not drop themain_from the ref store, yet, as asked by Duy.Thanks,
Stefanv1:
This applies on top of464416a2eaadf84d2bfdf795007863d03b222b7c(sb/packfiles-in-repository).
It is also available at https://github.com/stefanbeller/git/tree/object-store-3This series will bring the replacement mechanism (git replace) into the object store.
The first patches are cleaning up a bit, and patches 6-19 are converting one function at a time using the tick-tock pattern with the #define trick. See
cfc62fc98c(sha1_file: add repository argument tolink_alt_odb_entry, 2018-03-23) for explanation.Thanks,
StefanRené Scharfe (1):
replace_object: use oidmapStefan Beller (15):
replace_object.c: rename to use dash in file name
replace-object: movereplace_mapto object store
object-store: movelookup_replace_objecttoreplace-object.h
replace-object: eliminate replace objects prepared flag
replace-object:check_replace_refsis safe in multi repo environment
refs: add repository argument toget_main_ref_store
refs: add repository argument tofor_each_replace_ref
replace-object: add repository argument toprepare_replace_object
replace-object: add repository argument todo_lookup_replace_object
replace-object: add repository argument tolookup_replace_object
refs: store the main ref store inside the repository struct
refs: allowfor_each_replace_refto handle arbitrary repositories
replace-object: allowprepare_replace_objectto handle arbitrary
repositories
replace-object: allowdo_lookup_replace_objectto handle arbitrary
repositories
replace-object: allowlookup_replace_objectto handle arbitrary
repositoriesMakefile | 2 +- builtin/mktag.c | 3 +- builtin/pack-refs.c | 3 +- builtin/replace.c | 4 +- cache.h | 19 ------ environment.c | 2 +- object-store.h | 8 +++ object.c | 3 +- refs.c | 80 +++++++++++++------------ refs.h | 4 +- replace-object.c | 73 +++++++++++++++++++++++ replace-object.h | 36 ++++++++++++ replace_object.c | 120 -------------------------------------- repository.h | 3 + revision.c | 5 +- sha1_file.c | 7 ++- streaming.c | 3 +- t/helper/test-ref-store.c | 3 +- 18 files changed, 183 insertions(+), 195 deletions(-) create mode 100644 replace-object.c create mode 100644 replace-object.h delete mode 100644 replace_object.cSignature · 1 line
2.17.0.484.g0c8726318c-goog
Signature · 1 line
Brandon Williams
Quoted 6 lines · Brandon Williams
On 04/09, Stefan Beller wrote:
I've looked through the series and it looks good. My only concern is making sure that the ref-store is in a good place to be embedded into the repository struct.
Michael knows more about the ref store.
Could you have a look at (the preparatory)
https://public-inbox.org/git/[email protected]/
and
https://public-inbox.org/git/[email protected]/
Thanks,
Stefan
v3 cover replace_object.c: rename to use dash in file name
In reply to Stefan Bellerv3:
* interdiff below,
the only changes are renaming the variable
- struct ref_store *main_ref_store;
+ struct ref_store *refs;
in struct repository.
as well as dropping the file rename patch.
* improved commit messages from discussion on the single patches.
v2:
This applies on top of a merge of
origin/bc/object-id and origin/sb/packfiles-in-repository, both of which are pending merge to master. It is also available at
https://github.com/stefanbeller/git/tree/object-store-3
* removed whitespaces as noted by Stolee
* incorporated Renes patch as the first patch of this series (It may go independently if this series takes too long)
* Adressed Erics concern regarding sloppy commit messages (removed #Conflict markers), typo in comment
* I did not drop the main_ from the ref store, yet, as asked by Duy.
Thanks,
Stefan
v1:
This applies on top of 464416a2eaadf84d2bfdf795007863d03b222b7c (sb/packfiles-in-repository).
It is also available at https://github.com/stefanbeller/git/tree/object-store-3
This series will bring the replacement mechanism (git replace) into the object store.
The first patches are cleaning up a bit, and patches 6-19 are converting one function at a time using the tick-tock pattern with the #define trick. See cfc62fc98c (sha1_file: add repository argument to link_alt_odb_entry, 2018-03-23) for explanation.
Thanks,
Stefan
René Scharfe (1):
replace_object: use oidmap
Stefan Beller (14):
replace-object: move replace_map to object store
object-store: move lookup_replace_object to replace-object.h
replace-object: eliminate replace objects prepared flag
replace-object: check_replace_refs is safe in multi repo environment
refs: add repository argument to get_main_ref_store
refs: add repository argument to for_each_replace_ref
replace-object: add repository argument to prepare_replace_object
replace-object: add repository argument to do_lookup_replace_object
replace-object: add repository argument to lookup_replace_object
refs: store the main ref store inside the repository struct
refs: allow for_each_replace_ref to handle arbitrary repositories
replace-object: allow prepare_replace_object to handle arbitrary
repositories
replace-object: allow do_lookup_replace_object to handle arbitrary
repositories
replace-object: allow lookup_replace_object to handle arbitrary
repositories
builtin/mktag.c | 3 +- builtin/pack-refs.c | 3 +- builtin/replace.c | 4 +- cache.h | 19 -------- environment.c | 2 +- object-store.h | 8 ++++ object.c | 3 +- refs.c | 80 ++++++++++++++++---------------- refs.h | 4 +- refs/files-backend.c | 4 -- replace-object.h | 36 +++++++++++++++ replace_object.c | 97 ++++++++++----------------------------- repository.h | 3 ++ revision.c | 5 +- sha1_file.c | 7 +-- streaming.c | 3 +- t/helper/test-ref-store.c | 3 +- 17 files changed, 134 insertions(+), 150 deletions(-) create mode 100644 replace-object.h
$ git diff github/object-store-3 diff --git c/Makefile w/Makefile index 94e0bf47b1..de4b8f0c02 100644 --- c/Makefile +++ w/Makefile @@ -871,7 +871,7 @@ LIB_OBJS += refs/packed-backend.o LIB_OBJS += refs/ref-cache.o LIB_OBJS += ref-filter.o LIB_OBJS += remote.o -LIB_OBJS += replace-object.o +LIB_OBJS += replace_object.o LIB_OBJS += repository.o LIB_OBJS += rerere.o LIB_OBJS += resolve-undo.o
refs.c +4 -4
@@ -1651,11 +1651,11 @@ static struct ref_store *ref_store_init(const char *gitdir,struct ref_store *get_main_ref_store (struct repository *r) { -if (r->main_ref_store) -return r->main_ref_store; +if (r->refs) +return r->refs; - r->main_ref_store =ref_store_init (r->gitdir,REF_STORE_ALL_CAPS ); -return r->main_ref_store; + r->refs =ref_store_init (r->gitdir,REF_STORE_ALL_CAPS ); +return r->refs; }/*
refs/files-backend.c +0 -4
@@ -61,10 +61,6 @@ struct ref_lock {struct object_id old_oid; }; -/* -* Future: need to be in "struct repository" -* when doing a full libification. -*/ struct files_ref_store {struct ref_store base;unsigned int store_flags;
replace_object.c renamed from replace-object.c +0 -0
repository.h +1 -1
@@ -27,7 +27,7 @@ struct repository {struct raw_object_store *objects;/* The store in which the refs are held. */ -struct ref_store *main_ref_store; +struct ref_store *refs;/* * Path to the repository's graft file.
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 1/15 replace_object: use oidmap
From: René Scharfe <[email protected]>
Load the replace objects into an oidmap to allow for easy lookups in constant time.
--- replace_object.c | 76 ++++++++++-------------------------------------- 1 file changed, 16 insertions(+), 60 deletions(-)
replace_object.c +16 -60
@@ -1,54 +1,14 @@#include "cache.h" -#include "sha1-lookup.h" +#include "oidmap.h" #include "refs.h" #include "commit.h" -/* -* An array of replacements. The array is kept sorted by the original -* sha1. -*/ -static struct replace_object { -struct object_id original; +struct replace_object { +struct oidmap_entry original;struct object_id replacement; -} **replace_object; - -static int replace_object_alloc, replace_object_nr; +}; -static const unsigned char *replace_sha1_access (size_t index,void *table) -{ -struct replace_object **replace = table; -return replace[index]->original.hash; -} - -static int replace_object_pos (const unsigned char *sha1) -{ -return sha1_pos (sha1, replace_object, replace_object_nr, - replace_sha1_access); -} - -static int register_replace_object (struct replace_object *replace, -int ignore_dups) -{ -int pos =replace_object_pos (replace->original.hash); - -if (0 <= pos) { -if (ignore_dups) -free (replace); -else { -free (replace_object[pos]); - replace_object[pos] = replace; - } -return 1 ; - } - pos = -pos -1 ; -ALLOC_GROW (replace_object, replace_object_nr +1 , replace_object_alloc); - replace_object_nr++; -if (pos < replace_object_nr) -MOVE_ARRAY (replace_object + pos +1 , replace_object + pos, - replace_object_nr - pos -1 ); - replace_object[pos] = replace; -return 0 ; -} +static struct oidmap replace_map =OIDMAP_INIT ; static int register_replace_ref(const char *refname, const struct object_id *oid,
@@ -59,7 +19,7 @@ static int register_replace_ref(const char *refname,const char *hash = slash ? slash +1 : refname;struct replace_object *repl_obj =xmalloc (sizeof (*repl_obj)); -if (get_oid_hex (hash, &repl_obj->original)) { +if (get_oid_hex (hash, &repl_obj->original.oid)) {free (repl_obj);warning ("bad replace ref name: %s" , refname);return 0 ;
@@ -69,7 +29,7 @@ static int register_replace_ref(const char *refname,oidcpy (&repl_obj->replacement, oid);/* Register new object */ -if (register_replace_object (repl_obj,1 )) +if (oidmap_put (&replace_map, repl_obj))die ("duplicate replace ref: %s" , refname);return 0 ;
@@ -84,7 +44,7 @@ static void prepare_replace_object(void)for_each_replace_ref (register_replace_ref,NULL ); replace_object_prepared =1 ; -if (!replace_object_nr) +if (!replace_map.map.tablesize) check_replace_refs =0 ; }
@@ -100,21 +60,17 @@ static void prepare_replace_object(void)*/ const struct object_id *do_lookup_replace_object (const struct object_id *oid) { -int pos, depth =MAXREPLACEDEPTH ; +int depth =MAXREPLACEDEPTH ;const struct object_id *cur = oid;prepare_replace_object ();/* Try to recursively replace the object */ -do { -if (--depth <0 ) -die ("replace depth too high for object %s" , -oid_to_hex (oid)); - - pos =replace_object_pos (cur->hash); -if (0 <= pos) - cur = &replace_object[pos]->replacement; - }while (0 <= pos); - -return cur; +while (depth-- >0 ) { +struct replace_object *repl_obj =oidmap_get (&replace_map, cur); +if (!repl_obj) +return cur; + cur = &repl_obj->replacement; + } +die ("replace depth too high for object %s" ,oid_to_hex (oid)); }
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 2/15 replace-object: move replace_map to object store
In reply to Stefan BellerThe relationship between an object X and another object Y that replaces the object X is defined only within the scope of a single repository.
The exception in reachability rule around these replacement objects is also local to a repository (i.e. if traversal from refs reaches X, then both X and Y are reachable and need to be kept from gc).
--- object-store.h | 8 ++++++++ replace-object.h | 9 +++++++++ replace_object.c | 17 +++++++---------- 3 files changed, 24 insertions(+), 10 deletions(-) create mode 100644 replace-object.h
object-store.h +8 -0
@@ -1,6 +1,8 @@ #ifndef OBJECT_STORE_H #define OBJECT_STORE_H +#include "oidmap.h" + struct alternate_object_database { struct alternate_object_database *next;
@@ -93,6 +95,12 @@ struct raw_object_store {struct alternate_object_database *alt_odb_list;struct alternate_object_database **alt_odb_tail; +/* +* Objects that should be substituted by other objects +* (see git-replace(1)). +*/ +struct oidmap replace_map; +/* * private data *
replace-object.h new file +9 -0
@@ -0,0 +1,9 @@ +#ifndef REPLACE_OBJECT_H +#define REPLACE_OBJECT_H + +struct replace_object { +struct oidmap_entry original; +struct object_id replacement; +}; + +#endif /* REPLACE_OBJECT_H */
replace_object.c +7 -10
@@ -1,15 +1,11 @@#include "cache.h" #include "oidmap.h" +#include "object-store.h" +#include "replace-object.h" #include "refs.h" +#include "repository.h" #include "commit.h" -struct replace_object { -struct oidmap_entry original; -struct object_id replacement; -}; - -static struct oidmap replace_map =OIDMAP_INIT ; -static int register_replace_ref (const char *refname,const struct object_id *oid,int flag,void *cb_data)
@@ -29,7 +25,7 @@ static int register_replace_ref(const char *refname,oidcpy (&repl_obj->replacement, oid);/* Register new object */ -if (oidmap_put (&replace_map, repl_obj)) +if (oidmap_put (&the_repository->objects->replace_map, repl_obj))die ("duplicate replace ref: %s" , refname);return 0 ;
@@ -44,7 +40,7 @@ static void prepare_replace_object(void)for_each_replace_ref (register_replace_ref,NULL ); replace_object_prepared =1 ; -if (!replace_map.map.tablesize) +if (!the_repository->objects->replace_map.map.tablesize) check_replace_refs =0 ; }
@@ -67,7 +63,8 @@ const struct object_id *do_lookup_replace_object(const struct object_id *oid)/* Try to recursively replace the object */ while (depth-- >0 ) { -struct replace_object *repl_obj =oidmap_get (&replace_map, cur); +struct replace_object *repl_obj = +oidmap_get (&the_repository->objects->replace_map, cur);if (!repl_obj)return cur; cur = &repl_obj->replacement;
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 3/15 object-store: move lookup_replace_object to replace-object.h
In reply to Stefan Bellerlookup_replace_object is a low-level function that most users of the object store do not need to use directly.
Move it to replace-object.h to avoid a dependency loop in an upcoming change to its inline definition that will make use of repository.h.
--- builtin/mktag.c | 1 + cache.h | 19 ------------------- object.c | 1 + replace-object.h | 22 ++++++++++++++++++++++ sha1_file.c | 1 + streaming.c | 1 + 6 files changed, 26 insertions(+), 19 deletions(-)
builtin/mktag.c +1 -0
@@ -1,5 +1,6 @@#include "builtin.h" #include "tag.h" +#include "replace-object.h" /* * A signature file has a very simple fixed format: four lines
cache.h +0 -19
@@ -1187,25 +1187,6 @@ static inline void *read_object_file(const struct object_id *oid, enum object_tyreturn read_object_file_extended (oid, type, size,1 ); } -/* -* This internal function is only declared here for the benefit of -* lookup_replace_object(). Please do not call it directly. -*/ -extern const struct object_id *do_lookup_replace_object (const struct object_id *oid); - -/* -* If object sha1 should be replaced, return the replacement object's -* name (replaced recursively, if necessary). The return value is -* either sha1 or a pointer to a permanently-allocated value. When -* object replacement is suppressed, always return sha1. -*/ -static inline const struct object_id *lookup_replace_object (const struct object_id *oid) -{ -if (!check_replace_refs) -return oid; -return do_lookup_replace_object (oid); -} -/* Read and unpack an object file into memory, write memory to an object file */ extern int oid_object_info (const struct object_id *,unsigned long *);
object.c +1 -0
@@ -1,5 +1,6 @@#include "cache.h" #include "object.h" +#include "replace-object.h" #include "blob.h" #include "tree.h" #include "commit.h"
replace-object.h +22 -0
@@ -1,9 +1,31 @@#ifndef REPLACE_OBJECT_H #define REPLACE_OBJECT_H +#include "oidmap.h" +#include "repository.h" +struct replace_object {struct oidmap_entry original;struct object_id replacement; }; +/* +* This internal function is only declared here for the benefit of +* lookup_replace_object(). Please do not call it directly. +*/ +extern const struct object_id *do_lookup_replace_object (const struct object_id *oid); + +/* +* If object sha1 should be replaced, return the replacement object's +* name (replaced recursively, if necessary). The return value is +* either sha1 or a pointer to a permanently-allocated value. When +* object replacement is suppressed, always return sha1. +*/ +static inline const struct object_id *lookup_replace_object (const struct object_id *oid) +{ +if (!check_replace_refs) +return oid; +return do_lookup_replace_object (oid); +} +#endif /* REPLACE_OBJECT_H */
sha1_file.c +1 -0
@@ -23,6 +23,7 @@#include "sha1-lookup.h" #include "bulk-checkin.h" #include "repository.h" +#include "replace-object.h" #include "streaming.h" #include "dir.h" #include "list.h"
streaming.c +1 -0
@@ -5,6 +5,7 @@#include "streaming.h" #include "repository.h" #include "object-store.h" +#include "replace-object.h" #include "packfile.h" enum input_source {
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 4/15 replace-object: eliminate replace objects prepared flag
In reply to Stefan BellerMake the oidmap a pointer.
That way we eliminate the need for the global boolean variable 'replace_object_prepared' as we can put this information into the pointer being NULL or not.
Another advantage of this is that we would more quickly catch code that tries to access replace-map without initializing it.
This also allows the '#include "oidmap.h"' introduced in a previous patch to be replaced by the forward declaration of 'struct oidmap;'. Keeping the type opaque discourages circumventing accessor functions; not dragging in other headers avoids some compile time overhead.
One disadvantage of this is change is performance as we need to pay the overhead for a malloc. The alternative of moving the global variable into the object store is less modular code.
--- object-store.h | 2 +- replace_object.c | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-)
object-store.h +1 -1
@@ -99,7 +99,7 @@ struct raw_object_store {* Objects that should be substituted by other objects * (see git-replace(1)). */ -struct oidmap replace_map; +struct oidmap *replace_map;/* * private data
replace_object.c +9 -7
@@ -25,7 +25,7 @@ static int register_replace_ref(const char *refname,oidcpy (&repl_obj->replacement, oid);/* Register new object */ -if (oidmap_put (&the_repository->objects->replace_map, repl_obj)) +if (oidmap_put (the_repository->objects->replace_map, repl_obj))die ("duplicate replace ref: %s" , refname);return 0 ;
@@ -33,14 +33,16 @@ static int register_replace_ref(const char *refname,static void prepare_replace_object (void ) { -static int replace_object_prepared; - -if (replace_object_prepared) +if (the_repository->objects->replace_map)return ; + the_repository->objects->replace_map = +xmalloc (sizeof (*the_repository->objects->replace_map)); +oidmap_init (the_repository->objects->replace_map,0 ); +for_each_replace_ref (register_replace_ref,NULL ); - replace_object_prepared =1 ; -if (!the_repository->objects->replace_map.map.tablesize) + +if (!the_repository->objects->replace_map->map.tablesize) check_replace_refs =0 ; }
@@ -64,7 +66,7 @@ const struct object_id *do_lookup_replace_object(const struct object_id *oid)/* Try to recursively replace the object */ while (depth-- >0 ) {struct replace_object *repl_obj = -oidmap_get (&the_repository->objects->replace_map, cur); +oidmap_get (the_repository->objects->replace_map, cur);if (!repl_obj)return cur; cur = &repl_obj->replacement;
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 5/15 replace-object: check_replace_refs is safe in multi repo environment
In reply to Stefan BellerIn e1111cef23 (inline lookup_replace_object() calls, 2011-05-15) a shortcut for checking the object replacement was added by setting check_replace_refs to 0 once the replacements were evaluated to not exist. This works fine in with the assumption of only one repository in existence.
The assumption won't hold true any more when we work on multiple instances of a repository structs (e.g. one struct per submodule), as the first repository to be inspected may have no replacements and would set the global variable. Other repositories would then completely omit their evaluation of replacements.
This reverts back the meaning of the flag check_replace_refs of "Do we need to check with the lookup table?" to "Do we need to read the replacement definition?", adding the bypassing logic to lookup_replace_object after the replacement definition was read. As with the original patch, delay the renaming of the global variable
--- environment.c | 2 +- replace-object.h | 5 ++++- replace_object.c | 3 --- 3 files changed, 5 insertions(+), 5 deletions(-)
environment.c +1 -1
@@ -50,7 +50,7 @@ const char *editor_program;const char *askpass_program;const char *excludes_file;enum auto_crlf auto_crlf =AUTO_CRLF_FALSE ; -int check_replace_refs =1 ; +int check_replace_refs =1 ;/* NEEDSWORK: rename to read_replace_refs */ char *git_replace_ref_base;enum eol core_eol =EOL_UNSET ;int global_conv_flags_eol =CONV_EOL_RNDTRP_WARN ;
replace-object.h +4 -1
@@ -3,6 +3,7 @@#include "oidmap.h" #include "repository.h" +#include "object-store.h" struct replace_object {struct oidmap_entry original;
@@ -23,7 +24,9 @@ extern const struct object_id *do_lookup_replace_object(const struct object_id **/ static inline const struct object_id *lookup_replace_object (const struct object_id *oid) { -if (!check_replace_refs) +if (!check_replace_refs || + (the_repository->objects->replace_map && + the_repository->objects->replace_map->map.tablesize ==0 ))return oid;return do_lookup_replace_object (oid); }
replace_object.c +0 -3
@@ -41,9 +41,6 @@ static void prepare_replace_object(void)oidmap_init (the_repository->objects->replace_map,0 );for_each_replace_ref (register_replace_ref,NULL ); - -if (!the_repository->objects->replace_map->map.tablesize) - check_replace_refs =0 ; }/* We allow "recursive" replacement. Only within reason, though */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 6/15 refs: add repository argument to get_main_ref_store
In reply to Stefan BellerAdd a repository argument to allow the get_main_ref_store caller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- builtin/pack-refs.c | 3 +- refs.c | 67 ++++++++++++++++++++------------------- refs.h | 4 ++- revision.c | 5 +-- t/helper/test-ref-store.c | 3 +- 5 files changed, 44 insertions(+), 38 deletions(-)
builtin/pack-refs.c +2 -1
@@ -1,6 +1,7 @@#include "builtin.h" #include "parse-options.h" #include "refs.h" +#include "repository.h" static char const *const pack_refs_usage[] = {N_ ("git pack-refs [<options>]" ),
@@ -17,5 +18,5 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix) };if (parse_options (argc, argv, prefix, opts, pack_refs_usage,0 ))usage_with_options (pack_refs_usage, opts); -return refs_pack_refs (get_main_ref_store (), flags); +return refs_pack_refs (get_main_ref_store (the_repository), flags); }
refs.c +34 -33
269 lines not on this page · raw message
refs.h +3 -1
@@ -758,7 +758,9 @@ int reflog_expire(const char *refname, const struct object_id *oid,int ref_storage_backend_exists (const char *name); -struct ref_store *get_main_ref_store (void ); +#define get_main_ref_store (r) \ + get_main_ref_store_##r() +struct ref_store *get_main_ref_store_the_repository (void );/* * Return the ref_store instance for the specified submodule. For the * main repository, use submodule==NULL; such a call cannot fail. For
revision.c +3 -2
@@ -6,6 +6,7 @@#include "diff.h" #include "refs.h" #include "revision.h" +#include "repository.h" #include "graph.h" #include "grep.h" #include "reflog-walk.h"
@@ -1285,7 +1286,7 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags) cb.all_revs = revs; cb.all_flags = flags; - cb.refs =get_main_ref_store (); + cb.refs =get_main_ref_store (the_repository);for_each_reflog (handle_one_reflog, &cb);if (!revs->single_worktree)
@@ -2176,7 +2177,7 @@ static int handle_revision_pseudo_opt(const char *submodule,die ("BUG: --single-worktree cannot be used together with submodule" ); refs =get_submodule_ref_store (submodule); }else - refs =get_main_ref_store (); + refs =get_main_ref_store (the_repository);/* * NOTE!
t/helper/test-ref-store.c +2 -1
@@ -2,6 +2,7 @@#include "refs.h" #include "worktree.h" #include "object-store.h" +#include "repository.h" static const char *notnull (const char *arg,const char *name) {
@@ -22,7 +23,7 @@ static const char **get_store(const char **argv, struct ref_store **refs)if (!argv[0 ]) {die ("ref store required" ); }else if (!strcmp (argv[0 ],"main" )) { - *refs =get_main_ref_store (); + *refs =get_main_ref_store (the_repository); }else if (skip_prefix (argv[0 ],"submodule:" , &gitdir)) {struct strbuf sb =STRBUF_INIT ;int ret;
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 7/15 refs: add repository argument to for_each_replace_ref
In reply to Stefan BellerAdd a repository argument to allow for_each_replace_ref callers to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- builtin/replace.c | 4 +++- refs.c | 2 +- refs.h | 4 +++- replace_object.c | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-)
builtin/replace.c +3 -1
@@ -14,6 +14,8 @@#include "refs.h" #include "parse-options.h" #include "run-command.h" +#include "object-store.h" +#include "repository.h" #include "tag.h" static const char *const git_replace_usage[] = {
@@ -83,7 +85,7 @@ static int list_replace_refs(const char *pattern, const char *format)"valid formats are 'short', 'medium' and 'long'\n" , format); -for_each_replace_ref (show_reference, (void *)&data); +for_each_replace_ref (the_repository, show_reference, (void *)&data);return 0 ; }
refs.c +1 -1
@@ -1415,7 +1415,7 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,return do_for_each_ref (refs, prefix, fn,0 , flag, cb_data); } -int for_each_replace_ref (each_ref_fn fn,void *cb_data) +int for_each_replace_ref_the_repository (each_ref_fn fn,void *cb_data) { return do_for_each_ref(get_main_ref_store(the_repository), git_replace_ref_base, fn,
refs.h +3 -1
@@ -300,7 +300,9 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,int for_each_tag_ref (each_ref_fn fn,void *cb_data);int for_each_branch_ref (each_ref_fn fn,void *cb_data);int for_each_remote_ref (each_ref_fn fn,void *cb_data); -int for_each_replace_ref (each_ref_fn fn,void *cb_data); +#define for_each_replace_ref (r, fn, cb) \ + for_each_replace_ref_##r(fn, cb) +int for_each_replace_ref_the_repository (each_ref_fn fn,void *cb_data);int for_each_glob_ref (each_ref_fn fn,const char *pattern,void *cb_data);int for_each_glob_ref_in (each_ref_fn fn,const char *pattern,const char *prefix,void *cb_data);
replace_object.c +1 -1
@@ -40,7 +40,7 @@ static void prepare_replace_object(void)xmalloc (sizeof (*the_repository->objects->replace_map));oidmap_init (the_repository->objects->replace_map,0 ); -for_each_replace_ref (register_replace_ref,NULL ); +for_each_replace_ref (the_repository, register_replace_ref,NULL ); }/* We allow "recursive" replacement. Only within reason, though */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 8/15 replace-object: add repository argument to prepare_replace_object
In reply to Stefan BellerAdd a repository argument to allow the prepare_replace_object caller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- replace_object.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
replace_object.c +4 -2
@@ -31,7 +31,9 @@ static int register_replace_ref(const char *refname,return 0 ; } -static void prepare_replace_object (void ) +#define prepare_replace_object (r) \ + prepare_replace_object_##r() +static void prepare_replace_object_the_repository (void ) {if (the_repository->objects->replace_map)return ;
@@ -58,7 +60,7 @@ const struct object_id *do_lookup_replace_object(const struct object_id *oid)int depth =MAXREPLACEDEPTH ;const struct object_id *cur = oid; -prepare_replace_object (); +prepare_replace_object (the_repository);/* Try to recursively replace the object */ while (depth-- >0 ) {
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 9/15 replace-object: add repository argument to do_lookup_replace_object
In reply to Stefan BellerAdd a repository argument to allow the do_lookup_replace_object caller to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- replace-object.h | 5 +++-- replace_object.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-)
replace-object.h +3 -2
@@ -14,7 +14,8 @@ struct replace_object {* This internal function is only declared here for the benefit of * lookup_replace_object(). Please do not call it directly. */ -extern const struct object_id *do_lookup_replace_object (const struct object_id *oid); +#define do_lookup_replace_object (r, s) do_lookup_replace_object_##r(s) +extern const struct object_id *do_lookup_replace_object_the_repository (const struct object_id *oid);/* * If object sha1 should be replaced, return the replacement object's
@@ -28,7 +29,7 @@ static inline const struct object_id *lookup_replace_object(const struct object_ (the_repository->objects->replace_map && the_repository->objects->replace_map->map.tablesize == 0))return oid; - return do_lookup_replace_object(oid); + return do_lookup_replace_object(the_repository, oid); }#endif /* REPLACE_OBJECT_H */
replace_object.c +1 -1
@@ -55,7 +55,7 @@ static void prepare_replace_object_the_repository(void)* permanently-allocated value. This function always respects replace * references, regardless of the value of check_replace_refs. */ -const struct object_id *do_lookup_replace_object (const struct object_id *oid) +const struct object_id *do_lookup_replace_object_the_repository (const struct object_id *oid) {int depth =MAXREPLACEDEPTH ;const struct object_id *cur = oid;
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 10/15 replace-object: add repository argument to lookup_replace_object
In reply to Stefan BellerAdd a repository argument to allow callers of lookup_replace_object to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time.
--- builtin/mktag.c | 2 +- object.c | 2 +- replace-object.h | 3 ++- sha1_file.c | 6 +++--- streaming.c | 2 +- 5 files changed, 8 insertions(+), 7 deletions(-)
builtin/mktag.c +1 -1
@@ -25,7 +25,7 @@ static int verify_object(const struct object_id *oid, const char *expected_type)enum object_type type;unsigned long size;void *buffer =read_object_file (oid, &type, &size); -const struct object_id *repl =lookup_replace_object (oid); +const struct object_id *repl =lookup_replace_object (the_repository, oid);if (buffer) {if (type ==type_from_string (expected_type))
object.c +1 -1
@@ -247,7 +247,7 @@ struct object *parse_object(const struct object_id *oid)unsigned long size;enum object_type type;int eaten; -const struct object_id *repl =lookup_replace_object (oid); +const struct object_id *repl =lookup_replace_object (the_repository, oid);void *buffer;struct object *obj;
replace-object.h +2 -1
@@ -23,7 +23,8 @@ extern const struct object_id *do_lookup_replace_object_the_repository(const str* either sha1 or a pointer to a permanently-allocated value. When * object replacement is suppressed, always return sha1. */ -static inline const struct object_id *lookup_replace_object (const struct object_id *oid) +#define lookup_replace_object (r, s) lookup_replace_object_##r(s) +static inline const struct object_id *lookup_replace_object_the_repository (const struct object_id *oid) { if (!check_replace_refs || (the_repository->objects->replace_map &&
sha1_file.c +3 -3
@@ -1240,7 +1240,7 @@ int oid_object_info_extended(const struct object_id *oid, struct object_info *oiint already_retried =0 ;if (flags &OBJECT_INFO_LOOKUP_REPLACE ) - real =lookup_replace_object (oid); + real =lookup_replace_object (the_repository, oid);if (is_null_oid (real))return -1 ;
@@ -1379,8 +1379,8 @@ void *read_object_file_extended(const struct object_id *oid,const struct packed_git *p;const char *path;struct stat st; -const struct object_id *repl = lookup_replace ?lookup_replace_object (oid) - : oid; +const struct object_id *repl = lookup_replace ? +lookup_replace_object (the_repository, oid) : oid; errno =0 ; data =read_object (repl->hash, type, size);
streaming.c +1 -1
@@ -140,7 +140,7 @@ struct git_istream *open_istream(const struct object_id *oid, {struct git_istream *st;struct object_info oi =OBJECT_INFO_INIT ; -const struct object_id *real =lookup_replace_object (oid); +const struct object_id *real =lookup_replace_object (the_repository, oid);enum input_source src =istream_source (real, type, &oi);if (src <0 )
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 11/15 refs: store the main ref store inside the repository struct
In reply to Stefan BellerThis moves the 'main_ref_store', which was a global variable in refs.c into the repository struct.
This patch does not deal with the parts in the refs subsystem which deal with the submodules there. A later patch needs to get rid of the submodule exposure in the refs API, such as 'get_submodule_ref_store(path)'.
--- refs.c | 13 +++++-------- refs.h | 4 +--- refs/files-backend.c | 4 ---- repository.h | 3 +++ 4 files changed, 9 insertions(+), 15 deletions(-)
refs.c +5 -8
@@ -1608,9 +1608,6 @@ static struct ref_store_hash_entry *alloc_ref_store_hash_entry(return entry; } -/* A pointer to the ref_store for the main repository: */ -static struct ref_store *main_ref_store; -/* A hashmap of ref_stores, stored by submodule name: */ static struct hashmap submodule_ref_stores;
@@ -1652,13 +1649,13 @@ static struct ref_store *ref_store_init(const char *gitdir,return refs; } -struct ref_store *get_main_ref_store_the_repository (void ) +struct ref_store *get_main_ref_store (struct repository *r) { -if (main_ref_store) -return main_ref_store; +if (r->refs) +return r->refs; - main_ref_store =ref_store_init (get_git_dir (),REF_STORE_ALL_CAPS ); -return main_ref_store; + r->refs =ref_store_init (r->gitdir,REF_STORE_ALL_CAPS ); +return r->refs; }/*
refs.h +1 -3
@@ -760,9 +760,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,int ref_storage_backend_exists (const char *name); -#define get_main_ref_store (r) \ - get_main_ref_store_##r() -struct ref_store *get_main_ref_store_the_repository (void ); +struct ref_store *get_main_ref_store (struct repository *r);/* * Return the ref_store instance for the specified submodule. For the * main repository, use submodule==NULL; such a call cannot fail. For
refs/files-backend.c +0 -4
@@ -61,10 +61,6 @@ struct ref_lock {struct object_id old_oid; }; -/* -* Future: need to be in "struct repository" -* when doing a full libification. -*/ struct files_ref_store {struct ref_store base;unsigned int store_flags;
repository.h +3 -0
@@ -26,6 +26,9 @@ struct repository {*/ struct raw_object_store *objects; +/* The store in which the refs are held. */ +struct ref_store *refs; +/* * Path to the repository's graft file. * Cannot be NULL after initialization.
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 12/15 refs: allow for_each_replace_ref to handle arbitrary repositories
In reply to Stefan Beller--- refs.c | 4 ++-- refs.h | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-)
refs.c +2 -2
@@ -1415,9 +1415,9 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,return do_for_each_ref (refs, prefix, fn,0 , flag, cb_data); } -int for_each_replace_ref_the_repository (each_ref_fn fn,void *cb_data) +int for_each_replace_ref (struct repository *r,each_ref_fn fn,void *cb_data) { -return do_for_each_ref (get_main_ref_store (the_repository), +return do_for_each_ref (get_main_ref_store (r), git_replace_ref_base, fn,strlen (git_replace_ref_base),DO_FOR_EACH_INCLUDE_BROKEN , cb_data);
refs.h +1 -3
@@ -300,9 +300,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,int for_each_tag_ref (each_ref_fn fn,void *cb_data);int for_each_branch_ref (each_ref_fn fn,void *cb_data);int for_each_remote_ref (each_ref_fn fn,void *cb_data); -#define for_each_replace_ref (r, fn, cb) \ - for_each_replace_ref_##r(fn, cb) -int for_each_replace_ref_the_repository (each_ref_fn fn,void *cb_data); +int for_each_replace_ref (struct repository *r,each_ref_fn fn,void *cb_data);int for_each_glob_ref (each_ref_fn fn,const char *pattern,void *cb_data);int for_each_glob_ref_in (each_ref_fn fn,const char *pattern,const char *prefix,void *cb_data);
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 13/15 replace-object: allow prepare_replace_object to handle arbitrary repositories
In reply to Stefan Beller--- replace_object.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
replace_object.c +5 -7
@@ -31,18 +31,16 @@ static int register_replace_ref(const char *refname,return 0 ; } -#define prepare_replace_object (r) \ - prepare_replace_object_##r() -static void prepare_replace_object_the_repository (void ) +static void prepare_replace_object (struct repository *r) { -if (the_repository->objects->replace_map) +if (r->objects->replace_map)return ; - the_repository->objects->replace_map = + r->objects->replace_map =xmalloc (sizeof (*the_repository->objects->replace_map)); -oidmap_init (the_repository->objects->replace_map,0 ); +oidmap_init (r->objects->replace_map,0 ); -for_each_replace_ref (the_repository, register_replace_ref,NULL ); +for_each_replace_ref (r, register_replace_ref,NULL ); }/* We allow "recursive" replacement. Only within reason, though */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 14/15 replace-object: allow do_lookup_replace_object to handle arbitrary repositories
In reply to Stefan Beller--- replace-object.h | 4 ++-- replace_object.c | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-)
replace-object.h +2 -2
@@ -14,8 +14,8 @@ struct replace_object {* This internal function is only declared here for the benefit of * lookup_replace_object(). Please do not call it directly. */ -#define do_lookup_replace_object (r, s) do_lookup_replace_object_##r(s) -extern const struct object_id *do_lookup_replace_object_the_repository (const struct object_id *oid); +extern const struct object_id *do_lookup_replace_object (struct repository *r, +const struct object_id *oid);/* * If object sha1 should be replaced, return the replacement object's
replace_object.c +4 -3
@@ -53,17 +53,18 @@ static void prepare_replace_object(struct repository *r)* permanently-allocated value. This function always respects replace * references, regardless of the value of check_replace_refs. */ -const struct object_id *do_lookup_replace_object_the_repository (const struct object_id *oid) +const struct object_id *do_lookup_replace_object (struct repository *r, +const struct object_id *oid) {int depth =MAXREPLACEDEPTH ;const struct object_id *cur = oid; -prepare_replace_object (the_repository); +prepare_replace_object (r);/* Try to recursively replace the object */ while (depth-- >0 ) {struct replace_object *repl_obj = -oidmap_get (the_repository->objects->replace_map, cur); +oidmap_get (r->objects->replace_map, cur);if (!repl_obj)return cur; cur = &repl_obj->replacement;
Signature · 1 line
2.17.0.484.g0c8726318c-goog
PATCH v3 15/15 replace-object: allow lookup_replace_object to handle arbitrary repositories
In reply to Stefan Beller--- replace-object.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
replace-object.h +5 -5
@@ -23,14 +23,14 @@ extern const struct object_id *do_lookup_replace_object(struct repository *r,* either sha1 or a pointer to a permanently-allocated value. When * object replacement is suppressed, always return sha1. */ -#define lookup_replace_object (r, s) lookup_replace_object_##r(s) -static inline const struct object_id *lookup_replace_object_the_repository (const struct object_id *oid) +static inline const struct object_id *lookup_replace_object (struct repository *r, +const struct object_id *oid) {if (!check_replace_refs || - (the_repository->objects->replace_map && - the_repository->objects->replace_map->map.tablesize ==0 )) + (r->objects->replace_map && + r->objects->replace_map->map.tablesize ==0 ))return oid; -return do_lookup_replace_object (the_repository, oid); +return do_lookup_replace_object (r, oid); }#endif /* REPLACE_OBJECT_H */
Signature · 1 line
2.17.0.484.g0c8726318c-goog
Re: [PATCHv3 00/15] replace_object.c: rename to use dash in file name
In reply to Stefan BellerQuoted 8 lines · Stefan Beller
v3:
* interdiff below,
the only changes are renaming the variable
- structref_store*main_ref_store;
+ structref_store*refs;
in struct repository.
as well as dropping the file rename patch.
* improved commit messages from discussion on the single patches.
Looks good to me. Thanks!
-Stolee