cargo-public-api icon indicating copy to clipboard operation
cargo-public-api copied to clipboard

Support for `.cargo/config.toml`

Open repi opened this issue 3 years ago • 10 comments

On some of our internal crates that I've been testing cargo-public-api on they have successfully built with rustdoc (manually and through cargo public-api) but then failed on:

    Finished dev [unoptimized + debuginfo] target(s) in 7.05s
Error: Failed to read rustdoc JSON at "/home/repi/git/test-module/target/doc/module.json"

Caused by:
    No such file or directory (os error 2)

And indeed there is no rustdoc JSON filed output there. So feels like a rustdoc failure somehow even though it doesn't log out anything about it?

repi avatar Aug 20 '22 17:08 repi

Thank you for reporting! The rustdoc-json crate tries to figure out what the path to the JSON is by also taking into account workspaces. And it seems to work for the basic case. For example, in this project you can do:

% cd public-api
% cargo public-api --verbose
Processing "/home/martin/src/cargo-public-api/target/doc/public_api.json"

i.e. it figures out the right path.

The code for figuring out the JSON path is here:

https://github.com/Enselic/cargo-public-api/blob/a848f3a629c9e314fe121d0cae85bc27afd54b6c/rustdoc-json/src/build.rs#L73-L93

Could you describe a bit more what the workspace setup is for this crate? Or maybe add some debug prints in the above code and see if you can debug the problem? I completely understand if you don't have time over for that though. But it would be nice to figure out what the problem is here.

To run a local version of cargo-public-api against an arbitrary crate, you do: cargo run -- --manifest-path path/to/your/Cargo.toml while standing in the git root. That will use whatever local changes you have done to your git clone.

Enselic avatar Aug 20 '22 18:08 Enselic

Believe this was caused by running in a workspace that had a .cargo/config.toml that specified:

[build]
target = "wasm32-unknown-unknown"

which is the same as building with cargo rustdoc --target wasm32-unknown-unknown so it put the output json in target triple folder instead such as target/wasm32-unknown-unknown/doc/crate.json.

The good news is that building explicitly with the new --target support from #107 does work on the same crate. But for it to work in the general case without explicitly specifying target would need to also support .cargo/config.toml (and potential environment variables that can set the same?).

repi avatar Aug 21 '22 10:08 repi

Thank you so much for debugging. I didn't know about .cargo/config.toml actually. But this tool clearly should support it. Proper support is not trivial it seems. Docs are at https://doc.rust-lang.org/cargo/reference/config.html. Would be nice if there was some crate we could use so we wouldn't have to fully implement support ourselves.

Enselic avatar Aug 21 '22 11:08 Enselic

Indeed, feels like more Cargo-related tools would need the same so sharing the logic (likely extracted from Cargo itself) of exactly how it is parsed and resolved would be great.

repi avatar Aug 21 '22 11:08 repi

related https://github.com/rust-lang/cargo/issues/8842

This limitation could be overcome if rustdoc --output-format json also could specify a path to store the output for the json.

technically, you can get the target with cargo +nightly rustc --print target-libdir -Zunstable-options but that doesn't seem super nice

Emilgardis avatar Dec 11 '22 22:12 Emilgardis

I had to solve the equivalent problem in cargo-semver-checks which also generates rustdoc: https://github.com/obi1kenobi/cargo-semver-checks/issues/432

I ended up working out exactly how JSON names get created, and the fixed name generation is here: https://github.com/obi1kenobi/cargo-semver-checks/pull/433/files#diff-6a41cef264f628c6cf1f0f5a15db90570991cc0b3e9dbabd62449dd3efa8b30fR138-R151

Hope that helps! I'd imagine we probably run into a fair bit of the same rustdoc JSON issues, and I'd be happy to work more closely together and help each other find the answers we need :)

obi1kenobi avatar Jun 20 '23 05:06 obi1kenobi

That certainly helps, thanks for sharing! Totally agree we should look for and utilize synergies. Speaking of, maybe cargo-semver-checks can use our rustdoc-json crate to build rustdoc JSON? Then your fix could be incorporated there and the broader ecosystem could benefit more easily.

Enselic avatar Jun 25 '23 09:06 Enselic

Speaking of, maybe cargo-semver-checks can use our rustdoc-json crate to build rustdoc JSON?

I'd love to have a crate for building rustdoc JSON. Unfortunately I think cargo-semver-checks has more complex requirements for how rustdoc JSON is built than what rustdoc-json offers at the moment, such as:

  • we use the RUSTC_BOOTSTRAP flag to build rustdoc on the current Rust installation, even if it isn't nightly
  • we create a placeholder project with a dependency on the library crate in order to generate that library crate's rustdoc JSON, in order to avoid issues around stale dependencies in the existing lockfile like this one: https://github.com/obi1kenobi/cargo-semver-checks/issues/167#issuecomment-1382367128

If you're open to offering that same functionality in rustdoc-json, I'd love to switch to it!

obi1kenobi avatar Jun 30 '23 03:06 obi1kenobi

Happy to hear that!

  • we use the RUSTC_BOOTSTRAP flag to build rustdoc on the current Rust installation, even if it isn't nightly

I think a good way to solve this is to add an env() method to Builder that gets forwarded to the underlying Command::env().

  • we create a placeholder project with a dependency on the library crate in order to generate that library crate's rustdoc JSON, in order to avoid issues around stale dependencies in the existing lockfile

We also do some tricks with creating placeholder projects (see published_crate.rs), but I'm not sure it is architecturally sound to put that logic inside of the rustdoc-json crate. I think it makes more sense to let dependents do the dirty tricks with regards to creating the placeholder project, because I think that will look different from dependent to dependent. And let rustdoc-json be concerned only with building rustdoc JSON.

But I am certainly open to be convinced otherwise. I might very well be missing something. I think the best way to make progress here is to whip together a prototype and see how it ends up.

Enselic avatar Jul 01 '23 09:07 Enselic

I agree that placeholder logic can be tricky and not necessarily one-size-fits-all. But at least for cargo-semver-checks, there just isn't much logic outside of that placeholder stuff — the rest is just one std::process::Command builder and execution. I don't think there's enough code there to justify pulling just that bit out into a separate crate.

So from my point of view, we either have to figure out how to make a common crate with "generate JSON for an arbitrary crates.io crate by name and version" functionality with all the right placeholder magic, or there's not much else to extract in common here.

obi1kenobi avatar Jul 08 '23 21:07 obi1kenobi