Skip to content

build: Fix capnp package build for Android #25322

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.

Sign up for GitHub

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

Jump to bottom
Merged
fanquake merged 2 commits into from
Oct 10, 2022
Merged

build: Fix capnp package build for Android #25322

fanquake merged 2 commits into from
Oct 10, 2022

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Jun 9, 2022

On master (e3c08eb):

$ make -C depends capnp MULTIPROCESS=1 HOST=aarch64-linux-android ANDROID_SDK=$ANDROID_HOME ANDROID_NDK=$ANDROID_HOME/ndk/23.2.8568313 ANDROID_API_LEVEL=28 ANDROID_TOOLCHAIN_BIN=$ANDROID_HOME/ndk/23.2.8568313/toolchains/llvm/prebuilt/linux-x86_64/bin
...
ld: error: unable to find library -lkj
...

This PR fixes this error, and also improves configuring according to the docs.

hebasto added 2 commits June 9, 2022 15:56
@hebasto
build: Fix capnp package build for Android
a413595
@hebasto
build: Specify native binaries explicitly when building capnp package
8b8edc2 
From `configure --help`:
  --with-external-capnp   use the system capnp binary (or the one specified
                          with $CAPNP) instead of compiling a new one (useful
                          for cross-compiling)
@fanquake fanquake requested a review from ryanofsky June 9, 2022 15:35
ryanofsky
ryanofsky approved these changes Jun 9, 2022
View reviewed changes
Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Code review ACK 8b8edc2. I'd be a little curious to know what causes the error and how --disable-shared fixes it, but these changes all look good

@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 21, 2022 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

@fanquake
Copy link
Member

fanquake commented Aug 1, 2022

I'd be a little curious to know what causes the error and how --disable-shared fixes it, but these changes all look good

+1. Can you explain why this is the best / correct fix?

improves configuring according to the docs.

the capnp docs?

@fanquake
Copy link
Member

fanquake commented Oct 5, 2022

@hebasto can you follow up here?

@hebasto
Copy link
Member Author

hebasto commented Oct 9, 2022

I'd be a little curious to know what causes the error...

The build fails during the capnp_staged phase, when libtool triggered relink for the shared libkj-test library. Despite the ld: error: unable to find library -lkj message, the libkj-0.7.0.so is available in the stage directory, and -L option has been specified correctly as well. I guess it's libkj.la's responsibility.

As we are not using the libkj-test library to build the libmultiprocess one, it seems safe and reasonable to workaround this issue by disabling shared libraries in the capnp package altogether.

... and how --disable-shared fixes it, but these changes all look good

It avoids libtools relink steps.

improves configuring according to the docs.

the capnp docs?

Yes. To be precise, I mean ./configure --help from the capnp package (as the commit message quotes).

Also, see https://github.com/capnproto/capnproto/blob/v0.7.0/c++/configure.ac#L78-L94

@fanquake fanquake merged commit 57c1927 into bitcoin:master Oct 10, 2022
@hebasto hebasto deleted the 220609-capnp branch October 10, 2022 13:08
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 10, 2022
@fanquake
Merge bitcoin#25322: build: Fix capnp package build for Android
b8a7199 
8b8edc2 build: Specify native binaries explicitly when building `capnp` package (Hennadii Stepanov)
a413595 build: Fix `capnp` package build for Android (Hennadii Stepanov)

Pull request description:

  On master (e3c08eb):
  ```
  $ make -C depends capnp MULTIPROCESS=1 HOST=aarch64-linux-android ANDROID_SDK=$ANDROID_HOME ANDROID_NDK=$ANDROID_HOME/ndk/23.2.8568313 ANDROID_API_LEVEL=28 ANDROID_TOOLCHAIN_BIN=$ANDROID_HOME/ndk/23.2.8568313/toolchains/llvm/prebuilt/linux-x86_64/bin
  ...
  ld: error: unable to find library -lkj
  ...
  ```

  This PR fixes this error, and also improves configuring according to the docs.

ACKs for top commit:
  ryanofsky:
    Code review ACK 8b8edc2. I'd be a little curious to know what causes the error and how `--disable-shared` fixes it, but these changes all look good

Tree-SHA512: 1b07b75f2a83932d8dc1f007e42a67d8327bd5fe4566f554dab4599e2a1e04b0144648790a1fd2ab1c295dba728586035aa0ebdbe5cf49df048ec87736895aaf
@bitcoin bitcoin locked and limited conversation to collaborators Oct 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ryanofsky ryanofsky ryanofsky approved these changes

Assignees

No one assigned

Labels

Build system

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@hebasto @DrahtBot @fanquake @ryanofsky