win-dll-link: Fix to look for DLLs in lib/ #38632
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks, @Ericson2314! I'll try and give this a try tomorrow. |
|
Hi @Ericson2314, Sadly the script fails to run: The important bit The whole thing: Also, it's not clear from a quick browse of the new script that it will look in |
|
I don't have the time at the moment to take this over, sorry |
|
@expipiplus1 I fixed some things in the script, but the next problem is |
Ericson2314
force-pushed
the
windows-dll
branch
from
fb8621e
to
61588a9
Compare
|
Thanks for the hard work John, sadly this doesn't fix the original issue. I think because the libraries are in that strange pc-mingw directory. |
|
Oh! I think I know the solution now! We go backwards from |
DLLs are symlinked bin/ for the exes that use them, but they should always be installed in lib/ for sake of regularity and multiple outputs. Fix NixOS#38451
|
Ah, it still doesn't work. ld-wrapper adds some mod |
Ericson2314
force-pushed
the
windows-dll
branch
from
61588a9
to
409e07c
Compare
Ericson2314
referenced
this pull request
Jul 27, 2018
This will be very useful for bootstrapping, eventually.
|
What is the status of this pull request? |
|
Closing due to lack of activity, feel free re-open this if needed. |
|
I'm starting this up again and trying to make it work, and I'm finding that the To be clear, I checked the state of the Does anyone have guidance? In particular
|
|
Okay, so if I just manually force it to use the There are still a few libraries that aren't picked up. The main one seems to be |
|
Oh, I take it back. I was reading the output wrong. |
|
I'm seeing that |
|
I marked this as stale due to inactivity. → More info |
stale
bot
added
the
2.status: stale
10 tasks
Merged
12 tasks
|
#252459 a similar change was merged |
|
Great! I think I'll keep this draft to someday rebase and double-check how it might differ/not-differ, but not sure when I'll get around to that either. |
stale
bot
removed
the
2.status: stale
Artturin
reviewed
Sep 5, 2023
| local outName | ||
| for outName in $outputs; do | ||
| addToSearchPath DLLPATH "${!outName}/bin" | ||
| addToSearchPath outputsDllPath "${!outName}/lib64" |
There was a problem hiding this comment.
the new PR does not check lib64
wegank
added
2.status: stale
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here's a draft of what needs to be done. I don't really have time to test it so if one of you could take this over that would be great.
Motivation for this change
DLLs are symlinked bin/ for the exes that use them, but they should always be installed in lib/ for sake of regularity and multiple outputs.
Fix #38451
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)CC @expipiplus1 @angerman @dtzWill