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
Feature/mingw coreutils #43710
Feature/mingw coreutils #43710
Conversation
@@ -10,6 +10,7 @@ | |||
|
|||
assert aclSupport -> acl != null; | |||
assert selinuxSupport -> libselinux != null && libsepol != null; | |||
assert hostPlatform.libc != "msvcrt"; |
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.
I think it would make more sense to avoid the assertion and just do platforms = platforms.unix
below. Assertions are annoying because there is no way to override them with something like allowUnsupportedPlatform.
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.
fair point.
@@ -40,7 +40,7 @@ let | |||
libc_lib = if libc == null then null else getLib libc; | |||
cc_solib = getLib cc; | |||
# The wrapper scripts use 'cat' and 'grep', so we may need coreutils. | |||
coreutils_bin = if nativeTools then "" else getBin coreutils; | |||
coreutils_bin = if (nativeTools || targetPlatform.isWindows) then "" else getBin coreutils; |
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.
I still think this one is in error? It means cc-wrapper won't use coreutils, which seems wrong.
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.
No. this is right. It should not use coreutils on windows. We don't have them on windows.
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.
But that means you won't use them on Linux when you are making windows binaries. Use hostPlatform
to turn it just when compiling on Windows (for any target).
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.
hmm... I'll try hostPlatform
, let's see. My issue was that it tried to buld coreutils for windows when cross compiling windows stuff on linux.
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.
I think we don't want the cc-wrapper
changes. The rest is good, though.
@angerman Did you ever run into this bug:
? Here is the build: |
@matthewbauer I haven't. But I remember there being some similar sounding issues with recent git checkouts of GHC. I haven't looked into those though... |
Applied in acaa6c9. |
Motivation for this change
Broken out of #43559
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)