Skip to content

Several changes in build-script-helper.py to support building SourceKit-LSP in a unified build #597

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 8 commits into from
Aug 3, 2022

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Jul 29, 2022

This PR contains a couple of small changes that are necessary to build SourceKit-LSP in a unified SwiftPM build, removing the need to build indexstore-db twice (once for indexstore-db testing and once for SourceKit-LSP testing).

  • Don’t clean build directory prior to building
    • AFAICT cleaning the build directory prior to building was done to work around a bug in SwiftPM at some point. I don’t think that it’s needed anymore.
  • Explicitly build all targets
    • When we build SourceKit-LSP in a unified SwiftPM workspace, a simple swift build invocation will build all products in the unified workspace (not just SourceKit-LSP products).
  • Add a --multiroot-data-file option to build sourcekit-lsp in a unified build
  • Print environment variables that were specified when executing commands
    • Just some logging improvement
  • Add flake8 configuration
    • I formatted the build script according to these rules, maybe we can keep that formatting in the future
  • Don’t pass -no-toolchain-stdlib-rpath when installing sourcekit-lsp
  • Ignore stderr when retrieving SwifitPM’s bin path
    • I just noticed that since Improvements to build-script.py #598 the bin-path computation was wrong because the returned bin path included warnings about --build-path being deprecated in favor of --scratch-path
  • Rename --build-path to --scratch-path in SwiftPM invocation
    • --build-path has been deprectated in favor of --scratch-path in SwiftPM.

AFAICT cleaning the build directory prior to building was done to work around a bug in SwiftPM at some point. I don’t think that it’s needed anymore.
@ahoppen ahoppen changed the title WIP: Build sourcekit-lsp in a unified build Several changes in build-script-helper.py to support building SourceKit-LSP in a unified build Aug 3, 2022
@ahoppen ahoppen marked this pull request as ready for review August 3, 2022 06:00
@ahoppen ahoppen requested a review from benlangmuir as a code owner August 3, 2022 06:00
@ahoppen ahoppen requested a review from bnbarham August 3, 2022 06:00
@ahoppen
Copy link
Member Author

ahoppen commented Aug 3, 2022

@swift-ci Please test

ahoppen added 7 commits August 3, 2022 19:12
When we build SourceKit-LSP in a unified SwiftPM workspace, a simple `swift build` invocation will build all products in the unified workspace (not just SourceKit-LSP products).
Since swiftlang/swift-package-manager#4208 SwiftPM no longer adds rpath to the toolchain libraries, so we don’t need to exclude them.
`--build-path` has been deprectated in favor of `--scratch-path` in SwiftPM.
@ahoppen
Copy link
Member Author

ahoppen commented Aug 3, 2022

@swift-ci Please test

@ahoppen ahoppen merged commit 4f182dc into swiftlang:main Aug 3, 2022
@ahoppen ahoppen deleted the pr/unified-build branch August 3, 2022 19:28
finagolfin added a commit to finagolfin/sourcekit-lsp that referenced this pull request Feb 20, 2023
Pull swiftlang#597 incorrectly removed this, as it's still needed on ELF platforms.
finagolfin added a commit to finagolfin/sourcekit-lsp that referenced this pull request Feb 20, 2023
Pull swiftlang#597 incorrectly removed this, as it's still needed on ELF platforms.
finagolfin added a commit to finagolfin/sourcekit-lsp that referenced this pull request Mar 6, 2023
Pull swiftlang#597 incorrectly removed this, as it's still needed on ELF platforms.
finagolfin added a commit to finagolfin/sourcekit-lsp that referenced this pull request Mar 10, 2023
…linux

Pull swiftlang#597 incorrectly removed this, as it's still needed on ELF platforms.
finagolfin added a commit to finagolfin/sourcekit-lsp that referenced this pull request Mar 11, 2023
…linux

Pull swiftlang#597 incorrectly removed this, as it's still needed on ELF platforms.
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