Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Oct 3, 2022

This was added in 7fc5e86 but I can't see a reason why this should be forbidden.

This is also needed for other changes (bumping the minimum python version).

# shellcheck disable=SC2086
${CI_RETRY_EXE} pip3 install --user $PIP_PACKAGES
${CI_RETRY_EXE} CI_EXEC pip3 install --user $PIP_PACKAGES
Copy link
Member Author

Choose a reason for hiding this comment

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

Review note: Adding CI_EXEC is required, otherwise it would result in a failure.

To reproduce the failure:

$ RESTART_CI_DOCKER_BEFORE_RUN=1 FILE_ENV="./ci/test/00_setup_env_i686_centos.sh" ./ci/test_run_all.sh
...
bash: line 1: pip3: command not found

Copy link
Member Author

Choose a reason for hiding this comment

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

This also fixes a bug on historic commits. To reproduce the bug on the historic commits:

$ git checkout 3c3bd9022026a75e15492ba9cf8bf3b72866b9a9~ && RESTART_CI_DOCKER_BEFORE_RUN=1 FILE_ENV="./ci/test/00_setup_env_i686_multiprocess.sh" ./ci/test_run_all.sh
...
bash: line 1: pip3: command not found

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK fa684bc, I have reviewed the code and it looks OK, I agree it can be merged.

@maflcko
Copy link
Member Author

maflcko commented Oct 3, 2022

cc @fanquake (author of 7fc5e86)

@maflcko maflcko requested a review from fanquake October 3, 2022 12:48
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK fa684bc - I see lief still being installed.

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 3, 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:

  • #26226 (Bump minimum python version to 3.7 by MarcoFalke)
  • #25900 (ci: run docker wrapper with a non-root user by josibake)
  • #24798 ([POC] build: Hello Qt 6 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.

MacroFake added 2 commits October 4, 2022 11:51
This was added in 7fc5e86 but I can't
see a reason why this should be forbidden.
@maflcko
Copy link
Member Author

maflcko commented Oct 4, 2022

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

re-ACK fa6054e

@fanquake fanquake merged commit a23f8c8 into bitcoin:master Oct 4, 2022
@maflcko maflcko deleted the 2210-centos-pip-🏣 branch October 5, 2022 08:57
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 5, 2022
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 12, 2023
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 12, 2023
This was added in 7fc5e86 but I can't
see a reason why this should be forbidden.

Github-Pull: bitcoin#26234
Rebased-From: fa6054e
@fanquake fanquake mentioned this pull request Jan 12, 2023
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 12, 2023
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 12, 2023
This was added in 7fc5e86 but I can't
see a reason why this should be forbidden.

Github-Pull: bitcoin#26234
Rebased-From: fa6054e
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 12, 2023
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jan 12, 2023
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jan 12, 2023
This was added in 7fc5e86 but I can't
see a reason why this should be forbidden.

Github-Pull: bitcoin#26234
Rebased-From: fa6054e
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 12, 2023
This was added in 7fc5e86 but I can't
see a reason why this should be forbidden.

Github-Pull: bitcoin#26234
Rebased-From: fa6054e
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 12, 2023
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 12, 2023
jarolrod pushed a commit to jarolrod/bitcoin that referenced this pull request Jan 13, 2023
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 13, 2023
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 13, 2023
@bitcoin bitcoin locked and limited conversation to collaborators Oct 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants