-
Notifications
You must be signed in to change notification settings - Fork 156
Add swiftsourceinfo file to worker incremental outputs #697
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
Add swiftsourceinfo file to worker incremental outputs #697
Conversation
|
Thanks @BalestraPatrick! After reverting the one part, I can merge this. |
1bfc72d to
40cf41d
Compare
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
From slack it sounds like the thought with not adding it here is that it's always emitted whether you ask for it or not. I think doing this will fix that error but likely produce empty files instead |
40cf41d to
c27edab
Compare
|
I believe this fix is 100% correct. I'm not sure why the upstream change didn't have it. |
Because I have a pending change that adds it, along with a number of other incremental improvements, which I thought I would land by now. But other things got in the way 😅 We don't really use incremental mode internally (because historically it hasn't been well-tested and hasn't saved much when it has been used), so that would explain why we never experienced the issue. |
|
I am experiencing the same problem when building a mac CLI with bazel (using EDIT: |
|
huh, can you file an issue with a repro case? |
|
Sorry for late reply. I was now trying to reproduce the problem and provide a reproducible case, but seems I cant anymore 😕 Maybe I was doing something wrong, or could be that during last month other changes in my project fixed the problem |
After bumping our
rules_swiftversion to d69bc40, we noticed some failures possibly related to the changes in 9a1073d.About 10-20% of our builds started failing on CI and locally with errors such as:
Digging into the execution log, it seems like this file was in
listed_outputsbut not present inactual_outputsin some cases. We always build with sandbox on.Basically in https://github.com/bazelbuild/rules_swift/pull/523/files we wanted to add support for
swiftsourceinfo. Even though that PR wasn’t merged as far as I can see, the cherry-picked commit from upstream (#692) didn’t even contain the change totools/worker/output_file_map.cc. The following patch fixes it for me on a local machine that previously was failing 100% of the builds with errors like the one above.