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

Build: protobuf fixes for docker, cross-compilation, monero-gui #4945

Merged
merged 1 commit into from
Dec 31, 2018

Conversation

ph4r05
Copy link
Contributor

@ph4r05 ph4r05 commented Dec 5, 2018

Fixes /contrib/depends for protobuf so it works also in cross-compilation under docker.

Two packages are needed, native_protobuf and protobuf. native_protobuf builds protoc under host so it can use it to generate trezor messages. Native protoc is also used during build of the target host protobuf dependency.

Adds dependencies for GUI compilation. Changes to the monero-wallet-gui.pro file:

# REMOVE -lboost_thread from the LIBS list
TREZOR_LINKER = $$cat($$WALLET_ROOT/lib/trezor_link_flags.txt)
LIBS += $$TREZOR_LINKER -lboost_thread

@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 5, 2018

Waiting how the tests end up...

@TheCharlatan could you pls test this one then? I recall you had some issues with protobuf deps so I am wondering if this can fix it. Thanks!

@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 5, 2018

... still testing and fixing

@ph4r05 ph4r05 force-pushed the trezor/protobuf-deps branch 2 times, most recently from f819d65 to a69f258 Compare December 5, 2018 18:31
@ph4r05 ph4r05 force-pushed the trezor/protobuf-deps branch from a69f258 to 7beef91 Compare December 5, 2018 21:10
@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 5, 2018

@TheCharlatan I was wondering, can we make libusb compile also with mingw? It could work with --disable-udev.

@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 5, 2018

Hmm same error here as described in protocolbuffers/protobuf#5358

@TheCharlatan
Copy link
Contributor

TheCharlatan commented Dec 5, 2018

You can add plattform specific rules like this:

define $(package)_set_vars
  $(package)_config_opts_mingw32=--disable-udev
endef

@ph4r05 ph4r05 force-pushed the trezor/protobuf-deps branch from aaf8264 to 9e881e5 Compare December 6, 2018 06:48
@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 6, 2018

@TheCharlatan fix to the .travis.yml (posix alternative for gcc) seem to fix mingw problem. protocolbuffers/protobuf#5358

@TheCharlatan
Copy link
Contributor

Great! Can you comment it on the issue, I'll close it then. Can you also add a line in the readme about selecting the posix alternative?

@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 6, 2018

I will adopt the PR later today/tomorrow but it seems I have fully working compilation for all platforms with protobuf and libusb (still struggling with osx+libusb a bit but I will fix it soon)

@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 6, 2018

The key point is probably to set both gcc and g++ to posix versions and to regenerate docker dependencies & clean rebuild.

@ph4r05 ph4r05 mentioned this pull request Dec 6, 2018
31 tasks
@ph4r05 ph4r05 force-pushed the trezor/protobuf-deps branch from 9e881e5 to 3ea03a0 Compare December 7, 2018 00:46
@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 7, 2018

OK The build works, but Travis failed as it didn't make it under 50 minutes time limit for Win64.

I am done on this from my side.

@ph4r05 ph4r05 force-pushed the trezor/protobuf-deps branch from 3ea03a0 to 7c410fb Compare December 7, 2018 12:29
@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 7, 2018

@TheCharlatan thanks for the approval! I had to do one small fix in the cmake now 😞

@ph4r05 ph4r05 force-pushed the trezor/protobuf-deps branch from 7c410fb to fede76f Compare December 12, 2018 10:07
@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 12, 2018

@TheCharlatan I've rebased on master. Could I ask you for the review? :)

@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 12, 2018

@danrmiller could you pls check buildbot/monero-static-win32? It complains about some linking error on cmake.

C:/msys32/mingw32/bin/cmake.exe: error while loading shared libraries: ?: cannot open shared object file: No such file or directory
make: *** [Makefile:151: release-static-win32] Error 127

@ph4r05 ph4r05 force-pushed the trezor/protobuf-deps branch 3 times, most recently from 1f4e654 to 25e0553 Compare December 16, 2018 21:56
@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 16, 2018

OK another cmake improvements. Now I've added stuff for GUI compilation.

@dEBRUYNE-1
Copy link
Contributor

dEBRUYNE-1 commented Dec 17, 2018

@ph4r05 - For the GUI compilation, have you also looked at necessary changes in the GUI repository? Specifically, the monero-wallet-gui.pro file.

@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 17, 2018

@dEBRUYNE-1 it's only required to add:

TREZOR_LINKER = $$cat($$WALLET_ROOT/lib/trezor_link_flags.txt)
LIBS += $$TREZOR_LINKER

This is the simplest way I could think of. Trezor has variable dependencies, e.g. if there is a compatible libusb library present, it uses it. The file trezor_link_flags.txt contains linker flags which should be used for trezor deps. Cmake copies all dependencies to $$WALLET_ROOT/lib so the linking works correctly.

I am also having issues with -lboost_thread-mt which yields the following linking error:

Undefined symbols for architecture x86_64:
  "boost::this_thread::hidden::sleep_until(timespec const&)", referenced from:
      boost::this_thread::sleep(boost::posix_time::ptime const&) in libwallet_merged.a(miner.cpp.o)
  "boost::this_thread::hidden::sleep_for(timespec const&)", referenced from:
      boost::this_thread::sleep_for(boost::chrono::duration<long long, boost::ratio<1l, 1000000000l> > const&) in libwallet_merged.a(wallet_manager.cpp.o)
      boost::this_thread::sleep_for(boost::chrono::duration<long long, boost::ratio<1l, 1000000000l> > const&) in libepee.a(mlocker.cpp.o)
      boost::this_thread::sleep_for(boost::chrono::duration<long long, boost::ratio<1l, 1000000000l> > const&) in libwallet_merged.a(wallet2.cpp.o)
      boost::this_thread::sleep_for(boost::chrono::duration<long long, boost::ratio<1l, 1000000000l> > const&) in libwallet_merged.a(dns_utils.cpp.o)
      boost::this_thread::sleep_for(boost::chrono::duration<long long, boost::ratio<1l, 1000000000l> > const&) in libwallet_merged.a(node_rpc_proxy.cpp.o)
      boost::this_thread::sleep_for(boost::chrono::duration<long long, boost::ratio<1l, 1000000000l> > const&) in libwallet_merged.a(miner.cpp.o)
      boost::this_thread::sleep_for(boost::chrono::duration<long long, boost::ratio<1l, 1000000000l> > const&) in libwallet_merged.a(transport.cpp.o)
      ...
ld: symbol(s) not found for architecture x86_64

I had to switch it to -lboost_thread.

To get the GUI working with the Trezor I need a few more modifications (key image sync), which I will add as a separate PR.

@dEBRUYNE-1
Copy link
Contributor

Thanks for the clarification. I am, alas, not sufficiently knowledgeable to assist / help you with your building issues.

@danrmiller
Copy link
Contributor

@danrmiller could you pls check buildbot/monero-static-win32? It complains about some linking error on cmake.

C:/msys32/mingw32/bin/cmake.exe: error while loading shared libraries: ?: cannot open shared object file: No such file or directory
make: *** [Makefile:151: release-static-win32] Error 127

@ph4r05 Thanks, it builds now, just looks like I still need to look at something with a temp file in use during the tests.
https://build.getmonero.org/builders/monero-static-win32/builds/5820

@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 17, 2018

@dEBRUYNE-1 thanks. I have it building OK now. Now I just need to connect Trezor features to the GUI :)

@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 17, 2018

@danrmiller thanks for fixing the build. This test failure is quite common. It is present also on other architectures and I think quite old.

@ph4r05
Copy link
Contributor Author

ph4r05 commented Dec 17, 2018

I've created corresponding PR in monero-gui: monero-project/monero-gui#1838

@ph4r05 ph4r05 force-pushed the trezor/protobuf-deps branch from 25e0553 to f7715a8 Compare December 18, 2018 14:57
- docker protobuf dependencies, cross-compilation
- device/trezor protobuf build fixes, try_compile
- libusb built under all platforms, used by trezor for direct connect
@ph4r05 ph4r05 force-pushed the trezor/protobuf-deps branch from f7715a8 to e37154a Compare December 18, 2018 15:50
@ph4r05 ph4r05 changed the title Build: protobuf fixes for docker, cross-compilation Build: protobuf fixes for docker, cross-compilation, monero-gui Dec 19, 2018
@luigi1111 luigi1111 merged commit e37154a into monero-project:master Dec 31, 2018
luigi1111 added a commit that referenced this pull request Dec 31, 2018
e37154a build: protobuf dependency fixes, libusb build (ph4r05)
@ph4r05 ph4r05 deleted the trezor/protobuf-deps branch April 3, 2019 19:24
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.

5 participants