Re: [PATCH] maint: Avoid syntax-check failure for empty gnulib submodule.

[Simon Josefsson]

mån 2021-09-20 klockan 07:36 -0700 skrev Jim Meyering:
> On Mon, Sep 20, 2021 at 12:49 AM Simon Josefsson via Gnulib
> discussion
> list <bug-gnulib@gnu.org> wrote:
> > Hi,
> > 
> > Some syntax-check rules have a really poor failure mode (it prints
> > all
> > source code files to stdout) when the 'gnulib_dir' points to an
> > empty
> > git submodule checkout, see for example:
> > 
> > https://storage.googleapis.com/gitlab-gprd-artifacts/08/35/08352f7ac0f5a870c92a7777a01ca17d0b44be10aff8451893987ed8dbf48231/2021_09_19/1602830917/1729528459/job.log?response-content-type=text%2Fplain%3B%20charset%3Dutf-8&response-content-disposition=inline&GoogleAccessId=gitlab-object-storage-prd@gitlab-production.iam.gserviceaccount.com&Signature=fGIerRoWfE1woQD7hzvDst%2FPa67Kg9u9PjJxMB8vLpFD4DZIPDZNRBcqHQZV%0ARtRNSa2k2Er1W59wYe%2F%2FKY9C95YkIhMCftS5zBXfqsEPkRbU8CwcoG5clQoL%0A4Mpqy6OY1Kx7JUB86qBpYWFKlHLPosw0sscOolAGxcel2oZP1soxHLzxcgo%2B%0A66whawET8QK%2Bkj8FOElVzwVz93T8Qp0uZgvfoT7UVF2O9JGlEofYw7%2BrQm%2Ff%0AXOutkumck4OR17dGXSNaoWPO48Z7tDJLeAjWjnSUm%2F9ZPkYd%2B8IfxaEzACg%2B%0Av%2FS%2ByQpyp6eS1kdiliBmJFmL0Y7JJu7HgZiU21kkyw%3D%3D&Expires=1632123408
> > 
> > The patch below is not ideal -- consider if you did ./bootstrap --
> > no-git
> > with the goal of using GNULIB_SRCDIR but for some reason ALSO did
> > 'git
> > submodule update --init', then the syntax-check rules will use the
> > submodule checkout instead of the GNULIB_SRCDIR content -- but I
> > believe
> > things are better with the patch than without it.
> ...
> > -gnulib_dir ?= $(shell if test -d $(srcdir)/gnulib; then \
> > +gnulib_dir ?= $(shell if test -f $(srcdir)/gnulib/gnulib-tool;
> > then \
> >                         echo $(srcdir)/gnulib; \
> >                 else \
> >                         echo ${GNULIB_SRCDIR}; \
> 
> Hi Simon,
> 
> Maybe I'm misunderstanding the scenario, but if the gnulib dir is in
> an inconsistent state, isn't it more appropriate that syntax-check
> rules fail (even if noisily), than maybe-succeed because we pointed
> them at another dir that's not even being used yet?

Hi!  I don't know and I'm open for any behaviour, if it is consistently
applied.  Looking at the current state, it seems like 'make syntax-
check' fail silently when some pre-requirements (like missing tools)
are not met.  Having a properly setup gnulib directory is only a
requirement for very few rules (I think <5).  I think it makes sense to
run 'make syntax-check' from a tarball build, but then the tests that
require a gnulib directory cannot work obviously (and could fail
silently or with a warning).

Further, it is not really that the gnulib dir is in an inconsistent
state, it is that maint.mk does not really know what ./bootstrap used 
as the gnulib directory.  Idealy, the decision made by ./bootstrap
about which gnulib_dir it used should be preserved somehow, and then
the same value should used by 'make syntax-check'.  Achieving this
seems complicated.  Maybe it is sufficient that both scripts use
similar enough logic to derive the value that it isn't a problem in
practice.

> Does it make sense to run "make syntax-check" without first having
> run "make"? That latter requires a proper gnulib setup, in my
> experience.

Sure, but in the example above, there is a gnulib directory setup
correctly (through the GNULIB_SRCDIR environment variable set in the
docker image environment that ./bootstrap picks up) but 'make syntax-
check' doesn't use it, but prefer the local non-populated gnulib/ git
submodule.

/Simon

signature.asc
Description: This is a digitally signed message part