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

qt58: extend darwin compatibility #24340

Merged
merged 1 commit into from Apr 8, 2017
Merged

Conversation

periklis
Copy link
Contributor

@periklis periklis commented Mar 25, 2017

Motivation for this change

Enable failing QT-Modules for building qt58.full for x86_64-darwin:

  • QTConnectivity
  • QTLocation
  • QTMultimedia
  • QTSensors
    - [ ] QTWebEngine
    - [ ] QTWebKit

Wrap failing QT-Apps:

  • QTTools: Assistant.app, Designer.app, Linguist.app, pixeltool.app, qdbusviewer.app, qml.app
  • QTDeclarative: qmleasing, qmlscene, qmltestrunner

Enable darwin-specific QT-Modules:

  • QTMacextras
Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@periklis, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ttuegel, @bjornfor and @abbradar to be potential reviewers.

@periklis periklis force-pushed the topic_qt58_modules branch 11 times, most recently from ab86039 to dbd921a Compare April 1, 2017 14:37
@periklis periklis changed the title [WIP] qt58: extend darwin compatibility qt58: extend darwin compatibility Apr 3, 2017
@periklis
Copy link
Contributor Author

periklis commented Apr 3, 2017

I had to pull the last two packages qtwebkit and qtwebengine out of this PR. The reason is in the way we handle the qmake-mkspecs for the mac plattform. It needs rework in order to have a cleaner setup than the sed-lines in postPatch in qtbase/default.nix

@periklis
Copy link
Contributor Author

periklis commented Apr 3, 2017

@ttuegel & @acowley & @abbradar & @bjornfor : would you mind to review this PR?


propagatedBuildInputs = [
dbus zlib minizip alsaLib snappy nss protobuf jsoncpp libevent
#dbus zlib minizip snappy nss protobuf jsoncpp libevent
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should delete this line if each dependency is listed elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx, i'll have to give qtwebengine and qtwebkit a special look by rewriting the sed-stuff i mentioned above. The main reason is that because of the the sed-stuff OpenGL and CoreFoundation cannot be propagated correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, that sounds unusual, I look forward to seeing what's going on!

@abbradar
Copy link
Member

abbradar commented Apr 3, 2017

Seems okay to me (sans a small nitpick, same as @acowley's) but I did only a very quick review (I don't have much time now, sorry!). Also seen some generic improvements (wrapping some Qt utilities).

@periklis periklis force-pushed the topic_qt58_modules branch 3 times, most recently from 325635f to 27380c6 Compare April 4, 2017 12:02
@LnL7 LnL7 added the 6.topic: darwin Running or building packages on Darwin label Apr 4, 2017
@periklis
Copy link
Contributor Author

periklis commented Apr 5, 2017

@LnL7 would you mind to review and if you like merge this PR?

@periklis periklis force-pushed the topic_qt58_modules branch 2 times, most recently from 04821c5 to 3e3b287 Compare April 6, 2017 07:29
postFixup = ''
moveToOutput "bin/qdbus" "$out"
moveToOutput "bin/qtpaths" "$out"
'';

postInstall = ''
wrapQtProgram $out/bin/qcollectiongenerator
Copy link
Member

Choose a reason for hiding this comment

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

Are these all the binaries? If so it's probably better to use a glob here so programs added in a new version are also wrapped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No only the binaries that have dynamic library dependencies.

# Therefore WebKit provides adequate header files.
INCLUDEPATH = $${ROOT_WEBKIT_DIR}/Source/WTF/icu $$INCLUDEPATH
- LIBS += -licucore
+ LIBS += /usr/lib/libicucore.dylib
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be added to __impureHostDeps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx for the hint, but can you elaborate what impact __impureHostDeps has on buildInputs and the like?

@periklis periklis force-pushed the topic_qt58_modules branch 2 times, most recently from e620743 to 834c92c Compare April 7, 2017 08:35
@periklis
Copy link
Contributor Author

periklis commented Apr 8, 2017

@LnL7 Can we merge this? So that i can build upon these to figure out why qtwebkit fails. Thx in advance.

@ttuegel ttuegel merged commit e0b1288 into NixOS:master Apr 8, 2017
@periklis periklis deleted the topic_qt58_modules branch July 25, 2018 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants