Autotools RFC: Round 2

theuni commented at 5:18 am on July 1, 2013: member

Changes since Round 1:

Rebased on current master
Moved configure to root srcdir, and added a Makefile there
Split into several Makefile.am’s with recursive build [1]
Work towards building correctly under gitian
Fixed/verified several build scenarios
Fixed up ‘make dist’ to work as a viable end-result packaging mechanism [2]

Personally I believe that splitting up the build is a major regression from RFC1. This was done by request here. Most projects end up spending lots of time moving in the opposite direction, as a recursive build tends to be slower, more complicated, and more-error prone. That proves to be the case here as well, but I’ve attempted to handle it as gracefully as possible.
‘make dist’ can be used to produce a bootstrapped tarball with git information included, intended for distribution as an ‘official source release’. This is the equivalent of taking a fresh git clone, running ‘./autogen.sh’, and tar’ing up the result. This is the common procedure for autotools projects.

At this point I consider it to be merge-ready, with the exception of verification of gitian descriptors and documentation. Presumably removal of the old makefile.* would coincide with updated documentation. Several things can be improved and cleaned up after merge (dealing with versioning, for example), but I’ve tried to ignore these things as much as possible for the sake of a quick merge.

Edit: Removed crazy markup that looked like yelling – sorry.

Here is the second 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:

0./autogen.sh
1./configure
2make

Same for OS X, but the pkg-config path needs to be hooked up from macports first:

0echo "/opt/local/share/aclocal" | sudo tee -a /usr/share/aclocal/dirlist

For mingw it’s the same, but you will need to provide lots of paths in the form of:

0./autogen.sh
1CPPFLAGS="-I/path/to/include -I/path/to/other/include" LDFLAGS="-L/path/to/libs -L/path/to/other/libs" ./configure
2make

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.

Gitian should be working for win32 builds. I’ve guessed at Linux but have not yet verified.

‘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:

Verify gitian descriptors.
Add documentation

autotools: cleanup usage of MSG_NOSIGNAL

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.

0ceb047191

autotools: include bitcoin-config.h for autotools builds 10fb176687

autotools: add srcdir param for genbuild.sh

This allows us to search for .git, so that we can make a guess as to whether
we're in a repository or just an extracted tarball.

TODO: might want to check for GIT_DIR env var as well.

070ad14d4d

autotools: add autotools files a877f183e3

autotools: update gitian descriptors ec44b55b4b

theuni referenced this in commit 83e0a83aeb on Jul 2, 2013

leveldb: quote CC and CXX so that they can contain whitespace

For example, CC="ccache gcc" or CXX="ccache g++"

0196375871

theuni commented at 6:36 pm on July 5, 2013: member

@jgarzik @laanwj @sipa ping. Any comments?

squash me. autotools: quote vars when passing them to leveldb make b627f2821c

squash me. autotools: add ccache support

Speeds builds up a substantial amount, as expected. Local tests show full clean
builds are 25x-40x faster.

f184fbad95

squash me. autotools: only test for boost_test if tests are enabled 323ed237d1

squash me. gitian linux build needs libboost-test a36135b92c

sipa commented at 12:48 pm on July 7, 2013: member

Changes look good to me, except that I do not want to maintain two build systems in parallel. If we switch to autotools, we switch, and the existing makefiles go away. Gitian determinism can be fixed later (though just builds should be verified to work). Unfortunately; I can’t test myself now (I’m not at home).

About -Qt, you’ll want an ACK from @laanwj

I have no opinion about recursive makefiles or not.

in configure.ac: in a36135b92c outdated

29+AM_MAINTAINER_MODE([enable])
30+
31+dnl make the compilation flags quiet unless V=1 is used
32+m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
33+
34+AC_ARG_ENABLE([upnp],

luke-jr commented at 5:26 am on July 17, 2013:

AC_ARG_WITH is for dependencies.

theuni commented at 5:32 am on July 17, 2013:

Will change.

in configure.ac: in a36135b92c outdated

35+  [AS_HELP_STRING([--enable-upnp],
36+  [enable UPNP (default is yes if libminiupnpc is found)])],
37+  [use_upnp=$enableval],
38+  [use_upnp=auto])
39+
40+AC_ARG_ENABLE([upnp-default],

luke-jr commented at 5:26 am on July 17, 2013:

AC_ARG_ENABLE([upnp], …) makes sense here (after the dependency is moved to AC_ARG_WITH)

in configure.ac: in a36135b92c outdated

14+dnl reset CXXFLAGS if empty (autoconf adds its own defaults otherwise)
15+:${CFLAGS=""}
16+:${CXXFLAGS=""}
17+
18+dnl faketime breaks configure and is only needed for make. Disable it here.
19+unset FAKETIME

luke-jr commented at 5:27 am on July 17, 2013:

Wouldn’t it be better to just edit the gitian yml to not set it until after configure?

theuni commented at 5:31 am on July 17, 2013:

No, that doesn’t work. Makefile attempts to re-run configure because it think’s it’s been touched post-configure.

Besides, I would like to be able to create deterministic builds without the use of gitian. For now they’re only deterministic relative to the builder, but I hope to improve that soon.

luke-jr commented at 5:51 am on July 17, 2013:

Did you use –disable-maintainer-mode in gitian’s configure? That should solve it.

theuni commented at 6:00 am on July 17, 2013:

I verified that it does not. Gitian does use –disable-maintainer-mode anyway though, for a separate reason that I can’t recall atm.

in configure.ac: in a36135b92c outdated

23+dnl ==============================================================
24+
25+AM_INIT_AUTOMAKE([no-define subdir-objects foreign])
26+
27+dnl faketime messes with timestamps and causes configure to be re-run.
28+dnl maintainer-mode can be used to bypass this.

luke-jr commented at 5:31 am on July 17, 2013:

Rather, you mean –disable-maintainer-mode can…

theuni commented at 5:33 am on July 17, 2013:

Correct, will fix.

in configure.ac: in a36135b92c outdated

49+  [enable ipv6 (default is yes)])],
50+  [use_ipv6=$enableval],
51+  [use_ipv6=yes])
52+
53+dnl enable qt support
54+AC_ARG_ENABLE([qt],

luke-jr commented at 5:32 am on July 17, 2013:

AC_ARG_WITH([qt],…)

in configure.ac: in a36135b92c outdated

56+  [enable qt (default is yes)])],
57+  [use_qt=$enableval],
58+  [use_qt=yes])
59+
60+dnl compile with full warnings and debugging symbols
61+AC_ARG_ENABLE(debug,

luke-jr commented at 5:32 am on July 17, 2013:

This is usually a property of CXXFLAGS passed to configure.

theuni commented at 5:35 am on July 17, 2013:

This was added to mimic the debug flags in the current makefile, in an attempt to avoid complaints about missing features in the new buildsystem. I agree that it should not be here long-term.

in configure.ac: in a36135b92c outdated

67+AC_ARG_ENABLE(tests,
68+    AS_HELP_STRING([--enable-tests],[compile tests (default is yes)]),
69+    [use_tests=$enableval],
70+    [use_tests=yes])
71+
72+AC_ARG_ENABLE([qr],

luke-jr commented at 5:33 am on July 17, 2013:

AC_ARG_WITH([qrencode],…)

in configure.ac: in a36135b92c outdated

74+  [enable QR code support (default is yes if qt is enabled and libqrencode is found)])],
75+  [use_qr=$enableval],
76+  [use_qr=auto])
77+
78+AC_ARG_ENABLE([dbus],
79+  [AS_HELP_STRING([--enable-dbus],

luke-jr commented at 5:33 am on July 17, 2013:

AC_ARG_WITH([qtdbus],…)

in configure.ac: in a36135b92c outdated

79+  [AS_HELP_STRING([--enable-dbus],
80+  [enable DBus support (default is yes if qt is enabled and QtDBus is found)])],
81+  [use_dbus=$enableval],
82+  [use_dbus=auto])
83+
84+AC_ARG_ENABLE([hardening],

luke-jr commented at 5:34 am on July 17, 2013:

Also usually a property of CXXFLAGS

theuni commented at 5:36 am on July 17, 2013:

See above

in configure.ac: in a36135b92c outdated

85+  [AS_HELP_STRING([--enable-hardening],
86+  [attempt to harden the resulting executables (default is yes)])],
87+  [use_hardening=$enableval],
88+  [use_hardening=yes])
89+
90+AC_ARG_ENABLE([ccache],

luke-jr commented at 5:34 am on July 17, 2013:

This shouldn’t be necessary… ccache is transparent

theuni commented at 5:37 am on July 17, 2013:

Only if CXX is specified. Many other projects use this trick, it was added here for convenience. I can remove if necessary.

in configure.ac: in a36135b92c outdated

118+AC_PATH_PROG(CCACHE,ccache,none)
119+
120+## TODO: Remove these hard-coded paths and flags. They are here for the sake of
121+##       compatibility with the legacy buildsystem.
122+##
123+CXXFLAGS="$CXXFLAGS -Wall -Wextra -Wformat -Wformat-security -Wno-unused-parameter -frandom-seed=bitcoin"

luke-jr commented at 5:39 am on July 17, 2013:

-frandom-seed should be different for every file compiled!

theuni commented at 5:42 am on July 17, 2013:

This was pulled from the old makefiles and gitian descriptors. It’s required for deterministic builds.

luke-jr commented at 5:53 am on July 17, 2013:

I’m pretty sure it’s only used by gitian right now. Just because gitian does something (in a known/controlled environment) does not mean we want every build doing it… Gitian can still pass CXXFLAGS to configure.

theuni commented at 5:58 am on July 17, 2013:

I’m not sure that I see the harm, but I’ll back down on this until I have something to show.

in configure.ac: in a36135b92c outdated

238+dnl Check for MSG_NOSIGNAL
239+AC_MSG_CHECKING(for MSG_NOSIGNAL)
240+AC_TRY_COMPILE([#include <sys/socket.h>],
241+ [ int f = MSG_NOSIGNAL; ],
242+ [ AC_MSG_RESULT(yes); AC_DEFINE(HAVE_MSG_NOSIGNAL, 1,[Define this symbol if you have MSG_NOSIGNAL]) ],
243+ [ AC_MSG_RESULT(no)]

luke-jr commented at 5:41 am on July 17, 2013:

Wouldn’t it be better to just define MSG_NOSIGNAL here and get rid of the ifdefs all over that just define it?

theuni commented at 5:44 am on July 17, 2013:

There’s nothing to define it to.. the test is to see if the host toolchain defines it. If not, a definition must be spoofed as is currently done.

luke-jr commented at 5:53 am on July 17, 2013:

AC_DEFINE([MSG_NOSIGNAL],[0],[Defined if MSG_NOSIGNAL is missing from system headers])

theuni commented at 6:03 am on July 17, 2013:

That defies all convention. The correct thing would be to have a wrapper.h that includes such headers as necessary, and attempts to define them if missing. Then only one define is needed.

Sorry, but a big -1 from me on that one.

Diapolo commented at 8:45 pm on July 24, 2013:

Just a quick question, what about MSG_DONTWAIT? Why not add such a thing for this also?

in configure.ac: in a36135b92c outdated

62+    AS_HELP_STRING([--enable-debug],[compile with debug symbols (default is no)]),
63+    [use_debug=$enableval],
64+    [use_debug=no])
65+    AC_MSG_CHECKING([if compiling with debugging symbols])
66+
67+AC_ARG_ENABLE(tests,

luke-jr commented at 5:43 am on July 17, 2013:

qt and tests should probably be replaced by a –with-frontends=bitcoind,bitcoin-qt,test_bitcoin so it’s cleanly extensible Even if we only support building one at a time right now, doing this now will make things nicer in the future when all can be built together.

theuni commented at 5:45 am on July 17, 2013:

For qt/bitcoind, I completely agree. Disagree on tests though. –disable-tests is very common, and one of the first things likely to drive a distro package maintainer crazy if missing.

luke-jr commented at 5:54 am on July 17, 2013:

Sounds reasonable to keep tests separate. I’d forgotten the tests vary based on targets anyhow.

in configure.ac: in a36135b92c outdated

265+
266+BOOST_LIBS="$BOOST_LDFLAGS $BOOST_SYSTEM_LIB $BOOST_FILESYSTEM_LIB $BOOST_PROGRAM_OPTIONS_LIB $BOOST_THREAD_LIB"
267+BOOST_INCLUDES="$BOOST_CPPFLAGS"
268+
269+if test x$TARGET_OS = xwindows; then
270+  BOOST_LIBS="$BOOST_LIBS $BOOST_CHRONO_LIB"

luke-jr commented at 5:44 am on July 17, 2013:

If boost chrono is available, it is probably needed. This isn’t an OS-specific thing - newer boost require it on OSX and Linux too. And in cases it isn’t, we could use –as-needed…

theuni commented at 5:46 am on July 17, 2013:

Iirc for the first autotools attempt it was windows only. I suppose that’s changed now. Will fix.

luke-jr commented at 5:55 am on July 17, 2013:

It’s a disaster now: we’re using it only for Mac and Windows, but that’s because we lack any way to detect if it’s really needed. Automake fixes this :)

in configure.ac: in a36135b92c outdated

311+      LIBS="$QT_LIBS"
312+    fi
313+    AC_CHECK_LIB([qcncodecs],      [main],, AC_MSG_ERROR(libqcncodecs not found. Install it or use --disable-qt.))
314+    AC_CHECK_LIB([qjpcodecs],      [main],, AC_MSG_ERROR(libqjpcodecs not found. Install it or use --disable-qt.))
315+    AC_CHECK_LIB([qkrcodecs],      [main],, AC_MSG_ERROR(libqkrcodecs not found. Install it or use --disable-qt.))
316+    AC_CHECK_LIB([qtwcodecs],      [main],, AC_MSG_ERROR(libqtwcodecs not found. Install it or use --disable-qt.))

luke-jr commented at 5:46 am on July 17, 2013:

Huh? Why do we need Chinese, Japanese, Korean, and Taiwanese codecs to compile Bitcoin-Qt???

theuni commented at 5:49 am on July 17, 2013:

See: bitcoin-qt.pro: QTPLUGIN += qcncodecs qjpcodecs qtwcodecs qkrcodecs qtaccessiblewidgets and: src/qt/bitcoin.cpp:Q_IMPORT_PLUGIN(qtwcodecs)

Win32 links plugins in static.

Rather than changing behavior, I opted to take the (hopefully) less controversial route in order to get the buildsystem merged in more easily.

in configure.ac: in a36135b92c outdated

474+  BUILD_QT=qt
475+fi
476+
477+AC_MSG_RESULT($use_qt)
478+
479+dnl these are only used when qt is enabled

luke-jr commented at 5:47 am on July 17, 2013:

Probably shouldn’t even check for them if Bitcoin-Qt isn’t being built?

theuni commented at 5:52 am on July 17, 2013:

That is the case already.

Err.. looks like the qr lib will be checked anyway, will fix that one.

luke-jr commented at 5:50 am on July 17, 2013: member

I don’t see any code to look for bdb headers, much less figure out whether they’re in the standard /usr/include/db4.8 path or somewhere else common…

theuni commented at 5:55 am on July 17, 2013: member

Hmm, you’re correct on bdb headers, I’ll add the check. As for uncommon paths, it’ll be up to the user to add it to cppflags.

autotools: detect proper boost sleep in configure

Additionally, link in boost_crypto if necessary.

1b0732d6b5

autotools: change ARG_ENABLE to ARG_WITH for dependencies f48b27b0c6

autotools: fix broken output when qt is disabled e66711c1fe

autotools: don't check for qrencode if qt is disabled fc8c8a5787

autotools: check for bdb headers and cosmetics d48ed1b40f

autotools: remove frandom-seed. Let gitian set it. b7c68c0351

autotools: move flags to rhs of configure fa98a16020

autotools: fixup comment 834ee1bb62

in contrib/gitian-descriptors/gitian-win32.yml: in a36135b92c outdated

48-  export LD_PRELOAD=/usr/lib/faketime/libfaketime.so.1
49-  export FAKETIME=$REFERENCE_DATETIME
50-  export TZ=UTC
51-  make -f makefile.linux-mingw $MAKEOPTS DEPSDIR=$HOME/build bitcoind.exe USE_UPNP=0 DEBUGFLAGS="-frandom-seed=bitcoin"
52+  ./autogen.sh
53+  CPPFLAGS="-I$STAGING/include" LDFLAGS="-L$STAGING/lib" ./configure --prefix=$STAGING --host=i586-mingw32msvc --with-qt-bindir=$STAGING/bin --with-qt-plugindir=$STAGING/plugins  --with-qt-incdir=$STAGING/include --with-boost=$STAGING --disable-maintainer-mode --disable-dependency-tracking

luke-jr commented at 6:02 am on July 17, 2013:

CPPFLAGS and LDFLAGS should usually be on the right side of ./configure

theuni commented at 6:05 am on July 17, 2013:

sure.

theuni commented at 4:38 am on July 18, 2013: member

I’ve pushed the obvious fixes to your comments above. Thanks for the review. These will of course be squashed when ready.

I didn’t add the –with-frontends because with only 2 (tests would be separate), I think that is quite confusing. I hope you’ll agree and that can be addressed if/when a new frontend is added.

Waiting on replies to my comments on the rest.

luke-jr commented at 4:55 am on July 18, 2013: member

Addressed later means builders need to change their configure lines based on the version they are building, even if both versions support the same features. Better to have something extensible from the start IMO.

theuni commented at 5:03 am on July 18, 2013: member

Fair enough. But with 2 frontend’s, and 1 required, there’s no way to avoid an awkward syntax.

Compare:

Disable qt: –with-frontends=bitcoind

Some future (non-auto-enabled) thing: –with-frontends=bitcoind,bitcoin-qt,bitcoin-foo

vs.

Disable qt: –without-qt

Some future (non-auto-enabled) thing: –with-bitcoin-foo

Imo, with-frontends is not extensible, it’s just a complicated syntax for saying enable/disable that forces far more parsing.

luke-jr commented at 5:20 am on July 18, 2013: member

Disable qt: –without-frontends=bitcoin-qt

Some future (non-auto-enabled) thing: –with-frontends=bitcoin-qt

theuni commented at 5:32 am on July 18, 2013: member

–without-frontends sets the withval to ’no’. afaik –without-frontends=foo has no meaning, or at best would not be portable. Is there some magic I’m not aware of for determining that a withval was also negated?

Regardless of that, even if it is valid somehow, I’ve never seen that syntax, and a quick google search shows no results either. So I can’t imagine any user stumbling upon it…

luke-jr commented at 5:37 am on July 18, 2013: member

Hmm, I’ve seen this kind of thing before with KVM/qemu. They do “all targets” by default (and –target-list overrides entirely), nor use autoconf.

How about just building bitcoind by default and just –with-frontends=bitcoin-qt?

theuni commented at 5:45 am on July 18, 2013: member

So then you still have to add it for each one you want to build, but you’ve disabled the default for what (arguably) most builders will want. I still don’t see how it’s a step up in any way from –with-qt (or –with-bitcoin-qt) or –without-qt.

If we’re talking >5 or so front-ends in the future, sure. But presumably at that point they’d have to be split into separate projects by then anyway.

This whole discussion smells of an over-engineered bikeshed to me ;)

luke-jr commented at 6:09 am on July 18, 2013: member

–with-qt implies it’s the same client being built with or without Qt, which isn’t exactly the case here. You may have a point with regard to prioritizing splitting up the repositories before too many implementations grow on it.

theuni commented at 6:11 am on July 18, 2013: member

Compromise at –with-qt-frontend ?

theuni commented at 6:17 am on July 18, 2013: member

or even –enable-qt-frontend, since qt-frontend would be a feature in this case rather than a library for inclusion.

luke-jr commented at 6:19 am on July 18, 2013: member

Would it be hard to leave it out entirely so the builder just does: make bitcoin-qt make bitcoind make all ?

theuni commented at 6:25 am on July 18, 2013: member

Sure, those already work.

They’re awkward with the recursive makefiles, which is one of the reasons I was against. You have to be in the correct dir for them to work:

make (makes all) cd src; make bitcoind cd src/qt; make bitcoin-qt

But some phony targets would be no-brainers to add, then they’d work anywhere.

However you still need to be able to check for things. If you didn’t, you could try to make bitcoind, only to find qrcode missing, and we wouldn’t know if that was on purpose or an error. Then you’ve just negated the purpose of the buildsystem :)

So the options and checks in configure need to stay. As implemented, they’re what any package maintainer would expect to see.

luke-jr commented at 6:28 am on July 18, 2013: member

If qrencode is missing, it should be a warning (and disable itself), not an error, unless –with-qrencode is specified ;)

theuni commented at 6:38 am on July 18, 2013: member

It is, but qrencode is never checked unless qt is enabled. Nor should it be.

This discussion has gone way off track, and this is the kind of thing I was afraid of. This thing will never be merged if the points of contention are cosmetic. I’m happy to change those to whatever makes you guys happy and argue them post-merge.

luke-jr commented at 7:11 am on July 18, 2013: member

I don’t think anyone considers this an obstacle to merging. I believe that is just waiting on @laanwj at this point.

laanwj commented at 6:34 pm on July 22, 2013: member

I preferred a non-recursive makefile as well, but I don’t want to bikeshed about this.

ACK after squash

jgarzik commented at 4:40 pm on July 24, 2013: contributor

Looks really, really good.

The only issue I found during review: bitcoin-config.h should normally be the -very- first include, because it may need to change the behavior of certain includes that follow. This may or may not be needed with bitcoin, but it is general practice when coding autotools code.

Also, make sure bitcoin-config.h includes the standard header #ifndef FOO_H … guard at the top and bottom. Google claims AH_TOP and AH_BOTTOM may be used for this.

ACK once these issues are resolved.

Pull tester want any changes for this?

autotool: add multiple-include guard for bitcoin-config.h 2b9711199b

in src/qt/Makefile.am: in 834ee1bb62 outdated

 8+noinst_LIBRARIES = libbitcoinqt.a
 9+SUBDIRS = $(BUILD_TEST_QT)
10+DIST_SUBDIRS = test
11+
12+# bitcoin qt core #
13+QT_TS = locale/bitcoin_af_ZA.ts locale/bitcoin_ar.ts locale/bitcoin_bg.ts \

Diapolo commented at 8:48 pm on July 24, 2013:

How are these updated in the future (adding new ones to Bitcoin-Qt happens sometimes, when I create a Transifex sync pull)?

theuni commented at 9:45 pm on July 24, 2013:

Just add them to QT_TS and QT_QM here. Is that what you meant?

Diapolo commented at 5:47 am on July 25, 2013:

I mean currently the files need to be listed in src/qt/bitcoin.qrc to be usable by Bitcoin-Qt. Does this whole pull move completely away from our bitcoin-qt.pro file and qmake? If that is a dumb question, sorry I’m that Windows guy and makefiles are magic for me ^^.

theuni commented at 10:24 am on July 25, 2013:

No problem.

Yes, this does away with the need for qmake, instead we run the same tools that qmake uses, but we run them ourselves instead of describing the process in a .pro file.

theuni commented at 9:54 pm on July 24, 2013: member

@jgarzik good suggestion on bitcoin-configh.h, thanks. Done. As for the header order, I’ve changed it as requested, though I would disagree and say that if (in-project) include order matters, something else is broken somewhere. But that’s way out of scope here :)

autotools: always include bitcoin-config.h first add8f8b0a9

theuni commented at 10:00 pm on July 24, 2013: member

@jgarzik as for BitcoinPullTester, I’m not sure. It seems to have a hard-coded build process. If that’s the case, it should change to:

0./autogen.sh && ./configure && make check

autotools: fix bad copy/paste bc7e977b88

gavinandresen commented at 5:22 am on July 25, 2013: contributor

RE: the pull tester:

The pull tester build/test script lives at: https://github.com/TheBlueMatt/test-scripts/blob/master/build-script.sh

A patch that checks for configure.ac (or whatever) and Does the Right Thing in the right places would be nifty.

autotools: add windows-deploy target

This uses makensis to create the win32 installer exe.

Usage: make windows-deploy

7784ea1919

autotools: remove timestamps from qt generated files bad255871b

BitcoinPullTester commented at 0:36 am on July 26, 2013: none

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

This could happen for one of several reasons:

It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in contrib/test-scripts)
It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
It does not build on either Linux i386 or Win32 (via MinGW cross compile)
The test suite fails on either Linux i386 or Win32
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.

This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

wtogami commented at 5:59 am on August 18, 2013: contributor

What needs to be done to move this forward?

sipa commented at 2:05 pm on August 18, 2013: member

I believe all that is left is rebasing, squashing, removing the old makefiles, and updating some documentation?

Fixing determinism can be done after merge, IMHO, but the build should work on all supported environments. Fixing pulltester can only be done after merging as well.

theuni commented at 8:41 pm on August 19, 2013: member

I have a bunch of work in a local branch that needs to be cleaned up and pushed here.

I’ll have this merge-ready without fail by this time next week (my schedule is back to normal then).

theuni commented at 3:11 am on August 27, 2013: member

Closing in favor of a new (final) PR.

theuni closed this on Aug 27, 2013

theuni commented at 3:27 am on August 27, 2013: member

Edit: Whoops, fixed link. Continued at #2943.

Bushstar referenced this in commit 81eeff1c54 on Apr 5, 2019

DrahtBot locked this on Sep 8, 2021

Autotools RFC: Round 2 #2805