Code re-organization and autotools build system #180
Conversation
|
Small aesthetical error: The name of the generated binary seems to always be bitcoind, whether or not a GUI is included. If you're going for the "one binary, with some optional features", I'd call it bitcoin instead of bitcoind. Apart from that, it looks clean and seems to configure, compile and run fine. |
|
hi sipa, i've investigated the issue behind double binary build: bitcoin and bitcoind long story short: if we keep this, then all code in src/ will be compiled twice by make, with and without #define flags that activate the gui. CPU cycles are worth :^) and as we proceed cleaning the code for a libbitcoin, then libtool will do its job and we'll have separated directories and test units. but let's leave that to the next pull request. the next commit implements bitcoin and bitcoind build (current behaviour) and shines up some cxxflags |
|
Several things: 1. If you are making a pull request, you might as well just git push -f to overwrite the old commits with new ones instead of making new pull requests all the time. Github will change the pull request accordingly (unless you change the branch to match master, at which point Github closes the pull request for you). |
|
hi TheBlueMatt, On Sun, 24 Apr 2011, TheBlueMatt wrote:
wow! i didn't knew that and it saves me quite some time clicking
the 'configure' file is automatically generated by autoconf from in order to generate it, run: 'autoreconf -i' the configure file itself is never included in code revisioning, but tarballs can be produced very comfortably with 'make distcheck' jaromil, dyne.org developer, http://jaromil.dyne.org |
|
speaking of which, i've just noticed the 'distcheck' target fails to compile. that compilation target actually is there to facilitate tar.gz releases, but also to verify that the autotools packaging is well portable and working in all situations... so i'm having a look to fix that now in the next commits. |
|
Pull request is now rebased to an acceptable state. |
|
Looks great! One final blocker: cryptopp should not be built using the libtool machinery. All that accomplishes is making it slow, and creating an annoying build process where "make -s" does not work properly. All references to libtool should be removed, and cryptopp should be built via noinst_LIBRARIES = libcryptopp.a |
|
I support this re-org; jgarzik, can you do the pull when you're happy? The READMEs and build-*.txts should mention running autoreconf -i to bootstrap if you're building from git. |
|
The re-org should be pulled, but autotools isnt quite ready...I was hoping (and jaromil seemed to indicate that he would work on) support for mingw cross compiling before pull (discussed after pull request was submitted). And the whole ridiculous sed action required now (4870f7d) is not really pull-ready. |
|
NAK the mingw stuff from Matt -- it is absolutely not needed. That stuff varies from system to system, and hardcoding it into configure breaks platforms other than the one Matt's using. @gavin: can do |
|
@jgarzik Isnt that the point of autotools, it finds the stuff to include and link and uses that. As I said, the stuff in this pull request from my repo really should never be pulled (because of what you just said) |
|
You provide OS-specific details to a configure script via environment variables (or occasionally, command line arguments). The script takes it from there. That's how mingw32-configure works. It sets the configure variables correctly for the limited set of systems it knows about. configure already has all the proper cross-compiling gadgetry built it, to compile windows apps. |
|
source tree reorg pulled |
|
you did not pull, you actually did a "manual merge" i'll past here my reaction on irc: 22:59 oh, did you merged autotools? my reason to be upsed is that this confuses all the review process of this pull request so far and hereby request you merge my original commits into master, not a "manual merge" that deletes all the documentation written in the commits. this pull request is not yet finished, you can close this pull request if you really don't like a WIP here, i'll bring it back once completed. thanks for your attention to details, |
|
It takes two seconds to verify that commit 84c3fb0 is in bitcoin/bitcoin.git. Maybe three. |
|
i was tired and almost going to sleep when i saw your merge message, apologies for my abrupt reaction. ultimately i care that all the documentation so far written in the commit messages goes into history as it has been reviewed here, else this huge movement of files will look too suspicious. i did not noticed the autotools (2nd in this branch) and thought your merge message was its rebase without the original message (the term "manual merge" is what confused me). today i'll work on the second commit, the autotools build, and see if mingw32 can be integrated. In fact there is something strange still: autotools doesn't seems to honor environment flags and i want to look into it deeper, hopefully we can get rid of the hacky sed then. ciao |
|
seems now everything is fixed i'm rebasing the second commit of this branch into something presentable |
|
updated to apply on current HEAD updated to add BlueMatt's fixes from I can rebase it into one commit, or leave like this. |
|
Looking for confirmation that this builds w/ mingw32 and OSX... |
|
It does build. i'm also asking more people to test it. meanwhile this patch was contributed to fix build issues on BSD/OSX i can rebase it into this if you agree, yet i'm asking because this patch changes actual bitcoin code (just some #define conditionals), while so far this autotools pull req required nearly no change to code. hoping we can have this pull request merged and closed soon! |
|
I would like to see that patch as a separate commit, in this autotools pull. Makes it easier to look through, but the patch has my ACK. |
Adding autotools for build checks, configure flags for compile time configuration and handling of #define directives inside code and things that will possibily make it better for bitcoin to be packaged inside distributions, as well ported to different architectures. The bitcoind code itself was never modified: files were moved around and the header auto-config.h generated by autoconf was added to headers.h. Code modules have been separated in subdirectories and compiled as static libraries. To start first generate configure using 'autoreconf -i' then the usual ./configure && make Use make V=1 for verbose compulation output. Configure flags --------------- Besides the usual flags provided by autotools, the following are notable: --enable-upnp=0/1 has been added and configure.ac contains templates for adding more compile time choices in future. --enable-gui has been added also to activate compilation of the included WX GUI. other wx related flags are provided to indicate prefix or static library build. --enable-debug compiles the binaries with debugging flags, giving all warnings - anyone willing to help should run it at least once. --enable-profiling enables support for gprof the GNU profiler (will dump gmon.out files after run) and gconv the GNU coverage tool --with-wxdir=PATH Use uninstalled version of wxWidgets in PATH --with-wx-prefix=PREFIX Prefix where wxWidgets is installed --with-boost and more boost build configuration --with-boost-lib (use both boost flags to activate this option) Build targets -------------------- Please note also 'make distcheck' is working to facilitate stable releases distribution. Build was tested on Debian, Ubuntu, Apple OSX 10.5, CYGWIN win32 and Cross-compiled using MinGW32 on GNU/Linux
…and MSG_NOSIGNAL * switched to using HAVE_SO_NOSIGPIPE and HAVE_MSG_NOSIGNAL provided by autotools
|
OSX Broken? (10.6.7 xcode 4): mith@dair:0: |
|
Get the same thing running with autoconf 2.61 out of /usr/bin instead of macports. |
|
Yeah, it fails on Fedora as well, missing the WX macros and similar errors to OSX |
|
BlueMatt said use autoreconf -i; and that seems to work. If the mechanism to rebuild is not just "autoconf" it needs to be stuck into an autogen.sh / autoget.bat please. |
|
You also seem to have reverted all my stuff that made MinGW work, its not in the current code and wont compile. |
|
Also the resultant distclean target after autoconf/make does not remove the autoconf output. Can that be fixed? |
|
@jrmithdobbs: that last is standard. you want AM_MAINTAINER_MODE + "make maintainer-clean" (or "make maintainerclean", I forget) "distclean" is only supposed to return you to the state of a freshly-unpacked dist tarball |
|
Closing as outdated. We do want an autotools configury of some sort, but with Qt the picture changed a bit. So, ACK'ing the concept but NAK'ing the current pull. |
|
On Tue, 20 Dec 2011, Jeff Garzik wrote:
IMHO the problem with this pull request has been waiting too much to I understand that back then when I filed the pull request the ciao jaromil, dyne.org developer, http://jaromil.dyne.org |
Extract, move and test StrToInt64
e7eb805 Update seed blocks to 370000 (zathras-crypto)
add lock for nLargestBlockSeen
…ntract-txs Fix problems with spending zero-confirmation transactions to create/call contracts
release: set for v0.18.1.0 release
79a2576 doc: update ConnectionType Doxygen documentation (Jon Atack) f3153dc gui: improve markup handling of connection type tooltip (Jon Atack) 4f09615 gui: return inbound {full, block} relay type in peer details (Jon Atack) Pull request description: Three follow-ups to #163: - return relay type for inbound peers - improve markup handling in the tooltip to facilitate translations - update ConnectionType doxygen documentation  ACKs for top commit: hebasto: re-ACK 79a2576, only suggested changes since my [previous](bitcoin-core/gui#180 (review)) review. jarolrod: ACK 79a2576, tested on macOS 11.1 with Qt 5.15.2 laanwj: Code review ACK 79a2576 Tree-SHA512: 4a8d8f8bfbaefd68e8d1bf3b20d29e4a8e8cfe97b2f8d59d3a4c338a50b61de0a67d97bd8646c04bd5df5a9679c4954b9b46e7cba24bb89f4d0e44e94cf9d66c
- comma must follow "additionally" and "however" as they are introductory words or phrases - same for "last but not least", comma missing - "nothing else than", not perfect English; better "nothing but" - "get most use", not perfect English; better "get more benefits"
This pull request aims at adding autotools for build checks, configure
flags for compile time configuration and handling of #define
directives inside code and things that will possibily make it better
for bitcoin to be packaged inside distributions, as well ported to
different architectures.
Build of this branch was tested on Debian 6 (also with WX GUI), Apple
OSX 10.5 (no WX GUI) and CYGWIN win32 (no UPNP nor WX)
completed
add autotools build system
commit ref: jaromil/bitcoin@500da9e
this pull request follows as third attempt to previous 2:
and basically consists of a rebase of this branch
https://github.com/jaromil/bitcoin/commits/master eliminating all
those commits and squashing them in 2 steps.
1st commit moves files around under old build system
2nd commit converts the build system to autotools
(while we still leave the old build system in place, optional)
the bitcoind code itself was never modified: files were moved around
and the header auto-config.h generated by autoconf was added to
headers.h
code modules have been separated in subdirectories and compiled as
static libraries, still using libtool, which is the recommended
behaviour when using autotools.
interesting configure flags
templates for adding more compile time choices in future.
included WX GUI. other wx related flags are provided to indicate
prefix or static library build.
all warnings - anyone willing to help should run it at least once
:)
dump gmon.out files after run)
(optional)
code re-organization
commit ref: jaromil/bitcoin@84c3fb0
directory re-organization (keeps the old build system)
there is no internal modification of any file in this commit
files are moved into directories according to established
standards in sourcecode distribution; these directories contain:
binaries
of the implementation, and contributor checklists.
strings used in the program
party implementing scripts and auxiliary programs