Review of actions from last meeting

We carried over this action: Lars to create a label for blocking-high-profile-projects (name to be decided by Lars when he does it) and to label at least the Sequoia-PGP blockers if not also anything else he thinks may be blocking projects such as Rustup.

Review of the iteration that has ended

Subplot milestone 36 had the following issued closed:

The following issues remain open:

We moved the remaining issues, next milestone (Subplot milestone 37) and then closed off this iteration.

Review of the repositories

There is one in-progress MR in the subplot repository:

The subplot-container-images and subplot-web repositories do not have MRs or extra branches. The subplot repository has extra branches:

  • build.rs-support – a prototype for adding support to Subplot for using from another project’s build.rs by adding a function that does codegen and outputs Cargo build instructions

  • subplot-rust – Daniel is keeping this around until he has completed the work on Subplot issue 198.

We triggered the pipeline to build the container image for Subplot. All subplot pipelines are clean.

Current goal (goal 2; not changed for this iteration)

Subplot provides a set of libraries with identical capabilities in each of the supported languages. Python remains a supported language. Rust is promoted to supported-language status. Subplot will be tested with all supported languages. In addition, any quality of life improvements which can be done shall be done. This goal will be considered complete when a release of Subplot has been made with the unified language handling support complete.

Issue review

This time we reviewed only the recently changed open issues, in order to have more time and energy for discussions. We updated the issues as we reviewed with any new comments or decisions. Notably we made the following changes:

  • Subplot issue 240Could we mangle generated scenario function names in the Rust template?
    • Lars will try the suggestion, then comment on the task

Other discussion

Minimum supported Rust version (MSRV) for Subplot

The Sequoia project currently specifies an MSRV of 1.48.0, which is currently too old for Subplot. We discussed if Subplot should specify an MSRV of its own, and made the following observations:

  • If we decide on an MSRV, we should use the rust-toolchain approach to enforce it.
  • We probably want to build and test with both our MSRV and current Rust stable version.
  • We want it to be easy and convenient for other projects to use Subplot. When that happens by running the Subplot binary, it’s easy, but for using Subplot as a library, such as via the other project’s build.rs module, we need to work with their MSRV.
  • Currently this means that the subplotlib crate needs to work with older versions of the Rust toolchain.

We decided on the following:

  • The Subplot project supports the MSRV of the other major projects who depend on Subplot, for the parts of Subplot that are meant to be used as libraries from those other projects. Currently that means the Sequoia-PGP project. The Subplot project will consider other projects on a case-by-case basis and update this decision as needed.

We should do the following:

  • Lars and/or Daniel to add a Docker image for CI with Rust 1.48.0 installed, or add it to the main image.
  • Lars and/or Daniel to change .gitlab-ci.yml to build and test using the 1.48.0 toolchain.
  • Lars and Daniel to discuss whether to change ./check to take an option to specify which toolchain to use or to keep that outside of the script.

Using Subplot from other projects’ build.rs

Lars made a prototype for adding support for using Subplot codegen from the Sequoia-PGP sq build.rs module. In short, Subplot will provide a function that build.rs can call to handle all aspects of code generation. Daniel is OK with the conceptual, but has suggestions for doing it better. After discussion:

  • The codegen_buildrs function in the prototype calls panic! in an error situation. It should return an Err value instead.
  • The modified get_basedir_from function needs tests. The original function also needed them. If the changes fix an actual issue that affects the original, Lars should file an issue.
  • A comment is needed to explain why the add_search_path function needs to be called, when the original code didn’t need it.

Daniel suggests the following structure:

  • A new crate, subplot_build, is added for the code in Subplot needed to support use from build.rs, so that the new crate can make stronger interface stability guarantees than the rest of Subplot. The new crate can also be documented with a more targeted audience, who won’t care about most of Subplot.

We also discussed an alternative:

  • The subplot crate is changed to offer a smaller, more stable interface, meant to be used from other programs, as well as the subplot and subplot-filter binaries. The rest of the code would be moved to a new crate, subplot_internal.

The subplot_build crate is the smaller crate and does not preclude adopting the alternative later. Thus we decided to implement the subplot_build_ crate now.

Clearing the environment in test programs, and NixOS/Windows/macOS support

Daniel has started porting Subplot to NixOS, and it has overall been quite easy. However, the environment cleanup that the Subplot runnier does makes things difficult. NixOS does not follow the usual file system hierarchy of Linux distributions, and one aspect of this is that the PATH environment variable varies depending on what is installed on a system. Daniel pointed out that similar issues will probably arise when Subplot is ported to Windows or macOS.

Lars said that the environment cleanup is an attempt to make the test program run without being accidentally affected by unusual features in the environment it runs in. However, he points out that just cleaning up environment variables is not enough. Proper isolation of tests needs much more work than fiddling with environment variables, but containers or virtual machines is definitely outside the scope of Subplot.

After discussion we decided to drop environment cleanup and document for Subplot users that providing the environment in which the test program is run is the user’s responsibility.

Document metadata parsing

When Lars changed codegen to parse Markdown using the pulldown-cmark crate instead of Pandoc, he made the way the document YAML metadata is parsed much stricter. Pandoc basically accepts any valid YAML. Being strict means not accepting some things that Pandoc does, and, worse, we can’t know what Pandoc may make use, as it depends on the typesetting backend.

We discussed this, and came to the conclusion that we prefer to make a breaking change now, rather than later, and we prefer the ability to notice metadata problems early, and to be able to give better errors about them. Thus, we’ll change Subplot to accept a strictly defined set of metadata, but allow an arbitrary mapping YAML object in a pandoc: field in the metadata. When we construct the input to Pandoc for typesetting, we’ll add in the metadata from the pandoc: field.

Plan for next iteration

We opened Subplot milestone 37 to cover this iteration, with the following issues:

Other business

  • We are not yet ready to file an RFP bug to get Subplot packaged for Debian. It will happen after we think we won’t be making breaking changes anymore.

  • We are not yet ready to make a whole code base review of Subplot.

  • We will switch an issue based agenda when other people join the meeting.

  • We are still interested in offering a talk about Subplot to FOSDEM, and will start gathering ideas. We may create a media repository for this.

  • We are not moving on cargo release yet.

  • We discussed Subplot vs mdbook. We may some want to support the mdbook input files, or implement a Subplot backend for mdbook.

  • We are going to try drive adoption of Subplot by helping other projects use it: Sequoia-PGP and rustup, and possibly others.

Actions

  • Lars to create a label for blocking-high-profile-projects (name to be decided by Lars when he does it) and to label at least the Sequoia-PGP blockers if not also anything else he thinks may be blocking projects such as Rustup.

  • Lars to record decision about MSRV in DECISIONS.md in the subplot repository.

  • Lars and/or Daniel to add a Docker image for CI with Rust 1.48.0 installed, or add it to the main image.

  • Lars and/or Daniel to change .gitlab-ci.yml to build and test using the 1.48.0 toolchain.

  • Lars and Daniel to discuss whether to change ./check to take an option to specify which toolchain to use or to keep that outside of the script.

  • Lars to refactor the changes for build.rs support, as discussed, and push an MR for it.

  • Lars should file an issue about the get_basedir_from changes, if the problem can be reproduced with the original code.

  • Lars to document decision that it’s the user’s responsibility to set up the environment in which tests are run.