[aliyun-oss-cpp-sdk] add new port #40597

junluan · 2024-08-23T04:34:57Z

junluan · 2024-08-23T04:36:24Z

@microsoft-github-policy-service agree

ports/aliyun-oss-cpp-sdk/vcpkg-cmake-wrapper.cmake

dg0yt

Vendored deps in https://github.com/aliyun/aliyun-oss-cpp-sdk/tree/master/sdk/src/external should be replaced with vcpkg deps.

Cheney-W · 2024-08-23T10:52:33Z

/azp run

azure-pipelines · 2024-08-23T10:52:43Z

Azure Pipelines successfully started running 1 pipeline(s).

ports/aliyun-oss-cpp-sdk/unofficial-aliyun-oss-cpp-sdk-config.cmake

WangWeiLin-MV · 2024-08-26T08:34:10Z

All dependencies should be split out and packaged separately https://learn.microsoft.com/en-us/vcpkg/contributing/maintainer-guide#do-not-use-vendored-dependencies

https://github.com/aliyun/aliyun-oss-cpp-sdk/blob/1.10.0/sdk/CMakeLists.txt#L106-L126

Vendored deps in https://github.com/aliyun/aliyun-oss-cpp-sdk/tree/master/sdk/src/external should be replaced with vcpkg deps.

junluan · 2024-08-30T08:38:14Z

All dependencies should be split out and packaged separately https://learn.microsoft.com/en-us/vcpkg/contributing/maintainer-guide#do-not-use-vendored-dependencies

https://github.com/aliyun/aliyun-oss-cpp-sdk/blob/1.10.0/sdk/CMakeLists.txt#L106-L126

Vendored deps in https://github.com/aliyun/aliyun-oss-cpp-sdk/tree/master/sdk/src/external should be replaced with vcpkg deps.

Hi,
The 3rd deps used by source project is modified and enclosed with project's namespace. This should not conflict with the official deps

WangWeiLin-MV · 2024-08-30T08:53:12Z

ports/aliyun-oss-cpp-sdk/cmake-export.patch

+
+install(
+    EXPORT unofficial-aliyun-oss-cpp-sdk-config
+    NAMESPACE unofficial::


Suggested change

+ NAMESPACE unofficial::

+ NAMESPACE unofficial::aliyun-oss-cpp-sdk::

I changed this to unofficial::${TARGET_OUTPUT_NAME_PREFIX}. The project use this ${TARGET_OUTPUT_NAME_PREFIX}${PROJECT_NAME} target output name and the project name is cpp-sdk. So the export target name can be the same with the output lib name.

https://learn.microsoft.com/en-us/vcpkg/contributing/maintainer-guide#add-cmake-exports-in-an-unofficial--namespace

Any additional targets should be in the unofficial::<port>:: namespace.

It's now in the correct namespace. Please check.

find_package(unofficial-aliyun-oss-cpp-sdk CONFIG REQUIRED)
target_link_libraries(main PRIVATE unofficial::aliyun-oss-cpp-sdk)

Could not find the header file. And it looks like no public target_include_directories provided. Could you please give a simple example for testing the CMake config?

The header files are in default path: alibabacloud/oss/ directory

ports/aliyun-oss-cpp-sdk/cmake-export.patch

ports/aliyun-oss-cpp-sdk/portfile.cmake

WangWeiLin-MV · 2024-09-03T02:30:00Z

It's now in the correct namespace. Please check.

find_package(unofficial-aliyun-oss-cpp-sdk CONFIG REQUIRED) target_link_libraries(main PRIVATE unofficial::aliyun-oss-cpp-sdk)

The port usage failed with following test:

c.cpp:1:10: fatal error: alibabacloud/oss/Config.h: No such file or directory
    1 | #include "alibabacloud/oss/Config.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~

cmake_minimum_required(VERSION 3.25.1)
project(Prj LANGUAGES CXX)

add_executable(main c.cpp)
target_compile_features(main PRIVATE cxx_std_23)

find_package(unofficial-aliyun-oss-cpp-sdk CONFIG REQUIRED)
target_link_libraries(main PRIVATE unofficial::aliyun-oss-cpp-sdk)

junluan · 2024-09-10T12:07:05Z

It's now in the correct namespace. Please check.
find_package(unofficial-aliyun-oss-cpp-sdk CONFIG REQUIRED) target_link_libraries(main PRIVATE unofficial::aliyun-oss-cpp-sdk)

The port usage failed with following test:
c.cpp:1:10: fatal error: alibabacloud/oss/Config.h: No such file or directory
    1 | #include "alibabacloud/oss/Config.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
cmake_minimum_required(VERSION 3.25.1)
project(Prj LANGUAGES CXX)

add_executable(main c.cpp)
target_compile_features(main PRIVATE cxx_std_23)

find_package(unofficial-aliyun-oss-cpp-sdk CONFIG REQUIRED)
target_link_libraries(main PRIVATE unofficial::aliyun-oss-cpp-sdk)

Sorry for my mistake. I have test the port with the source cpp and cmake files:

#include "alibabacloud/oss/OssClient.h"

int main(int argc, char** argv) {
  AlibabaCloud::OSS::InitializeSdk();
  return 0;
}

cmake_minimum_required(VERSION 3.16)
project(Prj LANGUAGES CXX)

add_executable(main main.cpp)
target_compile_features(main PRIVATE cxx_std_14)

find_package(unofficial-aliyun-oss-cpp-sdk CONFIG REQUIRED)
target_link_libraries(main PRIVATE unofficial::aliyun-oss-cpp-sdk::cpp-sdk)

The output is fine.

-- Running vcpkg install
Detecting compiler hash for triplet x64-linux...
Compiler found: /usr/bin/c++
The following packages will be built and installed:
    aliyun-oss-cpp-sdk:[email protected]
  * curl[core,non-http,openssl,ssl]:[email protected]#1
  * openssl:[email protected]#1
  * vcpkg-cmake:x64-linux@2024-04-23
  * vcpkg-cmake-config:x64-linux@2024-05-23
  * vcpkg-cmake-get-vars:x64-linux@2023-12-31
  * zlib:[email protected]
Additional packages (*) will be modified to complete this operation.
Restored 7 package(s) from /home/admin/.cache/vcpkg/archives in 2.3 s. Use --debug to see more details.
Installing 1/7 vcpkg-cmake-config:x64-linux@2024-05-23...
Elapsed time to handle vcpkg-cmake-config:x64-linux: 21 ms
vcpkg-cmake-config:x64-linux package ABI: f69121c9d77d44a0199585f89b45af6e26d1a5992bc139c7bc9f405c88817f54
Installing 2/7 vcpkg-cmake:x64-linux@2024-04-23...
Elapsed time to handle vcpkg-cmake:x64-linux: 27.2 ms
vcpkg-cmake:x64-linux package ABI: f7184b8a7454d313691bff45bfd6761b060726952f06e6b25c5ac740fa51f4e4
Installing 3/7 vcpkg-cmake-get-vars:x64-linux@2023-12-31...
Elapsed time to handle vcpkg-cmake-get-vars:x64-linux: 28 ms
vcpkg-cmake-get-vars:x64-linux package ABI: c0e535c1083af29df239d4bdaade5b14cfb7679197e08d7be7907a82e8563fa0
Installing 4/7 openssl:[email protected]#1...
Elapsed time to handle openssl:x64-linux: 932 ms
openssl:x64-linux package ABI: c6dabfeceffecf0c2f96a396ad61ce5fe71c0ef0afe63f94764c7dd98ff02df4
Installing 5/7 zlib:[email protected]...
Elapsed time to handle zlib:x64-linux: 66.9 ms
zlib:x64-linux package ABI: 9dfe3dcbd9dc888f27df984f19994b916fc69acd60a3493d77991a3f8fc48dc8
Installing 6/7 curl[core,non-http,openssl,ssl]:[email protected]#1...
Elapsed time to handle curl:x64-linux: 231 ms
curl:x64-linux package ABI: 0654c9d6784b75d074b2722e6eaa17b440ea8f0634254373cd236532a53fa369
Installing 7/7 aliyun-oss-cpp-sdk:[email protected]...
Elapsed time to handle aliyun-oss-cpp-sdk:x64-linux: 1.4 s
aliyun-oss-cpp-sdk:x64-linux package ABI: fd504f7f4045597601b8c94cbff0b86097b114c21fe62b0f3269da65c0e2bf41
Total install time: 2.7 s
The package aliyun-oss-cpp-sdk provides CMake targets:

    find_package(unofficial-aliyun-oss-cpp-sdk CONFIG REQUIRED)
    target_link_libraries(main PRIVATE unofficial::aliyun-oss-cpp-sdk::cpp-sdk)

-- Running vcpkg install - done
-- The CXX compiler identification is GNU 10.2.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done (11.0s)
-- Generating done (0.0s)
-- Build files have been written to: /home/admin/workspace/aop_lab/app_source/projects/inference/test
(temp) ➜  test git:(master) ✗ make
[ 50%] Building CXX object CMakeFiles/main.dir/main.cpp.o
[100%] Linking CXX executable main
[100%] Built target main

WangWeiLin-MV

The port usage tests pass with the following triplets:

x64-linux

WangWeiLin-MV · 2024-09-11T02:50:48Z

ports/aliyun-oss-cpp-sdk/vcpkg.json

+  "description": "Alibaba Cloud Object Storage Service (OSS) is a cloud storage service provided by Alibaba Cloud, featuring massive capacity, security, a low cost, and high reliability.",
+  "homepage": "https://github.com/aliyun/aliyun-oss-cpp-sdk",
+  "license": "Apache-2.0",
+  "supports": "linux",


Upstream supports many platforms: https://github.com/aliyun/aliyun-oss-cpp-sdk/blob/1.10.0/CMakeLists.txt#L38-L58

Hi.
I add osx and android support. The source project use bundled libs when build on windows. This may have conflicts when using with vcpkg and also not match the design philosophy of vcpkg .Please check.

WangWeiLin-MV

The port usage tests pass with the following triplets:

x64-linux
arm64-osx

BillyONeal · 2024-09-13T03:12:41Z

Thanks for the new port!

https://github.com/aliyun/aliyun-oss-cpp-sdk/blob/0632d336ac861816669a1b0eecac30fb8ff71abb/CMakeLists.txt#L81-L90

I think what upstream is doing with this include is incorrect and they should just use find_package(OpenSSL REQUIRED) and friends, but I don't see obviously wrong output so I'm going to merge this as is. However, if you can contact upstream you might want to suggest this. Thanks again!

dg0yt · 2024-09-13T04:29:12Z

https://github.com/aliyun/aliyun-oss-cpp-sdk/blob/0632d336ac861816669a1b0eecac30fb8ff71abb/CMakeLists.txt#L81-L90

I think what upstream is doing with this include is incorrect and they should just use find_package(OpenSSL REQUIRED) and friends, but I don't see obviously wrong output so I'm going to merge this as is. However, if you can contact upstream you might want to suggest this. Thanks again!

This is badly wrong. Both curl and openssl have vcpkg cmake wrappers which are not activated by include(...). That command also doesn't set variables which can be expected by find modules, and it hides these modules from find_package debugging/tracing.

The massive problems are probably mitigated by FindCURL.make calling find_package(CURL CONFIG which triggers proper mechanism also for OpenSSL.

WangWeiLin-MV · 2024-09-13T07:52:04Z

https://github.com/aliyun/aliyun-oss-cpp-sdk/blob/0632d336ac861816669a1b0eecac30fb8ff71abb/CMakeLists.txt#L81-L90
I think what upstream is doing with this include is incorrect and they should just use find_package(OpenSSL REQUIRED) and friends, but I don't see obviously wrong output so I'm going to merge this as is. However, if you can contact upstream you might want to suggest this. Thanks again!

This is badly wrong. Both curl and openssl have vcpkg cmake wrappers which are not activated by include(...). That command also doesn't set variables which can be expected by find modules, and it hides these modules from find_package debugging/tracing.

The massive problems are probably mitigated by FindCURL.make calling find_package(CURL CONFIG which triggers proper mechanism also for OpenSSL.

Thanks for your comment, it has been modified.

牧亦 added 2 commits August 23, 2024 12:31

feature: add aliyun-oss-cpp-sdk

b65ece2

feature: add aliyun-oss-cpp-sdk

3cc64e9

junluan changed the title ~~Aliyun oss cpp sdk~~ [aliyun-oss-cpp-sdk] add new port Aug 23, 2024

MonicaLiu0311 assigned JonLiu1993 and WangWeiLin-MV and unassigned JonLiu1993 Aug 23, 2024

WangWeiLin-MV reviewed Aug 23, 2024

View reviewed changes

ports/aliyun-oss-cpp-sdk/vcpkg-cmake-wrapper.cmake Outdated Show resolved Hide resolved

WangWeiLin-MV added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Aug 23, 2024

dg0yt reviewed Aug 23, 2024

View reviewed changes

feature: add aliyun-oss-cpp-sdk

3b97297

牧亦 added 2 commits August 23, 2024 20:53

feature: add aliyun-oss-cpp-sdk

d74f9de

feature: add aliyun-oss-cpp-sdk

3e6c949

WangWeiLin-MV reviewed Aug 26, 2024

View reviewed changes

ports/aliyun-oss-cpp-sdk/unofficial-aliyun-oss-cpp-sdk-config.cmake Outdated Show resolved Hide resolved

牧亦 added 2 commits August 30, 2024 16:31

feature: add aliyun-oss-cpp-sdk

64ff7f4

feature: add aliyun-oss-cpp-sdk

5c05159

WangWeiLin-MV reviewed Aug 30, 2024

View reviewed changes

牧亦 added 6 commits August 30, 2024 17:02

feature: add aliyun-oss-cpp-sdk

b0cb076

feature: add aliyun-oss-cpp-sdk

6ddb96e

feature: add aliyun-oss-cpp-sdk

8280145

feature: add aliyun-oss-cpp-sdk

7190f6e

feature: add aliyun-oss-cpp-sdk

dc8a262

feature: add aliyun-oss-cpp-sdk

e5cadeb

WangWeiLin-MV marked this pull request as draft September 3, 2024 02:31

fix: fix public include directory

9606251

fix: fix public include directory

591f816

WangWeiLin-MV reviewed Sep 11, 2024

View reviewed changes

牧亦 added 2 commits September 11, 2024 12:52

fix: add osx and android support

c5b3176

fix: add osx and android support

f5e0cdc

junluan marked this pull request as ready for review September 12, 2024 02:21

WangWeiLin-MV approved these changes Sep 12, 2024

View reviewed changes

WangWeiLin-MV added the info:reviewed Pull Request changes follow basic guidelines label Sep 12, 2024

BillyONeal merged commit 29b2ea2 into microsoft:master Sep 13, 2024
16 checks passed

junluan deleted the aliyun-oss-cpp-sdk branch September 13, 2024 03:49

WangWeiLin-MV mentioned this pull request Sep 13, 2024

[aliyun-oss-cpp-sdk] Change to find package config mode #40954

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[aliyun-oss-cpp-sdk] add new port #40597

[aliyun-oss-cpp-sdk] add new port #40597

junluan commented Aug 23, 2024

junluan commented Aug 23, 2024

dg0yt left a comment

Cheney-W commented Aug 23, 2024

azure-pipelines bot commented Aug 23, 2024

WangWeiLin-MV commented Aug 26, 2024

junluan commented Aug 30, 2024

WangWeiLin-MV Aug 30, 2024

junluan Aug 30, 2024

WangWeiLin-MV Aug 30, 2024

junluan Aug 30, 2024

WangWeiLin-MV Sep 2, 2024

junluan Sep 2, 2024 •

edited

Loading

WangWeiLin-MV commented Sep 3, 2024 •

edited

Loading

junluan commented Sep 10, 2024

WangWeiLin-MV left a comment

WangWeiLin-MV Sep 11, 2024

junluan Sep 11, 2024

WangWeiLin-MV left a comment

BillyONeal commented Sep 13, 2024

dg0yt commented Sep 13, 2024

WangWeiLin-MV commented Sep 13, 2024

	+ NAMESPACE unofficial::
	+ NAMESPACE unofficial::aliyun-oss-cpp-sdk::

[aliyun-oss-cpp-sdk] add new port #40597

[aliyun-oss-cpp-sdk] add new port #40597

Conversation

junluan commented Aug 23, 2024

junluan commented Aug 23, 2024

dg0yt left a comment

Choose a reason for hiding this comment

Cheney-W commented Aug 23, 2024

azure-pipelines bot commented Aug 23, 2024

WangWeiLin-MV commented Aug 26, 2024

junluan commented Aug 30, 2024

WangWeiLin-MV Aug 30, 2024

Choose a reason for hiding this comment

junluan Aug 30, 2024

Choose a reason for hiding this comment

WangWeiLin-MV Aug 30, 2024

Choose a reason for hiding this comment

junluan Aug 30, 2024

Choose a reason for hiding this comment

WangWeiLin-MV Sep 2, 2024

Choose a reason for hiding this comment

junluan Sep 2, 2024 • edited Loading

Choose a reason for hiding this comment

WangWeiLin-MV commented Sep 3, 2024 • edited Loading

junluan commented Sep 10, 2024

WangWeiLin-MV left a comment

Choose a reason for hiding this comment

WangWeiLin-MV Sep 11, 2024

Choose a reason for hiding this comment

junluan Sep 11, 2024

Choose a reason for hiding this comment

WangWeiLin-MV left a comment

Choose a reason for hiding this comment

BillyONeal commented Sep 13, 2024

dg0yt commented Sep 13, 2024

WangWeiLin-MV commented Sep 13, 2024

junluan Sep 2, 2024 •

edited

Loading

WangWeiLin-MV commented Sep 3, 2024 •

edited

Loading