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

Add port - libosdp #37556

Merged
merged 1 commit into from
Mar 26, 2024
Merged

Add port - libosdp #37556

merged 1 commit into from
Mar 26, 2024

Conversation

sidcha
Copy link
Contributor

@sidcha sidcha commented Mar 19, 2024

This patch adds a port for libosdp - a cross-platform open source implementation of IEC 60839-11-5 Open Supervised Device Protocol (OSDP). The protocol is intended to improve interoperability among access control and security products. It supports Secure Channel (SC) for encrypted and authenticated communication between configured devices.

Upstream: https://github.com/goToMain/libosdp

  • Changes comply with the maintainer guide.
  • The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
  • Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all find_package calls are REQUIRED, are satisfied by vcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx.
  • The versioning scheme in vcpkg.json matches what upstream says.
  • The license declaration in vcpkg.json matches what upstream says.
  • The installed as the "copyright" file matches what upstream says.
  • The source code of the component installed comes from an authoritative source.
  • The generated "usage text" is accurate. See adding-usage for context.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is in the new port's versions file.
  • Only one version is added to each modified port's versions file.

@sidcha
Copy link
Contributor Author

sidcha commented Mar 19, 2024

@microsoft-github-policy-service agree

@jimwang118 jimwang118 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Mar 20, 2024
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.

IMO the comments in portfile.cmake can all be removed.
(Maybe the c-utils deserve a comment that they take the place of a git submodule. This gives a hint for choosing the REF when updating the port.)

ports/libosdp/portfile.cmake Outdated Show resolved Hide resolved
ports/libosdp/usage Outdated Show resolved Hide resolved
ports/libosdp/vcpkg.json Outdated Show resolved Hide resolved
@sidcha sidcha force-pushed the libosdp branch 2 times, most recently from 57728a3 to 05e67ce Compare March 20, 2024 21:59
@sidcha
Copy link
Contributor Author

sidcha commented Mar 21, 2024

@jimwang118 I need help with this CI failure. I can build LibOSDP on Windows with msvc tool chain but for some reason all Windows builds are failing (and hard to tell without the log file). Can you give me some hints on how I can reproduce this issue locally?

@sidcha sidcha requested a review from dg0yt March 21, 2024 06:56
@dg0yt
Copy link
Contributor

dg0yt commented Mar 21, 2024

The log files are available as "artifacts" downloads on azure pipelines. Use the three dots on the right side when hovering an artifact.
https://dev.azure.com/vcpkg/public/_build/results?buildId=100859&view=artifacts&pathAsName=false&type=publishedArtifacts

@sidcha sidcha force-pushed the libosdp branch 3 times, most recently from cd096bc to bd7af11 Compare March 21, 2024 19:48
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.

I think the patching is too invasive and should be trimmed.
Maybe set the EXCLUDE_FROM_ALL property on the target which isn't needed,
and limit the install step to the target which is needed.

ports/libosdp/cmake-add-static-shared-option.patch Outdated Show resolved Hide resolved
@sidcha sidcha force-pushed the libosdp branch 2 times, most recently from 74743bb to 3509e8e Compare March 22, 2024 13:06
@sidcha
Copy link
Contributor Author

sidcha commented Mar 22, 2024

@dg0yt Please ignore all patch files for now; they are also pushed to the upstream. Once I've ironed out all of those issues here, will make another release to remove all the patch files.

For now, I'm just trying to appease MSVC in different places :)

@sidcha sidcha force-pushed the libosdp branch 2 times, most recently from 938b5b9 to d97a1c4 Compare March 23, 2024 10:27
@sidcha sidcha requested a review from dg0yt March 23, 2024 11:17
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.

The port looks clean. Somebody else must review and approve.

There would be some nit-picks on the upstream CMake build, but I don't want to enter a thorough review now.

@jimwang118
Copy link
Contributor

Usage test pass with following triplets:

x86-windows
x64-windows

@jimwang118 jimwang118 added the info:reviewed Pull Request changes follow basic guidelines label Mar 25, 2024
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.

Can you apply these changes? I tried to but the PR was created without permission for me to do so...

PS D:\vcpkg> push-pr goToMain:libosdp    
ERROR: Permission to goToMain/vcpkg.git denied to BillyONeal.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
diff --git a/ports/libosdp/portfile.cmake b/ports/libosdp/portfile.cmake
index 45f166f14..8270cf7e9 100644
--- a/ports/libosdp/portfile.cmake
+++ b/ports/libosdp/portfile.cmake
@@ -6,7 +6,7 @@ vcpkg_from_github(
     HEAD_REF master
 )
 
-# Downloaod and extract the c-utils submodule at ${SOURCE_PATH}/utils as
+# Download and extract the c-utils submodule at ${SOURCE_PATH}/utils as
 # it would be during a recursive checkout.
 #
 # Note: During package upgrade, the submodule ref needs to be updated.
@@ -17,6 +17,7 @@ vcpkg_from_github(
     SHA512 a0902a504fe6ffd1ce0f32d0a16decf0e113d1211d19e63f4fb539082254769f0a6484414a49f52956e45ed802b2c2f8430e87a06c24ac84205421cdffb4d3f0
     HEAD_REF master
 )
+
 file(REMOVE_RECURSE "${SOURCE_PATH}/utils")
 file(COPY "${UTILS_SOURCE_PATH}/" DESTINATION "${SOURCE_PATH}/utils")
 
diff --git a/ports/libosdp/usage b/ports/libosdp/usage
index 5fc6d7afb..058891d78 100644
--- a/ports/libosdp/usage
+++ b/ports/libosdp/usage
@@ -5,5 +5,5 @@ libosdp provides CMake targets:
 
 libosdp provides pkg-config modules:
 
-    # Open Supervised Device Protocol (OSDP) Library
-    libosdp
+  # Open Supervised Device Protocol (OSDP) Library
+  libosdp
diff --git a/versions/l-/libosdp.json b/versions/l-/libosdp.json
index b408b5fcc..a59360af4 100644
--- a/versions/l-/libosdp.json
+++ b/versions/l-/libosdp.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "229759fd15e93ca2e175b1c421569480c77ba20c",
+      "git-tree": "fbbc5d6f19e023ef42737fc939fcb9bd186ecc4c",
       "version": "3.0.5",
       "port-version": 0
     }

This patch adds a port for libosdp - a cross-platform open source
implementation of IEC 60839-11-5 Open Supervised Device Protocol (OSDP).
The protocol is intended to improve interoperability among access
control and security products. It supports Secure Channel (SC) for
encrypted and authenticated communication between configured devices.

Upstream: https://github.com/goToMain/libosdp
@sidcha
Copy link
Contributor Author

sidcha commented Mar 26, 2024

@BillyONeal, thanks for the review. Applied.

@BillyONeal
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BillyONeal BillyONeal merged commit fe39c3c into microsoft:master Mar 26, 2024
16 checks passed
@BillyONeal
Copy link
Member

Thanks for the new port <3!

@sidcha sidcha deleted the libosdp branch March 28, 2024 09:38
@sidcha sidcha restored the libosdp branch March 28, 2024 09:38
@sidcha sidcha deleted the libosdp branch March 28, 2024 09:38
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.

None yet

4 participants