Subject: [PATCH v4] builtin/clone.c: add --reject-shallow option

From: "Li Linchao via GitGitGadget" <[email-masked]>
Date: Sun, 21 Feb 2021 07:05:47 +0000

From: lilinchao <[email-masked]>

In some scenarios, users may want more history than the repository
offered for cloning, which happens to be a shallow repository, can
give them. But because users don't know it is a shallow repository
until they download it to local, users should have the option to
refuse to clone this kind of repository, and may want to exit the
process immediately without creating any unnecessary files.

Althought there is an option '--depth=x' for users to decide how
deep history they can fetch, but as the unshallow cloning's depth
is INFINITY, we can't know exactly the minimun 'x' value that can
satisfy the minimum integrity, so we can't pass 'x' value to --depth,
and expect this can obtain a complete history of a repository.

In other scenarios, if we have an API that allow us to import external
repository, and then perform various operations on the repo.
But if the imported is a shallow one(which is actually possible), it
will affect the subsequent operations. So we can choose to refuse to
clone, and let's just import a normal repository.

This patch offers a new option '--reject-shallow' that can reject to
clone a shallow repository.

Signed-off-by: lilinchao <[email-masked]>
---
    builtin/clone.c: add --reject-shallow option
    
    Changes since v1:
    
     * Rename --no-shallow to --reject-shallow
     * Enable to reject a non-local clone
     * Enable --[no-]reject-shallow from CLI override configuration.
     * Add more testcases.
     * Reword commit messages and relative documentation.
    
    Changes since v3:
    
     * Add support to reject clone shallow repo over https protocol
     * Add testcase to reject clone shallow repo over https:// transport
     * Reword commit messages and relative documentation according
       suggestions from Junio.
    
    Signed-off-by: lilinchao [email-masked]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-865%2FCactusinhand%2Fgit-clone-options-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-865/Cactusinhand/git-clone-options-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/865

Range-diff vs v3:

 1:  f5ed6b2e4481 ! 1:  ee4fb840a32f builtin/clone.c: add --reject-shallow option
     @@ Commit message
          builtin/clone.c: add --reject-shallow option
      
          In some scenarios, users may want more history than the repository
     -    offered for cloning, which mostly to be a shallow repository, can
     -    give them. But users don't know it is a shallow repository until
     -    they download it to local, users should have the option to refuse
     -    to clone this kind of repository, and may want to exit the process
     -    immediately without creating any unnecessary files.
     +    offered for cloning, which happens to be a shallow repository, can
     +    give them. But because users don't know it is a shallow repository
     +    until they download it to local, users should have the option to
     +    refuse to clone this kind of repository, and may want to exit the
     +    process immediately without creating any unnecessary files.
      
          Althought there is an option '--depth=x' for users to decide how
          deep history they can fetch, but as the unshallow cloning's depth
     @@ Commit message
          satisfy the minimum integrity, so we can't pass 'x' value to --depth,
          and expect this can obtain a complete history of a repository.
      
     -    In other scenarios, given that we have an API that allow us to import
     -    external repository, and then perform various operations on the repo.
     +    In other scenarios, if we have an API that allow us to import external
     +    repository, and then perform various operations on the repo.
          But if the imported is a shallow one(which is actually possible), it
          will affect the subsequent operations. So we can choose to refuse to
     -    clone, and let's just import an unshallow repository.
     +    clone, and let's just import a normal repository.
      
          This patch offers a new option '--reject-shallow' that can reject to
          clone a shallow repository.
     @@ Documentation/git-clone.txt: SYNOPSIS
       	  [--depth <depth>] [--[no-]single-branch] [--no-tags]
       	  [--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
      -	  [--[no-]remote-submodules] [--jobs <n>] [--sparse]
     -+	  [--[no-]remote-submodules] [--jobs <n>] [--sparse] [--reject-shallow]
     ++	  [--[no-]remote-submodules] [--jobs <n>] [--sparse] [--[no-]reject-shallow]
       	  [--filter=<filter>] [--] <repository>
       	  [<directory>]
       
     @@ Documentation/git-clone.txt: objects from the source repository into a pack in t
       --no-checkout::
       	No checkout of HEAD is performed after the clone is complete.
       
     -+--reject-shallow::
     -+	Don't clone a shallow source repository. In some scenarios, clients
     -+	want the cloned repository information to be complete. Otherwise,
     -+	the cloning process should end immediately without creating any files,
     -+	which can save some disk space. This can override `clone.rejectshallow`
     -+	from the configuration:
     -+
     -+	--------------------------------------------------------------------
     -+	$ git -c clone.rejectshallow=false clone --reject-shallow source out
     -+	--------------------------------------------------------------------
     -+
     -+	While there is a way to countermand a configured "I always refuse to
     -+	clone from a shallow repository" with "but let's allow it only this time":
     -+
     -+	----------------------------------------------------------------------
     -+	$ git -c clone.rejectshallow=true clone --no-reject-shallow source out
     -+	----------------------------------------------------------------------
     ++--[no-]reject-shallow::
     ++	Fail if the source repository is a shallow repository. The
     ++	'clone.rejectshallow' configuration variable can be used to
     ++	give the default.
      +
       --bare::
       	Make a 'bare' Git repository.  That is, instead of
       	creating `<directory>` and placing the administrative
      
       ## builtin/clone.c ##
     +@@
     + #include "connected.h"
     + #include "packfile.h"
     + #include "list-objects-filter-options.h"
     ++#include "shallow.h"
     + 
     + /*
     +  * Overall FIXMEs:
      @@ builtin/clone.c: static int option_no_checkout, option_bare, option_mirror, option_single_branch
       static int option_local = -1, option_no_hardlinks, option_shared;
       static int option_no_tags;
       static int option_shallow_submodules;
     -+static int option_no_shallow = -1;  /* unspecified */
     ++static int option_shallow = -1;    /* unspecified */
      +static int config_shallow = -1;    /* unspecified */
       static int deepen;
       static char *option_template, *option_depth, *option_since;
     @@ builtin/clone.c: static struct option builtin_clone_options[] = {
       	OPT__VERBOSITY(&option_verbosity),
       	OPT_BOOL(0, "progress", &option_progress,
       		 N_("force progress reporting")),
     -+	OPT_BOOL(0, "reject-shallow", &option_no_shallow,
     ++	OPT_BOOL(0, "reject-shallow", &option_shallow,
      +		 N_("don't clone shallow repository")),
       	OPT_BOOL('n', "no-checkout", &option_no_checkout,
       		 N_("don't create a checkout")),
     @@ builtin/clone.c: static int path_exists(const char *path)
       int cmd_clone(int argc, const char **argv, const char *prefix)
       {
       	int is_bundle = 0, is_local;
     -+	int is_shallow = 0;
     ++	int local_shallow = 0;
     ++	int reject_shallow = 0;
       	const char *repo_name, *repo, *work_tree, *git_dir;
       	char *path, *dir, *display_repo = NULL;
       	int dest_exists, real_dest_exists = 0;
      @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
     + 	 */
     + 	git_config(git_clone_config, NULL);
       
     - 	path = get_repo_path(remote->url[0], &is_bundle);
     - 	is_local = option_local != 0 && path && !is_bundle;
     -+
     -+	/* Detect if the remote repository is shallow */
     -+	if (!access(mkpath("%s/shallow", path), F_OK)) {
     -+		is_shallow = 1;
     ++	/*
     ++	 * If option_shallow is specified from CLI option,
     ++	 * ignore config_shallow from git_clone_config.
     ++	 */
     ++	if (config_shallow != -1) {
     ++		reject_shallow = config_shallow;
      +	}
     -+
     - 	if (is_local) {
     - 		if (option_depth)
     - 			warning(_("--depth is ignored in local clones; use file:// instead."));
     ++	if (option_shallow != -1) {
     ++		reject_shallow = option_shallow;
     ++	}
     + 	/*
     + 	 * apply the remote name provided by --origin only after this second
     + 	 * call to git_config, to ensure it overrides all config-based values.
      @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
     - 			warning(_("--shallow-exclude is ignored in local clones; use file:// instead."));
       		if (filter_options.choice)
       			warning(_("--filter is ignored in local clones; use file:// instead."));
     --		if (!access(mkpath("%s/shallow", path), F_OK)) {
     -+		if (is_shallow) {
     + 		if (!access(mkpath("%s/shallow", path), F_OK)) {
     ++			local_shallow = 1;
       			if (option_local > 0)
       				warning(_("source repository is shallow, ignoring --local"));
       			is_local = 0;
      @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
     + 			goto cleanup;
       	}
     - 	if (option_local > 0 && !is_local)
     - 		warning(_("--local is ignored"));
     -+
     -+	if (is_shallow) {
     -+		int reject = 0;
     -+
     -+		/* If option_no_shallow is specified from CLI option,
     -+		 * ignore config_shallow from git_clone_config.
     -+		 */
     -+
     -+		if (config_shallow != -1) {
     -+			reject = config_shallow;
     -+		}
     -+		if (option_no_shallow != -1) {
     -+			reject = option_no_shallow;
     -+		}
     -+		if (reject) {
     + 
     ++	if (reject_shallow) {
     ++		if (local_shallow || is_repository_shallow(the_repository)) {
      +			die(_("source repository is shallow, reject to clone."));
      +		}
      +	}
      +
     - 	transport->cloning = 1;
     - 
     - 	transport_set_option(transport, TRANS_OPT_KEEP, "yes");
     + 	update_remote_refs(refs, mapped_refs, remote_head_points_at,
     + 			   branch_top.buf, reflog_msg.buf, transport,
     + 			   !is_local);
      
       ## t/t5606-clone-options.sh ##
     +@@ t/t5606-clone-options.sh: 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' '
     + 
      @@ t/t5606-clone-options.sh: 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
     @@ t/t5611-clone-config.sh: test_expect_success 'clone -c remote.<remote>.fetch=<re
      +	test_i18ngrep -e "source repository is shallow, reject to clone." err
      +'
      +
     -+test_expect_success 'clone.rejectshallow=false should succeed cloning' '
     ++test_expect_success 'clone.rejectshallow=false should succeed' '
      +	rm -rf child &&
      +	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 cloning normal repo' '
     ++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
     @@ t/t5611-clone-config.sh: test_expect_success 'clone -c remote.<remote>.fetch=<re
      +test_expect_success 'option --reject-shallow override clone.rejectshallow' '
      +	rm -rf child out &&
      +	git clone --depth=1 --no-local . child &&
     -+	test_must_fail git clone -c clone.rejectshallow=false  --reject-shallow --no-local child out 2>err &&
     ++	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' '
     ++test_expect_success 'option --no-reject-shallow override clone.rejectshallow' '
      +	rm -rf child &&
      +	git clone --depth=1 --no-local . child &&
      +	git -c clone.rejectshallow=true clone --no-reject-shallow --no-local child out

 Documentation/config/clone.txt |  4 +++
 Documentation/git-clone.txt    |  7 ++++-
 builtin/clone.c                | 27 +++++++++++++++++++
 t/t5606-clone-options.sh       | 47 ++++++++++++++++++++++++++++++++++
 t/t5611-clone-config.sh        | 32 +++++++++++++++++++++++
 5 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/Documentation/config/clone.txt b/Documentation/config/clone.txt
index 47de36a5fedf..50ebc170bb81 100644
--- a/Documentation/config/clone.txt
+++ b/Documentation/config/clone.txt
@@ -2,3 +2,7 @@ clone.defaultRemoteName::
 	The name of the remote to create when cloning a repository.  Defaults to
 	`origin`, and can be overridden by passing the `--origin` command-line
 	option to linkgit:git-clone[1].
+
+clone.rejectshallow::
+	Reject to clone a repository if it is a shallow one, can be overridden by
+	passing option `--reject-shallow` in command line. See linkgit:git-clone[1]
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 02d9c19cec75..cb458123eef6 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -15,7 +15,7 @@ SYNOPSIS
 	  [--dissociate] [--separate-git-dir <git dir>]
 	  [--depth <depth>] [--[no-]single-branch] [--no-tags]
 	  [--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
-	  [--[no-]remote-submodules] [--jobs <n>] [--sparse]
+	  [--[no-]remote-submodules] [--jobs <n>] [--sparse] [--[no-]reject-shallow]
 	  [--filter=<filter>] [--] <repository>
 	  [<directory>]
 
@@ -149,6 +149,11 @@ objects from the source repository into a pack in the cloned repository.
 --no-checkout::
 	No checkout of HEAD is performed after the clone is complete.
 
+--[no-]reject-shallow::
+	Fail if the source repository is a shallow repository. The
+	'clone.rejectshallow' configuration variable can be used to
+	give the default.
+
 --bare::
 	Make a 'bare' Git repository.  That is, instead of
 	creating `<directory>` and placing the administrative
diff --git a/builtin/clone.c b/builtin/clone.c
index 51e844a2de0a..6807eb954366 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -32,6 +32,7 @@
 #include "connected.h"
 #include "packfile.h"
 #include "list-objects-filter-options.h"
+#include "shallow.h"
 
 /*
  * Overall FIXMEs:
@@ -50,6 +51,8 @@ static int option_no_checkout, option_bare, option_mirror, option_single_branch
 static int option_local = -1, option_no_hardlinks, option_shared;
 static int option_no_tags;
 static int option_shallow_submodules;
+static int option_shallow = -1;    /* unspecified */
+static int config_shallow = -1;    /* unspecified */
 static int deepen;
 static char *option_template, *option_depth, *option_since;
 static char *option_origin = NULL;
@@ -90,6 +93,8 @@ static struct option builtin_clone_options[] = {
 	OPT__VERBOSITY(&option_verbosity),
 	OPT_BOOL(0, "progress", &option_progress,
 		 N_("force progress reporting")),
+	OPT_BOOL(0, "reject-shallow", &option_shallow,
+		 N_("don't clone shallow repository")),
 	OPT_BOOL('n', "no-checkout", &option_no_checkout,
 		 N_("don't create a checkout")),
 	OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")),
@@ -858,6 +863,9 @@ static int git_clone_config(const char *k, const char *v, void *cb)
 		free(remote_name);
 		remote_name = xstrdup(v);
 	}
+	if (!strcmp(k, "clone.rejectshallow")) {
+		config_shallow = git_config_bool(k, v);
+	}
 	return git_default_config(k, v, cb);
 }
 
@@ -963,6 +971,8 @@ static int path_exists(const char *path)
 int cmd_clone(int argc, const char **argv, const char *prefix)
 {
 	int is_bundle = 0, is_local;
+	int local_shallow = 0;
+	int reject_shallow = 0;
 	const char *repo_name, *repo, *work_tree, *git_dir;
 	char *path, *dir, *display_repo = NULL;
 	int dest_exists, real_dest_exists = 0;
@@ -1156,6 +1166,16 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	 */
 	git_config(git_clone_config, NULL);
 
+	/*
+	 * If option_shallow is specified from CLI option,
+	 * ignore config_shallow from git_clone_config.
+	 */
+	if (config_shallow != -1) {
+		reject_shallow = config_shallow;
+	}
+	if (option_shallow != -1) {
+		reject_shallow = option_shallow;
+	}
 	/*
 	 * apply the remote name provided by --origin only after this second
 	 * call to git_config, to ensure it overrides all config-based values.
@@ -1216,6 +1236,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		if (filter_options.choice)
 			warning(_("--filter is ignored in local clones; use file:// instead."));
 		if (!access(mkpath("%s/shallow", path), F_OK)) {
+			local_shallow = 1;
 			if (option_local > 0)
 				warning(_("source repository is shallow, ignoring --local"));
 			is_local = 0;
@@ -1363,6 +1384,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 			goto cleanup;
 	}
 
+	if (reject_shallow) {
+		if (local_shallow || is_repository_shallow(the_repository)) {
+			die(_("source repository is shallow, reject to clone."));
+		}
+	}
+
 	update_remote_refs(refs, mapped_refs, remote_head_points_at,
 			   branch_top.buf, reflog_msg.buf, transport,
 			   !is_local);
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..18268256bfe0 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 &&
+	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 &&
+	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 &&

base-commit: 2283e0e9af55689215afa39c03beb2315ce18e83
-- 
gitgitgadget