Pass compiler flags to leveldb make #2311

Merged
merged 4 commits into from Feb 18, 2013

Projects

None yet

5 participants

@gavinandresen
Member

Fixes issue#2288. Includes cleanups from Luke's pull 2243.

Tested on OSX, Linux, and cross-compiling in gitian.

"We" should refactor all of the common makefile code into a makefile.common, and include it from the other makefiles. There is an odd mix of CFLAGS / CXXFLAGS / xCXXFLAGS in our makefiles.

gavinandresen added some commits Feb 15, 2013
@gavinandresen gavinandresen Pass compiler flags down into leveldb make
Fixes issue#2288. Includes cleanups from Luke's pull 2243.
fbd8602
@gavinandresen gavinandresen Minor build fixes
Two changes: make some linux-specific linker options linux and linker specific.
And in the cross-compile environment, prefer the $HOME/qt/bin tools to
whatever might be somewhere else in the path.
efb6d9a
@luke-jr
Member
luke-jr commented Feb 16, 2013

ACK: tested clean builds of both bitcoin-qt and bitcoind individually with custom CXXFLAGS.

@luke-jr luke-jr referenced this pull request Feb 16, 2013
Merged

LevelDB build bugfix #2243

@Diapolo Diapolo commented on the diff Feb 16, 2013
bitcoin-qt.pro
@@ -29,19 +29,19 @@ contains(RELEASE, 1) {
!win32:!macx {
# Linux: static link
- LIBS += -Wl,-Bstatic
+ LIBS += -Wl,-Bstatic -Wl,-z,relro -Wl,-z,now
@Diapolo
Diapolo Feb 16, 2013

Are you sure all these are Linux only?

@gavinandresen
gavinandresen Feb 16, 2013 Member

They're definitely not OSX. I don't know if the mingw linker supports them-- can you find out please?

@luke-jr
luke-jr Feb 16, 2013 Member

relro at least is an ELF feature, so it doesn't make sense for Mac (Mach) or Windows (PE).

@Diapolo
Diapolo Feb 18, 2013

@gavinandresen

I tried ld -z,relro, which leads to:
ld: unrecognized option '-z,relro'
and ld -z,now, which leads to:
ld: unrecognized option '-z,now'

So these seem to be non present with MinGW. But before these were a QMAKE_CXXFLAGS and now you are using these with LIBS, which is correct? Just asking, as I don't know better :).

@Diapolo Diapolo commented on the diff Feb 18, 2013
bitcoin-qt.pro
@@ -29,19 +29,19 @@ contains(RELEASE, 1) {
!win32:!macx {
# Linux: static link
- LIBS += -Wl,-Bstatic
+ LIBS += -Wl,-Bstatic -Wl,-z,relro -Wl,-z,now
+ # for extra security (see: https://wiki.debian.org/Hardening)
+ QMAKE_CXXFLAGS *= -D_FORTIFY_SOURCE=2
@Diapolo
Diapolo Feb 18, 2013

From what I read after a quick websearch, this seems to be available to MinGW, as other projects are using that one also with MinGW.

@Diapolo
Diapolo Feb 19, 2013

@gavinandresen What was the reason to not consider or comment my comment above?

@BitcoinPullTester

Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/1d83141803477f9625e301149fc9611f43f94592 for binaries and test log.

This could happen for one of several reasons:

  1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts
  2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
  3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
  4. The test suite fails on either Linux i386 or Win32
  5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

@gavinandresen gavinandresen merged commit 1c3924d into bitcoin:master Feb 18, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment