[Returned for revision] SE-0303: Package Manager Extensible Build Tools

The review of SE-0303: Package Manager Extensible Build Tools has concluded and the core team has returned it for revision.

The feedback to idea of extending SwiftPM with plugins, the concrete design of the plugin system, and the tradeoffs the design make was very positive. Both authors and users of potential future plugins have provided input that the proposed design would work in real-world use cases, and that today require solutions outside of SwiftPM. However, there were a number of ideas that came up during the review that the core team feels would be good to adopt.

  1. The proposed package manifest syntax does not offer a structured way to configure the plugins added to the package. The underlying technical challenge is that in order to support type-safe configuration, the package manager will need to compile & load the plugins prior to be being able to parse the manifest (since its Swift code), causing a chicken-and-egg issue.

    A more complete solution would be to restructure the package manifest to multiple top level "stanzas" (or even multiple files): the plugins "build" and the application "build", which can then drive a two pass build solving the chicken-and-egg problem. Such solution will be significantly more complicated to implement, and requires changes to the build system.

    Some reviewers suggested using a non type-safe configuration (e.g. a dictionary) to work around the issue, but this is likely the worst of both world: compromises on user experience and type-safety and paints the API into a corner given SwiftPM’s backwards compatibility requirement.

    The solution the authors picked was to make this a plugin concern - in practice a plugin has access to the file system and as such can ask users to define configuration via a configuration file following some naming convention, etc.

    Given that the more complete two-pass build solution would not change the core plugin API, the core team felt the proposal makes the right trade-off, but would like to see more details on this topic in the "future directions" section.

  2. Technically related to the above, several reviewers pointed out that this having a single dependencies stanza implies the plugins and the package share the same version of the dependencies which could lead to undesirable revlock side-effects. Here too, the more complete solution is to restructure the package manifest to multiple top level "stanzas" (or even multiple files), as described above.

    Similarly, given that the more complete two-pass build solution would not change the core plugin API, the core team felt the proposal makes the right trade-off, but would like to see more details on this topic in the "future directions" section.

  3. The proposal describes that plugins gain access to the file system using simplistic file system abstraction, namely array of FilePath (from SwiftSystem). Bazel's depset was mentioned as an alternative for building up input file lists and the arguments for constructing shell commands, which seems like an interesting alternative. Several reviewers pointed our that the plugins API should provide something more useful, for example the type/category of the files, and some filtering/globbing capabilities to make the plugin authors job easier.

    The core team felt this is one area where the proposal can be improved and encourages the authors to explore the ideas suggested. The proposal should also explicitly call out any future ideas for this API in the "future directions" section.

  4. The proposed API does not provide contextual information about the target platform, which limits the API usefulness for building platform-specific plugins.

    The proposal authors deliberately left this feature for future iterations to control the scope of initial design and implementation. They felt it can be added later, and should not block progress in supporting useful use cases that do no required this capability.

    The core team agreed this can be a good future addition and as such would like to see more details on this topic in the "future directions" section.

  5. The proposal describes 3 initial plugin types: prebuild, postbuild, and buildTool, defined as the following:

    prebuild: runs before the build (like a script you run before the build), designed for plugins that do not know the exact inputs/outputs, but may be inefficient because of that (e.g. incremental builds)

    postbuild: runs after the build was complete (like a script you run after the build), designed for plugins that do not know the exact inputs/outputs, but may be inefficient because of that (e.g. incremental builds)

    buildTool: runs as part of the build, designed for plugins that can enumerate the inputs and outputs, runs as part of the build graph efficiently.

    Out of the three, postbuild received the least attention and seems to be the least useful. Many reviewers in both the pitch and proposal has a hard time grokking the difference between prebuild and buildTool and the trade-offs of picking one over the other.

    The core team felt the proposed design is not clear enough in that respect, leading to such confusion. It also made the observation that the separation between prebuild and buildTool reflects current limitation of the underlying build tools, not the desired API for when such constraints are lifted. As such, the core team would like to see this area of the design further refined, including:

    a. Consider removing postbuild given that its utility is unclear.

    b. Consider merging prebuild and buildTool into a single, intuitive API, or provide better guidance on how to choose one over the other.

12 Likes

Postbuild could be used for packaging or dsym upload

4 Likes

IT can be used for artifact uploads OR notifications. Please do not remove it.

3 Likes

Wrt. points 1 and 2: One solution I imagine could work quite nicely here, and would be nice even without this particular motivation, would be to compose with the upcoming script support and allow manifests to declare build-dependencies together with the import stanzas.

This would, I think, lead to a model that is quite understandable to the user, and apart from allowing type-safe configuration for build plugins it could allow common patterns seen in package files to be pulled into libraries.

This is, of course, a feature a little bigger than something focused on just build plugin config, but it's such a natural composition of features I think I quite like the idea.

From a quick check of a bunch of projects I have, postBuild can be used to:

  • Strip extraneous architectures of embedded frameworks (yeah that's still needed in some cases)
  • Update version number in a few plist files (app Info.plist but also app extensions and symbol files)
  • Upload symbols files to crash reporting system

IMO these are build script tasks and not a build phase. The tasks you're describing will fit perfectly fine in a travis CI configuration.
I think the problem they are trying to solve here is for tools that are required for compilation. Think of tools similar to clang.

In my projects I need to precompile resources.

The proposal states access to the package source would be read only. This means the plugins won’t work with the spm resources feature and will instead require manipulating the bundles in the build directory. Bundles aren’t the same across platforms and could change over time, so that would make the tools overly complex.

I currently use a command line tool to compile resources into the package source and then let spm do its thing. It would be nice if this feature allowed me to do it all in package.

An example would be turning Metal shaders into a Metal Library.
I need to do this with directX as well as convert 3D geometry, images, and audio to various formats.

The spm resources feature has an option for processing resources. Perhaps that could be rolled into this? A resourceTransform plugin or something like that. Just some thoughts. I’ll try to bring this up for the next revision.

Do you think it's feasible to make it into Xcode 13 with this feature?

1 Like

Any .metal shader in your swift package automatically goes into default.metallib (considered as a processed resources when built with xcodebuild / Xcode)

I do believe however that it would be able to customise this since at the moment to have .ci.metal shader files that link against core-image and regular metal shader files, you need a custom Build Rule in Xcode, this is a very specific case that's currently limiting

1 Like