Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> index-pack --strict looks up and follows parent commits. If shallow
> information is not ready by the time index-pack is run, index-pack may
> be lead to non-existent objects. Make fetch-pack save shallow file to
> disk before invoking index-pack.
>
> A new environment variable GIT_SHALLOW_FILE is used to inform
> index-pack to use an alternate shallow file. This variable is for
> internal use and thus not advertised in git.txt.
The idea to tell the index-pack to use a different shallow file is
sound, but is the environment variable the best way to go? I am
mostly worried about it being an implicit "apply everywhere"
mechanism and while I do not particularly like the idea of doing
everything in the same process as the top-level while dealing with
submodules, I am reasonably sure people would want to "clone"
recursively inside the same top-level process in the future, and
this new environment variable adds one more thing to be cleansed
when crossing a repository boundary.
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> commit.h | 1 +
> fetch-pack.c | 65 ++++++++++++++++++++++++---------------------------
> shallow.c | 30 ++++++++++++++++++++++--
> t/t5500-fetch-pack.sh | 7 ++++++
> 4 files changed, 67 insertions(+), 36 deletions(-)
>
> diff --git a/commit.h b/commit.h
> index 67bd509..0cef659 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -176,6 +176,7 @@ extern int for_each_commit_graft(each_commit_graft_fn, void *);
> extern int is_repository_shallow(void);
> extern struct commit_list *get_shallow_commits(struct object_array *heads,
> int depth, int shallow_flag, int not_shallow_flag);
> +extern void check_shallow_file_for_update(void);
>
> int is_descendant_of(struct commit *, struct commit_list *);
> int in_merge_bases(struct commit *, struct commit *);
> diff --git a/fetch-pack.c b/fetch-pack.c
> index f156dd4..17cfa88 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -20,6 +20,7 @@ static int no_done;
> static int fetch_fsck_objects = -1;
> static int transfer_fsck_objects = -1;
> static int agent_supported;
> +static struct lock_file shallow_lock;
>
> #define COMPLETE (1U << 0)
> #define COMMON (1U << 1)
> @@ -779,6 +780,26 @@ static int cmp_ref_by_name(const void *a_, const void *b_)
> return strcmp(a->name, b->name);
> }
>
> +static void setup_alternate_shallow(void)
> +{
> + struct strbuf sb = STRBUF_INIT;
> + int fd;
> +
> + check_shallow_file_for_update();
> + fd = hold_lock_file_for_update(&shallow_lock, git_path("shallow"),
> + LOCK_DIE_ON_ERROR);
> + if (write_shallow_commits(&sb, 0)) {
> + if (write_in_full(fd, sb.buf, sb.len) != sb.len)
> + die_errno("failed to write to %s", shallow_lock.filename);
> + if (setenv("GIT_SHALLOW_FILE", shallow_lock.filename, 1))
> + die_errno("failed to set GIT_SHALLOW_FILE");
> + } else {
> + if (setenv("GIT_SHALLOW_FILE", "", 1))
> + die_errno("failed to set GIT_SHALLOW_FILE");
> + }
> + strbuf_release(&sb);
> +}
> +
> static struct ref *do_fetch_pack(struct fetch_pack_args *args,
> int fd[2],
> const struct ref *orig_ref,
> @@ -858,6 +879,8 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
>
> if (args->stateless_rpc)
> packet_flush(fd[1]);
> + if (args->depth > 0)
> + setup_alternate_shallow();
> if (get_pack(args, fd, pack_lockfile))
> die("git fetch-pack: fetch failed.");
>
> @@ -936,15 +959,9 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
> struct ref **sought, int nr_sought,
> char **pack_lockfile)
> {
> - struct stat st;
> struct ref *ref_cpy;
>
> fetch_pack_setup();
> - if (args->depth > 0) {
> - if (stat(git_path("shallow"), &st))
> - st.st_mtime = 0;
> - }
> -
> if (nr_sought)
> nr_sought = remove_duplicates_in_refs(sought, nr_sought);
>
> @@ -955,34 +972,14 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
> ref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought, pack_lockfile);
>
> if (args->depth > 0) {
> - static struct lock_file lock;
> - struct cache_time mtime;
> - struct strbuf sb = STRBUF_INIT;
> - char *shallow = git_path("shallow");
> - int fd;
> -
> - mtime.sec = st.st_mtime;
> - mtime.nsec = ST_MTIME_NSEC(st);
> - if (stat(shallow, &st)) {
> - if (mtime.sec)
> - die("shallow file was removed during fetch");
> - } else if (st.st_mtime != mtime.sec
> -#ifdef USE_NSEC
> - || ST_MTIME_NSEC(st) != mtime.nsec
> -#endif
> - )
> - die("shallow file was changed during fetch");
> -
> - fd = hold_lock_file_for_update(&lock, shallow,
> - LOCK_DIE_ON_ERROR);
> - if (!write_shallow_commits(&sb, 0)
> - || write_in_full(fd, sb.buf, sb.len) != sb.len) {
> - unlink_or_warn(shallow);
> - rollback_lock_file(&lock);
> - } else {
> - commit_lock_file(&lock);
> - }
> - strbuf_release(&sb);
> + struct stat st;
> + if (!fstat(shallow_lock.fd, &st) &&
> + st.st_size == 0) {
> + unlink_or_warn(git_path("shallow"));
> + rollback_lock_file(&shallow_lock);
> + } else
> + commit_lock_file(&shallow_lock);
> + unsetenv("GIT_SHALLOW_FILE");
> }
>
> reprepare_packed_git();
> diff --git a/shallow.c b/shallow.c
> index 6be915f..9c03806 100644
> --- a/shallow.c
> +++ b/shallow.c
> @@ -3,6 +3,7 @@
> #include "tag.h"
>
> static int is_shallow = -1;
> +static struct stat shallow_stat;
>
> int register_shallow(const unsigned char *sha1)
> {
> @@ -21,12 +22,15 @@ int is_repository_shallow(void)
> {
> FILE *fp;
> char buf[1024];
> + const char *path;
>
> if (is_shallow >= 0)
> return is_shallow;
>
> - fp = fopen(git_path("shallow"), "r");
> - if (!fp) {
> + path = getenv("GIT_SHALLOW_FILE");
> + if (!path)
> + path = git_path("shallow");
> + if (stat(path, &shallow_stat) || (fp = fopen(path, "r")) == NULL) {
> is_shallow = 0;
> return is_shallow;
> }
> @@ -108,3 +112,25 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth,
>
> return result;
> }
> +
> +void check_shallow_file_for_update(void)
> +{
> + struct stat st;
> +
> + if (getenv("GIT_SHALLOW_FILE"))
> + die("GIT_SHALLOW_FILE should not be manually set");
> +
> + if (!is_shallow)
> + return;
> + else if (is_shallow == -1)
> + die("BUG: shallow must be initialized by now");
> +
> + if (stat(git_path("shallow"), &st))
> + die("shallow file was removed during fetch");
> + else if (st.st_mtime != shallow_stat.st_mtime
> +#ifdef USE_NSEC
> + || ST_MTIME_NSEC(st) != ST_MTIME_NSEC(shallow_stat)
> +#endif
> + )
> + die("shallow file was changed during fetch");
> +}
> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> index d574085..557b073 100755
> --- a/t/t5500-fetch-pack.sh
> +++ b/t/t5500-fetch-pack.sh
> @@ -135,6 +135,13 @@ test_expect_success 'clone shallow depth 1' '
> test "`git --git-dir=shallow0/.git rev-list --count HEAD`" = 1
> '
>
> +test_expect_success 'clone shallow depth 1 with fsck' '
> + git config --global fetch.fsckobjects true &&
> + git clone --no-single-branch --depth 1 "file://$(pwd)/." shallow0fsck &&
> + test "`git --git-dir=shallow0fsck/.git rev-list --count HEAD`" = 1 &&
> + git config --global --unset fetch.fsckobjects
> +'
> +
> test_expect_success 'clone shallow' '
> git clone --no-single-branch --depth 2 "file://$(pwd)/." shallow
> '
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html