-
Notifications
You must be signed in to change notification settings - Fork 552
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
Implement features required by Xcode #2257
Conversation
thanks |
Updated the tests and added some docs. If there are additional concerns, it might be few weeks until I can get back to this. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2257 +/- ##
==========================================
+ Coverage 30.91% 40.68% +9.77%
==========================================
Files 53 54 +1
Lines 20112 20732 +620
Branches 9755 9636 -119
==========================================
+ Hits 6217 8435 +2218
- Misses 7922 8150 +228
+ Partials 5973 4147 -1826 ☔ View full report in Codecov by Sentry. |
About test, I was thinking about something like in https://github.com/mozilla/sccache/blob/main/.github/workflows/integration-tests.yml |
Xcode sets it by default, but it can be disabled with `COMPILER_INDEX_STORE_ENABLE=NO` Xcode build setting.
Xcode requires these files to be present after compiling, so they need to be stored and restored for the build to complete.
075d25b
to
4b0d34b
Compare
Sorry about the churn for getting that integration test running, I don't think I have a good way to test it locally, so I'm developing it a bit blind. |
no worries. Could you please write a tiny PR that I could merge quickly so that you get the CI without approval? |
for example, typo fix like: |
well, looks like it worked :) |
Yep, the output now looks like what I expected. I don't think I have anything to add it, so on my side its ready, if no other concerns arise. |
run: | | ||
cargo build | ||
|
||
- name: Create wrapper script |
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.
could you please add a comment explaining why a wrapper script is 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.
Its actually explained in the docs (dfe8c5b#diff-8f7ae669918b1de5a217f0f28f8becd7d0f92625c4b8334b733315b8199dd102R47), but I guess I could add a comment here.
The issue is that I can't just set CC="sccache clang++"
, because it interprets the whole CC
variable as a command, so its searching for a file called sccache clang++
(so space is part of the command name). If you have other ideas for a workaround, I'm all ears.
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.
yeah but i would prefer to have it next to the code :)
It seems the continuous job failed, is it due to something I did, or is it something else? |
nope, we have some intermittent sometimes (when the cache is full i guess) |
These commits allow me to use sccache with Xcode.
Firstly, I just mark
-index-store-path
asHard
, as it is easy to configure Xcode not to use that. Not sure how impossible that would be to implement otherwise, didn't look in to it.And secondly, I'm artifacting the dep (
.d
) files and the-serialize-diagnostics
(.dia
) files, as Xcode seems to always request them, and the build will fail without them.When using it, I need to run the sccache server outside of Xcode, as otherwise it seems that Xcode will keep waiting for the daemon to timeout.
Then I have configured in Xcode build settings the CC/CXX variables, currently to a launcher script that sets the env variables and calls sccache with the right arguments (I am using it through cmake though). The setup might be improved now that I got it to work.