Skip to content

Move SwiftSyntaxBuilderGeneration to separate package that pins a version of swift-syntax #580

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

Closed
wants to merge 3 commits into from

Conversation

fwcd
Copy link
Member

@fwcd fwcd commented Aug 11, 2022

This PR addresses #577 by moving SwiftSyntaxBuilderGeneration to a separate package, i.e. replaces the current package structure

swift-syntax
└─ Sources
   ├─ SwiftSyntaxBuilder
   ├─ SwiftSyntaxBuilderGeneration
   └─ ...

with

swift-syntax
├─ Sources
│  ├─ SwiftSyntaxBuilder
│  └─ ...
└─ SwiftSyntaxGeneration
   └─ Sources
      └─ swift-syntax-builder-generation

The subpackage pins a specific commit of swift-syntax as a dependency and thereby is no longer susceptible to the bootstrapping problems outlined in #577 when adding new syntax nodes.

Things to figure out:

  • The build script, in particular how it has to be updated
  • How this plays with CI and its workspaces

Therefore I will mark this PR as a draft for now, so we can discuss these things here (and whether it's a good approach to take at all).

cc @ahoppen @CodaFi

let package = Package(
name: "SwiftSyntaxGeneration",
products: [
.executable(name: "swift-syntax-builder-generation", targets: ["swift-syntax-builder-generation"]),
Copy link
Member Author

Choose a reason for hiding this comment

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

Minor note: The reason for renaming SwiftSyntaxBuilderGeneration to swift-syntax-builder-generation (kebab-case) is twofold:

  1. Until the pinned revision of swift-syntax no longer has a target named SwiftSyntaxBuilderGeneration the name will conflict with this target
  2. Most Swift repos follow the convention of naming executable targets in kebab-case (e.g. this repo's lit-test-helper, SourceKit-LSP's sourcekit-lsp, SwiftFormat's swift-format, ...). Therefore this would be good opportunity to update the name.

@fwcd fwcd force-pushed the separate-generation-package branch from 84133a8 to d4c7e19 Compare August 11, 2022 22:39
@ahoppen
Copy link
Member

ahoppen commented Aug 12, 2022

I don’t think this is necessary. We just need to make code generation a three-step instead of a two-step process, which I’m doing in #583.

@ahoppen
Copy link
Member

ahoppen commented Aug 12, 2022

I think renaming SwiftSyntaxBuilderGeneration to swift-syntax-builder-generation would still be a change that’s worthwhile.

@ahoppen
Copy link
Member

ahoppen commented Aug 23, 2022

The underlying build issue has been fixed by #583

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.

2 participants