path: root/blog/2021/10/09/meeting.mdwn

[[!meta title="Iteration planning: October 9th"]]
[[!tag meeting]]
[[!meta date="Sat, 09 Oct 2021 15:29:40 +0300"]]

[[!toc levels=2]]

# 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

[[!milestone 36]] had the following issued closed:

* [[!issue 196]] -- _Python runner stops on first failure_
* [[!issue 236]] -- _`codegen` on a zero-scenario document should be an_
  error

The following issues remain open:

* [[!issue 198]] -- _Unify language handling for multi-impl-lang test suites_
* [[!issue 207]] -- _Release process is too manual, CI build of release .deb changes files_
* [[!issue 218]] -- _Automatable parts of release process are manual_
* [[!issue 219]] -- _Needs a release after 0.3.0 to test release automation_
* [[!issue 238]] -- _Doesn't support `abstract` in document metadata_
* [[!issue 239]] -- _In metadata, `author` is a single string_

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

# Review of the repositories

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

- [[!mr 220]] -- _Draft: Support Nix-based and NixOS systems - plus
  fewer assumptions for Windows/MacOS_

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 [[!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:

- [[!issue 240]] -- _Could 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](https://en.wikipedia.org/wiki/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 [[!milestone 37]] to cover this iteration, with the
following issues:

- [[!issue 198]] -- _Unify language handling for multi-impl-lang test suites_
- [[!issue 207]] -- _Release process is too manual, CI build of release .deb changes files_
- [[!issue 218]] -- _Automatable parts of release process are manual_
- [[!issue 219]] -- _Needs a release after 0.3.0 to test release automation_
- [[!issue 238]] -- _Doesn't support `abstract` in document metadata_
- [[!issue 239]] -- _In metadata, `author` is a single string_

# 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.