last_time_needed looks good to me.

prepare_stats may not even be necessary after the artifact has been prepared and the metrics logged. What happens when this field is removed?

Maybe add a comment that in case there are multiple wasmtime versions being used, is an error. Example:

 $ cargo tree --package=aead --depth=0
error: There are multiple `aead` packages in your project, and the specification `aead` is ambiguous.
Please re-run this command with one of the following specifications:
  [email protected]
  [email protected]
  [email protected]

I think an error in that case great, we definitely don't want to be mixing wasmtime versions anyway.

This is somehow captured. Right now this shows a warning and the compilation will fail if they try to refer to the environment variable. Would that be enough?

I mean a comment would be useful for readers of the code. I was wondering what would happen if there were multiple versions of wasmtime, I didn't know it would be an error until I tried it.

This is over the top. We don't need this macro, nor do we need to concat this at compile time. We are using format anyway to generate the name.

You mean we do something like this instead?

fn artifact_prefix() -> String {
	format!("{}{}_{}{}", RUNTIME_PREFIX, RUNTIME_VERSION, NODE_PREFIX, NODE_VERSION)
}

Yes

This is the very first version. Now it still feels odd to compute the same thing over and over.. I understand this is not remotely close to hot and this isn't about performance. I understand that the macro thing may feel like too much at the first glance. When I looked at how much cleaner the code became with the const/static compared to a function call, I felt it's worth it. A const is just so much easier to use than a function, as when a value is static, it probably should be used as one. Besides, isn't it nice that drop/dealloc/format calls are wiped off from the assembly? :))

If it's a function you can at least put it behind a OnceCell or something. Otherwise I did a search and there are some crates that do this: const_format, concat_const, etc. But it would be sad to pull in a whole dependency for this. I wish std had this... Meh, let's just merge as-is (with the function). I'll press the green button once conflicts are resolved.

If it's a function you can at least put it behind a OnceCell or something.

Function, cell, lazy static, considered all that before doing the macro as it results in the simplest interface, a static str. I won't change it back. Let's get this merged. 🚀 I'm just trying to understand why is a function call better here.

I'm just trying to understand why is a function call better here.

The problem with the macro is that it's a lot of code for readers to understand, just to declare a constant. TBH, all the options are kind of meh, but given that the code is cold, optimizing for maintainability with a plain function seems like the right call.

Turns out that --locked alone would achieve both.

Can you please still handle them gracefully. I mean this is here just by returning true.

Should be true right? Could use a test for this case if we don't have one yet.

Oops! That's right. But this line of can only be reached when

• the path end in "/..", or
• the path contain non-Unicode

These are all screened out at the call site, so I don't even know how to write a test that could fail because of this line lol.

I guess is_corrupt can be extracted and unit-tested, but whatever, looks good already!

We should probably stick to testing the interfaces, this kind of tests often become maintenance liability (prone to change). 😉