Skip to content

[CMake] Replace early swift-syntax with FetchContent #68408

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

Merged
merged 13 commits into from
Sep 28, 2023

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Sep 8, 2023

Use FetchContent to include swift-syntax directly in swift. This can be thought of as an add_subdirectory for a directory outside the root.

The default build directory will be _deps/swiftsyntax-subbuild/, though the modules and shared libraries will be built in lib/swift/host by passing down SWIFT_HOST_LIBRARIES_DEST_DIR to avoid copying them as we were doing previously.

@rintaro
Copy link
Member Author

rintaro commented Sep 8, 2023

@rintaro
Copy link
Member Author

rintaro commented Sep 8, 2023

@rintaro
Copy link
Member Author

rintaro commented Sep 11, 2023

@rintaro
Copy link
Member Author

rintaro commented Sep 12, 2023

@finagolfin
Copy link
Member

Spurred by #68401, I started looking into cross-compiling swift-syntax for the native Android compiler, only to find early-swift-syntax doesn't support cross-compilation. With a tweak or two to this pull, I think it can be made to work: is that the plan?

Because I don't know about macOS, but the recent shipping of swift-syntax with the linux Swift compiler means that cross-compiling the toolchain is currently broken on Unix platforms, ie swift-syntax won't be cross-compiled.

Also, any plans to backport this pull to 5.9?

@rintaro
Copy link
Member Author

rintaro commented Sep 12, 2023

@rintaro
Copy link
Member Author

rintaro commented Sep 12, 2023

@rintaro
Copy link
Member Author

rintaro commented Sep 13, 2023

@rintaro
Copy link
Member Author

rintaro commented Sep 16, 2023

@rintaro
Copy link
Member Author

rintaro commented Sep 18, 2023

@rintaro
Copy link
Member Author

rintaro commented Sep 18, 2023

bnbarham and others added 9 commits September 18, 2023 14:44
Use FetchContent to include swift-syntax directly in swift. This can be
thought of as an `add_subdirectory` for a directory outside the root.

The default build directory will be `_deps/swiftsyntax-subbuild/`, though
the modules and shared libraries will be built in `lib/swift/host` by
passing down `SWIFT_HOST_LIBRARIES_DEST_DIR` to avoid copying them as we
were doing previously.
And pass the value to 'swift-syntax', so they gets correct deployment
target.
This used to overwrite the RUNPATH when copying libraries from
earlyswiftsyntax directory. Since earlyswiftsyntax is now replaced with
FetchContent, we don't need this anymore
So that swift-syntax host libraries can be installed independently.
'--install-swift-syntax' is now a sugar of '--install-swift
--swift-install-component=swift-syntax-lib'
@rintaro
Copy link
Member Author

rintaro commented Sep 18, 2023

@rintaro
Copy link
Member Author

rintaro commented Sep 18, 2023

1 similar comment
@rintaro
Copy link
Member Author

rintaro commented Sep 19, 2023

@rintaro
Copy link
Member Author

rintaro commented Sep 20, 2023

@finagolfin

Spurred by #68401, I started looking into cross-compiling swift-syntax for the native Android compiler, only to find early-swift-syntax doesn't support cross-compilation. With a tweak or two to this pull, I think it can be made to work: is that the plan?

The main motivation of this PR is that single cmake --build /path/to/swift-build-dir invocation rebuilds swift-syntax libraries if modified. But I'm aware of that the earlyswiftsyntax didn't support cross compiling, and this PR may make it easier. I'd appreciate it if you can check (and fix) it after we merge this PR.

Also, any plans to backport this pull to 5.9?

I cannot promise anything, but I personally want this in 5.9 too.

MACCATALYST_BUILD_FLAVOR ""
DEPLOYMENT_VERSION "${DEPLOYMENT_VERSION}")

swift_get_host_triple(target)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this just use SWIFT_HOST_TRIPLE?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

@finagolfin
Copy link
Member

finagolfin commented Sep 20, 2023

But I'm aware of that the earlyswiftsyntax didn't support cross compiling, and this PR may make it easier. I'd appreciate it if you can check (and fix) it after we merge this PR.

I got the flags added locally to cross-compile this and the regex parser, will submit that once this is in.

I've also backported this to 5.9, along with #68401. Since that adds more checks of SWIFT_SWIFT_PARSER, whichever one of these two pulls goes in first will mean the second will have to be updated.

* Move the logic to right before include(lib) because swift-syntax is a
  "lib"
* Use function to limit the scope of variables for FetchContent
* Use standard 'CMAKE_*_OUTPUT_DIRECTORY' instead of custom
  variable names.
@ahoppen ahoppen removed their request for review September 21, 2023 00:30
Instead of calling 'get_swift_host_triple()' repeatedly, because it
always return the same value.
The argument list handling was a hack and confusing.
@rintaro
Copy link
Member Author

rintaro commented Sep 21, 2023

@rintaro
Copy link
Member Author

rintaro commented Sep 21, 2023

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

🥳

@rintaro
Copy link
Member Author

rintaro commented Sep 21, 2023

@rintaro
Copy link
Member Author

rintaro commented Sep 21, 2023

@xedin xedin removed their request for review September 27, 2023 01:18
@rintaro rintaro merged commit 8dbde04 into swiftlang:main Sep 28, 2023
rintaro added a commit to rintaro/swift that referenced this pull request Sep 29, 2023
[CMake] Replace early swift-syntax with FetchContent

(cherry picked from commit 8dbde04)

 Conflicts:
	lib/Frontend/PrintingDiagnosticConsumer.cpp
	lib/Parse/ParseType.cpp
rintaro added a commit to rintaro/swift that referenced this pull request Sep 29, 2023
[CMake] Replace early swift-syntax with FetchContent

(cherry picked from commit 8dbde04)
rintaro added a commit to rintaro/swift that referenced this pull request Oct 2, 2023
[CMake] Replace early swift-syntax with FetchContent

(cherry picked from commit 8dbde04)

 Conflicts:
	lib/Frontend/PrintingDiagnosticConsumer.cpp
	lib/Parse/ParseType.cpp
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.

3 participants