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

nicotine-plus: 1.4.1 -> unstable-2020-05-24 #86683

Open
wants to merge 3 commits into
base: master
from

Conversation

@emilazy
Copy link
Member

emilazy commented May 3, 2020

Motivation for this change

Includes port to Python 3 and GTK+ 3/PyGObject; see Nicotine-Plus/nicotine-plus#106 for details.

This unstable version identifies itself as 1.4.3, and is planned to be released as 2.0.0 after it has had more fixes and testing through the official upstream PPA. The codebase is old and flaky to begin with, and there have been several unrelated bug fixes in the years since the last stable release, so I think it's worth updating to be able to use non-obsolescent versions of the core dependencies, and have nixos-unstable users help shake out any remaining issues with the port. I verified that searching and downloading seem to work fine.

Drop the rename of the executable to nicotine-plus, as the test depends on the executable name and there doesn't seem to be any strong reason to diverge from upstream here (the original Nicotine is long dead). This is technically a breaking change, so it might be best to include a symlink at $out/bin/nicotine-plus temporarily.

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
    • 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.
@emilazy emilazy marked this pull request as draft May 4, 2020
@emilazy
Copy link
Member Author

emilazy commented May 4, 2020

@emilazy emilazy force-pushed the emilazy:nicotine-plus-1.4.3 branch from bb5e86b to d97fee4 May 18, 2020
@emilazy emilazy changed the title nicotine-plus: 1.4.1 -> 1.4.3 nicotine-plus: 1.4.1 -> unstable-2020-05-18 May 18, 2020
@emilazy emilazy force-pushed the emilazy:nicotine-plus-1.4.3 branch from d97fee4 to 4345570 May 18, 2020
@emilazy
Copy link
Member Author

emilazy commented May 18, 2020

Updated; the integration tests now run successfully. Marking as ready for review per Nicotine-Plus/nicotine-plus#99 (comment) :)

@emilazy
Copy link
Member Author

emilazy commented May 18, 2020

@GrahamcOfBorg build nicotine-plus

@emilazy emilazy force-pushed the emilazy:nicotine-plus-1.4.3 branch from 6bdf215 to 523cee1 May 18, 2020
@emilazy
Copy link
Member Author

emilazy commented May 18, 2020

Simplified with strictDeps = false at the suggestion of @jtojnar.

@Ericson2314 I guess with gobject-introspection, Python, and checkPhase this is already a mess for cross-compilation, but disabling strictDeps makes me a little anxious; is there a better way to get setup hooks running for (install)CheckPhase? Previously none of the setup hooks for gdk-pixbuf and similar were running.

@ofborg ofborg bot requested a review from klntsky May 18, 2020
@emilazy emilazy force-pushed the emilazy:nicotine-plus-1.4.3 branch 2 times, most recently from 608034a to 25681b4 May 18, 2020
@emilazy
Copy link
Member Author

emilazy commented May 18, 2020

Bumped upstream commit; it now uses the gobject-introspection libnotify bindings.

@emilazy emilazy force-pushed the emilazy:nicotine-plus-1.4.3 branch from 25681b4 to a1d1998 May 18, 2020
@emilazy
Copy link
Member Author

emilazy commented May 18, 2020

Pushed with some cleanups and optimistically enabling xvfb_run on Darwin in the hopes that that will make the build and tests work (xorg.xorgserver is supported there, after all). Let's see what ofborg thinks of it.

@emilazy emilazy requested review from FRidh and jonringer as code owners May 18, 2020
@ofborg ofborg bot added the 6.topic: python label May 19, 2020
@ofborg ofborg bot requested a review from peterhoeg May 19, 2020
@ofborg ofborg bot removed the 10.rebuild-linux: 1 label May 19, 2020
@emilazy emilazy force-pushed the emilazy:nicotine-plus-1.4.3 branch from 8c33432 to 2907239 May 19, 2020
utillinux is used only for mcookie, which can be replaced with `openssl
rand -hex 16`, per https://bugs.freedesktop.org/show_bug.cgi?id=7052.
This should hopefully fix the build on Darwin.
@emilazy emilazy force-pushed the emilazy:nicotine-plus-1.4.3 branch 2 times, most recently from f933d80 to b8834c6 May 19, 2020
@emilazy
Copy link
Member Author

emilazy commented May 19, 2020

This now builds on Darwin, though I can't test it. The xvfb_run portability change is no longer required for that, as we can't run the integration tests anyway due to gtk3 not supporting X11 by default on Darwin, so I can drop it if it's undesired.

@emilazy emilazy force-pushed the emilazy:nicotine-plus-1.4.3 branch from b8834c6 to 458e515 May 19, 2020
@emilazy emilazy changed the title nicotine-plus: 1.4.1 -> unstable-2020-05-18 nicotine-plus: 1.4.1 -> unstable-2020-05-19 May 19, 2020
@emilazy
Copy link
Member Author

emilazy commented May 19, 2020

Updated with new upstream commits, some licensing pedantry, and a fix to the installation code that makes the desktop entry and manual page work.

There's now an exception being thrown in the integration tests (that somehow doesn't make them fail?) that I don't fully understand and should investigate further at some point:

nicotine-plus-unstable> Run nicotine as shell command                                         Traceback (most recent call last):
nicotine-plus-unstable> File "/nix/store/gpl8jbk7r4p9m7b9jmk5s7jym39p5cy2-nicotine-plus-unstable-2020-05-19/bin/.nicotine-wrapped", line 277, in <module>
nicotine-plus-unstable> run()
nicotine-plus-unstable> File "/nix/store/gpl8jbk7r4p9m7b9jmk5s7jym39p5cy2-nicotine-plus-unstable-2020-05-19/bin/.nicotine-wrapped", line 253, in run
nicotine-plus-unstable> app = frame.MainApp(data_dir, config, plugins, trayicon, hidden, bindip, port)
nicotine-plus-unstable> File "/nix/store/gpl8jbk7r4p9m7b9jmk5s7jym39p5cy2-nicotine-plus-unstable-2020-05-19/lib/python3.7/site-packages/pynicotine/gtkgui/frame.py", line 3730, in __init__
nicotine-plus-unstable> self.frame = NicotineFrame(data_dir, config, plugins, trayicon, start_hidden, bindip, port)
nicotine-plus-unstable> File "/nix/store/gpl8jbk7r4p9m7b9jmk5s7jym39p5cy2-nicotine-plus-unstable-2020-05-19/lib/python3.7/site-packages/pynicotine/gtkgui/frame.py", line 517, in __init__
nicotine-plus-unstable> self.downloads = Downloads(self)
nicotine-plus-unstable> File "/nix/store/gpl8jbk7r4p9m7b9jmk5s7jym39p5cy2-nicotine-plus-unstable-2020-05-19/lib/python3.7/site-packages/pynicotine/gtkgui/downloads.py", line 100, in __init__
nicotine-plus-unstable> cols[i].set_visible(self.frame.np.config.sections["columns"]["downloads_columns"][i])
nicotine-plus-unstable> IndexError: list index out of range

But it seems to work fine outside the build.

@mathiascode
Copy link

mathiascode commented May 19, 2020

@emilazy Some column counts were outdated in the config file. Nicotine-Plus/nicotine-plus#134 should take care of the issue.

@emilazy emilazy force-pushed the emilazy:nicotine-plus-1.4.3 branch 3 times, most recently from 1eeb74d to 7596ba7 May 19, 2020
@emilazy emilazy force-pushed the emilazy:nicotine-plus-1.4.3 branch from 7596ba7 to fe8ca5f May 23, 2020
Includes port to Python 3 and GTK+ 3/PyGObject; see
Nicotine-Plus/nicotine-plus#106 for details.

This unstable version identifies itself as 1.4.3, and is planned to be
released as 2.0.0 after it has had more fixes and testing through the
official upstream PPA. The codebase is old and flaky to begin with,
and there have been several unrelated bug fixes in the years since
the last stable release, so I think it's worth updating to be able
to use non-obsolescent versions of the core dependencies, and have
nixos-unstable users help shake out any remaining issues with the port.
I verified that searching and downloading seem to work fine.

Drop the rename of the executable to `nicotine-plus`, as the test
depends on the executable name and there doesn't seem to be any strong
reason to diverge from upstream here (the original Nicotine is long
dead). This is technically a breaking change, so it might be best to
include a symlink at `$out/bin/nicotine-plus` temporarily.
@emilazy emilazy force-pushed the emilazy:nicotine-plus-1.4.3 branch from fe8ca5f to d4940ee May 24, 2020
@emilazy emilazy changed the title nicotine-plus: 1.4.1 -> unstable-2020-05-19 nicotine-plus: 1.4.1 -> unstable-2020-05-24 May 24, 2020
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

3 participants
You can’t perform that action at this time.