Release analyzeme, decodeme, and measureme to crates.io

[weihanglo]

During the course of vendoring rustc-perf in rust-lang/rust. There were a couple of issues found:

• analyzeme and decodeme haven't yet published to crates.io.
• analyzeme depends on old versions of measureme via git branches. Those versions are not available on crates.io.

While it is possible to tweak rustc bootstrapping process to allow Git dependencies, it might be under the risk that a commit is missing or a branch points to a different commit.

Proposed solution

The current rust-lang/rustc-perf@892c681 depends on these packages:

└── analyzeme v11.0.0 (https://github.com/rust-lang/measureme?branch=stable#f2914503)
    ├── analyzeme v9.2.0 (https://github.com/rust-lang/measureme?tag=9.2.0#9f51cde2)
    │   ├── measureme v9.2.0 (https://github.com/rust-lang/measureme?tag=9.2.0#9f51cde2)
    ├── decodeme v10.1.2 (https://github.com/rust-lang/measureme?tag=10.1.2#f9f84d1a)
    │   ├── measureme v10.1.2 (https://github.com/rust-lang/measureme?tag=10.1.2#f9f84d1a)
    ├── decodeme v11.0.0 (https://github.com/rust-lang/measureme?branch=stable#f2914503)
    │   ├── measureme v11.0.0 (https://github.com/rust-lang/measureme?branch=stable#f2914503)
    ├── measureme v10.1.2 (https://github.com/rust-lang/measureme?tag=10.1.2#f9f84d1a) (*)
    └── measureme v11.0.0 (https://github.com/rust-lang/measureme?branch=stable#f2914503) (*)

I would suggest following these step and publishing theses packages if possible:

Version 9.2.1

1. Create a v9 branch from tag 9.2.0
2. @weihanglo will create PR and changelog for the release 9.2.0...weihanglo:measureme:prepare-9.2.1
3. Review and merge the PR.
4. Publish [email protected] and [email protected] to crates.io (in this order).

Version 10.1.3

1. Create a v10 branch from tag 10.1.2
2. @weihanglo will create PR and changelog for the release 10.1.2...weihanglo:measureme:prepare-10.1.3
3. Review and merge the PR.
4. Publish [email protected], [email protected], and [email protected] to crates.io (in this order).

Version 11.0.2

1. @weihanglo will create PR and changelog for the release 11.0.1...weihanglo:measureme:prepare-11.0.2
2. Review and merge the PR.
3. Publish [email protected], [email protected], and [email protected] to crates.io (in this order).

Additional questions

I am aware of the existence of the stable branch, though I am not sure about the semantic of it. Perhaps not really useful? I see that in rust-lang/rust we don't really use that branch except in rustc-perf.

[Kobzol]

I think that first we should consolidate the usage of these crates in rustc-perf. It doesn't really make sense to use three different versions. I'll take a look at that.

[Kobzol]

Ah, I forgot that this is implicit in this crate, as analyzeme 11 depends on both versions 10 and 9. That's.. unfortunate, and quite complicated for crates.io release. @michaelwoerister do you think that we still need this?

[michaelwoerister]

Yeah, the setup is not quite standard. Depending on multiple older versions makes it easy to maintain backwards compatibility but if it causes trouble, we can look into flattening that (i.e. copying the decoder implementations of the older versions in-tree).

[michaelwoerister]

@weihanglo, if we make analyzeme not depend on older versions of itself and decodeme, then we don't need to backport any of the licensing changes, right?

[weihanglo]

Yeah, I believe so. It would also make updating dependencies way easier. Happy to work on that route if desired!

[michaelwoerister]

Yes, let's do that then. We can merge all of decodeme into analyzeme. That already has the file_formats module with v7, v8, v9 in it. I think, the refactoring would mostly consist of replacing imports from old versions with the copy-and-pasted code from these old versions:

measureme/analyzeme/src/file_formats/v8.rs

Lines 13 to 20 in 9783098

	use decodeme::Metadata;
	use decodeme_10_1_2::event_payload::EventPayload as OldEventPayload;
	use decodeme_10_1_2::event_payload::Timestamp as OldTimestamp;
	use decodeme_10_1_2::lightweight_event::LightweightEvent as OldLightweightEvent;
	pub use decodeme_10_1_2::EventDecoder;
	use decodeme_10_1_2::Metadata as OldMetadata;
	pub const FILE_FORMAT: u32 = measureme_10_1_2::file_header::CURRENT_FILE_FORMAT_VERSION;

[michaelwoerister]

OK, given that inlining the older versions does not seem like an attractive solution, I'd say we go with the original plan outlined above. Sorry for sending you down that rabbit hole, @weihanglo!

I've created https://github.com/rust-lang/measureme/tree/9.x-service-branch and https://github.com/rust-lang/measureme/tree/10.x-service-branch that we can backport against. I've also backported #221 to these, so that publishing doesn't have to be done manually. Let's make sure to update that workflow to include the other crates too.

[Kobzol]

I haven't seen the approach of using older versions of formats by depending on older versions of the same crate used anywhere.. Is this really needed?

The README says "...It is currently only meant to be used within rustc itself, so APIs may change at any moment.". Do we really need to keep compatibility with older formats, even though they are not used anywhere anymore (?).

[michaelwoerister]

Well, the main reason we want to support at least one previous version is that it makes updating perf.rlo much easier. That's why we originally started adding backwards compatibility.

I think depending on older versions of the same crate is a reasonable approach, if these older versions are being serviced. It makes maintenance easier, I think.

We could drop support for the v7 format (which is really old now) and change our policy to only support one previous version.

[Kobzol]

👍 for only keeping compatibility for the last two versions. That already makes the whole situation simpler!

FWIW, I would suggest either always keeping the last two versions inline in source of measureme, or modifying rustc-perf so that it can support two different formats by just depending on two versions of measureme.

That being said, using the trick of depending on an older measureme, provided that it's only one version, and it is actually published on crates.io instead of being referenced through git, seems fine to me.

By the way, how is it even possible that measureme is published on crates.io when it has git dependencies? 🤔

Edit: I see, measureme doesn't have git dependencies, analyzeme does. This prevents the current version of analyzeme to be published on crates.io, though.

[michaelwoerister]

measureme does not have git dependencies. It's decide to contain the bare minimum for emitting profiling data. It cannot decode or analyze that data.

[Kobzol]

Thanks, I noticed it only after sending the comment. This prevents analyzeme to be published to crates.io though, and since the goal of this issue is to avoid rustc-perf depending on git dependencies, it will need analyzeme to be on crates.io (since it uses it). In that case, it seems to me like the only reasonable way forward is to inline the old format into analyseme. Otherwise we'd have to somehow reconstruct and publish the last three versions of analyzeme in a way that avoids git dependencies, which sounds annoying, because each version depends on N previous versions.

[michaelwoerister]

I'm wondering, do we actually need to vendor the old measureme versions as #230 does? These versions are on crates.io and we can pull them in for their definitions.

Maybe we could do the following:

• Start a 12.x release that drops support for the v7 format. That would already decrease the amount of stuff we need to vendor.
• Do not inline measureme versions and depend on older crates.io versions instead.
• Move the stuff that refactor: inline packages for decoding v7 and v8 profdata files #230 has in analyzeme/src/file_formats/decodeme_10_1_2 into the analyzeme/src/file_formats/v8 folder/module (since we don't care about crate versions, only about file format versions).

That's a kind of hybrid approach where still depend on older versions of measureme from crates.io, but the decodeme crate would go away.

[Kobzol]

That sounds like a great compromise!

[michaelwoerister]

Let's give it a try 🙂

@weihanglo, would you mind updating #230 according to the above comment?

[michaelwoerister]

Actually, it occurs to me that if we drop support for the v7 file format, the dependency graph becomes a lot simpler (the 9.x is the last major version that did not have the analyzeme/decodeme split).

We would only need to do a release of decodeme 10.x. We don't need analyzeme 10.x or any other previous version of analyzeme -- that was the point of splitting out decodeme. That would make things even easier.

[michaelwoerister]

The new dep-graph would look like this:

└── analyzeme v12.0.0
    ├── decodeme v10.1.2
    │   ├── measureme v10.1.2
    ├── decodeme v12.0.0
    │   ├── measureme v12.0.0
    ├── measureme v10.1.2
    └── measureme v12.0.0

We don't need crate version 11.x to show up anywhere because we only have the current version (12.x) and the latest old version that had a file format change.

[weihanglo]

Sounds great. Guess we still need to
have v10.1.3 since decodeme 10.1.2 has no license field set?

[michaelwoerister]

Yup

[weihanglo]

The patch is for 10.1.3 is prepared: 10.1.2...weihanglo:measureme:prepare-10.1.3

@michaelwoerister do you want to create a temporary v10 branch so I can file a PR against, or do you want cherry pick the change and do it locally?

[michaelwoerister]

Please use https://github.com/rust-lang/measureme/tree/10.x-service-branch

[weihanglo]

PR is ready: #231

Let me know if you've published them, or anything needs to update :)

[weihanglo]

PR dropping v7 support is up: #232.

[weihanglo]

This is resolved by #233. Thanks everyone 🎉.

[michaelwoerister]

Thanks for putting in all this work, @weihanglo!
Also, thanks for the feedback and thinking through this, @Kobzol. That saved us a bunch of work 🙂