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

Windows: Don't link std (and run-make) against advapi32, except on win7 #138233

Merged
merged 2 commits into from
Mar 9, 2025
+19 −13
Merged

Windows: Don't link std (and run-make) against advapi32, except on win7 #138233

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9278a3e
Don't link against advapi32, except on win7.
smmalis37 Mar 8, 2025
31e22c6
re-add gnu lib and tweak comment
smmalis37 Mar 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion library/windows_targets/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub macro link {
}

#[cfg(not(feature = "windows_raw_dylib"))]
#[link(name = "advapi32")]
#[cfg_attr(target_vendor = "win7", link(name = "advapi32"))]
#[link(name = "ntdll")]
#[link(name = "userenv")]
#[link(name = "ws2_32")]
Expand Down
1 change: 0 additions & 1 deletion src/bootstrap/src/core/build_steps/dist.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ fn make_win_dist(
"libpthread.a",
//Windows import libs
//This should contain only the set of libraries necessary to link the standard library.
"libadvapi32.a",
Copy link
Member

Choose a reason for hiding this comment

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

While we should only distribute the libraries windows-gnu needs for std, we've had problems with people accidentally relying on them being there so I'm reluctant to clean up this list too much. But cc @mati865 as target maintainer

Copy link

Choose a reason for hiding this comment

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

I was wondering if there were potential backcompat hazards lurking inside this change. I was just going off the comment here though. Maybe it should be reworded if we decide to keep this list longer.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, rewording this comment would make sense to me. Or at least adding a comment that notes the hazard.

Copy link

Choose a reason for hiding this comment

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

Done

"libbcrypt.a",
"libcomctl32.a",
"libcomdlg32.a",
Expand Down
16 changes: 7 additions & 9 deletions src/tools/run-make-support/src/external_deps/c_cxx_compiler/extras.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
use crate::{is_msvc, is_windows, uname};
use crate::{is_msvc, is_win7, is_windows, uname};

/// `EXTRACFLAGS`
pub fn extra_c_flags() -> Vec<&'static str> {
if is_windows() {
if is_msvc() {
vec![
"ws2_32.lib",
"userenv.lib",
"advapi32.lib",
"bcrypt.lib",
"ntdll.lib",
"synchronization.lib",
]
let mut libs =
vec!["ws2_32.lib", "userenv.lib", "bcrypt.lib", "ntdll.lib", "synchronization.lib"];
if is_win7() {
libs.push("advapi32.lib");
}
libs
} else {
vec!["-lws2_32", "-luserenv", "-lbcrypt", "-lntdll", "-lsynchronization"]
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/run-make-support/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub use run::{cmd, run, run_fail, run_with_args};

/// Helpers for checking target information.
pub use targets::{
apple_os, is_aix, is_darwin, is_msvc, is_windows, is_windows_gnu, llvm_components_contain,
apple_os, is_aix, is_darwin, is_msvc, is_windows, is_windows_gnu, is_win7, llvm_components_contain,
target, uname,
};

Expand Down
6 changes: 6 additions & 0 deletions src/tools/run-make-support/src/targets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ pub fn is_windows_gnu() -> bool {
target().ends_with("windows-gnu")
}

/// Check if target is win7.
#[must_use]
pub fn is_win7() -> bool {
target().contains("win7")
}

/// Check if target uses macOS.
#[must_use]
pub fn is_darwin() -> bool {
Expand Down
Loading