Windows: Take 2 - Githubissues

conda-forge / cunit-feedstock

A conda-smithy repository for cunit.

BSD 3-Clause "New" or "Revised" License

0 stars 7 forks source link

Windows: Take 2 #10

Closed Tobias-Fischer closed 3 years ago

Tobias-Fischer commented 3 years ago

Checklist

[x] Used a personal fork of the feedstock to propose changes
[x] Bumped the build number (if the version is unchanged)
[ ] Reset the build number to 0 (if the version changed)
[x] Re-rendered with the latest conda-smithy (Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[x] Ensured the license file is being packaged.

conda-forge-linter commented 3 years ago

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.

Tobias-Fischer commented 3 years ago

@conda-forge-admin, please rerender

Tobias-Fischer commented 3 years ago

So it seems like this builds fine. It is based on #7. It creates and installs a cunit.lib, but not a cunit.dll. I'm not a Windows person and have no idea whether there should be a dll, too. I commented out some stuff in the CMakeLists.txt as it was throwing an error.

Could you have a look and let me know what you think @traversaro @wolfv?

wolfv commented 3 years ago

I think we just have to turn this option on BUILD_SHARED_LIBS. I can push that change

wolfv commented 3 years ago

thanks for picking this up, let's get it over the finish line this time :)

wolfv commented 3 years ago

dll is built, but installed into lib, instead of bin. It should be installed into bin. I am checking.

Tobias-Fischer commented 3 years ago

Do we have to comment the stuff in the CMakeLists back in?

wolfv commented 3 years ago

ah, no I think pdb files are for debugging. I just changed RUNTIME DESTINATION from lib to bin. Let's see if that does the trick.

traversaro commented 3 years ago

The last problem is that the check for existence checks for libcunit.lib, but the default CMake convention for names of add_library(cunit) (or also find_library(cunit) ) is:

Unix : libcunit.<ext>
Windows: cunit.<ext>

traversaro commented 3 years ago

ah, no I think pdb files are for debugging. I just changed RUNTIME DESTINATION from lib to bin. Let's see if that does the trick.

FYI the DESTINATION argument is not required anymore since CMake 3.14 (see https://cmake.org/cmake/help/v3.14/command/install.html#targets). However, the default value uses CMAKE_INSTALL_LIBDIR (if GNUInstallDirs was previously included) that default to lib64 on non-Debian distros, so for conda-forge use is important to set it to lib explicitly.

github-actions[bot] commented 3 years ago

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

linter: passed
drone: passed
travis: passed
azure: passed

Thus the PR was passing and merged! Have a great day!