From: Stefan Beller <sbeller@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
Jonathan Nieder <jrnieder@gmail.com>,
Martin Fick <mfick@codeaurora.org>
Subject: Re: [PATCH] builtin/describe: introduce --broken flag
Date: Tue, 21 Mar 2017 15:27:56 -0700 [thread overview]
Message-ID: <CAGZ79kbjsm1p+Ag5Q8fii3ncbxSsVLYRwGP=Va=btx8Tfy3aOA@mail.gmail.com> (raw)
In-Reply-To: <xmqq7f3invr6.fsf@gitster.mtv.corp.google.com>
On Tue, Mar 21, 2017 at 2:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> This patch helps to fix the root cause in [1], which tries to work around
>> this situation.
>
> I do not necessarily think it is reasonable to give $version-dirty
> and proceed when a repository corruption is detected; if there is a
> breakage in the repository, "git describe" is correct to die when a
> populated submodule is broken. IOW, I do not agree that [1] below
> is working with a sensible expectation.
ok, so I won't quote it in the commit message
>
> This is a tangent, but how does the Gerrit repository get corrupted
> in the way described in [1] in the first place? That might be what
> needs to be corrected, perhaps?
AFAICT, someone is (was?) using a version of Git that doesn't contain
f8eaa0ba98 (submodule--helper, module_clone: always operate
on absolute paths, 2016-03-31). So then the submodule paths were
made absolute paths on creation of the Gerrit repo.
And then someone moved the repo and the absolute paths broke.
Even after an upgrade of Git to its latest and greatest version, the underlying
issue of having broken submodule paths remains in that case.
So there are a couple of ways forward
0) as an immediate fix, manually fix the absolute path or make them relative
1A) have more error resilient tools in Git
1B) have a tool in git (e.g. "git submodule fsck-setup") that rewrites
the .git file link and the core.worktree setting to be relative and correct.
I think we should do both A and B, I decided to go with A first, specifically
"git-describe" as that was reported to not work well in this situation with the
given broken data
> The wording for the "--dirty" is already awkward, but this one is
> even more so ("Describe the working tree. It means" conveys no
> useful information). I however cannot come up with something much
> better. This is the best I could come up with:
>
> Describe the state of the working tree. When the working
> tree matches HEAD, the output is the same as "git describe
> HEAD" and "-dirty" is appended to it if the working tree has
> local modification. When a repository is corrupt and Git
> cannot determine if there is local modification, instead of
> dying, append "-broken" instead.
ok, I'll reuse parts of it.
>> +static const char *append, *dirty, *broken;
>
> Perhaps call it "suffix" or something?
done
>> + argv_array_pushl(&args, "diff-index", "--quiet", "HEAD", "--", NULL);
..
> Wouldn't argv_array_pushv() into these two different args
> array from the same template work better?
yes, looks much saner. Thanks
next prev parent reply other threads:[~2017-03-21 22:37 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 [this message]
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
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='CAGZ79kbjsm1p+Ag5Q8fii3ncbxSsVLYRwGP=Va=btx8Tfy3aOA@mail.gmail.com' \
--to=sbeller@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=mfick@codeaurora.org \
/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).