Skip to content

Document the lib if a lib and bin have the same name #4627

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 25, 2017
Merged

Document the lib if a lib and bin have the same name #4627

merged 2 commits into from
Oct 25, 2017

Conversation

@carols10cents
Copy link
Member

Fixes #4341, as discussed in that issue.

  • Removes the check that bailed if there was a bin and lib with the
    same name
  • Exclude bins with the same name as libs from the proposed targets to
    build when compiling docs
  • Adjust tests to expect this behavior

@rust-highfive
Copy link

r? @matklad

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

matklad
matklad reviewed Oct 16, 2017
View reviewed changes
Copy link
Contributor

@matklad matklad left a comment

Choose a reason for hiding this comment

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

Excellent, thanks @carols10cents !

What do you think about adding a tests, where bin and lib have the same name, but you invoke cargo doc as cargo doc --lib and cargo doc --bin? Presumably, these forms should both work?

tests/doc.rs Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 ❤️

tests/rustdoc.rs Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, I didn't realize we have separate cargo doc and cargo rustdoc commands!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup! 🤷‍♀️

@carols10cents
Copy link
Member Author

What do you think about adding a tests, where bin and lib have the same name, but you invoke cargo doc as cargo doc --lib and cargo doc --bin? Presumably, these forms should both work?

I'm into it! I tested them manually and they work, will add tests :)

@carols10cents
Copy link
Member Author

More tests for you @matklad! ❤️

@matklad
Copy link
Contributor

matklad commented Oct 16, 2017

Tests always make me happy, yay!

@bors r+

@bors
Copy link
Contributor

bors commented Oct 16, 2017

📌 Commit f3953b8 has been approved by matklad

@bors
Copy link
Contributor

bors commented Oct 16, 2017

⌛ Testing commit f3953b8b2146a5ee67a0ae5a153b1c3326380690 with merge 9ff0930c21ad0e7f9182e90a33ead552920df3e9...

@bors
Copy link
Contributor

bors commented Oct 16, 2017

💔 Test failed - status-appveyor

@bors
Copy link
Contributor

bors commented Oct 23, 2017

⌛ Testing commit f3953b8b2146a5ee67a0ae5a153b1c3326380690 with merge 825f3e34f6fe10984d735485d3095a62a3329c19...

@bors
Copy link
Contributor

bors commented Oct 23, 2017

💔 Test failed - status-appveyor

@carols10cents
Copy link
Member Author

ummmmm why is this failing on appveyor...?

---- build_script_needed_for_host_and_target stdout ----
	thread 'build_script_needed_for_host_and_target' panicked at 'Cannot cross compile to i686-pc-windows-msvc.
This failure can be safely ignored. If you would prefer to not see this
failure, you can set the environment variable CFG_DISABLE_CROSS_TESTS to "1".
Alternatively, you can install the necessary libraries for cross-compilation with
    rustup target add i686-pc-windows-msvc
', tests\cargotest\support\cross_compile.rs:88:4
note: Run with `RUST_BACKTRACE=1` for a backtrace.
failures:
    build_script_needed_for_host_and_target
test result: FAILED. 18 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
error: test failed, to rerun pass '--test cross-compile'
Command exited with code 101

Shouldn't appveyor have i686-pc-windows-msvc...?

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Oct 25, 2017

⌛ Testing commit f3953b8b2146a5ee67a0ae5a153b1c3326380690 with merge fb36031a84a7afcbad862b47677d7a72d6c2dbca...

@bors
Copy link
Contributor

bors commented Oct 25, 2017

💔 Test failed - status-appveyor

@matklad
Copy link
Contributor

matklad commented Oct 25, 2017

@carols10cents failure seems legit, but mysterious.

failures:
---- doc_multiple_targets_same_name stdout ----
	running `C:\projects\cargo\target\debug\cargo.exe doc --all`
thread 'doc_multiple_targets_same_name' panicked at '
Expected: execs
    but: expected to find:
[DOCUMENTING] foo v0.1.0 (file:///C:/projects/cargo/target/cit/t10/foo[/]foo)
did not find in output:
 Documenting bar v0.1.0 (file:///C:/projects/cargo/target/cit/t10/foo/bar)
 Documenting foo v0.1.0 (file:///C:/projects/cargo/target/cit/t10/foo/foo)
    Finished dev [unoptimized + debuginfo] target(s) in 0.68 secs
', C:\Users\appveyor\.cargo\registry\src\gitproxy.zycloud.tk-1ecc6299db9ec823\hamcrest-0.1.1\src\core.rs:31:12
note: Run with `RUST_BACKTRACE=1` for a backtrace.

@matklad
Copy link
Contributor

matklad commented Oct 25, 2017

@carols10cents failure seems legit, but mysterious.

Ah, I think I know what's going on: the real code prints paths with /, despite the fact that we are on windows. There then check for [/] which is translated to \ on windows. Looks like this is a bug, DOCUMENTING messages should ideally use \ on window.

@carols10cents
Copy link
Member Author

ahhhh i'll take out the square brackets and file a bug for the bug.

@carols10cents
Document the lib if a lib and bin have the same name
b60eacc 
Fixes #4341.

- Removes the check that bailed if there was a  bin and lib with the
  same name
- Exclude bins with the same name as libs from the proposed targets to
  build when compiling docs
- Adjust tests to expect this behavior
@carols10cents
Add more tests about documenting lib vs bins with the same name
e98b9db
@matklad
Copy link
Contributor

matklad commented Oct 25, 2017

@bors r+

@bors
Copy link
Contributor

bors commented Oct 25, 2017

📌 Commit e98b9db has been approved by matklad

@bors
Copy link
Contributor

bors commented Oct 25, 2017

⌛ Testing commit e98b9db with merge b8f7976...

bors added a commit that referenced this pull request Oct 25, 2017
@bors
Auto merge of #4627 - integer32llc:document-libs-and-bins, r=matklad
b8f7976 
Document the lib if a lib and bin have the same name

Fixes #4341, as discussed in that issue.

- Removes the check that bailed if there was a  bin and lib with the
  same name
- Exclude bins with the same name as libs from the proposed targets to
  build when compiling docs
- Adjust tests to expect this behavior
@carols10cents carols10cents mentioned this pull request Oct 25, 2017
Closed
@bors
Copy link
Contributor

bors commented Oct 25, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing b8f7976 to master...

@bors bors merged commit e98b9db into rust-lang:master Oct 25, 2017
@ehuss ehuss added this to the 1.23.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@matklad matklad matklad left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@matklad matklad

Labels

None yet

Projects

None yet

Milestone

1.23.0

Development

Successfully merging this pull request may close these issues.

For a package where a library and a binary have the same name, have cargo doc ignore the bin

6 participants

@carols10cents @rust-highfive @matklad @bors @alexcrichton @ehuss