Use copy of pigweed.json maintained in Matter SDK to prevent pulling in unneeded CIPD package #29548
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
|
PR #29548: Size comparison from 1248c18 to 6e33240 Decreases (2 builds for linux)
Full report (25 builds for cc13x4_26x4, cc32xx, cyw30739, k32w, linux, mbed, nrfconnect, psoc6, qpg)
|
|
PR #29548: Size comparison from 1248c18 to d938771 Full report (1 build for cc32xx)
|
|
PR #29548: Size comparison from 1248c18 to fa04c6f Increases (2 builds for k32w)
Decreases (5 builds for k32w, linux, telink)
Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
tehampson
changed the title
pullapprove
bot
requested review from
andy31415,
andyg-apple,
anush-apple,
arkq,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
electrocucaracha,
gjc13,
harsha-rajendran,
hawk248 and
hicklin
pullapprove
bot
requested review from
wiba-nordic,
woody-apple,
younghak-hwang,
yufengwangca and
yunhanw-google
andy31415
reviewed
Oct 3, 2023
andy31415
approved these changes
Oct 3, 2023
|
@tehampson tidy seems to say I wonder if we somehow pull an even newer (or somehow different) clang in the updated file than what chip currently has. |
This may also be an actual bug :/ |
Yeah it is. I am trying to reproduce locally and test out a real fix |
|
PR #29548: Size comparison from 1248c18 to fbf0768 Increases (2 builds for k32w)
Decreases (4 builds for k32w, linux)
Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
This package only gets pulled in for windows platforms, so it does not contribute the the space issue we are seeing on CI. We don't have any CI that runs Windows compilation, so I would like to avoid removing it in this PR, on the off chance there is a developer somewhere that has got matter Windows compilation working and this potential breaks them. |
|
PR #29548: Size comparison from 1248c18 to 94b9c6f Increases (27 builds for bl702, bl702l, cc32xx, k32w, linux, psoc6)
Decreases (68 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
bzbarsky-apple
approved these changes
Oct 5, 2023
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
|
PR #29548: Size comparison from 1248c18 to 9d42875 Increases above 0.2%:
Increases (17 builds for bl602, bl702, bl702l, cc32xx, k32w, linux, psoc6)
Decreases (68 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
PR #29548: Size comparison from a75d2e5 to 408e750 Increases (1 build for telink)
Decreases (2 builds for linux)
Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
HunsupJung
pushed a commit
to HunsupJung/connectedhomeip
that referenced
this pull request
Oct 23, 2023
…in unneeded CIPD package (project-chip#29548) * Initial test to confirm issue * Remove rust CIPD package * Changed android workflow to no longer delete rust folder * Changed android smoketest workflow to no longer delete rust folder * Fix new clang tidy error * Address PR comment * Update .github/workflows/full-android.yaml Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> --------- Co-authored-by: Andrei Litvin <andy314@gmail.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
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.
This initial implementation is anticipated to only be a temporary measure. This prevents bootstrap script from ever downloading the rust cipd package into the pigweed
.envirnomentdirectory and save 1.3GB. This will help reduce the out of memory issue we are brushing up against in github CI.We have issue #29547 to track the current temporary fix that we are hoping to shortly remove for a more sustainable long term solution where. What makes this temporary solution not sustainable because the matter repo is now maintaining the pigweed.json file is manually updated from (here)[https://github.com/google/pigweed/blob/main/pw_env_setup/py/pw_env_setup/cipd_setup/pigweed.json].
The burden of addressing pigweed rollups issues has typically been andy31415, and going forward that will also be on me. So the burden introduced by this temp measure shouldn't affect anyone else.
Test: Confirmed CI passes