optional INSTALL_PATH qmake flag (on unix) #247

seb314 · 2020-05-18T23:00:32Z

I packaged Jamulus for nix/nixos and it would
be neat if there was an option to specify a
path at which to install the binary. I'm not
really familiar with qmake -- is this a good
way to achieve this?

(nixpkgs pull request: NixOS/nixpkgs#87914)

I packaged Jamulus for nix/nixos and it would be neat if there was an option to specify a path at which to install the binary. I'm not really familiar with qmake -- is this a good way to achieve this? (nixpkgs pull request: NixOS/nixpkgs#87914)

jtojnar · 2020-05-18T23:42:34Z

Maybe something like this might be better:

isEmpty(PREFIX) {
 PREFIX = /usr/local
}
isEmpty(BINDIR) {
 BINDIR = bin
}
BINDIR = $$absolute_path($$BINDIR, $$PREFIX)

target.path = $$BINDIR

Nixpkgs qmake setup hook already passes PREFIX to qmake.

corrados · 2020-05-20T15:25:19Z

Thanks for your contributions. What shall I add now? The proposal from jtojnar seems to be more suited, right? So if you agree, I would just add these code lines to the pro-file.

seb314 · 2020-05-20T19:46:39Z

Thanks for your contributions. What shall I add now? The proposal from jtojnar seems to be more suited, right? So if you agree, I would just add these code lines to the pro-file.

That would be great!
Updated the PR with jtojnar's version. (I assume the INSTALLS += target line is still required to enable the install target, right?)

corrados · 2020-05-21T06:51:38Z

I assume the INSTALLS += target line is still required to enable the install target, right?

Lets clarify that first. @jtojnar Can you please comment?

jtojnar · 2020-05-21T09:48:11Z

I am not familiar with qmake at all but I would expect it to be required.

jtojnar · 2020-05-21T09:48:20Z

Jamulus.pro

+    INSTALLS += target
+    target.path = $$BINDIR


This will likely need to be swapped:

Suggested change

INSTALLS += target

target.path = $$BINDIR

target.path = $$BINDIR

INSTALLS += target

Not sure if this is valid syntax but it might also be nicer to use more descriptive target name:

Suggested change

INSTALLS += target

target.path = $$BINDIR

jamulus_bin_target.path = $$BINDIR

INSTALLS += jamulus_bin_target

both works (on my laptop)
EDIT what I checked here is swapping the lines, not renaming the target. Not sure if renaming the target breaks other stuff in the .pro file

Reading https://doc.qt.io/qt-5/qmake-advanced-usage.html#installing-files, target seems to be a magic name that will choose something in unspecified manner. For the other installed files, you will need to specify .files.

jtojnar · 2020-05-21T09:50:23Z

Jamulus.pro

+    isEmpty(PREFIX) {
+        PREFIX = /usr/local
+    }
+    isEmpty(BINDIR) {
+        BINDIR = bin
+    }
+    BINDIR = $$absolute_path($$BINDIR, $$PREFIX)


I would move this to the top of unix conditional. It will be useful when we decide to install other files like .desktop entry or icon.

Oh, these are actually available in the repo and AUR package installs them:

https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=jamulus#n26

isEmpty(DATADIR) { DATADIR = share } DATADIR = $$absolute_path($$DATADIR, $$PREFIX) ICONDIR = $$DATADIR/icons/hicolor/48x48/apps # https://specifications.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html#install_icons DESKTOPDIR = $$DATADIR/applications ... icon_target.files = src/res/fronticon.png icon_target.path = $$ICONDIR INSTALLS += icon_target desktop_file_target.files = distributions/jamulus.desktop desktop_file_target.path = $$DESKTOPDIR INSTALLS += desktop_file_target

We should probably do that too.

tormodvolden · 2020-05-21T12:36:18Z

Isn't this what DESTDIR is for? https://cmake.org/cmake/help/latest/envvar/DESTDIR.html

I believe this is how the Debian package build installs to a temporary directory before copying everything into package files.

[EDIT] Or I get your motivation wrong. You really want to install the binary somewhere else than $PREFIX/bin ?

BTW, the AUR script could possibly also use "make DESTDIR=$pkgdir" and "make install".

jtojnar · 2020-05-21T13:47:05Z

@tormodvolden No, DESTDIR only serves to change the temporary installation root, it does not affect the directory structure or final destination where things will be installed. For example, Debian would set DESTDIR=/tmp/build-foo and PREFIX=/usr. The separately overridable BINDIR is a GNU convention (see https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html).

Also this project does not use CMake but qmake, and, on UNIX, does not install any files at all (there is no install make target). This PR attempts to fix that.

corrados · 2020-05-21T19:39:40Z

Ok, so I take it that the change in that current pull request is the final code to be added? So I will merge it now. If anything is open or shall be changed, just create a new pull request.

jtojnar · 2020-05-21T21:08:03Z

We would still want to add installation of icon and desktop files as described above: #247 (comment)

seb314 · 2020-05-24T22:27:29Z

We would still want to add installation of icon and desktop files as described above: #247 (comment)

I tried those extra changes (here) but (at least when building with nix) there are some issues that make it not straighforward:

the existing .desktop file assumes a lowercase binary "jamulus", so the .desktop install target will break for everyone who doesn't use CONFIG+=noupcasename
the icon doesn't work (at least not the gnome I tested it with desktopManager.gnome3.enable = true; in nixos). Tried copying fronticon.png to jamulus.png, but no success so far. The AUR package does install -Dm644 src/res/fronticon.png $pkgdir/usr/share/pixmaps/jamulus.png, didn't try that yet for the nix package.

Anyway, the install target for the binary works fine with changes that are already merged

jtojnar · 2020-05-24T23:22:19Z

Looking at https://github.com/seb314/jamulus/blob/submit/install_add_icon_and_desktop/distributions/debian/rules, we might want to install the icon from distributions instead. And also the service file, thought it would require more changes. But changing

https://github.com/seb314/jamulus/blob/f0d4e0a4f28b1b769c17bcd20ff899eba4c9c1bf/distributions/jamulus-server.service#L11

to something like

ExecStart=/usr/bin/jamulus -s -n --servername %u -l %L/jamulus -e jamulus.fischvolk.de -g -o "%u;;"

might be more useful anyway. See https://www.freedesktop.org/software/systemd/man/systemd.service.html#ExecStart=, https://www.freedesktop.org/software/systemd/man/systemd.service.html#Command%20lines and https://www.freedesktop.org/software/systemd/man/systemd.unit.html#Specifiers for some info.

Ideally the desktop and service file would contain @BIN_PATH@ placeholder that would be substituted based on the installation path but I was not able to find a way to do that in qmake. (Autotools have AC_CONFIG_FILES, CMake has configure_file and Meson has configure_file.)

Maybe it would be fine to port the project to a different build system as qmake will apparently not be supported by Qt 6.

jtojnar · 2020-05-24T23:50:43Z

Just found QMAKE_SUBSTITUTES. Of course it is not even mentioned in the qmake documentation.

corrados · 2020-05-25T14:51:35Z

Maybe it would be fine to port the project to a different build system as qmake will apparently not be supported by Qt 6.

To my knowledge qmake will still be supported by Qt6 but cmake is the preferred tool to use. But I may be wrong. I also read that Qt will create a script to convert qmake files to cmake files. Let's wait until this script is ready and tested. Then we can try out to convert the Jamulus qmake file.

seb314 mentioned this pull request May 18, 2020

jamulus: init at 3.5.3 NixOS/nixpkgs#87914

Merged

10 tasks

use jtojnar's suggestion, add default install target with PREFIX/BINDIR

660e07e

jtojnar reviewed May 21, 2020

View reviewed changes

corrados merged commit 003eebe into jamulussoftware:master May 21, 2020

This was referenced Jun 2, 2020

Status of Jamulus integration in Linux distributions #223

Closed

Install jamulus.desktop #331

Merged

jtojnar mentioned this pull request Jul 6, 2020

qjournalctl: init at 0.6.2 NixOS/nixpkgs#78815

Closed

10 tasks

jtojnar mentioned this pull request Sep 16, 2020

Implement the rest of the checks jtojnar/nixpkgs-hammering#1

Open

34 tasks

jtojnar mentioned this pull request Feb 4, 2022

jamesdsp: init at 2.3 NixOS/nixpkgs#154158

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

optional INSTALL_PATH qmake flag (on unix) #247

optional INSTALL_PATH qmake flag (on unix) #247

seb314 commented May 18, 2020

jtojnar commented May 18, 2020

corrados commented May 20, 2020

seb314 commented May 20, 2020

corrados commented May 21, 2020

jtojnar commented May 21, 2020

jtojnar May 21, 2020

jtojnar May 21, 2020

seb314 May 21, 2020 •

edited

Loading

jtojnar May 21, 2020

jtojnar May 21, 2020

jtojnar May 21, 2020 •

edited

Loading

tormodvolden commented May 21, 2020 •

edited

Loading

jtojnar commented May 21, 2020 •

edited

Loading

corrados commented May 21, 2020

jtojnar commented May 21, 2020

seb314 commented May 24, 2020

jtojnar commented May 24, 2020

jtojnar commented May 24, 2020

corrados commented May 25, 2020

optional INSTALL_PATH qmake flag (on unix) #247

optional INSTALL_PATH qmake flag (on unix) #247

Conversation

seb314 commented May 18, 2020

jtojnar commented May 18, 2020

corrados commented May 20, 2020

seb314 commented May 20, 2020

corrados commented May 21, 2020

jtojnar commented May 21, 2020

jtojnar May 21, 2020

Choose a reason for hiding this comment

jtojnar May 21, 2020

Choose a reason for hiding this comment

seb314 May 21, 2020 • edited Loading

Choose a reason for hiding this comment

jtojnar May 21, 2020

Choose a reason for hiding this comment

jtojnar May 21, 2020

Choose a reason for hiding this comment

jtojnar May 21, 2020 • edited Loading

Choose a reason for hiding this comment

tormodvolden commented May 21, 2020 • edited Loading

jtojnar commented May 21, 2020 • edited Loading

corrados commented May 21, 2020

jtojnar commented May 21, 2020

seb314 commented May 24, 2020

jtojnar commented May 24, 2020

jtojnar commented May 24, 2020

corrados commented May 25, 2020

seb314 May 21, 2020 •

edited

Loading

jtojnar May 21, 2020 •

edited

Loading

tormodvolden commented May 21, 2020 •

edited

Loading

jtojnar commented May 21, 2020 •

edited

Loading