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

Fix compiling for x86_64-unknown-linux-gnu when it's not the default target #1559

Merged
merged 3 commits into from
Nov 28, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Nov 24, 2021

Fixes #1556 by only special casing proc-macros instead of any crate targeting the host.

r? @Nemo157

@jyn514 jyn514 added A-builds Area: Building the documentation for a crate S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Nov 24, 2021
crate_, version, target, crate_path
);

assert!(target_docs_present);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I double checked and this does fail without the fix in this commit.

[2021-11-24T17:58:10Z DEBUG docs_rs::docbuilder::rustwide_builder] adding sources into database
[2021-11-24T17:58:10Z DEBUG docs_rs::db::add_package] Adding package into database
[2021-11-24T17:58:10Z DEBUG docs_rs::db::add_package] Adding doc coverage into database
[2021-11-24T17:58:10Z DEBUG docs_rs::db::add_package] Adding build into database
[2021-11-24T17:58:10Z INFO  docs_rs::db::add_package] Updating crate data for xingapi
[2021-11-24T17:58:10Z DEBUG docs_rs::docbuilder::rustwide_builder] cleaning old storage folder rustdoc/xingapi/0.3.3/
[2021-11-24T17:58:10Z DEBUG docs_rs::docbuilder::rustwide_builder] cleaning old storage folder sources/xingapi/0.3.3/
[2021-11-24T17:58:11Z ERROR docs_rs::web::page::templates] Failed to load rustc resource suffix: missing rustc version
[2021-11-24T17:58:11Z INFO  docs_rs::web] Running docs.rs web server on http://127.0.0.1:39737
thread 'docbuilder::rustwide_builder::tests::test_cross_compile_non_host_default' panicked at 'assertion failed: target_docs_present', src/docbuilder/rustwide_builder.rs:1004:13

failures:
    docbuilder::rustwide_builder::tests::test_cross_compile_non_host_default

@Nemo157
Copy link
Member

Nemo157 commented Nov 24, 2021

I think we probably want to override Metadata::targets to only return a single target for proc-macros too (since we don't actually obey the target setting). I found a random proc-macro without a target override and it only has docs for the default target: https://docs.rs/datatest-derive/0.6.3/datatest_derive/.

@jyn514 jyn514 force-pushed the proc-macro-targets branch 3 times, most recently from 3fb8c49 to 7629bd8 Compare November 24, 2021 21:51
@Nemo157
Copy link
Member

Nemo157 commented Nov 24, 2021

add_essential_files needs to be updated to look in the target dir

-let source = build.host_target_dir().join("doc");
+let source = build.host_target_dir().join(HOST_TARGET).join("doc");

@jyn514 jyn514 force-pushed the proc-macro-targets branch 4 times, most recently from 2c619f9 to 9672ae5 Compare November 27, 2021 15:09
- only skip `--target` for proc-macros
- don't try build non-host targets for proc-macros, since they're not
  supported anyway.
- change rustwide_builder to look in the platform-specific target
  directory for the default target
- stores all source docs in the target/{target_name} directory, rather
  than trying to special case anything
  - this also creates the `{target_name}` directory before copying,
    because `rename` delegates directly to the syscall and doesn't
    create parent directories
@jyn514
Copy link
Member Author

jyn514 commented Nov 27, 2021

Yay, this is finally passing tests :) this should be ready for rereview.

Copy link
Member

@Nemo157 Nemo157 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All LGTM now. Tested locally with a variety of proc-macro/non-proc-macro/default=host/default=non-host crates and all build variants appear to work correctly.

@jyn514 jyn514 merged commit 3e9a87f into rust-lang:master Nov 28, 2021
@jyn514 jyn514 deleted the proc-macro-targets branch November 28, 2021 12:25
@jyn514 jyn514 mentioned this pull request Dec 14, 2021
Nemo157 added a commit to Nemo157/docs.rs that referenced this pull request Aug 23, 2022
This was the previous behaviour before
rust-lang#1559 for the default target
and is most likely expected by users.
jyn514 pushed a commit that referenced this pull request Aug 24, 2022
This was the previous behaviour before
#1559 for the default target
and is most likely expected by users.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-builds Area: Building the documentation for a crate S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

linux target does not appear in docs.rs even targets in metadata is set
2 participants