[guile] Added guile port #26336

verifiedtm · 2022-08-13T15:17:29Z

Describe the pull request

What does your PR fix?

Fixes [New Port Request] Guile #11252
Which triplets are supported/not supported? Have you updated the CI baseline?

This port currently only supports linux platforms.

Guile is notoriously tricky to build on non-GNU platforms so this isn't really a surprise to me. I believe these triples will be supportable with further patching/test work, but that is not something I'm able to do for this initial port.

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

github-actions

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: checked-in files for guile have changed but the version was not updated
version: 3.0.8#1
old SHA: 2cc19e8916fff51419747a2e9865743427f530da
new SHA: 99bfa48e99617de3d61b694585a4c12452c1f9ed
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

ports/guile/vcpkg.json

scripts/ci.baseline.txt

Cheney-W · 2022-08-15T06:01:55Z

Please do not add empty usage file.

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff

diff --git a/ports/guile/vcpkg.json b/ports/guile/vcpkg.json
index 941aa2c..97fa0b9 100644
--- a/ports/guile/vcpkg.json
+++ b/ports/guile/vcpkg.json
@@ -4,8 +4,8 @@
   "description": "GNU's programming and extension language",
   "homepage": "https://www.gnu.org/software/guile/",
   "documentation": "https://www.gnu.org/software/guile/manual/",
-  "supports": "linux",
   "license": "LGPL-3.0-or-later",
+  "supports": "linux",
   "dependencies": [
     "bdwgc",
     "libunistring"

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout d293ac220dd126165d24907b6f07e6b658b3329f -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 0f0d086..c8fc876 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -2758,7 +2758,7 @@
     },
     "guile": {
       "baseline": "3.0.8",
-      "port-version": 1
+      "port-version": 0
     },
     "guilite": {
       "baseline": "2022-05-05",
diff --git a/versions/g-/guile.json b/versions/g-/guile.json
index 3deac21..500126c 100644
--- a/versions/g-/guile.json
+++ b/versions/g-/guile.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "76a4347fc7a2d962674de8f006974a34423a6e3d",
+      "version": "3.0.8",
+      "port-version": 0
+    },
     {
       "git-tree": "99bfa48e99617de3d61b694585a4c12452c1f9ed",
       "version": "3.0.8",

github-actions

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: checked-in files for guile have changed but the version was not updated
version: 3.0.8
old SHA: 24a34ad3dbc6fe5c13c1b7c97f282d121ae6df5b
new SHA: 76a4347fc7a2d962674de8f006974a34423a6e3d
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

JavierMatosD · 2022-08-16T21:55:22Z

Describe the pull request

What does your PR fix?
Fixes [New Port Request] Guile #11252

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

This port currently only supports linux platforms.

Guile is notoriously tricky to build on non-GNU platforms so this isn't really a surprise to me. I believe these triples will be supportable with further patching/test work, but that is not something I'm able to do for this initial port.

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

Building this port fails with the following output:
Can't exec "autopoint": No such file or directory at /usr/share/autoconf/Autom4te/FileUtils.pm line 345.
My understanding is that autopoint is a part of gettext. Since this port depends on gettext, it should be included in the dependencies along with any other dependencies it might require. However, even after installing gettext, the port fails to build.

github-actions

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: checked-in files for guile have changed but the version was not updated
version: 3.0.8
old SHA: 76a4347fc7a2d962674de8f006974a34423a6e3d
new SHA: 0f41c7303e1186595d21d6918ec4f6cc8eb99f9c
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

verifiedtm · 2022-08-16T22:14:30Z

I'll certainly add gettext to the dependency list.

What other build error do you get with gettext installed?

update: I've got a pure nix-shell working and am now able to recreate the error

BillyONeal · 2022-08-19T19:09:17Z

Approved since I didn't see any major problems. However, given that we had a failure due to missing autopoint I would prefer if there was a blob at the top of the portfile like:

if (VCPKG_TARGET_IS_LINUX)
    message(WARNING "${PORT} currently requires autopoint from the system package manager:\n\nIt can be installed on Ubuntu systems via apt-get install autopoint.")
endif()

Since there seem to be indications that this is related to gettext it would also be nice to understand if our gettext port should be producing it. (In which case there's some missing binding to the gettext port, probably because it's a host dependency and that isn't being parachuted in here...)

BillyONeal

I just checked and gettext does produce autopoint; the port needs to be hooked up to that. Probably missing a vcpkg_add_to_path?

verifiedtm · 2022-08-19T19:13:07Z

The updates to the port manifest now solve the autopoint binary problem. In my first draft of the port I had missed this dependency (and others) because I was not running the port in an isolated environment.

BillyONeal · 2022-08-19T19:14:49Z

The updates to the port manifest now solve the autopoint binary problem. In my first draft of the port I had missed this dependency (and others) because I was not running the port in an isolated environment.

I don't see where it is hooked up. There is ADD_BIN_TO_PATH but that won't do the right thing when the host triplet != the target triplet.

verifiedtm · 2022-08-19T19:17:31Z

#26336 (comment)

Re this review, setting the correct dependencies in the manifest seem to have added it to the path. When I tested it in my local environment this worked.

If this is not correct I can certainly add the directory to the path manually, I just want to make sure I'm on the right page.

BillyONeal · 2022-08-19T19:19:22Z

#26336 (comment)

Re this review, setting the correct dependencies in the manifest seem to have added it to the path. When I tested it in my local environment this worked.

If this is not correct I can certainly add the directory to the path manually, I just want to make sure I'm on the right page.

I see. Hmmm.... I don't see where it's doing that. Let me ask around briefly..

ras0219-msft · 2022-08-19T20:33:01Z

ADD_BIN_TO_PATH means "I am not capable of cross-compilation -- I need .so/.dll files from the target triplet available so I can immediately try to run things during my build".

If the build instead wants to run a tool from a "host": true dependency, you probably want a directive like vcpkg_add_to_path("${CURRENT_HOST_INSTALLED_DIR}/tools/something")

verifiedtm · 2022-08-19T21:28:51Z

The gettext port does install the autopoint binary, so in that case what is the correct way to get that tools directory added to the path?

BillyONeal · 2022-08-19T21:50:04Z

The gettext port does install the autopoint binary, so in that case what is the correct way to get that tools directory added to the path?

Based on:

I believe it should be:

vcpkg_add_to_path("${CURRENT_HOST_INSTALLED_DIR}/tools/gettext/bin")

dg0yt · 2022-08-20T05:08:01Z

You don't have to deal with PATH if you use the proper host dependency: It is handled by gettext's vcpkg-port-config.cmake.

verifiedtm · 2022-08-22T01:54:54Z

I have (re)checked in my isolated environment, without the autopoint binary present system-wide, and without manually adding the tools directory to the path, it does appear to be getting the correct autopoint binary. I think @dg0yt is correct, and having the tools feature on the dependency is all that's required.

dg0yt · 2022-08-22T03:39:39Z

You don't have to deal with PATH if you use the proper host dependency: It is handled by gettext's vcpkg-port-config.cmake.

.. we are relying on it in other ports.

verifiedtm · 2022-08-24T03:05:46Z

@BillyONeal @Cheney-W is there anything else that can be done for this? in it's current state the ports dependencies are correctly configured to use the gettext-provided autopoint binary, as far as I can tell this works fully.

Thomas1664

The quotes are important to prevent errors if the path contains spaces. vcpkg_copy_pdbs does nothing on non Windows platforms so it isn't really a problem.

Thomas1664 · 2022-08-24T15:22:58Z

ports/guile/portfile.cmake

+      SHA512 7b2728e849a3ee482fe9a167dd76cc4835e911cc94ca0724dd51e8a813a240c6b5d2de84de16b46469ab24305b5b153a3c812fec942e007d3310bba4d1cf947d
+  )
+
+vcpkg_extract_source_archive(GUILE_SOURCES ARCHIVE ${GUILE_ARCHIVE})


Suggested change

vcpkg_extract_source_archive(GUILE_SOURCES ARCHIVE ${GUILE_ARCHIVE})

vcpkg_extract_source_archive(GUILE_SOURCES ARCHIVE "${GUILE_ARCHIVE}")

Please add quotes.

Thomas1664 · 2022-08-24T15:24:02Z

ports/guile/portfile.cmake

+    AUTOCONFIG
+  )
+vcpkg_install_make()
+vcpkg_copy_pdbs()


Suggested change

vcpkg_copy_pdbs()

vcpkg_copy_pdbs()

This is only needed on Windows, so you can safely remove it.

JavierMatosD · 2022-08-24T16:03:58Z

Thank you!

Fix

github-actions bot requested changes Aug 13, 2022

View reviewed changes

github-actions bot previously approved these changes Aug 13, 2022

View reviewed changes

verifiedtm dismissed GitHub Actions’s stale review via eaa5b2a August 13, 2022 15:27

github-actions bot previously approved these changes Aug 13, 2022

View reviewed changes

verifiedtm dismissed GitHub Actions’s stale review via b586138 August 13, 2022 15:36

github-actions bot previously approved these changes Aug 13, 2022

View reviewed changes

verifiedtm marked this pull request as ready for review August 13, 2022 15:44

Cheney-W self-assigned this Aug 15, 2022

Cheney-W added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Aug 15, 2022

Cheney-W reviewed Aug 15, 2022

View reviewed changes

ports/guile/vcpkg.json Outdated Show resolved Hide resolved

Cheney-W reviewed Aug 15, 2022

View reviewed changes

scripts/ci.baseline.txt Outdated Show resolved Hide resolved

Cheney-W added the requires:author-response label Aug 15, 2022

verifiedtm dismissed GitHub Actions’s stale review via 75a66d2 August 15, 2022 12:54

verifiedtm requested a review from Cheney-W August 15, 2022 12:54

github-actions bot requested changes Aug 15, 2022

View reviewed changes

github-actions bot previously approved these changes Aug 15, 2022

View reviewed changes

Cheney-W previously approved these changes Aug 16, 2022

View reviewed changes

Cheney-W added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Aug 16, 2022

JavierMatosD added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Aug 16, 2022

verifiedtm dismissed stale reviews from Cheney-W and GitHub Actions via 9c27b19 August 16, 2022 22:13

github-actions bot requested changes Aug 16, 2022

View reviewed changes

github-actions bot previously approved these changes Aug 16, 2022

View reviewed changes

verifiedtm dismissed GitHub Actions’s stale review via f293b10 August 16, 2022 22:59

BillyONeal previously requested changes Aug 19, 2022

View reviewed changes

JavierMatosD added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Aug 19, 2022

JavierMatosD added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Aug 19, 2022

JavierMatosD added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Aug 19, 2022

verifiedtm requested a review from BillyONeal August 22, 2022 01:56

[guile] Added guile port

4d2fcf1

verifiedtm dismissed stale reviews from Cheney-W and GitHub Actions via 4d2fcf1 August 22, 2022 14:33

github-actions bot approved these changes Aug 22, 2022

View reviewed changes

Cheney-W added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Aug 24, 2022

Thomas1664 reviewed Aug 24, 2022

View reviewed changes

JavierMatosD merged commit c350275 into microsoft:master Aug 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[guile] Added guile port #26336

[guile] Added guile port #26336

verifiedtm commented Aug 13, 2022 •

edited

Loading

github-actions bot left a comment

Cheney-W commented Aug 15, 2022

github-actions bot left a comment

github-actions bot left a comment

JavierMatosD commented Aug 16, 2022

What does your PR fix?

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

Does your PR follow the maintainer guide?

If you have added/updated a port: Have you run `./vcpkg x-add-version --all` and committed the result?

github-actions bot left a comment

verifiedtm commented Aug 16, 2022 •

edited

Loading

BillyONeal commented Aug 19, 2022

BillyONeal left a comment •

edited

Loading

verifiedtm commented Aug 19, 2022

BillyONeal commented Aug 19, 2022

verifiedtm commented Aug 19, 2022

BillyONeal commented Aug 19, 2022

ras0219-msft commented Aug 19, 2022

verifiedtm commented Aug 19, 2022

BillyONeal commented Aug 19, 2022

dg0yt commented Aug 20, 2022

verifiedtm commented Aug 22, 2022

dg0yt commented Aug 22, 2022

verifiedtm commented Aug 24, 2022

Thomas1664 left a comment

Thomas1664 Aug 24, 2022

Thomas1664 Aug 24, 2022

JavierMatosD commented Aug 24, 2022

	vcpkg_extract_source_archive(GUILE_SOURCES ARCHIVE ${GUILE_ARCHIVE})
	vcpkg_extract_source_archive(GUILE_SOURCES ARCHIVE "${GUILE_ARCHIVE}")

[guile] Added guile port #26336

[guile] Added guile port #26336

Conversation

verifiedtm commented Aug 13, 2022 • edited Loading

What does your PR fix?

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

Does your PR follow the maintainer guide?

If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

github-actions bot left a comment

Choose a reason for hiding this comment

Cheney-W commented Aug 15, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

JavierMatosD commented Aug 16, 2022

What does your PR fix?

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

Does your PR follow the maintainer guide?

If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

github-actions bot left a comment

Choose a reason for hiding this comment

verifiedtm commented Aug 16, 2022 • edited Loading

BillyONeal commented Aug 19, 2022

BillyONeal left a comment • edited Loading

Choose a reason for hiding this comment

verifiedtm commented Aug 19, 2022

BillyONeal commented Aug 19, 2022

verifiedtm commented Aug 19, 2022

BillyONeal commented Aug 19, 2022

ras0219-msft commented Aug 19, 2022

verifiedtm commented Aug 19, 2022

BillyONeal commented Aug 19, 2022

dg0yt commented Aug 20, 2022

verifiedtm commented Aug 22, 2022

dg0yt commented Aug 22, 2022

verifiedtm commented Aug 24, 2022

Thomas1664 left a comment

Choose a reason for hiding this comment

Thomas1664 Aug 24, 2022

Choose a reason for hiding this comment

Thomas1664 Aug 24, 2022

Choose a reason for hiding this comment

JavierMatosD commented Aug 24, 2022

verifiedtm commented Aug 13, 2022 •

edited

Loading

If you have added/updated a port: Have you run `./vcpkg x-add-version --all` and committed the result?

If you have added/updated a port: Have you run `./vcpkg x-add-version --all` and committed the result?

verifiedtm commented Aug 16, 2022 •

edited

Loading

BillyONeal left a comment •

edited

Loading