-
Notifications
You must be signed in to change notification settings - Fork 1.4k
scripts: Uplift scancode-toolkit v32.4.1 #25039
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
scripts: Uplift scancode-toolkit v32.4.1 #25039
Conversation
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: dc1c19a688062c149007479ad1cd866542f0995f more detailssdk-nrf:
Github labels
List of changed files detected by CI (6)
Outputs:ToolchainVersion: Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please checkup commit changes and ordering.
There are changes in one commit being overruled by following commit. Should those be squashed ?
Also commit cd3da98 message is:
scripts: Fixed windows requirements for scancode
Uppercase AMD64 instead of amd64.
But the commit does much more than this.
.github/workflows/west-commands.yml
Outdated
run: | | ||
pip3 install -r nrf/scripts/requirements-fixed.txt | ||
pip3 install -r nrf/scripts/requirements-west-ncs-sbom.txt | ||
pip3 install -r nrf/scripts/requirements-extra.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought requirements-extra.txt was included when populated requirements-fixed.txt, so is this line still needed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right, if a PR has requirements-fixed up to date(which it should) with changes in requirements-west-ncs-sbom and requirements-extra then those requirements are not required. I will remove both requirements-extra.txt and requirements-west-ncs-sbom.txt
.github/workflows/west-commands.yml
Outdated
python -m pip install --upgrade pip | ||
python -m pip install -r nrf/scripts/requirements-fixed.txt | ||
python -m pip install -r nrf/scripts/requirements-west-ncs-sbom.txt | ||
python -m pip install -r nrf/scripts/requirements-extra.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought requirements-extra.txt was included when populated requirements-fixed.txt, so is this line still needed ?
this PR was merged and then reverted.
|
I can try and redo the commits to have basically 3: workflow update, scancode uplift and scancode json adaptation. |
Required for west ncs-sbom. Signed-off-by: Nicolae Dicu <[email protected]>
Scancode outputs json format 4.1.0 now. Sbom has been adapted. Signed-off-by: Nicolae Dicu <[email protected]>
Run west ncs-sbom command for supported OS: windows-latest ubuntu-24.04 on dummy file and verify scancode version. Signed-off-by: Nicolae Dicu <[email protected]>
cd3da98
to
a2dc370
Compare
Nordic version of x690 does not include `tests` folder. exhuma/x690#2 Signed-off-by: Jan Gałda <[email protected]>
019b94a
to
dc1c19a
Compare
@nicu1989 This is ready to merge, just need a +1 |
@carlescufi and @tejlmand both approved the previous PR. |
Required for west ncs-sbom.
Requires PR: nrfconnect/sdk-zephyr#3282
#24571 was reverted for a test regression.
This is a redo.