Skip to content

Conversation

@stephanlachnit
Copy link
Contributor

Will fail to build on Windows due to #2436 (comment) (this problem already persists with current cpr). I pledge to merge this anyway since this doesn't change the status quo. Alternatively, I could disable Windows builds in CI and error in the Meson build files if this is preferred.

@stephanlachnit
Copy link
Contributor Author

Opened an issue about symbol visibility here: libcpr/cpr#1273

Copy link
Collaborator

@bgilbert bgilbert left a comment

Choose a reason for hiding this comment

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

Better to let Windows CI continue to fail, I think.

version: '1.13.0',
license: 'MIT',
default_options: 'cpp_std=c++17',
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's also add meson_version: '>=0.63.0' while we're here. (Alternatively 0.54.0 if you move cpp_std to override_options on the library() and executable() calls, preferably via an intermediate variable.)

includes = include_directories('.', 'include')
subdir('cpr')

includes = include_directories('include')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think . was included because cpr/cpr.h includes cpr/cprver.h, and the latter is generated in cpr/ in the build directory and currently is not installed. So this will break the includes.

You should install cprver.h, of course, but that won't help a superproject or sibling subproject find that header when cpr hasn't been installed. I'd suggest generating cprver.h from a new include/cpr/meson.build, and also calling install_headers() from there. (I'm open to other ideas, though.)

If you do that, and invoke subdir('include/cpr') before subdir('cpr'), includes shouldn't need to contain '.' at all.

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.

2 participants