Skip to content

build: Drop per-host faketime wrappers in gitian-linux build #19627

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
Closed
hebasto wants to merge 3 commits into from
Closed

build: Drop per-host faketime wrappers in gitian-linux build #19627

hebasto wants to merge 3 commits into from

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Jul 30, 2020

This PR is based on #19622, and the first two commits belong to #19622.

This is another step to removing libfaketime from gitian builds.

hebasto added 3 commits July 29, 2020 23:50
@hebasto
build: Clean and sort package list in gitian-linux.yml
1ba1496
@hebasto
build: Drop old hack which is unneeded now
d3fe00d 
This hack is described in bitcoin#8188. The current implementation was
introduced in bitcoin#8315.
@hebasto
build: Drop per-host faketime wrappers in gitian-linux build
55f3f74 
Replaced with the --enable-determinism configure script option.
@hebasto hebasto mentioned this pull request Jul 30, 2020
Merged
@maflcko
Copy link
Member

maflcko commented Jul 30, 2020

Concept ACK on removing code, assuming that everything is still correct

dongcarl
dongcarl reviewed Jul 30, 2020
View reviewed changes
Copy link
Contributor

@dongcarl dongcarl left a comment

Choose a reason for hiding this comment

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

Got some hashes you can post? ☺️

contrib/gitian-descriptors/gitian-linux.yml
Comment on lines 18 to 21
- "g++"
- "g++-arm-linux-gnueabihf"
- "g++-aarch64-linux-gnu"
- "g++-riscv64-linux-gnu"
Copy link
Contributor

@dongcarl dongcarl Jul 30, 2020

Choose a reason for hiding this comment

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

Let's not remove the explicit -8 to be more explicit...
We can use update-alternatives in the script to change the default gcc/g++ I think

@hebasto
Copy link
Member Author

hebasto commented Jul 30, 2020

@dongcarl

Got some hashes you can post?

1f6339377bcf2eb7f01233f730c8e36c07511c5a4be94a1bead96a9b2cb9aee0  bitcoin-55f3f74bf970-aarch64-linux-gnu-debug.tar.gz
8346d46076f47d39704ac7ab186a5bde4a9b549a8838ce925665f47b0f904dca  bitcoin-55f3f74bf970-aarch64-linux-gnu.tar.gz
c8ce9b2dd3c2d63cbab7d4757870999fb3900e0396d73b69f71b8c8a7c9f4caf  bitcoin-55f3f74bf970-arm-linux-gnueabihf-debug.tar.gz
d7a2aab88e81d9efe0b419944f1a97b8fb06f6aa08de4d96e1faefa13619b99d  bitcoin-55f3f74bf970-arm-linux-gnueabihf.tar.gz
7290e27027bd3e7a3fbbbb090d66838df401f80abd1868cb700152592892a6e5  bitcoin-55f3f74bf970-riscv64-linux-gnu-debug.tar.gz
3ec37df653eaaa504e2f1b02ab168093998e49e77d073978af8c80af11b54c47  bitcoin-55f3f74bf970-riscv64-linux-gnu.tar.gz
13c88bf1a3e38af78b5e6abebd9b2ed7c724d0dbd961339159f603ced0508df5  bitcoin-55f3f74bf970-x86_64-linux-gnu-debug.tar.gz
e65a8627729bfc578ca738bacca87ee1b7d692a9a50d022d563f182c83b14138  bitcoin-55f3f74bf970-x86_64-linux-gnu.tar.gz
2430de380ac7d943740ff1ce492a6a66e91da7071b4d381895ebf8400b4533ea  src/bitcoin-55f3f74bf970.tar.gz

@dongcarl
Copy link
Contributor

dongcarl commented Jul 30, 2020

@hebasto See: #19622 (comment)

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 30, 2020 •
edited
Loading

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

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.

@DrahtBot DrahtBot mentioned this pull request Jul 31, 2020
Closed
fanquake
fanquake reviewed Jul 31, 2020
View reviewed changes
contrib/gitian-descriptors/gitian-linux.yml
HOSTS="x86_64-linux-gnu arm-linux-gnueabihf aarch64-linux-gnu riscv64-linux-gnu"
CONFIGFLAGS="--enable-glibc-back-compat --enable-reduce-exports --disable-bench --disable-gui-tests"
FAKETIME_HOST_PROGS="gcc g++"
CONFIGFLAGS="--enable-determinism --enable-glibc-back-compat --enable-reduce-exports --disable-bench --disable-gui-tests"
Copy link
Member

@fanquake fanquake Jul 31, 2020

Choose a reason for hiding this comment

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

Passing --enable-determinism here isn't going to do anything, as that option currently only enables an additional linker argument when building for Windows.

Copy link
Member Author

@hebasto hebasto Jul 31, 2020

Choose a reason for hiding this comment

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

🤦 indeed

bitcoin/configure.ac

Lines 807 to 811 in a63a26f

if test x$enable_determinism = xyes; then
if test x$TARGET_OS = xwindows; then
AX_CHECK_LINK_FLAG([[-Wl,--no-insert-timestamp]], [LDFLAGS="$LDFLAGS -Wl,--no-insert-timestamp"],, [[$LDFLAG_WERROR]])
fi
fi

Copy link
Member Author

@hebasto hebasto Jul 31, 2020

Choose a reason for hiding this comment

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

Why do I observe deterministic builds without faketime wrappers?

@DrahtBot DrahtBot mentioned this pull request Jul 31, 2020
Merged
@hebasto hebasto closed this Aug 2, 2020
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@fanquake fanquake fanquake left review comments

+1 more reviewer

@dongcarl dongcarl dongcarl left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Build system

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@hebasto @maflcko @dongcarl @DrahtBot @fanquake