From: Junio C Hamano <email@example.com> To: Stefan Beller <firstname.lastname@example.org> Cc: email@example.com, firstname.lastname@example.org, email@example.com Subject: Re: [PATCH 0/3] git-describe deals gracefully with broken submodules Date: Mon, 20 Mar 2017 23:28:27 -0700 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> Stefan Beller <firstname.lastname@example.org> writes: > Our own version generation in GIT-VERSION-GEN is somewhat sane by testing > if we have a .git dir, and use that as a signal whether the obtained > copy of git was obtained using git (clone/fetch) or if it is just a > downloaded tar ball. > > Other scripts to generate a version are not as cautious and just run > "git describe". An error from git-describe is treated as a sufficient > signal to assume it is not a git repository. > > When submodules come into play, this is not true, as a submodule > may be damaged instead, such that we're still in a git repository > but error out for the sake of reporting a severly broken submodule. > > Add a flag to git-describe that instructs it to treat severe submodule > errors as "dirty" instead of erroring out. I do not have a strong preference for or against the "treat a broken repository as if nothing is wrong with the revision, but just mark it as dirty" idea. I would be more receptive if it substituted the "-dirty" marker with something else, e.g. "-broken", though. My knee-jerk reaction to the code change is that treating submodule as something very special is probably not a good idea. Even if you do not use submodules, if some of the objects referenced from your index and/or HEAD are damaged or otherwise causes some error while accessing, the diff machinery would die, wouldn't it? I saw that some new symbolic constants in the code to tell the machinery to "gracefully die" (or "hide the breakage under the rug") are named with SUBMODULE in them, which is probably a bad sign that the design is being too centric to submodules. The implementation that covers only breakages in submodule as its first step may be OK (you have to start somewhere, after all), but I think the aspiration should be to cover all kinds of breakages in the end and turn them to be "graceful", and if you had that goal in mind, you wouldn't be naming these constants with SUBMODULE in them. If "treat a broken repository as just being 'dirty'" were a good idea, I'd suspect that we would want to see all breakages, not just ones related to submodules, to be treated the same way. But it is possible there may be a reason why submodules are special. I do not think the third paragraph quoted above is a good justification. A repository with broken submodule is just as broken and untrustworthy as a broken repository without a submodule, and if you want to allow such a checkout with broken submodule to call itself v2.0-dirty, you would also want to allow a broken checkout without any submodule to do so, too.
next prev parent reply index Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-03-21 0:11 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 ` Junio C Hamano [this message] 2017-03-21 6:54 ` [PATCH 0/3] git-describe deals gracefully with broken submodules 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 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 publically 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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.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
email@example.com mailing list mirror (one of many) Archives are clonable: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.org/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ or Tor2web: https://www.tor2web.org/ AGPL code for this site: git clone https://public-inbox.org/ public-inbox