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

[sqlite-orm] enable C++17 and C++20 #43168

Open
petersteneteg opened this issue Jan 8, 2025 · 4 comments
Open

[sqlite-orm] enable C++17 and C++20 #43168

petersteneteg opened this issue Jan 8, 2025 · 4 comments
Assignees
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist

Comments

@petersteneteg
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Currently sqlite-orm disables features from c++17 and c++20

-DSQLITE_ORM_ENABLE_CXX_17=OFF
-DSQLITE_ORM_ENABLE_CXX_20=OFF

Is there any reason for this? I would like to enable them.
And if not as default can one add it as a feature?

Or should this depend on the triplet used?

Proposed solution

Remove the mentioned lines above

Describe alternatives you've considered

Derive this from the triplet used

Additional context

No response

@petersteneteg petersteneteg added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Jan 8, 2025
@JonLiu1993
Copy link
Member

JonLiu1993 commented Jan 9, 2025

@trueqbit, Some users hope that vcpkg can enable these two C++ standards,

-DSQLITE_ORM_ENABLE_CXX_17=OFF
-DSQLITE_ORM_ENABLE_CXX_20=OFF 

but I noticed that when the upstream added it, it was disabled by default because some configurations could not be satisfied. I wonder if we can turn on these two options normally now? If so, I will turn them on in vcpkg.

@dg0yt
Copy link
Contributor

dg0yt commented Jan 9, 2025

The options might not be supported by old compilers. Maybe it is also not possible to combine source code which is written for C++11 with the library when compiled with C++20.

It is possible already now to customize the triplet file:

if(PORT STREQUAL "sqlite-orm")
    list(APPEND VCPKG_CMAKE_CONFIGURE_OPTIONS "-DSQLITE_ORM_ENABLE_CXX_17=ON")
endif()

https://learn.microsoft.com/en-us/vcpkg/users/triplets#per-port-customization

@petersteneteg
Copy link
Contributor Author

@dg0yt Thanks for the tip! I did not know about that possibility of modifying a port through the triplet file. I expected to have to do a overlay port.

I would argue that the options was added long ago, 2019 and 2021, so I think compiler support should not be much of an issue anymore. And one can also disable the settings in a triplet file in the same way.

If one builds with a compiler that has c++20 support. I think the option should be on by default.

Keeping them off to defend against potential issues that we don't even know if they exists seems overly defensive.

@dg0yt
Copy link
Contributor

dg0yt commented Jan 10, 2025

There is no need to stick to limitations of the past when also CI was at diffent compiler versions.
However, mind that features must be additive. If one port builds with the default configuration and new one asks for another feature, the first port must not break. C++17 also removes some library features IIRC. So this choice doesn't easily fit to "features". A quick grep yields only three ports with "cxx[-0-9][0-9] features:

ports/abseil/vcpkg.json:    "cxx17": {
ports/redis-plus-plus/vcpkg.json:    "cxx17": {
ports/upa-url/vcpkg.json:    "cxx11": {

upa-url uses C++17 by default, and it carries an extra patch to lock the C++ level "so that consuming code gets what matches the feature selection; otherwise there will be ABI breaks", #40964 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist
Projects
None yet
Development

No branches or pull requests

3 participants