RFC: Managing versions and long-pending changes

[ppannuto]

Pull Request Overview

This tries to write up a more complete summary of my thinking from #3585. As I was working on this, it bled over to a couple distinct concerns that arguably could be 4-5 separate RFCs, but these are all interrelated to a degree. I tried to keep the document reasonably organized which hopefully can facilitate logical threads of conversation.

Testing Strategy

N/A

TODO or Help Wanted

Your thoughts! And talk me off the ledge of cfg maybe being a good idea for something. I think I talked myself out of it already, but there was a real moment there where I was almost going to advocate for it.

---

Rendered

[bradjc]

This is duplicated from the next section.

[bradjc]

This section header doesn't seem to match the contents.

[bradjc]

Can this just move to the "Preparing a Release" section?

[bradjc]

CHANGELOG takes work, and it's work no one wants to do (as evidenced by it not getting updated). Perhaps eventually we hire someone who can help. But I don't think any amount of documenting is going to get the current set of core members interested in maintaining a changelog.

This is fairly on-the-nose, forgive me, but rather than creating this PR, why not direct your "service" tock time towards maintaining the changelog? You created it (a32a34d) but I don't think have touched it since 2018, and I think it's reasonable to say you are the main person who remembers it is there.

[bradjc]

This is a substantial con.

[bradjc]

Suggested change

	4. ...?
	4. Open a tracking issue for the change needed for the next release. Include the diff required to implement the change in the issue text.
	- Pro: Work to implement the change already done.
	- Pro: Because by definition this is a change to some stabilized portion of the kernel, the patch is likely to cleanly apply or be very close when the next release is prepared.
	- Con: Someone has to remember to address the tracking issue.

[bradjc]

This seems like the opposite of this section, as it would make it harder to know if your app relies on stable functionality or not.

[jrvanwhy]

EDIT: There is a summary below so you don't have to read all this.

I'm thinking through what the conditional compilation option would look like. I'll use the current state (current version is Tock 2.x but we want to carry 3.0 changes) as an example.

Idea 1: tock-3 cargo feature

In any crate that has 3.0, changes, we can add a tock-3 feature to Cargo.toml:

[features]
tock-3 = []

Then in the code, we can use that feature to control compilation:

#[cfg(not(feature = "tock-3"))]
fn foo_v2() {}

#[cfg(feature="tock-3"))]
fn foo_v3() {}

However, it then becomes difficult to switch to tock-3 without forgetting to clean the cfgs up. If you remove the feature without removing the #[cfg()] annotations, the above code will still build, and will pick the 2.0 version of foo!

Idea 2: tock-2 cargo feature

Instead, we can add a tock-2 feature to Cargo.toml. We'll have to make it a default feature, though, because otherwise everyone using our crates will forget to enable it:

[features]
default = ["tock-2"]
tock-2 = []

#[cfg(feature = "tock-2")]
fn foo_v2() {}

#[cfg(not(feature = "tock-2")]
fn foo_v3() {}

When we release Tock 3.0, we can remove the tock-2 feature from our Cargo.tomls entirely, and we'll end up with the v3 implementations of everything. We may still have some #[cfg()]s laying around, but they won't be doing any harm.

However, we have a different problem. When one of our crates depends on another one of our crates, it'll enable tock-2 by default. To fix that, we have to add a tock-2 feature to every one of our crates, specify default-features = false every time one of our crates depends on another one of our crates, and make our crate dependencies forward through the tock-2 feature. That is a lot of boilerplate, and we'll have to update it at every release!

Idea 3: Environment variable, build.rs, custom cfg key.

This idea abandons the use of cargo features, and instead uses a build script to perform the conditional compilation. In each crate with 3.0 changes, we would need a build.rs that does the following:

// Sets the tock2 cfg if the TOCK3 environment variable is NOT defined.
fn main() {
    println!("cargo:rerun-if-env-changed=TOCK3");
    if std::env::var_os("TOCK3").is_none() {
        println!("cargo:rustc-cfg=tock2");
    }
}

Then we can use cfg to control functionality as follows:

#[cfg(tock2)]
fn foo_v2() {}

#[cfg(not(tock2))]
fn foo_v3() {}

Then when we perform the v3 release, we can remove all the build.rs files, and it will continue to use the v3 functionality.

Downside: This may be really difficult for those of us using Bazel to deal with. Hopefully those users can just hardcode the --cfg=tock2 argument though?

[jrvanwhy]

EDIT: There is a summary below so you don't have to read all this.

Just came up with some more ideas.

Idea 4: Procedural macro

Add a new procedural macro crate that exports attribute macros for each version (tock2 and tock3). The tock2 macro would only build an item if TOCK3 is unspecified and the tock3 macro would only build an item if TOCK3 is set. Example (not-production-quality) implementation:

extern crate proc_macro;
use proc_macro::TokenStream;

// Builds the specified item if and only if TOCK3 is empty.
#[proc_macro_attribute]
pub fn tock2(_attr: TokenStream, item: TokenStream) -> TokenStream {
    match std::env::var_os("TOCK3") {
        None => item,
        Some(_) => Default::default(),
    }
}

// Builds the specified item if and only if TOCK3 is specified.
#[proc_macro_attribute]
pub fn tock3(_attr: TokenStream, item: TokenStream) -> TokenStream {
    match std::env::var_os("TOCK3") {
        None => Default::default(),
        Some(_) => item,
    }
}

In each crate that has Tock 3.0 changes, we would add a dependency on this procedural macro and use it as follows:

#[tock2]
fn foo_v2() {}

#[tock3]
fn foo_v3() {}

This has the advantage of working for users who do not use Cargo. Unlike the cfg-based options, the above code will stop compiling when the tock2 macro is removed, giving us a way to ensure that we don't accidentally use Tock 2.0 code after Tock 3.0 is released. One downside is we have to depend on this new crate from any Tock crate that has Tock 3.0 code, but I think that's a reasonable compromise.

Big downside: The build system will be oblivious to the fact our crates depend on the value of TOCK3, as procedural macros do not yet have a way to specify that they depend on an environment variable.

Idea 5: idea 4 + idea 3

We could have build.rs in the procedural macro crate read TOCK3 and set a cfg variable that is used by the macro implementations to control their behavior. That way, Cargo will know to rebuild the procedural macro crate if the value of TOCK3 changes...

Procedural macro build.rs:

fn main() {
    println!("cargo:rerun-if-env-changed=TOCK3");
    if std::env::var_os("TOCK3").is_some() {
        println!("cargo:rustc-cfg=tock3");
    }
}

Procedural macro lib.rs:

extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn tock2(_attr: TokenStream, _item: TokenStream) -> TokenStream {
    #[cfg(not(tock3))]
    return _item;
    #[cfg(tock3)]
    return Default::default();
}

#[proc_macro_attribute]
pub fn tock3(_attr: TokenStream, _item: TokenStream) -> TokenStream {
    #[cfg(not(tock3))]
    return Default::default();
    #[cfg(tock3)]
    return _item;
}

Usage:

#[tock2]
fn foo_v2() {}

#[tock3]
fn foo_v3() {}

This has the advantage of just Doing The Right Thing (building Tock 2.0 code) if someone builds the crate without invoking build.rs. We also only need to remember to clean up old #[cfg()] attributes in one place (the proc macro crate); if we leave any #[tock2] annotations around we'll get a compile error.

[jrvanwhy]

I realize I should summarize my previous comments, especially because this may be discussed at the next core WG call, which I do not expect to attend.

I thought through how we could implement "The cfg option" in Tock. There are some issues with using feature flags or custom rustc --cfg= arguments, but there is a viable option for implementing conditional compilation. The viable option is to introduce a (relatively simple) procedural macro crate that looks for the presence of an environment variable to determine whether to compile the current-release code or the next-release code. This option plays reasonably well with non-Cargo build systems and gives us a reliable way to clean up the old code once we've created a new major Tock release.

Truthfully, I think the conditional-compilation option is lower maintenance than the "keep PRs open until the next major release" and "always keep a next-major-release branch open" options. If we don't come up with any better ideas, then I would advocate for it.

[bradjc]

Idea 6: Diff in tracking issue

Open a tracking issue for the change needed for the next release. Include the diff required to implement the change in the issue text.

Pro:

• Work to implement the change already done.
• Because by definition this is a change to some stabilized portion of the kernel, the patch is likely to cleanly apply or be very close when the next release is prepared.

Con:

• Someone has to remember to address the tracking issue.

[bradjc]

My pushback to all of the automatic/cfg/etc. options is that we would effectively be maintaining two versions of the kernel (and giving users a [relatively] easy option to use either). I argue we have trouble keeping up with one version of the kernel, and don't want the inevitable issue: "I enabled the tock-3.0 flag and userspace doesn't work how do I fix it".