From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: jrnieder@gmail.com, git@vger.kernel.org, mfick@codeaurora.org
Subject: Re: [PATCH v2] builtin/describe: introduce --broken flag
Date: Wed, 22 Mar 2017 10:21:37 -0700 [thread overview]
Message-ID: <xmqqinn1mdlq.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170321225718.18633-1-sbeller@google.com> (Stefan Beller's message of "Tue, 21 Mar 2017 15:57:18 -0700")
Stefan Beller <sbeller@google.com> writes:
> git-describe tells you the version number you're at, or errors out, e.g.
> when you run it outside of a repository, which may happen when downloading
> a tar ball instead of using git to obtain the source code.
>
> To keep this property of only erroring out, when not in a repository,
> severe (submodule) errors must be downgraded to reporting them gently
> instead of having git-describe error out completely.
>
> To achieve that a flag '--broken' is introduced, which is in the same
> vein as '--dirty' but uses an actual child process to check for dirtiness.
> When that child dies unexpectedly, we'll append '-broken' instead of
> '-dirty'.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> Documentation/git-describe.txt | 11 +++++++---
> builtin/describe.c | 47 ++++++++++++++++++++++++++++++++++--------
> t/t6120-describe.sh | 20 ++++++++++++++++++
> 3 files changed, 66 insertions(+), 12 deletions(-)
I admit that my suggestion to use pushv() was done without trying it
out myself, but I do agree that the result looks better, especially
because the "dirty" (not "broken") side does not even have to use a
separate argv_array to prepare the command line in the first place
(which I failed to spot exactly because I didn't try it myself ;-).
We probably _could_ use cp.argv to point at the diff_index_args()
without doing pushv(&cp.args), and that would save a bit of
allocation, but it would not matter in practice as this is not a
performance critical codepath.
Thanks.
> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> index 167491fd5b..16952e44fc 100755
> --- a/t/t6120-describe.sh
> +++ b/t/t6120-describe.sh
> @@ -233,4 +233,24 @@ test_expect_success 'describe --contains and --no-match' '
> test_cmp expect actual
> '
>
> +test_expect_success 'setup and absorb a submodule' '
> + test_create_repo sub1 &&
> + test_commit -C sub1 initial &&
> + git submodule add ./sub1 &&
> + git submodule absorbgitdirs &&
> + git commit -a -m "add submodule" &&
> + git describe --dirty >expect &&
> + git describe --broken >out &&
> + test_cmp expect out
> +'
> +
> +test_expect_success 'describe chokes on severly broken submodules' '
> + mv .git/modules/sub1/ .git/modules/sub_moved &&
> + test_must_fail git describe --dirty
> +'
> +test_expect_success 'describe ignoring a borken submodule' '
> + git describe --broken >out &&
> + grep broken out
> +'
> +
> test_done
next prev parent reply other threads:[~2017-03-22 17:22 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-21 0:11 [PATCH 0/3] git-describe deals gracefully with broken submodules Stefan Beller
2017-03-21 0:11 ` [PATCH 1/3] submodule.c: port is_submodule_modified to use porcelain 2 Stefan Beller
2017-03-21 0:11 ` [PATCH 2/3] revision machinery: gentle submodule errors Stefan Beller
2017-03-21 0:11 ` [PATCH 3/3] builtin/describe: introduce --submodule-error-as-dirty flag Stefan Beller
2017-03-21 6:28 ` [PATCH 0/3] git-describe deals gracefully with broken submodules Junio C Hamano
2017-03-21 6:54 ` Junio C Hamano
2017-03-21 18:51 ` [PATCH] builtin/describe: introduce --broken flag Stefan Beller
2017-03-21 21:51 ` Junio C Hamano
2017-03-21 22:27 ` Stefan Beller
2017-03-21 22:41 ` Junio C Hamano
2017-03-21 22:50 ` Stefan Beller
2017-03-21 22:57 ` [PATCH v2] " Stefan Beller
2017-03-22 17:21 ` Junio C Hamano [this message]
2017-03-22 21:50 ` Jakub Narębski
2017-03-21 17:46 ` [PATCH 0/3] git-describe deals gracefully with broken submodules Stefan Beller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqinn1mdlq.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jrnieder@gmail.com \
--cc=mfick@codeaurora.org \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).