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

[new port] Ableton Link #25438

Merged
merged 6 commits into from
Jul 5, 2022
Merged

[new port] Ableton Link #25438

merged 6 commits into from
Jul 5, 2022

Conversation

JoergAtGithub
Copy link
Contributor

Describe the pull request

  • What does your PR fix?

    It adds a new port for Ableton Link: https://www.ableton.com/en/link/

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

    I guess it should work everywhere, but I tested only on Windows7

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

github-actions[bot]
github-actions bot previously approved these changes Jun 25, 2022
@JoergAtGithub JoergAtGithub marked this pull request as ready for review June 25, 2022 23:04
@Cheney-W Cheney-W added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Jun 27, 2022
@Cheney-W
Copy link
Contributor

  1. Please add double quotes to all relative paths in portfile.cmake.
  2. Missing expected versions file at: C:\a\1\s\versions\a-\ableton.json
    Run:
    vcpkg x-add-version ableton
    to create the versions file.
  3. All triplets failed during vcpkg_cmake_configure, you can get the failure logs from here: https://dev.azure.com/vcpkg/public/_build/results?buildId=74231&view=artifacts&pathAsName=false&type=publishedArtifacts

github-actions[bot]
github-actions bot previously approved these changes Jun 27, 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.

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 ableton have changed but the version was not updated
version: 3.0.5
old SHA: 83f39b9916d183f31a5b8654a69c1fcd31702c07
new SHA: 7d98ee50a8c03b33581f116105f2b921e464d7fa
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 Jun 27, 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.

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 ableton have changed but the version was not updated
version: 3.0.5
old SHA: 7d98ee50a8c03b33581f116105f2b921e464d7fa
new SHA: c3c16bf1628f5d92266d74a7921aa2e654ecb4c4
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!

All manifest files must be formatted

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

Diff
diff --git a/ports/ableton/vcpkg.json b/ports/ableton/vcpkg.json
index f55edfd..bcfe363 100644
--- a/ports/ableton/vcpkg.json
+++ b/ports/ableton/vcpkg.json
@@ -6,11 +6,11 @@
   "documentation": "http://ableton.github.io/link/",
   "license": "GPL-2.0-or-later",
   "dependencies": [
-    "catch2",
     {
       "name": "asio",
       "platform": "windows"
     },
+    "catch2",
     {
       "name": "vcpkg-cmake",
       "host": true
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 ableton have changed but the version was not updated
version: 3.0.5
old SHA: 7d98ee50a8c03b33581f116105f2b921e464d7fa
new SHA: 9d57348ae5c39a83b1089b9c33182f08467e22fa
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 Jun 27, 2022
github-actions[bot]
github-actions bot previously approved these changes Jun 27, 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.

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 ableton have changed but the version was not updated
version: 3.0.5
old SHA: d09e10ef9f1365094a6f019c5665fce0e19012ee
new SHA: 44f77e6b9f3c3d69235399aea71921a0db9871bb
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 Jun 27, 2022
@JoergAtGithub
Copy link
Contributor Author

  1. Please add double quotes to all relative paths in portfile.cmake.

  2. Missing expected versions file at: C:\a\1\s\versions\a-\ableton.json
    Run:
    vcpkg x-add-version ableton
    to create the versions file.

  3. All triplets failed during vcpkg_cmake_configure, you can get the failure logs from here: https://dev.azure.com/vcpkg/public/_build/results?buildId=74231&view=artifacts&pathAsName=false&type=publishedArtifacts

@Cheney-W I fixed 1.) and 2.) but with the configure fail I need help:
On my local computer it always finds the Catch2 dependency (I've no other copies of Catch2 on my system). On repeated Azure builds I saw it sometimes pass, but most time it failed to find Catch2.
The Catch2 package is only needed for the features coretest and discoverytest. Maybe I defined this in a wrong way?

@Cheney-W
Copy link
Contributor

I see that the part about catch2 in the cmakefile.txt of this port does not have any conditional restrictions:

Line 34 of ${SOURCE_PATH}\CMakeLists.txt: include(cmake_include/CatchConfig.cmake)
Line 81 of ${SOURCE_PATH}\src\CMakeLists.txt: target_link_libraries(${target} Catch2::Catch2WithMain Ableton::Link)

To fix configure fail, you need to make the above part only work under certain features.


file(INSTALL "${SOURCE_PATH}/AbletonLinkConfig.cmake" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}/")
file(INSTALL "${SOURCE_PATH}/cmake_include/" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}/cmake_include/")
file(INSTALL "${SOURCE_PATH}/include/" DESTINATION "${CURRENT_PACKAGES_DIR}/include")
Copy link
Contributor

Choose a reason for hiding this comment

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

Directly installing the entire include folder will also install the CMakeLists.txt file in it, and this file needs to be removed.

github-actions[bot]
github-actions bot previously approved these changes Jun 30, 2022
github-actions[bot]
github-actions bot previously approved these changes Jun 30, 2022
github-actions[bot]
github-actions bot previously approved these changes Jun 30, 2022
github-actions[bot]
github-actions bot previously approved these changes Jun 30, 2022
github-actions[bot]
github-actions bot previously approved these changes Jun 30, 2022
@Cheney-W
Copy link
Contributor

Cheney-W commented Jul 1, 2022

Could you please handle the new failure in x64-linux and x64-osx? The error as below:

CMake Error at /Users/vagrant/Data/work/1/s/scripts/buildsystems/vcpkg.cmake:824 (_find_package):
  By not providing "Findasio.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "asio", but
  CMake did not find one.

  Could not find a package configuration file provided by "asio" with any of
  the following names:

    asioConfig.cmake
    asio-config.cmake

  Add the installation prefix of "asio" to CMAKE_PREFIX_PATH or set
  "asio_DIR" to a directory containing one of the above files.  If "asio"
  provides a separate development package or SDK, be sure it has been
  installed.
Call Stack (most recent call first):
  cmake_include/AsioStandaloneConfig.cmake:3 (find_package)
  AbletonLinkConfig.cmake:41 (include)
  CMakeLists.txt:35 (include)

I saw below contents in the vcpkg.json file:

  "dependencies": [
    {
      "name": "asio",
      "platform": "windows"
    },

I think find_package(asio REQUIRED) should also have a conditional limit for platform judgment too.

github-actions[bot]
github-actions bot previously approved these changes Jul 1, 2022
@JoergAtGithub
Copy link
Contributor Author

@Cheney-W Thank you very much for your support! Thanks to your help, all CI jobs are now Pass!

@Cheney-W Cheney-W added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Jul 4, 2022
@vicroms vicroms merged commit 06d29ad into microsoft:master Jul 5, 2022
@JoergAtGithub
Copy link
Contributor Author

JoergAtGithub commented Jul 5, 2022

Thank you!

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.

3 participants