Reduce use of AbsolutePath with precondition failure
#5797
Merged
Conversation
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
neonichu
force-pushed
the
reduce-use-of-absolutepath-with-precondition-failure
branch
from
ef50e94 to
f68a78a
Compare
|
@swift-ci please smoke test |
|
Moved to draft since I'll be addressing the remaining call sites as well as discussed. |
neonichu
added a commit
to neonichu/swift-tools-support-core
that referenced
this pull request
Oct 14, 2022
neonichu
added a commit
to neonichu/swift-tools-support-core
that referenced
this pull request
Oct 17, 2022
neonichu
added a commit
to neonichu/swift-tools-support-core
that referenced
this pull request
Oct 17, 2022
neonichu
added a commit
to neonichu/swift-tools-support-core
that referenced
this pull request
Oct 17, 2022
neonichu
added a commit
to neonichu/swift-tools-support-core
that referenced
this pull request
Oct 18, 2022
neonichu
force-pushed
the
reduce-use-of-absolutepath-with-precondition-failure
branch
from
f68a78a to
9a04192
Compare
neonichu
added a commit
to neonichu/swift-tools-support-core
that referenced
this pull request
Oct 18, 2022
neonichu
force-pushed
the
reduce-use-of-absolutepath-with-precondition-failure
branch
from
9a04192 to
1c9a81e
Compare
|
swiftlang/swift-tools-support-core#353 |
Since it comes up regularly as a perceived user facing crash, we should reduce our use of the plain `AbsolutePath` initializer. I actually don't think it makes sense to even provide a non-throwing initializer that takes anything but a `StaticString` and even that's debatable. This doesn't resolve all possible uses and specifically excludes uses in tests, but does fix several user facing issues, e.g. ``` ❯ export SWIFTPM_TESTS_MODULECACHE=blah ❯ swift test zsh: illegal hardware instruction swift test ```
neonichu
force-pushed
the
reduce-use-of-absolutepath-with-precondition-failure
branch
from
1c9a81e to
136ffef
Compare
|
swiftlang/swift-tools-support-core#353 |
|
Self-hosted jobs are expected to fail because they don't support cross-repo testing. |
tomerd
reviewed
Oct 18, 2022
tomerd
approved these changes
Oct 18, 2022
tomerd
reviewed
Oct 18, 2022
|
swiftlang/swift-driver#1204 |
neonichu
added a commit
to neonichu/swift-tools-support-core
that referenced
this pull request
Oct 18, 2022
neonichu
added a commit
to swiftlang/swift-tools-support-core
that referenced
this pull request
Oct 19, 2022
neonichu
deleted the
reduce-use-of-absolutepath-with-precondition-failure
branch
neonichu
added a commit
that referenced
this pull request
Nov 4, 2022
In #5797, I introduced the outer `do`-block here, but did not move the existing `defer` from the now inner `do`-block. Related to rdar://101956252
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.
Since it comes up regularly as a perceived user facing crash, we should reduce our use of the plain
AbsolutePathinitializer. I actually don't think it makes sense to even provide a non-throwing initializer that takes anything but aStaticStringand even that's debatable.This doesn't resolve all possible uses and specifically excludes uses in tests, but does fix several user facing issues, e.g.