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

[nlohmann-json] Always install multiple headers to allow forward declarations #12117

Merged
merged 3 commits into from Jul 9, 2020
Merged

[nlohmann-json] Always install multiple headers to allow forward declarations #12117

merged 3 commits into from Jul 9, 2020

Conversation

kevinlul
Copy link
Contributor

Closes #10795

Should work everywhere that the single include already does.

@ras0219
Copy link
Contributor

ras0219 commented Jun 26, 2020

Is there a reason to not simply enable this always?

NancyLi1013
NancyLi1013 suggested changes Jun 28, 2020
View reviewed changes
ports/nlohmann-json/portfile.cmake Outdated Show resolved Hide resolved
ports/nlohmann-json/portfile.cmake Outdated Show resolved Hide resolved
@NancyLi1013 NancyLi1013 added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist requires:author-response labels Jun 28, 2020
@NancyLi1013
Copy link
Contributor

Hi @kevinlul
Thanks for this PR.
Have you tested the feature multiple-headers and if it can work on at least x86-windows, x64-windows, x64-windows-static and x64-linux triplets?

@kevinlul @NancyLi1013
Update ports/nlohmann-json/portfile.cmake
a17db27 
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
@kevinlul
Copy link
Contributor Author

Yes, I have checked on those triplets, made easier by the fact that this is a header-only library. However, I noticed that since #11941 and the 3.8.0 update, because the library now ships its own CMake module files, two directories are created in installed/TRIPLET/share: nlohmann-json for the vcpkg copyright since that's the port name here, and nlohmann_json for the CMakes because that's the self-ascribed name of the library.

@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Jun 29, 2020
@kevinlul kevinlul changed the title [nlohmann-json] Add multiple-headers feature [nlohmann-json] Always install multiple headers to allow forward declarations Jul 6, 2020
@ras0219-msft ras0219-msft merged commit a6ba0da into microsoft:master Jul 9, 2020
@ras0219-msft
Copy link
Contributor

Thanks for the PR!

@kevinlul kevinlul deleted the nlohmann-json-multi branch July 9, 2020 22:35
@SpaceIm SpaceIm mentioned this pull request Apr 12, 2021
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ras0219-msft ras0219-msft ras0219-msft approved these changes

@NancyLi1013 NancyLi1013 NancyLi1013 approved these changes

Assignees

@NancyLi1013 NancyLi1013

Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[nlohmann-json] Support multiple header install
4 participants
@kevinlul @ras0219 @NancyLi1013 @ras0219-msft