From: Junio C Hamano <gitster@pobox.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH v3] upload-pack: send shallow info over stdin to pack-objects
Date: Thu, 06 Mar 2014 10:37:24 -0800 [thread overview]
Message-ID: <xmqqfvmvyxzv.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1394095783-24402-1-git-send-email-pclouds@gmail.com> ("Nguyễn Thái Ngọc Duy"'s message of "Thu, 6 Mar 2014 15:49:43 +0700")
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> From: Duy Nguyen <pclouds@gmail.com>
>
> Before cdab485 (upload-pack: delegate rev walking in shallow fetch to
> pack-objects - 2013-08-16) upload-pack does not write to the source
> repository. cdab485 starts to write $GIT_DIR/shallow_XXXXXX if it's a
> shallow fetch, so the source repo must be writable.
>
> git:// servers do not need write access to repos and usually don't
> have it, which means cdab485 breaks shallow clone over git://
>
> Instead of using a temporary file as the media for shallow points, we
> can send them over stdin to pack-objects as well. Prepend shallow
> SHA-1 with --shallow so pack-objects knows what is
> what.
>
> read_object_list_from_stdin() does not accept --shallow lines because
> upload-pack never uses that code. When upload-pack does, then it can
> learn about --shallow lines.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> OK new approach, stop creating shallow_XXXXXX in upload-pack.
Thanks.
I like what I see in this patch, but I wonder if we can essentially
revert that "temporary shallow file" patch and replace it with the
same (or a similar) mechanism uniformly?
On the receive-pack side, the comment at the bottom of
preprare_shallow_update() makes it clear that, if we wanted to use
hooks, we cannot avoid having the proposed new shallow-file in a
temporary file, which is unfortunate. Do we have a similar issue on
hooks on the upload-pack side?
> builtin/pack-objects.c | 7 +++++++
> shallow.c | 2 ++
> t/t5537-fetch-shallow.sh | 13 +++++++++++++
> upload-pack.c | 21 ++++++++++++---------
> 4 files changed, 34 insertions(+), 9 deletions(-)
Nothing for Documentation/ anywhere?
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index c733379..79e848e 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -2467,6 +2467,13 @@ static void get_object_list(int ac, const char **av)
> write_bitmap_index = 0;
> continue;
> }
> + if (starts_with(line, "--shallow ")) {
> + unsigned char sha1[20];
> + if (get_sha1_hex(line + 10, sha1))
> + die("not an SHA-1 '%s'", line + 10);
> + register_shallow(sha1);
> + continue;
> + }
> die("not a rev '%s'", line);
> }
> if (handle_revision_arg(line, &revs, flags, REVARG_CANNOT_BE_FILENAME))
> diff --git a/shallow.c b/shallow.c
> index bbc98b5..41ff4a0 100644
> --- a/shallow.c
> +++ b/shallow.c
> @@ -33,6 +33,8 @@ int register_shallow(const unsigned char *sha1)
> graft->nr_parent = -1;
> if (commit && commit->object.parsed)
> commit->parents = NULL;
> + if (is_shallow == -1)
> + is_shallow = 1;
> return register_commit_graft(graft, 0);
> }
>
> diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
> index 3ae9092..a980574 100755
> --- a/t/t5537-fetch-shallow.sh
> +++ b/t/t5537-fetch-shallow.sh
> @@ -173,4 +173,17 @@ EOF
> )
> '
>
> +test_expect_success POSIXPERM,SANITY 'shallow fetch from a read-only repo' '
> + cp -R .git read-only.git &&
> + find read-only.git -print | xargs chmod -w &&
> + test_when_finished "find read-only.git -type d -print | xargs chmod +w" &&
> + git clone --no-local --depth=2 read-only.git from-read-only &&
> + git --git-dir=from-read-only/.git log --format=%s >actual &&
> + cat >expect <<EOF &&
> +add-1-back
> +4
> +EOF
> + test_cmp expect actual
> +'
> +
> test_done
> diff --git a/upload-pack.c b/upload-pack.c
> index 0c44f6b..a5c50e4 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -70,6 +70,14 @@ static ssize_t send_client_data(int fd, const char *data, ssize_t sz)
> return sz;
> }
>
> +static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
> +{
> + FILE *fp = cb_data;
> + if (graft->nr_parent == -1)
> + fprintf(fp, "--shallow %s\n", sha1_to_hex(graft->sha1));
> + return 0;
> +}
> +
> static void create_pack_file(void)
> {
> struct child_process pack_objects;
> @@ -81,12 +89,10 @@ static void create_pack_file(void)
> const char *argv[12];
> int i, arg = 0;
> FILE *pipe_fd;
> - char *shallow_file = NULL;
>
> if (shallow_nr) {
> - shallow_file = setup_temporary_shallow(NULL);
> argv[arg++] = "--shallow-file";
> - argv[arg++] = shallow_file;
> + argv[arg++] = "";
> }
> argv[arg++] = "pack-objects";
> argv[arg++] = "--revs";
> @@ -114,6 +120,9 @@ static void create_pack_file(void)
>
> pipe_fd = xfdopen(pack_objects.in, "w");
>
> + if (shallow_nr)
> + for_each_commit_graft(write_one_shallow, pipe_fd);
> +
> for (i = 0; i < want_obj.nr; i++)
> fprintf(pipe_fd, "%s\n",
> sha1_to_hex(want_obj.objects[i].item->sha1));
> @@ -242,12 +251,6 @@ static void create_pack_file(void)
> error("git upload-pack: git-pack-objects died with error.");
> goto fail;
> }
> - if (shallow_file) {
> - if (*shallow_file)
> - unlink(shallow_file);
> - free(shallow_file);
> - }
> -
> /* flush the data */
> if (0 <= buffered) {
> data[0] = buffered;
next prev parent reply other threads:[~2014-03-06 18:37 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-27 7:13 [PATCH] upload-pack: allow shallow fetching from a read-only repository Nguyễn Thái Ngọc Duy
2014-02-27 9:04 ` Jeff King
2014-02-27 9:10 ` [PATCH] shallow: verify shallow file after taking lock Jeff King
2014-02-27 9:22 ` Jeff King
2014-02-27 10:18 ` Duy Nguyen
2014-02-27 10:56 ` [PATCH] shallow: use stat_validity to check for up-to-date file Jeff King
2014-02-27 10:11 ` [PATCH] upload-pack: allow shallow fetching from a read-only repository Duy Nguyen
2014-02-27 11:25 ` [PATCH] shallow: automatically clean up shallow tempfiles Jeff King
2014-03-04 12:30 ` [PATCH v2] upload-pack: allow shallow fetching from a read-only repository Nguyễn Thái Ngọc Duy
2014-03-04 18:10 ` Junio C Hamano
2014-03-05 12:43 ` Duy Nguyen
2014-03-05 19:50 ` Junio C Hamano
2014-03-06 8:49 ` [PATCH v3] upload-pack: send shallow info over stdin to pack-objects Nguyễn Thái Ngọc Duy
2014-03-06 18:37 ` Junio C Hamano [this message]
2014-03-06 23:13 ` Duy Nguyen
2014-03-07 18:27 ` Junio C Hamano
2014-03-08 0:08 ` Duy Nguyen
2014-03-10 15:23 ` Junio C Hamano
2014-03-07 1:24 ` Duy Nguyen
2014-03-07 18:33 ` Junio C Hamano
2014-03-11 12:59 ` [PATCH v4] " Nguyễn Thái Ngọc Duy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqfvmvyxzv.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).