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

Support mac frameworks with ecm output #388

Closed
wants to merge 8 commits into from

Conversation

mnutt
Copy link
Collaborator

@mnutt mnutt commented Dec 31, 2016

Supercedes #386, this is just squashed and cleaned up.

This allows a mac application to use QtWebKit dropped in via QT += webkit webkitwidgets. QtWebKit is built as a framework, so this updates the *.pri and *.pc files to pass the correct -framework QtWebKit values to clang++.

This doesn't change the linux pri files, except to add QT.webkit.module = Qt5WebKit, which I believe normalizes it with the built-in qt modules.

This ensures they can be easily included in projects with `QT += webkit`, etc.
@mnutt mnutt mentioned this pull request Dec 31, 2016
@mnutt mnutt changed the title ecm mac frameworks Support mac frameworks with ecm output Dec 31, 2016
@annulen annulen mentioned this pull request Jan 6, 2017
@mnutt
Copy link
Collaborator Author

mnutt commented Feb 10, 2017

I believe I have this where it generates correct pkgconfig and pri files for both linux and mac without substantially changing the ecm api. The only minor difference is that linux's pri file changes the name from QtWebKit to Qt5WebKit, which I believe is just descriptive and should be fine? If we want to keep it QtWebKit we can do so but it will involve adding an extra ecm argument.

LIB_NAME QtWebKit
BASE_NAME ${WEBKIT_BASE_NAME}
LIB_NAME ${WEBKIT_LIB_NAME}
INCLUDE_INSTALL_DIR "${KDE_INSTALL_INCLUDEDIR}/QtWebKit"
Copy link
Member

Choose a reason for hiding this comment

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

Bug here: INCLUDE_INSTALL_DIR is defined above in WebKit_PRI_ARGUMENTS.

It is crucial that when we install into Qt prefix (KDE_INSTALL_USE_QT_SYS_PATHS condition) qmake variable $$QT_MODULE_INCLUDE_BASE is used instead of prefix path known to CMake. This makes .pri files relocatable, and also allows compatibility with unusual installation layouts of Qt

LIB_NAME QtWebKitWidgets
BASE_NAME ${WEBKITWIDGETS_BASE_NAME}
LIB_NAME ${WEBKITWIDGETS_LIB_NAME}
INCLUDE_INSTALL_DIR "${KDE_INSTALL_INCLUDEDIR}/QtWebKitWidgets"
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

@@ -185,6 +185,13 @@ function(ECM_GENERATE_PRI_FILE)
set(PRI_TARGET_CONFIG "staticlib")
endif ()

if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
Copy link
Member

Choose a reason for hiding this comment

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

  • I think it's OK to use shorter APPLE instead of ${CMAKE_SYSTEM_NAME} MATCHES "Darwin", at least we do this in lots of places already
  • In static builds we don't produce frameworks, so you at least should check for APPLE AND NOT QT_STATIC_BUILD. See also 846a83b
  • Though it's also possible to create Qt build that uses shared libraries, but does not create frameworks. We don't support this configuration yet, but it would be nice to have

Choose a reason for hiding this comment

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

where is the endif ?

if (EGPF_FILENAME_VAR)
set(${EGPF_FILENAME_VAR} ${PKGCONFIG_FILENAME} PARENT_SCOPE)
endif()

set(PROJECT_LIBS "-L${CMAKE_INSTALL_PREFIX}/${EGPF_LIB_INSTALL_DIR} -l${PKGCONFIG_TARGET_LIBNAME}")

if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
Copy link
Member

Choose a reason for hiding this comment

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

See below about frameworks

Choose a reason for hiding this comment

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

where is the endif ?

if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
set(PROJECT_LIBS "-F${CMAKE_INSTALL_PREFIX}/${EGPF_LIB_INSTALL_DIR} -framework ${PKGCONFIG_TARGET_LIBNAME}")
set(PKGCONFIG_TARGET_INCLUDES "-I${CMAKE_INSTALL_PREFIX}/${EGPF_LIB_INSTALL_DIR}/${PKGCONFIG_TARGET_LIBNAME}.framework/includes")
endif ()
Copy link
Member

Choose a reason for hiding this comment

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

@jiyuusoft Here is endif

@annulen annulen added this to the Alpha milestone Feb 22, 2017
annulen added a commit that referenced this pull request Apr 12, 2017
Also don't produce pkg-config files for frameworks as this is not
supported by Qt.

Based on PR #388 by @mnutt.

Change-Id: I4d64a1ca107e8ef5e211d1c3fd133019b5268b4e
@annulen
Copy link
Member

annulen commented Apr 12, 2017

@mnutt I've merged a part of your changes as 1686903 with some adjustments

@annulen
Copy link
Member

annulen commented Dec 28, 2017

I've imported latest version of ECMGeneratePkgConfigFile from upstream, it adds DESCRIPTION field. I'll delay fixes for mac frameworks, as at the moment Qt doesn't provide .pc files for framework builds, so QtWebKit's files are not useful. This may change in future, I believe Qt should strive to provide first class pkg-config support on all desktop platforms

@annulen annulen removed this from the v5.212.0-beta milestone Dec 28, 2017
@mnutt mnutt closed this Jan 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants