Re: Regarding "Use Invoke" or "Return a boolean from phase" commits

[Mark H Weaver]

Hi Ludovic,

address@hidden (Ludovic Courtès) writes:

> Mark H Weaver <address@hidden> skribis:
>
>> Furthermore, it will be important for a static code analyzer to be able
>> to infer that phases and snippets always return #t, so let's try not to
>> be too clever about it.  We'll need a static analyzer to gain confidence
>> that we can start ignoring the return values without losing failure
>> reports.
>>
>> Quite a while ago, I hacked up a preliminary static analyzer to do this.
>> Obviously it would be good to integrate something like this into our
>> linter, but I haven't gotten around to it yet.  In the meantime, I've
>> attached my preliminary code below.  Here's how it's used:
>
> Nice!  It’ll greatly help gain confidence that the code is correct when
> we decide to switch.  IIUC it only works for ‘gnu-build-system’, right?

(possible-phase-problem-pkgs) works for any build system that accepts
"#:phases" in its argument list.  However, it only checks the phases
that are overridden; it assumes that everything in %standard-phases is
correct.

To check the phases in the build systems themselves, there's
(possible-phase-problem-pkgs-from-drvs DRV-FILES), but it's a bit hacky
and makes assumptions about the form of the code in the *-guile-builder
script.  At one point I ran this checker on every .drv file on my
core-updates system, and thereby found many build system issues.  I
should probably do this again soon.

For build systems that expect "#:builder" in the argument list
(e.g. trivial-build-system) there's (possible-builder-problem-pkgs),
although I'm not yet sure whether it makes sense to switch to an
exception-only error reporting model for builders.  I'm not (yet?)
proposing to ignore the boolean results from builders.

Finally, there's (possible-snippet-problem-pkgs), which I forgot to
mention in my earlier email.  At one point I fixed every snippet in
core-updates to return #t, but since then many new snippets have been
introduced via 'master' that return an unspecified value.

> If the analyzer doesn’t cover 100% of the packages, we’ll rely on manual
> testing as we rebuild everything anyway—pretty much like when we do a
> regular ‘core-updates’ cycle.

If I'm not mistaken, the combination of checkers mentioned above should
give 100% coverage for phases and snippets of packages that
'fold-packages' iterates over.  However, it might miss packages that are
somehow hidden from 'fold-packages', e.g. packages bound to variables
that are not exported, and packages that are returned by procedures.

       Mark