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

build: fix can't find rapidjson/prettywriter.h #1224

Closed
wants to merge 1 commit into from
Closed

build: fix can't find rapidjson/prettywriter.h #1224

wants to merge 1 commit into from

Conversation

Crazyokd
Copy link

@Crazyokd Crazyokd commented Aug 3, 2024

fix #1223

I found the rapidjson_dep only used in example. And the meson seems to discard unused parameter information.

So add rapidjson_dep to examples/meson.build directly maybe a better choice?

@kiplingw
Copy link
Member

kiplingw commented Aug 3, 2024

If it needs to be added as a build dependency then you also need to add it to debian/control. However, if I recall correctly, there was a good reason @Tachi107 had raised ages ago why it was never an explicit build dependency. But I can't remember the reason.

@kiplingw kiplingw requested a review from Tachi107 August 3, 2024 05:32
@kiplingw kiplingw added bug fix in progress dependencies Pull requests that update a dependency file labels Aug 3, 2024
Tachi107 added a commit that referenced this pull request Aug 3, 2024
Since a RapidJSON header is included in
include/pistache/serializer/rapidjson.h, users using pistache must also
add RapidJSON to their include path. Adding rapidjson_dep to the list of
dependencies of pistache_dep makes rapidjson_dep a transitive (public)
dependency, doing just that.

Should fix the issue encountered by Duncan in
<https://github.com/pistacheio/pistache/pull/1221/files#r1693921738>.
Also closes #1224
@Crazyokd
Copy link
Author

Crazyokd commented Aug 3, 2024

now if I need pass tests, I should modify the workflows, and add -DPISTACHE_USE_RAPIDJSON=true in the stage of configuring meson.

before that expect your better reason.

@Tachi107
Copy link
Member

Tachi107 commented Aug 3, 2024

Hi @Crazyokd, I've pushed #1225 which should fix the issue you've encountered. Also, note that RapidJSON is not used only in the examples, but also in include/pistache/serializer/rapidjson.h. This is the reason why this patch broke CI.

@kiplingw, rapidjson-dev is already listed in debian/control :)

@kiplingw kiplingw closed this in 0524ff1 Aug 3, 2024
@Crazyokd
Copy link
Author

Crazyokd commented Aug 4, 2024

@Tachi107 Yeah, that's a better fix.

dgreatwood pushed a commit to dgreatwood/pistachefork that referenced this pull request Nov 1, 2024
Since a RapidJSON header is included in
include/pistache/serializer/rapidjson.h, users using pistache must also
add RapidJSON to their include path. Adding rapidjson_dep to the list of
dependencies of pistache_dep makes rapidjson_dep a transitive (public)
dependency, doing just that.

Should fix the issue encountered by Duncan in
<https://github.com/pistacheio/pistache/pull/1221/files#r1693921738>.
Also closes pistacheio#1224
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug dependencies Pull requests that update a dependency file fix in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

failed to build example
3 participants