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.

Sign up for GitHub

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

Jump to bottom

Create windows packaging #4

Merged
merged 1 commit into from
May 17, 2021
Merged

Create windows packaging #4

merged 1 commit into from
May 17, 2021

Conversation

adriendelsalle
Copy link
Member

@adriendelsalle adriendelsalle commented Jul 31, 2020 •
edited

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.
  • Fix nlohmann_json feedstock for broken dependency

Description

Related to Windows packaging issue where Windows packages are empty.

Proposition

Add a bld.bat file to handle Windows packaging

Discussion

@wolfv
nlohmann_json feedstock is currently broken and need to be fixed before packaging this project (v3.8.0 required and not available on conda-forge).
I was not able to build locally and check the recipe due to this other issue.

@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.

@wolfv
Copy link
Member

wolfv commented Aug 3, 2020

nlohmann json is updated now :)

@adriendelsalle
Copy link
Member Author

@conda-forge-admin, please rerender

@adriendelsalle
Copy link
Member Author

adriendelsalle commented May 16, 2021 •
edited

Thanks @wolfv it should be fixed now!
I also removed test executable from build to have minimal tarball

@adriendelsalle adriendelsalle marked this pull request as ready for review May 16, 2021 19:39
@wolfv
Copy link
Member

wolfv commented May 17, 2021

Would be cool to add some tests to check that a dll is created and header files are placed in the right locations on Windows.

wolfv
wolfv reviewed May 17, 2021
View reviewed changes
recipe/build.sh Outdated
cmake -DCMAKE_INSTALL_PREFIX=$PREFIX -DBUILD_SHARED_LIBS=y . -Bbuilddir
cmake . \
-DCMAKE_INSTALL_PREFIX=$PREFIX \
-DBUILD_SHARED_LIBS=y \
Copy link
Member

Choose a reason for hiding this comment

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

maybe instead of y also use ON or TRUE?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just copy/pasted from linux existing build script but if you prefer I can also change that

Copy link
Member

Choose a reason for hiding this comment

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

yep would be nice!

wolfv
wolfv reviewed May 17, 2021
View reviewed changes
recipe/meta.yaml Outdated
- test -f ${PREFIX}/lib/libnlohmann_json_schema_validator.so # [unix]
- test -f ${PREFIX}/lib/libnlohmann_json_schema_validator.dll # [win]
Copy link
Member

Choose a reason for hiding this comment

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

might also be good to check %PREFIX%\bin\nlohmann_json_schema_validator.bin` (I think).

Also ${PREFIX} isn't gonna work on win

@wolfv
Copy link
Member

wolfv commented May 17, 2021

can you also activate osx?

@wolfv
Copy link
Member

wolfv commented May 17, 2021

Note the missing lib-prefix.

-- Installing: D:/bld/json_schema_validator_1621241783930/_h_env/Library/lib/nlohmann_json_schema_validator.lib
-- Installing: D:/bld/json_schema_validator_1621241783930/_h_env/Library/bin/nlohmann_json_schema_validator.dll
-- Installing: D:/bld/json_schema_validator_1621241783930/_h_env/Library/include/nlohmann/json-schema.hpp

@adriendelsalle adriendelsalle force-pushed the create-windows-packaging branch 2 times, most recently from 9ada236 to 4fa0175 Compare May 17, 2021 10:03
@adriendelsalle
bump build number
50673b3 
update linux and win build scripts
add win tests
activate osx
@wolfv wolfv merged commit 9eeb590 into conda-forge:master May 17, 2021
@adriendelsalle adriendelsalle deleted the create-windows-packaging branch May 17, 2021 11:56
@adriendelsalle adriendelsalle mentioned this pull request May 17, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wolfv wolfv wolfv left review comments

@asafkahlon asafkahlon Awaiting requested review from asafkahlon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@adriendelsalle @conda-forge-linter @wolfv