Re: [PATCH v5] builtin/clone.c: add --reject-shallow option

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 8366 bytes --]

Hi,

On Mon, 1 Mar 2021, lilinchao@oschina.cn wrote:

> >@@ -1440,6 +1444,8 @@ static void receive_shallow_info(struct fetch_pack_args *args,
> > * shallow. In v0, remote refs that reach these objects are
> > * rejected (unless --update-shallow is set); do the same.
> > */
> >+	if (args->remote_shallow)
> >+	die("source repository is shallow, reject to clone.");
>
> I just found that Johannes Schindelin wrote a document 14 year ago
> in Documentation/technical/shallow.txt:
>
> "There are some unfinished ends of the whole shallow business:
>
> A special handling of a shallow upstream is needed. At some stage,
> upload-pack has to check if it sends a shallow commit, and it should
> send that information early (or fail, if the client does not support 
> shallow repositories). There is no support at all for this in this patch
> series."

Oh wow, what a blast from the past.

I do agree that your patch is an improvement over the current situation.

Thanks,
Johannes

> It seems that my patch can sovle his worry in some degree,
> and maybe we could warn client in fetch-pack stage, if we don't
> choose to reject shallow cloning.
>
> 		if (args->remote_shallow)
> 			die("source repository is shallow, reject to clone.");
> 		else
> 			warning("remote source repository is shallow.");
>
> > prepare_shallow_info(si, shallows);
> > if (si->nr_ours || si->nr_theirs)
> > alternate_shallow_file =
> >diff --git a/fetch-pack.h b/fetch-pack.h
> >index 736a3dae467a..6e4f8f0d738c 100644
> >--- a/fetch-pack.h
> >+++ b/fetch-pack.h
> >@@ -39,6 +39,7 @@ struct fetch_pack_args {
> > unsigned self_contained_and_connected:1;
> > unsigned cloning:1;
> > unsigned update_shallow:1;
> >+	unsigned remote_shallow:1;
> > unsigned deepen:1;
> >
> > /*
> >diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh
> >index 52e5789fb050..6170d0513227 100755
> >--- a/t/t5606-clone-options.sh
> >+++ b/t/t5606-clone-options.sh
> >@@ -5,6 +5,8 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> > export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> >
> > . ./test-lib.sh
> >+. "$TEST_DIRECTORY"/lib-httpd.sh
> >+start_httpd
> >
> > test_expect_success 'setup' '
> >
> >@@ -45,6 +47,51 @@ test_expect_success 'disallows --bare with --separate-git-dir' '
> >
> > '
> >
> >+test_expect_success 'fail to clone http shallow repository' '
> >+	git clone --depth=1 --no-local parent shallow-repo &&
> >+	git clone --bare --no-local shallow-repo "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> >+	test_must_fail git clone --reject-shallow $HTTPD_URL/smart/repo.git out 2>err &&
> >+	test_i18ngrep -e "source repository is shallow, reject to clone." err
> >+
> >+'
> >+
> >+test_expect_success 'fail to clone shallow repository' '
> >+	rm -rf shallow-repo &&
> >+	git clone --depth=1 --no-local parent shallow-repo &&
> >+	test_must_fail git clone --reject-shallow shallow-repo out 2>err &&
> >+	test_i18ngrep -e "source repository is shallow, reject to clone." err
> >+
> >+'
> >+
> >+test_expect_success 'fail to clone non-local shallow repository' '
> >+	rm -rf shallow-repo &&
> >+	git clone --depth=1 --no-local parent shallow-repo &&
> >+	test_must_fail git clone --reject-shallow --no-local shallow-repo out 2>err &&
> >+	test_i18ngrep -e "source repository is shallow, reject to clone." err
> >+
> >+'
> >+
> >+test_expect_success 'clone shallow repository with --no-reject-shallow' '
> >+	rm -rf shallow-repo &&
> >+	git clone --depth=1 --no-local parent shallow-repo &&
> >+	git clone --no-reject-shallow --no-local shallow-repo clone-repo
> >+
> >+'
> >+
> >+test_expect_success 'clone normal repository with --reject-shallow' '
> >+	rm -rf clone-repo &&
> >+	git clone --no-local parent normal-repo &&
> >+	git clone --reject-shallow --no-local normal-repo clone-repo
> >+
> >+'
> >+
> >+test_expect_success 'unspecified any configs or options' '
> >+	rm -rf shallow-repo clone-repo &&
> >+	git clone --depth=1 --no-local parent shallow-repo &&
> >+	git clone shallow-repo clone-repo
> >+
> >+'
> >+
> > test_expect_success 'uses "origin" for default remote name' '
> >
> > git clone parent clone-default-origin &&
> >diff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh
> >index 9f555b87ecdf..da10d3f10352 100755
> >--- a/t/t5611-clone-config.sh
> >+++ b/t/t5611-clone-config.sh
> >@@ -95,6 +95,38 @@ test_expect_success 'clone -c remote.<remote>.fetch=<refspec> --origin=<name>' '
> > test_cmp expect actual
> > '
> >
> >+test_expect_success 'clone.rejectshallow=true should fail to clone' '
> >+	rm -rf child &&
> >+	git clone --depth=1 --no-local . child &&
> >+	test_must_fail git -c clone.rejectshallow=true clone --no-local child out 2>err &&
> >+	test_i18ngrep -e "source repository is shallow, reject to clone." err
> >+'
> >+
> >+test_expect_success 'clone.rejectshallow=false should succeed' '
> >+	rm -rf child out &&
> >+	git clone --depth=1 --no-local . child &&
> >+	git -c clone.rejectshallow=false clone --no-local child out
> >+'
> >+
> >+test_expect_success 'clone.rejectshallow=true should succeed with normal repo' '
> >+	rm -rf child out &&
> >+	git clone --no-local . child &&
> >+	git -c clone.rejectshallow=true clone --no-local child out
> >+'
> >+
> >+test_expect_success 'option --reject-shallow override clone.rejectshallow' '
> >+	rm -rf child out &&
> >+	git clone --depth=1 --no-local . child &&
> >+	test_must_fail git -c clone.rejectshallow=false clone --reject-shallow --no-local child out 2>err &&
> >+	test_i18ngrep -e "source repository is shallow, reject to clone." err
> >+'
> >+
> >+test_expect_success 'option --no-reject-shallow override clone.rejectshallow' '
> >+	rm -rf child out &&
> >+	git clone --depth=1 --no-local . child &&
> >+	git -c clone.rejectshallow=true clone --no-reject-shallow --no-local child out
> >+'
> >+
> > test_expect_success MINGW 'clone -c core.hideDotFiles' '
> > test_commit attributes .gitattributes "" &&
> > rm -rf child &&
> >diff --git a/transport-helper.c b/transport-helper.c
> >index 49b7fb4dcb9a..b57b55c8180c 100644
> >--- a/transport-helper.c
> >+++ b/transport-helper.c
> >@@ -265,7 +265,8 @@ static const char *boolean_options[] = {
> > TRANS_OPT_THIN,
> > TRANS_OPT_KEEP,
> > TRANS_OPT_FOLLOWTAGS,
> >-	TRANS_OPT_DEEPEN_RELATIVE
> >+	TRANS_OPT_DEEPEN_RELATIVE,
> >+	TRANS_OPT_REJECT_SHALLOW
> > };
> >
> > static int strbuf_set_helper_option(struct helper_data *data,
> >diff --git a/transport.c b/transport.c
> >index b13fab5dc3b1..34fe01221ee0 100644
> >--- a/transport.c
> >+++ b/transport.c
> >@@ -236,6 +236,9 @@ static int set_git_option(struct git_transport_options *opts,
> > list_objects_filter_die_if_populated(&opts->filter_options);
> > parse_list_objects_filter(&opts->filter_options, value);
> > return 0;
> >+	} else if (!strcmp(name, TRANS_OPT_REJECT_SHALLOW)) {
> >+	opts->reject_shallow = !!value;
> >+	return 0;
> > }
> > return 1;
> > }
> >@@ -370,6 +373,7 @@ static int fetch_refs_via_pack(struct transport *transport,
> > args.stateless_rpc = transport->stateless_rpc;
> > args.server_options = transport->server_options;
> > args.negotiation_tips = data->options.negotiation_tips;
> >+	args.remote_shallow = transport->smart_options->reject_shallow;
> >
> > if (!data->got_remote_heads) {
> > int i;
> >diff --git a/transport.h b/transport.h
> >index 24e15799e714..f6273d268b09 100644
> >--- a/transport.h
> >+++ b/transport.h
> >@@ -14,6 +14,7 @@ struct git_transport_options {
> > unsigned check_self_contained_and_connected : 1;
> > unsigned self_contained_and_connected : 1;
> > unsigned update_shallow : 1;
> >+	unsigned reject_shallow : 1;
> > unsigned deepen_relative : 1;
> >
> > /* see documentation of corresponding flag in fetch-pack.h */
> >@@ -194,6 +195,9 @@ void transport_check_allowed(const char *type);
> > /* Aggressively fetch annotated tags if possible */
> > #define TRANS_OPT_FOLLOWTAGS "followtags"
> >
> >+/* reject shallow repo transport  */
> >+#define TRANS_OPT_REJECT_SHALLOW "rejectshallow"
> >+
> > /* Accept refs that may update .git/shallow without --depth */
> > #define TRANS_OPT_UPDATE_SHALLOW "updateshallow"
> >
> >
> >base-commit: 225365fb5195e804274ab569ac3cc4919451dc7f
> >--
> >gitgitgadget
> >