configure summary is confusing or incorrect

changed milestone to %Tor: 0.4.4.x-final

added 044-should component::core tor/tor milestone::Tor: 0.4.4.x-final owner::dgoulet points::0.5 priority::medium resolution::fixed reviewer::teor severity::normal status::closed tor-build type::defect version::tor 0.4.3.1-alpha labels

Replying to teor:

Some of the options in the new configure summary are confusing or incorrect. The configure summary was introduced in #31373 (moved).

Wrong place:

maybe --enable-systemd should be an optional library?

Agree.

Inverted, should be --enable-* :

--disable-seccomp

--disable-libscrypt

--disable-gcc-hardening ?

--disable-linker-hardening ?

--disable-module-dirauth

--disable-unittests

All these are enabled by default and thus why we put in the --disable. If one does autocompletion on ./configure --enable-, none of the above shows up.

Remove Double-Negative ? :

assert()s disabled (--disable-asserts-in-tests, dev only): no

assert()s enabled (--enable-asserts-in-tests, dev only): yes

Agree.

Broken:

--enable-gcc-warnings:

is not Verbose Warnings

is an alias for --enable-fatal-warnings, which is already listed

did you mean --disable-gcc-warnings-advisory ?

I did not realize it was an alias and it is deprecated. I will remove it.

--disable-asciidoc

also disables manpages and HTML, but they are shown as enabled

Added!

--enable-fragile-hardening

should be true if --enable-expensive-hardening is set, but is shown as false

Fixed.

Missing:

--disable-module-relay

That one is there.

please build the module feature summary from the list of modules in configure

Done.

--disable-asciidoc

Fixed.

--enable-lzma

--enable-zstd

--enable-cargo-online-mode

Added the above.

--with-tor-user=[user]

--with-tor-group=[group]

I have no clue where this is used. I can't find TORUSER or TORGROUP being used anywhere?

and a few others

and a whole bunch of env vars

For now this is a good improvement! We can add the rest as we notice them.

Branch: ticket32230_043_01 PR: https://github.com/torproject/tor/pull/1701

(Putting teor as a reviewer in order to confirm if this is acceptable from their original post).

Trac:
Status: assigned to needs_review
Reviewer: N/A to teor

Replying to dgoulet:

Replying to teor:

Inverted, should be --enable-* :

--disable-seccomp

--disable-libscrypt

--disable-gcc-hardening ?

--disable-linker-hardening ?

--disable-module-dirauth

--disable-unittests

All these are enabled by default and thus why we put in the --disable. If one does autocompletion on ./configure --enable-, none of the above shows up.

Ok, but the output is still wrong:

test "x$enable_seccomp" != "xno" && value=1 || value=0
PPRINT_PROP_BOOL([libseccomp (--disable-seccomp)], $value)

If enable_seccomp is yes, then value is 1, and we print --disable-seccomp yes. But we should actually print one of these:

--enable-seccomp yes
--disable-seccomp no

I prefer the output without the double-negative. But I can understand if you want to print the non-default option as well.

--with-tor-user=[user]

--with-tor-group=[group]

I have no clue where this is used. I can't find TORUSER or TORGROUP being used anywhere?

Maybe nickm can help?

and a few others

and a whole bunch of env vars

For now this is a good improvement! We can add the rest as we notice them.

Yes, I agree. A good summary doesn't need everything. Just the important and useful things.

Branch: ticket32230_043_01 PR: https://github.com/torproject/tor/pull/1701

I'll do a full review once the PR has been revised.

Trac:
Cc: N/A to nickm
Status: needs_review to needs_revision

Replying to teor:

Replying to dgoulet:

--with-tor-user=[user]

--with-tor-group=[group]

I have no clue where this is used. I can't find TORUSER or TORGROUP being used anywhere?

Maybe nickm can help?

I can't find any evidence of these fields either. They used to be used by some of the scripts in contrib/dist, but all those scripts are now removed, since they were obsolete and unmaintained.

Replying to teor:

Replying to dgoulet:

Replying to teor:

Inverted, should be --enable-* :

--disable-seccomp

--disable-libscrypt

--disable-gcc-hardening ?

--disable-linker-hardening ?

--disable-module-dirauth

--disable-unittests

All these are enabled by default and thus why we put in the --disable. If one does autocompletion on ./configure --enable-, none of the above shows up.

Ok, but the output is still wrong:

{{{ test "x$enable_seccomp" != "xno" && value=1 || value=0 PPRINT_PROP_BOOL([libseccomp (--disable-seccomp)], $value) }}}

If enable_seccomp is yes, then value is 1, and we print --disable-seccomp yes. But we should actually print one of these:

--enable-seccomp yes

--disable-seccomp no

I prefer the output without the double-negative. But I can understand if you want to print the non-default option as well.

Right, the string is hardcoded regardless of the value of the option. Basically, the --disable-seccomp is the configure option that is available, regardless if you enabled it or not.

The way to look at it is libseccomp = yes/no. The configure options there were added so the user knows how to switch the option on or off.

I agree it is confusing. We would need to handle different text output for each option depending on their values. Maybe another ticket?

Another way to look at it is that --enable-seccomp virtually doesn't exists, you either have no option which enables seccomp by default or you disable it with --disable.

Trac:
Status: needs_revision to needs_review

Can we try a slightly different format, that includes the value of each configure option?

For example:

libseccomp (--disable-seccomp=no)    yes

This format makes the double-negative explicit, so it may be less confusing.

We could generate "yes" or "no" based on a boolean argument, rather than printing the option value directly? If that's easier?

Trac:
Status: needs_review to needs_revision

I've tried for 5 min to add that value and the m4 world is not allowing me for some reasons. I think because the moon is unaligned and too much voodoo with autoconf/m4.......

I'll drop this for now, I can't spent more time on this :S.

Trac:
Milestone: Tor: 0.4.3.x-final to Tor: unspecified
Keywords: 043-must deleted, N/A added

Ok, let's merge what we have in 0.4.3, and then see if we can do better in 0.4.4 ?

We need to fix a typo in the changes file, the configure summary was only released in 0.4.3.1-alpha.

Then we can merge this PR:

0.4.3 (master): https://github.com/torproject/tor/pull/1701

But keep the ticket open.

Trac:
Milestone: Tor: unspecified to Tor: 0.4.3.x-final
Version: Tor: unspecified to Tor: 0.4.3.1-alpha
Status: needs_revision to merge_ready
Keywords: N/A deleted, 044-should, tor-build added

Fixed and merged to master; leaving open for further fixes in 0.4.4.

Trac:
Status: merge_ready to needs_revision

Trac:
Milestone: Tor: 0.4.3.x-final to Tor: 0.4.4.x-final

The big fixes are done; we can do further improvements as we see the need.

Trac:
Resolution: N/A to fixed
Status: needs_revision to closed

closed

changed time estimate to 4h

mentioned in issue #33646 (moved)

moved to tpo/core/tor#32230 (closed)

mentioned in issue tpo/core/tor#33646 (closed)

configure summary is confusing or incorrect

Child items ...

Activity