Add RapidJSON recipe #2192

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

@wesm
Member
wesm commented Jan 10, 2017

This is a header-only C++ library

@conda-forge-linter

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 (recipes/rapidjson) and found it was in an excellent condition.

@wesm wesm Add RapidJSON recipe
78e5837
@SylvainCorlay

I was about to work on this recipe and came across your PR. I left a few comments regarding the cmake installation steps.

+
+cmake -DRAPIDJSON_HAS_STDSTRING=ON \
+ -DCMAKE_INSTALL_PREFIX=$PREFIX \
+ -DRAPIDJSON_BUILD_CXX11=ON \
@SylvainCorlay
SylvainCorlay Jan 17, 2017 Member

This is already the default value for RAPIDJSON_BUILD_CXX11.

+ md5: badd12c511e081fec6c89c43a7027bce
+
+build:
+ number: 1
@SylvainCorlay
SylvainCorlay Jan 17, 2017 Member

Shouldn't the build number start at 0?

+test:
+ commands:
+ - test -e $PREFIX/include/rapidjson/rapidjson.h # [unix]
+ - if exist %LIBRARY_INC%\rapidjson\rapidjson.h (exit 0) else (exit 1) # [win]
@SylvainCorlay
SylvainCorlay Jan 17, 2017 Member

Could we add a test for the presence of the RapidJSONConfig.cmake and RapidJSONConfigVersion.cmake?

The RapidJSONcmake install places these files in directory which is PREFIX/lib/cmake/RapidJSON which is quite unusual but resolved correctly by cmake.

+
+extra:
+ recipe-maintainers:
+ - wesm
@SylvainCorlay
SylvainCorlay Jan 17, 2017 edited Member

Would you be OK adding me and @JohanMabille as maintainers of the recipe? We were just about to propose a recipe for this.

+set RAPIDJSON_DIR=%SRC_DIR%
+
+REM Copy headers
+xcopy /S %RAPIDJSON_DIR%\include %LIBRARY_INC%
@SylvainCorlay
SylvainCorlay Jan 17, 2017 Member

Would it be possible to use the make install approach on windows too? The benefit is that it will place the rapidjsonConfig.cmake and rapidjsonConfigVersion.cmake files in the right configuration folder for cmake under the prefix, allowing cmake to find it when building packages that depend upon rapidjson.

@SylvainCorlay
SylvainCorlay Jan 17, 2017 Member

(We can confirm that the make install works on windows for rapidJSON.)

@wesm
wesm Jan 18, 2017 Member

This is a bit outside my expertise -- can you propose a new recipe (feel free to start from this one if you'd like) with the Windows fixes? I am happy to be on the maintainer list

+ ..
+
+make -j${CPU_COUNT}
+ctest
@JohanMabille
JohanMabille Jan 17, 2017

Not sure you can run ctest if you don't build the examples (with -DRAPIDJSON_BUILD_TESTS=ON).

@wesm
Member
wesm commented Jan 18, 2017

hey @SylvainCorlay and @JohanMabille, I will absolutely make you maintainers. As an aside, I don't support I could propose a collaboration to help maintain the C++ library stack necessary to build Apache Arrow and Parquet's C++ libraries on Windows. I'm a bit lacking in Windows expertise

@wesm
Member
wesm commented Jan 18, 2017

Per my comment above, I think it would be better if you took over this initial recipe since you know the Windows issues better than I do (I'm working on getting a Windows testing VM set up)

@ocefpaf ocefpaf closed this in #2236 Jan 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment