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 DLL/LIB install locations on Windows #20

Merged
merged 14 commits into from
Jul 30, 2018

Conversation

jakirkham
Copy link
Member

Fixes #19

Appears that Blosc installed everything to the lib directory. However this makes it hard to load DLLs at runtime as they are not in bin directory, which is added to the path. This tries to correct that issue by setting the runtime and archive output directories to bin and lib respectively as well as checking their contents after the build.

Should make sure the DLL and LIB files are installed in the proper
locations.
Rebuild the package now that libraries are installed in the correct
locations.
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

This was referenced Mar 10, 2018
@jakirkham
Copy link
Member Author

This installs to the right location. However the test fails to link (presumably due to the dynamic library moving location). Maybe there is another CMake option needed here? Suggestions welcome.

@jakirkham
Copy link
Member Author

Any ideas what might be going wrong here, @FrancescAlted?

@tomkooij tomkooij mentioned this pull request May 2, 2018
@tomkooij
Copy link
Contributor

tomkooij commented May 2, 2018

This should be fixable by copying bld.bat from the anaconda recipe: https://github.com/AnacondaRecipes/blosc-feedstock/

@tomkooij
Copy link
Contributor

tomkooij commented May 2, 2018

I simply fetched bld.bat from AnacondaRecipes

Tests seem to pass: https://ci.appveyor.com/project/tomkooij/blosc-feedstock/build/1.0.5/job/tn0kb2ph92dnfgsy

@jakirkham : I pushed a PR against this in your fork jakirkham-feedstocks#1
(But feel free to just cherry-pick/squash/whatever is easiest)

Use anaconda version of bld.bat
recipe/bld.bat Outdated
if %ERRORLEVEL% neq 0 exit 1

del %LIBRARY_BIN%\msvc*.dll
del %LIBRARY_BIN%\Microsoft.*
Copy link
Member Author

Choose a reason for hiding this comment

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

These bits seem a bit curious. Do we know why they got added?

recipe/bld.bat Outdated

del %LIBRARY_BIN%\msvc*.dll
del %LIBRARY_BIN%\Microsoft.*
move %LIBRARY_LIB%\blosc.dll %LIBRARY_BIN%\
Copy link
Member Author

Choose a reason for hiding this comment

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

This is interesting. Would have hoped this got installed in the right place already. Might need another look to figure out what is happening (or not happening) to require this.

recipe/bld.bat Outdated
@@ -14,14 +14,21 @@ cmake -G "NMake Makefiles" ^
-DBUILD_SHARED:BOOL=ON ^
-DBUILD_TESTS:BOOL=ON ^
-DBUILD_BENCHMARKS:BOOL=OFF ^
-DPREFER_EXTERNAL_SNAPPY:BOOL=ON ^
-DPREFER_EXTERNAL_LZ4:BOOL=ON ^
-DPREFER_EXTERNAL_ZLIB:BOOL=ON ^
Copy link
Member Author

Choose a reason for hiding this comment

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

Since we are not currently including these, might be best to disable them for now. Though agree we should strive to use external packages to meet these needs as much as possible. Already issue ( #18 ) is open on this point.

This was needed to align the VC requirement at runtime with old versions
of `conda-build`. It is no longer needed with newer versions of
`conda-build` as this is handled automatically with the package. So go
ahead and drop it.
The `conda inspect` commands are being deprecated in `conda-build`. Plus
these are not needed with `conda-build` 3 as it includes this
information anyways.
The extension varies between the two platforms, but that can be smoothed
over by `conda-build` with `$SHLIB_EXT`. So go ahead and combine these
tests together.
As we don't include these dependencies, we shouldn't be searching for
them when building. Otherwise we might pick up system copies on the
build machine, which wouldn't be available at install time. So drop
these flags for now until those dependencies are supplied.
@jakirkham
Copy link
Member Author

@conda-forge-admin, please relint.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@jakirkham jakirkham merged commit 2fd13ff into conda-forge:master Jul 30, 2018
@jakirkham jakirkham deleted the fix_install_loc branch July 30, 2018 22:27
@jakirkham
Copy link
Member Author

Thanks for helping with this @tomkooij.

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

Successfully merging this pull request may close these issues.

3 participants