Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build system rewrite #23

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Build system rewrite #23

wants to merge 9 commits into from

Conversation

0x5c
Copy link

@0x5c 0x5c commented Apr 4, 2023

This has dramatic effects on the build system complexity, and in the process exposing lots of dead weight.

Split header files (.h1 .h2) are merged, and header files that only contained a single define have been fully replaced with -D. The makefile is no longer generating scripts, and their files that hardcoded cc/ld/ar calls are gone.

The build system is now fully compatible with standard variables like CC, PREFIX, DESTDIR, and such. The compiler is autodetected when unspecified, build flags are checked for compatibility, and installation directories can now be specified with configure flags.

The configure script was heavily inspired by XBPS's.

The "compile" script is gone along with the original top-level Makefile, both replaced with straightforward and more conventional makefiles. The "man" and "completions" subdirs are now integrated into the build system and use configure-time variables for the paths.

No sed calls or patches should be required to set any paths or toolchain settings.

Usage of .gitattributes and $Id$ strings is now replaced with a VERSION define in the configure script, which optionally uses git to append the 8-digit commit hash to the version string. This also makes -V more meaningful.

Multiple configure checks were removed, some were unused, while others required to be run on the host at build time and were not compatible with cross. This means we are now assuming C99 compiler and availability of poll(); both seem reasonable.

After ~17 years of being deprecated, excluded from builds, and de-documented, svwait{up,down} and runsv{ctrl,stat} were removed from the codebase.

Closes #11
Closes #12

Things left to do

  • make check is broken has to be fixed. Long term there's probably room to improve/modernise the tests.

Going further

  • Should the docs be integrated in the build system now or as a later time (or not at all)?

  • The selection of default CFLAGS, warning flags, and LDFLAGS is mostly taken from XBPS's configure script. Are they adequate here, and are there any to add?

  • Lots of checks/etc seem to assume old and exotic platforms, but it's unclear to me which checks only apply to platforms excluded by the new requirements (C99, presence of poll(), $CC as linker) or that we can safely stop supporting.

    While trying to identify those, I found that there's too much variance with even the mainstream BSDs for that to be done in this PR. Anyways, the important part is that they're no longer tangled with the rest of the build system, and should be much easier to tackle individually later.

  • The configure check for the socket library checks for linker flags to add for socket.h to link properly. The only relevant results when searching online for libxnet and libnsl are about Solaris and a possibly unrelated libnsl.so missing on RHEL; does it make sense to keep this? A build on void does not require these.

    Decided to drop the check, details in commit

  • The old build system would check for the operating system and change the ar behaviour based on that, does this make sense nowadays? It seems the platforms it special-cases are all business unixes or obsolete. (It would run ranlib after ar on all platforms but some, currently replaced with adding the s modifier on ar)

    Decided to drop the special-casing, details in commit

  • PREFIX/EPREFIX usage was made to match XBPS, however it seems that XBPS does not properly use them according to the configure help text, is this correct? (EPREFIX should be used for "architecture-dependent" files, but is used or documentation but not executables)

    Turned into an xbps "bug?" report


Most of this was a learning experience, so I may have missed obvious things; feedback welcome.

About that complexity...

The makefile in src/ before
The original makefile
... and after.
The new makefile

.dots here.
I recommend opening original.dot in an interactive dot viewer to better see the node relationships.

prot.c provides two functions: prot_gid and prot_uid.

prot_gid is fully unused, and depends on {chk,try}shsgr, both of which must be
run at build-time and require that the user has additional groups. They are not
needed by anything else and their removal gets rid of cross-compilation jank.

prot_uid is used (once) by chpst, but since it is a no-op wrapper around
setuid(), its use can be replaced by a normal call to setuid().

Solves the big questions from void-linux#11
The "systype" machanism is merely a system info gatherer, and the info it
collects is (mostly) not used anywhere. The only seemingly important info is
the kernel and kernel version (or the lack thereof), and has been moved to
where it is used (print-ar.sh)

This gets rid of two cross-incompatible buildtime programs.
One tried to find if the build was happening on NeXT, but that's not an
operating system that print-ar.sh cares about. The other tried to do cpuid on
x86, which is useless to the build.

This seems to implement what void-linux#11 started.
@0x5c
Copy link
Author

0x5c commented Apr 4, 2023

Long term there's probably room to improve/modernise the tests.

Hum yeah, the mechanism is clunky and most of these don't seem very useful at all

@0x5c
Copy link
Author

0x5c commented Apr 13, 2023

-std=c99 was added to the CFLAGS to get uint64_t, however this now means that many files generate -Wimplicit-function-declaration, only solved by the definition of _DEFAULT_SOURCE or _POSIX_SOURCE in those files.

Are the feature macros an ok solution? Should they be defined everywhere? Is there a flag that does it better? y understanding is that -std=c99 does deactivate some defaults, but it's not obvious to me if there's a flag to reactivate them.

@nekopsykose
Copy link

Should they be defined everywhere?

unless there's something you're gaining by selectively defining them (you would know; generally it's to selectively expose a different standard of a function in some places intentionally), yes, just -D global cflags.

This gets rid of tryulong64.c, a buildtime configure binary which interferes
with cross-compilation, at the cost of bumping the minimum C version to C99.
Since poll() is available on all platforms we care about, we don't need to
check if it is.

This gets rid of trypoll.c, the last configure-time check that required to be
run on the host (messing with cross). We also get rid of trysysel.c, since
select() was only used as a fallback when poll() isn't avaialble.
This has dramatic effects on the build system complexity, and in the process
exposing lots of dead weight.

Split header files (.h1 .h2) are merged, and header files that only contained a
single define have been fully replaced with -D. The makefile is no longer
geneating scripts, and their files that hardcoded cc/ld/ar calls are gone.

The build system is now fully compatible with standard variables like CC,
PREFIX, DESTDIR, and such. The compiler is autodetected when unspecified, build
flags are checked for compatibility, and installation directories can now be
specified with configure flags.

The configure script was heavily inspired by XBPS's.

The "compile" script is gone along with the original top-level Makefile, both
replaced with straightforward and more conventional makefiles. The "man" and
"completions" subdirs are now integrated into the build system and use
configure-time variables for the paths.

No sed calls or patches should be required to set any paths or toolchain
settings.

Usage of .gitattributes and "$Id$" strings is now replaced with a VERSION
define in the configure script, which optionally uses git to append the 8-digit
commit hash to the version string. This also makes '-V' more meaningful.
They were all deprecated in runit 1.3.2 (Dec 2005), replaced with sv and
removed from the default targets of the makefile by 1.4.0 (Mar 2006), and fully
undocumented by 1.5.1 (May 2006).
This makes for a ~17 years deprecation notice, pretty good.
Many of the tests were broken from expecting a specific ~~version~~ commit hash
string in the command output (the $Id$ stuff).
Another was broken by a new argument added by void.
The unixes where the special case mattered are so niche nowadays that it seems
reasonable to change the value of $AR only if it is provided to configure.
~~~
runit 0.13.1
Mon, 19 Jan 2004 18:32:58 +0000
  * trysocketlib.c: new; check for libraries needed for socket() on some
    systems (fixes link failure on solaris, thx Uffe Jakobsen).
~~~

19 years later, we don't really need to care about solaris oddities anymore.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants