./configure inverts --disable logic

[jkroonza]

Still looking for a generic fix (I find this bug exists in a great many number of ./configure scripts), but in short as an example:

$ ./configure --disable-redis
...
Build with REDIS............ : yes
...

Not having written any serious configure.ac in my life, I reckon what is needed is some structure like:

if (--disable-feature given) {
    disable_feature
} else {
    run tests for feature; # can stop at first failure if done smartly.
    if (some test failed) {
        if (--enable-feature given) {
            fail_configure
        }
        disable_feature
    } else {
        enable_feature
    }
}

Now, it becomes more complicated since multiple feature can have the same requirements (eg, --enable-ssl and --enable-tls - although, I still need to look at this), so ideally what we really need is a more complex structure, however, AC already caches, so whether or not it's really an issue to run ACCHECK* multiple times is debatable. Or we really want to check if it has been run already and use the previous result if it was (which is probably a simpler strategy than trying to first determine which checks all we need etc ...).

Still brain-storming on this. But in short, this is actually looks more complex than I had hoped. Just filing the issue for now as I need to sort this for captagent since I'm packaging for Gentoo based on customer requirement, and to do that properly/correctly I need a predictable ./configure where I can pass both --enable-feature and --disable-feature and have it work correctly and as expected.

[jkroonza]

@kYroL01

Is there any CI in place that I can somehow trigger to see if I break anything on this? Ideally that I can run from the source tree rather than having to push a PR?

[adubovikov]

https://github.com/sipcapture/captagent/commit/a27139cd27aae3392c7b8e8bdeed0db50f0f2503

Please check it as well.

/configure --enable-redis --enable-ipv6 --enable-mysql --enable-epan --enable-tls --enable-pcre

captagent 6.3.0

Build directory............. :
Installation prefix......... : /usr/local/captagent
HEP Compression............. : no
IPv6 support.................: yes
HEP SSL/TLS................. : no
Flex........................ : flex
Bison....................... : bison -y

Build with REDIS............ : yes
Build with MySQL............ : yes
Build with PCRE............. : yes
Build with LibUV............ : yes
Build with EPAN............. : yes
Build with TLS.............. : yes

./configure --disable-redis --disable-ipv6 --disable-mysql --disable-epan --disable-tls --disable-pcre

captagent 6.3.0

Build directory............. :
Installation prefix......... : /usr/local/captagent
HEP Compression............. : no
IPv6 support.................: no
HEP SSL/TLS................. : no
Flex........................ : flex
Bison....................... : bison -y

Build with REDIS............ : no
Build with MySQL............ : no
Build with PCRE............. : no
Build with LibUV............ : yes
Build with EPAN............. : no
Build with TLS.............. : no

[jkroonza]

Looks good for the "default disabled" case, which is what this project specifically uses. Are there plans for a 6.3.2 some time soon which includes these changes? Else I'll include the patch separately.

[adubovikov]

yes, if it's ok for you, we can make a release

[jkroonza]

yes, if it's ok for you, we can make a release

Tests OK on this side, had to download the patch for that, so come to think of it, just batch it up with the next release. Do you have some form of release notification mechanism on new releases so that I can keep Gentoo up to date on this?

Thank you for handling this, you're most welcome to close.

[jkroonza]

Actually, I was too quick:

./configure --prefix=/usr --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc --localstatedir=/var/lib --disable-dependency-tracking --disable-silent-rules --docdir=/usr/share/doc/captagent-6.3.1 --htmldir=/usr/share/doc/captagent-6.3.1/html --with-sysroot=/ --libdir=/usr/lib64 --enable-compression --disable-epan --enable-ipv6 --enable-mysql --enable-pcre --disable-redis --enable-tls --enable-ssl
configure: loading site script /usr/share/config.site
...
Build with REDIS............ : no
...

But still the following files are being installed:

$ equery files captagent | grep redis
/etc/captagent/database_redis.xml
/usr/lib64/captagent/modules/database_redis.a
/usr/lib64/captagent/modules/database_redis.la
/usr/lib64/captagent/modules/database_redis.so

The draft PR for Gentoo at https://github.com/gentoo/gentoo/pull/20854 in case you are interested (and also why this is somewhat important to get fixed, not only for Gentoo, but as a general rule). We like to specify as many of the --enable/--disable switches as we can to be very explicit about what it is that we want. Other distributions have somewhat different ways of dealing with this and would run a single compile and split the modules off into different packages (Debian I know is kings of this).

Some ./configure's auto-detects what's available and compiles all features for which deps are available automatically, this results in what we call automatic dependencies (as per what redis above would currently cause when specifying --disable-redis).

[adubovikov]

no, it's correct, because --disable arguments disable library check and linking, but the module will be compiled anyway but without a library support.

[jkroonza]

Ok ... that doesn't make a lot of sense for me but I can understand it.

I'd assume that a database_redis.so module won't make sense unless you compile and support redis ... looking at the code in database_redis.c, specifically insert_and_update() (which should probably be static and not in the header file) becomes a no-op, as does many other commands ...

This is perhaps something to then look at, it seems modules are abstracted anyway, so the only symbol they really need to export is the "exports" struct. Everything else can (and should) thus be static.

Looking at that, load_module does LERR complaining that redis support wasn't compiled in but then happily proceeds to still set up all the profiles. In a case like this I'd suggest that the module doesn't even get built. If I find time I'll what I can do w.r.t. a PR.

Thanks for the information.

[adubovikov]

i will also take a look and add white/black list for that