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

libappindicator: 12.10.0 -> 12.10.1+20.10.20200706.1 #93453

Merged
merged 1 commit into from Aug 25, 2020

Conversation

euank
Copy link
Member

@euank euank commented Jul 19, 2020

This moves libappindicator to use a different upstream source. Rather
than use the 8 year old (!) version displayed on its homepage
(https://launchpad.net/libappindicator), this switches us to the
maintained lp:libappindicator branch, browseable over here:
https://code.launchpad.net/~indicator-applet-developers/libappindicator/trunk.

This includes numerous fixes, remains updated, and matches what ubuntu
uses.

Due to a personal preference for git over bzr, I have the package using
ubuntu's git mirror of the package for the source rather than the bzr
repo where I think development actually takes place.

This also removes the no-python patch, because per revision 292
(https://bazaar.launchpad.net/~indicator-applet-developers/libappindicator/trunk/revision/292),
that has been dropped from upstream already, so the patch is no longer
needed.

The primary motivation behind this change is to fix a crash with
libappindicator (reported
https://bugs.launchpad.net/ubuntu/+source/libappindicator/+bug/1867996
and in various other places).

The relevant patch for that should be included in this version.

Motivation for this change

Fix crashes in libappindicator apps that have been fixed in the more-up-to-date upstream fork of libappindicator (maintained by canonical)

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.

Note: For "tested compilation of all packages", it did result in failures:

$ nixpkgs-review rev update-libappindicator
81 packages updated:
appimage-run audio-recorder autokey blueman caja-dropbox deltachat-electron devdocs-desktop discord discord-canary discord-ptb dropbox dropbox-cli elementary-greeter gitter gnome-control-center gromit-mpx gSpeech gxneur handbrake hubstaff indicator-application-gtk2 indicator-application-gtk3 irccloud joplin-desktop kazam keybase-gui ledger-live-desktop libappindicator-gtk2 (12.10.0 → 12.10.1+20.10.20200706.1) libappindicator (12.10.0 → 12.10.1+20.10.20200706.1) libappindicator-gtk3 (12.10.0 → 12.10.1+20.10.20200706.1) marktext-v0.16.2-binary mate-control-center mate-polkit meteo minetime minishift modem-manager-gui mullvad-vpn network-manager-applet notable obsidian pasystray Patchwork perl5.28.3-Gtk2-AppIndicator perl5.30.3-Gtk2-AppIndicator quodlibet-full quodlibet-xine-full radiotray-ng redshift redshift-plasma-applet redshift-wlr remmina runwayml safeeyes sc-controller shutter signal-desktop skypeforlinux skypeforlinux slack slack standardnotes station steam-run switchboard switchboard-plug-network Sylk syncthing-gtk syncthing-gtk syncthing-tray systrayhelper tusk-v0.23.0 udiskie uget uget-integrator ulauncher unityhub wingpanel wootility xflux-gui zulip


error: build of '/nix/store/3bx27pfs6410i1ali3lss3c8ch7cn834-env.drv' failed
22 packages failed to build:
Sylk appimage-run deltachat-electron devdocs-desktop irccloud joplin-desktop ledger-live-desktop marktext minetime notable obsidian quodlibet-full quodlibet-xine-full runwayml ssb-patchwork standardnotes station steam-run-native tusk unityhub wootility zulip

For those error packages, picking a random set to retry manually, I get that they're marked as broken due to a transitive dependency on a broken gst-plugins-base. For example:

nix-build -I . -E 'with import ./. {}; pkgs.zulip'
error: Package ‘gst-plugins-base-0.10.36’ in /home/esk/dev/nix/nixpkgs/pkgs/development/libraries/gstreamer/legacy/gst-plugins-base/default.nix:55 is marked as broken, refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowBroken = true; }
in configuration.nix to override this.

I didn't see a clean way to exaustively verify all of them were broken in that same way, but the ones I sampled were.

This moves libappindicator to use a different upstream source. Rather
than use the 8 year old (!) version displayed on its homepage
(https://launchpad.net/libappindicator), this switches us to the
maintained lp:libappindicator branch, browseable over here:
https://code.launchpad.net/~indicator-applet-developers/libappindicator/trunk.

This includes numerous fixes, remains updated, and matches what ubuntu
uses.

Due to a personal preference for git over bzr, I have the package using
ubuntu's git mirror of the package for the source rather than the bzr
repo where I _think_ development actually takes place.

This also removes the no-python patch, because per revision 292
(https://bazaar.launchpad.net/~indicator-applet-developers/libappindicator/trunk/revision/292),
that has been dropped from upstream already, so the patch is no longer
needed.

The primary motivation behind this change is to fix a crash with
libappindicator (reported
https://bugs.launchpad.net/ubuntu/+source/libappindicator/+bug/1867996
and in various other places).

The relevant patch for that should be included in this version.
@euank
Copy link
Member Author

euank commented Jul 19, 2020

cc @msteen since you're marked as the maintainer of this package, and may have a better idea if there are any other caveats I may need to be aware of about this libappindicator bump.

@JJJollyjim
Copy link
Member

This would resolve #96063 (as me and @goffrie found in that issue). Can we get this merged?

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

LGTM

failures are broken on target branch

https://github.com/NixOS/nixpkgs/pull/93453
3 packages failed to build:
quodlibet-full quodlibet-xine-full steam-run-native

74 packages built:
Sylk appimage-run audio-recorder autokey blueman deltachat-electron devdocs-desktop discord discord-canary discord-ptb dropbox dropbox-cli gitter gnome3.gnome-control-center networkmanagerapplet gromit-mpx gspeech gxneur handbrake hubstaff indicator-application-gtk2 indicator-application-gtk3 irccloud joplin-desktop kazam keybase-gui ledger-live-desktop libappindicator libappindicator-gtk2 marktext mate.caja-dropbox mate.mate-control-center mate.mate-polkit meteo minetime minishift modem-manager-gui mullvad-vpn notable obsidian pantheon.elementary-greeter pantheon.switchboard-plug-network pantheon.switchboard-with-plugs pantheon.wingpanel-with-indicators pasystray perl528Packages.Gtk2AppIndicator perl530Packages.Gtk2AppIndicator syncthing-gtk radiotray-ng redshift redshift-plasma-applet redshift-wlr remmina runwayml safeeyes sc-controller shutter signal-desktop skypeforlinux slack ssb-patchwork standardnotes station syncthing-tray systrayhelper tusk udiskie uget uget-integrator ulauncher unityhub wootility xflux-gui zulip

@jonringer jonringer merged commit 98e0716 into NixOS:master Aug 25, 2020
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