Skip to content

Add VCPKG_BUILD_TYPE environment variable#17510

Closed
dg0yt wants to merge 1 commit intomicrosoft:masterfrom
dg0yt:build-type
Closed

Add VCPKG_BUILD_TYPE environment variable#17510
dg0yt wants to merge 1 commit intomicrosoft:masterfrom
dg0yt:build-type

Conversation

@dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Apr 26, 2021

  • What does your PR fix?

    This PR adds an environment variable which allows to select a single build type configuration.
    This helps to cut build times and disk storage requirements in local development and in
    continuous integration setups, without requiring modification to triplet files.

    However, ATM the only supported value is release (cf. [x64-windows-release] add a single config community triplet based on x64-windows and make it work on some ports #15983 (comment), [CI] debug variant of #15983 #16110). I still think it might be useful. Issues and PRs show that the release-only configuration is used. Removing barriers to test release-only builds locally can improve PR quality even in absence of CI coverage.

    The placement of the new macro was chosen to interact with ABI tracking without needing changes to vcpkg tool. When accessing vcpkg_get_tags.cmake, vcpkg tool modifies CMAKE_CURRENT_LIST_DIR so I didn't want to include another file from that cmake file.

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

    all, -/-

  • 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?

    -/-

@JackBoosY JackBoosY self-assigned this Apr 26, 2021
@Neumann-A
Copy link
Contributor

I think a community triplet or a triplet overlay is better for this kind of stuff.

@JackBoosY JackBoosY added requires:discussion category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed labels Apr 26, 2021
@BillyONeal
Copy link
Member

I think this breaks binary caching?

I cancelled the ongoing build because it's the only thing running on the just replaced VMs; after result of discussion we'll either close this or merge with master (which will trigger another rebuild-the-world, but this time on the new VMs)

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 26, 2021

I think a community triplet or a triplet overlay is better for this kind of stuff.

Hm, this answer doesn't come unexpected.

ATM I'm patching the triplets for my personal CI. (All, for simplicity. Four triplets are actually used, but I'm waiting for Android.) I touch this on each of the three machines which I use for local tests (Windows, Linux, macOS).
It really doesn't really feel like triplets are the answer to all questions. Or overlay triplets must become additive.

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 26, 2021

I think this breaks binary caching?

It is meant to create different cache keys to avoid breaking binary caching.
However, I do understand that a mix of build types in a single installed tree might not be desired.

I cancelled the ongoing build

Fine, I really didn't want to rebuild the world. But I can neither stop nor start anything here. I will try to not update the PR without consent.

@BillyONeal
Copy link
Member

I cancelled the ongoing build

Fine, I really didn't want to rebuild the world. But I can neither stop nor start anything here. I will try to not update the PR without consent.

Right, I just wanted you to understand that I didn't cancel it as a vote against doing this and it was for infrastructure reasons under the covers :)

@Neumann-A
Copy link
Contributor

ATM I'm patching the triplets for my personal CI. (All, for simplicity. Four triplets are actually used, but I'm waiting for Android.) [....] Or overlay triplets must become additive.

I think you have not fully understood the power of overlay triplets. You can create arbitrary named triplets with it and make them available to vcpkg with an overlay, e.g. x64-windows-release or something similar. The same applies to overlay ports.

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 26, 2021

ATM I'm patching the triplets for my personal CI. (All, for simplicity. Four triplets are actually used, but I'm waiting for Android.) [....] Or overlay triplets must become additive.

I think you have not fully understood the power of overlay triplets. You can create arbitrary named triplets with it and make them available to vcpkg with an overlay, e.g. x64-windows-release or something similar. The same applies to overlay ports.

I think I understood the power, it is complete cmake script mode language. Only to verify with regard to binary caching and ABI tracking for custom triplets: Can I safely use include("base-triplet.cmake") in a custom triplet and have the contents of the included file affect ABI tags? (This is what I meant by 'additive'.)

@Neumann-A
Copy link
Contributor

Can I safely use include("base-triplet.cmake")

no but you could additionally do the following in the triplet to be safe:

set(BASE_TRIPLET_HASH "someprecaluclatedhash")
file(SHA512 "path_to_included_base_triplet" COMPARE_HASH)
if(NOT COMPARE_HASH EQUAL BASE_TRIPLET_HASH )
     message(FATAL_ERROR)
endif()

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 27, 2021

a mix of build types in a single installed tree might not be desired.

Perhaps this is the strongest argument against this PR? It may cause build failures in a mixed installation (i.e. after manual install and removal with different build types).

not fully understood the power of overlay triplets

Maybe the hard thing is not understanding the power of triplets but adopting the particular vcpkg triplet mindset that a triplet is 'an imaginary "target configuration set" for every library'. (This is a reflection, not meant negative.) If vcpkg triplets are to define a consistent build tree, then this PR fails to comply with that.


I'm still looking for a way on how to give software developers an easy way to do fast release-only builds, cross-platform, out of the box.
Would this be more acceptable:

$ vcpkg install vcpkg-release-triplets
...
The package vcpkg-release-triplets created the following triplets:

    x64-linux-release
...
    release (based on triplet x64-linux)
 
$ vcpkg install zlib:release
...
-- Using community triplet release. This triplet configuration is not guaranteed to succeed.
...
-- Configuring release-rel
-- Building release-rel
...
$ $ find installed/release/
installed/release/
installed/release/share
installed/release/share/zlib
installed/release/share/zlib/vcpkg_abi_info.txt
installed/release/share/zlib/usage
installed/release/share/zlib/copyright
installed/release/lib
installed/release/lib/libz.a
installed/release/lib/pkgconfig
installed/release/lib/pkgconfig/zlib.pc
installed/release/include
installed/release/include/zconf.h
installed/release/include/zlib.h

where the convenience name 'release' maps to the default triplet + VCPKG_BUILD_TYPE=release.

(This needs some extra care to reasonably deal with VCPKG_DEFAULT_TRIPLET, VCPKG_DEFAULT_HOST_TRIPLET, and vcpkg remove vcpkg-release-triplets but it looks feasible.)

@dg0yt
Copy link
Contributor Author

dg0yt commented May 6, 2021

Withdrawing this PR in its current form. It is too error-prone with the current design and state of vcpkg.

@dg0yt dg0yt closed this May 6, 2021
@dg0yt dg0yt deleted the build-type branch August 16, 2023 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants