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

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

[Øystein Walle]

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(+)

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

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

[Jonathan Nieder]

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?

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

[...]
> --- 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/

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

?

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

?

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

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

[Junio C Hamano]

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.

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

[Øystein Walle]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 1344 bytes --]

> 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