Bug 255291

Summary:

Make GNU_CONFIGURE option checking fatal

Product:

Ports & Packages

Reporter:

Tobias Kortkamp <tobik>

Component:

Ports Framework

Assignee:

Port Management Team <portmgr>

Status:

Closed Not Accepted

Severity:

Affects Only Me

CC:

diizzy, ports-bugs, swills, tcberner

Priority:

---

Flags:

tobik: exp-run?

Version:

Latest

Hardware:

Any

OS:

Any

Attachments:

Description	Flags
enable_option_checking.diff	none
enable_option_checking.diff	none

Description Tobias Kortkamp freebsd_committer

2021-04-21 07:16:20 UTC

Created attachment 224322 [details]
enable_option_checking.diff

I wonder if an exp-run that adds --enable-option-checking=fatal for
autotools builds would be a good idea.  The default is to only print
a warning on wrong options but this would make it fatal.

The goal would be to clean up all the wrong or obsolete configure args
that have accumulated over the years.  Later it would also prevent bugs
like bug #255280 where an argument has subtly changed and the maintainer
missed it.

Fallout might look like:
===>  Configuring for libpurple-2.14.3_1
configure: error: unrecognized options: --with-python, --with-html-dir, --disable-gtk-doc

Comment 1 Tobias Kortkamp freebsd_committer

2021-04-21 07:48:38 UTC

Created attachment 224323 [details]
enable_option_checking.diff

- Let ports override the setting if they need to

Comment 2 Tobias C. Berner freebsd_committer

2021-04-21 08:34:59 UTC

Moin moin 

globbing instead of the three conditions might make this a bit more readable?

ala ":M(dis|en)able-option-checking*"

mfg Tobias

Comment 3 Daniel Engberg freebsd_committer

2021-04-21 08:36:41 UTC

That's a good idea imho

Comment 4 Tobias Kortkamp freebsd_committer

2021-04-21 08:46:10 UTC

(In reply to Tobias C. Berner from comment #2)
:M does not support '|'.  But let's assume it did then it would also catch
options like --enable-option-checking-foobar which does not seem right.

Why do you find the current version unreadable?

Comment 5 Tobias C. Berner freebsd_committer

2021-04-21 08:56:52 UTC

(In reply to Tobias Kortkamp from comment #4)
Unreadable might be a strong word -- but generally, if you can get away without using && / || it makes it more concise and understandable at a glance.

But yeah, if it is not supported, than that is a moot point :) 


mfg Tobias

Comment 6 Antoine Brodin freebsd_committer

2021-04-21 20:09:04 UTC

More than 22k ports were skipped

New failures logs:

http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/libiconv-1.16.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/sqlite3-3.34.1_1,1.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/unixODBC-2.3.9.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/ccache-3.7.1_1.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/ccache-static-3.7.1_1.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/gettext-runtime-0.21.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/libpciaccess-0.16.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/swig-4.0.2.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/tcltls-1.7.18.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/tex-synctex-1.17.0_1.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/racket-minimal-7.9.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/carbonzipper-0.74.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/check_ups_health-2.9.2.4.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/lldpd-1.0.8_1.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/pmacct-1.7.5.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/tex-ptexenc-1.3.3_2.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/samhain-4.4.3.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/samhain-client-4.4.3.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/samhain-server-4.4.3.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/incron-2017.11.13_1.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/squid-4.14.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/squid-devel-5.0.5.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/libfontenc-1.1.4.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/libXau-1.0.9.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/libXdmcp-1.1.3.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/libxshmfence-1.3.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/pixman-0.40.0_1.log
http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-21_17h41m47s/logs/xtrans-1.4.0.log

Comment 7 Mathieu Arnold freebsd_committer

2021-04-22 08:13:15 UTC

Why not guard this with DEVELOPER being defined, so that what is there still builds but maintainers have to fix their ports.

Comment 8 Tobias Kortkamp freebsd_committer

2021-04-22 08:48:04 UTC

It isn't so easy.  Some of the failures (libiconv, gettext-runtime)
are caused by how AC_CONFIG_SUBDIRS works in autotools:

> Because the packages generally support different --with-package
> and --enable-feature options, the GNU Coding Standards say they
> must accept unrecognized options without halting.  Even a warning
> message is undesirable here, so AC_CONFIG_SUBDIRS automatically
> disables the warnings.

https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.70/html_node/Option-Checking.html

Maintainers cannot really fix that and it would just look like
appending --enable-option-checking=yes or --disable-option-checking
everywhere which I don't think is an improvement.

We cannot pass --enable-option-checking=fatal to ports that use
AC_CONFIG_SUBDIRS.  But we can check for this in SET_LATE_CONFIGURE_ARGS
and only enable for ports that do not use it.

And yes we should guard that with DEVELOPER because this is really
finicky stuff better left to the maintainers.

I fixed a couple of ports: libiconv, gettext-runtime, and the
USES=xorg-cat:lib ports should build now.

I've pushed this to https://github.com/t6/freebsd-ports/tree/tobik/enable-option-checking
An updated patch can be fetched from here:
https://github.com/freebsd/freebsd-ports/compare/main...t6:tobik/enable-option-checking.patch

Comment 9 Tobias Kortkamp freebsd_committer

2021-04-22 09:01:55 UTC

Guarding it with DEVELOPER is not really practical, since Poudriere does not
seem to export it port testing mode for configure(?)

Comment 10 Mathieu Arnold freebsd_committer

2021-04-22 09:05:45 UTC

Poudriere puts DEVELOPER=yes in the jail's make.conf, so it should work just fine, if not, then it is a bug in poudriere.

Comment 11 Mathieu Arnold freebsd_committer

2021-04-22 09:07:37 UTC

The link to the patch contains unrelated stuff, like opt_LIB_DEPENDS changes, it makes it harder to figure out what goes on. (also, using .diff instead of .patch is better for this use, it gives you one unified diff, and not a suite of patches.)

Comment 12 commit-hook freebsd_committer

2021-04-24 06:46:25 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=7069bc1a13af0d6e8dae4308602afd4b8fb40749

commit 7069bc1a13af0d6e8dae4308602afd4b8fb40749
Author:     Tobias Kortkamp <tobik@FreeBSD.org>
AuthorDate: 2021-04-22 08:03:45 +0000
Commit:     Tobias Kortkamp <tobik@FreeBSD.org>
CommitDate: 2021-04-24 06:46:08 +0000

    net-mgmt/pmacct: Unbreak build with --enable-option-checking=fatal

    ===>  Configuring for pmacct-1.7.5
    configure: error: unrecognized options: --disable-, --enable-64bit

    There is no --enable-64bit option anymore.  The --disable- is caused
    by the extra = after KAFKA_CONFIGURE_ENABLE= because the framework
    splits *_CONFIGURE_ENABLE on =.  Arguably it should raise an error
    here instead of appending nonsense like --disable-.  This took
    forever to find.  :-(

    PR:             255291

 net-mgmt/pmacct/Makefile | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)