Bug 255291

Summary: Make GNU_CONFIGURE option checking fatal
Product: Ports & Packages Reporter: Tobias Kortkamp <tobik>
Component: Ports FrameworkAssignee: Port Management Team <portmgr>
Status: New ---    
Severity: Affects Only Me CC: daniel.engberg.lists, ports-bugs, swills, tcberner
Priority: --- Flags: tobik: exp-run?
Version: Latest   
Hardware: Any   
OS: Any   
Description Flags
enable_option_checking.diff none

Description Tobias Kortkamp freebsd_committer 2021-04-21 07:16:20 UTC
Created attachment 224322 [details]

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]

- 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 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:

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.


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:
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(-)