Re: [PATCH 3/3] sha1-file: change alternate "error:" message to "warning:"
On Wed, Dec 05, 2018 at 12:37:58PM +0900, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason 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
Re: [PATCH 3/3] sha1-file: change alternate "error:" message to "warning:"
Ævar Arnfjörð Bjarmason 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.
[PATCH 3/3] sha1-file: change alternate "error:" message to "warning:"
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
---
sha1-file.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
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;
}
--
2.20.0.rc2.403.gdbc3b29805
