Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Debug Netlify Build #1130

Closed
wants to merge 9 commits into from
Closed

Conversation

davidbarsky
Copy link
Member

@davidbarsky davidbarsky commented Dec 9, 2020

It seems like netlify/build-image#477 might have broken our documentation previews, but in fairness to Netlify, I think we were the only ones who were doing a "render and deploy cargo doc" for each PR. Here's what I think is happening with netlify/build-image#477:

  1. The cache_cwd_directory function (callsite, definition) moves Cargo's build output to $NETLIFY_CACHE_DIR.
  2. cache_cwd_directory calls move_cache, which removes the Cargo-generated target directory. This explains why we see errors from Netlify that informing us that "target/doc does not exist".

To get around this, I've tried to change the output directory to not be target, but that's not sufficient.

@davidbarsky davidbarsky requested review from hawkw and a team as code owners December 9, 2020 20:48
@mraerino
Copy link
Contributor

mraerino commented Dec 9, 2020

Hi! We use a bunch of Rust at Netlify and I started using it to publish docs for private crates since we introduced support for rustup yesterday.
I haven't looked closely, but i can try to give some clues.

If you have a rust-toolchain file (old or new format) cargo should be able to automatically install the correct toolchain for you, so your build command could be reduced to just cargo doc --no-deps.

@davidbarsky
Copy link
Member Author

davidbarsky commented Dec 9, 2020

Hi! We use a bunch of Rust at Netlify and I started using it to publish docs for private crates since we introduced support for rustup yesterday.

That's rad!

I haven't looked closely, but i can try to give some clues.

I know it's a bit late for you, but I've written a more extensive writeup of what I think happened here: netlify/build-image#505. I'd be happy to move the discussion wherever.

If you have a rust-toolchain file (old or new format) cargo should be able to automatically install the correct toolchain for you, so your build command could be reduced to just cargo doc --no-deps.

That's a pretty handy feature! Unfortunately, I don't think a rust-toolchain will easily work with our MSRV tests or our (nightly-only) documentation previews.

@mraerino
Copy link
Contributor

i opened a PR here that should work: #1131

@hawkw hawkw closed this in d59f4bc Dec 10, 2020
hawkw pushed a commit that referenced this pull request Dec 14, 2020
## Motivation

At Netlify we recently introduced native Rust support in the build
system: netlify/build-image#477

## Solution

This PR cleans up the Netlify build config to use a more
straight-forward way of building rust docs.

This also introduces a workaround for
netlify/build-image#505

## Kudos

We're big fans of the `tracing` crate at Netlify and using it for many
new systems recently. Very happy we can give something back!

Closes #1130
hawkw pushed a commit that referenced this pull request Dec 14, 2020
## Motivation

At Netlify we recently introduced native Rust support in the build
system: netlify/build-image#477

## Solution

This PR cleans up the Netlify build config to use a more
straight-forward way of building rust docs.

This also introduces a workaround for
netlify/build-image#505

## Kudos

We're big fans of the `tracing` crate at Netlify and using it for many
new systems recently. Very happy we can give something back!

Closes #1130
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants