Fix xcodebuild to actually work on Darwin even without ZAP installed. #24539
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 #24539: Size comparison from 6648ef2 to 6e7ce8f Increases (6 builds for nrfconnect, psoc6, qpg, telink)
Decreases (4 builds for psoc6, telink)
Full report (33 builds for cyw30739, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
bzbarsky-apple
force-pushed
the
fix-darwin-builds
branch
from
6e7ce8f to
7e77004
Compare
bzbarsky-apple
force-pushed
the
fix-darwin-builds
branch
from
7e77004 to
c56a2e3
Compare
pullapprove
bot
requested review from
andy31415,
anush-apple,
arkq,
Byungjoo-Lee,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
franck-apple,
gjc13,
harimau-qirex,
harsha-rajendran,
hawk248,
jelderton,
jepenven-silabs,
jmartinez-silabs,
jmeg-sfy,
jtung-apple and
ksperling-apple
pullapprove
bot
requested review from
tcarmelveilleux,
tecimovic,
vijs,
vivien-apple,
woody-apple,
xylophone21 and
yunhanw-google
|
PR #24539: Size comparison from 6648ef2 to c56a2e3 Increases (5 builds for nrfconnect, qpg, telink)
Decreases (1 build for esp32)
Full report (35 builds for cyw30739, esp32, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
bzbarsky-apple
force-pushed
the
fix-darwin-builds
branch
from
c56a2e3 to
a103473
Compare
|
PR #24539: Size comparison from cbc8fc4 to a103473 Increases (7 builds for bl702, psoc6, telink)
Decreases (6 builds for bl602, nrfconnect, psoc6, telink)
Full report (54 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
bzbarsky-apple
force-pushed
the
fix-darwin-builds
branch
from
a103473 to
d131906
Compare
|
PR #24539: Size comparison from f683392 to d131906 Increases (11 builds for bl602, bl702, esp32, psoc6, telink)
Decreases (4 builds for bl602, esp32, psoc6, qpg)
Full report (43 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
andy31415
requested changes
Jan 20, 2023
There was a problem hiding this comment.
Please update summary to explain what was wrong and the fix.
I am trying to understand the code by review, however there seem to be several changes like env location updates, codegen logic updates and xcode updates. I would like to understand so if we ever change xcode again, this kind of error does not occur anymore.
andy31415
reviewed
Jan 20, 2023
andy31415
reviewed
Jan 20, 2023
andy31415
reviewed
Jan 20, 2023
andy31415
reviewed
Jan 20, 2023
I added a bit of a description here, however I am not sure about the cpp-app codegen. Why do we codegen those, it was not needed before. We are going down the wrong path if we start adding more codegen pregen. |
I tried to make it clearer a bit what the problem was. Sorry; I was dealing with this very late last night. To be clear, I view this as a temp fix to unblock us for now, until we fix the "libCHIP.a depends on codegen" bug in the build system. Thank you for filling in the summary! |
andy31415
approved these changes
Jan 20, 2023
bzbarsky-apple
added a commit
to bzbarsky-apple/connectedhomeip
that referenced
this pull request
Jan 20, 2023
…ose are not needed anymore.
bzbarsky-apple
added a commit
that referenced
this pull request
Jan 20, 2023
* Remove dependency of libChipController on app-specific codegen. * Remove the unused cluster id member from ClusterBase. * Make ClusterBase constructor public. * Use ClusterBase instead of generated cluster structs in src/controller. * Remove the CHIPClusters.h includes and codegen dependency. * Revert the non-workflow changes from PR ##24539, since those are not needed anymore. * Regenerate generated files.
lecndav
pushed a commit
to lecndav/connectedhomeip
that referenced
this pull request
Mar 22, 2023
lecndav
pushed a commit
to lecndav/connectedhomeip
that referenced
this pull request
Mar 22, 2023
…ect-chip#24555) * Remove dependency of libChipController on app-specific codegen. * Remove the unused cluster id member from ClusterBase. * Make ClusterBase constructor public. * Use ClusterBase instead of generated cluster structs in src/controller. * Remove the CHIPClusters.h includes and codegen dependency. * Revert the non-workflow changes from PR #project-chip#24539, since those are not needed anymore. * Regenerate generated files.
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.
Darwin builds without ZAP installation fails and this failure is not detected by CI. This is because when building libCHIP.a we insist on running codegen even though nothing that depends on codegen should be present in libCHIP.a.
The fix for now is to change
zzz_generated/darwinto be the output of pregenerate and set chip_code_pre_generated_directory to that directory when building libCHIP.a.Changes: