-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
[pkgconf] enable search for system libs on linux #23010
[pkgconf] enable search for system libs on linux #23010
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/pkgconf/vcpkg.json
Valid values for the license field are listed at https://spdx.org/licenses/
@JackBoosY Feel free to propose a license here. I would probably go with |
Since these paths are target specific (e.g. a sysroot when cross compiling), it seems like we must not bake them in at compile time and instead they must be dynamically added to What do you mean specifically by
Do you mean that if we want CMake to be able to use pkgconf from vcpkg, it needs system paths? Or are you saying that today CMake 3.22 is broken because it is erroneously finding pkgconf and you're proposing we fix it by making pkgconf work? Another potential approach would be to generate script wrappers in the target triplets that prepend/append the appropriate values to |
If we want to use cmake 3.22 we need to have pkgconf behave like the system pkg-config because otherwise ports will fail sporadically since
I am proposing to make it work like the system one. For the cross-compiling case the users need to define the correct non-system behavior anyway. |
So possible way to determine system defaults is to run
That would be
similar result for libpkgconfig:
|
|
After thinking about it I would like to have a solution which does not require the system pkg-config. |
It is probably enough to use reasonable system paths plus user environment variable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/pkgconf/vcpkg.json
Valid values for the license field are listed at https://spdx.org/licenses/
The user may not installed pkgconfig yet. |
No problem if it is not called. |
|
Finally! This particular issue has been very elusive to repro. Thanks :) -- I 100% agree that we need a solution that doesn't rely on a system pkg-config (since that kinda defeats the point in a lot of ways :)). However, I think it's important that we offer users the option to control the results. Looking at the man page1, I see that
So we should ensure that our solution respects that. For example, if we went with a script-wrapper solution like I mentioned above, the script wrapper should not append the system paths if this env was defined. I'm additionally not familiar enough with current Linux to know how we should handle
Perhaps there's something more clever that would be reasonable, like transforming all elements of |
I think there is an error in the CMakeLists.txt using an absolute path in the installation. Cmake tries to
environment variable
this again requires system pkg-config OpenSuse defaults:
pkg-config
debian is like ubuntu
(That are the 3 linux distros I could find in WSL) |
or
the follwing variables might be helpful:
|
Will be fixed in another PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 d15470bb6464a3bb0ce3d98d118190cfd5189080 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/p-/pkgconf.json b/versions/p-/pkgconf.json
index 8277f3f..e4a222d 100644
--- a/versions/p-/pkgconf.json
+++ b/versions/p-/pkgconf.json
@@ -1,7 +1,7 @@
{
"versions": [
{
- "git-tree": "849c6b13074045c18f89394e7c7350efe8a630df",
+ "git-tree": "0338b251230ade33a8740f1da0fb4c209e34fb63",
"version": "1.8.0",
"port-version": 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/pkgconf/portfile.cmake
You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/pkgconf/vcpkg.json
Valid values for the license field can be found in the documentation
I think asking CMake for this stuff is the solution. Just need to run it on opensuse once. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!
PRs must add only one version and must not modify any published versions
When making any changes to a library, the version or port-version in vcpkg.json
or CONTROL
must be modified.
Error: Local changes detected for colmap but no changes to version or port version.
-- Version: 3.7
-- Old SHA: 81329023ad5f9b2bd27554d2d8a97f8e63aef708
-- New SHA: 0f09c643f7fef8b9a1ee89ba82f4d17c40b3a348
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout d15470bb6464a3bb0ce3d98d118190cfd5189080 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/p-/pkgconf.json b/versions/p-/pkgconf.json
index 8277f3f..e4a222d 100644
--- a/versions/p-/pkgconf.json
+++ b/versions/p-/pkgconf.json
@@ -1,7 +1,7 @@
{
"versions": [
{
- "git-tree": "849c6b13074045c18f89394e7c7350efe8a630df",
+ "git-tree": "0338b251230ade33a8740f1da0fb4c209e34fb63",
"version": "1.8.0",
"port-version": 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/pkgconf/portfile.cmake
You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/pkgconf/vcpkg.json
Valid values for the license field can be found in the documentation
@ras0219-msft: hmm is the type of linux system part of the binary hash? If not this will definitely break binary caching since the output result will depend on system introspection not considered in the binary hash. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!
PRs must add only one version and must not modify any published versions
When making any changes to a library, the version or port-version in vcpkg.json
or CONTROL
must be modified.
Error: Local changes detected for colmap but no changes to version or port version.
-- Version: 3.7
-- Old SHA: 81329023ad5f9b2bd27554d2d8a97f8e63aef708
-- New SHA: 0f09c643f7fef8b9a1ee89ba82f4d17c40b3a348
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout d15470bb6464a3bb0ce3d98d118190cfd5189080 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/p-/pkgconf.json b/versions/p-/pkgconf.json
index 8277f3f..87f6390 100644
--- a/versions/p-/pkgconf.json
+++ b/versions/p-/pkgconf.json
@@ -1,7 +1,7 @@
{
"versions": [
{
- "git-tree": "849c6b13074045c18f89394e7c7350efe8a630df",
+ "git-tree": "5b599bd7dbbbc7837f7dbcf1b111478da076e2a4",
"version": "1.8.0",
"port-version": 2
},
You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/pkgconf/vcpkg.json
Valid values for the license field can be found in the documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 d15470bb6464a3bb0ce3d98d118190cfd5189080 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/p-/pkgconf.json b/versions/p-/pkgconf.json
index 8277f3f..87f6390 100644
--- a/versions/p-/pkgconf.json
+++ b/versions/p-/pkgconf.json
@@ -1,7 +1,7 @@
{
"versions": [
{
- "git-tree": "849c6b13074045c18f89394e7c7350efe8a630df",
+ "git-tree": "5b599bd7dbbbc7837f7dbcf1b111478da076e2a4",
"version": "1.8.0",
"port-version": 2
},
You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/pkgconf/vcpkg.json
Valid values for the license field can be found in the documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/pkgconf/vcpkg.json
Valid values for the license field can be found in the documentation
@ras0219-msft why did you close this? This is not about colmap. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/pkgconf/vcpkg.json
Valid values for the license field can be found in the documentation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/pkgconf/vcpkg.json
Valid values for the license field can be found in the documentation
@JackBoosY That is a bug in the dependency chain of colmap somewhere. Something is pulling in openblas if it is installed. Basically the repro is to first install openblas than everything colmap needs than deinstall openblas and retry to install colmap. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/pkgconf/vcpkg.json
Valid values for the license field can be found in the documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/pkgconf/vcpkg.json
Valid values for the license field can be found in the documentation
Thanks for your patience everyone! |
* master: (57 commits) [vcpkg-tools] update cmake and git (windows only) (microsoft#22985) Update vcpkg tool to 2022-02-24. (microsoft#23162) [vcpkg baseline] Move cspice headers (microsoft#23272) Fixed inaccurate Chinese words (microsoft#23179) [vcpkg] Add fixed changelog generator. (microsoft#23255) [authentication.md] Add Jenkins section (microsoft#23226) [vcpkg] Meson osx sysroot (microsoft#21772) [pkgconf] enable search for system libs on linux (microsoft#23010) [yasm/yasm-tool] Incorporate yasm-tool into yasm (microsoft#23218) [lapack-reference] Update to 3.10 (microsoft#23228) [skia] Arm64 for skia on osx (microsoft#23222) [libfido2] Update to 1.10.0 (microsoft#23241) [Tracy] Fixing issue where version 0.7.8 was pulling the wrong version (microsoft#23061) [libgpiod] Add new port. (microsoft#23221) [drogon] Update to 1.7.5 (microsoft#23227) [tinyexif] Remove from fail list. (microsoft#23163) [vcpkg docs][ES] Sync with English readme (microsoft#19834) (microsoft#22618) [vcpkg baseline][libao] Disable dlfcn check under windows (microsoft#23235) [OpenCV] upgrade to v4.5.5 (microsoft#22801) [libcurl-simple-https] New port (microsoft#22917) ...
This is required to make cmake 3.22 work since it will pickup
pkgconf
from vcpkg. As suchpkgconf
from vcpkg needs to be able to pickup system dependencies which it only can if the default paths are correctly set. Unfortunately I don't know of a way to get the correct paths from the system'spkg-config
(maybe the paths can be dumped somehow? String literals in the binary?)This also belongs in the category that the host build requires information for the target to correctly set pkgconf up.
@ras0219: Maybe you have a better solution?
(maybe build it from the contents of
/etc/ld.so.conf
/etc/ld.so.conf.d/*.conf
?)