-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Package mingw dependencies #16957
Package mingw dependencies #16957
Conversation
@@ -1401,6 +1401,9 @@ fn link_args(cmd: &mut Command, | |||
cmd.arg("-nodefaultlibs"); | |||
} | |||
|
|||
// Rust does its' own LTO | |||
cmd.arg("-fno-lto").arg("-fno-use-linker-plugin"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we only run these on windows, or is this ok for all invocations of gcc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LTO is off by default, so why does this need to be done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexcrichton: I think this is ok for all platforms. That'll be one less gcc dependency to worry about if we ever switch to direct linker invocation.
@thestinger: gcc still tells linker to load its' lto plugin, I guess on the off chance that one of the libraries being linked was compiled with lto.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks clang builds on OSX since clang doesn't know the argument: -fno-use-linker-plugin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to work around this with export ARCHFLAGS="-Wno-error=unused-command-line-argument-hard-error-in-future"
.
Not sure what to do about this in the long term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can clang vs gcc be detected before the option is passed? If it's known which one is being called, then this can be passed to gcc but not clang.
This is amazing 👾! |
😻 @vadimcn, my hero. |
Am I correct in thinking that once we do the first snap after this that the toolchains on the bots will no longer be used, or is the snapshot bundling unchanged? It may make upgrading the toolchain very awkward if our snapshot is always overriding it. |
😎
No, this does not affect snaps. Rust devs are still expected to install msys/mingw. Stuff's only gonna get bundled into Windows installer. And even then, /bin is appended to the PATH, so any locally installed toolchain will take precedence. |
r? |
}; | ||
debug!("new PATH: {}", String::from_utf8_lossy(new_path.as_slice())); | ||
os::setenv("PATH", new_path); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I just realize that this may yield surprising results when dealing with usage like rustdoc --test
. In that case each test is compiled within the same process, which means that PATH
will be appended to repeatedly (which may end badly).
Could this perhaps be wrapped in a Once
or something similar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't that already be a problem with FileSearch::add_dylib_search_paths()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm yes indeed. I suppose we can just wait and see what happens!
r? @brson, this all looks good to me, but I want to be extra sure that we're covered on the licensing front |
On second thought, maybe there already was a problem with over-long paths. Doctests do fail on Windows after all. |
@alexcrichton @vadimcn it does seem like we need to do some due diligence on the licensing here. This directory is included with both the snaps and the installer. Right now it lists two specific files as being GPLv3+runtime exception. Figuring out the exact license of all these bins will suck, but I'm guessing they are all either GPLv3 or GPLv3+runtime exception. A number of these files have names that are frighteningly close to MS DLLs (libkernel32.a, libmsvcrt.a, libuser32.a). Are we sure these are part of cygwin/mingw? If we are confident that these are all GPLv3 or GPLv3+runtime exception then (I think) I would be satisfied just changing the text of the third-party README to be something like: "Certain binaries in this distribution do not originate from the Rust project, but are distributed with it in its binary form. These binaries, including gcc and other parts of the GNU compiler toolchain, are licensed either under the terms of the GNU General Public License, or the GNU General Public License with the GCC Runtime Library Exception, as published by the Free Software Foundation, either version 3, or (at your option) any later version. See the files COPYING3 and COPYING.RUNTIME respectively. You can obtain a copy of the source of these libraries from the MinGW-builds project[1]. [1]: link to somewhere useful" |
4067ace
to
484edd0
Compare
Updated license notice, fixed overlong paths (also for dylibs). |
r? |
…bin/) to PATH during linking, so that rustc can invoke them.
gcc, ld, ar, dlltool, windres go into $(RUST)/bin/rustlib/<triple>/bin/ platform libraries and startup objects got into $(RUST)/bin/rustlib/<triple>/lib/
484edd0
to
7085b3e
Compare
Package rustc's mingw dependencies into Windows installer to avoid requiring a separate mingw install. Closes #11782
Does this make rustc work from cmd.exe? |
@seanmonstar |
Finding mingw a pain to get working correctly on each Windows install, I've |
@seanmonstar Yes , you won't require mingw after this. |
…lnicola fix: check for client support of relative glob patterns before using them Fixes rust-lang#16955
Package rustc's mingw dependencies into Windows installer to avoid requiring a separate mingw install.
Closes #11782