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

Print summary at the end of the configure script #1218

Closed
wants to merge 3 commits into from

Conversation

Labels
None yet
Projects
None yet
4 participants
@dgoulet-tor
Copy link
Contributor

@dgoulet-tor dgoulet-tor commented Aug 8, 2019

Signed-off-by: David Goulet dgoulet@torproject.org

Signed-off-by: David Goulet <dgoulet@torproject.org>
@coveralls
Copy link

@coveralls coveralls commented Aug 9, 2019

Pull Request Test Coverage Report for Build 6678

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 180 unchanged lines in 2 files lost coverage.
  • Overall coverage remained the same at 63.366%

Files with Coverage Reduction New Missed Lines %
src/feature/dirparse/authcert_parse.c 17 69.23%
src/feature/dirparse/ns_parse.c 163 68.55%
Totals Coverage Status
Change from base Build 6650: 0.0%
Covered Lines: 47997
Relevant Lines: 75746

💛 - Coveralls

@dgoulet-tor dgoulet-tor force-pushed the ticket31373_042_01 branch from ca3227a to e02cff8 Sep 12, 2019
configure.ac Outdated
PPRINT_SUBTITLE([Build Features])

test "x$enable_gcc_warnings" != "xno" && value=1 || value=0
PPRINT_PROP_BOOL([GCC Warnings], $value)
Copy link
Contributor

@nmathewson nmathewson Sep 18, 2019

We should call this "verbose warnings"; it applies to clang too.

Copy link
Contributor

@nmathewson nmathewson Sep 18, 2019

Maybe it would be a good idea to also print, for this setting and all the others, how to override it?

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 16, 2019

Yup. There is one fixup commit that fixes it all. I recommend you rerun the configure script to see the end result. I changed many things from your review :).

configure.ac Outdated
PPRINT_PROP_BOOL([GCC Warnings], $value)

test "x$enable_fatal_warnings" != "xno" && value=1 || value=0
PPRINT_PROP_BOOL([GCC Fatal Warnings], $value)
Copy link
Contributor

@nmathewson nmathewson Sep 18, 2019

Let's call this "Warnings are fatal"

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 16, 2019

+1

configure.ac Outdated
PPRINT_PROP_BOOL([GCC Fatal Warnings], $value)

test "x$enable_coverage" = "xyes" && value=1 || value=0
PPRINT_PROP_BOOL([Code Coverage], $value)
Copy link
Contributor

@nmathewson nmathewson Sep 18, 2019

This maybe belongs in a testing section.

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 16, 2019

+1

configure.ac Outdated
fi

AS_ECHO
PPRINT_SUBTITLE([Security])
Copy link
Contributor

@nmathewson nmathewson Sep 18, 2019

Maybe we should have an "optional libraries" section for NSS and libscrypt and libsystemd. This section makes it sould like your system is less secure if it doesn't have NSS, which isn't really true.

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 16, 2019

+1

configure.ac Outdated
PPRINT_PROP_BOOL([libscrypt support], $value)

test "x$enable_fragile_hardening" != "xno" && value=1 || value=0
PPRINT_PROP_BOOL([libasan support], $value)
Copy link
Contributor

@nmathewson nmathewson Sep 18, 2019

Let's call this "fragile hardening" to match the option. Also let's mark it as "not for production use" somehow-- people should use this in production.

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 16, 2019

+1

configure.ac Outdated
PPRINT_PROP_BOOL([libasan support], $value)

test "x$enable_gcc_hardening" = "xyes" && value=1 || value=0
PPRINT_PROP_BOOL([GCC Hardening], $value)
Copy link
Contributor

@nmathewson nmathewson Sep 18, 2019

Let's call this "compiler hardening"?

This option is supposed to be on by default, but when I ran the configure script, it said that it was off. I think there might be a bug.

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 16, 2019

+1

I fixed the bug I believe.

configure.ac Outdated
PPRINT_PROP_BOOL([Unit tests], $value)

test "x$enable_asserts_in_tests" = "xyes" && value=1 || value=0
PPRINT_PROP_BOOL([Unit tests assert()], $value)
Copy link
Contributor

@nmathewson nmathewson Sep 18, 2019

This option was mis-reported when I ran the script. It is almost always on.

Its name is a little confusing, too. How about invert this, can call it "assert()s disabled for unit tests"

We should also mark this option as not safe to use --disable-asserts-in-tests in production.

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 16, 2019

+1

AS_ECHO
PPRINT_SUBTITLE([Install Directories])

report_mandir="`eval eval echo $mandir`"
Copy link
Contributor

@nmathewson nmathewson Sep 18, 2019

eval eval? I don't understand this.

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 16, 2019

Because just one eval gives: ${prefix}/share/man so the second one evaluates that prefix.

report_mandir="`eval eval echo $mandir`"
PPRINT_PROP_STRING([Binaries], [$BINDIR])
PPRINT_PROP_STRING([Configuration], [$CONFDIR])
PPRINT_PROP_STRING([Man Pages], [$report_mandir])
Copy link
Contributor

@nmathewson nmathewson Sep 18, 2019

Is there any way that we can get the complete configure command line as it was provided to us? For a lot of cases, that would be more useful for debugging than all the rest of this stuff.

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 16, 2019

I added a last line that does that :).

@dgoulet-tor dgoulet-tor force-pushed the ticket31373_042_01 branch from e02cff8 to 36625b1 Oct 16, 2019
@torproject-pusher torproject-pusher deleted the branch torproject:master May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment