| Message ID | 20181204132716.19208-4-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] sha1-file: test the error behavior of alt_odb_usable() | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Change the "error" message emitted by alt_odb_usable() to be a > "warning" instead. As noted in commits leading up to this one this has > never impacted the exit code ever since the check was initially added > in 26125f6b9b ("detect broken alternates.", 2006-02-22). > > It's confusing to emit an "error" when e.g. "git fsck" will exit with > 0, so let's emit a "warning:" instead. OK, that sounds sensible. An alternative that may be more sensible is to actually diagnose this as an error, but the purpose of this message is to help users diagnose a possible misconfiguration and keeping the repository "working" with the remaining set of object stores by leaving it as a mere warning, like this patch does, is probably a better approach.
On Wed, Dec 05, 2018 at 12:37:58PM +0900, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > > > Change the "error" message emitted by alt_odb_usable() to be a > > "warning" instead. As noted in commits leading up to this one this has > > never impacted the exit code ever since the check was initially added > > in 26125f6b9b ("detect broken alternates.", 2006-02-22). > > > > It's confusing to emit an "error" when e.g. "git fsck" will exit with > > 0, so let's emit a "warning:" instead. > > OK, that sounds sensible. An alternative that may be more sensible > is to actually diagnose this as an error, but the purpose of this > message is to help users diagnose a possible misconfiguration and > keeping the repository "working" with the remaining set of object > stores by leaving it as a mere warning, like this patch does, is > probably a better approach. Yeah, I think it's better to keep it as a warning. It's actually reasonably likely to be benign (e.g., you did a "git repack -ad && rm /path/to/alternate" to remove the dependency, but forgot to clean up the alternates). And when it _is_ a problem, the object-reading code paths will definitely let you know. -Peff
diff --git a/sha1-file.c b/sha1-file.c index f142f81658..4b9b63bdcb 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -378,17 +378,17 @@ static int alt_odb_usable(struct raw_object_store *o, if (!is_directory(path->buf)) { /* Detect cases where alternate disappeared */ - error(_("object directory %s does not exist; " - "check .git/objects/info/alternates"), - path->buf); + warning(_("object directory %s does not exist; " + "check .git/objects/info/alternates"), + path->buf); return 0; } else if (is_directory(mkpath("%s/objects", path->buf)) || is_directory(mkpath("%s/.git/objects", path->buf))) { /* Detect cases where alternate is a git repository */ - error(_("object directory %s looks like a git repository; " - "alternates must point to the 'objects' directory. " - "check .git/objects/info/alternates"), - path->buf); + warning(_("object directory %s looks like a git repository; " + "alternates must point to the 'objects' directory. " + "check .git/objects/info/alternates"), + path->buf); return 0; }
Change the "error" message emitted by alt_odb_usable() to be a "warning" instead. As noted in commits leading up to this one this has never impacted the exit code ever since the check was initially added in 26125f6b9b ("detect broken alternates.", 2006-02-22). It's confusing to emit an "error" when e.g. "git fsck" will exit with 0, so let's emit a "warning:" instead. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- sha1-file.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)