[vcpkg-tool-ninja] add port for ninja #23201
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 5cf60186a241e84e8232641ee973395d4fde90e1 -- versions
./vcpkg x-add-version --allDiff
diff --git a/versions/baseline.json b/versions/baseline.json
index 6b3147e..6a68e99 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7193,7 +7193,7 @@
"port-version": 1
},
"vcpkg-cmake": {
- "baseline": "2022-01-19",
+ "baseline": "2022-02-21",
"port-version": 0
},
"vcpkg-cmake-config": {
@@ -7218,7 +7218,11 @@
},
"vcpkg-tool-meson": {
"baseline": "0.60.2",
- "port-version": 1
+ "port-version": 2
+ },
+ "vcpkg-tool-ninja": {
+ "baseline": "2022-02-21",
+ "port-version": 0
},
"vcpkg-tool-nodejs": {
"baseline": "14.17.4",
diff --git a/versions/v-/vcpkg-cmake.json b/versions/v-/vcpkg-cmake.json
index b61a6fe..819c4a7 100644
--- a/versions/v-/vcpkg-cmake.json
+++ b/versions/v-/vcpkg-cmake.json
@@ -1,5 +1,10 @@
{
"versions": [
+ {
+ "git-tree": "06ef058e0250cdf8aeb7375958ee68c6942485f2",
+ "version-date": "2022-02-21",
+ "port-version": 0
+ },
{
"git-tree": "b7c050fe60f91dcedef6d87a3f87584151bf8aee",
"version-date": "2022-01-19",
diff --git a/versions/v-/vcpkg-tool-meson.json b/versions/v-/vcpkg-tool-meson.json
index 69969c8..d0d71f9 100644
--- a/versions/v-/vcpkg-tool-meson.json
+++ b/versions/v-/vcpkg-tool-meson.json
@@ -1,5 +1,10 @@
{
"versions": [
+ {
+ "git-tree": "d93e36d6abc3b860f77444fe3294ac1de6cc2939",
+ "version": "0.60.2",
+ "port-version": 2
+ },
{
"git-tree": "932036adfc24dd5fa63787b825974b6938402700",
"version": "0.60.2",You have modified or added at least one portfile where deprecated functions are used.
If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
In the ports that use the new function, you have to add the corresponding dependencies:
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}The following files are affected:
ports/vcpkg-tool-ninja/portfile.cmake
|
couldn't you use |
There was a problem hiding this comment.
This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 5cf60186a241e84e8232641ee973395d4fde90e1 -- versions
./vcpkg x-add-version --allDiff
diff --git a/versions/baseline.json b/versions/baseline.json
index 6b3147e..0bc3e01 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7193,7 +7193,7 @@
"port-version": 1
},
"vcpkg-cmake": {
- "baseline": "2022-01-19",
+ "baseline": "2022-02-21",
"port-version": 0
},
"vcpkg-cmake-config": {
@@ -7218,7 +7218,11 @@
},
"vcpkg-tool-meson": {
"baseline": "0.60.2",
- "port-version": 1
+ "port-version": 2
+ },
+ "vcpkg-tool-ninja": {
+ "baseline": "2022-02-21",
+ "port-version": 0
},
"vcpkg-tool-nodejs": {
"baseline": "14.17.4",
@@ -7542,7 +7546,7 @@
},
"zlib": {
"baseline": "1.2.11",
- "port-version": 13
+ "port-version": 14
},
"zlib-ng": {
"baseline": "2.0.5",
diff --git a/versions/v-/vcpkg-cmake.json b/versions/v-/vcpkg-cmake.json
index b61a6fe..819c4a7 100644
--- a/versions/v-/vcpkg-cmake.json
+++ b/versions/v-/vcpkg-cmake.json
@@ -1,5 +1,10 @@
{
"versions": [
+ {
+ "git-tree": "06ef058e0250cdf8aeb7375958ee68c6942485f2",
+ "version-date": "2022-02-21",
+ "port-version": 0
+ },
{
"git-tree": "b7c050fe60f91dcedef6d87a3f87584151bf8aee",
"version-date": "2022-01-19",
diff --git a/versions/v-/vcpkg-tool-meson.json b/versions/v-/vcpkg-tool-meson.json
index 69969c8..d0d71f9 100644
--- a/versions/v-/vcpkg-tool-meson.json
+++ b/versions/v-/vcpkg-tool-meson.json
@@ -1,5 +1,10 @@
{
"versions": [
+ {
+ "git-tree": "d93e36d6abc3b860f77444fe3294ac1de6cc2939",
+ "version": "0.60.2",
+ "port-version": 2
+ },
{
"git-tree": "932036adfc24dd5fa63787b825974b6938402700",
"version": "0.60.2",
diff --git a/versions/z-/zlib.json b/versions/z-/zlib.json
index 7c0d68f..c1199a8 100644
--- a/versions/z-/zlib.json
+++ b/versions/z-/zlib.json
@@ -1,5 +1,10 @@
{
"versions": [
+ {
+ "git-tree": "d35981027a171575f248325b81c271c97c028325",
+ "version": "1.2.11",
+ "port-version": 14
+ },
{
"git-tree": "92cfe30c807d343c6359d272242f0765ad906740",
"version": "1.2.11",You have modified or added at least one portfile where deprecated functions are used.
If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
In the ports that use the new function, you have to add the corresponding dependencies:
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}The following files are affected:
ports/vcpkg-tool-ninja/portfile.cmake
unfortunately the patch didn't apply cleanly the first time I tried so I removed the unnecessary/unrelated changes. |
JackBoosY
added
the
category:new-port
|
regressions look like baseline issues to me |
There was a problem hiding this comment.
You have modified or added at least one portfile where deprecated functions are used.
If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
In the ports that use the new function, you have to add the corresponding dependencies:
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}The following files are affected:
ports/vcpkg-tool-ninja/portfile.cmake
|
basically those two ports in different triplets. (or just the first the second is just in x64-windows) |
Yeah I know. Avoiding cycles in the dependency graph. Need to use the old functions to bootstrap ninja and inject it into |
|
libio: baseline regression, fixing. openxr-loader: |
|
The |
Related to port python3? |
|
@JackBoosY CI is green now so these are definitely baseline regressions. libao is probably picking up some optional dependency. |
Yes, we will fix this today. The reason why it faild is because Edit: this will be fixed by PR #23235. |
| set(tool_subdirectory "${program_version}-linux") | ||
| set(paths_to_search "${DOWNLOADS}/tools/ninja-${program_version}-linux") | ||
| set(download_sha512 93e802e9c17fb59636cddde4bad1ddaadad624f4ecfee00d5c78790330a4e9d433180e795718cda27da57215ce643c3929cf72c85337ee019d868c56f2deeef3) | ||
| if(EXISTS "${CURRENT_HOST_INSTALLED_DIR}/share/ninja/version.txt") |
There was a problem hiding this comment.
I'd like to move this part to port vcpkg-tool-ninja thoroughly if we want these tools to fully support version control.
And for the installation rules, I think we can make it in another scripts such as vcpkg_common_definitions.
What do you think about?
There was a problem hiding this comment.
due to versioning you cannot do that. vcpkg_find_acquire_program needs to be able to always find ninja somehow with arbitrary old versions of the ports. Also: ninja is not really ABI relevant. We just need to update it for newer build features/supports.
What could be done is remove vcpkg_find_acquire_program from the newer buildscripts in vcpkg-cmake and vcpkg-tool-meson and set the path directly instead.
There was a problem hiding this comment.
I'd prefer if vcpkg_find_acquire_program didn't search for ninja in CURRENT_HOST_INSTALLED_DIR, just like how vcpkg_configure_cmake still does the old logic - just make vcpkg_*_install/vcpkg_*_build use the vcpkg-tool-ninja.
There was a problem hiding this comment.
I'd prefer if vcpkg_find_acquire_program didn't search for ninja in CURRENT_HOST_INSTALLED_DIR, just like how vcpkg_configure_cmake still does the old logic
I disagree with that. With the current approach I can just add a single dependency on vcpkg-tool-ninja in an early dependency in my build graph and get the benefit of the newer ninja in manifest mode or in classic mode simply install vcpkg-tool-ninja before everything else and get the same effect. This is crucial to at least get partial support for long paths with older versions. (Since it is a host port it may always get installed earlier?)
just make vcpkg_install/vcpkg_build use the vcpkg-tool-ninja.
That is something I can do but this burns in the ninja build by vcpkg. With the current approach I could simply set NINJA in the triplet and overwrite everything vcpkg does.
Personally for not ABI relevant tools I would prefer the flexibility instead a rigid construct. Maybe discuss it in the team if this is a PR you want to merge at all (since I did not yet get any positiv feedback on it) and if yes how exactly you want to do it. +
So the questions are:
- Merge the tool at all ? (Considering that upstream seems to be a bit slow I would say it is a requirement)
- Lock out old versions? (vs flexible search in
vcpkg_find_acquire_program) - Disable overwrite via triplet? (change of current behavior by not using
vcpkg_find_acquire_program)
There was a problem hiding this comment.
I'll add it to the list of things to discuss tomorrow, but my personal answers:
- yes absolutely
- yes; it's unfortunate, but I don't think it's reasonable to change behavior in this way
- allowing overrides should be done; there are ways to do so without
vcpkg_find_acquire_program
note that my opinion here is based on my opinion generally, which is that vcpkg_find_acquire_program should be sunsetted.
There was a problem hiding this comment.
note that my opinion here is based on my opinion generally, which is that vcpkg_find_acquire_program should be sunsetted.
I understand that but there needs to be a common way to deal with it. We could drop the acquire part and just have a vcpkg_find_program which searches at defined places within vcpkg reading in the required info from somewhere in the installed tree.
Still vcpkg_find_acquire_program can not be removed due to versioning so having it around and doing the same as a not yet invented vcpkg_find_program would deprecate it in the long run.
You may ask: Why not hardcode the path?
Hardcoding the path in script like helpers would be somewhat ok but in portfiles it is a no go. portfiles need an abstraction to find general usage tools instead of hardcoding every path. (Because otherwise changing the path/install layout will be a nightmare)
allowing overrides should be done; there are ways to do so without
Yes we could define a VCPKG_<program> for every override but I would like a solution that scales (which the current vcpkg_find_acquire_program does not since it knows every program it can acquire).
vcpkg_find_program which reads in a list of cmake files from some well-defined location within the installed tree and simply using find_program would probably work nicely (similar to the script configs). We could implement vcpkg_find_program in such a way that on some platforms the hardcoded path is used while on other platforms find_program is used.
Simply said, the layout for such an solution needs to be defined.
JackBoosY
added
the
requires:vcpkg-team-review
# Conflicts: # versions/baseline.json # versions/v-/vcpkg-tool-meson.json
There was a problem hiding this comment.
You have modified or added at least one portfile where deprecated functions are used.
If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
In the ports that use the new function, you have to add the corresponding dependencies:
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}The following files are affected:
ports/vcpkg-tool-ninja/portfile.cmake
|
cspice is a baseline regression |
|
|
@JackBoosY already opened: #23272 |
There was a problem hiding this comment.
You have modified or added at least one portfile where deprecated functions are used.
If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
In the ports that use the new function, you have to add the corresponding dependencies:
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}The following files are affected:
ports/vcpkg-tool-ninja/portfile.cmake
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@JackBoosY 7zip is currently screwing up CI so no need to |
There was a problem hiding this comment.
You have modified or added at least one portfile where deprecated functions are used.
If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
In the ports that use the new function, you have to add the corresponding dependencies:
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}The following files are affected:
ports/vcpkg-tool-ninja/portfile.cmake
| @@ -0,0 +1,80 @@ | |||
| set(VCPKG_POLICY_EMPTY_PACKAGE enabled) | |||
There was a problem hiding this comment.
This will result in increased build times, which I think needs to be considered.
There was a problem hiding this comment.
Why did you comment it here? And how much is the increase?
There was a problem hiding this comment.
If I don't understand wrong, vcpkg-tool-ninja uses ninja's source to build ninja executable.
In the original code, ninja would be directly downloaded and unpacked for use, rather than being built.
So I think adding this will result in an increase in build time.
There was a problem hiding this comment.
yeah but if you build something like qtdeclarative or even qtwebengine the added build time of ninja is minimal compared to the build time of these ports.
Elapsed time for package vcpkg-tool-ninja:x64-windows: 27.29 s
Elapsed time for package qtbase:x64-windows: 6.89 min
Elapsed time for package qtdeclarative:x64-windows: 10.94 min
Elapsed time for package qtwebengine:x64-windows: 1.705 h
Compare to build times from: #22846
Elapsed time for package qtbase:x64-windows: 9.129 min
Elapsed time for package qtdeclarative:x64-windows: 13.94 min
Elapsed time for package qtwebengine:x64-windows: 2.063 h
So compared to the previous build the updated ninja version seems to consistently lower the build times! (but it might depend on the runner so some averages should be made to confirm this) + you get long path support.
There was a problem hiding this comment.
You have modified or added at least one portfile where deprecated functions are used.
If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
In the ports that use the new function, you have to add the corresponding dependencies:
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}The following files are affected:
ports/vcpkg-tool-ninja/portfile.cmake
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
You have modified or added at least one portfile where deprecated functions are used.
If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
In the ports that use the new function, you have to add the corresponding dependencies:
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}The following files are affected:
ports/vcpkg-tool-ninja/portfile.cmake
|
This is also required to build |
|
Our thoughts: We like this for qt and flang; however, we don't want to make this more general than it is. We would like this if it:
|
Ok the worst outcome then. |
|
How about setting |
Would still require an explicit dependency on |
|
PR is continued in #23911 with only the port being added. |
Qt6.3 will require ninja with long path support on windows to build qtwebengine with the default build paths vcpkg uses.
Proof of fix: ninja-build/ninja#2056 (comment)