Skip to content

[daw-json-link] New port#18411

Merged
dan-shaw merged 147 commits intomicrosoft:masterfrom
mheyman:master
Aug 4, 2021
Merged

[daw-json-link] New port#18411
dan-shaw merged 147 commits intomicrosoft:masterfrom
mheyman:master

Conversation

@mheyman
Copy link
Copy Markdown
Contributor

@mheyman mheyman commented Jun 12, 2021

Describe the pull request

  • What does your PR fix?

    New port for json-link (perhaps the fastest JSON deserializer/serializer). Because we don't yet have enough JSON libraries...

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    all (I hope - only checked windows and linux). No need to update ci.baseline.txt.

  • Does your PR follow the maintainer guide?

Yes.

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    Yes.

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

@JonLiu1993 JonLiu1993 self-assigned this Jun 15, 2021
@JonLiu1993 JonLiu1993 changed the title json-link port [json-link] New port Jun 15, 2021
@JonLiu1993 JonLiu1993 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Jun 15, 2021
JonLiu1993
JonLiu1993 suggested changes Jun 16, 2021
View reviewed changes
@JonLiu1993
Copy link
Copy Markdown
Contributor

JonLiu1993 commented Jun 17, 2021

@mheyman ,Could you please take a look?
config-x86-windows-out.log

@mheyman
Copy link
Copy Markdown
Contributor Author

mheyman commented Jun 18, 2021

The failure looks like it occurs in a cmake FetchContent_MakeAvailable() call. This call is identical for linux and windows. The failure occurs after a find_package(Git QUIET) fails to find git. I suspect git is available in the path for linux and osx but not for windows builds.

Darrell Wright's json-link library pulls in headers from two other of his projects. That is what the FetchContent_MakeAvailable() is trying to do.

If FetchContent_MakeAvailable() (with a requirement on git) is unavailable for vcpkg builds, it is possible that I can rework this port into three ports, one for each project. I'd have to patch-out the calls to FetchContent...(). If I do that, I think I should rename the port from json-link to daw-json-link with a dependency on daw-utf-range and daw-header-libraries to make it more obvious they are related (daw-utf-range is also dependent on daw-header-libraries). Much of the header files already land under a daw/ directory (I'm a little dissapointed they don't all land under daw/).

Is that how I should proceed? Make three ports that never call FetchContent_MakeAvailable()? Or, is there an easier solution?

@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Jun 21, 2021

@mheyman For dependent third-party libraries, we have two ways:

  1. Use the port in vcpkg if it exists.
  2. Add vcpkg_from_github or other download methods to pre-download in portfile.cmake, and copy the decompressed folder to source path.

@JonLiu1993
Copy link
Copy Markdown
Contributor

JonLiu1993 commented Jul 6, 2021

@mheyman ，Are you still working on fixing this pr?

@mheyman
Copy link
Copy Markdown
Contributor Author

mheyman commented Jul 6, 2021 via email

@JackBoosY JackBoosY marked this pull request as draft July 7, 2021 02:03
Neumann-A and others added 18 commits July 10, 2021 14:07
@Neumann-A @mheyman
[vcpkg-tools] update nuget to 5.9.1 (#18447)
9b38654
@NancyLi1013 @mheyman
[vcpkg_download_distfile] Add condition for padding the SHA (#18459)
a6067ec
@traversaro @mheyman
[open6254] Enable uwp support (#18390)
1723d90 
* open62541: Enable uwp support

* Update versions for open62541
@NancyLi1013 @mheyman
[aubio] Add ws2_32 to linkage (#18385)
1af0e45 
* [aubio] Add ws2_32 to linkage

* Update version files
@PhoebeHui @mheyman
[devil] Fix ilut header (#18384)
f1679eb 
* [devil Fix ilut header

* Update baseline
@Neumann-A @mheyman
[libpq] Add Secur32.lib and openssl to libpq cmake wrapper (#18377)
0fd86ab 
* [libpq] add secur32.lib to wrapper

* version stuff

* add openssl fix.

* fix version stuff
@MichaelGoulding @mheyman
[ryml] [c4core] Update RapidYaml (ryml) to newer version (#18230)
53f90f0 
* remove old port version

* fix versions yet again

Co-authored-by: Michael Goulding <Michael.Goulding@microsoft.com>
@myd7349 @mheyman
[yyjson] Update to v0.3.0 (#18359)
862d38b 
* [yyjson] Update to 0.3.0

* [yyjson] vcpkg x-add-version yyjson
@Pospelove @mheyman
[raylib] Fix MinGW build (#18362)
999761e 
.
@ianmcook @GPSnoopy
@ras0219-msft @mheyman
[arrow] Update to 4.0.0 (#17975)
356c9fb 
* Update arrow to 4.0.0

* Format

* Try fix thrift

* Update versions/ files

* Do not set ZSTD_ROOT

* Remove double quotes causing Windows problems

* Apply patches

* Remove LIB_DIR_OPTIONS

* Tweak zstd flags

* Update version hash

* Format

* Fail early on x86

* Update hash

* Fail early on arm, arm64

* Update hash

* Add expected failures to to scripts/ci.baseline.txt

* Exclude mallocs from default features

* Update hash

* Set default-features to false for aws-sdk-cpp

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>

* Specify only x64 support in manifest

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>

* Remove unneeded ci.baseline.txt entries

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>

* Remove dataset from default-features

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>

* Update hash

* Remove zstd path args

* Update hash

Co-authored-by: Tanguy Fautre <tanguy@fautre.com>
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
@JackBoosY @mheyman
[minizip] Fix usage, fix minizipConfig.cmake (#18472)
3723946 
* [minizip] Fix usage, fix minizipConfig.cmake

* update version record
@dg0yt @mheyman
[windres-rc] Fix wrapper output (#17045)
305cd6a
@Sibras @BillyONeal
@JackBoosY @mheyman
[ffmpeg] Warn/fail when selecting unsupported features. (#17153)
17e4a24 
* [ffmpeg] Warn/fail when selecting unsupported features.

* Update ffmpeg.json

* Bump port-version

* Update ffmpeg.json

* [ffmpeg] Check for gpl/nonfree aswell as all option.

* avisynth now support static linking

* fixup typo

* Update ffmpeg.json

* Revert to fatal error on even when all is selected

* Update ffmpeg.json

* [ffmpeg] Disable openh264 on uwp

* update

* Update versions/f-/ffmpeg.json

* update

* Update ffmpeg.json

* update

* update

* Update

Co-authored-by: Billy Robert ONeal III <bion@microsoft.com>
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
@JonLiu1993 @mheyman
[uwebsockets] update to 19.2.0 (#18525)
38d9bae 
* [uwebsockets] update to <v19.2.0>

* update version
@mcmtroffaes @mheyman
[libass] fix fontconfig dependency in .pc file (#18516)
0962c2e 
* [libass] fix fontconfig dependency in .pc file on x64-linux

* [libass] bump port version

* [libass] x-add-version
@NancyLi1013 @mheyman
[libgpg-error] Remove COPYING.LIB from lib folder (#18492)
dc318e4 
* [libgpg-error] Remove COPYING.LIB from lib folder

* Update version files
@NancyLi1013 @mheyman
[flashlight-cuda] Fix installation (#18489)
13e202e 
* [flashlight-cuda] Fix installation

* Update version files
@gjasny @mheyman
[function2] Update to version 4.2.0 (#18487)
420c0e7
@JackBoosY JackBoosY added requires:author-response depends:upstream-changes Waiting on a change to the upstream project labels Jul 21, 2021
@mheyman mheyman mentioned this pull request Jul 22, 2021
Closed
@beached
Copy link
Copy Markdown

beached commented Jul 23, 2021

One comment on the testing. It is primarily tested on x64 linux/macos/windows in c++17, in develop C++20 too. It has less primarily tested on 32bit arm/32bit windows but not as a first class. When travis had free CI it was tested on BE ppc and/or S390 I think. It's tested in constant expressions so should work on all the triplets, but I don't know that with certainty

beached added a commit to beached/daw_json_link that referenced this pull request Jul 23, 2021
@beached
Updates to facilitate vcpkg integration
8335ec9 
Updates to facilitate vcpkg integration
microsoft/vcpkg#18411
#247

* Added option to disable FetchContent of dependencies DAW_USE_PACKAGE_MANAGEMENT
* Moved third_part into daw/ subfolder to prevent collisions
* Changed project name to daw-json-link but maintained alias library of
  daw::json_link
@JackBoosY JackBoosY removed the depends:upstream-changes Waiting on a change to the upstream project label Jul 23, 2021
@mheyman
Copy link
Copy Markdown
Contributor Author

mheyman commented Jul 25, 2021

The latest pull request includes the excellent source updates from beached that removed any need for patch files.

JackBoosY
JackBoosY suggested changes Jul 26, 2021
View reviewed changes
@mheyman
Copy link
Copy Markdown
Contributor Author

mheyman commented Jul 31, 2021

Header-only comments applied. Sorry for the delay. Life intervened but I did find a second flock of chickens in the neighborhood :-)

JackBoosY
JackBoosY reviewed Aug 2, 2021
View reviewed changes
@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Aug 2, 2021
@dan-shaw dan-shaw merged commit bd5ea16 into microsoft:master Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] approved these changes

@JonLiu1993 JonLiu1993 Awaiting requested review from JonLiu1993

+2 more reviewers

@beached beached beached left review comments

@JackBoosY JackBoosY JackBoosY approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@JonLiu1993 JonLiu1993

Labels

category:new-port The issue is requesting a new library to be added; consider making a PR! info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.