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.

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

src/doc/build-man.sh is not able to run on platforms missing sh #11717

Closed
weihanglo opened this issue Feb 15, 2023 · 2 comments · Fixed by #12048
Closed

src/doc/build-man.sh is not able to run on platforms missing sh #11717

weihanglo opened this issue Feb 15, 2023 · 2 comments · Fixed by #12048
Labels
A-documenting-cargo-itself Area: Cargo's documentation A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. C-enhancement Category: enhancement S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@weihanglo
Copy link
Member

weihanglo commented Feb 15, 2023 •
edited
Loading

Problem

From #11646 (comment), people struggle to run src/doc/build-man.sh on Windows. This might prevent Windows users from contributing Cargo documentations. We may want to find a way making every contributor feel inclusive without introducing more external tool dependencies.

Proposed Solution

Two options off the top of my head:

  • EASY: Translate src/doc/build-man.sh to Windows batch script or powershell.
  • EASY: Use a python script instead. See this comment.
  • HARD?: Introduce some workflow like cargo-xtask (blocked on nested workspaces feature).

Need to take care of crates/mdman/build-man.sh as well.

Notes

Was reading this article and it reminded me this issue.
@weihanglo weihanglo added A-documenting-cargo-itself Area: Cargo's documentation A-building-cargo-itself Area: issues with building cargo C-enhancement Category: enhancement labels Feb 15, 2023
@Muscraft
Copy link
Member

One solution could be converting to a python script. The biggest problem with making it a python script is that python is not installed by default for all OS. I think it could be reasonable to say python is required as we already have publish.py, and most contributors should have python already installed.

@epage
Copy link
Contributor

epage commented Feb 15, 2023

Another option is cargo-script.

Overall, I assume a cargo-xtask makes the most sense if we can figure out the workspace issue.

@weihanglo weihanglo mentioned this issue Mar 14, 2023
Merged
7 tasks
bors added a commit that referenced this issue Apr 15, 2023
@bors
Auto merge of #11851 - weihanglo:make-cargo-a-workspace, r=ehuss
39116cc 
Make cargo a workspace

### What does this PR try to resolve?

The first step of making cargo a workspace.

Benefits:

* Dogfooding ourselves.
* Unblock #11831: It got stuck because the new version of tempfile using `windows-sys` but some issues haven't yet be solved in rust-lang/rust.
* Make `cargo xtask` or similar developer workflow possible (e.g., #11717)
* Having our own Cargo.lock, so our CI can cover the exact binary going to ship. Also free Cargo from CI breaks due to dependency patch releases.
* Probably more? Please add them by yourself.

### How should we test and review this PR?

Please review it commit by commit. A companion PR is here rust-lang/rust#109133, and should be reviewed together.

### Unresolved issues

To limit the scope of this pull request, the following issues are intentionally left unresolved. They will be addressed right after this pull request gets merged.

- [x] Make `benches/capture` and `benches/capture` workspace members. (Addressed with 2cf9718)
- [x] Make `crates/resolver-tests` a workspace member. (Addressed with #11886)
- [ ] ~~Fix clippy warnings and re-enable clippy check in CI for all workspace members.~~
  - Blocked on rust-lang/rfcs#3389 so we can more easily propagate our clippy settings
- [ ] Fix rustdoc warnings and re-enable rustdoc check in CI for all workspace members.
- [ ] Fix `linkchecker.sh` warnings in CI (#11851 (comment))
- [ ] Leverage workspace flag `--workspace` when running `cargo build` or `cargo test`, instead of using flag `-p`.
- [ ] Leverage glob syntax when probing members in `[workspace]` in Cargo.toml (i.e., `crates/*`).

### Additional information

This depends on prior works from `@Muscraft` and `@ehuss.` Credits to them!
@weihanglo weihanglo added A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed A-building-cargo-itself Area: issues with building cargo S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. labels Apr 25, 2023
@weihanglo weihanglo mentioned this issue Apr 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-documenting-cargo-itself Area: Cargo's documentation A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. C-enhancement Category: enhancement S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: move build-man workflow away from shell weihanglo/cargo
3 participants
@epage @weihanglo @Muscraft