-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Autotools RFC: Round 1 #2748
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
Autotools RFC: Round 1 #2748
Conversation
This is Part of an autotools buildsystem port. Previously MSG_NOSIGNAL was defined in a few places, and -DMSG_NOSIGNAL=0 was passed by the osx build to signify that it was unavailable there. For Win32, it was assumed to be unavailable. This commit flips the logic so that the unix build declares the feature. The HAVE_MSG_NOSIGNAL define will be (automatically detected and) set by autotools as well, so that both build methods will give the same results. There should be no observable changes in functionality here.
|
Looks mostly OK, well done. Comments:
|
|
Looks good. One question: why is the configure script and autogen.sh in src/, and not in the root of the project like most projects do? |
|
|
@laanwj I suppose there's no real reason, it just made more sense to me this way. One current issue with moving to the top would mean that the .pro file would have to move (if it's kept around after autotools merge), since it produces a Makefile that would conflict. |
|
@sipa the gpl in those files is effectively meaningless. Since they're always distributed in source-form, and resulting scripts are exempt from infection, the only way to violate would be to strip the attribution (which would violate MIT as well). So the net effect here is effectively nil. |
|
@theuni just assume that either the .pro will be used or the autotools build system, not both in parallel, so the makefiles won't get in each others way. |
|
I have a slight preference for configure in the top level, but I doń't feel strongly about it. |
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/fa5805fadc369ab8fd695dbd2fa0cc7a0bee1514 for binaries and test log. |
|
Just a generic comment: I'm strongly in favor of moving towards a real build system, but under the condition that we switch entirely. I wouldn't want to retain both the old and the new, and needing to maintain them both. So, as soon as this can be used to build on linux/linux-mingw/osx/win32, and the gitian builds work, I'm fine with merging. Perhaps @laanwj wants to retain the .pro file, but assuming the autotools plugin to Qt Creator is powerful enough, I'd rather get rid of it. |
|
Ok. I'm working on the reorg concerns above, I didn't have much time to get to it this week. In the meantime, though, @warren has helped me to get the gitian descriptors up and running for win32, and linux should be easy after that. I've gotten the same vibe from the others, so I'll finish up with the impression that back-compat is not needed. Also, @laanwj said he is ok with figuring out some solution for himself for the .pro file, so I'll not concern myself with that either. |
|
@theuni Hi, any updates about this? There have been a few refactors lately, which may impact this work (core.cpp/h and chainparams.cpp/h were added), but I think the largest changes are over now. I'd really like to see this merged. |
|
Hi @sipa I've been very occupied for the last few weeks, sorry for the lack of updates here. I have a branch that splits out the makefiles as requested. Personally I think it's a sizable regression, but I'm ok with that if it's what it takes to get it merged. I'll try find some time tomorrow to push up a rebased branch for review round2. |
|
@theuni Great, no actual hurry - I just wouldn't like this work to get lost. |
|
Closing in favor of round 2: #2805 |
* Fix "chainlock" in WalletTxToJSON * Add `AssertLockHeld(cs_main); // for mapBlockIndex` * move assert higher
Here is the first go at an Autotools build system replacement. It is meant to be a drop-in replacement for the current system(s), providing the same features with no net changes. It can also live side-by-side with the old system while sharing the same build-related variables in order to facilitate a smooth transition.
I hope the benefits are obvious enough: A single/shared build procedure, portability, ease of packaging, ease for downstreams, ease for repository maintainers, cross-compilation, etc. I don't vouch for Autotools in any way, in fact, this configure.am is downright ugly (mainly just because of mingw though), but it's portable and well-established.
I've opted not to write the documentation yet, because I would like to see what comments/concerns come out of the first round of review before committing to anything.
This does away with the need for qmake, as the Makefile is capable of generating everything it needs in a portable way.
Qt-creator can be used with the Autotools plugin, and is working nicely. For those who wish to use it this way, install the Autotools plugin from the 'about' menu, then open Makefile.am as a project. It handles the build procedure, so there is no need to mess with the command-line procedures listed below.
Building from CLI:
For Linux, assuming the dependencies have been met, the build procedure looks like this:
Same for OS X, but the pkg-config path needs to be hooked up from macports first:
For mingw it's the same, but you will need to provide lots of paths in the form of:
In addition, there are helpers for qt and boost to help with finding some locations.
Use ./configure --help to see the available options.
Native windows built is untested, as I don't have a windows environment at my disposal.
I have verified that linux builds are deterministic insofar as a fresh git clone will build bit-exact binaries to match another clean checkout on the same machine. To my knowledge, that means that gitian shouldn't have any trouble making the switch (after the descriptors are updated to use autotools).
'make check' will run the unit tests and print the results.
I've done my best to avoid adding any new behavior or features, and I would much prefer to aim for feature-parity before making any improvements.
TODO: