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: support OpenBSD in depends #23998

Merged
merged 2 commits into from Feb 11, 2022

Conversation

fanquake
Copy link
Member

@fanquake fanquake commented Jan 7, 2022

Completes the BSD trifecta. No Qt. Includes a commit that adds a default build_TAR to depends, to make it easier to override and use gnu/gtar, as OpenBSDs tar does not support some of the options we use. Also includes this patch, boostorg/test@684f067, to fix a compilation issue in Boost test.

Have tested on OpenBSD 7.0 with:

gmake -C depends -j9
./autogen.sh
CONFIG_SITE=/home/vagrant/bitcoin/depends/amd64-unknown-openbsd7.0/share/config.site ./configure --disable-external-signer MAKE=gmake
gmake check -j9

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 7, 2022

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24301 (build: header-only Boost by fanquake)
  • #23955 (build: add support for NetBSD in depends by fanquake)
  • #23571 (build: Propagate user-defined tools to native packages by hebasto)
  • #22811 (build: Fix depends build system when working with subtargets by hebasto)
  • #22708 (build, qt: Add Wayland support for Linux builds with depends by hebasto)
  • #21995 (build: Make built dependency packages reproducible by hebasto)
  • #19952 (build, ci: Add file-based logging for individual packages by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@theStack
Copy link
Contributor

Concept ACK

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

I could successfully build and run the tests on OpenBSD 7.0 with the instructions provided in the PR description. However, I had to prepend TAR=gtar to the first gmake call, otherwise it would fail due to the base system tar not supporting the needed options.

Is this supposed to be specified by the user or was the idea to set TAR=gtar by default in the Makefile on OpenBSD?

@laanwj
Copy link
Member

laanwj commented Feb 10, 2022

We need to document the needed packages/ports somewhere (in build-openbsd.md would be the most likely place? or maybe in depend's README.md):

pkg_add gtar bash
  • gtar as mentioned
  • bash for gen_id only i think, not installing it will give the following output but it seems to proceed otherwise:
env: ./gen_id: No such file or directory
env: ./gen_id: No such file or directory

@fanquake
Copy link
Member Author

Rebased.

Is this supposed to be specified by the user or was the idea to set TAR=gtar by default in the Makefile on OpenBSD?

Made a (currently untested) change such that using gtar should be set automatically, so no need to specify TAR=gtar when building.

We need to document the needed packages/ports somewhere

I've added these to the depends README.

@laanwj
Copy link
Member

laanwj commented Feb 10, 2022

Thanks! Updated to commit 8793ee3.
I don't think the automatic workaround is working. gtar is installed but:

Extracting native_b2...
(SHA256) /home/user/bitcoin/depends/sources/boost_1_71_0.tar.bz2: OK
tar: unknown option -- -
usage: tar {crtux}[014578befHhjLmNOoPpqsvwXZz]
           [blocking-factor | archive | replstr] [-C directory] [-I file]
           [file ...]
       tar {-crtux} [-014578eHhjLmNOoPpqvwXZz] [-b blocking-factor]
           [-C directory] [-f archive] [-I file] [-s replstr] [file ...]
gmake: *** [funcs.mk:282: /home/user/bitcoin/depends/work/build/amd64-unknown-openbsd7.0/native_b2/1_71_0-091b88c6ae3/.stamp_extracted] Error 1

Adding TAR=gtar gets it through that.

@laanwj
Copy link
Member

laanwj commented Feb 10, 2022

zeromq build fails with:

Provide an AUTOCONF_VERSION environment variable, please
autogen.sh: error: autoreconf exited with status 127
gmake: *** [funcs.mk:283: /home/user/bitcoin/depends/work/build/amd64-unknown-openbsd7.0/zeromq/4.3.4-a71c7ab8497/./.stamp_configured] Error 1

same as or the normal build instructions, i suppose:

export AUTOCONF_VERSION=2.69
export AUTOMAKE_VERSION=1.16

@fanquake
Copy link
Member Author

I don't think the automatic workaround is working. gtar is installed but:

Sorry, should be addressed now. I've reworked how it's set. Will test shortly myself.

@laanwj
Copy link
Member

laanwj commented Feb 10, 2022

The gtar issue is fixed. I didn't have to explicitly specify it this time. It succeeds in building the default dependencies (except Qt, as documented).

However, running into the following during configure:

Edit: never mind, I did something wrong with command line arguments I think. It works now!

@laanwj
Copy link
Member

laanwj commented Feb 10, 2022

Tested ACK 471d155
I did a depends build on OpenBSD 7.0, then built bitcoind, and ran both the unit tests and functional tests succesfully. I also checked with ldd that all depends were linked statically.

@fanquake
Copy link
Member Author

@theStack would you like to take another look here?

@@ -201,11 +201,11 @@ define $(package)_extract_cmds
echo "$($(package)_qttools_sha256_hash) $($(package)_source_dir)/$($(package)_qttools_file_name)" >> $($(package)_extract_dir)/.$($(package)_file_name).hash && \
$(build_SHA256SUM) -c $($(package)_extract_dir)/.$($(package)_file_name).hash && \
mkdir qtbase && \
tar --no-same-owner --strip-components=1 -xf $($(package)_source) -C qtbase && \
$(build_TAR) --no-same-owner --strip-components=1 -xf $($(package)_source) -C qtbase && \
Copy link
Member

@laanwj laanwj Feb 11, 2022

Choose a reason for hiding this comment

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

I think it's fine to require gtar—but I think if someone really wanted, they could work around this without any special tar options. I don't really understand --no-same-owner here at all (it's only relevant when running this as root, and who builds as root?). --strip-components=1 will be harder to replace (but not impossible as the first component will be predictable based on the archive name IIRC).

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

Tested ACK 471d155
(on OpenBSD 7.0, amd64)

gtar is now selected automatically. If it's missing, the message "gtar: not found" appears and the build is aborted:

$ gmake -C depends/ -j4
gmake: Entering directory '/home/honey/bitcoin_prrev/depends'
Extracting native_b2...
(SHA256) /home/honey/bitcoin_prrev/depends/sources/boost_1_71_0.tar.bz2: OK
/bin/sh: gtar: not found
gmake: *** [funcs.mk:282: /home/honey/bitcoin_prrev/depends/work/build/amd64-unknown-openbsd7.0/native_b2/1_71_0-c97b4ba1fa7/.stamp_extracted] Error 127
gmake: Leaving directory '/home/honey/bitcoin_prrev/depends'

After installing gtar ($ doas pkg_add gtar) the build succeeds with the instruction in the PR description.

I did a depends build on OpenBSD 7.0, then built bitcoind, and ran both the unit tests and functional tests succesfully. I also checked with ldd that all depends were linked statically.

Did the same and can confirm that everything succeeds and the only dynamically linked libraries in bitcoind belong to the base system (libpthread, libz, libm, libc++, libc++abi, libc, ld) and not to depends.

@fanquake fanquake merged commit 3bb9394 into bitcoin:master Feb 11, 2022
@fanquake fanquake deleted the depends_support_openbsd branch February 11, 2022 21:26
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 12, 2022
471d155 build: add support for OpenBSD to depends (fanquake)
75ae39e build: add a default build tar in depends (fanquake)

Pull request description:

  Completes the BSD trifecta. No Qt. Includes a commit that adds a default build_TAR to depends, to make it easier to override and use gnu/gtar, as OpenBSDs tar does not support some of the options we use. Also includes this patch, boostorg/test@684f067, to fix a compilation issue in Boost test.

  Have tested on OpenBSD 7.0 with:
  ```bash
  gmake -C depends -j9
  ./autogen.sh
  CONFIG_SITE=/home/vagrant/bitcoin/depends/amd64-unknown-openbsd7.0/share/config.site ./configure --disable-external-signer MAKE=gmake
  gmake check -j9
  ```

ACKs for top commit:
  laanwj:
    Tested ACK 471d155
  theStack:
    Tested ACK 471d155

Tree-SHA512: 7009b5d4c84355ebe4ece7042e0ce131659b97eace611fb77f1f16901aafb4b28118961a608a245289772ef7c4acb606dc18357a82c91cdceaf6466fc1cfd242
fanquake added a commit that referenced this pull request Mar 25, 2022
28f17c1 build: fix copypasta in OpenBSD C{XX} flags (fanquake)

Pull request description:

  Introduced in #23998.

ACKs for top commit:
  hebasto:
    ACK 28f17c1, I have reviewed the code and it looks OK, not tested on OpenBSD though.

Tree-SHA512: d905161534075f518c8924d3c42cca7ff8d4898e559f1daa9bd03dac95b109b2c3e76790fb8bc65b9e45e8a59566825afbf4dc3734ad74617dfdf797430e486b
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 2, 2022
28f17c1 build: fix copypasta in OpenBSD C{XX} flags (fanquake)

Pull request description:

  Introduced in bitcoin#23998.

ACKs for top commit:
  hebasto:
    ACK 28f17c1, I have reviewed the code and it looks OK, not tested on OpenBSD though.

Tree-SHA512: d905161534075f518c8924d3c42cca7ff8d4898e559f1daa9bd03dac95b109b2c3e76790fb8bc65b9e45e8a59566825afbf4dc3734ad74617dfdf797430e486b
@bitcoin bitcoin locked and limited conversation to collaborators Feb 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants