Partial clone part 2: fsck and promisors
Subsystems (2)
v5 cover Partial clone part 2: fsck and promisors
From: Jeff Hostetler <[email protected]>
This is V5 of part 2 of partial clone. This assumes V5 of part 1 is already present. V5 includes minor cleanup over V4 and better separates the --exclude-promisor-objects and --missing arguments.
Part 2 is concerned with fsck, gc, initial support for dynamic object fetching, and tracking promisor objects. Jonathan Tan originally developed this code. I have moved it on top of part 1 and updated it slightly.
Jonathan Tan (10):
extension.partialclone: introduce partial clone extension
fsck: introduce partialclone extension
fsck: support refs pointing to promisor objects
fsck: support referenced promisor objects
fsck: support promisor objects as CLI argument
index-pack: refactor writing of .keep files introduce fetch-object: fetch one promisor object
sha1_file: support lazily fetching missing objects
rev-list: support termination at promisor objects
gc: do not repack promisor packfiles
Documentation/git-pack-objects.txt | 11 + Documentation/gitremote-helpers.txt | 6 + Documentation/rev-list-options.txt | 11 + Documentation/technical/repository-version.txt | 12 + Makefile | 1 + builtin/cat-file.c | 2 + builtin/fetch-pack.c | 10 + builtin/fsck.c | 26 +- builtin/gc.c | 3 + builtin/index-pack.c | 113 ++++---- builtin/pack-objects.c | 37 ++- builtin/prune.c | 7 + builtin/repack.c | 8 +- builtin/rev-list.c | 71 ++++- cache.h | 13 +- environment.c | 1 + fetch-object.c | 27 ++ fetch-object.h | 6 + fetch-pack.c | 8 +- fetch-pack.h | 2 + list-objects.c | 29 ++- object.c | 2 +- packfile.c | 77 +++++- packfile.h | 13 + remote-curl.c | 14 +- revision.c | 33 ++- revision.h | 5 +- setup.c | 7 +- sha1_file.c | 38 ++- t/t0410-partial-clone.sh | 343 +++++++++++++++++++++++++ transport.c | 8 + transport.h | 8 + 32 files changed, 869 insertions(+), 83 deletions(-) create mode 100644 fetch-object.c create mode 100644 fetch-object.h create mode 100755 t/t0410-partial-clone.sh
Signature · 1 line
2.9.3
PATCH v5 1/10 extension.partialclone: introduce partial clone extension
From: Jonathan Tan <[email protected]>
Introduce new repository extension option:
extensions.partialclone
See the update to Documentation/technical/repository-version.txt in this patch for more information.
--- Documentation/technical/repository-version.txt | 12 ++++++++++++ cache.h | 2 ++ environment.c | 1 + setup.c | 7 ++++++- 4 files changed, 21 insertions(+), 1 deletion(-)
Documentation/technical/repository-version.txt +12 -0
@@ -86,3 +86,15 @@ for testing format-1 compatibility. When the config key `extensions.preciousObjects` is set to `true`, objects in the repository MUST NOT be deleted (e.g., by `git-prune` or `git repack -d`). + +`partialclone` +~~~~~~~~~~~~~~ + +When the config key `extensions.partialclone` is set, it indicates +that the repo was created with a partial clone (or later performed +a partial fetch) and that the remote may have omitted sending +certain unwanted objects. Such a remote is called a "promisor remote" +and it promises that all such omitted objects can be fetched from it +in the future. + +The value of this key is the name of the promisor remote.
cache.h +2 -0
@@ -860,10 +860,12 @@ extern int grafts_replace_parents;#define GIT_REPO_VERSION 0#define GIT_REPO_VERSION_READ 1extern int repository_format_precious_objects; +extern char *repository_format_partial_clone;struct repository_format {int version;int precious_objects; +char *partial_clone;/* value of extensions.partialclone */ int is_bare;char *work_tree;struct string_list unknown_extensions;
environment.c +1 -0
@@ -27,6 +27,7 @@ int warn_ambiguous_refs = 1;int warn_on_object_refname_ambiguity =1 ;int ref_paranoia =-1 ;int repository_format_precious_objects; +char *repository_format_partial_clone;const char *git_commit_encoding;const char *git_log_output_encoding;const char *apply_default_whitespace;
setup.c +6 -1
@@ -420,7 +420,11 @@ static int check_repo_format(const char *var, const char *value, void *vdata) ; else if (!strcmp(ext, "preciousobjects")) data->precious_objects =git_config_bool (var, value); - else + else if (!strcmp(ext, "partialclone")) { +if (!value) +return config_error_nonbool (var); + data->partial_clone =xstrdup (value); + } else string_list_append(&data->unknown_extensions, ext); }else if (strcmp (var,"core.bare" ) ==0 ) { data->is_bare =git_config_bool (var, value);
@@ -463,6 +467,7 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok) } repository_format_precious_objects = candidate.precious_objects; + repository_format_partial_clone = candidate.partial_clone;string_list_clear (&candidate.unknown_extensions,0 );if (!has_common) {if (candidate.is_bare !=-1 ) {
Signature · 1 line
2.9.3
PATCH v5 2/10 fsck: introduce partialclone extension
In reply to Jeff HostetlerFrom: Jonathan Tan <[email protected]>
Currently, Git does not support repos with very large numbers of objects or repos that wish to minimize manipulation of certain blobs (for example, because they are very large) very well, even if the user operates mostly on part of the repo, because Git is designed on the assumption that every referenced object is available somewhere in the repo storage. In such an arrangement, the full set of objects is usually available in remote storage, ready to be lazily downloaded.
Teach fsck about the new state of affairs. In this commit, teach fsck that missing promisor objects referenced from the reflog are not an error case; in future commits, fsck will be taught about other cases.
--- builtin/fsck.c | 2 +- cache.h | 3 +- packfile.c | 77 +++++++++++++++++++++++++++++++++++++++++++-- packfile.h | 13 ++++++++ t/t0410-partial-clone.sh | 81 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 171 insertions(+), 5 deletions(-) create mode 100755 t/t0410-partial-clone.sh
builtin/fsck.c +1 -1
@@ -398,7 +398,7 @@ static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid, xstrfmt("%s@{%"PRItime"}", refname, timestamp)); obj->flags |=USED ;mark_object_reachable (obj); - }else { + }else if (!is_promisor_object (oid)) {error ("%s: invalid reflog entry %s" , refname,oid_to_hex (oid)); errors_found |=ERROR_REACHABLE ; }
cache.h +2 -1
@@ -1587,7 +1587,8 @@ extern struct packed_git { unsigned pack_local:1, pack_keep:1, freshened:1, - do_not_close:1; + do_not_close:1, + pack_promisor:1;unsigned char sha1[20 ];struct revindex_entry *revindex;/* something like ".git/objects/pack/xxxxx.pack" */
packfile.c +74 -3
112 lines not on this page · raw message
packfile.h +13 -0
@@ -1,6 +1,8 @@#ifndef PACKFILE_H #define PACKFILE_H +#include "oidset.h" +/* * Generate the filename to be used for a pack file with checksum "sha1" and * extension "ext". The result is written into the strbuf "buf", overwriting
@@ -125,6 +127,11 @@ extern int has_sha1_pack(const unsigned char *sha1);extern int has_pack_index (const unsigned char *sha1);/* +* Only iterate over packs obtained from the promisor remote. +*/ +#define FOR_EACH_OBJECT_PROMISOR_ONLY 2 + +/* * Iterate over packed objects in both the local * repository and any alternates repositories (unless the * FOR_EACH_OBJECT_LOCAL_ONLY flag, defined in cache.h, is set).
@@ -135,4 +142,10 @@ typedef int each_packed_object_fn(const struct object_id *oid, void *data);extern int for_each_packed_object (each_packed_object_fn ,void *,unsigned flags); +/* +* Return 1 if an object in a promisor packfile is or refers to the given +* object, 0 otherwise. +*/ +extern int is_promisor_object (const struct object_id *oid); +#endif
t/t0410-partial-clone.sh new file +81 -0
81 lines not on this page · raw message
Signature · 1 line
2.9.3
PATCH v5 3/10 fsck: support refs pointing to promisor objects
In reply to Jeff HostetlerFrom: Jonathan Tan <[email protected]>
Teach fsck to not treat refs referring to missing promisor objects as an error when extensions.partialclone is set.
For the purposes of warning about no default refs, such refs are still treated as legitimate refs.
--- builtin/fsck.c | 8 ++++++++ t/t0410-partial-clone.sh | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+)
builtin/fsck.c +8 -0
@@ -434,6 +434,14 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid, obj =parse_object (oid);if (!obj) { +if (is_promisor_object (oid)) { +/* +* Increment default_refs anyway, because this is a +* valid ref. +*/ + default_refs++; +return 0 ; + }error ("%s: invalid sha1 pointer %s" , refname,oid_to_hex (oid)); errors_found |=ERROR_REACHABLE ;/* We'll continue with the rest despite the error.. */
t/t0410-partial-clone.sh +24 -0
@@ -13,6 +13,14 @@ pack_as_from_promisor () { >repo/.git/objects/pack/pack-$HASH.promisor } +promise_and_delete () { + HASH=$(git -C repo rev-parse "$1") && + git -C repo tag -a -m message my_annotated_tag "$HASH" && + git -C repo rev-parse my_annotated_tag | pack_as_from_promisor && + git -C repo tag -d my_annotated_tag && + delete_object repo "$HASH" +} + test_expect_success 'missing reflog object, but promised by a commit, passes fsck' ' test_create_repo repo && test_commit -C repo my_commit &&
@@ -78,4 +86,20 @@ test_expect_success 'missing reflog object alone fails fsck, even with extension test_must_fail git -C repo fsck ' +test_expect_success 'missing ref object, but promised, passes fsck' ' + rm -rf repo && + test_create_repo repo && + test_commit -C repo my_commit && + + A=$(git -C repo commit-tree -m a HEAD^{tree}) && + + # Reference $A only from ref + git -C repo branch my_branch "$A" && + promise_and_delete "$A" && + + git -C repo config core.repositoryformatversion 1 && + git -C repo config extensions.partialclone "arbitrary string" && + git -C repo fsck +' + test_done
Signature · 1 line
2.9.3
PATCH v5 4/10 fsck: support referenced promisor objects
In reply to Jeff HostetlerFrom: Jonathan Tan <[email protected]>
Teach fsck to not treat missing promisor objects indirectly pointed to by refs as an error when extensions.partialclone is set.
--- builtin/fsck.c | 11 +++++++++++ t/t0410-partial-clone.sh | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+)
builtin/fsck.c +11 -0
@@ -149,6 +149,15 @@ static int mark_object(struct object *obj, int type, void *data, struct fsck_optif (obj->flags &REACHABLE )return 0 ; obj->flags |=REACHABLE ; + +if (is_promisor_object (&obj->oid)) +/* +* Further recursion does not need to be performed on this +* object since it is a promisor object (so it does not need to +* be added to "pending"). +*/ +return 0 ; +if (!(obj->flags &HAS_OBJ )) {if (parent && !has_object_file (&obj->oid)) { printf("broken link from %7s %s\n",
@@ -208,6 +217,8 @@ static void check_reachable_object(struct object *obj)* do a full fsck */ if (!(obj->flags &HAS_OBJ )) { +if (is_promisor_object (&obj->oid)) +return ;if (has_sha1_pack (obj->oid.hash))return ;/* it is in pack - forget about it */ printf("missing %s %s\n", printable_type(obj),
t/t0410-partial-clone.sh +23 -0
@@ -102,4 +102,27 @@ test_expect_success 'missing ref object, but promised, passes fsck' ' git -C repo fsck ' +test_expect_success 'missing object, but promised, passes fsck' ' + rm -rf repo && + test_create_repo repo && + test_commit -C repo 1 && + test_commit -C repo 2 && + test_commit -C repo 3 && + git -C repo tag -a annotated_tag -m "annotated tag" && + + C=$(git -C repo rev-parse 1) && + T=$(git -C repo rev-parse 2^{tree}) && + B=$(git hash-object repo/3.t) && + AT=$(git -C repo rev-parse annotated_tag) && + + promise_and_delete "$C" && + promise_and_delete "$T" && + promise_and_delete "$B" && + promise_and_delete "$AT" && + + git -C repo config core.repositoryformatversion 1 && + git -C repo config extensions.partialclone "arbitrary string" && + git -C repo fsck +' + test_done
Signature · 1 line
2.9.3
PATCH v5 5/10 fsck: support promisor objects as CLI argument
In reply to Jeff HostetlerFrom: Jonathan Tan <[email protected]>
Teach fsck to not treat missing promisor objects provided on the CLI as an error when extensions.partialclone is set.
--- builtin/fsck.c | 2 ++ t/t0410-partial-clone.sh | 13 +++++++++++++ 2 files changed, 15 insertions(+)
builtin/fsck.c +2 -0
@@ -750,6 +750,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)struct object *obj =lookup_object (oid.hash);if (!obj || !(obj->flags &HAS_OBJ )) { +if (is_promisor_object (&oid)) +continue ;error ("%s: object missing" ,oid_to_hex (&oid)); errors_found |=ERROR_OBJECT ;continue ;
t/t0410-partial-clone.sh +13 -0
@@ -125,4 +125,17 @@ test_expect_success 'missing object, but promised, passes fsck' ' git -C repo fsck ' +test_expect_success 'missing CLI object, but promised, passes fsck' ' + rm -rf repo && + test_create_repo repo && + test_commit -C repo my_commit && + + A=$(git -C repo commit-tree -m a HEAD^{tree}) && + promise_and_delete "$A" && + + git -C repo config core.repositoryformatversion 1 && + git -C repo config extensions.partialclone "arbitrary string" && + git -C repo fsck "$A" +' + test_done
Signature · 1 line
2.9.3
PATCH v5 6/10 index-pack: refactor writing of .keep files
In reply to Jeff HostetlerFrom: Jonathan Tan <[email protected]>
In a subsequent commit, index-pack will be taught to write ".promisor" files which are similar to the ".keep" files it knows how to write. Refactor the writing of ".keep" files, so that the implementation of writing ".promisor" files becomes easier.
--- builtin/index-pack.c | 99 ++++++++++++++++++++++++++++------------------------ 1 file changed, 53 insertions(+), 46 deletions(-)
builtin/index-pack.c +53 -46
@@ -1389,15 +1389,58 @@ static void fix_unresolved_deltas(struct sha1file *f)free (sorted_by_pos); } +static const char *derive_filename (const char *pack_name,const char *suffix, +struct strbuf *buf) +{ +size_t len; +if (!strip_suffix (pack_name,".pack" , &len)) +die (_ ("packfile name '%s' does not end with '.pack'" ), + pack_name); +strbuf_add (buf, pack_name, len); +strbuf_addch (buf,'.' ); +strbuf_addstr (buf, suffix); +return buf->buf; +} + +static void write_special_file (const char *suffix,const char *msg, +const char *pack_name,const unsigned char *sha1, +const char **report) +{ +struct strbuf name_buf =STRBUF_INIT ; +const char *filename; +int fd; +int msg_len =strlen (msg); + +if (pack_name) + filename =derive_filename (pack_name, suffix, &name_buf); +else + filename =odb_pack_name (&name_buf, sha1, suffix); + + fd =odb_pack_keep (filename); +if (fd <0 ) { +if (errno !=EEXIST ) +die_errno (_ ("cannot write %s file '%s'" ), + suffix, filename); + }else { +if (msg_len >0 ) { +write_or_die (fd, msg, msg_len); +write_or_die (fd,"\n" ,1 ); + } +if (close (fd) !=0 ) +die_errno (_ ("cannot close written %s file '%s'" ), + suffix, filename); + *report = suffix; + } +strbuf_release (&name_buf); +} +static void final (const char *final_pack_name,const char *curr_pack_name,const char *final_index_name,const char *curr_index_name, -const char *keep_name,const char *keep_msg, -unsigned char *sha1) +const char *keep_msg,unsigned char *sha1) {const char *report ="pack" ;struct strbuf pack_name =STRBUF_INIT ;struct strbuf index_name =STRBUF_INIT ; -struct strbuf keep_name_buf =STRBUF_INIT ;int err;if (!from_stdin) {
@@ -1409,28 +1452,9 @@ static void final(const char *final_pack_name, const char *curr_pack_name,die_errno (_ ("error while closing pack file" )); } -if (keep_msg) { -int keep_fd, keep_msg_len =strlen (keep_msg); - -if (!keep_name) - keep_name =odb_pack_name (&keep_name_buf, sha1,"keep" ); - - keep_fd =odb_pack_keep (keep_name); -if (keep_fd <0 ) { -if (errno !=EEXIST ) -die_errno (_ ("cannot write keep file '%s'" ), - keep_name); - }else { -if (keep_msg_len >0 ) { -write_or_die (keep_fd, keep_msg, keep_msg_len); -write_or_die (keep_fd,"\n" ,1 ); - } -if (close (keep_fd) !=0 ) -die_errno (_ ("cannot close written keep file '%s'" ), - keep_name); - report ="keep" ; - } - } +if (keep_msg) +write_special_file ("keep" , keep_msg, final_pack_name, sha1, + &report);if (final_pack_name != curr_pack_name) {if (!final_pack_name)
@@ -1472,7 +1496,6 @@ static void final(const char *final_pack_name, const char *curr_pack_name,strbuf_release (&index_name);strbuf_release (&pack_name); -strbuf_release (&keep_name_buf); }static int git_index_pack_config (const char *k,const char *v,void *cb)
@@ -1615,26 +1638,13 @@ static void show_pack_info(int stat_only) } } -static const char *derive_filename (const char *pack_name,const char *suffix, -struct strbuf *buf) -{ -size_t len; -if (!strip_suffix (pack_name,".pack" , &len)) -die (_ ("packfile name '%s' does not end with '.pack'" ), - pack_name); -strbuf_add (buf, pack_name, len); -strbuf_addstr (buf, suffix); -return buf->buf; -} -int cmd_index_pack (int argc,const char **argv,const char *prefix) {int i, fix_thin_pack =0 , verify =0 , stat_only =0 ;const char *curr_index;const char *index_name =NULL , *pack_name =NULL ; -const char *keep_name =NULL , *keep_msg =NULL ; -struct strbuf index_name_buf =STRBUF_INIT , - keep_name_buf =STRBUF_INIT ; +const char *keep_msg =NULL ; +struct strbuf index_name_buf =STRBUF_INIT ;struct pack_idx_entry **idx_objects;struct pack_idx_option opts;unsigned char pack_sha1[20 ];
@@ -1745,9 +1755,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)if (from_stdin && !startup_info->have_repository)die (_ ("--stdin requires a git repository" ));if (!index_name && pack_name) - index_name =derive_filename (pack_name,".idx" , &index_name_buf); -if (keep_msg && !keep_name && pack_name) - keep_name =derive_filename (pack_name,".keep" , &keep_name_buf); + index_name =derive_filename (pack_name,"idx" , &index_name_buf);if (verify) {if (!index_name)
@@ -1795,13 +1803,12 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)if (!verify)final (pack_name, curr_pack, index_name, curr_index, - keep_name, keep_msg, + keep_msg, pack_sha1);else close (input_fd);free (objects);strbuf_release (&index_name_buf); -strbuf_release (&keep_name_buf);if (pack_name ==NULL )free ((void *) curr_pack);if (index_name ==NULL )
Signature · 1 line
2.9.3
PATCH v5 7/10 introduce fetch-object: fetch one promisor object
In reply to Jeff HostetlerFrom: Jonathan Tan <[email protected]>
Introduce fetch-object, providing the ability to fetch one object from a promisor remote.
This uses fetch-pack. To do this, the transport mechanism has been updated with 2 flags, "from-promisor" to indicate that the resulting pack comes from a promisor remote (and thus should be annotated as such by index-pack), and "no-haves" to suppress the sending of "have" lines.
This will be tested in a subsequent commit.
NEEDSWORK: update this when we have more information about protocol v2, which should allow a way to suppress the ref advertisement and officially allow any object type to be "want"-ed.
--- Documentation/gitremote-helpers.txt | 6 ++++++ Makefile | 1 + builtin/fetch-pack.c | 8 ++++++++ builtin/index-pack.c | 16 +++++++++++++--- fetch-object.c | 24 ++++++++++++++++++++++++ fetch-object.h | 6 ++++++ fetch-pack.c | 8 ++++++-- fetch-pack.h | 2 ++ remote-curl.c | 14 +++++++++++++- transport.c | 8 ++++++++ transport.h | 8 ++++++++ 11 files changed, 95 insertions(+), 6 deletions(-) create mode 100644 fetch-object.c create mode 100644 fetch-object.h
Documentation/gitremote-helpers.txt +6 -0
@@ -466,6 +466,12 @@ set by Git if the remote helper has the 'option' capability. Transmit <string> as a push option. As the push option must not contain LF or NUL characters, the string is not encoded. +'option from-promisor' {'true'|'false'}:: + Indicate that these objects are being fetched from a promisor. + +'option no-haves' {'true'|'false'}:: + Do not send "have" lines. + SEE ALSO -------- linkgit:git-remote[1]
Makefile +1 -0
@@ -792,6 +792,7 @@ 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 += fetch-object.o LIB_OBJS += fetch-pack.o LIB_OBJS += fsck.o LIB_OBJS += gettext.o
builtin/fetch-pack.c +8 -0
@@ -143,6 +143,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) args.update_shallow =1 ;continue ; } +if (!strcmp ("--from-promisor" , arg)) { + args.from_promisor =1 ; +continue ; + } +if (!strcmp ("--no-haves" , arg)) { + args.no_haves =1 ; +continue ; + }usage (fetch_pack_usage); }if (deepen_not.nr)
builtin/index-pack.c +13 -3
@@ -1429,14 +1429,16 @@ static void write_special_file(const char *suffix, const char *msg,if (close (fd) !=0 )die_errno (_ ("cannot close written %s file '%s'" ), suffix, filename); - *report = suffix; +if (report) + *report = suffix; }strbuf_release (&name_buf); }static void final (const char *final_pack_name,const char *curr_pack_name,const char *final_index_name,const char *curr_index_name, -const char *keep_msg,unsigned char *sha1) +const char *keep_msg,const char *promisor_msg, +unsigned char *sha1) {const char *report ="pack" ;struct strbuf pack_name =STRBUF_INIT ;
@@ -1455,6 +1457,9 @@ static void final(const char *final_pack_name, const char *curr_pack_name,if (keep_msg)write_special_file ("keep" , keep_msg, final_pack_name, sha1, &report); +if (promisor_msg) +write_special_file ("promisor" , promisor_msg, final_pack_name, + sha1,NULL );if (final_pack_name != curr_pack_name) {if (!final_pack_name)
@@ -1644,6 +1649,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)const char *curr_index;const char *index_name =NULL , *pack_name =NULL ;const char *keep_msg =NULL ; +const char *promisor_msg =NULL ;struct strbuf index_name_buf =STRBUF_INIT ;struct pack_idx_entry **idx_objects;struct pack_idx_option opts;
@@ -1693,6 +1699,10 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) keep_msg ="" ; }else if (starts_with (arg,"--keep=" )) { keep_msg = arg +7 ; + }else if (!strcmp (arg,"--promisor" )) { + promisor_msg ="" ; + }else if (starts_with (arg,"--promisor=" )) { + promisor_msg = arg +strlen ("--promisor=" ); }else if (starts_with (arg,"--threads=" )) {char *end; nr_threads =strtoul (arg+10 , &end,0 );
@@ -1803,7 +1813,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)if (!verify)final (pack_name, curr_pack, index_name, curr_index, - keep_msg, + keep_msg, promisor_msg, pack_sha1);else close (input_fd);
fetch-object.c new file +24 -0
@@ -0,0 +1,24 @@ +#include "cache.h" +#include "packfile.h" +#include "pkt-line.h" +#include "strbuf.h" +#include "transport.h" +#include "fetch-object.h" + +void fetch_object (const char *remote_name,const unsigned char *sha1) +{ +struct remote *remote; +struct transport *transport; +struct ref *ref; + + remote =remote_get (remote_name); +if (!remote->url[0 ]) +die (_ ("Remote with no URL" )); + transport =transport_get (remote, remote->url[0 ]); + + ref =alloc_ref (sha1_to_hex (sha1)); +hashcpy (ref->old_oid.hash, sha1); +transport_set_option (transport,TRANS_OPT_FROM_PROMISOR ,"1" ); +transport_set_option (transport,TRANS_OPT_NO_HAVES ,"1" ); +transport_fetch_refs (transport, ref); +}
fetch-object.h new file +6 -0
@@ -0,0 +1,6 @@ +#ifndef FETCH_OBJECT_H +#define FETCH_OBJECT_H + +extern void fetch_object (const char *remote_name,const unsigned char *sha1); + +#endif
fetch-pack.c +6 -2
@@ -450,6 +450,8 @@ static int find_common(struct fetch_pack_args *args, flushes =0 ; retval =-1 ; +if (args->no_haves) + goto done;while ((oid =get_rev ())) {packet_buf_write (&req_buf,"have %s\n" ,oid_to_hex (oid));print_verbose (args,"have %s" ,oid_to_hex (oid));
@@ -832,7 +834,7 @@ static int get_pack(struct fetch_pack_args *args,argv_array_push (&cmd.args, alternate_shallow_file); } -if (do_keep) { +if (do_keep || args->from_promisor) {if (pack_lockfile) cmd.out =-1 ; cmd_name ="index-pack" ;
@@ -842,7 +844,7 @@ static int get_pack(struct fetch_pack_args *args,argv_array_push (&cmd.args,"-v" );if (args->use_thin_pack)argv_array_push (&cmd.args,"--fix-thin" ); -if (args->lock_pack || unpack_limit) { +if (do_keep && (args->lock_pack || unpack_limit)) {char hostname[HOST_NAME_MAX +1 ];if (xgethostname (hostname,sizeof (hostname)))xsnprintf (hostname,sizeof (hostname),"localhost" );
@@ -852,6 +854,8 @@ static int get_pack(struct fetch_pack_args *args, }if (args->check_self_contained_and_connected)argv_array_push (&cmd.args,"--check-self-contained-and-connected" ); +if (args->from_promisor) +argv_array_push (&cmd.args,"--promisor" ); }else { cmd_name ="unpack-objects" ;
fetch-pack.h +2 -0
@@ -29,6 +29,8 @@ struct fetch_pack_args { unsigned cloning:1; unsigned update_shallow:1; unsigned deepen:1; + unsigned from_promisor:1; + unsigned no_haves:1; };/*
remote-curl.c +13 -1
@@ -33,7 +33,9 @@ struct options { thin : 1, /* One of the SEND_PACK_PUSH_CERT_* constants. */ push_cert : 2, - deepen_relative : 1; + deepen_relative : 1, + from_promisor : 1, + no_haves : 1; };static struct options options;static struct string_list cas_options =STRING_LIST_INIT_DUP ;
@@ -157,6 +159,12 @@ static int set_option(const char *name, const char *value)return -1 ;return 0 ;#endif /* LIBCURL_VERSION_NUM >= 0x070a08 */ + }else if (!strcmp (name,"from-promisor" )) { + options.from_promisor =1 ; +return 0 ; + }else if (!strcmp (name,"no-haves" )) { + options.no_haves =1 ; +return 0 ; }else {return 1 /* unsupported */ ; }
@@ -822,6 +830,10 @@ static int fetch_git(struct discovery *heads, options.deepen_not.items[i].string);if (options.deepen_relative && options.depth)argv_array_push (&args,"--deepen-relative" ); +if (options.from_promisor) +argv_array_push (&args,"--from-promisor" ); +if (options.no_haves) +argv_array_push (&args,"--no-haves" );argv_array_push (&args, url.buf);for (i =0 ; i < nr_heads; i++) {
transport.c +8 -0
@@ -160,6 +160,12 @@ static int set_git_option(struct git_transport_options *opts, }else if (!strcmp (name,TRANS_OPT_DEEPEN_RELATIVE )) { opts->deepen_relative = !!value;return 0 ; + }else if (!strcmp (name,TRANS_OPT_FROM_PROMISOR )) { + opts->from_promisor = !!value; +return 0 ; + }else if (!strcmp (name,TRANS_OPT_NO_HAVES )) { + opts->no_haves = !!value; +return 0 ; }return 1 ; }
@@ -228,6 +234,8 @@ static int fetch_refs_via_pack(struct transport *transport, data->options.check_self_contained_and_connected; args.cloning = transport->cloning; args.update_shallow = data->options.update_shallow; + args.from_promisor = data->options.from_promisor; + args.no_haves = data->options.no_haves;if (!data->got_remote_heads) {connect_setup (transport,0 );
transport.h +8 -0
@@ -15,6 +15,8 @@ struct git_transport_options { unsigned self_contained_and_connected : 1; unsigned update_shallow : 1; unsigned deepen_relative : 1; + unsigned from_promisor : 1; + unsigned no_haves : 1;int depth;const char *deepen_since;const struct string_list *deepen_not;
@@ -210,6 +212,12 @@ void transport_check_allowed(const char *type);/* Send push certificates */ #define TRANS_OPT_PUSH_CERT "pushcert" +/* Indicate that these objects are being fetched by a promisor */ +#define TRANS_OPT_FROM_PROMISOR "from-promisor" + +/* Do not send "have" lines */ +#define TRANS_OPT_NO_HAVES "no-haves" +/** * Returns 0 if the option was used, non-zero otherwise. Prints a * message to stderr if the option is not used.
Signature · 1 line
2.9.3
Re: [PATCH v5 07/10] introduce fetch-object: fetch one promisor object
Quoted 9 lines · Jeff Hostetler
From: Jonathan Tan <[email protected]>
fetch-object.c new file +7
-0+void fetch_object (const char *remote_name,const unsigned char *sha1) +{ +struct remote *remote; +struct transport *transport; +struct ref *ref; + + remote =remote_get (remote_name);
fetch-object.c line 24
Quoted 10 lines
fetch-object.c new file +10
-0+if (!remote->url[0 ]) +die (_ ("Remote with no URL" )); + transport =transport_get (remote, remote->url[0 ]); + + ref =alloc_ref (sha1_to_hex (sha1)); +hashcpy (ref->old_oid.hash, sha1); +transport_set_option (transport,TRANS_OPT_FROM_PROMISOR ,"1" ); +transport_set_option (transport,TRANS_OPT_NO_HAVES ,"1" ); +transport_fetch_refs (transport, ref); +}
I think it would be interesting to return what transport_fetch_refs() returns, so that a caller could know if an error happened.
Re: [PATCH v5 07/10] introduce fetch-object: fetch one promisor object
Quoted 15 lines · Christian Couder
Quoted 14 lines · Jeff Hostetler
From: Jonathan Tan <[email protected]>
fetch-object.c new file +12
-0+void fetch_object (const char *remote_name,const unsigned char *sha1) +{ +struct remote *remote; +struct transport *transport; +struct ref *ref; + + remote =remote_get (remote_name); +if (!remote->url[0 ]) +die (_ ("Remote with no URL" )); + transport =transport_get (remote, remote->url[0 ]); + + ref =alloc_ref (sha1_to_hex (sha1));
Quoted 9 lines
fetch-object.c line 24
Quoted 5 lines
fetch-object.c new file +5
-0+hashcpy (ref->old_oid.hash, sha1); +transport_set_option (transport,TRANS_OPT_FROM_PROMISOR ,"1" ); +transport_set_option (transport,TRANS_OPT_NO_HAVES ,"1" ); +transport_fetch_refs (transport, ref); +}I think it would be interesting to return what
transport_fetch_refs()returns, so that a caller could know if an error happened.
That might help the retry/found_packed loop mentioned in your response on the next patch in this series.
I'll make a TODO item for this to investigate.
Thanks
Jeff
PATCH v5 8/10 sha1_file: support lazily fetching missing objects
In reply to Jeff HostetlerFrom: Jonathan Tan <[email protected]>
Teach sha1_file to fetch objects from the remote configured in extensions.partialclone whenever an object is requested but missing.
The fetching of objects can be suppressed through a global variable. This is used by fsck and index-pack.
However, by default, such fetching is not suppressed. This is meant as a temporary measure to ensure that all Git commands work in such a situation. Future patches will update some commands to either tolerate missing objects (without fetching them) or be more efficient in fetching them.
In order to determine the code changes in sha1_file.c necessary, I investigated the following:
(1) functions in sha1_file.c that take in a hash, without the user regarding how the object is stored (loose or packed)
(2) functions in packfile.c (because I need to check callers that know about the loose/packed distinction and operate on both differently, and ensure that they can handle the concept of objects that are neither loose nor packed)
(1) is handled by the modification to sha1_object_info_extended().
For (2), I looked at for_each_packed_object and others. For for_each_packed_object, the callers either already work or are fixed in this patch:
- reachable - only to find recent objects
- builtin/fsck - already knows about missing objects
- builtin/cat-file - warning message added in this commit
Callers of the other functions do not need to be changed:
- parse_pack_index
- http - indirectly from http_get_info_packs
- find_pack_entry_one
- this searches a single pack that is provided as an argument; the caller already knows (through other means) that the sought object is in a specific pack
- find_sha1_pack
- fast-import - appears to be an optimization to not store a file if it is already in a pack
- http-walker - to search through a struct alt_base
- http-push - to search through remote packs
- has_sha1_pack
- builtin/fsck - already knows about promisor objects
- builtin/count-objects - informational purposes only (check if loose object is also packed)
- builtin/prune-packed - check if object to be pruned is packed (if not, don't prune it)
- revision - used to exclude packed objects if requested by user
- diff - just for optimization
--- builtin/cat-file.c | 2 ++ builtin/fetch-pack.c | 2 ++ builtin/fsck.c | 3 +++ builtin/index-pack.c | 6 ++++++ cache.h | 8 ++++++++ fetch-object.c | 3 +++ sha1_file.c | 38 ++++++++++++++++++++++++------------ t/t0410-partial-clone.sh | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 100 insertions(+), 13 deletions(-)
builtin/cat-file.c +2 -0
@@ -475,6 +475,8 @@ static int batch_objects(struct batch_options *opt)for_each_loose_object (batch_loose_object, &sa,0 );for_each_packed_object (batch_packed_object, &sa,0 ); +if (repository_format_partial_clone) +warning ("This repository has extensions.partialClone set. Some objects may not be loaded." ); cb.opt = opt; cb.expand = &data;
builtin/fetch-pack.c +2 -0
@@ -53,6 +53,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)struct oid_array shallow =OID_ARRAY_INIT ;struct string_list deepen_not =STRING_LIST_INIT_DUP ; + fetch_if_missing =0 ; +packet_trace_identity ("fetch-pack" );memset (&args,0 ,sizeof (args));
builtin/fsck.c +3 -0
@@ -678,6 +678,9 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)int i;struct alternate_object_database *alt; +/* fsck knows how to handle missing promisor objects */ + fetch_if_missing =0 ; + errors_found =0 ; check_replace_refs =0 ;
builtin/index-pack.c +6 -0
@@ -1657,6 +1657,12 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)unsigned foreign_nr =1 ;/* zero is a "good" value, assume bad */ int report_end_of_input =0 ; +/* +* index-pack never needs to fetch missing objects, since it only +* accesses the repo to do hash collision checks +*/ + fetch_if_missing =0 ; +if (argc ==2 && !strcmp (argv[1 ],"-h" ))usage (index_pack_usage);
cache.h +8 -0
@@ -1727,6 +1727,14 @@ struct object_info {#define OBJECT_INFO_QUICK 8extern int sha1_object_info_extended (const unsigned char *,struct object_info *,unsigned flags); +/* +* Set this to 0 to prevent sha1_object_info_extended() from fetching missing +* blobs. This has a difference only if extensions.partialClone is set. +* +* Its default value is 1. +*/ +extern int fetch_if_missing; +/* Dumb servers support */ extern int update_server_info (int );
fetch-object.c +3 -0
@@ -10,7 +10,9 @@ void fetch_object(const char *remote_name, const unsigned char *sha1)struct remote *remote;struct transport *transport;struct ref *ref; +int original_fetch_if_missing = fetch_if_missing; + fetch_if_missing =0 ; remote =remote_get (remote_name);if (!remote->url[0 ])die (_ ("Remote with no URL" ));
@@ -21,4 +23,5 @@ void fetch_object(const char *remote_name, const unsigned char *sha1)transport_set_option (transport,TRANS_OPT_FROM_PROMISOR ,"1" );transport_set_option (transport,TRANS_OPT_NO_HAVES ,"1" );transport_fetch_refs (transport, ref); + fetch_if_missing = original_fetch_if_missing; }
sha1_file.c +25 -13
@@ -29,6 +29,7 @@#include "mergesort.h" #include "quote.h" #include "packfile.h" +#include "fetch-object.h" const unsigned char null_sha1[GIT_MAX_RAWSZ ];const struct object_id null_oid;
@@ -1144,6 +1145,8 @@ static int sha1_loose_object_info(const unsigned char *sha1,return (status <0 ) ? status :0 ; } +int fetch_if_missing =1 ; +int sha1_object_info_extended (const unsigned char *sha1,struct object_info *oi,unsigned flags) {static struct object_info blank_oi =OBJECT_INFO_INIT ;
@@ -1152,6 +1155,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,const unsigned char *real = (flags &OBJECT_INFO_LOOKUP_REPLACE ) ?lookup_replace_object (sha1) : sha1; +int already_retried =0 ;if (!oi) oi = &blank_oi;
@@ -1176,28 +1180,36 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi, } } -if (!find_pack_entry (real, &e)) { -/* Most likely it's a loose object. */ -if (!sha1_loose_object_info (real, oi, flags)) -return 0 ; +retry: +if (find_pack_entry (real, &e)) + goto found_packed; -/* Not a loose object; someone else may have just packed it. */ -if (flags &OBJECT_INFO_QUICK ) { -return -1 ; - }else { -reprepare_packed_git (); -if (!find_pack_entry (real, &e)) -return -1 ; - } +/* Most likely it's a loose object. */ +if (!sha1_loose_object_info (real, oi, flags)) +return 0 ; + +/* Not a loose object; someone else may have just packed it. */ +reprepare_packed_git (); +if (find_pack_entry (real, &e)) + goto found_packed; + +/* Check if it is a missing object */ +if (fetch_if_missing && repository_format_partial_clone && + !already_retried) { +fetch_object (repository_format_partial_clone, real); + already_retried =1 ; + goto retry; } +return -1 ; + +found_packed:if (oi == &blank_oi)/* * We know that the caller doesn't actually need the * information below, so return early. */ return 0 ; - rtype =packed_object_info (e.p, e.offset, oi);if (rtype <0 ) {mark_bad_packed_object (e.p, real);
t/t0410-partial-clone.sh +51 -0
55 lines not on this page · raw message
Signature · 1 line
2.9.3
Re: [PATCH v5 08/10] sha1_file: support lazily fetching missing objects
Quoted 71 lines · Jeff Hostetler
From: Jonathan Tan <[email protected]>
sha1_file.c +25
-12@@ -29,6 +29,7 @@#include "mergesort.h" #include "quote.h" #include "packfile.h" +#include "fetch-object.h" const unsigned char null_sha1[GIT_MAX_RAWSZ ];const struct object_id null_oid;@@ -1144,6 +1145,8 @@ static int sha1_loose_object_info(const unsigned char *sha1,return (status <0 ) ? status :0 ; } +int fetch_if_missing =1 ; +int sha1_object_info_extended (const unsigned char *sha1,struct object_info *oi,unsigned flags) {static struct object_info blank_oi =OBJECT_INFO_INIT ;@@ -1152,6 +1155,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,const unsigned char *real = (flags &OBJECT_INFO_LOOKUP_REPLACE ) ?lookup_replace_object (sha1) : sha1; +int already_retried =0 ;if (!oi) oi = &blank_oi;@@ -1176,28 +1180,36 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi, } } -if (!find_pack_entry (real, &e)) { -/* Most likely it's a loose object. */ -if (!sha1_loose_object_info (real, oi, flags)) -return 0 ; +retry: +if (find_pack_entry (real, &e)) + goto found_packed; -/* Not a loose object; someone else may have just packed it. */ -if (flags &OBJECT_INFO_QUICK ) { -return -1 ; - }else { -reprepare_packed_git (); -if (!find_pack_entry (real, &e)) -return -1 ; - } +/* Most likely it's a loose object. */ +if (!sha1_loose_object_info (real, oi, flags)) +return 0 ; + +/* Not a loose object; someone else may have just packed it. */ +reprepare_packed_git (); +if (find_pack_entry (real, &e)) + goto found_packed; + +/* Check if it is a missing object */ +if (fetch_if_missing && repository_format_partial_clone && + !already_retried) { +fetch_object (repository_format_partial_clone, real); + already_retried =1 ; + goto retry; } +return -1 ; + +found_packed:
sha1_file.c int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi, · line 1215
Quoted 10 lines
sha1_file.c +0
-1if (oi == &blank_oi)/* * We know that the caller doesn't actually need the * information below, so return early. */ return 0 ; - rtype =packed_object_info (e.p, e.offset, oi);if (rtype <0 ) {mark_bad_packed_object (e.p, real);
The above is adding 2 different gotos into this function while there are quite simple ways to avoid them. See
https://public-inbox.org/git/CAP8UFD2THRj7+RXmismUtUOpXQv4wM7aZsejpd_FHEOycP+ZJA@mail.gmail.com/
and the follow up email in the thread.
Re: [PATCH v5 08/10] sha1_file: support lazily fetching missing objects
Quoted 2 lines · Christian Couder
Quoted 1 line · Jeff Hostetler
From: Jonathan Tan <[email protected]>
[...]
Quoted 2 lines
Quoted 2 lines
int
sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi, unsigned flags){
[...]
Quoted 38 lines
Quoted 38 lines
sha1_file.c +21
-12-if (!find_pack_entry (real, &e)) { -/* Most likely it's a loose object. */ -if (!sha1_loose_object_info (real, oi, flags)) -return 0 ; +retry: +if (find_pack_entry (real, &e)) + goto found_packed; -/* Not a loose object; someone else may have just packed it. */ -if (flags &OBJECT_INFO_QUICK ) { -return -1 ; - }else { -reprepare_packed_git (); -if (!find_pack_entry (real, &e)) -return -1 ; - } +/* Most likely it's a loose object. */ +if (!sha1_loose_object_info (real, oi, flags)) +return 0 ; + +/* Not a loose object; someone else may have just packed it. */ +reprepare_packed_git (); +if (find_pack_entry (real, &e)) + goto found_packed; + +/* Check if it is a missing object */ +if (fetch_if_missing && repository_format_partial_clone && + !already_retried) { +fetch_object (repository_format_partial_clone, real); + already_retried =1 ; + goto retry; } +return -1 ; + +found_packed:if (oi == &blank_oi)
[...]
Quoted 5 lines
The above is adding 2 different gotos into this function while there are quite simple ways to avoid them. See
https://public-inbox.org/git/CAP8UFD2THRj7+RXmismUtUOpXQv4wM7aZsejpd_FHEOycP+ZJA@mail.gmail.com/
and the follow up email in the thread.
Personally, I'm OK with limited goto's like these. Yes, they are ugly, but not that complicated. Alternatively, we could put everything between the 2 labels in a while (1) {...} and replace the goto's with break's and continue's. I think it would be better to revisit this later, as I'd rather not start refactoring sha1_file.c in the middle of what is already a 30+ commit patch series (for the 3 parts of partial clone).
Thoughts?
Jeff
PATCH v5 9/10 rev-list: support termination at promisor objects
In reply to Jeff HostetlerFrom: Jonathan Tan <[email protected]>
Teach rev-list to support termination of an object traversal at any object from a promisor remote (whether one that the local repo also has, or one that the local repo knows about because it has another promisor object that references it).
This will be used subsequently in gc and in the connectivity check used by fetch.
For efficiency, if an object is referenced by a promisor object, and is in the local repo only as a non-promisor object, object traversal will not stop there. This is to avoid building the list of promisor object references.
(In list-objects.c, the case where obj is NULL in process_blob() and process_tree() do not need to be changed because those happen only when there is a conflict between the expected type and the existing object. If the object doesn't exist, an object will be synthesized, which is fine.)
--- Documentation/rev-list-options.txt | 11 ++++ builtin/rev-list.c | 71 +++++++++++++++++++++++--- list-objects.c | 29 ++++++++++- object.c | 2 +- revision.c | 33 +++++++++++- revision.h | 5 +- t/t0410-partial-clone.sh | 101 +++++++++++++++++++++++++++++++++++++ 7 files changed, 240 insertions(+), 12 deletions(-)
Documentation/rev-list-options.txt +11 -0
@@ -740,10 +740,21 @@ The form '--missing=allow-any' will allow object traversal to continue if a missing object is encountered. Missing objects will silently be omitted from the results. + +The form '--missing=allow-promisor' is like 'allow-any', but will only +allow object traversal to continue for EXPECTED promisor missing objects. +Unexpected missing objects will raise an error. ++ The form '--missing=print' is like 'allow-any', but will also print a list of the missing objects. Object IDs are prefixed with a ``?'' character. endif::git-rev-list[] +--exclude-promisor-objects:: + (For internal use only.) Prefilter object traversal at + promisor boundary. This is used with partial clone. This is + stronger than `--missing=allow-promisor` because it limits the + traversal, rather than just silencing errors about missing + objects. + --no-walk[=(sorted|unsorted)]:: Only show the given commits, but do not traverse their ancestors. This has no effect if a range is specified. If the argument
builtin/rev-list.c +64 -7
134 lines not on this page · raw message
list-objects.c +28 -1
@@ -9,6 +9,7 @@#include "list-objects.h" #include "list-objects-filter.h" #include "list-objects-filter-options.h" +#include "packfile.h" static void process_blob(struct rev_info *revs, struct blob *blob,
@@ -30,6 +31,20 @@ static void process_blob(struct rev_info *revs,if (obj->flags & (UNINTERESTING |SEEN ))return ; +/* +* Pre-filter known-missing objects when explicitly requested. +* Otherwise, a missing object error message may be reported +* later (depending on other filtering criteria). +* +* Note that this "--exclude-promisor-objects" pre-filtering +* may cause the actual filter to report an incomplete list +* of missing objects. +*/ +if (revs->exclude_promisor_objects && + !has_object_file (&obj->oid) && +is_promisor_object (&obj->oid)) +return ; + pathlen = path->len;strbuf_addstr (path, name);if (filter_fn)
@@ -91,6 +106,8 @@ static void process_tree(struct rev_info *revs, all_entries_interesting: entry_not_interesting;int baselen = base->len;enum list_objects_filter_result r =LOFR_MARK_SEEN |LOFR_DO_SHOW ; +int gently = revs->ignore_missing_links || + revs->exclude_promisor_objects;if (!revs->tree_objects)return ;
@@ -98,9 +115,19 @@ static void process_tree(struct rev_info *revs,die ("bad tree object" );if (obj->flags & (UNINTERESTING |SEEN ))return ; -if (parse_tree_gently (tree, revs->ignore_missing_links) <0 ) { +if (parse_tree_gently (tree, gently) <0 ) {if (revs->ignore_missing_links)return ; + +/* +* Pre-filter known-missing tree objects when explicitly +* requested. This may cause the actual filter to report +* an incomplete list of missing objects. +*/ +if (revs->exclude_promisor_objects && +is_promisor_object (&obj->oid)) +return ; +die ("bad tree object %s" ,oid_to_hex (&obj->oid)); }
object.c +1 -1
@@ -252,7 +252,7 @@ struct object *parse_object(const struct object_id *oid)if (obj && obj->parsed)return obj; -if ((obj && obj->type ==OBJ_BLOB ) || +if ((obj && obj->type ==OBJ_BLOB &&has_object_file (oid)) || (!obj &&has_object_file (oid) &&sha1_object_info (oid->hash,NULL ) ==OBJ_BLOB )) {if (check_sha1_signature (repl,NULL ,0 ,NULL ) <0 ) {
revision.c +31 -2
@@ -198,6 +198,8 @@ static struct object *get_reference(struct rev_info *revs, const char *name,if (!object) {if (revs->ignore_missing)return object; +if (revs->exclude_promisor_objects &&is_promisor_object (oid)) +return NULL ;die ("bad object %s" , name); } object->flags |= flags;
@@ -790,9 +792,17 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,for (parent = commit->parents; parent; parent = parent->next) {struct commit *p = parent->item; - -if (parse_commit_gently (p, revs->ignore_missing_links) <0 ) +int gently = revs->ignore_missing_links || + revs->exclude_promisor_objects; +if (parse_commit_gently (p, gently) <0 ) { +if (revs->exclude_promisor_objects && +is_promisor_object (&p->object.oid)) { +if (revs->first_parent_only) +break ; +continue ; + }return -1 ; + }if (revs->show_source && !p->util) p->util = commit->util; p->object.flags |= left_flag;
@@ -2088,6 +2098,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg revs->limited =1 ; }else if (!strcmp (arg,"--ignore-missing" )) { revs->ignore_missing =1 ; + }else if (!strcmp (arg,"--exclude-promisor-objects" )) { +if (fetch_if_missing) +die ("BUG: exclude_promisor_objects can only be used when fetch_if_missing is 0" ); + revs->exclude_promisor_objects =1 ; }else {int opts =diff_opt_parse (&revs->diffopt, argv, argc, revs->prefix);if (!opts)
@@ -2830,6 +2844,16 @@ void reset_revision_walk(void)clear_object_flags (SEEN |ADDED |SHOWN ); } +static int mark_uninteresting (const struct object_id *oid, +struct packed_git *pack, +uint32_t pos, +void *unused) +{ +struct object *o =parse_object (oid); + o->flags |=UNINTERESTING |SEEN ; +return 0 ; +} +int prepare_revision_walk (struct rev_info *revs) {int i;
@@ -2858,6 +2882,11 @@ int prepare_revision_walk(struct rev_info *revs) (revs->limited && limiting_can_increase_treesame(revs))) revs->treesame.name = "treesame"; +if (revs->exclude_promisor_objects) { +for_each_packed_object (mark_uninteresting,NULL , +FOR_EACH_OBJECT_PROMISOR_ONLY ); + } +if (revs->no_walk !=REVISION_WALK_NO_WALK_UNSORTED )commit_list_sort_by_date (&revs->commits);if (revs->no_walk)
revision.h +4 -1
@@ -121,7 +121,10 @@ struct rev_info { bisect:1, ancestry_path:1, first_parent_only:1, - line_level_traverse:1; + line_level_traverse:1, + + /* for internal use only */ + exclude_promisor_objects:1;/* Diff flags */ unsigned int diff:1,
t/t0410-partial-clone.sh +101 -0
107 lines not on this page · raw message
Signature · 1 line
2.9.3
PATCH v5 10/10 gc: do not repack promisor packfiles
In reply to Jeff HostetlerFrom: Jonathan Tan <[email protected]>
Teach gc to stop traversal at promisor objects, and to leave promisor packfiles alone. This has the effect of only repacking non-promisor packfiles, and preserves the distinction between promisor packfiles and non-promisor packfiles.
--- Documentation/git-pack-objects.txt | 11 ++++++++ builtin/gc.c | 3 +++ builtin/pack-objects.c | 37 +++++++++++++++++++++++++-- builtin/prune.c | 7 +++++ builtin/repack.c | 8 ++++-- t/t0410-partial-clone.sh | 52 +++++++++++++++++++++++++++++++++++++- 6 files changed, 113 insertions(+), 5 deletions(-)
Documentation/git-pack-objects.txt +11 -0
@@ -252,6 +252,17 @@ a missing object is encountered. This is the default action. The form '--missing=allow-any' will allow object traversal to continue if a missing object is encountered. Missing objects will silently be omitted from the results. ++ +The form '--missing=allow-promisor' is like 'allow-any', but will only +allow object traversal to continue for EXPECTED promisor missing objects. +Unexpected missing object will raise an error. + +--exclude-promisor-objects:: + Omit objects that are known to be in the promisor remote. (This + option has the purpose of operating only on locally created objects, + so that when we repack, we still maintain a distinction between + locally created objects [without .promisor] and objects from the + promisor remote [with .promisor].) This is used with partial clone. SEE ALSO --------
builtin/gc.c +3 -0
@@ -458,6 +458,9 @@ int cmd_gc(int argc, const char **argv, const char *prefix)argv_array_push (&prune, prune_expire);if (quiet)argv_array_push (&prune,"--no-progress" ); +if (repository_format_partial_clone) +argv_array_push (&prune, +"--exclude-promisor-objects" );if (run_command_v_opt (prune.argv,RUN_GIT_CMD ))return error (FAILED_RUN , prune.argv[0 ]); }
builtin/pack-objects.c +35 -2
@@ -75,6 +75,8 @@ static int use_bitmap_index = -1;static int write_bitmap_index;static uint16_t write_bitmap_options; +static int exclude_promisor_objects; +static unsigned long delta_cache_size =0 ;static unsigned long max_delta_cache_size =256 *1024 *1024 ;static unsigned long cache_max_small_delta_size =1000 ;
@@ -84,8 +86,9 @@ static unsigned long window_memory_limit = 0;static struct list_objects_filter_options filter_options;enum missing_action { -MA_ERROR =0 ,/* fail if any missing objects are encountered */ -MA_ALLOW_ANY ,/* silently allow ALL missing objects */ +MA_ERROR =0 ,/* fail if any missing objects are encountered */ +MA_ALLOW_ANY ,/* silently allow ALL missing objects */ +MA_ALLOW_PROMISOR ,/* silently allow all missing PROMISOR objects */ };static enum missing_action arg_missing_action;static show_object_fn fn_show_object;
@@ -2577,6 +2580,20 @@ static void show_object__ma_allow_any(struct object *obj, const char *name, voidshow_object (obj, name, data); } +static void show_object__ma_allow_promisor (struct object *obj,const char *name,void *data) +{ +assert (arg_missing_action ==MA_ALLOW_PROMISOR ); + +/* +* Quietly ignore EXPECTED missing objects. This avoids problems with +* staging them now and getting an odd error later. +*/ +if (!has_object_file (&obj->oid) &&is_promisor_object (&obj->oid)) +return ; + +show_object (obj, name, data); +} +static int option_parse_missing_action (const struct option *opt,const char *arg,int unset) {
@@ -2591,10 +2608,18 @@ static int option_parse_missing_action(const struct option *opt,if (!strcmp (arg,"allow-any" )) { arg_missing_action =MA_ALLOW_ANY ; + fetch_if_missing =0 ; fn_show_object = show_object__ma_allow_any;return 0 ; } +if (!strcmp (arg,"allow-promisor" )) { + arg_missing_action =MA_ALLOW_PROMISOR ; + fetch_if_missing =0 ; + fn_show_object = show_object__ma_allow_promisor; +return 0 ; + } +die (_ ("invalid value for --missing" ));return 0 ; }
@@ -3008,6 +3033,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) {OPTION_CALLBACK ,0 ,"missing" ,NULL ,N_ ("action" ),N_ ("handling for missing objects" ),PARSE_OPT_NONEG , option_parse_missing_action }, +OPT_BOOL (0 ,"exclude-promisor-objects" , &exclude_promisor_objects, +N_ ("do not pack objects in promisor packfiles" )), OPT_END(), };
@@ -3053,6 +3080,12 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)argv_array_push (&rp,"--unpacked" ); } +if (exclude_promisor_objects) { + use_internal_rev_list =1 ; + fetch_if_missing =0 ; +argv_array_push (&rp,"--exclude-promisor-objects" ); + } +if (!reuse_object) reuse_delta =0 ;if (pack_compression_level ==-1 )
builtin/prune.c +7 -0
@@ -101,12 +101,15 @@ int cmd_prune(int argc, const char **argv, const char *prefix) {struct rev_info revs;struct progress *progress =NULL ; +int exclude_promisor_objects =0 ;const struct option options[] = {OPT__DRY_RUN (&show_only,N_ ("do not remove, show only" )),OPT__VERBOSE (&verbose,N_ ("report pruned objects" )),OPT_BOOL (0 ,"progress" , &show_progress,N_ ("show progress" )),OPT_EXPIRY_DATE (0 ,"expire" , &expire,N_ ("expire objects older than <time>" )), +OPT_BOOL (0 ,"exclude-promisor-objects" , &exclude_promisor_objects, +N_ ("limit traversal to objects outside promisor packfiles" )),OPT_END () };char *s;
@@ -139,6 +142,10 @@ int cmd_prune(int argc, const char **argv, const char *prefix) show_progress =isatty (2 );if (show_progress) progress =start_delayed_progress (_ ("Checking connectivity" ),0 ); +if (exclude_promisor_objects) { + fetch_if_missing =0 ; + revs.exclude_promisor_objects =1 ; + }mark_reachable_objects (&revs,1 , expire, progress);stop_progress (&progress);
builtin/repack.c +6 -2
@@ -83,7 +83,8 @@ static void remove_pack_on_signal(int signo)/* * Adds all packs hex strings to the fname list, which do not -* have a corresponding .keep file. +* have a corresponding .keep or .promisor file. These packs are not to +* be kept if we are going to pack everything into one file. */ static void get_non_kept_pack_filenames (struct string_list *fname_list) {
@@ -101,7 +102,8 @@ static void get_non_kept_pack_filenames(struct string_list *fname_list) fname =xmemdupz (e->d_name, len); -if (!file_exists (mkpath ("%s/%s.keep" , packdir, fname))) +if (!file_exists (mkpath ("%s/%s.keep" , packdir, fname)) && + !file_exists (mkpath ("%s/%s.promisor" , packdir, fname)))string_list_append_nodup (fname_list, fname);else free (fname);
@@ -232,6 +234,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)argv_array_push (&cmd.args,"--all" );argv_array_push (&cmd.args,"--reflog" );argv_array_push (&cmd.args,"--indexed-objects" ); +if (repository_format_partial_clone) +argv_array_push (&cmd.args,"--exclude-promisor-objects" );if (window)argv_array_pushf (&cmd.args,"--window=%s" , window);if (window_memory)
t/t0410-partial-clone.sh +51 -1
70 lines not on this page · raw message
Signature · 1 line
2.9.3
Re: [PATCH v5 10/10] gc: do not repack promisor packfiles
Quoted 7 lines · Jeff Hostetler
From: Jonathan Tan <[email protected]>
pack_as_from_promisor () { HASH=$(git -C repo pack-objects .git/objects/pack/pack) && >repo/.git/objects/pack/pack-$HASH.promisor+ echo $HASH
}
Now the exit code of the above function will always be the exit code of "echo $HASH".
It seems to me that it would be better to add "&&" at the end of the line above the "echo $HASH".
Re: [PATCH v5 10/10] gc: do not repack promisor packfiles
Quoted 9 lines · Christian Couder
Quoted 7 lines · Jeff Hostetler
From: Jonathan Tan <[email protected]>
pack_as_from_promisor () { HASH=$(git -C repo pack-objects .git/objects/pack/pack) && >repo/.git/objects/pack/pack-$HASH.promisor+ echo $HASH
}
Quoted 5 lines
Now the exit code of the above function will always be the exit code of "echo $HASH".
It seems to me that it would be better to add "&&" at the end of the line above the "echo $HASH".
Fixed. Thanks.
Jeff
Quoted 10 lines · Jeff Hostetler
From: Jeff Hostetler <[email protected]>
This is V5 of part 2 of partial clone. This assumes V5 of part 1 is already present. V5 includes minor cleanup over V4 and better separates the --exclude-promisor-objects and --missing arguments.
Part 2 is concerned with fsck, gc, initial support for dynamic object fetching, and tracking promisor objects. Jonathan Tan originally developed this code. I have moved it on top of part 1 and updated it slightly.
Thanks, will replace/queue all three series. I am getting a feeling that the first one is already ready for 'next', while the other two may want to see a bit more comments?
Quoted 8 lines · On Wed, 22 Nov 2017 14:25:13 +0900 Junio C Hamano <[email protected]> wrote:
Quoted 6 lines · Jeff Hostetler
From: Jeff Hostetler <[email protected]>
This is V5 of part 2 of partial clone. This assumes V5 of part 1 is already present. V5 includes minor cleanup over V4 and better separates the --exclude-promisor-objects and --missing arguments.
Quoted 8 lines
Quoted 4 lines
Part 2 is concerned with fsck, gc, initial support for dynamic object fetching, and tracking promisor objects. Jonathan Tan originally developed this code. I have moved it on top of part 1 and updated it slightly.
Thanks, will replace/queue all three series. I am getting a feeling that the first one is already ready for 'next', while the other two may want to see a bit more comments?
Yes, I think so too.
Jeff Hostetler and I noticed some issues occuring when some other Git commands dynamically fetch objects due to the fact that those commands and fetch-pack use overlapping object flags. At the very least, we should look at that before it goes into next.
Quoted 7 lines · Jonathan Tan
Quoted 3 lines · On Wed, 22 Nov 2017 14:25:13 +0900 Junio C Hamano <[email protected]> wrote:
Thanks, will replace/queue all three series. I am getting a feeling that the first one is already ready for 'next', while the other two may want to see a bit more comments?
Quoted 6 lines
Yes, I think so too.
Jeff Hostetler and I noticed some issues occuring when some other Git commands dynamically fetch objects due to the fact that those commands and fetch-pack use overlapping object flags. At the very least, we should look at that before it goes into next.
In the end, making fetch-pack refrain from setting object flags when being used to dynamically fetch objects works (see the new "introduce
fetch-object: fetch one promisor object" commit that I just pushed to [1]). I'll coordinate with Jeff Hostetler about sending v6 to the list (most likely after the U.S. Thanksgiving holidays).