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

Package modem-manager-gui #48959

Merged
merged 2 commits into from Oct 30, 2018
Merged

Conversation

alesya-h
Copy link
Member

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • 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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

@alesya-h alesya-h changed the title Package network-manager-gui Package modem-manager-gui Oct 24, 2018

buildInputs = [
pkgconfig
gtk3-x11
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just gtk3?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't knew it existed - nix-env -f "<nixpkgs>" -qaP|grep gtk3 doesn't list gtk3, only gtk3-x11. Is there a command that would have listed gtk3?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

I would use nix-index’s nix-locate. It is available on IRC as well, as ,locale libgtk-3 command, but it suffers from the same problem – Nix does not know which attribute path is canonical. Grepping nixpkgs source code is still the best way, IMHO.

};

buildInputs = [
pkgconfig
Copy link
Contributor

Choose a reason for hiding this comment

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

Programs executed during build should go to nativeBuildInputs.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

];

preConfigure = ''
sed -i 's|! test -e "/usr/include/gdbm/gdbm.h"|false|' configure
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you create a patch file instead? sed calls get outdated pretty fast without any warning.

sha256 = "04p08gzvi068bd3slbnaghajwzp4h5jacib640k5shngqimkl84p";
};

buildInputs = [
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems you forgot to add meson to nativeBuildInputs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Even though meson and ninja are specified on the project page, it also builds with just ./configure and make. I tried to make it work with meson, but for some reason it turned out to be much harder.

Copy link
Contributor

Choose a reason for hiding this comment

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

Meson is the preferred build system. What issues are you encountering specifically?

Copy link
Member Author

Choose a reason for hiding this comment

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

...
Build targets in project: 24
Found ninja-1.8.2 at /nix/store/35wvn5g0igx8xpqzm66j3anpi0icy6rg-ninja-1.8.2/bin/ninja
meson: enabled parallel building
building
build flags: -j1 -l1
[1/66] Generating modem-manager-gui.appdata.xml_merge with a custom command.
[2/66] Generating modem-manager-gui.desktop_merge with a custom command.
[3/66] Generating man-de with a custom command.
FAILED: man/de/modem-manager-gui.1.gz
/build/hg-archive/man/manhelper.py ../man/de/de.po man/de/modem-manager-gui.1.gz
/bin/sh: /build/hg-archive/man/manhelper.py: not found
ninja: build stopped: subcommand failed.
builder for '/nix/store/jnn4w26z9z13kqgij31s0mw0y1p8p2m0-modem-manager-gui.drv' failed with exit code 1

Copy link
Contributor

Choose a reason for hiding this comment

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

It cannot find the /usr/bin/env in the shebang of the script You will need to do

postPatch = ''
  patchShebangs man/manhelper.py
'';

possibly preceded by chmod +xing the file.

Copy link
Member Author

Choose a reason for hiding this comment

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

manhelper.py is already executable, so it worked without chmod, but then the script itself failed:

meson: enabled parallel building
building
build flags: -j1 -l1
[1/66] Generating modem-manager-gui.appdata.xml_merge with a custom command.
[2/66] Generating modem-manager-gui.desktop_merge with a custom command.
[3/66] Generating man-de with a custom command.
FAILED: man/de/modem-manager-gui.1.gz
/build/hg-archive/man/manhelper.py ../man/de/de.po man/de/modem-manager-gui.1.gz
Traceback (most recent call last):
  File "/build/hg-archive/man/manhelper.py", line 20, in <module>
    for line in intfile:
  File "/nix/store/nrl0l79a48924xb0897ap572xf29ciir-python3-3.6.6/lib/python3.6/encodings/ascii.py", line 26, in decode
    return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 363: ordinal not in range(128)
ninja: build stopped: subcommand failed.
builder for '/nix/store/v9v1sbnpr6xazzp8qrr98yb32wnpmyi9-modem-manager-gui.drv' failed with exit code 1

Copy link
Contributor

Choose a reason for hiding this comment

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

It is one of issues with our python. You can fix it by adding LC_ALL = "en_US.utf-8" attribute to the derivation and glibcLocales to nativeBuildInputs.

Copy link
Member Author

Choose a reason for hiding this comment

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

That worked. Updated the package to build with meson and ninja.

gdbm
gtkspell3

itstool
Copy link
Contributor

Choose a reason for hiding this comment

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

itstool belongs to nativeBuildInputs

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@@ -0,0 +1,16 @@
--- a/configure 2018-10-25 14:19:33.361529316 +1100
Copy link
Contributor

@jtojnar jtojnar Oct 25, 2018

Choose a reason for hiding this comment

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

This patch should no longer be required with meson.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

{ stdenv, buildEnv, pkgconfig, python3, fetchhg, gtk3, glib, gdbm, gtkspell3, itstool, libappindicator-gtk3, perldevelPackages, glibcLocales, meson, ninja }:

stdenv.mkDerivation rec {
name = "modem-manager-gui";
Copy link
Contributor

Choose a reason for hiding this comment

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

name is supposed to be modem-manager-gui-${version}. At least until NixOS/rfcs#35 is implemented.

stdenv.mkDerivation rec {
name = "modem-manager-gui";
version = "0.4.17";
rev = "v${version}";
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this here?

Copy link
Member Author

Choose a reason for hiding this comment

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

a copy-paste error

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed


meta = with stdenv.lib; {
homepage = https://linuxonly.ru/page/modem-manager-gui;
description = ''
Copy link
Contributor

Choose a reason for hiding this comment

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

description is supposed to be short: see https://repology.org/metapackage/modem-manager-gui/information#Summaries for what other distros use. You can add this to longDescription.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@jtojnar
Copy link
Contributor

jtojnar commented Oct 26, 2018

In case you want ofono, I have packaged it in this branch: https://github.com/jtojnar/nixpkgs/tree/librem-phone

@alesya-h
Copy link
Member Author

@jtojnar thanks. ModemManager backend works for my hardware, so maybe another time.

@@ -12389,7 +12389,7 @@ let
export PERL_MB_OPT="--install_base=$out --prefix=$out"
substituteInPlace Po4aBuilder.pm --replace "\$self->install_sets(\$self->installdirs)->{'bindoc'}" "'$out/share/man/man1'"
'';
buildPhase = "perl Build.PL --install_base=$out; ./Build build";
buildPhase = "perl Build.PL --install_base=$out --install_path=\"sbin=$out/bin\" --install_path=\"lib=$out/lib/perl5/site_perl\"; ./Build build ";
Copy link
Contributor

Choose a reason for hiding this comment

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

Why a this needed?

Is not there something like python.sitePackages for Perl so the path does not need to be specified manually?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know what python.sitePackages is. Without this change the package was broken - whenever you try to run it's executables the app can't find it's own packages. I copied this buildPhase from another perl package and it fixed that issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

--install_path=\"lib=$out/${perl.libPrefix}\" seems to be enough. Also no need for adding the final space.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@jtojnar
Copy link
Contributor

jtojnar commented Oct 30, 2018

Thank you.

@jtojnar jtojnar merged commit 6d34e36 into NixOS:master Oct 30, 2018
@alesya-h alesya-h deleted the package-network-manager-gui branch November 5, 2018 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants