Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[vcpkg baseline][omplapp] Skip due to conflict with ompl #40151

Conversation

WangWeiLin-MV
Copy link
Contributor

@WangWeiLin-MV WangWeiLin-MV commented Jul 29, 2024

Fix https://dev.azure.com/vcpkg/public/_build/results?buildId=105429&view=logs&j=ac0731cb-f2f9-5764-2e0a-a6fa8ebb4a08

error: The following files are already installed in /Users/vcpkg/Data/installed/arm64-osx and are in conflict with omplapp:arm64-osx
Installed by ompl:arm64-osx  
debug/lib/libompl.dylib
    lib/libompl.dylib

Since omplapp coupled with ompl, and ompl has been fixed and enabled CI test in #40080:

  • Mark omplapp as skip
  • Remove command of "delete conflicts ompl files".

Checklist

  • Changes comply with the maintainer guide.
  • SHA512s are updated for each updated download.
  • The "supports" clause reflects platforms that may be fixed by this new version.
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

Test

The port installation tests pass with the following triplets:

  • x64-linux

@WangWeiLin-MV WangWeiLin-MV added category:port-bug The issue is with a library, which is something the port should already support info:internal This PR or Issue was filed by the vcpkg team. labels Jul 29, 2024
@WangWeiLin-MV WangWeiLin-MV force-pushed the ports/omplapp/remove-ompl-libs branch 2 times, most recently from 40beea7 to f77df9c Compare July 29, 2024 10:06
@WangWeiLin-MV WangWeiLin-MV marked this pull request as ready for review July 29, 2024 10:22
MonicaLiu0311
MonicaLiu0311 previously approved these changes Jul 30, 2024
@MonicaLiu0311 MonicaLiu0311 added the info:reviewed Pull Request changes follow basic guidelines label Jul 30, 2024
@dg0yt
Copy link
Contributor

dg0yt commented Jul 30, 2024

How do you ensure that the vendored ompl runtime libs created and used during the build of omplapp are compatible with the runtime libs installed by ompl?

@WangWeiLin-MV WangWeiLin-MV marked this pull request as draft July 30, 2024 06:27
@WangWeiLin-MV WangWeiLin-MV removed the info:reviewed Pull Request changes follow basic guidelines label Jul 30, 2024
@WangWeiLin-MV
Copy link
Contributor Author

How do you ensure that the vendored ompl runtime libs created and used during the build of omplapp are compatible with the runtime libs installed by ompl?

Thanks for your comment, the build-in ompl version is not the same as ompl.


Given that omplapp is highly coupled to its built-in ompl, and based on the history of current port (#18908), temporarily disable it in ci.baseline.txt, which also allows for conflicting files to not need to be deleted.

@WangWeiLin-MV WangWeiLin-MV changed the title [omplapp] Remove conflicts files [omplapp] Skip due to conflict with omplapp Jul 30, 2024
@WangWeiLin-MV WangWeiLin-MV changed the title [omplapp] Skip due to conflict with omplapp [omplapp] Skip due to conflict with ompl Jul 30, 2024
@WangWeiLin-MV WangWeiLin-MV force-pushed the ports/omplapp/remove-ompl-libs branch 3 times, most recently from 31d34c3 to 732fe2e Compare July 30, 2024 08:43
@WangWeiLin-MV WangWeiLin-MV force-pushed the ports/omplapp/remove-ompl-libs branch from 732fe2e to c0a66a9 Compare July 30, 2024 09:10
@BillyONeal
Copy link
Member

According to https://ompl.kavrakilab.org/download.html "The OMPL.app repository contains the OMPL repository as a submodule.". I checked, and omplapp's submodule for 1.6 references the same SHA as for ompl 1.6, and they were released on the same day.

Does it make sense to remove omplapp and add ompl[app] which locks the inner ompl sources at the same value?

@WangWeiLin-MV
Copy link
Contributor Author

According to https://ompl.kavrakilab.org/download.html "The OMPL.app repository contains the OMPL repository as a submodule.". I checked, and omplapp's submodule for 1.6 references the same SHA as for ompl 1.6, and they were released on the same day.

Does it make sense to remove omplapp and add ompl[app] which locks the inner ompl sources at the same value?

omplapp does not directly use the artifact of ompl, but bypasses ompl/CMakeLists.txt to compile its components like this omplapp/CMakeLists.txt#L252-L257:

add_subdirectory(ompl/doc)
add_subdirectory(ompl/src)
add_subdirectory(ompl/py-bindings)
add_subdirectory(ompl/tests)
add_subdirectory(ompl/demos)
add_subdirectory(ompl/scripts)

And there are many different parts between the two files omplapp/CMakeLists.txt and ompl/CMakeLists.txt.

Splitting the two projects seems need a lot of changes, I will discuss with upstream.

@talregev
Copy link
Contributor

talregev commented Aug 2, 2024

I will discuss with upstream.

Can you post the link to the discussion?

@WangWeiLin-MV
Copy link
Contributor Author

@talregev Sorry for late, I submitted the issue now.

@talregev
Copy link
Contributor

talregev commented Aug 6, 2024

@WangWeiLin-MV Don't worry about it.
Thank you for sharing the link.

@WangWeiLin-MV WangWeiLin-MV changed the title [omplapp] Skip due to conflict with ompl [vcpkg baseline][omplapp] Skip due to conflict with ompl Aug 9, 2024
@data-queue data-queue added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Aug 13, 2024
@MonicaLiu0311 MonicaLiu0311 removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Nov 8, 2024
@WangWeiLin-MV
Copy link
Contributor Author

Close before upstream update.

@WangWeiLin-MV WangWeiLin-MV deleted the ports/omplapp/remove-ompl-libs branch November 8, 2024 10:15
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants