Nguyễn Thái Ngọc Duy <pclo...@gmail.com> 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.
>
> git learns new global option --shallow-file to pass on the alternate
> shallow file path. Undocumented (and not even support --shallow-file=
> syntax) because it's unlikely to be used again elsewhere.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
> ---
> commit.h | 2 ++
> fetch-pack.c | 69
> +++++++++++++++++++++++++--------------------------
> git.c | 5 ++++
> shallow.c | 45 +++++++++++++++++++++++++++++++--
> t/t5500-fetch-pack.sh | 7 ++++++
> 5 files changed, 91 insertions(+), 37 deletions(-)
>
> diff --git a/commit.h b/commit.h
> index 67bd509..6e9c7cd 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -176,6 +176,8 @@ 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);
> +extern void set_alternate_shallow_file(const char *path);
>
> 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..1ca4f6b 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -20,6 +20,8 @@ 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;
> +static const char *alternate_shallow_file;
>
> #define COMPLETE (1U << 0)
> #define COMMON (1U << 1)
> @@ -683,7 +685,7 @@ static int get_pack(struct fetch_pack_args *args,
> int xd[2], char **pack_lockfile)
> {
> struct async demux;
> - const char *argv[20];
> + const char *argv[22];
> char keep_arg[256];
> char hdr_arg[256];
> const char **av;
> @@ -724,6 +726,11 @@ static int get_pack(struct fetch_pack_args *args,
> do_keep = 1;
> }
>
> + if (alternate_shallow_file) {
> + *av++ = "--shallow-file";
> + *av++ = alternate_shallow_file;
> + }
> +
> if (do_keep) {
> if (pack_lockfile)
> cmd.out = -1;
> @@ -779,6 +786,23 @@ 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);
> + alternate_shallow_file = shallow_lock.filename;
> + } else
> + alternate_shallow_file = "";
This "empty string, not NULL" trick needs to be commented here to
match the other place that uses this as a cue to do things
differently.
> + strbuf_release(&sb);
> +}
> +
> static struct ref *do_fetch_pack(struct fetch_pack_args *args,
> int fd[2],
> const struct ref *orig_ref,
> @@ -858,6 +882,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 +962,9 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
> struct ref **sought, int nr_sought,
> char **pack_lockfile)
> {
> struct ref *ref_cpy;
>
> fetch_pack_setup();
> if (nr_sought)
> nr_sought = remove_duplicates_in_refs(sought, nr_sought);
>
> @@ -955,34 +975,13 @@ 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) {
> + struct stat st;
> + if (!fstat(shallow_lock.fd, &st) &&
> + st.st_size == 0) {
> + unlink_or_warn(git_path("shallow"));
Are we unlinking the right file here?
> + rollback_lock_file(&shallow_lock);
> + } else
> + commit_lock_file(&shallow_lock);
> }
>
> reprepare_packed_git();
> diff --git a/git.c b/git.c
> index 1ada169..6450a38 100644
> --- a/git.c
> +++ b/git.c
> @@ -4,6 +4,7 @@
> #include "help.h"
> #include "quote.h"
> #include "run-command.h"
> +#include "commit.h"
>
> const char git_usage_string[] =
> "git [--version] [--help] [-c name=value]\n"
> @@ -146,6 +147,10 @@ static int handle_options(const char ***argv, int *argc,
> int *envchanged)
> setenv(GIT_LITERAL_PATHSPECS_ENVIRONMENT, "0", 1);
> if (envchanged)
> *envchanged = 1;
> + } else if (!strcmp(cmd, "--shallow-file")) {
> + (*argv)++;
> + (*argc)--;
> + set_alternate_shallow_file((*argv)[0]);
Should this count as "envchanged"?
> diff --git a/shallow.c b/shallow.c
> index 6be915f..bdae988 100644
> --- a/shallow.c
> +++ b/shallow.c
> @@ -3,6 +3,16 @@
> #include "tag.h"
>
> static int is_shallow = -1;
> +static struct stat shallow_stat;
> +static char *alternate_shallow_file;
> +
> +void set_alternate_shallow_file(const char *path)
> +{
> + if (is_shallow != -1)
> + die("BUG: is_repository_shallow must not be called before
> set_alternate_shallow_file");
> + free(alternate_shallow_file);
> + alternate_shallow_file = path ? xstrdup(path) : NULL;
> +}
>
> int register_shallow(const unsigned char *sha1)
> {
> @@ -21,12 +31,21 @@ int is_repository_shallow(void)
> {
> FILE *fp;
> char buf[1024];
> + const char *path = alternate_shallow_file;
>
> if (is_shallow >= 0)
> return is_shallow;
>
> - fp = fopen(git_path("shallow"), "r");
> - if (!fp) {
> + if (!path)
> + path = git_path("shallow");
> + /*
> + * fetch-pack set '--shallow-file ""' as an indicator that no
s/set/&s/?
> + * shallow file should be used. We could just open it and it
> + * will likely fail. But let's do an explicit check instead.
> + */
> + if (!*path ||
> + stat(path, &shallow_stat) ||
> + (fp = fopen(path, "r")) == NULL) {
> is_shallow = 0;
> return is_shallow;
> }
> @@ -108,3 +127,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");
Forgot to remove this?
> + 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
