Skip to content
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

Switch build system to autotools #487

Merged
merged 99 commits into from
Nov 16, 2017

Conversation

TheCharlatan
Copy link
Contributor

@TheCharlatan TheCharlatan commented Aug 6, 2017

This pull request will switch the current build system to autotools. Gridcoin core functions are now built into a library that is shared between the daemon and gui client. It compiles by default with Qt5 and adds some necessary features, that are needed for deterministic gitian builds. The depends system needed for gitian is not active yet, but will be completed with a future pull request.
To build run:
./autogen.sh , which runs autoreconf to generate the configure script
./configure , with any desired options to generate the makefile. To compile with Qt5 for your current system architecture use ./configure --with-gui=qt5
make , V=1 will enter more verbose mode, use -j # of cores for multithreading

With standard configuration, the system will build both the daemon and the gui client. The daemon is placed in src/gridcoinresearchd and the gui in src/qt/gridcoinresearch.

During compilation a library of core gridcoin functionality called libgridcoin_util is built, that is later linked with the daemon main entry point and with the rest of the qt files for the gui client. The main function of the deamon has been moved into a seperate file called bitcoind.cpp .

Please do not merge the pull request before @Lederstrumpf has ack'ed.

Edit: The current state of the pull request can be tracked here:
https://trello.com/b/91uwJaI3/autotools-milestone

@tomasbrod
Copy link
Member

tomasbrod commented Aug 6, 2017

Woah.
I know it is not done yet. Can you explain why you had to add so many files?
Sixty six thousand lines juts to support building with autotools. This is clearly unacceptable.
Besides this is Gridcoin, not bitcoin.

On the bright side, I like the idea with shared library.

Copy link
Member

@tomasbrod tomasbrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clean it dang, how do I delete this :(

@denravonska
Copy link
Member

A lot of new files (doc, contrib etc) from Bitcoin it seems.

@gridcoin
Copy link
Contributor

gridcoin commented Aug 6, 2017

If this were merged in, would we still be able to do a native build on windows with the pro file, or would that break?

@denravonska
Copy link
Member

@gridcoin We would need a new procedure but I'll try to fix that. Hopefully this removes the need for a dev vdisk.

#include <QtGlobal>
// Automatically generated by extract_strings.py

// Automatically generated by extract_strings_qt.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to regenerate this?

#if QT_VERSION < 0x050000
#include <QtConcurrentRun>
#else
#include <QtConcurrent/QtConcurrentRun>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really the way to do it? When using qmake it automatically appends the correct include path (although with -isystem for some reason) so you don't have to specify the module prefix:

-isystem /usr/include/x86_64-linux-gnu/qt5/QtConcurrent

This should be the same for both Qt4 and 5 IIRC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The m4 for the qt checks was not properly configured. This is fixed now and the #if block is no longer required.

@TheCharlatan
Copy link
Contributor Author

@tomasbrod I removed a bunch of the files, that were not needed and were just stuck in there, because it was easiest, when porting the build system. I started with some of the renaming, maybe a task that I can do now every evening for an hour over the next few days. I tried it with search and replace, but since bitcoin has some clearer term distinctions, I ended up with duplicate definitions.

Currently the added files are:

  • m4 macros, Makefiles, scripts and config files for autoconf
  • Edited and new source files due to creating a library and enabling support for both qt4 and qt5
  • The depends system as it currently is contained in the bitcoin source code
  • Hook files for gitian
  • Documentation for autotools, gitian and depends

As it currently stands, the .pro file does not work. However quite about every ide has good support for autotools and switching does not seem to be too much of a headache, or at least I had the feeling when doing it on QtCreator.

@@ -324,11 +324,11 @@ int main(int argc, char *argv[])
}
catch (std::exception& e)
{
handleRunawayException(&e);
/* handleRunawayException(&e); */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No handling anymore? :(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were removed by @Lederstrumpf for debugging purposes. Readded them now.

@TheCharlatan TheCharlatan force-pushed the librarize branch 7 times, most recently from fa3f2f5 to b496552 Compare August 10, 2017 22:45
@denravonska
Copy link
Member

contrib/GridcoinGalaza/GridcoinGalaza/Resources/Thumbs.db snuck back in.

@denravonska
Copy link
Member

denravonska commented Aug 11, 2017

I get an error when configuring for a Qt which lacks Qt Charts:

$ ./configure --with-gui=qt5 --with-incompatible-bdb
...
checking for QT5... no
configure: error: Qt dependencies not found

$ cat config.log
...
configure:23799: checking for QT5
configure:23806: $PKG_CONFIG --exists --print-errors "$qt5_modules"
Package Qt5Charts was not found in the pkg-config search path.
Perhaps you should add the directory containing `Qt5Charts.pc'
to the PKG_CONFIG_PATH environment variable
No package 'Qt5Charts' found
configure:23809: $? = 1
configure:23823: $PKG_CONFIG --exists --print-errors "$qt5_modules"

@TheCharlatan TheCharlatan force-pushed the librarize branch 4 times, most recently from 3b7a57a to b545f6e Compare August 11, 2017 21:11
@TheCharlatan
Copy link
Contributor Author

TheCharlatan commented Aug 11, 2017

The Qt5Charts issue is solved now, at least for Linux. I tested the build on openSUSE running Qt 5.4 and 5.9.1, and on Debian Jessie. The m4 check for the Qt Version was not properly implemented.
Edit: Removed the Thumbs.db file again.

@TheCharlatan TheCharlatan force-pushed the librarize branch 4 times, most recently from 7e31eb1 to 90157c8 Compare August 12, 2017 16:27
These include the activeqt function calls in main, which were moved to seperate files wrapped into their own namespaces, the upgrader rpc calls in bitcoinrpc and init code initialize point.
@denravonska
Copy link
Member

Merging this to end the maintenance. Remaining issues:

  • Versioning is incorrect. It says v0.14.99.0-unk.
  • In OSX I get the old icon on testnet and no icon in prod. Might affect dev/ already.

@denravonska denravonska merged commit b086a9c into gridcoin-community:development Nov 16, 2017
@TheCharlatan TheCharlatan deleted the librarize branch March 5, 2018 14:26
@TheCharlatan TheCharlatan restored the librarize branch March 5, 2018 14:26
@TheCharlatan TheCharlatan deleted the librarize branch March 5, 2018 14:26
denravonska added a commit that referenced this pull request May 25, 2018
Fixed
 - Fixes for displaying on high DPI displays, #517 (@skcin).
 - Re-enable unit tests, add unit test to Travis, #769, #808 (@TheCharlatan).
 - Fix empty string in sendalert2 (@tomasbrod).

Added
 - Neural Report RPC command, #1063 (@tomasbrod).
 - GUI wallet redign with new icons and purple native style (@skcin).

Changed
 - Switch to autotools and Depends from Bitcoin, #487 (@TheCharlatan).
 - Clean and update docs for new build system, remove outdated, #828 (@TheCharlatan).
 - Change estimated time to stake calculations to be more accurate, #1084 (@jamescowens).
 - Move logging to tinyformat, #1009 (@TheCharlatan).
 - Improve appcache performance, #734 (@denravonska).
 - Improve block index memory access performance, #679 (@denravonska).
 - NN fixes: clean logging, explain mag single response, move contract to ndata_nresp (@denravonska)
 - Updated translations:
    - Turkish, #771 (@confuest).
    - Chinese, #1012 (@linnaea).
 - RPC refactor: Cleaner locks, better error handling, move execute calls to straght rpc calls, #1024 (@Foggyx420).
 - Change locking primitives from Boost to STL, #1029 (@Foggyx420).

Removed
 - gridcoindiagnostic RPC call (@denravonska).
 - Galaza, #945 (@barton2526).
 - Assertion in SignSignature, #998 (@TheCharlatan).
 - Upgrade menu, #1094 (@jamescowens).
 - Acid test functions, #871 (@tomasbrod).
 - Qt4 support, #801 (@denravonska).
@theMarix
Copy link
Contributor

I know I am a little late to the party. But, there used to be a NO_UPGRADE flag which enabled package maintainers to disable the upgrader as this does not serve any purpose if Gridcoin is installed from a distribution package. Sadly I can't find anything like this in configure --help. Is there still some way to do this or did it get lost in translation?

@TheCharlatan
Copy link
Contributor Author

The upgrader was permanently disabled. Libzip and curl have been removed as dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants