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

Simplify Xcode CI / Docs #2266

Merged
merged 3 commits into from
Oct 7, 2024
Merged

Simplify Xcode CI / Docs #2266

merged 3 commits into from
Oct 7, 2024

Conversation

scoopr
Copy link
Contributor

@scoopr scoopr commented Oct 2, 2024

I happened to stumble upon the C_COMPILER_LAUNCHER Xcode attribute,
which fits the bill perfectly, so use that instead.

@scoopr
Copy link
Contributor Author

scoopr commented Oct 2, 2024

Meh, just as we got the #2257 merged, some Xcode warning exposes that it supports C_COMPILER_LAUNCHER attribute, which is a lot simpler than the wrapper script

@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2024

Codecov Report

Attention: Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 40.62%. Comparing base (0cc0c62) to head (dc91279).
Report is 81 commits behind head on main.

Files with missing lines Patch % Lines
src/compiler/gcc.rs 0.00% 1 Missing and 1 partial ⚠️
src/compiler/compiler.rs 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2266      +/-   ##
==========================================
+ Coverage   30.91%   40.62%   +9.71%     
==========================================
  Files          53       54       +1     
  Lines       20112    20734     +622     
  Branches     9755     9637     -118     
==========================================
+ Hits         6217     8423    +2206     
- Misses       7922     8148     +226     
+ Partials     5973     4163    -1810     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@scoopr
Copy link
Contributor Author

scoopr commented Oct 2, 2024

Aha, but actually now it enables the response file use, so that needs to be disabled, I was a bit hasty on this PR, it needs a bit more work to be correct.

@scoopr scoopr force-pushed the simplify-xcode branch 2 times, most recently from 3094640 to b428d49 Compare October 2, 2024 11:18
@scoopr
Copy link
Contributor Author

scoopr commented Oct 2, 2024

Alright, when using C_COMPILER_LAUNCHER, instead of wrapper script for CC, it seems to be passing a bit more flags (as it knows the compiler better), So I added it to passthrough the -ivfstatcache path. Also noticed a single -x objective-c++-header from my project, so went ahead and added that as supported too.

I think it is much better like this.

I happened to stumble upon the `C_COMPILER_LAUNCHER` Xcode attribute,
which fits the bill perfectly, so use that instead.
@scoopr
Copy link
Contributor Author

scoopr commented Oct 7, 2024

Just a friendly poke, was there any concerns here?

@sylvestre sylvestre merged commit 2932b22 into mozilla:main Oct 7, 2024
56 checks passed
@sylvestre
Copy link
Collaborator

nope, thanks for the ping :)

@scoopr scoopr deleted the simplify-xcode branch October 8, 2024 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants