[Silabs] Update Silicon Labs container #27531
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
woody-apple
approved these changes
Jun 28, 2023
|
PR #27531: Size comparison from 3c8b8cf to 25c285e Increases (3 builds for bl602, bl702, nrfconnect)
Decreases (3 builds for bl602, bl702, cc32xx)
Full report (14 builds for bl602, bl702, bl702l, cc32xx, linux, mbed, nrfconnect, qpg)
|
jepenven-silabs
force-pushed
the
silabs_Docker_update
branch
from
25c285e to
1d8af94
Compare
pullapprove
bot
requested review from
amitnj,
andy31415,
arkq,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
CodeChronos928,
Damian-Nordic,
dhrishi,
electrocucaracha,
gjc13,
harimau-qirex,
harsha-rajendran,
hawk248,
jelderton,
jmartinez-silabs,
jmeg-sfy,
joonhaengHeo,
kkasperczyk-no,
lazarkov,
lpbeliveau-silabs,
LuDuda,
mlepage-google and
mrjerryjohns
pullapprove
bot
requested review from
tcarmelveilleux,
tecimovic,
tehampson,
turon,
vijs,
xylophone21,
younghak-hwang and
yufengwangca
selissia
approved these changes
Jun 28, 2023
|
PR #27531: Size comparison from 25a9150 to 1d8af94 Increases (14 builds for bl602, bl702, cc32xx, esp32, nrfconnect, psoc6, telink)
Decreases (5 builds for nrfconnect, telink)
Full report (58 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
andy31415
requested changes
Jun 28, 2023
There was a problem hiding this comment.
In this particular case, please describe WHY the changes are needed, not just the changes themselves.
In particular, I wonder if the vscode image will need updating (and if so, how we would merge openjdk requirements between what android/java images want and openjdk items installed for efr32.
jepenven-silabs
force-pushed
the
silabs_Docker_update
branch
from
1d8af94 to
6c02878
Compare
|
PR #27531: Size comparison from a7edf3d to 6c02878 Increases (12 builds for bl702l, esp32, psoc6, telink)
Decreases (10 builds for bl602, bl702, cc32xx, esp32, psoc6, telink)
Full report (58 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
The Vscode image won't need updating in that case since it doesn't use slc-cli yet. This is still and ongoing effort, but on completion we should be able to remove a complete submodule from our sdk and at that point the vscode image would indeed need an update. |
andy31415
approved these changes
Jun 29, 2023
There was a problem hiding this comment.
Adding checkmark based on separate chat to understand the issue.
Apparently the problem is that slc_cli_linux.zip is a moving target, so docker images are inconsistent.
This change makes it so that docker build does not fail, however slc_cli as it currently exists cannot be relied upon to not change.
We only validate a sanity check for EFR32 for now, however long term expectation is that we fix slc_cli somehow (maybe submodule) and at that point we figure out the stable dependencies to put in efr32 and vscode to make it work.
Until then, developers are not expected to validate/run the slc-cli tools (which seem to be meant for a IDE integration mostly)
andy31415
reviewed
Jun 29, 2023
| # RUN slc configuration --sdk="$GSDK_ROOT" && \ | ||
| # slc signature trust --sdk "$GSDK_ROOT" | ||
|
|
||
| # Install Python Packages |
There was a problem hiding this comment.
Based on logs now I see that the above command on line 29 will fail since requirements.txt seems gone from the zip file.
andy31415
reviewed
Jun 29, 2023
|
|
||
| # SLC required Python Packages |
There was a problem hiding this comment.
these are not required by SLC, my guess is these are codegen.py requirements by the looks of it (happen to be familiar with that tool, these are exactly its dependencies).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As the title described.
Add Sphinx, and Java 17 since slc-cli (Silicon Labs specific platform generation tools) required its.
SLC CLI doesn't have the option to fetch a specific version. This is why we removed the calls from the Dockerfile. These steps, which are really quick to execute, will be made directly inside the EFR32 CI step to prevent DOcker container breakage should the SLC CLI version changes. This is far from ideal but it's the best way to do dommage control IMO.