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

[openldap] Add new port #26122

Merged
merged 3 commits into from
Aug 12, 2022
Merged

[openldap] Add new port #26122

merged 3 commits into from
Aug 12, 2022

Conversation

GordonSmith
Copy link
Contributor

Signed-off-by: Gordon Smith [email protected]

Add new port for OpenLDAP

  • What does your PR fix?

    Fixes [New Port Request] libldap #18072

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

    !windows 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

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

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: checked-in files for openldap have changed but the version was not updated
version: 2.5.13
old SHA: 7b0d7b82fade87577d0e65fad39745d194554d79
new SHA: 23b70b41580bacd3ef2d99f6e7b18144099fa1da
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/openldap/vcpkg.json

Valid values for the license field can be found in the documentation

github-actions[bot]
github-actions bot previously approved these changes Aug 2, 2022
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.

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/openldap/vcpkg.json

Valid values for the license field can be found in the documentation

github-actions[bot]
github-actions bot previously approved these changes Aug 2, 2022
@Cheney-W Cheney-W self-assigned this Aug 3, 2022
@Cheney-W Cheney-W added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Aug 3, 2022
@Cheney-W Cheney-W changed the title [openlap] Add new port [openldap] Add new port Aug 3, 2022
ports/openldap/openssl.patch Outdated Show resolved Hide resolved
ports/openldap/portfile.cmake Outdated Show resolved Hide resolved
ports/openldap/portfile.cmake Outdated Show resolved Hide resolved
ports/openldap/portfile.cmake Outdated Show resolved Hide resolved
ports/openldap/portfile.cmake Outdated Show resolved Hide resolved
ports/openldap/portfile.cmake Outdated Show resolved Hide resolved
ports/openldap/portfile.cmake Outdated Show resolved Hide resolved
ports/openldap/portfile.cmake Outdated Show resolved Hide resolved
ports/openldap/vcpkg.json Outdated Show resolved Hide resolved
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: checked-in files for openldap have changed but the version was not updated
version: 2.5.13
old SHA: 33594c83f3e0627a4c32a259e1a0dff1f81f8e7e
new SHA: ec7e922c8b0660a12c0d9b7fd384dd58c118358f
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

@GordonSmith GordonSmith marked this pull request as draft August 3, 2022 16:57
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!

All manifest files must be formatted

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

Diff
diff --git a/ports/openldap/vcpkg.json b/ports/openldap/vcpkg.json
index bbad82e..7904e6c 100644
--- a/ports/openldap/vcpkg.json
+++ b/ports/openldap/vcpkg.json
@@ -12,4 +12,4 @@
       "host": true
     }
   ]
-}
\ No newline at end of file
+}
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 openldap have changed but the version was not updated
version: 2.5.13
old SHA: 33594c83f3e0627a4c32a259e1a0dff1f81f8e7e
new SHA: 861348de2d6256deb5b456f1c4916acfbcc68104
Did you remember to update the version or port version?
Use --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: checked-in files for openldap have changed but the version was not updated
version: 2.5.13
old SHA: 43d564aeeabf794b6066cffdec46a5afa7e0481a
new SHA: a7c9db055376c0c6e2255bc7f11cd028df6b0185
Did you remember to update the version or port version?
Use --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: checked-in files for openldap have changed but the version was not updated
version: 2.5.13
old SHA: a7c9db055376c0c6e2255bc7f11cd028df6b0185
new SHA: 921c7b6c6f8e7cb391cab2b6b3d996c732117ef1
Did you remember to update the version or port version?
Use --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: checked-in files for openldap have changed but the version was not updated
version: 2.5.13
old SHA: 921c7b6c6f8e7cb391cab2b6b3d996c732117ef1
new SHA: 94e7becc924bd4509cbe49a6e5afadd9f11b402e
Did you remember to update the version or port version?
Use --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: checked-in files for openldap have changed but the version was not updated
version: 2.5.13
old SHA: 94e7becc924bd4509cbe49a6e5afadd9f11b402e
new SHA: 7e675d72951dda5e915a9bd84321fa286cb771bb
Did you remember to update the version or port version?
Use --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: checked-in files for openldap have changed but the version was not updated
version: 2.5.13
old SHA: 7e675d72951dda5e915a9bd84321fa286cb771bb
new SHA: b4f30c6e699a72d1aa4c4ebfad5226a9eb8af489
Did you remember to update the version or port version?
Use --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: checked-in files for openldap have changed but the version was not updated
version: 2.5.13
old SHA: b4f30c6e699a72d1aa4c4ebfad5226a9eb8af489
new SHA: e93ceffd321b6b33b5b8a0142213e42392d9c239
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

github-actions[bot]
github-actions bot previously approved these changes Aug 5, 2022
@GordonSmith
Copy link
Contributor Author

@dy0yt - I added the m4 patch into the openssl.patch and suspect we have a winner!

dg0yt
dg0yt previously approved these changes Aug 5, 2022
Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

Great!
Maybe it is better to not squash too often/too early. The repo maintainers squash the PR with the merge anyways.

@GordonSmith
Copy link
Contributor Author

I normally do that - but I thought the bots here didn't like that?

@dg0yt
Copy link
Contributor

dg0yt commented Aug 5, 2022

I normally do that - but I thought the bots here didn't like that?

There is only one relevant bot, checking that the git tree object referenced in versions does actually exist. It won't complain as long as you update versions (every time) after committing port changes. It is easy to miss this update after amending a PR :-)

m4.patch is needed when libtool < 2.4.6
m4.patch will only work with autoconf < 2.70

Signed-off-by: Gordon Smith <[email protected]>
@GordonSmith GordonSmith dismissed stale reviews from dg0yt and github-actions[bot] via 6ffee86 August 8, 2022 11:34
github-actions[bot]
github-actions bot previously approved these changes Aug 8, 2022
@GordonSmith
Copy link
Contributor Author

@dg0yt Another tweak to fix OSX build - FYI the m4.patch won't work work with autoconf version >= 2.70 - looks like this ties in with libtool = 2.4.6 which doesn't need the m4 patch.

@GordonSmith GordonSmith requested a review from Cheney-W August 8, 2022 14:09
@Cheney-W Cheney-W added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Aug 9, 2022
Cheney-W
Cheney-W previously approved these changes Aug 9, 2022
Comment on lines +9 to +13
# Check autoconf version < 2.70
execute_process(COMMAND autoconf --version OUTPUT_VARIABLE AUTOCONF_VERSION_STR)
if(NOT "${AUTOCONF_VERSION_STR}" STREQUAL "" AND "${AUTOCONF_VERSION_STR}" MATCHES ".*2\\.[0-6].*")
vcpkg_list(APPEND EXTRA_PATCHES m4.patch)
endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer it the patch could be rewritten in a way so that it always applies. If it is only used on some systems, it may be ignored by CI and bitrot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree - unfortunately I have zero experience with configure and pkg-config...

The reason the m4 patch is being added is to trick the autoconfig -vfi to work. The other option is to remove the AUTOCONFIG and replace with a manual call to: autoreconf -vf and then there is no need to for the m4 patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making the portfile.cmake "clean" for libtool=2.4.6 breaks it for 2.4.2 and vice versa (the suggestion to add AC_CONFIG_MACRO_DIR([m4]) doesn't work when autoconf >= 2.70)

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!

All manifest files must be formatted

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

Diff
diff --git a/ports/openldap/vcpkg.json b/ports/openldap/vcpkg.json
index b579a60..ef51737 100644
--- a/ports/openldap/vcpkg.json
+++ b/ports/openldap/vcpkg.json
@@ -9,11 +9,11 @@
     "openssl"
   ],
   "features": {
-    "tools": {
-      "description": "Enable client tools"
-    },
     "cyrus-sasl": {
       "description": "with Cyrus SASL support"
+    },
+    "tools": {
+      "description": "Enable client tools"
     }
   }
-}
\ No newline at end of file
+}
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 openldap have changed but the version was not updated
version: 2.5.13
old SHA: ac4e7528c2ce477a77f0a1587b4cf9ae686afc93
new SHA: 63f4c67d336142daba83711983e54a8954794331
Did you remember to update the version or port version?
Use --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: checked-in files for openldap have changed but the version was not updated
version: 2.5.13
old SHA: 20c25ddcf22b069bed145e1350a02dec49ecff03
new SHA: 63f4c67d336142daba83711983e54a8954794331
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

I think this is fine but would like to ensure @dg0yt is happy before merging.

New port checklist:

  • Review the code ✅
  • Check the name against https://repology.org/
  • Check the name against Bing/Google. (If the first results aren't C++ related, try "name C++") ✅
  • Check the source code for optional find_packages ✅ (not a CMake port)
  • Check that the versioning scheme and license match what upstream says✅
  • Check that the source code comes from the upstream project's authoritative source.✅
  • Check that the generated usage is accurate. ✅ (I'm assuming that it's OK given that explicit usage is provided and looks reasonable)
  • Check that the reported license is accurate.✅

@BillyONeal
Copy link
Member

@dg0yt Can you confirm that you're happy? If I don't hear by tomorrow morning I will merge this.

@BillyONeal BillyONeal merged commit 83ef251 into microsoft:master Aug 12, 2022
@BillyONeal
Copy link
Member

Thanks!

@GordonSmith GordonSmith deleted the OPEN_LDAP branch August 12, 2022 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[New Port Request] libldap
4 participants