Skip to content

[cpprestsdk] patch to fix clang error with unique_ptr to incomplete type#51307

Draft
nmatteo-scitec wants to merge 1 commit into
microsoft:masterfrom
nmatteo-scitec:add-cpprestsdk-patch
Draft

[cpprestsdk] patch to fix clang error with unique_ptr to incomplete type#51307
nmatteo-scitec wants to merge 1 commit into
microsoft:masterfrom
nmatteo-scitec:add-cpprestsdk-patch

Conversation

@nmatteo-scitec
Copy link
Copy Markdown

Clang (at least in versions 18, 19, and 20) in C++23 mode using libstdc++ fails to compile cpprestsdk due to issue llvm/llvm-project#111185 involving a unique_ptr to an incomplete type.

Defining the json::value destructor in the .cpp file where the full definition is visible fixes the issue and allows compiling with C++23.

Cpprestsdk upstream is no longer being updated.

  • 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, or no changes were necessary.
  • Any fixed CI baseline and CI feature baseline entries are removed from that file, or no entries needed to be changed.
  • All patch files in the port are applied and succeed.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Exactly one version is added in each modified versions file.

Copy link
Copy Markdown
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

vcpkg is not a clearinghouse for unmaintained things to become maintained; if upstream is gone we need to remove it entirely rather than trying to patch around it.

@nmatteo-scitec
Copy link
Copy Markdown
Author

vcpkg is not a clearinghouse for unmaintained things to become maintained; if upstream is gone we need to remove it entirely rather than trying to patch around it.

Upstream is not entirely 'gone'; they (Microsoft) claim they will continue to fix critical bugs and address security issues. But I don't think this compilation fix falls into either of those categories.

I think this patch is pretty comparable in scope and intent to the ones that are already present in the port. If those other fixes to make sure the package is compatible with various compilers and systems were deemed appropriate it seems to me that this one should be as well.

@nmatteo-scitec
Copy link
Copy Markdown
Author

The failures compiling on android and windows platforms seem to be caused by the issue microsoft/cpprestsdk#1812 and would not be affected one way or the other by this patch.

@nmatteo-scitec
Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="SciTec"

@nmatteo-scitec nmatteo-scitec marked this pull request as ready for review May 4, 2026 14:56
@nmatteo-scitec nmatteo-scitec requested a review from BillyONeal May 4, 2026 14:56
@BillyONeal BillyONeal added the depends:upstream-changes Waiting on a change to the upstream project label May 4, 2026
@BillyONeal
Copy link
Copy Markdown
Member

Upstream is not entirely 'gone'; they (Microsoft) claim they will continue to fix critical bugs and address security issues. But I don't think this compilation fix falls into either of those categories.

That means the bar is like "exploited in the wild" type of bug. It doesn't even compile with current asio, for example, which is why all the asio touching stuff is effectively deindexed in vcpkg.

I think this patch is pretty comparable in scope and intent to the ones that are already present in the port.

I guess that's fair. Just don't be surprised if we are forced to officially deindex it in a short time (~weeks)

@BillyONeal BillyONeal marked this pull request as draft May 5, 2026 23:16
@BillyONeal
Copy link
Copy Markdown
Member

Drafting due to build failures. You may be able to fix by editing scripts/ci.feature.baseline.txt.

@nmatteo-scitec nmatteo-scitec force-pushed the add-cpprestsdk-patch branch 2 times, most recently from 216d538 to c1cdd9f Compare May 6, 2026 17:40
@BillyONeal
Copy link
Copy Markdown
Member

There is a bug in the feature baseline handling I ran into in #51750 and fixed in microsoft/vcpkg-tool#2017

@BillyONeal BillyONeal dismissed their stale review May 14, 2026 22:07

They're still working on it

@nmatteo-scitec nmatteo-scitec force-pushed the add-cpprestsdk-patch branch 2 times, most recently from 5889488 to 6b6e84f Compare May 15, 2026 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

depends:upstream-changes Waiting on a change to the upstream project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants