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

Use lib_InternalSwiftSyntaxParser resolved by SPM #223

Merged
merged 1 commit into from
Mar 15, 2023

Conversation

fummicc1
Copy link
Collaborator

@fummicc1 fummicc1 commented Mar 13, 2023

Overview

One of the install option; downloading mockolo.tar.gz does not work if we generate mockolo binary withlib_InternalSwiftSyntaxParser which is placed inside Xcode toolchain.

I think there is no need to copy lib_internalSwiftSyntaxParser.dylib from xcode toolchain and we can just use one resolved by running swift build.

please feel free to correct me if there is a better way.

Issue

@fummicc1 fummicc1 changed the title Use lib_InternalSwiftSyntaxParser prepared by swift-syntax Use lib_InternalSwiftSyntaxParser resolved by SPM Mar 14, 2023
@fummicc1 fummicc1 marked this pull request as ready for review March 14, 2023 15:23
cp "$(xcode-select -p)"/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/macosx/lib_InternalSwiftSyntaxParser.dylib .
if [[ ! -e ./lib_InternalSwiftSyntaxParser.dylib ]]; then
echo "** Copy lib_InternalSwiftSyntaxParser.dylib from Xcode toolchain..."
cp "$(xcode-select -p)"/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/macosx/lib_InternalSwiftSyntaxParser.dylib .
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this fallback necessary?
In almost all cases, this will not work and the original issue will be hidden.
This script should fail here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your review.

Is this fallback necessary?

In my understanding, if swift version is under 5.6, lib_InternalSwiftSyntaxParser will not be prepared in resolving by SPM.
so in case user's swift environment is under swift 5.6, I didn't delete option to copy from xcode toolchain.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my understand, this script is used for distribution.

For legacy swift user, artifacts exist in release page. Also they can build mockolo theirself ( this is guaranteed by https://github.com/uber/mockolo/blob/master/.github/workflows/builds.yml )
Some escape hatches exist so not needs to consider backward compatibility I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you are correct. install-script.sh is not an install option but distribution way.
I removed the fallback.

Copy link
Collaborator

@sidepelican sidepelican left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@sidepelican sidepelican merged commit dd5e7e7 into uber:master Mar 15, 2023
@fummicc1 fummicc1 deleted the fix-issue-219 branch March 15, 2023 12:25
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.

Xcode14.2 Fatal error: The operation couldn’t be completed. (SwiftSyntaxParser.ParserError error 1.)
2 participants