-
Notifications
You must be signed in to change notification settings - Fork 56
[SwiftDriver] Part III - Introduce synchronization between swift-frontend invocations - #210
Merged
Merged
[SwiftDriver] Part III - Introduce synchronization between swift-frontend invocations #210polac24 merged 6 commits into
polac24 merged 6 commits into
Conversation
polac24
polac24
commented
Jun 4, 2023
```shell
ditto "${SCRIPT_INPUT_FILE_0}" "${SCRIPT_OUTPUT_FILE_0}"
[ -f "${SCRIPT_INPUT_FILE_1}" ] && ditto "${SCRIPT_INPUT_FILE_1}" "${SCRIPT_OUTPUT_FILE_1}" || rm "${SCRIPT_OUTPUT_FILE_1}"
[ -f "${SCRIPT_INPUT_FILE_1}" ] && ditto "${SCRIPT_INPUT_FILE_1}" "${SCRIPT_OUTPUT_FILE_1}" || rm -f "${SCRIPT_OUTPUT_FILE_1}"
Collaborator
Author
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.
sidefix: found that if a first build has a cache miss, this snippet fails with an error that $SCRIPT_OUTPUT_FILE_1 doesn't exist. In such cases, we can safely no-op (so forcing rm)
polac24
marked this pull request as ready for review
June 4, 2023 15:58
polac24
requested review from
PatrikBillgren,
acbdev,
aleksandergrzyb and
vadim-smal
June 4, 2023 15:58
aleksandergrzyb
aleksandergrzyb
previously approved these changes
Jun 6, 2023
@aleksandergrzyb
aleksandergrzyb
left a comment
Contributor
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.
Nice work!
## Architectural designs
Follow the [Architectural designs](docs/design/ArchitecturalDesigns.md) document that describes and documents XCRemoteCache designs and implementation details.
Contributor
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.
👍 Minor - the graphs are hard to read for dark mode.
Collaborator
Author
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.
thanks for spotting that! Fixed it.
/// the critical section, the code realizes that remote cache cannot be used
/// (in practice - a new file has been added)
/// None of compilation process (so with '-c' args) can continue until the entire emit-module logic finishes
/// Because it is expected to happen no that often and emit-module is usually quite fast, this makes the
Contributor
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.
Suggested change
/// Because it is expected to happen no that often and emit-module is usually quite fast, this makes the
/// Because it is expected to happen not that often and emit-module is usually quite fast, this makes the
aleksandergrzyb
aleksandergrzyb
approved these changes
Jun 6, 2023
This file contains hidden or 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
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.
Uh oh!
There was an error while loading. Please reload this page.
This is the main PR of the Swift driver integration: synchronizes all swift-frontend invocations so emit-module (which is responsible to make the last check if the cached artifact can be used) is blocking
swift-frontent -c(compilation) invocations.Context
Because the design and implementation might not be obvious(relies on the swift driver integration in Xcode), they are documented in the markdown file (part of this PR), available to review here.
Previous PRs (#209 and #208)
Next steps
swift-frontentexecutables to the released .zip package^ All these missing parts will be added in Part IV (Draft)