Skip to content

Get rid of embedding completely #7837

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.

Sign up for GitHub

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

Draft
Rossmaxx wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Get rid of embedding completely #7837

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b6fef20
nuked the x11embed submodule
Rossmaxx Apr 7, 2025
aa989ad
begone embedding
Rossmaxx Apr 7, 2025
da2af7c
fix builds
Rossmaxx Apr 7, 2025
07c940b
tidied up everything and a few fixes here and there
Rossmaxx Apr 7, 2025
671467f
more fixing builds
Rossmaxx Apr 7, 2025
4279daf
fix ci
Rossmaxx Apr 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
more fixing builds
  • Loading branch information
@Rossmaxx
Rossmaxx committed Apr 7, 2025
commit 671467f65e8c85cf46c619bd465f23df53cef173
11 changes: 10 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,12 @@ check_library_exists(rt shm_open "" LMMS_HAVE_LIBRT)

LIST(APPEND CMAKE_PREFIX_PATH "${CMAKE_INSTALL_PREFIX}")

FIND_PACKAGE(Qt5 5.9.0 COMPONENTS Core Gui Widgets Xml Svg REQUIRED)
IF(LMMS_BUILD_LINUX)
FIND_PACKAGE(Qt5 REQUIRED COMPONENTS Core Gui Widgets Xml Svg X11Extras REQUIRED)
Copy link
Copy Markdown
Contributor Author

@Rossmaxx Rossmaxx Apr 7, 2025

Choose a reason for hiding this comment

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

I would appreciate if we find a way to push the x11extras stuff to the vst cmake scripts

Copy link
Copy Markdown
Member

@tresf tresf Apr 7, 2025

Choose a reason for hiding this comment

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

I would appreciate if we find a way to push the x11extras stuff to the vst cmake scripts

If you're removing embedding in this PR, why would you still need them?

Copy link
Copy Markdown
Member

@tresf tresf Apr 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@Rossmaxx on second thought, maybe we keep this... QtX11Info seems to mentioned in this migration guide: https://doc.qt.io/qt-6/extras-changes-qt6.html. (Also we can patch this on qt6 branch)

With regards to where this code belongs, we'll likely need this logic for other plugin types (e.g. LV2) so it's most likely in the right place. We could guard it with a LINUX + VST check if we wanted to be more explicit.

Copy link
Copy Markdown
Contributor

@sakertooth sakertooth Apr 13, 2025

Choose a reason for hiding this comment

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

x11extras isn't related to the qt5-x11embed submodule we ship right? It should be fine to leave it alone if my understanding is correct.

ELSE()
FIND_PACKAGE(Qt5 REQUIRED COMPONENTS Core Gui Widgets Xml Svg REQUIRED)
ENDIF()

FIND_PACKAGE(Qt5 COMPONENTS LinguistTools QUIET)

include_directories(SYSTEM
Expand All @@ -192,6 +197,10 @@ SET(QT_LIBRARIES
Qt5::Svg
)

IF(LMMS_BUILD_LINUX)
list(APPEND QT_LIBRARIES Qt5::X11Extras)
ENDIF()

# Resolve Qt5::qmake to full path for use in packaging scripts
GET_TARGET_PROPERTY(QT_QMAKE_EXECUTABLE "${Qt5Core_QMAKE_EXECUTABLE}" IMPORTED_LOCATION)

Expand Down
Loading