Please consider reducing build system churn

Please consider reducing build system churn

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 1423 bytes --]

Hi,

I had a request to bump man-pages in Gentoo [0] and I must admit that
I've been fatigued recently, both by other work, but also the large
number of build system changes between man-pages releases.

I already try to review all build system changes but Recent Events (TM)
means that one must take even more care and not be fatigued by changes.

man-pages in particular is a critical package and if I can't review its
changes properly, I can't package updates to it.

There's a lot of churn commits like
* d0d2e2f9a21e623db208b599643e7728d71e7e6f,
* 57d0f8b4f1ba43fd4c4bfbe826d1b255c6ef9fd5,
* cdb8bd9563dd8a0d32138de40bc64f0c3475790c,
* 6658bc41a8d66afbf348126f8adcffe86227e2e1,
* d7b3f23c53d83ff714dd829c929a27f2c05aab23,
* 7de1139b82571dfe55224022e05e095a32b1ea89
and many others.

With regard to d0d2e2f9a21e623db208b599643e7728d71e7e6f specifically, it
might be worth explaining: a) what liba2i is; b) what commits were
actually pulled in (possibly with 'cherry-picked from ...' or shortlog
output or something at least.)

Alex already knows my feelings on homebrew complex Makefiles so I won't
repeat them here, but I will say that homebrew complex Makefiles
combined with a high amount of regular refactoring makes life much
harder.

Please consider either getting all the refactoring out of the way if
possible or reducing the amount of churn in the build system.

[0] https://bugs.gentoo.org/930798

thanks,
sam

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]

Re: Please consider reducing build system churn

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2748 bytes --]

Hey Sam!

On Sun, Apr 28, 2024 at 02:19:55AM +0100, Sam James wrote:
> Hi,
> 
> I had a request to bump man-pages in Gentoo [0] and I must admit that
> I've been fatigued recently, both by other work, but also the large
> number of build system changes between man-pages releases.
> 
> I already try to review all build system changes but Recent Events (TM)
> means that one must take even more care and not be fatigued by changes.

Yup, I had the same feeling.  :-)

> man-pages in particular is a critical package and if I can't review its
> changes properly, I can't package updates to it.
> 
> There's a lot of churn commits like
> * d0d2e2f9a21e623db208b599643e7728d71e7e6f,

The rationale for that is that I diffed the whole build system of this
project with liba2i's, to reduce maintainance work.  Any spurious
difference was removed if it was unnecessary.

> * 57d0f8b4f1ba43fd4c4bfbe826d1b255c6ef9fd5,
> * cdb8bd9563dd8a0d32138de40bc64f0c3475790c,

This one was because man2html(1) crashed on a page from tzdb (which we
import).  I guess that's a necessary change.

> * 6658bc41a8d66afbf348126f8adcffe86227e2e1,
> * d7b3f23c53d83ff714dd829c929a27f2c05aab23,
> * 7de1139b82571dfe55224022e05e095a32b1ea89

This one was requested by Debian, so that known lint failures don't
pollute their packaging checks.

> and many others.

Those that I didn't comment, or the many others, were triggered by your
feedback that the build system was complex and hard to understand.  I've
been trying to keep it very organized.  But yeah, all that was prior to
Recent Events (TM), and I want to reduce build system churn
significantly from now on.

> 
> With regard to d0d2e2f9a21e623db208b599643e7728d71e7e6f specifically, it
> might be worth explaining: a) what liba2i is; b) what commits were
> actually pulled in (possibly with 'cherry-picked from ...' or shortlog
> output or something at least.)

I didn't cherry-pick anything specifically; I run diffoscope(1) between
both trees, and tried to remove the trivial differences such as white
space, so that it only shows the important diff.

> Alex already knows my feelings on homebrew complex Makefiles so I won't
> repeat them here, but I will say that homebrew complex Makefiles
> combined with a high amount of regular refactoring makes life much
> harder.
> 
> Please consider either getting all the refactoring out of the way if
> possible or reducing the amount of churn in the build system.

Agree.

Have a lovely Sunday!
Alex

> [0] https://bugs.gentoo.org/930798
> 
> thanks,
> sam



-- 
<https://www.alejandro-colomar.es/>
A client is hiring kernel driver, mm, and/or crypto developers;
contact me if interested.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Please consider reducing build system churn

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3388 bytes --]

Hi Sam,

On Sun, Apr 28, 2024 at 10:37:23AM +0200, Alejandro Colomar wrote:
> On Sun, Apr 28, 2024 at 02:19:55AM +0100, Sam James wrote:
> > With regard to d0d2e2f9a21e623db208b599643e7728d71e7e6f specifically, it
> > might be worth explaining: a) what liba2i is;

I forgot to explain what liba2i is.  You know, but for the list:

I've been contributing to the shadow project for some years, and
maintaining its stable branches for half a year.  While doing that, I
found consistent misuses of strto[u]l(3) and its family of functions.
And in the cases where it was correctly being used, the code was less
than desirable.

I claim that it's due to the messy interface of those functions; while
they are useful, they are very difficult to use correctly.

The first problem I fixed was that strtoul(3) doesn't reject negative
numbers, and behaves in a way most programmers wouldn't expect (unless
they know that function really well).
<https://github.com/shadow-maint/shadow/pull/875>

Then, I tried to do was use the NetBSD APIs strtoi/u(3), which are
provided by libbsd for GNU/Linux systems.  However, I realized that it
was still imperfect, since it doesn't provide type safety.  Also, since
Fedora and some other systems don't want libbsd as a dependency of
shadow, I had to implement my own strtoi(3) to work when shadow wasn't
being linked to libbsd.
<https://github.com/shadow-maint/shadow/pull/890>

When testing my implementation and libbsd's one, I realized that they
behaved different in a corner case, which results it was a bug in the
original design of the function.  Luckily, it was an undocumented
behavior, and we could fix it (and BTW, fixing it, fixed many existing
calls in both NetBSD and Debian).
<https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=57828>

I found an old internal function of the shadow project, getlong(), which
seemed to have a better API, which provided type safety.  I then came up
with a design for a set of functions and macros which would be a hybrid
between shadow's getlong() and NetBSD's strtoi(3).  Those functions have
already been added to shadow recently.
<https://github.com/shadow-maint/shadow/pull/891>

And now I have a huge patch set for modifying the entire code base to
use calls to those new functions, replacing every call to older APIs.
<https://github.com/shadow-maint/shadow/pull/893>

While changing all uses to use the new APIs that I designed, I found
several bugs in the process, which are being fixed (most silently, with
no bug reports).  In some cases, I found code that is pasted from other
projects (e.g., the reproducible builds project).  That's why I thought
that writing a small library that would house these APIs would be an
improvement to many projects, not just this one.  The project is hosted
in <kernel.org>.  It has a mailing list (CCed in this message).
<https://git.kernel.org/pub/scm/libs/liba2i/liba2i.git>

I wrote a build system for it based on the one in this project, the
Linux man-pages.  I'm trying to keep both build systems as similar as
possible, to reduce the job of maintaining both.  For that, I use
diffoscope(1), to see the differences in every file.

Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es/>
A client is hiring kernel driver, mm, and/or crypto developers;
contact me if interested.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]