builtin/clone.c: add --no-shallow option

From: lilinchao <[email protected]>

This patch add a new option that reject to clone a shallow repository.

Clients don't know it's a shallow repository until they download it locally, in some scenariors, clients just don't want to clone this kind

of repository, and want to exit the process immediately without creating any unnecessary files.

builtin/clone.c +1 -0

@@ -963,6 +968,7 @@ 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;
 	const char *repo_name, *repo, *work_tree, *git_dir;
 	char *path, *dir, *display_repo = NULL;
 	int dest_exists, real_dest_exists = 0;

t/t5606-clone-options.sh +0 -0

index 7f082fb23b6a..9d310dbb158a 100755

Quoted 3 lines · Li Linchao via GitGitGadget

From: lilinchao <[email protected]>

This patch add a new option that reject to clone a shallow repository.

A canonical form of our log message starts by explaining the need, and then presents the solution at the end.

Quoted 3 lines

Clients don't know it's a shallow repository until they download it locally, in some scenariors, clients just don't want to clone this kind

"scenarios".  "in some scenarios" would have to be clarified a bit more to justify why it is a good idea to have such a feature.

Quoted 3 lines

of repository, and want to exit the process immediately without creating any unnecessary files.

"clients don't know it's a shallow repository until they download" leading to "so let's reject immediately upon finding out that they are shallow" does make sense as a flow of thought, though.

Documentation/git-clone.txt objects from the source repository into a pack in the cloned repository. · line 149

Quoted 2 lines

+--no-shallow::

Documentation/git-clone.txt +1 -0

+	Don't clone a shallow source repository. In some scenariors, clients

"scenarios" (no 'r').

builtin/clone.c

Quoted 3 lines

builtin/clone.c +0 -0

old mode 100644
new mode 100755

Unwarranted "chmod +x"; accidents do happen, but please be careful before making what you did public ;-)

builtin/clone.c static struct option builtin_clone_options[] = { · line 97

Quoted 9 lines

builtin/clone.c +1 -0

@@ -90,6 +91,7 @@ static struct option builtin_clone_options[] = {
   OPT__VERBOSITY(&option_verbosity),
   OPT_BOOL(0, "progress", &option_progress,
   N_("force progress reporting")),
+	OPT_BOOL(0, "no-shallow", &option_no_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")),

It is a bad idea to give a name that begins with "no-" to an option whose default can be tweaked by a configuration variable [*].  If the configuration is named "rejectshallow", perhaps it is better to call it "--reject-shallow" instead.

This is because configured default must be overridable from the command line.  I.e. even if you have in your ~/.gitconfig this:

    [clone]
        rejectshallow = true

you should be able to say "allow it only this time", with

$ git clone --no-reject-shallow http://github.com/git/git/ git

and you do not want to have to say "--no-no-shallow", which sounds just silly.

Side note. it is a bad idea in general, even if the option does not have corresponding configuration variable.  The existing "no-checkout" is a historical accident that happened long time ago and cannot be removed due to compatibility.  Let's not introduce a new option that follows such a bad pattern.

Quoted 9 lines

builtin/clone.c +1 -0

@@ -963,6 +968,7 @@ 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;
   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) · line 1227

Quoted 7 lines

builtin/clone.c +1 -0

@@ -1215,6 +1221,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)) {
+	is_shallow = 1;
   if (option_local > 0)
   warning(_("source repository is shallow, ignoring --local"));
   is_local = 0;

This change is to the local clone codepath.  Cloning over the wire would not go through this part.  And throughout the patch, this is the only place that sets is_shallow to 1.

Also let's note that this is after we called parse_options(), so the value of option_no_shallow is known at this point.

So, this patch does not even *need* to introduce a new "is_shallow" variable at all.  It only needs to add

                        if (option_no_shallow)
                                die(...);

instead of adding "is_shallow = 1" to the above hunk.

I somehow think that this is only half a feature---wouldn't it be more useful if we also rejected a non-local clone from a shallow repository?

And for that ...

Quoted 9 lines

t/t5606-clone-options.sh +0 -0

index 7f082fb23b6a..9d310dbb158a 100755
@@ -42,6 +42,13 @@ test_expect_success 'disallows --bare with --separate-git-dir' '

  '

t/t5606-clone-options.sh test_expect_success 'disallows --bare with --separate-git-dir' ' · line 51

Quoted 7 lines

t/t5606-clone-options.sh +7 -0

+test_expect_success 'reject clone shallow repository' '
+	git clone --depth=1 --no-local parent shallow-repo &&
+	test_must_fail git clone --no-shallow shallow-repo out 2>err &&
+	test_i18ngrep -e "source repository is shallow, reject to clone." err
+
+'
+

... in addition to the test for a local clone above, you'd also want to test a non-local clone, perhaps like so:

test_expect_success 'reject clone shallow repository' '

	rm -fr shallow-repo &&
	git clone --depth=1 --no-local parent shallow-repo &&
	test_must_fail git clone --no-shallow --no-local shallow-repo out 2>err &&
	test_i18ngrep -e "source repository is shallow, reject to clone." err

'

Ditto for the other test script.

Also, you would want to make sure that the command line overrides the configured default.  I.e.

	git -c clone.rejectshallow=false clone --reject-shallow

should refuse to clone from a shallow one, while there should be a way to countermand a configured "I always refuse to clone from a shallow repository" with "but let's allow it only this time", i.e.

	git -c clone.rejectshallow=true clone --no-reject-shallow

or something along the line.

t/t5611-clone-config.sh test_expect_success 'clone -c remote.<remote>.fetch=<refspec> --origin=<name>' ' · line 98

Quoted 12 lines

t/t5611-clone-config.sh +0 -0

index 8e0fd398236b..3aab86ad4def 100755
@@ -92,6 +92,13 @@ test_expect_success 'clone -c remote.<remote>.fetch=<refspec> --origin=<name>' '

  test_cmp expect actual
  '

t/t5611-clone-config.sh +4 -0

+test_expect_success 'clone -c clone.rejectshallow' '
+	rm -rf child &&
+	git clone --depth=1 --no-local . child &&
+	test_must_fail git clone -c clone.rejectshallow child out 2>err &&

This is not quite right, even though it may happen to work.  The "clone.rejectshallow" variable is a configuration about what should happen when creating a new repository by cloning, so letting "git clone -c var[=val]" to set the variable _in_ the resulting repository would not make much sense.  Even if the clone succeeded, nobody would look at that particular configuration variable that is set in the resulting repository.

I think it would communicate to the readers better what we are trying to do, if we write

test_must_fail git -c clone.rejectshallow=true clone child out

instead.

Thanks.

Hi,

in addition to what Junio said:

On Thu, 4 Feb 2021, Li Linchao via GitGitGadget wrote:

builtin/clone.c static int git_clone_config(const char *k, const char *v, void *cb) · line 864

Quoted 10 lines

builtin/clone.c +2 -0

index e335734b4cfd..b07d867e6641
@@ -858,6 +860,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")) {
+		option_no_shallow = 1;

This needs to use git_config_bool(k, v) to allow for clone.rejectShallow = false.

I found an issue described like this:

The blame information can be completely wrong when fetching it from a shallow clone, without errors or warnings. When the outcome is invalid data, it's extremely difficult to diagnose that it comes from a shallow clone. If a line in a file was not changed in the commits that were downloaded as part of the shallow fetch, git will report the first known commit as the author. This has a big impact on the auto-assignment of new issues.

It looks like this is another scenario that can prove this feature is necessary.

After I applied your review suggestions above, then we can reject a non-local clone from shallow repo. For now, it will clone a empty repo with --no-local option.

Range-diff vs v1:

 1:  594323684af0 = 1:  2f9602495eb5 builtin/clone.c: add --no-shallow option
 -:  ------------ > 2:  cfcfc3ec6b37 builtin/clone.c: add --reject-shallow option

Quoted 4 lines

Range-diff vs v1:

  1:  594323684af0 = 1:  2f9602495eb5 builtin/clone.c: add --no-shallow option -:  ------------ > 2:  cfcfc3ec6b37 builtin/clone.c: add --reject-shallow option

You should rewrite your history to make this one commit again. You can do this by running 'git rebase -i HEAD~2' then editing the todo file to 'squash' the second commit in to the first. You will be prompted to edit the combined commit message, and you should preserve only the message from the second commit.

Thanks,
-Stolee

From: lilinchao <[email protected]>

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.

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, given that we have an API that allow us to import external repository, and then perform various operations on the repo.

Documentation/git-clone.txt +7 -0

+--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:
+

Documentation/git-clone.txt +2 -0

@@ -90,6 +92,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_no_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")),

Documentation/git-clone.txt +1 -0

@@ -963,6 +970,7 @@ 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;
 	const char *repo_name, *repo, *work_tree, *git_dir;
 	char *path, *dir, *display_repo = NULL;
 	int dest_exists, real_dest_exists = 0;

Documentation/git-clone.txt +4 -0

@@ -1222,6 +1236,25 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	}
 	if (option_local > 0 && !is_local)
 		warning(_("--local is ignored"));
+
+	if (is_shallow) {
+		int reject = 0;
+

Documentation/git-clone.txt +5 -0

+		if (reject) {
+			die(_("source repository is shallow, reject to clone."));
+		}
 
+	}
+
 	transport->cloning = 1;
 

transport_set_option(transport, TRANS_OPT_KEEP, "yes");

Quoted 5 lines · Li Linchao via GitGitGadget

From: lilinchao <[email protected]>

In some scenarios, users may want more history than the repository offered for cloning, which mostly to be a shallow repository, can give them.

Sorry, but I find this hard to understand.  Are you saying that most of the repositories that users try to clone from are shallow?

Quoted 4 lines

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.

This one on the other hand is easy to understand, but we would probably need something like s/But/But because/.

Quoted 5 lines

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.

Hmph, that is an interesting point.  This makes me wonder if we can achieve the same without adding a new option at the UI level (e.g. by allowing "--depth" to take "infinity" and reject cloning if we find out that the origin repository is a shallow one).  But we can worry about it later once after we get the machinery driven by the UI right.

Quoted 2 lines

In other scenarios, given that we have an API that allow us to import external repository, and then perform various operations on the repo.

Sorry, but I do not understand what you want to say with these two lines ("Given that X and Y" needs to be followed by a concluding statement, e.g. "Given that we have API to import and operate, we can do Z"---you are missing that "we can do Z" part).

Quoted 6 lines

Documentation/git-clone.txt +0 -0

index 02d9c19cec75..af5a97903a05 100644
@@ -15,7 +15,7 @@ SYNOPSIS

    [--dissociate] [--separate-git-dir <git dir>]

Documentation/git-clone.txt SYNOPSIS · line 18

Quoted 4 lines

    [--depth <depth>] [--[no-]single-branch] [--no-tags] [--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]

Documentation/git-clone.txt +1 -1

-	  [--[no-]remote-submodules] [--jobs <n>] [--sparse]
+	  [--[no-]remote-submodules] [--jobs <n>] [--sparse] [--reject-shallow]

Isn't it "--[no-]reject-shallow"?  Offering the negation from the command line is essential if "[clone] rejectshallow" configuration is allowed to set the default to true.

Quoted 7 lines

+--reject-shallow::

Documentation/git-clone.txt +6 -0

+	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:
+

Documentation/git-clone.txt objects from the source repository into a pack in the cloned repository. · line 168

Quoted 10 lines

Documentation/git-clone.txt +10 -0

+	--------------------------------------------------------------------
+	$ 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
+	----------------------------------------------------------------------

This is way too verbose and gives unnecessary details that readers already know or do not need to know (e.g. setting configuration from the command line and immediately override it from the command line is not something end-users would EVER need to do---only test writers who develop Git would need it).  Something like

    Fail if the source repository is a shallow repository.  The clone.rejectShallow configuration variable can be used to give the default.

would be sufficient.  All readers ought to know when a configuration and command line option exist, the latter can be used to override the default former gives, and it is *not* a job for the description of an individual option to teach them to such a detail like the above does.

Documentation/git-clone.txt

Quoted 2 lines

Documentation/git-clone.txt +2 -0

+static int option_no_shallow = -1;  /* unspecified */
+static int config_shallow = -1;    /* unspecified */

Hmph.  I would have expected the usual "prepare a single variable and initialize it to the default, read the config to set it, and then parse the command line to overwrite it" sequence would suffice so it is puzzling why we want two separate variables here.

Let's read on to find out.

Quoted 10 lines

Documentation/git-clone.txt +2 -0

@@ -90,6 +92,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_no_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")),

Documentation/git-clone.txt static int git_clone_config(const char *k, const char *v, void *cb) · line 869

Quoted 8 lines

Documentation/git-clone.txt +3 -0

@@ -858,6 +862,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);
   }

You are adding to git_clone_config(), so instead of setting the value to config_shallow, setting the value to the same variable that will be used in builtin_clone_options[] array should be sufficient.

cmd_clone() begins like so:

	git_config(git_clone_config, NULL);
	argc = parse_options(...);

which means that single variable (let's call it reject_shallow) can (1) stay to be its initial value if no config or option is given, (2) if there is config, the git_config() call will cause that variable assigned, (3) if there is option, parse_options() call will cause that variable assigned, possibly overwriting the value taken from the config.

Which is exactly what we want.  So in short, declare just a single

    static int reject_shallow; /* default to false */

instead of "option_no_shallow" and "config_shallow", and use it in both builtin_clone_options[] given to parse_options, and git_clone_config() that is given to git_config(), and you'd be fine, I think.

Quoted 12 lines

Documentation/git-clone.txt +1 -0

@@ -963,6 +970,7 @@ 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;
   const char *repo_name, *repo, *work_tree, *git_dir;
   char *path, *dir, *display_repo = NULL;
   int dest_exists, real_dest_exists = 0;

@@ -1205,6 +1213,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)

  path = get_repo_path(remote->url[0], &is_bundle); is_local = option_local != 0 && path && !is_bundle;

Documentation/git-clone.txt

Quoted 5 lines

Documentation/git-clone.txt +5 -0

+
+	/* Detect if the remote repository is shallow */
+	if (!access(mkpath("%s/shallow", path), F_OK)) {
+	is_shallow = 1;
+	}

This is only for cloning from a local repository, no?  IOW, path at this point may even be "git://github.com/git/git/" and checking with access() does not make sense.

Ah, it is even worse.  get_repo_path() can return NULL, so mkpath() will crash in such a case.  This must be at least

	if (path && !access(mkpath("%s/shallow", path), F_OK))
	is_shallow = 1;

but I think the logic fits better in the body of "if (is_Local)" thing that immediately follows.  It is specific to the case where cloning from a local repository and access(mkpath()) that is about the local filesystem (as opposed to going through the transport layer) belongs there.

Quoted 10 lines

  if (is_local) {
  if (option_depth)
  warning(_("--depth is ignored in local clones; use file:// instead."));

Documentation/git-clone.txt +1 -1

@@ -1214,7 +1228,7 @@ 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 (option_local > 0)

Documentation/git-clone.txt int cmd_clone(int argc, const char **argv, const char *prefix) · line 1234

Quoted 2 lines

Documentation/git-clone.txt +0 -0

   warning(_("source repository is shallow, ignoring --local"));
   is_local = 0;

So, I think the above two hunks are making the code worse.  If we are to detect and reject cloning from the shallow repository when going through the transport layer (i.e. "--no-local" or cloning from "git://github.com/git/git", or "https://github.com/git/git", if it were a shallow repository), that must be handled separately.

Quoted 8 lines

Documentation/git-clone.txt +4 -0

@@ -1222,6 +1236,25 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
   }
   if (option_local > 0 && !is_local)
   warning(_("--local is ignored"));
+
+	if (is_shallow) {
+	int reject = 0;
+

Documentation/git-clone.txt int cmd_clone(int argc, const char **argv, const char *prefix) · line 1252

Quoted 10 lines

Documentation/git-clone.txt +10 -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;
+	}

I do not think any of the above is necessary with just a single reject_shallow variable that is initialized to 0, can be set by git_config() callback, and can further be set by parse_options().

Quoted 9 lines

Documentation/git-clone.txt +5 -0

+	if (reject) {
+	die(_("source repository is shallow, reject to clone."));
+	}
 
+	}
+
   transport->cloning = 1;

  transport_set_option(transport, TRANS_OPT_KEEP, "yes");

I do not see how this change would allow users to reject cloning
http://github.com/git/git, if that repository were shallow, though. I think that would need changes to the code that interacts with these transport_* functions we see later part of this functrion.

Thanks.

Sorry, you may forget there is a re-read git-config under these lines

Quoted 9 lines

This is only for cloning from a local repository, no?  IOW, path at this point may even be "git://github.com/git/git/" and checking with access() does not make sense.

Ah, it is even worse.  get_repo_path() can return NULL, so mkpath() will crash in such a case.  This must be at least

	if (path && !access(mkpath("%s/shallow", path), F_OK))
	is_shallow = 1;

Quoted 8 lines

...

So, I think the above two hunks are making the code worse.  If we are to detect and reject cloning from the shallow repository when going through the transport layer (i.e. "--no-local" or cloning from "git://github.com/git/git", or "https://github.com/git/git", if it were a shallow repository), that must be handled separately.

Sorry, I made the question simple. Reject cloning a shallow repository should apply to all four type transport protocols. There still a bunch of work to be done.

Quoted 1 line · "[email protected]" <[email protected]> writes:

Sorry, you may forget there is a re-read git-config under these lines

Ahhhh, thanks.  You're right that "clone" is "special" in that way and requires unusual order---I forgot about that.

Quoted 17 lines

Quoted 9 lines

This is only for cloning from a local repository, no?  IOW, path at this point may even be "git://github.com/git/git/" and checking with access() does not make sense.

Ah, it is even worse.  get_repo_path() can return NULL, so mkpath() will crash in such a case.  This must be at least

	if (path && !access(mkpath("%s/shallow", path), F_OK))
	is_shallow = 1;

Quoted 8 lines

...

So, I think the above two hunks are making the code worse.  If we are to detect and reject cloning from the shallow repository when going through the transport layer (i.e. "--no-local" or cloning from "git://github.com/git/git", or "https://github.com/git/git", if it were a shallow repository), that must be handled separately.

Quoted 3 lines

Sorry, I made the question simple. Reject cloning a shallow repository should apply to all four type transport protocols. There still a bunch of work to be done.

Sorry, but I didn't realize that this is just a work-in-progress that shows an early "only local transport is covered" preview.  I think, modulo that the access(mkpath()) thing should be in the "if (is_local)" block, the patch covers the case of local transport well enough.  I think by "four types of transport" you mean ssh://, git://
https:// etc., but they all go through the same transport API, so hopefully once one of them, say, git://, is covered, that would take us a long way.

Thanks.

[jc: I've CC'ed Jonathan Tan, who is much more knowledgeable than I am on the transport layer issues, to sanity check my assumption]

builtin/clone.c int cmd_clone(int argc, const char **argv, const char *prefix) · line 1388

Quoted 6 lines · Li Linchao via GitGitGadget

builtin/clone.c +0 -0

@@ -1363,6 +1384,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
   goto cleanup;
   }

builtin/clone.c +2 -0

+	if (reject_shallow) {
+	if (local_shallow || is_repository_shallow(the_repository)) {

This may reject to clone from a shallow repository, but at this point the bulk of the tranfer from the origin repository has already happened, no?  Rejecting after transferring many megabytes feels a bit too late.  That is one of the reasons why I kept hinting that the transport layer needs to be taught an option to reject talking to a shallow counterpart if we want to add this feature [*1*].

Also, wouldn't "clone --depth=1 --reject-shallow" from a repository that is not shallow make the_repository a shallow one at this point and makes it fail?  If the goal of the --reject-shallow option were to make sure the resulting repository is not shallow, then that is a technically correct implementation (even though it is wasteful to transfer a full tree worth of megabytes and then abort), but is the feature is explained to reject cloning from a shallow one, then users would be suprised to see ...

builtin/clone.c int cmd_clone(int argc, const char **argv, const char *prefix) · line 1389

Quoted 1 line

builtin/clone.c +1 -0

+	die(_("source repository is shallow, reject to clone."));

... this message, when cloning from well known publich repositories that are not shallow.

I think cloning with --depth=<n> when the source repository is deep enough, should be allowed, so the cleanest solution for the latter may be to notice the combination of options that make the resulting repository shallow (I mentioned --depth=<n>, but there may be others) and the --reject-shallow option and error out before even talking to the other side at the time we parse the command line.

Thanks.

[Footnote]

*1* Looking at Documentation/technical/pack-protocol.txt, "git fetch" seem to learn if the repository is shallow immediately upon contacting "upload-pack" during the Reference Discovery phase (we'd see 'shallow' packets if they are shallow). I suspect that the right solution would be to teach the codepath on the "git fetch" side that accepts, parses, and acts on this packet to optionally stop communication and error out when the caller asks not to talk with a shallow repository.

From: lilinchao <[email protected]>

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 protected]>

---
    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 protected]

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

Range-diff vs v4:

1:  ee4fb840a32f ! 1:  3f765e49e4a7 builtin/clone.c: add --reject-shallow option
     @@ Documentation/git-clone.txt: objects from the source repository into a pack in t 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;
     @@ 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 local_shallow = 0;
      + int reject_shallow = 0;
       const char *repo_name, *repo, *work_tree, *git_dir;
       char *path, *dir, *display_repo = NULL;
     @@ builtin/clone.c: 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 (reject_shallow)
     ++ die("source repository is shallow, reject to clone.");
       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 (reject_shallow) {
     -+ if (local_shallow || is_repository_shallow(the_repository)) {
     -+ die(_("source repository is shallow, reject to clone."));
     -+ }
     -+ }
     + transport_set_option(transport, TRANS_OPT_KEEP, "yes");
     +
     ++ if (reject_shallow)
     ++ transport_set_option(transport, TRANS_OPT_REJECT_SHALLOW, "1");
     + if (option_depth)
     + transport_set_option(transport, TRANS_OPT_DEPTH,
     +      option_depth);
     +
     + ## fetch-pack.c ##
     +@@ fetch-pack.c: static struct ref *do_fetch_pack(struct fetch_pack_args *args,
     +
     + if (args->stateless_rpc)
     + packet_flush(fd[1]);
     ++
     ++ if (!args->deepen && args->remote_shallow)
     ++ 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);
     + if (args->deepen)
     + setup_alternate_shallow(&shallow_lock, &alternate_shallow_file,
     + NULL);
     +@@ fetch-pack.c: 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.");
     + prepare_shallow_info(si, shallows);
     + if (si->nr_ours || si->nr_theirs)
     + alternate_shallow_file =
     +
     + ## fetch-pack.h ##
     +@@ fetch-pack.h: struct fetch_pack_args {
     + unsigned self_contained_and_connected:1;
     + unsigned cloning:1;
     + unsigned update_shallow:1;
     ++ unsigned remote_shallow:1;
     + unsigned deepen:1;
     +
     + /*

       ## t/t5606-clone-options.sh ##
      @@ t/t5606-clone-options.sh: GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
     @@ t/t5611-clone-config.sh: test_expect_success 'clone -c remote.<remote>.fetch=<re +'
      +
      +test_expect_success 'clone.rejectshallow=false should succeed' '
     -+ rm -rf child &&
     ++ rm -rf child out &&
      + git clone --depth=1 --no-local . child &&
      + git -c clone.rejectshallow=false clone --no-local child out
      +'
     @@ t/t5611-clone-config.sh: test_expect_success 'clone -c remote.<remote>.fetch=<re +'
      +
      +test_expect_success 'option --no-reject-shallow override clone.rejectshallow' '
     -+ rm -rf child &&
     ++ rm -rf child out &&
      + git clone --depth=1 --no-local . child &&
      + git -c clone.rejectshallow=true clone --no-reject-shallow --no-local child out
      +'
     @@ t/t5611-clone-config.sh: test_expect_success 'clone -c remote.<remote>.fetch=<re test_expect_success MINGW 'clone -c core.hideDotFiles' '
       test_commit attributes .gitattributes "" &&
       rm -rf child &&
     +
     + ## transport-helper.c ##
     +@@ transport-helper.c: 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,
     +
     + ## transport.c ##
     +@@ transport.c: 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;
     + }
     +@@ transport.c: 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;
     +
     + ## transport.h ##
     +@@ transport.h: 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 */
     +@@ transport.h: 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"
     +

Documentation/config/clone.txt |  4 +++ Documentation/git-clone.txt    |  7 ++++-
builtin/clone.c                | 22 ++++++++++++++++ fetch-pack.c                   |  6 +++++
fetch-pack.h                   |  1 + t/t5606-clone-options.sh       | 47 ++++++++++++++++++++++++++++++++++ t/t5611-clone-config.sh        | 32 +++++++++++++++++++++++ transport-helper.c             |  3 ++-
transport.c                    |  4 +++
transport.h                    |  4 +++

10 files changed, 128 insertions(+), 2 deletions(-)

Documentation/config/clone.txt +0 -0

index 47de36a5fedf..50ebc170bb81 100644
@@ -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].

Documentation/config/clone.txt +4 -0

+
+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]

Documentation/git-clone.txt +0 -0

index 02d9c19cec75..cb458123eef6 100644
@@ -15,7 +15,7 @@ SYNOPSIS

  [--dissociate] [--separate-git-dir <git dir>] [--depth <depth>] [--[no-]single-branch] [--no-tags] [--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]

Documentation/git-clone.txt +1 -1

-	  [--[no-]remote-submodules] [--jobs <n>] [--sparse]
+	  [--[no-]remote-submodules] [--jobs <n>] [--sparse] [--[no-]reject-shallow]

  [--filter=<filter>] [--] <repository>
  [<directory>]

Documentation/git-clone.txt +0 -1

@@ -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::

Documentation/git-clone.txt +4 -1

+	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 +22 -0

index 51e844a2de0a..70d8ca77988f 100644
@@ -50,6 +50,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 +92,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 +862,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 +970,7 @@ 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 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 +1164,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 +1234,8 @@ 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)) {
+	if (reject_shallow)
+	die("source repository is shallow, reject to clone.");
 if (option_local > 0)
 warning(_("source repository is shallow, ignoring --local"));
 is_local = 0;

@@ -1227,6 +1247,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 transport_set_option(transport, TRANS_OPT_KEEP, "yes");
 
+	if (reject_shallow)
+	transport_set_option(transport, TRANS_OPT_REJECT_SHALLOW, "1");
 if (option_depth)
 transport_set_option(transport, TRANS_OPT_DEPTH,
      option_depth);

fetch-pack.c +0 -0

index 1eaedcb5dc2e..41ef700bde38 100644

fetch-pack.c +0 -0

 prepare_shallow_info(si, shallows);
 if (si->nr_ours || si->nr_theirs)

alternate_shallow_file =

fetch-pack.h +1 -0

index 736a3dae467a..6e4f8f0d738c 100644
@@ -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;
 
 /*

t/t5606-clone-options.sh +0 -0

index 52e5789fb050..6170d0513227 100755
@@ -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' '

t/t5606-clone-options.sh +0 -0

@@ -45,6 +47,51 @@ test_expect_success 'disallows --bare with --separate-git-dir' '
 

'

t/t5606-clone-options.sh +45 -0

+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 &&

t/t5611-clone-config.sh +0 -0

index 9f555b87ecdf..da10d3f10352 100755
@@ -95,6 +95,38 @@ test_expect_success 'clone -c remote.<remote>.fetch=<refspec> --origin=<name>' '

test_cmp expect actual
'

t/t5611-clone-config.sh +32 -0

+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 &&

transport-helper.c +2 -1

index 49b7fb4dcb9a..b57b55c8180c 100644
@@ -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,

transport.c +4 -0

index b13fab5dc3b1..34fe01221ee0 100644
@@ -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;

transport.h +4 -0

index 24e15799e714..f6273d268b09 100644
@@ -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

This may reject to clone from a shallow repository, but at this point the bulk of the tranfer from the origin repository has already happened, no? Rejecting after transferring many megabytes feels a bit too late. That is one of the reasons why I kept hinting that the transport layer needs to be taught an option to reject talking to a shallow counterpart if we want to add this feature [*1*].

[Footnote]

*1* Looking at Documentation/technical/pack-protocol.txt, "git
fetch" seem to learn if the repository is shallow immediately upon contacting "upload-pack" during the Reference Discovery phase (we'd see 'shallow' packets if they are shallow). I suspect that the right solution would be to teach the codepath on the "git fetch" side that accepts, parses, and acts on this packet to optionally stop communication and error out when the caller asks not to talk with a shallow repository.

Quoted 6 lines

This may reject to clone from a shallow repository, but at this point the bulk of the tranfer from the origin repository has already happened, no? Rejecting after transferring many megabytes feels a bit too late. That is one of the reasons why I kept hinting that the transport layer needs to be taught an option to reject talking to a shallow counterpart if we want to add this feature [*1*].

Extending the transport layer in this way might not be too difficult in the case of native (SSH, git://) protocols and using protocol v0, since handshake() in transport.c (called indirectly from transport_get_remote_refs()) writes shallow information to a data structure that we could potentially expose for the caller to use (before it calls transport_fetch_refs(). I couldn't see how remote-using protocols (e.g. HTTP) communicate shallow information, though (remote-curl.c seems to just keep it for itself), so that will be a more difficult task. And of course there's the matter of protocol v2, which I discuss below.

Quoted 10 lines

[Footnote]

*1* Looking at Documentation/technical/pack-protocol.txt, "git
fetch" seem to learn if the repository is shallow immediately upon contacting "upload-pack" during the Reference Discovery phase (we'd see 'shallow' packets if they are shallow). I suspect that the right solution would be to teach the codepath on the "git fetch" side that accepts, parses, and acts on this packet to optionally stop communication and error out when the caller asks not to talk with a shallow repository.

This is true with protocol v0, but protocol v2 bundles all shallow information (whether coming from the fact that the remote is shallow or the fact that the fetcher specified --depth etc.) and sends them together with the packfile. It may be possible to stop packfile download (saving bandwidth on the client side, at least) once such information is returned, though.

fetch-pack.c static void receive_shallow_info(struct fetch_pack_args *args, · line 1448

Quoted 6 lines

fetch-pack.c +2 -0

@@ -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."

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.");

Quoted 186 lines

fetch-pack.c +0 -0

 prepare_shallow_info(si, shallows);
 if (si->nr_ours || si->nr_theirs)

alternate_shallow_file =

fetch-pack.h +1 -0

index 736a3dae467a..6e4f8f0d738c 100644
@@ -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;
 
 /*

t/t5606-clone-options.sh +0 -0

index 52e5789fb050..6170d0513227 100755
@@ -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' '

t/t5606-clone-options.sh +0 -0

@@ -45,6 +47,51 @@ test_expect_success 'disallows --bare with --separate-git-dir' '
 

'

t/t5606-clone-options.sh +45 -0

+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 &&

t/t5611-clone-config.sh +0 -0

index 9f555b87ecdf..da10d3f10352 100755
@@ -95,6 +95,38 @@ test_expect_success 'clone -c remote.<remote>.fetch=<refspec> --origin=<name>' '

test_cmp expect actual
'

t/t5611-clone-config.sh +32 -0

+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 &&

transport-helper.c +2 -1

index 49b7fb4dcb9a..b57b55c8180c 100644
@@ -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,

transport.c +4 -0

index b13fab5dc3b1..34fe01221ee0 100644
@@ -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;

transport.h +4 -0

index 24e15799e714..f6273d268b09 100644
@@ -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

Quoted 6 lines

This may reject to clone from a shallow repository, but at this point the bulk of the tranfer from the origin repository has already happened, no?  Rejecting after transferring many megabytes feels a bit too late.  That is one of the reasons why I kept hinting that the transport layer needs to be taught an option to reject talking to a shallow counterpart if we want to add this feature [*1*].

Extending the transport layer in this way might not be too difficult in the case of native (SSH, git://) protocols and using protocol v0, since handshake() in transport.c (called indirectly from transport_get_remote_refs()) writes shallow information to a data structure that we could potentially expose for the caller to use (before it calls transport_fetch_refs(). I couldn't see how remote-using protocols (e.g. HTTP) communicate shallow information, though (remote-curl.c seems to just keep it for itself), so that will be a more difficult task. And of course there's the matter of protocol v2, which I discuss below.

Quoted 10 lines

[Footnote]

*1* Looking at Documentation/technical/pack-protocol.txt, "git fetch" seem to learn if the repository is shallow immediately upon contacting "upload-pack" during the Reference Discovery phase (we'd see 'shallow' packets if they are shallow). I suspect that the right solution would be to teach the codepath on the "git fetch" side that accepts, parses, and acts on this packet to optionally stop communication and error out when the caller asks not to talk with a shallow repository.

This is true with protocol v0, but protocol v2 bundles all shallow information (whether coming from the fact that the remote is shallow or the fact that the fetcher specified --depth etc.) and sends them together with the packfile. It may be possible to stop packfile download (saving bandwidth on the client side, at least) once such information is returned, though.

Documentation/config/clone.txt +0 -0

@@ -1156,6 +1164,16 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	 */
 	git_config(git_clone_config, NULL);
 

This is true with protocol v0, but protocol v2 bundles all shallow information (whether coming from the fact that the remote is shallow or the fact that the fetcher specified --depth etc.) and sends them together with the packfile.

It may be possible to stop packfile download (saving bandwidth on the client side, at least) once such information is returned, though.

Quoted 4 lines · Jonathan Tan

This is true with protocol v0, but protocol v2 bundles all shallow information (whether coming from the fact that the remote is shallow or the fact that the fetcher specified --depth etc.) and sends them together with the packfile.

By the above do you mean what happens in FETCH_GET_PACK arm where receive_shallow_info() is called when "shallow-info" header is seen, before the code continues to process wanted-refs, packfile-uris and then finally the packfile?

I do not think it makes much sense to ask any option to make us shallow (like --depth=<n>) while --reject-shallow is in use (after all, if the other side is deep enough to make us <n> commits deep, there is no reason to reject the other side as the source), so your "whether coming from the fact ..." part, while is a valid observation, can be ignored in practice (meaning: it is OK to make "--reject-shallow" be in effect only when we are trying to make a full clone, and reject combinations of it with --depth=<n> and such at the command parsing time).

Quoted 3 lines

It may be possible to stop packfile download (saving bandwidth on the client side, at least) once such information is returned, though.

Just like "upload-pack" does not get upset by a client that comes only for the initial refs advertisement and disconnects without asking for any "want" (aka "ls-remote"), the server side should be prepared to see if the other side cuts off after seeing the "shallow-info" section header or after seeing the the whole "shallow-info" section, so we should be able to leave early without having to download the bulk data. If the "upload-pack" spends unnecessary cycles when it happens, then we need to fix that. Even if the "fetch" client does not willingly disconnect in the middle, the network disconnect may happen at any point in the exchange, and we'd need to be prepared for it.

Do we need to read and parse the "shallow-info" section, or would the mere presense of the section mean the other side knows this side needs to futz with the .git/shallow information (either because we asked to be made shallow with --depth and the like, or because we tried to clone from them and they are shallow)?

Documentation/config/clone.txt clone.defaultRemoteName:: · line 8

Quoted 12 lines · Li Linchao via GitGitGadget

Documentation/config/clone.txt +0 -0

index 47de36a5fedf..50ebc170bb81 100644
@@ -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].

Documentation/config/clone.txt +4 -0

+
+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]

Let's camelCase this, i.e. "clone.rejectShallow", as this file would be a good candidate to be the authoritative record of canonical spelling of these variables.

cf. https://lore.kernel.org/git/[email protected]/

Documentation/config/clone.txt

Quoted 5 lines

+--[no-]reject-shallow::

Documentation/config/clone.txt +3 -0

+	Fail if the source repository is a shallow repository. The
+	'clone.rejectshallow' configuration variable can be used to
+	give the default.

Let's camelCase the reference to the variable, too.  Also, typeset in monospace, i.e.

The clone.rejectShallow configuration variable ...

Documentation/config/clone.txt static int git_clone_config(const char *k, const char *v, void *cb) · line 867

Quoted 7 lines

Documentation/config/clone.txt +3 -0

@@ -858,6 +862,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);
+	}

No need to use {} around a single-statement block, especially when the "if" statement does not have an "else" block.

The use of strcmp() against the variable name in all lowercase is correct here.

Quoted 4 lines

Documentation/config/clone.txt +0 -0

@@ -1156,6 +1164,16 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
   */
   git_config(git_clone_config, NULL);

Documentation/config/clone.txt

Quoted 10 lines

Documentation/config/clone.txt +10 -0

+	/*
+	* 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;
+	}

Needless use of {} around single-statement blocks.

As reject_shallow is initialized to 0, this lets the option to be of the most priority, then the config (presumably coming from the per-user or per-system configuration), by without them, defaults to false.  Good.

We'll have an extra git_config() call later, but that one will only read into config_shallow, never to be looked at because we will use reject_shallow variable anyway.  OK.

Documentation/config/clone.txt int cmd_clone(int argc, const char **argv, const char *prefix) · line 1238

Quoted 6 lines

Documentation/config/clone.txt +2 -0

@@ -1216,6 +1234,8 @@ 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)) {
+	if (reject_shallow)
+	die("source repository is shallow, reject to clone.");

With the local transport, it (hopefully) is trivial to see if the source is shallow.  OK.

Quoted 9 lines

Documentation/config/clone.txt +0 -0

   if (option_local > 0)
   warning(_("source repository is shallow, ignoring --local"));
   is_local = 0;

@@ -1227,6 +1247,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)

  transport_set_option(transport, TRANS_OPT_KEEP, "yes");

Documentation/config/clone.txt +2 -0

+	if (reject_shallow)
+	transport_set_option(transport, TRANS_OPT_REJECT_SHALLOW, "1");

Quoted 3 lines

  if (option_depth) transport_set_option(transport, TRANS_OPT_DEPTH, option_depth);

OK.  What is really interesting will all happen inside the transport layer; the caller only has to ask for it.

The asymmetry with other options like "--depth" stands out and puzzles readers, though.

Do we really want to add the clone.rejectShallow configuration?

After all, we do not give "clone.depth = 1" etc., and that is the reason why we only need "if (option_depth)" here in the near-by code.

I'd stop here for today, hoping that somebody much more familiar with the transport layer than I am will review and comment on the changes there.

Thanks.

Hi,

Quoted 10 lines · On Mon, 1 Mar 2021, [email protected] wrote:

fetch-pack.c static void receive_shallow_info(struct fetch_pack_args *args, · line 1448

Quoted 6 lines

fetch-pack.c +2 -0

@@ -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:

Quoted 7 lines

"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

Quoted 9 lines

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.");

From: lilinchao <[email protected]>

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 protected]>

---
    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.

    Changes since v5:

     * camelcase config variable
     * warning client that source repo is shallow
     * better support ssh:// and git:// protocol v1, v2

Signed-off-by lilinchao [email protected]

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

Range-diff vs v5:

1:  3f765e49e4a7 ! 1:  953122588ca8 builtin/clone.c: add --reject-shallow option @@ Documentation/config/clone.txt: clone.defaultRemoteName::
       origin, and can be overridden by passing the --origin command-line option to linkgit:git-clone[1].
      +
     -+clone.rejectshallow::
     ++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]

     @@ Documentation/git-clone.txt: objects from the source repository into a pack in t 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
     ++ Fail if the source repository is a shallow repository.
     ++ The 'clone.rejectShallow' configuration variable can be used to
      + give the default.
      +
       --bare::
     @@ builtin/clone.c: static int git_clone_config(const char *k, const char *v, void free(remote_name);
       remote_name = xstrdup(v);
       }
     -+ if (!strcmp(k, "clone.rejectshallow")) {
     ++ if (!strcmp(k, "clone.rejectshallow"))
      + config_shallow = git_config_bool(k, v);
     -+ }
     ++
       return git_default_config(k, v, cb);
       }

     @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
      + * If option_shallow is specified from CLI option,
      + * ignore config_shallow from git_clone_config.
      + */
     -+ if (config_shallow != -1) {
     ++ if (config_shallow != -1)
      + reject_shallow = config_shallow;
     -+ }
     -+ if (option_shallow != -1) {
     ++
     ++ 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) if (!access(mkpath("%s/shallow", path), F_OK)) {
      + if (reject_shallow)
      + die("source repository is shallow, reject to clone.");
     ++ else
     ++ warning("source repository is shallow.");
       if (option_local > 0)
       warning(_("source repository is shallow, ignoring --local")); is_local = 0;
     @@ fetch-pack.c: static struct ref *do_fetch_pack(struct fetch_pack_args *args,
       if (args->stateless_rpc)
       packet_flush(fd[1]);
     -+
     -+ if (!args->deepen && args->remote_shallow)
     -+ die("source repository is shallow, reject to clone.");
      +
       if (args->deepen)
       setup_alternate_shallow(&shallow_lock, &alternate_shallow_file, NULL);

     +-	else if (si->nr_ours || si->nr_theirs)
     ++	else if (si->nr_ours || si->nr_theirs) {

     ++ if (args->remote_shallow)
     ++ die("source repository is shallow, reject to clone.");
     ++ else
     ++ warning("source repository is shallow.");
     + alternate_shallow_file = setup_temporary_shallow(si->shallow);
     +- else
     ++ } else
     + alternate_shallow_file = NULL;
     + if (get_pack(args, fd, pack_lockfiles, NULL, sought, nr_sought,
     +      &gitmodules_oids))
      @@ fetch-pack.c: 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.");
       prepare_shallow_info(si, shallows);

     - if (si->nr_ours || si->nr_theirs)
     +-	if (si->nr_ours || si->nr_theirs)
     ++	if (si->nr_ours || si->nr_theirs) {

     ++ if (args->remote_shallow)
     ++ die("source repository is shallow, reject to clone.");
     ++ else
     ++ warning("source repository is shallow.");
       alternate_shallow_file =
     + setup_temporary_shallow(si->shallow);
     +- else
     ++ } else
     + alternate_shallow_file = NULL;
     + } else {
     + alternate_shallow_file = NULL;

       ## fetch-pack.h ##
      @@ fetch-pack.h: struct fetch_pack_args {
     @@ t/t5611-clone-config.sh: test_expect_success 'clone -c remote.<remote>.fetch=<re test_commit attributes .gitattributes "" &&
       rm -rf child &&

     - ## transport-helper.c ##
     -@@ transport-helper.c: 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,
     -
       ## transport.c ##
      @@ transport.c: static int set_git_option(struct git_transport_options *opts, list_objects_filter_die_if_populated(&opts->filter_options);
     @@ transport.h: void transport_check_allowed(const char *type);
       /* Aggressively fetch annotated tags if possible */
       #define TRANS_OPT_FOLLOWTAGS "followtags"

     -+/* reject shallow repo transport  */
     ++/* Reject shallow repo transport */
      +#define TRANS_OPT_REJECT_SHALLOW "rejectshallow"
      +
       /* Accept refs that may update .git/shallow without --depth */

Documentation/config/clone.txt |  4 +++ Documentation/git-clone.txt    |  7 ++++-
builtin/clone.c                | 24 +++++++++++++++++ fetch-pack.c                   | 17 +++++++++--- fetch-pack.h                   |  1 + t/t5606-clone-options.sh       | 47 ++++++++++++++++++++++++++++++++++ t/t5611-clone-config.sh        | 32 +++++++++++++++++++++++ transport.c                    |  4 +++
transport.h                    |  4 +++

9 files changed, 135 insertions(+), 5 deletions(-)

Documentation/config/clone.txt +0 -0

index 47de36a5fedf..7bcfbd18a52a 100644
@@ -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].

Documentation/config/clone.txt +4 -0

+
+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]

Documentation/git-clone.txt +0 -0

index 02d9c19cec75..0adc98fa7eee 100644
@@ -15,7 +15,7 @@ SYNOPSIS

  [--dissociate] [--separate-git-dir <git dir>] [--depth <depth>] [--[no-]single-branch] [--no-tags] [--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]

Documentation/git-clone.txt +1 -1

-	  [--[no-]remote-submodules] [--jobs <n>] [--sparse]
+	  [--[no-]remote-submodules] [--jobs <n>] [--sparse] [--[no-]reject-shallow]

  [--filter=<filter>] [--] <repository>
  [<directory>]

Documentation/git-clone.txt +0 -1

@@ -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::

Documentation/git-clone.txt +4 -1

+	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 +24 -0

index 51e844a2de0a..5c64837e8f7b 100644
@@ -50,6 +50,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 +92,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 +862,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 +970,7 @@ 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 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 +1164,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 +1234,10 @@ 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)) {
+	if (reject_shallow)
+	die("source repository is shallow, reject to clone.");
+	else
+	warning("source repository is shallow.");
 if (option_local > 0)
 warning(_("source repository is shallow, ignoring --local"));
 is_local = 0;

@@ -1227,6 +1249,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 transport_set_option(transport, TRANS_OPT_KEEP, "yes");
 
+	if (reject_shallow)
+	transport_set_option(transport, TRANS_OPT_REJECT_SHALLOW, "1");
 if (option_depth)
 transport_set_option(transport, TRANS_OPT_DEPTH,
      option_depth);

fetch-pack.c +12 -3

index 0cb59acc4866..860ff45d46e7 100644
@@ -1126,12 +1126,17 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 
 if (args->stateless_rpc)
 packet_flush(fd[1]);
+
 if (args->deepen)
 setup_alternate_shallow(&shallow_lock, &alternate_shallow_file,
 NULL);
-	else if (si->nr_ours || si->nr_theirs)
+	else if (si->nr_ours || si->nr_theirs) {
+	if (args->remote_shallow)
+	die("source repository is shallow, reject to clone.");
+	else
+	warning("source repository is shallow.");
 alternate_shallow_file = setup_temporary_shallow(si->shallow);
-	else
+	} else
 alternate_shallow_file = NULL;
 if (get_pack(args, fd, pack_lockfiles, NULL, sought, nr_sought,
      &gitmodules_oids))

@@ -1498,10 +1503,14 @@ static void receive_shallow_info(struct fetch_pack_args *args,
 * rejected (unless --update-shallow is set); do the same.
 */
 prepare_shallow_info(si, shallows);
-	if (si->nr_ours || si->nr_theirs)
+	if (si->nr_ours || si->nr_theirs) {
+	if (args->remote_shallow)
+	die("source repository is shallow, reject to clone.");
+	else
+	warning("source repository is shallow.");

alternate_shallow_file =
setup_temporary_shallow(si->shallow);

fetch-pack.c +1 -1

-	else
+	} else

alternate_shallow_file = NULL;
} else {
alternate_shallow_file = NULL;

fetch-pack.h +1 -0

index 736a3dae467a..6e4f8f0d738c 100644
@@ -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;
 
 /*

t/t5606-clone-options.sh +0 -0

index 52e5789fb050..6170d0513227 100755
@@ -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' '

t/t5606-clone-options.sh +0 -0

@@ -45,6 +47,51 @@ test_expect_success 'disallows --bare with --separate-git-dir' '
 

'

t/t5606-clone-options.sh +45 -0

+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 &&

t/t5611-clone-config.sh +0 -0

index 9f555b87ecdf..da10d3f10352 100755
@@ -95,6 +95,38 @@ test_expect_success 'clone -c remote.<remote>.fetch=<refspec> --origin=<name>' '

test_cmp expect actual
'

t/t5611-clone-config.sh +32 -0

+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 &&

transport.c +4 -0

index b13fab5dc3b1..34fe01221ee0 100644
@@ -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;

transport.h +4 -0

index 24e15799e714..4d5db0a7f22b 100644
@@ -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: f01623b2c9d14207e497b21ebc6b3ec4afaf4b46 --
gitgitgadget