Skip to content

Read SwiftSyntax version for macro template from a configuration file #6774

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 28, 2023
Merged

Read SwiftSyntax version for macro template from a configuration file #6774

merged 3 commits into from
Sep 28, 2023

Conversation

neonichu
Copy link
Contributor

@neonichu neonichu commented Aug 3, 2023

In 5.9 we're using a version that's hard-coded in the template, but that is annoying to maintain. It also seems like ideally we would be using a version that's aligned with the toolchain that's being used. This adds a new configuration file that we can ship as 'usr/share/pm/config.json' in the toolchain to configure this. There's a fallback to the 5.9 version if we can't find the config and also a way to customize it when instantiating a UserToolchain in case a client needs to do that.

rdar://113287350

@neonichu neonichu requested a review from ahoppen August 3, 2023 00:39
@neonichu neonichu self-assigned this Aug 3, 2023
@neonichu
Copy link
Contributor Author

neonichu commented Aug 3, 2023

Draft since does not actually come with a config file yet and doesn't handle the installation of it.

ahoppen
ahoppen reviewed Aug 3, 2023
View reviewed changes
@neonichu neonichu force-pushed the macro-template-swift-syntax-version-from-toolchain branch from f4ed941 to 36c3619 Compare September 18, 2023 22:16
@neonichu neonichu marked this pull request as ready for review September 18, 2023 22:38
@neonichu
Copy link
Contributor Author

Should be ready now, but I wasn't able to test the installation locally because I am running into random CMake errors like:

/Users/neonacho/Projects/public/swift-driver/Sources/SwiftDriverExecution/llbuild.swift:24:29: error: module 'llbuildSwift' was created for incompatible target arm64-apple-macosx12.0: /Users/neonacho/Projects/public/swift-package-manager/.build/arm64-apple-macosx/llbuild/products/llbuildSwift/llbuildSwift.swiftmodule
@_implementationOnly import llbuildSwift
                            ^

@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu
Copy link
Contributor Author 

Traceback (most recent call last):
  File "/home/build-user/swiftpm/Utilities/bootstrap", line 817, in <module>
    main()
  File "/home/build-user/swiftpm/Utilities/bootstrap", line 65, in main
    args.func(args)
  File "/home/build-user/swiftpm/Utilities/bootstrap", line 397, in install
    install_swiftpm(prefix, args)
  File "/home/build-user/swiftpm/Utilities/bootstrap", line 436, in install_swiftpm
    install_dylib(args, "PackageDescription", dest, ["PackageDescription", "CompilerPluginSupport"])
  File "/home/build-user/swiftpm/Utilities/bootstrap", line 446, in install_dylib
    install_binary(args, g_shared_lib_prefix + library_name + g_shared_lib_suffix, install_dir)
  File "/home/build-user/swiftpm/Utilities/bootstrap", line 465, in install_binary
    install_file(args, src, destination, destination_is_directory=destination_is_directory, ignored_patterns=ignored_patterns)
  File "/home/build-user/swiftpm/Utilities/bootstrap", line 469, in install_file
    dest = os.path.join(destination, binary)
NameError: name 'binary' is not defined

This is an issue with the installation changes.

@neonichu
Read SwiftSyntax version for macro template from a configuration file
3324ef5 
In 5.9 we're using a version that's hard-coded in the template, but that is annoying to maintain. It also seems like ideally we would be using a version that's aligned with the toolchain that's being used. This adds a new configuration file that we can ship as 'usr/share/pm/config.json' in the toolchain to configure this. There's a fallback to the 5.9 version if we can't find the config and also a way to customize it when instantiating a `UserToolchain` in case a client needs to do that.

rdar://113287350
@neonichu neonichu force-pushed the macro-template-swift-syntax-version-from-toolchain branch from fb84040 to 8da0662 Compare September 20, 2023 00:19
@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu
Copy link
Contributor Author

@swift-ci please test windows

@neonichu neonichu force-pushed the macro-template-swift-syntax-version-from-toolchain branch from 8da0662 to a5f17fe Compare September 20, 2023 01:47
@neonichu
Copy link
Contributor Author

Hadn't actually pushed my changes 🫠

@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu
Copy link
Contributor Author

@swift-ci please test windows

@ahoppen ahoppen self-requested a review September 27, 2023 20:03
ahoppen
ahoppen approved these changes Sep 27, 2023
View reviewed changes
@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu
Copy link
Contributor Author

@swift-ci please test windows

@neonichu neonichu merged commit cf5facd into main Sep 28, 2023
@neonichu neonichu deleted the macro-template-swift-syntax-version-from-toolchain branch September 28, 2023 17:44
@stmontgomery stmontgomery mentioned this pull request Oct 18, 2023
Merged
stmontgomery added a commit to stmontgomery/swift-package-manager that referenced this pull request Oct 18, 2023
@stmontgomery
SwiftSyntax version in new macro template has trailing period in depe…
870fb95 
…ndency version, causing error reading manifest

This fixes a regression I believe was introduced in swiftlang#6774 where the new macro package template has its dependency on swift-syntax expressed as `from: "509.0.0."`, including an unncessary trailing period character. This causes an error reading the manifest.

I have not tested this, but I experienced the bug and from source inspection believe this is likely the issue.

Resolves rdar://117132800
neonichu pushed a commit that referenced this pull request Oct 18, 2023
@stmontgomery
SwiftSyntax version in new macro template has trailing period in depe…
55e79b1 
…ndency version, causing error reading manifest (#7018)

This fixes a regression I believe was introduced in #6774 where the new
macro package template has its dependency on swift-syntax expressed as
`from: "509.0.0."`, including an unncessary trailing period character.
This causes an error reading the manifest.

I have not tested this, but I experienced the bug and from source
inspection believe this is likely the issue.

Resolves rdar://117132800
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahoppen ahoppen ahoppen approved these changes

@tomerd tomerd Awaiting requested review from tomerd

@MaxDesiatov MaxDesiatov Awaiting requested review from MaxDesiatov

Assignees

@neonichu neonichu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@neonichu @ahoppen