On 2023/04/28 13:51:06 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> On Fri, Apr 28, 2023 at 11:30:12AM +0200, Omar Polo wrote:
> > While the fix would be really simple, just turn an == into a != and
> > call it a day, I was wondering why gotwebd is the only component that
> > explicitly ignores GOT_ERR_LONELY_PACKIDX?  Can we fail like got, tog
> > etc... do?
>  
> This error will liekly happen if git is used to fetch changes into
> the repository over https. Since this is a legitimate way of syncing
> repositories it is indeed important to ignore this error otherwise
> gotwebd will only show error pages on affected repositories.
> And it doesn't make sense to display this error to people who cannot
> do anything about it. Git ignores this condition but we dicided to
> error because it could be repository corruption (such as an accidentally
> deleted .pack file).

Thanks for the insight, I agree we should keep ignoring this error.

> Please fix this broken log instead:
> 
>                 error = gotweb_load_got_path(c, repo_dir);
>                 if (error && error->code == GOT_ERR_LONELY_PACKIDX) {
>                         if (error->code != GOT_ERR_NOT_GIT_REPO)
>                                   ^^^^^^^^^^^ always true

Hummm, not sure.  There are few places where gotweb_load_got_path()
returns GOT_ERR_NOT_GIT_REPO, so not logging those errors seems a good
idea to me.

Here's the updated patch that fixes the logic error I introduced.

diff /home/op/w/gotacl
commit - 76cbc7c5854f28fb476b6c80d69163c6a0796725
path + /home/op/w/gotacl
blob - 532cb2f0f6b9415c1433f00e174bb0fd10c72992
file + gotwebd/gotweb.c
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -865,7 +865,7 @@ gotweb_render_index(struct template *tp)
 			continue;
 
 		error = gotweb_load_got_path(c, repo_dir);
-		if (error && error->code == GOT_ERR_LONELY_PACKIDX) {
+		if (error && error->code != GOT_ERR_LONELY_PACKIDX) {
 			if (error->code != GOT_ERR_NOT_GIT_REPO)
 				log_warnx("%s: %s: %s", __func__,
 				    sd_dent[d_i]->d_name, error->msg);