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

[cmake common definitions] add variable VCPKG_BUILD_DEBUG #21206

Closed
wants to merge 1 commit into from

Conversation

autoantwort
Copy link
Contributor

Add this convenience variable so that you don't have to write

if(NOT DEFINED VCPKG_BUILD_TYPE OR VCPKG_BUILD_TYPE STREQUAL "debug")
   ...
endif()

all the time.

We can also add VCPKG_BUILD_RELEASE but I think there is no port that supports debug only builds.

@cenit
Copy link
Contributor

cenit commented Nov 7, 2021

We can also add VCPKG_BUILD_RELEASE but I think there is no port that supports debug only builds.

see #16110

vcpkg itself does not support debug only for now

@JonLiu1993 JonLiu1993 assigned JonLiu1993 and JackBoosY and unassigned JonLiu1993 Nov 8, 2021
@JackBoosY JackBoosY added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Nov 8, 2021
@JackBoosY
Copy link
Contributor

We can also add VCPKG_BUILD_RELEASE but I think there is no port that supports debug only builds.

see #16110

vcpkg itself does not support debug only for now

We did not prevent users from setting VCPKG_BUILD_TYPE, so I think it is good.

@Osyotr
Copy link
Contributor

Osyotr commented Nov 8, 2021

@cenit I'm curious, what is the purpose of debug-only library?

@cenit
Copy link
Contributor

cenit commented Nov 8, 2021

I also expect very low usage of such a triplet too. I can imagine quick iterations on -dev version libraries (debug are quicker to build than release, because of missing optimizations, and in case of problems you have full symbols to inspect runtime errors), but nothing more.
I was just saying that while a release-only triplet might have a lot of sense (and I am pushing for it for a long time now, also in CI), a "debug-only one" is broken by design for now in vcpkg, whatever the usage might be

@dg0yt
Copy link
Contributor

dg0yt commented Nov 8, 2021

In fact, we mostly desire to have an easy way to build a single primary config (usually called release, but it might be release with debug info, or with profiling, even per port), without wasting space and time for another config in /debug.
In this regard, I would even accept the assymmetry in only having VCPKG_BUILD_DEBUG.

@cenit
Copy link
Contributor

cenit commented Nov 8, 2021

exactly.
It's not the release, but the functionality of single config that should be more prominent.
We are wasting so much computing time due to errors in ports that break this functionality that vcpkg team budget would be nothing in comparison...

@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Nov 12, 2021
@dan-shaw dan-shaw added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:reviewed Pull Request changes follow basic guidelines labels Nov 13, 2021
@ras0219-msft
Copy link
Contributor

Agreed with points above; the future for VCPKG_BUILD_TYPE is likely to only be either empty or "release" and it really means "Please build only one flavor". Users that want that one flavor to be debug-able can simply use debug flags as their CXXFLAGS. There are still open questions about library-specific debug settings (trace macros, etc), but in the long view I believe we'll need to separate that from the "Debug" versus "Release" configurations.

Given that, and given that VCPKG_BUILD_TYPE == "debug" is hopelessly broken for every non-trivial package, I think ports should use if(NOT VCPKG_BUILD_TYPE) where they would use this VCPKG_BUILD_DEBUG.

@JackBoosY JackBoosY removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Nov 15, 2021
@JackBoosY
Copy link
Contributor

Agreed with points above; the future for VCPKG_BUILD_TYPE is likely to only be either empty or "release" and it really means "Please build only one flavor". Users that want that one flavor to be debug-able can simply use debug flags as their CXXFLAGS. There are still open questions about library-specific debug settings (trace macros, etc), but in the long view I believe we'll need to separate that from the "Debug" versus "Release" configurations.

Given that, and given that VCPKG_BUILD_TYPE == "debug" is hopelessly broken for every non-trivial package, I think ports should use if(NOT VCPKG_BUILD_TYPE) where they would use this VCPKG_BUILD_DEBUG.

But for now, it is unrealistic to use the condition if (NOT VCPKG_BUILD_TYPE). VCPKG_BUILD_TYPE is not defined or empty means that debug and release are built at the same time.

@dg0yt
Copy link
Contributor

dg0yt commented Nov 15, 2021

Given that, and given that VCPKG_BUILD_TYPE == "debug" is hopelessly broken for every non-trivial package, I think ports should use if(NOT VCPKG_BUILD_TYPE) where they would use this VCPKG_BUILD_DEBUG.

But for now, it is unrealistic to use the condition if (NOT VCPKG_BUILD_TYPE). VCPKG_BUILD_TYPE is not defined or empty means that debug and release are built at the same time.

I think that the point was:
VCPKG_BUILD_DEBUG effectively implies NOT [DEFINED] VCPKG_BUILD_TYPE, "given that VCPKG_BUILD_TYPE == "debug" is hopelessly broken for every non-trivial package".

@ras0219-msft
Copy link
Contributor

It's exactly as @dg0yt said, though I'm sorry for using somewhat confusing language. My observations are:

  1. VCPKG_BUILD_TYPE only has two realistic values that work today: "" and "release".
  2. The only two modes that will reliably build across the catalog are "Build debug and release" and "Build release"
  3. Therefore, there isn't terribly much use in a dedicated variable for VCPKG_BUILD_DEBUG because the statement if(VCPKG_BUILD_DEBUG) is exactly the same as if(NOT VCPKG_BUILD_TYPE).
  4. Saving 3 characters doesn't seem worth adding an entire new variable, especially since it may be confusing in the future as we try to move to a distinction of "dual build" versus "single build" instead of "release, debug, or release&debug".

@JackBoosY JackBoosY added invalid and removed category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed labels Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants