Support building swift-format as part of Swift #207
Conversation
There was a problem hiding this comment.
I'm not sure what is the best way to document the fact that master branch now depends on the latest build of Swift as opposed to a specific snapshot. I updated the README to note that the latest snapshot is necessary, but let me know if you think of a better way to document that.
I wonder if that table cell should just say "Swift at master". "Latest development snapshot" will be true most of the time, except in cases where someone merges a breaking change to apple/swift master that propagates down to apple/swift-syntax and apple/swift-format. In those cases, someone would have to build against a master checkout until a new toolchain snapshot is available (usually within a day, but could be delayed if there are other build breakages).
Otherwise this looks good to me, but I'm gonna hand it off to @akyrtzi to really review it since he's more familiar with these build scripts. 😄
That sounds like a good idea to me. I updated it. |
|
@benlangmuir could you take a look at the build-script changes? |
Package.resolved
Outdated
| "version": "0.0.4" | ||
| "branch": "master", | ||
| "revision": "eb51f949cdd0c9d88abba9ce79d37eb7ea1231d0", | ||
| "version": null |
There was a problem hiding this comment.
Now that your dependencies are on branches rather than tags/versions, does it still make sense to have Package.resolved file checked in? Our CI system will build this against ToT for each dependency, but if a user checks it out or uses it as a dependency they will get whatever version is stamped here. That seems like it will cause mismatches.
There was a problem hiding this comment.
@allevato Is it safe to delete this?
There was a problem hiding this comment.
Yeah, it should be deleted and added to .gitignore. Swift Package Manager will regenerate it when it does the checkout, but if it exists already it will be used, so we don't want wrong revisions to leak to users.
There was a problem hiding this comment.
Please make sure to add Package.resolved to .gitignore as well.
There was a problem hiding this comment.
Overlooked that, thanks.
build-script-helper.py
Outdated
| def parse_args(args): | ||
| parser = argparse.ArgumentParser(prog='BUILD-SCRIPT-HELPER.PY') | ||
|
|
||
| parser.add_argument('--package-dir', default='') |
There was a problem hiding this comment.
Please use --package-path to match swift build as well as other build-script-helper scripts such as sourcekit-lsp.
There was a problem hiding this comment.
I've renamed this param and the others that you flagged.
Just FYI - these names were taken from an existing build-script-helper.py, so you or someone else may want to rename the params on that script for consistency.
There was a problem hiding this comment.
Just FYI - these names were taken from an existing build-script-helper.py, so you or someone else may want to rename the params on that script for consistency.
/cc @nathawes
…syntax. The build script will be used by the Swift project's build system to build & test swift-format, and eventually install it into the Swift toolchain. This script is based on other Swift projects (e.g. swift-syntax and source-kit-stress-tester. Local deps allow building swift-format based on a local checkout of swift-syntax, instead of a tagged or released version from GitHub. This allows building and testing swift-format against a true HEAD version of all other Swift projects.
* Add CI job for macOS 11 (Big Sur) * Add bottle job for macOS Big Sur in publish workflow
There are 3 changes to support building swift-format as part of the Swift project:
When developing just for swift-format, it's still possible to use a recent DEV snapshot of the Swift toolchain to build and test without using local checkouts of the dependencies. That is the default behavior used by the Swift PM package (see the dependencies added with and without the
SWIFTCI_USE_LOCAL_DEPSenv var.I ran into some trouble when using
multiroot-data-filefor Swift PM when combining swift-format with the other swift-syntax dependent projects, because the other projects rely on test discovery. Test discovery seems to work fine for swift-format, so I've enabled it when using the build script. TheLinuxMaintesting support file is preserved because other existing workflows rely on that file to test on Linux when test discovery isn't supported.This follows the instructions for incorporating swift-format into the Swift project's CI system:
https://forums.swift.org/t/testing-swift-format-as-part-of-swift-s-ci-infrastructure/30619
Open Question
I'm not sure what is the best way to document the fact that
masterbranch now depends on the latest build of Swift as opposed to a specific snapshot. I updated the README to note that the latest snapshot is necessary, but let me know if you think of a better way to document that.