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
Add Darwin support for directx-shader-compiler #107188
Conversation
Result of 1 package built:
|
I merged the other PR. Please rebase this one. |
92d69d6
to
be8a89e
Compare
Done. Why was this necessary? |
If am not mistaken it could happen that the init commit could be doubled if the hash changed through rebasing or something. Just wanted to not risk this. |
Result of 1 package built:
|
@@ -36,14 +36,14 @@ stdenv.mkDerivation rec { | |||
installPhase = '' | |||
mkdir -p $out/bin $out/lib $dev/include | |||
mv bin/dxc* $out/bin/ | |||
mv lib/libdxcompiler.so* $out/lib/ | |||
mv lib/libdxcompiler.so* lib/libdxcompiler.*dylib $out/lib/ |
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 will not work as expected if either *.so files or *.dylib files are missing
I suggest reworking this into
mv lib/libdxcompiler.so* lib/libdxcompiler.*dylib $out/lib/ | |
mv lib/libdxcompiler.so* $out/lib/ || : | |
mv lib/libdxcompiler.*dylib $out/lib/ || : |
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.
It seems to build without issue on both linux and osx though, perhaps null_glob is enabled or something.
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.
It seems you are right -
nixpkgs/pkgs/stdenv/generic/setup.sh
Line 234 in b6e98f1
shopt -s nullglob |
Thanks, today I learned something new :-)
Result of 1 package built:
|
Is it possible to merge this, please? |
No unless you address my suggestion above. Don't know why it was marked it as resolved, when it was not addressed |
sorry, @prusnak I was certain that I left a comment before marking as resolved. I'll re-comment now |
Thanks!
…On Thu, Dec 24, 2020, 9:38 PM Pavol Rusnak ***@***.***> wrote:
Merged #107188 <#107188> into master.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#107188 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGRJXAG2DOVKHDDVCY2CJ3SWM75HANCNFSM4VCB3YZQ>
.
|
I can't test this so if someone would be kind enough to check it runs
I'd be grateful.
Built on #104377
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)