threads / patch / 46782

patchrev-parse: rev-parse: add --is-shallow-repository

Subject: [PATCH] rev-parse: rev-parse: add --is-shallow-repository

## tl;dr

4 messages between Sep 18, 2017 and Sep 19, 2017. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Øystein Walle· Sep 18, 2017, 17:04 UTC · lore

Running `git fetch --unshallow` on a repo that is not in fact shallow produces a fatal error message. Add a helper to rev-parse that scripters can use to determine whether a repo is shallow or not.

Signed-off-by: Øystein Walle <oystwa@gmail.com>
---
 Documentation/git-rev-parse.txt |  3 +++
 builtin/rev-parse.c             |  5 +++++
 t/t1500-rev-parse.sh            | 15 +++++++++++++++
 3 files changed, 23 insertions(+)
Show changes to 3 files +23 −0

Documentation/git-rev-parse.txt, builtin/rev-parse.c, t/t1500-rev-parse.sh

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index b1293f24b..0917b8207 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -235,6 +235,9 @@ print a message to stderr and exit with nonzero status.
 --is-bare-repository::
 	When the repository is bare print "true", otherwise "false".
 
+--is-shallow-repository::
+	When the repository is shallow print "true", otherwise "false".
+
 --resolve-git-dir <path>::
 	Check if <path> is a valid repository or a gitfile that
 	points at a valid repository, and print the location of the
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 2bd28d3c0..c923207f2 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -868,6 +868,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 						: "false");
 				continue;
 			}
+			if (!strcmp(arg, "--is-shallow-repository")) {
+				printf("%s\n", is_repository_shallow() ? "true"
+						: "false");
+				continue;
+			}
 			if (!strcmp(arg, "--shared-index-path")) {
 				if (read_cache() < 0)
 					die(_("Could not read the index"));
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 03d3c7f6d..9d3433a30 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -116,6 +116,21 @@ test_expect_success 'git-path inside sub-dir' '
 	test_cmp expect actual
 '
 
+test_expect_success 'git-path shallow repository' '
+	test_commit test_commit &&
+	echo true >expect &&
+	git clone --depth 1 --no-local . shallow &&
+	test_when_finished "rm -rf shallow" &&
+	git -C shallow rev-parse --is-shallow-repository >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git-path notshallow repository' '
+	echo false >expect &&
+	git rev-parse --is-shallow-repository >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'showing the superproject correctly' '
 	git rev-parse --show-superproject-working-tree >out &&
 	test_must_be_empty out &&

-- 
2.11.0.485.g4e59582
Jonathan Nieder· Sep 19, 2017, 02:33 UTC · re: Øystein Walle · lore

Re: [PATCH] rev-parse: rev-parse: add --is-shallow-repository

Hi,
Øystein Walle wrote:
> Running `git fetch --unshallow` on a repo that is not in fact shallow
> produces a fatal error message.

Hm, can you say more about the context? From a certain point of view, it might make sense for that command to succeed instead: if the repo is already unshallow, then why should't "fetch --unshallow" complain instead of declaring victory?

Show 9 quoted lines
>                                 Add a helper to rev-parse that scripters
> can use to determine whether a repo is shallow or not.
>
> Signed-off-by: Øystein Walle <oystwa@gmail.com>
> ---
>  Documentation/git-rev-parse.txt |  3 +++
>  builtin/rev-parse.c             |  5 +++++
>  t/t1500-rev-parse.sh            | 15 +++++++++++++++
>  3 files changed, 23 insertions(+)

Regardless, this new rev-parse --is-shallow helper looks like a good feature.

[...]
Show 11 quoted lines
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -868,6 +868,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  						: "false");
>  				continue;
>  			}
> +			if (!strcmp(arg, "--is-shallow-repository")) {
> +				printf("%s\n", is_repository_shallow() ? "true"
> +						: "false");
> +				continue;
> +			}
The implementation is straightforward and correct.
[...]
> --- a/t/t1500-rev-parse.sh
> +++ b/t/t1500-rev-parse.sh
Thanks for writing tests. \o/
Show 5 quoted lines
> @@ -116,6 +116,21 @@ test_expect_success 'git-path inside sub-dir' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'git-path shallow repository' '

What does git-path mean here? I wonder if it's a copy/paste error. Did you mean something like

 test_expect_success 'rev-parse --is-shallow-repository in shallow repo' '
?
Show 9 quoted lines
> +	test_commit test_commit &&
> +	echo true >expect &&
> +	git clone --depth 1 --no-local . shallow &&
> +	test_when_finished "rm -rf shallow" &&
> +	git -C shallow rev-parse --is-shallow-repository >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git-path notshallow repository' '
Likewise: should this be
 test_expect_success 'rev-parse --is-shallow-repository in non-shallow repo' '
?
Show 6 quoted lines
> +	echo false >expect &&
> +	git rev-parse --is-shallow-repository >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'showing the superproject correctly' '
With the two tweaks mentioned above,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.
Junio C Hamano· Sep 19, 2017, 03:18 UTC · re: Jonathan Nieder · lore

Re: [PATCH] rev-parse: rev-parse: add --is-shallow-repository

Jonathan Nieder <jrnieder@gmail.com> writes:
>>  test_expect_success 'showing the superproject correctly' '
>
> With the two tweaks mentioned above,
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

I agree with the fixes to the test titles suggested, so I'll queue the patch with the fixes squashed in. Hearing "yeah, the titles were copy-pasted without adjusting, thanks for fixing, Jonathan!" sent by Øystein would be super nice.

Thanks, both.
Øystein Walle· Sep 19, 2017, 18:51 UTC · re: Junio C Hamano · lore

Re: [PATCH] rev-parse: rev-parse: add --is-shallow-repository

> Hm, can you say more about the context?  From a certain point of view,
> it might make sense for that command to succeed instead: if the repo
> is already unshallow, then why should't "fetch --unshallow" complain
> instead of declaring victory?

A fellow in #git on Freenode was writing a script for automation and encountered this error, and asked how to find out whether a repo was shallow. My *first instinct* was to check if rev-parse had a flag for it; I wouldn't have been surprised if it did.

I agree that treating it as a fatal error is a bit much in the first place, but I also think having a way to check can be useful. I also wonder if a lot of the stuff rev-parse is used for now should be moved to some sort of `git misc` command, but that's a different can of worms, so into rev-parse a new flag went.

> What does git-path mean here?  I wonder if it's a copy/paste error.
> ...
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Yeah, the titles were copy-pasted without adjusting, thanks for fixing, Jonathan! ;)

> I agree with the fixes to the test titles suggested, so I'll queue the
> patch with the fixes squashed in.  Hearing "yeah, the titles were
> copy-pasted without adjusting, thanks for fixing, Jonathan!" sent by
> =C3=98ystein would be super nice.
Sounds good. Thanks for queueing my patch. My fourth!
�sse

← back to recent threads