[vcpkg] Add vcpkg_minimum_required as a replacement for VERSION.txt. #15638

BillyONeal · 2021-01-14T03:57:11Z

No description provided.

ras0219 · 2021-01-14T06:08:23Z

scripts/cmake/vcpkg_minimum_required.cmake

+
+    string(REGEX MATCH "[12][0-9][0-9][0-9]-[01][0-9]-[0-2][0-9]" _vcpkg_matched_test_version "${_vcpkg_VERSION}")
+    if (NOT _vcpkg_matched_test_version STREQUAL _vcpkg_VERSION)
+        message(FATAL_ERROR "VERSION parameter to vcpkg_minimum_required was not a valid date.")


It would be good to print the current vcpkg tool's _vcpkg_matched_base_version in this error case as well.

ras0219 · 2021-01-14T06:09:53Z

toolsrc/CMakeLists.txt

 endif()

+set(VCPKG_BASE_VERSION "2020-01-13")


Suggested change

set(VCPKG_BASE_VERSION "2020-01-13")

set(VCPKG_BASE_VERSION "2021-01-13")

😄

At least I was consistent about it I guess 😂

ras0219 · 2021-01-14T06:10:00Z

scripts/ports.cmake

-endif()
+list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR}/cmake)
+include(vcpkg_minimum_required)
+vcpkg_minimum_required(VERSION 2020-01-13)


Suggested change

vcpkg_minimum_required(VERSION 2020-01-13)

vcpkg_minimum_required(VERSION 2021-01-13)

ras0219 · 2021-01-14T06:10:04Z

scripts/e2e_ports/overlays/vcpkg-requires-old-date/portfile.cmake

@@ -0,0 +1,2 @@
+vcpkg_minimum_required(VERSION 2020-01-12)


Suggested change

vcpkg_minimum_required(VERSION 2020-01-12)

vcpkg_minimum_required(VERSION 2021-01-12)

ras0219 · 2021-01-14T06:10:14Z

scripts/cmake/vcpkg_minimum_required.cmake

+## Usage
+```cmake
+vcpkg_check_linkage(
+    VERSION 2020-01-13


Suggested change

VERSION 2020-01-13

VERSION 2021-01-13

ras0219 · 2021-01-14T06:10:19Z

scripts/bootstrap.ps1

@@ -369,7 +369,8 @@ else
 }

 $arguments = (
-"`"/p:VCPKG_VERSION=-unknownhash`"",
+"`"/p:VCPKG_VERSION=unknownhash`"",
+"`"/p:VCPKG_BASE_VERSION=2020-01-13`"",  # Note: This duplicate date version will be short lived. See https://github.com/microsoft/vcpkg/pull/15474


Suggested change

"`"/p:VCPKG_BASE_VERSION=2020-01-13`"", # Note: This duplicate date version will be short lived. See https://github.com/microsoft/vcpkg/pull/15474

"`"/p:VCPKG_BASE_VERSION=2021-01-13`"", # Note: This duplicate date version will be short lived. See https://github.com/microsoft/vcpkg/pull/15474

Neumann-A · 2021-01-14T08:08:11Z

scripts/cmake/vcpkg_minimum_required.cmake

+
+## Usage
+```cmake
+vcpkg_check_linkage(


Suggested change

vcpkg_check_linkage(

vcpkg_minimum_required(

strega-nil

Minor changes requested, then I'm an approve.

strega-nil · 2021-01-15T00:57:07Z

scripts/cmake/vcpkg_minimum_required.cmake

+        )
+    endif()
+
+    string(REGEX MATCH "[12][0-9][0-9][0-9]-[01][0-9]-[0-2][0-9]" _vcpkg_matched_base_version "${VCPKG_BASE_VERSION}")


Suggested change

string(REGEX MATCH "[12][0-9][0-9][0-9]-[01][0-9]-[0-2][0-9]" _vcpkg_matched_base_version "${VCPKG_BASE_VERSION}")

string(REGEX MATCH "^[12][0-9][0-9][0-9]-(0[0-9]|1[0-2])-([0-2][0-9]|3[01])$" _vcpkg_matched_base_version "${VCPKG_BASE_VERSION}")

this doesn't match "2020-01-31" as it is right now, and it also matches "foobar2423-13-12"

Alternatively,

Suggested change

string(REGEX MATCH "[12][0-9][0-9][0-9]-[01][0-9]-[0-2][0-9]" _vcpkg_matched_base_version "${VCPKG_BASE_VERSION}")

string(REGEX MATCH "^\d{4}-\d{2}-\d{2}$" _vcpkg_matched_base_version "${VCPKG_BASE_VERSION}")

strega-nil · 2021-01-15T00:59:06Z

scripts/cmake/vcpkg_minimum_required.cmake

+
+    string(REPLACE "-" "." _VCPKG_BASE_VERSION_as_dotted "${_vcpkg_matched_base_version}")
+
+    string(REGEX MATCH "[12][0-9][0-9][0-9]-[01][0-9]-[0-2][0-9]" _vcpkg_matched_test_version "${_vcpkg_VERSION}")


you should either split out the matcher, or use a simpler matcher.

strega-nil · 2021-01-15T01:02:05Z

toolsrc/src/vcpkg/commands.create.cpp

+            {"PORT_PATH", fs::generic_u8string(paths.builtin_ports_directory() / fs::u8path(port_name))},
+            {"VCPKG_BASE_VERSION", Commands::Version::base_version()},


This is a minor nit, but

Suggested change

{"PORT_PATH", fs::generic_u8string(paths.builtin_ports_directory() / fs::u8path(port_name))},

{"VCPKG_BASE_VERSION", Commands::Version::base_version()},

{"VCPKG_BASE_VERSION", Commands::Version::base_version()},

{"PORT_PATH", fs::generic_u8string(paths.builtin_ports_directory() / fs::u8path(port_name))},

… related machinery.

…s of that type in checks.cpp.

… for them.

ras0219 · 2021-01-16T07:37:15Z

scripts/azure-pipelines/end-to-end-tests.ps1

+    New-Item -Path $WorkingRoot -ItemType Directory
+}
+
+$WorkingRoot = (Get-Item $WorkingRoot).FullName


Duplicate of below

# Conflicts: # .gitignore

docs/maintainers/vcpkg_minimum_required.md

ras0219 · 2021-01-19T23:19:17Z

scripts/cmake/vcpkg_minimum_required.cmake

+        message(FATAL_ERROR
+            "Your vcpkg executable is from ${VCPKG_BASE_VERSION} which is older than required by the caller "
+            "of vcpkg_minimum_required (${_vcpkg_VERSION}). "
+            "Please re-acquire vcpkg by running bootstrap-vcpkg."


This message might need to additionally suggest git pull.

I think that would be harmful given the status quo that the only way the user can have this happen if they've already changed branches. I agree it makes sense when most of our users are using externally acquired vcpkg / registries, but when that happens the right fix won't be bootstrap-vcpkg either.

(That is, that message might change the versions of bits users are using)

strega-nil

Some minor comments and questions but approved :)

strega-nil · 2021-01-20T18:16:31Z

toolsrc/include/vcpkg/base/lineinfo.h

-        constexpr LineInfo() noexcept : m_line_number(0), m_file_name("") { }
-        constexpr LineInfo(const int lineno, const char* filename) : m_line_number(lineno), m_file_name(filename) { }
-
-        std::string to_string() const;


were these just never defined?

The ctors are defined inline there, to_string was defined in checks.cpp.

strega-nil · 2021-01-20T18:17:09Z

toolsrc/include/vcpkg/base/stringview.h

@@ -49,7 +34,7 @@ namespace vcpkg
        std::string to_string() const;
        void to_string(std::string& out) const;

-        StringView substr(size_t pos, size_t count = std::numeric_limits<size_t>::max()) const noexcept;
+        StringView substr(size_t pos, size_t count = static_cast<size_t>(-1)) const noexcept;


I don't like this change.

Without this change we need to add #include <limits> and we were trying to make this header very small?

Given that we're using PCHs, I don't have a huge issue with using standard headers, even if they're pretty large. However, I won't block this change if you feel strongly about it.

strega-nil · 2021-01-20T18:18:40Z

toolsrc/src/vcpkg/base/files.cpp


+            file_stream.close();


Why aren't we just letting the RAII do its thing? (not a change from your PR, but while you're editing anyways)

strega-nil · 2021-01-20T18:19:45Z

toolsrc/src/vcpkg/base/system.cpp

@@ -39,7 +39,7 @@ namespace vcpkg
            case CPUArchitecture::ARM: return "arm";
            case CPUArchitecture::ARM64: return "arm64";
            case CPUArchitecture::S390X: return "s390x";
-            default: Checks::exit_with_message(VCPKG_LINE_INFO, "unexpected vcpkg::System::CPUArchitecture");
+            default: Checks::exit_maybe_upgrade(VCPKG_LINE_INFO, "unexpected vcpkg::System::CPUArchitecture");


Suggested change

default: Checks::exit_maybe_upgrade(VCPKG_LINE_INFO, "unexpected vcpkg::System::CPUArchitecture");

default: Checks::unexpected(VCPKG_LINE_INFO);

This is an enumeration, if a value is not in the list then that's a coding error.

…ing related to fstream sets errno.

utilities.cmake: Disable warnings that are firing on Azure Pipelines CI machines due to different Clang version. expected.h: Fix a bug I introduced in microsoft#15638 because I was under the impression expected worked like optional (in that value_or_exit for it should be treated as a program bug) build.cpp: Add (void)s to silence warnings. binarycaching.cpp: Repair assumption that the current directory is C: which isn't true on the Hosted Azure Pipelines agents. others: Make unit tests respect %VCPKG_ROOT%, as necessary in the vcpkg_tool repo. Note that this required splitting vcpkgcmdarguments::ImbueFromEnvironment into the once-only process modifying part and the just imbue from environment part.

utilities.cmake: Disable warnings that are firing on Azure Pipelines CI machines due to different Clang version. expected.h: Fix a bug I introduced in microsoft/vcpkg#15638 because I was under the impression expected worked like optional (in that value_or_exit for it should be treated as a program bug) build.cpp: Add (void)s to silence warnings. binarycaching.cpp: Repair assumption that the current directory is C: which isn't true on the Hosted Azure Pipelines agents. others: Make unit tests respect %VCPKG_ROOT%, as necessary in the vcpkg_tool repo. Note that this required splitting vcpkgcmdarguments::ImbueFromEnvironment into the once-only process modifying part and the just imbue from environment part.

utilities.cmake: Disable warnings that are firing on Azure Pipelines CI machines due to different Clang version. expected.h: Fix a bug I introduced in #15638 because I was under the impression expected worked like optional (in that value_or_exit for it should be treated as a program bug) build.cpp: Add (void)s to silence warnings. binarycaching.cpp: Repair assumption that the current directory is C: which isn't true on the Hosted Azure Pipelines agents. others: Make unit tests respect %VCPKG_ROOT%, as necessary in the vcpkg_tool repo. Note that this required splitting vcpkgcmdarguments::ImbueFromEnvironment into the once-only process modifying part and the just imbue from environment part.

utilities.cmake: Disable warnings that are firing on Azure Pipelines CI machines due to different Clang version. expected.h: Fix a bug I introduced in microsoft/vcpkg#15638 because I was under the impression expected worked like optional (in that value_or_exit for it should be treated as a program bug) build.cpp: Add (void)s to silence warnings. binarycaching.cpp: Repair assumption that the current directory is C: which isn't true on the Hosted Azure Pipelines agents. others: Make unit tests respect %VCPKG_ROOT%, as necessary in the vcpkg_tool repo. Note that this required splitting vcpkgcmdarguments::ImbueFromEnvironment into the once-only process modifying part and the just imbue from environment part.

…icrosoft#15638)

utilities.cmake: Disable warnings that are firing on Azure Pipelines CI machines due to different Clang version. expected.h: Fix a bug I introduced in microsoft/vcpkg#15638 because I was under the impression expected worked like optional (in that value_or_exit for it should be treated as a program bug) build.cpp: Add (void)s to silence warnings. binarycaching.cpp: Repair assumption that the current directory is C: which isn't true on the Hosted Azure Pipelines agents. others: Make unit tests respect %VCPKG_ROOT%, as necessary in the vcpkg_tool repo. Note that this required splitting vcpkgcmdarguments::ImbueFromEnvironment into the once-only process modifying part and the just imbue from environment part.

BillyONeal added category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:internal This PR or Issue was filed by the vcpkg team. labels Jan 14, 2021

BillyONeal mentioned this pull request Jan 14, 2021

[vcpkg] Download vcpkg.exe rather than building it in bootstrap on Windows. #15474

Merged

ras0219 approved these changes Jan 14, 2021

View reviewed changes

Neumann-A reviewed Jan 14, 2021

View reviewed changes

JackBoosY assigned NancyLi1013 Jan 14, 2021

strega-nil suggested changes Jan 15, 2021

View reviewed changes

BillyONeal added 14 commits January 15, 2021 22:33

Add vcpkg_minimum_required as a replacement for version.txt.

5a8410c

Add update things message.

2f1596c

Robert feedback.

d8f03a6

clang-format

12cc31c

docs

012d8f9

Remove unreferenced local scan3 function.

482fda8

Test fixes and add missing e2e test run on Linux .

0b3a1d6

Function typo and format.

6834c61

Skip all binary caching tests on Linux.

b2d23e5

Nicole CR feedback.

2f54acb

Sort CMake variables.

7370ecc

Untangle circular dependency between vcpkg::StringView, checks.h, and…

662b783

… related machinery.

Remove need for lineinfo forward declaration by putting all to_string…

b2f0385

…s of that type in checks.cpp.

Audit all exits with message and emit the "maybe upgrading will help"…

6f9c3b1

… for them.

BillyONeal force-pushed the vcpkg_version branch from 4910555 to 6f9c3b1 Compare January 16, 2021 07:10

BillyONeal added 2 commits January 15, 2021 23:17

Add missing formatted versions.

83055de

Avoid numeric_limits dependency.

22ebb92

ras0219 reviewed Jan 16, 2021

View reviewed changes

BillyONeal added 3 commits January 19, 2021 14:01

Merge remote-tracking branch 'origin/master' into vcpkg_version

bea78f1

# Conflicts: # .gitignore

Attempt to repair test on Linux.

d80798b

Dedupe merge conflict and add all tests passed message.

785b45b

ras0219 approved these changes Jan 19, 2021

View reviewed changes

Remove unecessary newlines from vcpkg_minimum_required docs.

ad7a0c6

strega-nil approved these changes Jan 20, 2021

View reviewed changes

BillyONeal added 4 commits January 20, 2021 10:51

Merge remote-tracking branch 'origin/master' into vcpkg_version

00461df

Back out the numeric_limits change in StringView as requested by Nicole.

caa93ee

Use "unreachable" for all switches over enums, as requested by Nicole.

3c85d8f

RAII-ize use of fstream in files.cpp and remove assumption that anyth…

dcdfcee

…ing related to fstream sets errno.

strega-nil approved these changes Jan 20, 2021

View reviewed changes

Use MATCHES and actually make the files.cpp change work.

10ae36b

BillyONeal merged commit 4d136ef into microsoft:master Jan 20, 2021

BillyONeal deleted the vcpkg_version branch January 20, 2021 20:11

BillyONeal mentioned this pull request Feb 3, 2021

[vcpkg] Tool fixes needed for vcpkg-tool extraction #16012

Merged

strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021

[vcpkg] Add vcpkg_minimum_required as a replacement for VERSION.txt. (m…

786a047

…icrosoft#15638)

Cheney-W mentioned this pull request Jun 9, 2022

[vcpkg upgrade] CMake Error at scripts/ports.cmake:14 (message) Your vcpkg executable is outdated #12222

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vcpkg] Add vcpkg_minimum_required as a replacement for VERSION.txt. #15638

[vcpkg] Add vcpkg_minimum_required as a replacement for VERSION.txt. #15638

BillyONeal commented Jan 14, 2021

ras0219 Jan 14, 2021

ras0219 Jan 14, 2021

BillyONeal Jan 14, 2021

ras0219 Jan 14, 2021

ras0219 Jan 14, 2021

ras0219 Jan 14, 2021

ras0219 Jan 14, 2021

Neumann-A Jan 14, 2021

BillyONeal Jan 14, 2021

strega-nil left a comment

strega-nil Jan 15, 2021

strega-nil Jan 15, 2021

strega-nil Jan 15, 2021

strega-nil Jan 15, 2021

ras0219 Jan 16, 2021

ras0219 Jan 19, 2021

BillyONeal Jan 19, 2021

BillyONeal Jan 19, 2021

strega-nil left a comment

strega-nil Jan 20, 2021

BillyONeal Jan 20, 2021

strega-nil Jan 20, 2021

BillyONeal Jan 20, 2021

strega-nil Jan 20, 2021

strega-nil Jan 20, 2021 •

edited

strega-nil Jan 20, 2021

	set(VCPKG_BASE_VERSION "2020-01-13")
	set(VCPKG_BASE_VERSION "2021-01-13")

	vcpkg_minimum_required(VERSION 2020-01-13)
	vcpkg_minimum_required(VERSION 2021-01-13)

	"`"/p:VCPKG_BASE_VERSION=2020-01-13`"", # Note: This duplicate date version will be short lived. See https://github.com/microsoft/vcpkg/pull/15474
	"`"/p:VCPKG_BASE_VERSION=2021-01-13`"", # Note: This duplicate date version will be short lived. See https://github.com/microsoft/vcpkg/pull/15474

	string(REGEX MATCH "[12][0-9][0-9][0-9]-[01][0-9]-[0-2][0-9]" _vcpkg_matched_base_version "${VCPKG_BASE_VERSION}")
	string(REGEX MATCH "^[12][0-9][0-9][0-9]-(0[0-9]\|1[0-2])-([0-2][0-9]\|3[01])$" _vcpkg_matched_base_version "${VCPKG_BASE_VERSION}")

	string(REGEX MATCH "[12][0-9][0-9][0-9]-[01][0-9]-[0-2][0-9]" _vcpkg_matched_base_version "${VCPKG_BASE_VERSION}")
	string(REGEX MATCH "^\d{4}-\d{2}-\d{2}$" _vcpkg_matched_base_version "${VCPKG_BASE_VERSION}")


		string(REPLACE "-" "." _VCPKG_BASE_VERSION_as_dotted "${_vcpkg_matched_base_version}")

		string(REGEX MATCH "[12][0-9][0-9][0-9]-[01][0-9]-[0-2][0-9]" _vcpkg_matched_test_version "${_vcpkg_VERSION}")

		{"PORT_PATH", fs::generic_u8string(paths.builtin_ports_directory() / fs::u8path(port_name))},
		{"VCPKG_BASE_VERSION", Commands::Version::base_version()},

	default: Checks::exit_maybe_upgrade(VCPKG_LINE_INFO, "unexpected vcpkg::System::CPUArchitecture");
	default: Checks::unexpected(VCPKG_LINE_INFO);

[vcpkg] Add vcpkg_minimum_required as a replacement for VERSION.txt. #15638

[vcpkg] Add vcpkg_minimum_required as a replacement for VERSION.txt. #15638

Conversation

BillyONeal commented Jan 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strega-nil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strega-nil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strega-nil Jan 20, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strega-nil Jan 20, 2021 •

edited