Backport leveldb build integration to 0.12 #8148
Conversation
This is a cherry-pick of a4625ac with minor conflict resolution. Conflicts: src/Makefile.am ===== leveldb: integrate leveldb into our buildsystem leveldb's buildsystem causes us a few problems: - breaks out-of-tree builds - forces flags used for some tools - limits cross builds Rather than continuing to add wrappers around it, simply integrate it into our build.
|
I think NACK, since this doesn't actually fix any bugs? |
|
@luke-jr It allows building the 0.12 branch using VPATH builds. That's a major convenience for me to be able to share the same build/test scripting between master and 0.12; I expect for others as well. |
|
Every feature is a convenience to someone. |
|
Ok, whatever. I don't think I've ever submitted a pull request here that didn't immediately run into weird objections. |
|
This could be useful to keep the testing the same. However, out-of-tree build support needs more changes for proper support. There was #7925, and one is open even now: #8133. Backporting just that commit is not enough. |
|
@jcorgan You don't need to close a PR because one person complains. Things
can be discussed.
We typically only backport bug fixes and soft forks to older releases, but
perhaps it makes sense to keep the build system compatible as well. I don't
have a strong opinion here.
|
|
Agree with @sipa. |
|
If you think this is something you'll eventually merge, I have no problem working with @theuni to ensure any needed changes from 0.13 make it back into 0.12. I'll go ahead and get #7925 added, and when #8133 is merged, that too. However, I won't waste my own or anyone else's time if this is not the direction you want to go, though I still find this useful enough for my own purposes. |
===== qt: Fix out-of-tree GUI builds Without this patch: - When I compile the GUI from the bitcoin directory itself, it works as expected. - When I build the GUI in an out-of-tree build, I cannot get it to select tabs. When I click, say the "Receive" tab nothing happens, the button selects but it doesn't switch the page. The rest - even the debug window - seems to work. See full discussion here: bitcoin#7911 (comment) This turned out to be caused by a mismatch in the arguments to moc, preventing it from finding `bitcoin-config.h`. Fix this by passing `$(DEFAULT_INCLUDES)` to it, which gets set to the appropriate path by autoconf itself.
===== Re-instate TARGET_OS=linux in configure.ac. Removed by 351abf9.
|
@jmcorgan I have no problem with backporting this and a few others to make life easier, but as @laanwj said, aiming to backport fully-supported VPATH builds is probably over-reaching, given that we're still cleaning up minor things in master. As for #7982, no, that shouldn't be backported as c++11 won't be a requirement for 0.12.x. |
|
I'm a little confused then--if the backport is done properly, don't VPATH builds just come "for free"? In other words, is there some extra effort needed other than ensuring any leveldb-build related commits on master get cherry-picked appropriately? |
|
@jmcorgan The changes you have here should be enough for most basic VPATH builds, but there are several edge-cases that remain, mostly related to packaging/release stuff which we can live without in the backport. |
|
Ok, great. I'll do some testing and A/B comparison between the build artifacts created with and without this branch then. Do you prefer to have this squashed or have the individual cherry-picks left in? |
Let's leave those in. |
|
Tested ACK (including GUI) 9462e79 |
|
Interesting. Is there any git wrapper that resets the author on cherry-picks? |
Not aware of one, at least. |
6 participants
This is a cherry-pick of a4625ac with minor conflict
resolution.
Conflicts:
src/Makefile.am
leveldb: integrate leveldb into our buildsystem
leveldb's buildsystem causes us a few problems:
Rather than continuing to add wrappers around it, simply integrate it into our
build.