Fix metabuild compile errors with --message-format=json.

[ehuss]

If an error occurs while compiling a metabuild target with --message-format=json, it would panic because it was unable to serialize Target. This change makes it so that it places a fake "metabuild.rs" string in the src_path in this situation.

I'm very unhappy with this solution, but I'm unable to think of something better. Changing src_path to an Option (or something) would break existing tools (this might break, but maybe not catastrophically?). I tried implementing something that resets the src_path to the correct path in the target dir after the workspace is configured, but it felt very brittle – you have to fix up after all dependencies are downloaded, and there's not a good place to ensure that happens correctly. Any alternate ideas?

This adds a with_json_contains_unordered to help with tests.

[rust-highfive]

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

[dwijnand]

This adds a with_json_contains_unordered to help with tests.

👍

Changing src_path to an Option (or something) would break existing tools (this might break, but maybe not catastrophically?)

Do we have commitments to other tools on our serialised data formats?

Any alternate ideas?

The wonderful (terrible) thing about string-based data is you don't need no Option: it already comes with its own action-ready null object value: ""!

[ehuss]

Do we have commitments to other tools on our serialised data formats?

I don't know if Cargo has any hard rules for stability. Maybe we should clarify that? The tough part about the artifact messages is that there is no way to version them (unlike cargo metadata which has a version flag).

The wonderful (terrible) thing about string-based data is you don't need no Option: it already comes with its own action-ready null object value: ""!

Yea, I considered that, too. The issue is if a tool wants to actually read the file, it will fail (either with "" or the fake path I used). I figured the fake path would at least be a little more visible if there is an error.

[alexcrichton]

Without having tons of context I'd actually naively expect that we could use an Option here to faithfully represent the lack of a source file because all existing tools would continue to work (as metabuild isn't stable) and they'd just need an update to work with metabuild. That to me seems like a naively ok tradeoff at least.

If this only happens during actual compilation though, we should have a path to the actual file as well though, right?

[ehuss]

OK, I decided to go ahead and just use None for SerializedTarget::src_path. I hate breaking things, but hopefully the fallout will be small. If it is much of a problem, we can always revisit this later.

If this only happens during actual compilation though, we should have a path to the actual file as well though, right?

The problem is:

• The serialization code does not have access to target_dir to compute the correct path.
• The Targets can be mutated to "fix" the path after they are created (target_dir isn't known when manifests are parsed). However, this is a game of whack-a-mole trying to find every place a manifest is created (at a minimum, it would be after creating a workspace, and in several places in the guts of Downloads). Alternatively it can "fix" the paths during compilation (like it does here), but that will also be very difficult to ensure every location is covered.

[dwijnand]

The new unwraps are unfortunate, but nothing seems to fall over in CI, LGTM

[alexcrichton]

@bors: r+

[bors]

📌 Commit 48d56a4 has been approved by alexcrichton

[bors]

⌛ Testing commit 48d56a4 with merge 8ffc960...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 8ffc960 to master...