Skip to content

bootstrap: translate Windows paths in a way that works for both Cygwin and MSYS2 #141556

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 1 commit into from
May 27, 2025
Merged

bootstrap: translate Windows paths in a way that works for both Cygwin and MSYS2 #141556

merged 1 commit into from
May 27, 2025

Conversation

jeremyd2019
Copy link
Contributor

Cygwin defaults to rooting Windows paths in /cygdrive/X, while MSYS2 configures them to be /X. Regardless of configuration, drives are always accessible as /proc/cygdrive/X, so use that.

If there are other shells on Windows that are supported and use /X style paths, perhaps something more complicated needs to be done.

r? @jieyouxu

@Berrysoft @mati865

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels May 25, 2025
@jeremyd2019
Copy link
Contributor Author

#137819 (comment)

What confused me is why the is_cygwin path wasn't being hit on Cygwin. Turns out:

rust/src/bootstrap/src/core/config/config.rs

Lines 581 to 593 in 88b3b52

pub fn is_windows(&self) -> bool {
self.contains("windows")
}
pub fn is_windows_gnu(&self) -> bool {
self.ends_with("windows-gnu")
}
pub fn is_cygwin(&self) -> bool {
self.is_windows() &&
// ref. https://cygwin.com/pipermail/cygwin/2022-February/250802.html
env::var("OSTYPE").is_ok_and(|v| v.to_lowercase().contains("cygwin"))
}

It can only be considered cygwin if the target has windows in it, and it seems the Cygwin target went with x86_64-pc-cygwin rather than x86_64-pc-windows-cygnus which is what LLVM normalizes a Cygwin target to.

@Berrysoft
Copy link
Contributor

is_cygwin is older than cygwin target. It was used to detect MSYS2 on Windows, to generate and call bash scripts.

@Berrysoft
Copy link
Contributor

As for target name, Cygwin was decided to be a unix target rather than windows target in the beginning: #134999 . We wanted to reuse most code path for unix, and it seems to be a success now.

@mati865
Copy link
Member

mati865 commented May 25, 2025

If there are other shells on Windows that are supported and use /X style paths, perhaps something more complicated needs to be done.

Haven't heard of any.

@Berrysoft
Copy link
Contributor

There's a shell in busybox, but it uses Windows style path. There's no shell in GnuWin32.

@jieyouxu
Copy link
Member

Thanks
@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 26, 2025

📌 Commit 79c0db6 has been approved by jieyouxu

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented May 26, 2025

🌲 The tree is currently closed for pull requests below priority 10. This pull request will be tested once the tree is reopened.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 26, 2025
@jieyouxu
Copy link
Member

Actually, can u pls leave a comment (i.e. what PR description says)? Otherwise it seems as if we pull a completely random incantation out of nowhere.

@jieyouxu
Copy link
Member

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 26, 2025
@jeremyd2019
bootstrap: translate Windows paths in a way that works for both Cygwi…
108c16e 
…n and MSYS2

Cygwin defaults to rooting Windows paths in /cygdrive/X, while MSYS2
configures them to be /X.  Regardless of configuration, drives are
always accessible as /proc/cygdrive/X, so use that.
jieyouxu
jieyouxu approved these changes May 26, 2025
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 26, 2025

📌 Commit 108c16e has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 26, 2025
@compiler-errors compiler-errors mentioned this pull request May 27, 2025
Merged
bors added a commit that referenced this pull request May 27, 2025
@bors
Auto merge of #141644 - compiler-errors:rollup-gl5hize, r=compiler-er…
c583fa6 
…rors

Rollup of 17 pull requests

Successful merges:

 - #140591 (Fix malformed suggestion for E0061 when method is a macro token in macro context)
 - #141536 (Improve `ambiguous_wide_pointer_comparisons` lint compare diagnostics)
 - #141552 (Pull out dedicated `cfg_version` syntax test from feature gate test)
 - #141556 (bootstrap: translate Windows paths in a way that works for both Cygwin and MSYS2)
 - #141563 (Remove out-of-date `noop_*` names.)
 - #141568 (dist: make sure llvm-project submodule is present)
 - #141580 (Use more detailed spans in dyn compat errors within bodies)
 - #141582 (intrinsics, ScalarInt: minor cleanup)
 - #141584 (Support `opaque_types_defined_by` for `SyntheticCoroutineBody`)
 - #141587 (Add missing edition directives for async-await tests)
 - #141594 (Add `generic_arg_infer` test)
 - #141596 (rustc book: fix erratic sentence by making it more simple)
 - #141599 (Remove an unnecessary use of `Box::into_inner`.)
 - #141611 (Update mdbook to 0.4.51)
 - #141616 (Remove spastorino from vacations)
 - #141623 (use custom types to clarify arguments to `emit_ptr_va_arg`)
 - #141635 (further dedup `WalkItemKind` for `mut_visit` and `visit`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f1371a8 into rust-lang:master May 27, 2025
7 checks passed
@rustbot rustbot added this to the 1.89.0 milestone May 27, 2025
@jeremyd2019 jeremyd2019 deleted the patch-1 branch May 27, 2025 18:56
rust-timer added a commit that referenced this pull request May 27, 2025
@rust-timer
Unrolled build for #141556
dda0b3c 
Rollup merge of #141556 - jeremyd2019:patch-1, r=jieyouxu

bootstrap: translate Windows paths in a way that works for both Cygwin and MSYS2

Cygwin defaults to rooting Windows paths in /cygdrive/X, while MSYS2 configures them to be /X.  Regardless of configuration, drives are always accessible as /proc/cygdrive/X, so use that.

If there are other shells on Windows that are supported and use /X style paths, perhaps something more complicated needs to be done.

r? `@jieyouxu`

`@Berrysoft` `@mati865`
@jeremyd2019
Copy link
Contributor Author

yay, now I just need to wait for the cargo submodule to be updated and I won't have to patch anything to build rust-nightly on Cygwin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mati865 mati865 mati865 approved these changes

@jieyouxu jieyouxu jieyouxu approved these changes

+1 more reviewer

@Berrysoft Berrysoft Berrysoft approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@jieyouxu jieyouxu

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
1.89.0
Development

Successfully merging this pull request may close these issues.
6 participants
@jeremyd2019 @Berrysoft @mati865 @jieyouxu @bors @rustbot