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

[libspatialite] Use pkgconfig for nmake and autotools #20480

Merged
merged 8 commits into from
Nov 19, 2021

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Oct 2, 2021

  • What does your PR fix?

    • Use pkg-config for determining libraries to pass even to nmake, instead of hardcoding.
      Needed to allow features in dependencies.
    • Fix building with mingw
    • Install pc file for windows
    • Add feature interface
  • Which triplets are supported/not supported? Have you updated the CI baseline?

    all, yes

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

    yes

Copy link

@github-actions github-actions bot left a 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 2a31089e777fc187f1cc05338250b8e1810cfb52 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/l-/libspatialite.json b/versions/l-/libspatialite.json
index 1e8dd8d..54f66e8 100644
--- a/versions/l-/libspatialite.json
+++ b/versions/l-/libspatialite.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "22ad27813f5f3bc3382718115ce3eb5a45174afd",
+      "git-tree": "6dc8227e99fb6e52b3102f8a076c32f78a0e7285",
       "version": "5.0.1",
       "port-version": 0
     },

@dg0yt dg0yt force-pushed the libspatialite-nmake branch from 59a9789 to 1973af8 Compare October 2, 2021 21:01
Copy link

@github-actions github-actions bot left a 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 2a31089e777fc187f1cc05338250b8e1810cfb52 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/l-/libspatialite.json b/versions/l-/libspatialite.json
index 1e8dd8d..e7ff6b2 100644
--- a/versions/l-/libspatialite.json
+++ b/versions/l-/libspatialite.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "22ad27813f5f3bc3382718115ce3eb5a45174afd",
+      "git-tree": "4cdba16dd948881b12ad8b7b841d69e60a6a5e33",
       "version": "5.0.1",
       "port-version": 0
     },

@dg0yt dg0yt force-pushed the libspatialite-nmake branch from 1973af8 to 3a6ec29 Compare October 2, 2021 21:04
Copy link

@github-actions github-actions bot left a 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 2a31089e777fc187f1cc05338250b8e1810cfb52 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/l-/libspatialite.json b/versions/l-/libspatialite.json
index 1e8dd8d..645efc9 100644
--- a/versions/l-/libspatialite.json
+++ b/versions/l-/libspatialite.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "22ad27813f5f3bc3382718115ce3eb5a45174afd",
+      "git-tree": "e53c784258eff41f93fd2c94368935f3861dbc10",
       "version": "5.0.1",
       "port-version": 0
     },

@dg0yt dg0yt force-pushed the libspatialite-nmake branch from 3a6ec29 to 86c373e Compare October 3, 2021 05:30
Copy link

@github-actions github-actions bot left a 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 libspatialite but no changes to version or port version.
-- Version: 5.0.0#3
-- Old SHA: d7d124e4fcb212dba3d650817056ce23b6dc2a2f
-- New SHA: 1451d331c4c79f5baab92209c85283a9d6ed9eaa
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@Neumann-A
Copy link
Contributor

I have the feeling you want to pass '--msvc-syntax' to pkg-config and avoid the transformation?

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 3, 2021

I have the feeling you want to pass '--msvc-syntax' to pkg-config and avoid the transformation?

Possibly, yes. TBH I know the option but I don't know enough about nmake to know for sure. I only see explicit FOO_LIBS=D:/some_path/z.lib, while the output from pkg-config is:

$ pkg-config.exe --libs --msvc-syntax zlib
/libpath:D:/msys64/mingw64/lib z.lib

If it is so easy, why all these explicit paths e.g. in https://github.com/microsoft/vcpkg/blob/master/ports/gdal/dependency_win.cmake? I'm really curious, because this nmake dependency stuff is a blocker for improvements.

@Neumann-A
Copy link
Contributor

It simply predates the pkg-config stuff and nobody tried to change it yet because it could be a headache to get it to work again. I only introduced the pkg-config stuff in qt5-base because it fixed a CI build order/dependency regression.

Copy link

@github-actions github-actions bot left a 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 geos but no changes to version or port version.
-- Version: 3.9.1#2
-- Old SHA: 149daa68785ac3796019e4489a6d966fb8cc4cc5
-- New SHA: 12e0860d9b7dc52b93f83b7d6f94e4bb5a1c85c4
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for libspatialite but no changes to version or port version.
-- Version: 5.0.0#3
-- Old SHA: d7d124e4fcb212dba3d650817056ce23b6dc2a2f
-- New SHA: 82b7c49ffb68f8997a440a9ecdbf4611ea6ee12e
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 3, 2021

Okay, I will give it a try as long as CI is idle.

In the geographic ports, there is a lot of nmake+autotools meeting a lot of features, with significant effect on dependencies and license options. This really needs to be improved.

For x_vcpkg_pkgconfig_get_modules I have a draft change to catch and report errors (via RESULT_VARIABLE). I would suggest to add a REQUIRED keyword to enable this behaviour.

Copy link

@github-actions github-actions bot left a 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 geos but no changes to version or port version.
-- Version: 3.9.1#2
-- Old SHA: 149daa68785ac3796019e4489a6d966fb8cc4cc5
-- New SHA: 12e0860d9b7dc52b93f83b7d6f94e4bb5a1c85c4
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for libspatialite but no changes to version or port version.
-- Version: 5.0.0#3
-- Old SHA: d7d124e4fcb212dba3d650817056ce23b6dc2a2f
-- New SHA: 4f419d5594288c1c7f1222ce108cd0270010bc08
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 3, 2021

Okay, I will give it a try as long as CI is idle.

Works for libspatialite. This makes it much easier. 👍

IIUC vcpkg_build_nmake doesn't do anyhing to pass /LIBPATH or adjust ENV{LIB} for vcpkg's installed dir.

Copy link

@github-actions github-actions bot left a 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 libspatialite but no changes to version or port version.
-- Version: 5.0.0#3
-- Old SHA: d7d124e4fcb212dba3d650817056ce23b6dc2a2f
-- New SHA: 2664ab3ce1be58c38cef34ff16d1247e389d7420
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

Copy link

@github-actions github-actions bot left a 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 libspatialite but no changes to version or port version.
-- Version: 5.0.0#3
-- Old SHA: d7d124e4fcb212dba3d650817056ce23b6dc2a2f
-- New SHA: aab1a670f15a9de47dacf9fc60a4299ec899bf4f
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

Copy link

@github-actions github-actions bot left a 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 libspatialite but no changes to version or port version.
-- Version: 5.0.1
-- Old SHA: 22ad27813f5f3bc3382718115ce3eb5a45174afd
-- New SHA: 83a71b0c02ca4a4c375c087e6ca5cad14c4227e6
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@PhoebeHui PhoebeHui self-assigned this Oct 8, 2021
@PhoebeHui PhoebeHui added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Oct 8, 2021
Copy link

@github-actions github-actions bot left a 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 libspatialite but no changes to version or port version.
-- Version: 5.0.1
-- Old SHA: 22ad27813f5f3bc3382718115ce3eb5a45174afd
-- New SHA: 2afa3fd80bdef276fd25f649850ad52258e33438
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

Copy link

@github-actions github-actions bot left a 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 libspatialite but no changes to version or port version.
-- Version: 5.0.1
-- Old SHA: 22ad27813f5f3bc3382718115ce3eb5a45174afd
-- New SHA: 2afa3fd80bdef276fd25f649850ad52258e33438
-- 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 973a7d517c09c8cfb7e6a548fcc260ca34ba7b60 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/l-/librttopo.json b/versions/l-/librttopo.json
index 12aa9b6..5c3773f 100644
--- a/versions/l-/librttopo.json
+++ b/versions/l-/librttopo.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "8b8db2d9576ec296a5f4c8b60fa341cc58405cec",
+      "git-tree": "d747c88d429ff781b11627623d0958967f912a41",
       "version": "1.1.0",
       "port-version": 5
     },

@dg0yt dg0yt force-pushed the libspatialite-nmake branch from 4634499 to a570500 Compare October 13, 2021 07:26
@BillyONeal
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dg0yt dg0yt mentioned this pull request Oct 15, 2021
@dg0yt dg0yt force-pushed the libspatialite-nmake branch from 8cfe9dd to a11cdd1 Compare October 28, 2021 16:12
@dg0yt dg0yt force-pushed the libspatialite-nmake branch from 6b6669a to a11cdd1 Compare October 28, 2021 17:08
@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 28, 2021

So, geos needs libm on linux:

/usr/bin/ld: /home/dg0yt/Projekte/vcpkg/vcpkg/installed/x64-linux/debug/lib/libgeosd.a(BufferOp.cpp.o): undefined reference to symbol 'pow@@GLIBC_2.2.5'
/usr/x86_64-linux-gnu/lib/libm.so.6: error adding symbols: DSO missing from command line

Let's update geos first...

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 29, 2021

Let's update geos first...

#21051

@dg0yt dg0yt mentioned this pull request Nov 1, 2021
@dg0yt dg0yt marked this pull request as ready for review November 10, 2021 01:45
@dg0yt
Copy link
Contributor Author

dg0yt commented Nov 12, 2021

@PhoebeHui Ping for review. Needed for proceeding with #20443.

@dg0yt
Copy link
Contributor Author

dg0yt commented Nov 12, 2021

Also needed for #21261.

@dg0yt dg0yt mentioned this pull request Nov 15, 2021
@dg0yt
Copy link
Contributor Author

dg0yt commented Nov 15, 2021

Ping again @PhoebeHui. This and #20443 was meant to be finished before #16494.

@PhoebeHui
Copy link
Contributor

@dg0yt, I'm sorry for my late response, I will test these features in my local machine firstly. And invite other members help review at the same time.

@PhoebeHui PhoebeHui requested a review from vicroms November 18, 2021 11:08
@PhoebeHui PhoebeHui added the requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function label Nov 18, 2021
@PhoebeHui
Copy link
Contributor

PhoebeHui commented Nov 19, 2021

All features test passed with x64-windows and x64-linux. Everything looks correct in .pc files after checked the output.

@PhoebeHui PhoebeHui removed the requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function label Nov 19, 2021
Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Nov 19, 2021
@vicroms
Copy link
Member

vicroms commented Nov 19, 2021

This looks great! Thanks for the PR!

@vicroms vicroms merged commit 94b2a07 into microsoft:master Nov 19, 2021
@dg0yt
Copy link
Contributor Author

dg0yt commented Nov 19, 2021

Thank you @PhoebeHui @vicroms.

@dg0yt dg0yt deleted the libspatialite-nmake branch December 11, 2021 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants