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.

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

Fix issue 2258 where dependency target binaries are not copied #2259

Merged
merged 4 commits into from
Jun 11, 2022

Conversation

rikkimax
Copy link
Contributor

@rikkimax rikkimax commented Jun 6, 2022

This is very likely to create issues surrounding shared library support in ldc (it will detect regressions), I have entirely left out dmd and gdc support right now.

Fixes #2258.

@rikkimax
Copy link
Contributor Author

rikkimax commented Jun 6, 2022

Okay, looks like I have broken CI somewhat for *nix.

I won't fix that tonight.

But it is working locally for my code base. So close!

@rikkimax
Copy link
Contributor Author

rikkimax commented Jun 7, 2022

After thinking about this, I think I am doing this the wrong way.

For instance the build directory may not even exist by the time I do the copy.

I'll have to start again.

@rikkimax rikkimax reopened this Jun 7, 2022
@rikkimax
Copy link
Contributor Author

rikkimax commented Jun 7, 2022

TIL: determining build artifacts in dub is pretty complicated.

But last build had a couple of green *nix platforms with test running for ldc, so looks like this should be green now.

@rikkimax
Copy link
Contributor Author

rikkimax commented Jun 7, 2022

I think I got super unlucky and just missed a fix for vibe.d that landed only a few minutes after the CI tests went and got built.

vibe-d/vibe.d#2668

As a result this should probably be green now.

@rikkimax
Copy link
Contributor Author

rikkimax commented Jun 7, 2022

Okay this isn't just me: vibe-d/vibe.d#2647

So it's actually green yay.

source/dub/generators/build.d Show resolved Hide resolved
source/dub/compilers/utils.d Outdated Show resolved Hide resolved
source/dub/compilers/utils.d Outdated Show resolved Hide resolved
source/dub/generators/build.d Show resolved Hide resolved
source/dub/generators/build.d Show resolved Hide resolved
Copy link
Member

@WebFreak001 WebFreak001 left a comment

Choose a reason for hiding this comment

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

change LGTM

I don't really have much experience with shared libraries on windows platforms, can you link to some reference why .lib and .exp also need to be copied to the target path? I thought they would be compiled in statically and not need to be copied.

@rikkimax
Copy link
Contributor Author

I don't really have much experience with shared libraries on windows platforms, can you link to some reference why .lib and .exp also need to be copied to the target path? I thought they would be compiled in statically and not need to be copied.

They are compiled in statically yes.

You may want to distribute them for shared libraries, even if they are also being used as a dependency. In that combination, there is no reason to not copy them. It's common enough.

The copying of executable import and export libraries I am not sure is correct, but I am erring on the side of caution and doing it regardless because the last thing we want is people having to dig into .dub for these files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

targetType: dynamicLibrary includes DLL causes compiler to error
4 participants