Skip to content

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Jan 8, 2024

The ALLOW_HOST_PACKAGES variable was introduced in #10508 "to speed up build and avoid timeout".

It is no longer the case for our CI infrastructure, which uses self- hosted persistent workers and depends caching.

In the current circumstances, it does not seem worth porting this feature to the upcoming CMake-based build system.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 8, 2024

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK fanquake, TheCharlatan
Concept ACK theuni

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #29091 ([28.x] build: Bump g++ minimum supported version to 11 by maflcko)

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.

The `ALLOW_HOST_PACKAGES` variable was introduced in bitcoin#10508 "to
speed up build and avoid timeout".

It is no longer the case for our CI infrastructure, which uses self-
hosted persistent workers and depends caching.

In the current circumstances, it does not seem worth porting this
feature to the upcoming CMake-based build system.
@fanquake
Copy link
Member

fanquake commented Jan 9, 2024

ACK 080763a - I can't imagine this option got any use outside our CI. It's also mostly just at odds with the idea of a self-contained dependency builder.

@theuni
Copy link
Member

theuni commented Jan 9, 2024

Concept ACK. Happy to have this gone.

Copy link
Contributor

@sedited sedited left a comment

Choose a reason for hiding this comment

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

ACK 080763a

@DrahtBot DrahtBot requested a review from theuni January 9, 2024 10:57
@fanquake fanquake merged commit b3b19be into bitcoin:master Jan 9, 2024
@hebasto hebasto deleted the 240108-allow branch January 9, 2024 16:09
export CI_IMAGE_NAME_TAG="docker.io/debian:bullseye"
# Use minimum supported python3.9 and gcc-10, see doc/dependencies.md
export PACKAGES="gcc-10 g++-10 python3-zmq qtbase5-dev qttools5-dev-tools libdbus-1-dev libharfbuzz-dev"
export DEP_OPTS="NO_QT=1 NO_UPNP=1 NO_NATPMP=1 DEBUG=1 ALLOW_HOST_PACKAGES=1 CC=gcc-10 CXX=g++-10"
Copy link
Member

Choose a reason for hiding this comment

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

nit: Forgot to adjust the cirrus yaml description of this task?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for #29218 :)

hebasto added a commit to hebasto/bitcoin that referenced this pull request Jan 18, 2024
b965507 fixup! ci: Test CMake edge cases (Hennadii Stepanov)
fb5023d fixup! cmake: Add cross-compiling support (Hennadii Stepanov)
84f94a8 fixup! build: Generate `share/toolchain.cmake` in depends (Hennadii Stepanov)

Pull request description:

  This PR mirrors changes from bitcoin#29203.

ACKs for top commit:
  TheCharlatan:
    ACK b965507

Tree-SHA512: edeea1943825032c64c95452d7e7b2662bb63dfabb1a35cdbf32136112822b07fc0fb558db942c0d53c0bf3ffaf1cb480169cabd028e3311b0aa9420ccc6e4d4
@bitcoin bitcoin locked and limited conversation to collaborators Jan 12, 2025
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.

6 participants