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

[json-c] update to 0.17 2023-08-12 #34381

Merged
merged 17 commits into from
Feb 6, 2024
Merged

Conversation

jswillard
Copy link
Contributor

  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@MonicaLiu0311 MonicaLiu0311 added the category:port-update The issue is with a library, which is requesting update new revision label Oct 10, 2023
@MonicaLiu0311
Copy link
Contributor

clamav depends on json-c: ports/clamav/vcpkg.json#L10.

Please get failure logs here:

FAILED: libclamav/CMakeFiles/regex.dir/regex/regerror.c.obj 
C:\PROGRA~1\MICROS~1\2022\ENTERP~1\VC\Tools\MSVC\1437~1.328\bin\Hostx64\x64\cl.exe   -DHAVE_CONFIG_H -DHAVE_STRUCT_TIMESPEC -DWIN32_LEAN_AND_MEAN -ID:\installed\x64-windows\include\libxml2 -ID:\buildtrees\clamav\x64-windows-dbg\libclamav -ID:\buildtrees\clamav\src\av-0.103.0-0a88d9c7f3.clean\libclamunrar_iface -ID:\buildtrees\clamav\src\av-0.103.0-0a88d9c7f3.clean\libclamav\.. -ID:\buildtrees\clamav\src\av-0.103.0-0a88d9c7f3.clean\win32\compat -ID:\buildtrees\clamav\x64-windows-dbg -ID:\buildtrees\clamav\src\av-0.103.0-0a88d9c7f3.clean\libclamav -external:ID:\installed\x64-windows\include -external:ID:\installed\x64-windows\include\json-c -external:W0 /nologo /DWIN32 /D_WINDOWS /W3 /utf-8 /MP  /D_DEBUG /MDd /Z7 /Ob0 /Od /RTC1 /showIncludes /Folibclamav\CMakeFiles\regex.dir\regex\regerror.c.obj /Fdlibclamav\CMakeFiles\regex.dir\ /FS -c D:\buildtrees\clamav\src\av-0.103.0-0a88d9c7f3.clean\libclamav\regex\regerror.c
D:\installed\x64-windows\include\json-c\json_inttypes.h(34): error C2628: 'SSIZE_T' followed by 'int' is illegal (did you forget a ';'?)

@MonicaLiu0311
Copy link
Contributor

Note: I will be converting your PR to draft status. When you're ready, please revert to "ready for review".

@MonicaLiu0311 MonicaLiu0311 marked this pull request as draft October 10, 2023 04:12
@jswillard
Copy link
Contributor Author

clamav has since been updated to fix compatibility with the latest version of json-c, see Cisco-Talos/clamav#1002 - what is the preferred way of handling this? Should I submit a separate PR updating clamav to version 1.2.0, which includes that pr fixing this issue?

@MonicaLiu0311
Copy link
Contributor

MonicaLiu0311 commented Oct 11, 2023

clamav has since been updated to fix compatibility with the latest version of json-c, see Cisco-Talos/clamav#1002 - what is the preferred way of handling this? Should I submit a separate PR updating clamav to version 1.2.0, which includes that pr fixing this issue?

You need to update both clamav and json-c in this PR to ensure that they are installed successfully, thank you.

@MonicaLiu0311
Copy link
Contributor

MonicaLiu0311 commented Oct 19, 2023

Ping @jswillard Any progress here?

@micahsnyder
Copy link

As a heads up we're backporting the clamav - json-0.17 compatibility fix for 1.1 and 1.0 (just merged the fix a bit ago) and will be publishing the new versions sometime next week if all goes well. You're of course welcome to upgrade to 1.2 instead of 1.0 but thought I may as well mention it since @jswillard asked about updating clamav to 1.2.0. Also we'll be publishing 1.2.1 next week in addition.

@jswillard jswillard marked this pull request as ready for review November 7, 2023 17:46
@jswillard
Copy link
Contributor Author

I added two more commits removing the extraneous port version for clamav

@MonicaLiu0311
Copy link
Contributor

The usage test passed on x64-windows (header files found):

json-c provides CMake targets:

  # this is heuristically generated, and may not be correct
  find_package(json-c CONFIG REQUIRED)
  target_link_libraries(main PRIVATE json-c::json-c json-c::json-c-static)

MonicaLiu0311
MonicaLiu0311 previously approved these changes Nov 10, 2023
@MonicaLiu0311 MonicaLiu0311 added the info:reviewed Pull Request changes follow basic guidelines label Nov 10, 2023
@dg0yt
Copy link
Contributor

dg0yt commented Nov 10, 2023

If the suggest usage is:

target_link_libraries(main PRIVATE json-c::json-c json-c::json-c-static)

then we need an explicit usage file.
And reviewers who realize that.

@MonicaLiu0311 MonicaLiu0311 removed the info:reviewed Pull Request changes follow basic guidelines label Nov 10, 2023
@MonicaLiu0311
Copy link
Contributor

The usage test passed on x64-windows and x64-windows-static (header files found):

json-c provides CMake targets:

  # this is heuristically generated, and may not be correct
  find_package(json-c CONFIG REQUIRED)
  target_link_libraries(main PRIVATE json-c::json-c)

ports/json-c/portfile.cmake Outdated Show resolved Hide resolved
@MonicaLiu0311 MonicaLiu0311 added the info:reviewed Pull Request changes follow basic guidelines label Nov 17, 2023
"dll.hpp" )
if(WIN32)
set_target_properties(clamunrar PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS ON)
+ target_sources( clamunrar PRIVATE "isnt.cpp")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you submit this patch upstream?

Copy link
Contributor

Choose a reason for hiding this comment

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

Upstream (1.2.1) is much ahead of this port (0.103.11) and has similar code since 1.0.
Cisco-Talos/clamav@e60843a

Copy link
Contributor

Choose a reason for hiding this comment

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

@jswillard Would you like to update clamav to the latest version 1.2.1?

@data-queue data-queue marked this pull request as draft November 17, 2023 22:36
@MonicaLiu0311 MonicaLiu0311 removed the info:reviewed Pull Request changes follow basic guidelines label Nov 20, 2023
@MonicaLiu0311
Copy link
Contributor

Is there any new progress?

@MonicaLiu0311
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@data-queue
Copy link
Contributor

@jswillard still working on this?

@jswillard
Copy link
Contributor Author

I'm not sure what work still needs to be done here. json-c is updated and the builds are passing status checks. I would prefer not to update clamav to 1.2.1 since I am only interested in json-c being updated. If what's blocking this pr is that patch to clamav, I can go ahead and submit a pr to clamav to get it changed upstream.

@jswillard jswillard marked this pull request as ready for review February 5, 2024 22:14
@MonicaLiu0311
Copy link
Contributor

If what's blocking this pr is that patch to clamav, I can go ahead and submit a pr to clamav to get it changed upstream.

If the latest upstream version has a similar fix, reporting this patch to the upstream may not get a response.

I'm not sure what work still needs to be done here. json-c is updated and the builds are passing status checks. I would prefer not to update clamav to 1.2.1 since I am only interested in json-c being updated.

OK, I understand what you mean and will mark it as info:reviewed.

@MonicaLiu0311 MonicaLiu0311 added the info:reviewed Pull Request changes follow basic guidelines label Feb 6, 2024
@data-queue data-queue merged commit 3975146 into microsoft:master Feb 6, 2024
16 checks passed
TomKatom pushed a commit to TomKatom/vcpkg that referenced this pull request Feb 23, 2024
* [json-c] update to version 0.17 2023-08-12

* [json-c] update version database

* [clamav] update to version 0.103.11

* [clamav] update version db

* [clamav] add patch to compile isnt.cpp on windows

* [clamav] update version db

* [clamav] remove extra port version

* [clamav] update version db

* fix cmakelists.txt

* update version

* fix cmakelists.txt

* update git-tree

* remove fix-cmakelists.patch

* update git-tree

* format

* update git-tree

---------

Co-authored-by: Monica <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants