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.

Sign up for GitHub

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

Jump to bottom

./configure updates #4222

Merged
merged 10 commits into from
Jan 30, 2020
Merged

./configure updates #4222

merged 10 commits into from
Jan 30, 2020

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Nov 12, 2019

str4d and others added 10 commits November 12, 2019 13:01
@str4d
Revert "Add configure flags for enabling ASan/UBSan and TSan"
72aa644 
This reverts commit c4379e3.
@paveljanik @str4d
Rewrite help texts for features enabled by default.
fda3f64
@murrayn @str4d
Add build support for 'gprof' profiling.
c699c93
@eklitzke @str4d
Add --with-sanitizers option to configure
ce0a143 
This enables the use of different compiler sanitizers, coresponding to
the -fsanitize option in GCC and Clang.
@practicalswift @str4d
build: Show enabled sanitizers in configure output
76125f6
@eklitzke @str4d
Make --enable-debug to pick better options
45e2031 
Various changes:

 * Don't check $GCC and $GXX
 * Prefer -Og instead of -O0
 * If -g3 isn't available, use -g

This also incidentally fixes compiler warnings with GCC and glibc when using
--enable-debug, as the old default values mixed poorly with the hardening flags.
@ken2812221 @str4d
Reset default -g -O2 flags when enable debug
88c9e22
@practicalswift @str4d
Add -ftrapv to DEBUG_CXXFLAGS when --enable-debug is used
a8780a9
@dongcarl @str4d
autoconf: Sane --enable-debug defaults.
81ddf86 
Don't optimize at all when --enable-debug is supplied. This makes sure
that nothing is optimized out.
@str4d
configure: Re-introduce additional sanitizer flags
bbdba3b 
Re-adds flags that were removed in 72aa644.
@str4d str4d added A-build Area: Build system C-upstream-port Category: Changes that are ported from the Bitcoin Core codebase. labels Nov 12, 2019
@str4d str4d mentioned this pull request Nov 12, 2019
Open
@str4d str4d requested review from daira and defuse November 12, 2019 14:43
@str4d
Copy link
Contributor Author

str4d commented Nov 12, 2019

Best reviewed commit-by-commit.

I backported these while doing more work on #58.

@str4d str4d mentioned this pull request Nov 12, 2019
Closed
@oxarbitrage
Copy link
Contributor

The second commit for https://github.com/bitcoin/bitcoin/pull/12686/commits is not here. Any reason for it ?

@str4d
Copy link
Contributor Author

str4d commented Jan 29, 2020

We don't use Travis CI, and those patches generally don't apply to the .travis.yml in our codebase as we're missing a bunch of interleaving commits. We should just delete that file.

@oxarbitrage oxarbitrage mentioned this pull request Jan 29, 2020
Merged
oxarbitrage
oxarbitrage approved these changes Jan 29, 2020
View reviewed changes
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't tested all the new features introduced but it is compiling, all tests are passing, commits from bitcoin are exactly the same code, removing and readding sanitizer features look good to me.

zkbot added a commit that referenced this pull request Jan 29, 2020
@zkbot
Auto merge of #4314 - oxarbitrage:patch-2, r=str4d
cafc8c7 
Remove travis

As per #4222 (comment) deleting travis file.
@str4d str4d removed the request for review from daira January 30, 2020 15:58
@str4d
Copy link
Contributor Author

str4d commented Jan 30, 2020

Thanks!

@zkbot r+

@zkbot
Copy link
Contributor

zkbot commented Jan 30, 2020

📌 Commit bbdba3b has been approved by str4d

@zkbot
Copy link
Contributor

zkbot commented Jan 30, 2020

⌛ Testing commit bbdba3b with merge 8e8a935...

zkbot added a commit that referenced this pull request Jan 30, 2020
@zkbot
Auto merge of #4222 - str4d:configure-updates, r=str4d
8e8a935 
./configure updates

Includes code cherry-picked from the following upstream Bitcoin Core PRs:

- bitcoin/bitcoin#6748
- bitcoin/bitcoin#12373
- bitcoin/bitcoin#12692
- bitcoin/bitcoin#12901
- bitcoin/bitcoin#13005
- bitcoin/bitcoin#13445
- bitcoin/bitcoin#12686
- bitcoin/bitcoin#16435

Part of #2074.
@str4d str4d added the S-scratching-an-itch Status: Something we haven't planned for a sprint but are doing anyway label Jan 30, 2020
@str4d str4d added this to the Core Sprint 2020-03 milestone Jan 30, 2020
@zkbot
Copy link
Contributor

zkbot commented Jan 30, 2020

☀️ Test successful - pr-merge
Approved by: str4d
Pushing 8e8a935 to master...

@zkbot zkbot merged commit bbdba3b into zcash:master Jan 30, 2020
@str4d str4d deleted the configure-updates branch January 30, 2020 17:59
@daira
Copy link
Contributor

daira commented Jan 31, 2020 •
edited
Loading

I thought we used -ftrapv always?

Edit: it's actually -fwrapv that we use always. This is a problem because -ftrapv and -fwrapv conflict. Whichever is last on the gcc/g++ command line takes precedence, and I don't know which that is. The intent of enabling -ftrapv instead of -fwrapv in debug builds is perfectly fine, I just don't know whether the ordering of options will be correct.

@str4d
Copy link
Contributor Author

str4d commented Jan 31, 2020

Per this Automake FAQ, the ordering will be AM_CXXFLAGS CXXFLAGS. So with --enable-debug we should see ... -ftrapv ... -fwrapv ..., which is not what was intended here. I'll make a PR that moves -fwrapv to be in the else side of the enable_debug check.

@daira daira mentioned this pull request Jan 31, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oxarbitrage oxarbitrage oxarbitrage approved these changes

@defuse defuse Awaiting requested review from defuse

Assignees
No one assigned
Labels
A-build Area: Build system C-upstream-port Category: Changes that are ported from the Bitcoin Core codebase. S-scratching-an-itch Status: Something we haven't planned for a sprint but are doing anyway
Projects
None yet
Milestone
Core Sprint 2020-03
Development

Successfully merging this pull request may close these issues.
10 participants
@str4d @oxarbitrage @zkbot @daira @paveljanik @murrayn @eklitzke @practicalswift @ken2812221 @dongcarl