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

ci: workflows: pin python dependencies #87609

Merged
merged 3 commits into from
Mar 29, 2025

Conversation

nashif
Copy link
Member

@nashif nashif commented Mar 25, 2025

Pin python dependencies to hashes and cleanup/unify python setup steps in
various workflows.

We now have one dependency file containing all requirements for github
actions that is managed centrally with hashes. No direct pip installs
are needed in workflow files and everything shall go via the
requirements file.

Pinning to specific version and hashes helps with preventing supply
chain attacks.

Signed-off-by: Anas Nashif [email protected]

@nashif nashif force-pushed the topic/ci/pin_deps branch 2 times, most recently from e13a855 to 68c2a9b Compare March 25, 2025 08:41
@nashif nashif force-pushed the topic/ci/pin_deps branch 2 times, most recently from 18b79c3 to 374367f Compare March 25, 2025 08:59
@nashif
Copy link
Member Author

nashif commented Mar 25, 2025

hmm, not sure about this one:

 Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.12.9/x64/bin/gitlint", line 5, in <module>
    from gitlint.cli import cli
  File "/opt/hostedtoolcache/Python/3.12.9/x64/lib/python3.12/site-packages/gitlint/cli.py", line 3, in <module>
    from gitlint.config import LintConfig, LintConfigError
  File "/opt/hostedtoolcache/Python/3.12.9/x64/lib/python3.12/site-packages/gitlint/config.py", line 2, in <module>
    import ConfigParser
ModuleNotFoundError: No module named 'ConfigParser'
Compliance error, check for error messages in the "Run Compliance Tests" st

I have not seen this during testing :(

@pdgendt
Copy link
Collaborator

pdgendt commented Mar 25, 2025

hmm, not sure about this one:

 Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.12.9/x64/bin/gitlint", line 5, in <module>
    from gitlint.cli import cli
  File "/opt/hostedtoolcache/Python/3.12.9/x64/lib/python3.12/site-packages/gitlint/cli.py", line 3, in <module>
    from gitlint.config import LintConfig, LintConfigError
  File "/opt/hostedtoolcache/Python/3.12.9/x64/lib/python3.12/site-packages/gitlint/config.py", line 2, in <module>
    import ConfigParser
ModuleNotFoundError: No module named 'ConfigParser'
Compliance error, check for error messages in the "Run Compliance Tests" st

I have not seen this during testing :(

Can we update the gitlint package? Also for the requirements.in file, we could specify gitlint-core instead.

@nashif nashif force-pushed the topic/ci/pin_deps branch 2 times, most recently from f7b1d2e to 4ebcffb Compare March 26, 2025 02:24
@cfriedt
Copy link
Member

cfriedt commented Mar 27, 2025

Should help with caching / false positives as well.

@pdgendt
Copy link
Collaborator

pdgendt commented Mar 28, 2025

Why was the "Checkout source code" step skipped in the failed "Publish Unit Tests Results"? 🤔

@nashif nashif force-pushed the topic/ci/pin_deps branch from 30216ea to 81f8987 Compare March 28, 2025 10:19
@nashif
Copy link
Member Author

nashif commented Mar 28, 2025

Why was the "Checkout source code" step skipped in the failed "Publish Unit Tests Results"? 🤔

this is fixed now

@pdgendt
Copy link
Collaborator

pdgendt commented Mar 28, 2025

The commit to temporarily disable the failing test also touches the .github/workflows/twister.yaml file to checkout the pull request head. Is this intentional?

@nashif
Copy link
Member Author

nashif commented Mar 28, 2025

The commit to temporarily disable the failing test also touches the .github/workflows/twister.yaml file to checkout the pull request head. Is this intentional?

no, will fix, thanks.

@nashif nashif force-pushed the topic/ci/pin_deps branch from 81f8987 to fb12ef5 Compare March 28, 2025 19:05
@nashif
Copy link
Member Author

nashif commented Mar 28, 2025

The commit to temporarily disable the failing test also touches the .github/workflows/twister.yaml file to checkout the pull request head. Is this intentional?

no, will fix, thanks.

ok, fixed

@nashif nashif assigned nashif and stephanosio and unassigned aescolar Mar 28, 2025
@@ -178,7 +178,6 @@ jobs:

- name: Merge Test Results
run: |
pip install junitparser junit2html
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like you just dropped them as I can't see any step where you install the requirements?

Copy link
Member Author

Choose a reason for hiding this comment

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

those are already in the docker image, no need to install them

cache: pip
cache-dependency-path: scripts/requirements-actions.txt

- name: install-packages
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you have a chance to do a s/install-packages/Install Python packages/g so that steps have consistent naming that would be great (non blocking obviously)

Comment on lines 40 to 53
- name: Rebase
continue-on-error: true
env:
BASE_REF: ${{ github.base_ref }}
PR_HEAD: ${{ github.event.pull_request.head.sha }}
run: |
git config --global user.email "[email protected]"
git config --global user.name "Github Actions"
rm -fr ".git/rebase-apply"
rm -fr ".git/rebase-merge"
git rebase origin/${BASE_REF}
git clean -f -d
git log --graph --oneline HEAD...${PR_HEAD}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unrelated change?

nashif added 3 commits March 28, 2025 18:25
Pin python dependencies to hashes and cleanup/unify python setup steps in
various workflows.

We now have one dependency file containing all requirements for github
actions that is managed centrally with hashes. No direct pip installs
are needed in workflow files and everything shall go via the
requirements file.

Pinning to specific version and hashes helps with preventing supply
chain attacks.

Signed-off-by: Anas Nashif <[email protected]>
Pin dependencies on the workflow and move it from using docker to the
zephyr setup action.

Signed-off-by: Anas Nashif <[email protected]>
This test fails on older python versions (3.10) and only on CI.
Disabling it while we investigate. The test itself verifies inline logs
options, so the functionality test is not impacted.

Tracked in zephyrproject-rtos#87769

Signed-off-by: Anas Nashif <[email protected]>
@nashif nashif force-pushed the topic/ci/pin_deps branch from 9b43181 to fc7e570 Compare March 28, 2025 22:26
@nashif nashif merged commit 19c6240 into zephyrproject-rtos:main Mar 29, 2025
72 of 73 checks passed
@teburd
Copy link
Collaborator

teburd commented Mar 29, 2025

Lets do it with the entire environment using nix :-D Hash all the things!

@pdgendt
Copy link
Collaborator

pdgendt commented Mar 31, 2025

Missed it last time I reviewed, but commit messages shouldn't contain Github #number references 🫣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants