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

texmacs: 1.99.11 -> 1.99.13 #84816

Merged
merged 1 commit into from Oct 20, 2020
Merged

texmacs: 1.99.11 -> 1.99.13 #84816

merged 1 commit into from Oct 20, 2020

Conversation

@FlorianFranzen
Copy link
Contributor

@FlorianFranzen FlorianFranzen commented Apr 9, 2020

Motivation for this change

Update texmacs to latest release, now build against qt5

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS (not relevant, has own derivation)
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@FlorianFranzen
Copy link
Contributor Author

@FlorianFranzen FlorianFranzen commented May 20, 2020

@roconnor This is a pretty straight forward update. Can I do anything to move this along?

@FlorianFranzen FlorianFranzen changed the title texmacs: 1.99.11 -> 1.99.12 texmacs: 1.99.11 -> 1.99.13 Jul 2, 2020
@FlorianFranzen
Copy link
Contributor Author

@FlorianFranzen FlorianFranzen commented Jul 2, 2020

Rebased on latest master and bumped version update to 1.99.13.

@das-g
Copy link
Member

@das-g das-g commented Jul 2, 2020

Result of nixpkgs-review pr 84816 1

1 package built:
- texmacs

das-g
das-g approved these changes Jul 2, 2020
Copy link
Member

@das-g das-g left a comment

👍 change LGTM
👍 commit LGTM
✔️ resulting texmacs binary works

Edit: I've tested the wrong revision.

Copy link
Member

@das-g das-g left a comment

Tried to test the binary with

git checkout 49f14e49ebbe42699ba9393aa68e7aea73fae6fa
nix run -f . texmacs -c texmacs

this resulted in

Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
qt.qpa.plugin: Could not find the Qt platform plugin "xcb" in ""
This application failed to start because no Qt platform plugin could be initialized. Reinstalling the application may fix this problem.

Aborted (Speicherabzug geschrieben)

@balsoft
Copy link
Member

@balsoft balsoft commented Jul 2, 2020

Appears to work for me. NixOS unstable, sway 1.4,

nix shell github:FlorianFranzen/nixpkgs/texmacs#texmacs -c texmacs

Opens a window with TexMacs, all the editing works. However, for some reason it shows

Welcome to GNU TeXmacs 1.99.12

Which doesn't seem to match the version in the PR's title.

@FlorianFranzen
Copy link
Contributor Author

@FlorianFranzen FlorianFranzen commented Jul 2, 2020

@balsoft: Title does not match because upstream forgot to update the version string used by CMake before tagging the release. And the fix is only in master.

Also it should be noted that @das-g was able to resolve his issues by installing qt5.qtwayland, which is necessary to run Qt application natively under Wayland.

Copy link
Member

@das-g das-g left a comment

I can confirm that after adding qt5.qtwayland to environment.systemPackages (followed by nixos-rebuild switch),

nix-shell -I nixpkgs=. -p texmacs --run 'QT_QPA_PLATFORM=wayland texmacs'

in a repo with 49f14e4 checked out does work and launches a texmacs window. 🚀

Interestingly, despite setting QT_QPA_PLATFORM, the following warning is still emitted:

Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.

(Without the QT_QPA_PLATFORM=wayland, I'd still get the error messages qt.qpa.plugin: Could not find the Qt platform plugin "xcb" in "" and Aborted (Speicherabzug geschrieben) additionally to the warning, and no texmacs window would appear.)

The toolbar icons and the UI in general seem much larger than they were with texmacs 1.99.11:
Bildschirmfoto von 2020-07-03 00-46-32

@das-g das-g requested review from das-g and removed request for das-g Jul 2, 2020
@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 11, 2020

Result of nixpkgs-review pr 84816 1

1 package failed to build:
  • texmacs

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 11, 2020

Build fails for me:

/build/TeXmacs-1.99.13/src/Plugins/Qt/QTMPipeLink.cpp: In member function 'bool QTMPipeLink::launchCmd()':
/build/TeXmacs-1.99.13/src/Plugins/Qt/QTMPipeLink.cpp:52:39: warning: 'void QProcess::start(const QString&, QIODevice::OpenMode)' is deprecated: Use QProcess::start(const QString &program, const QStringList &arguments,OpenMode mode = ReadWrite) instead [-Wdeprecated-declarations]
   52 |   QProcess::start(utf8_to_qstring(cmd));
      |                                       ^
In file included from /nix/store/njywy8s62w93ymr822sc9cl03nv72q7r-qtbase-5.15.0-dev/include/QtCore/QProcess:1,
                 from /build/TeXmacs-1.99.13/src/Plugins/Qt/QTMPipeLink.hpp:17,
                 from /build/TeXmacs-1.99.13/src/Plugins/Qt/QTMPipeLink.cpp:15:
/nix/store/njywy8s62w93ymr822sc9cl03nv72q7r-qtbase-5.15.0-dev/include/QtCore/qprocess.h:168:10: note: declared here
  168 |     void start(const QString &command, OpenMode mode = ReadWrite);
      |          ^~~~~
[ 96%] Building CXX object src/CMakeFiles/texmacs_body.dir/Plugins/Qt/QTMWindow.cpp.o
[ 96%] Building CXX object src/CMakeFiles/texmacs_body.dir/Plugins/Qt/qt_chooser_widget.cpp.o
[ 96%] Building CXX object src/CMakeFiles/texmacs_body.dir/Plugins/Qt/qt_color_picker_widget.cpp.o
[ 96%] Building CXX object src/CMakeFiles/texmacs_body.dir/Plugins/Qt/qt_dialogues.cpp.o
/build/TeXmacs-1.99.13/src/Plugins/Qt/QTMStyle.cpp: In function 'void qtmDrawRoundedRect(QPainter*, const QRectF&, qreal, qreal, Qt::SizeMode)':
/build/TeXmacs-1.99.13/src/Plugins/Qt/QTMStyle.cpp:194:16: error: aggregate 'QPainterPath path' has incomplete type and cannot be defined
  194 |   QPainterPath path;
      |                ^~~~
make[2]: *** [src/CMakeFiles/texmacs_body.dir/build.make:6946: src/CMakeFiles/texmacs_body.dir/Plugins/Qt/QTMStyle.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
/build/TeXmacs-1.99.13/src/Plugins/Qt/QTMWidget.cpp: In member function 'virtual void QTMWidget::paintEvent(QPaintEvent*)':
/build/TeXmacs-1.99.13/src/Plugins/Qt/QTMWidget.cpp:240:48: warning: 'QVector<QRect> QRegion::rects() const' is deprecated: Use begin()/end() instead [-Wdeprecated-declarations]
  240 |   QVector<QRect> rects = event->region().rects();
      |                                                ^
In file included from /nix/store/njywy8s62w93ymr822sc9cl03nv72q7r-qtbase-5.15.0-dev/include/QtGui/qmatrix.h:45,
                 from /nix/store/njywy8s62w93ymr822sc9cl03nv72q7r-qtbase-5.15.0-dev/include/QtGui/qbrush.h:49,
                 from /nix/store/njywy8s62w93ymr822sc9cl03nv72q7r-qtbase-5.15.0-dev/include/QtGui/qpalette.h:46,
                 from /nix/store/njywy8s62w93ymr822sc9cl03nv72q7r-qtbase-5.15.0-dev/include/QtWidgets/qwidget.h:48,
                 from /nix/store/njywy8s62w93ymr822sc9cl03nv72q7r-qtbase-5.15.0-dev/include/QtWidgets/qframe.h:44,
                 from /nix/store/njywy8s62w93ymr822sc9cl03nv72q7r-qtbase-5.15.0-dev/include/QtWidgets/qabstractscrollarea.h:44,
                 from /nix/store/njywy8s62w93ymr822sc9cl03nv72q7r-qtbase-5.15.0-dev/include/QtWidgets/QAbstractScrollArea:1,
                 from /build/TeXmacs-1.99.13/src/Plugins/Qt/QTMScrollView.hpp:15,
                 from /build/TeXmacs-1.99.13/src/Plugins/Qt/QTMWidget.hpp:16,
                 from /build/TeXmacs-1.99.13/src/Plugins/Qt/QTMWidget.cpp:12:
/nix/store/njywy8s62w93ymr822sc9cl03nv72q7r-qtbase-5.15.0-dev/include/QtGui/qregion.h:125:20: note: declared here
  125 |     QVector<QRect> rects() const;
      |                    ^~~~~
make[1]: *** [CMakeFiles/Makefile2:317: src/CMakeFiles/texmacs_body.dir/all] Error 2
make: *** [Makefile:149: all] Error 2

@FlorianFranzen
Copy link
Contributor Author

@FlorianFranzen FlorianFranzen commented Oct 12, 2020

@doronbehar Thank you for your feedback. It indeed requires some minor backward-compatible patching with Qt 5.15, which hopefully will be included in the next release (texmacs/texmacs#43).

I included the patch and rebased everything on master.

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 12, 2020

builder for '/nix/store/qvhx713b8hl2qx7y4h39qvs24p6y8dr0-3cf56af92326b74538f5e943928199ba6e963d0b.patch.drv' failed with exit code 1; last 7 log lines:

  trying https://github.com/texmacs/texmacs/commits/3cf56af92326b74538f5e943928199ba6e963d0b.patch
    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                   Dload  Upload   Total   Spent    Left  Speed
    0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  curl: (22) The requested URL returned error: 404 Not Found
  error: cannot download 3cf56af92326b74538f5e943928199ba6e963d0b.patch from any mirror

@FlorianFranzen
Copy link
Contributor Author

@FlorianFranzen FlorianFranzen commented Oct 12, 2020

@doronbehar I am terribly sorry for wasting your time. Somehow snuck in a minor typo hidden by caching. Fixed now.

Should I also include da5b670 to fix the returned version as well?

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 12, 2020

Should I also include da5b670 to fix the returned version as well?

I'm surprised upstream didn't commit that before they tagged the release.. Yes, I think you should.

@FlorianFranzen
Copy link
Contributor Author

@FlorianFranzen FlorianFranzen commented Oct 12, 2020

I was just as surprised. But they do not use CMake for the official release and this only effect CMake builds. Anyhow, I included aforementioned commit/patch as well.

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 13, 2020

Result of nixpkgs-review pr 84816 1

1 package built:
  • texmacs

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 13, 2020

But it won't launch:

Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
qt.qpa.plugin: Could not find the Qt platform plugin "wayland" in ""
This application failed to start because no Qt platform plugin could be initialized. Reinstalling the application may fix this problem.

Aborted (core dumped)

You should use mkDerivation instead of stdenv.mkDerivation as libsForQt5.callPackage will wrap it properly. Additionally, to make the resulting executable not being double wrapped, you should replace wrapProgram with an appropriate addition to makeWrapperArgs.

@FlorianFranzen FlorianFranzen force-pushed the texmacs branch 2 times, most recently from 675cd0f to ccfb18f Oct 14, 2020
@FlorianFranzen
Copy link
Contributor Author

@FlorianFranzen FlorianFranzen commented Oct 14, 2020

@doronbehar: Thanks again for the feedback. That seemed to have been the issue indeed. Switched to Qt5's mkDerivation and replaced makeWrapper with qtWrapperArgs instead.

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 14, 2020

Result of nixpkgs-review pr 84816 1

1 package built:
  • texmacs

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 14, 2020

@FlorianFranzen is this launching for you without qt errors? It doesn't for me, and it won't create a wrapper. I suspect that it's because upstream's bin/texmacs is a shell script. So:

diff --git i/pkgs/applications/editors/texmacs/default.nix w/pkgs/applications/editors/texmacs/default.nix
index 8e985711213..aeef43e3f0b 100644
--- i/pkgs/applications/editors/texmacs/default.nix
+++ w/pkgs/applications/editors/texmacs/default.nix
@@ -69,6 +69,10 @@ mkDerivation {
     ])
   ];
 
+  postFixup = ''
+    wrapQtApp $out/bin/texmacs
+  '';
+
   inherit (common) postPatch;
 
   meta = common.meta // {

@FlorianFranzen
Copy link
Contributor Author

@FlorianFranzen FlorianFranzen commented Oct 17, 2020

Forgot that nix-build, nix-env and nix-shell all work slightly different and that resulted in me overriding the shell incorrectly. 🤦

With your fix it now also works in a pure nix-shell (i.e. without impure access to my systems qtwayland). Thank you for all your input.

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 20, 2020

(i.e. without impure access to my systems qtwayland). Thank you for all your input.

You should never need that. Current wrapQtAppsHook implementation wraps executables with env vars for wayland users as well. However, you may want:

export QT_QPA_PLATFORM=wayland

Credit: https://wiki.archlinux.org/index.php/Wayland#GUI_libraries

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 20, 2020

Result of nixpkgs-review pr 84816 1

1 package built:
  • texmacs

@doronbehar doronbehar merged commit 0f3d40e into NixOS:master Oct 20, 2020
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants