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

gnumeric: fix wrapping #54638

Merged
merged 1 commit into from Jan 27, 2019
Merged

gnumeric: fix wrapping #54638

merged 1 commit into from Jan 27, 2019

Conversation

tollb
Copy link
Contributor

@tollb tollb commented Jan 26, 2019

Incorporate wrapGAppsHook so that all gnumeric binaries are wrapped,
following the convention used by many gnome applications.

This addresses two issues:

  1. The packaged ssconvert, ssdiff, ssgrep, and ssindex executables
    in bin are not currently wrapped so some expected environment
    variables including XDG_DATA_DIRS and GIO_EXTRA_MODULES are not
    set. The result is many warnings on stderr when running these
    commands, e.g.

    CRITICAL **:...go_conf_add_monitor: assertion 'node || key' failed
    CRITICAL **:...go_conf_get_node: assertion 'parent || key' failed
    WARNING **:...unknown GOConfMonitor id.

  2. None of the binaries, including gnumeric, currently wrap the
    environment variable GDK_PIXBUF_MODULE_FILE. This can cause
    segfaults if an incompatible GDK_PIXBUF_MODULE_FILE is already set
    in the environment (e.g. by plasma5). This could be encountered
    running a nixos pre-19.03 gnumeric binary from a nixos 18.09 KDE
    session.

NOTE 1: This change could potentially impact the darwin package as it
removes an explicit isDarwin check related to GIO_EXTRA_MODULES. The
Darwin case is hopefully handled correctly by conditional code in
wrapGAppsHook. Testing required.

NOTE 2: As with other packages wrapped by wrapGAppsHook, the wrapping
introduces an extra level of wrapping when it encounters a soft link
to an executable in bin. In this package, there is a soft link
from gnumeric to gnumeric-1.12.44. After wrapping with wrapGAppsHook,
the execution flow for gnumeric looks like this:

gnumeric execs .gnumeric-wrapped which is a link to
gnumeric-1.12.44 that then execs .gnumeric-1.12.44-wrapped

This may not be optimal, but seems harmless, and could be fixed by
a future update to wrapGAppsHook.

  • 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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Incorporate wrapGAppsHook so that all gnumeric binaries are wrapped,
following the convention used by many gnome applications.

This addresses two issues:

1.  The packaged ssconvert, ssdiff, ssgrep, and ssindex executables
    in bin are not currently wrapped so some expected environment
    variables including XDG_DATA_DIRS and GIO_EXTRA_MODULES are not
    set. The result is many warnings on stderr when running these
    commands, e.g.

    ==================================================================
    CRITICAL **:...go_conf_add_monitor: assertion 'node || key' failed
    CRITICAL **:...go_conf_get_node: assertion 'parent || key' failed
    WARNING **:...unknown GOConfMonitor id.
    ==================================================================

2.  None of the binaries, including gnumeric, currently wrap the
    environment variable GDK_PIXBUF_MODULE_FILE. This can cause
    segfaults if an incompatible GDK_PIXBUF_MODULE_FILE is already set
    in the environment (e.g. by plasma5). This could be encountered
    running a nixos pre-19.03 gnumeric binary from a nixos 18.09 KDE
    session.
@bgamari
Copy link
Contributor

bgamari commented Jan 26, 2019

I wonder if this is why gnumeric's toolbars are all blank on my machine.

@tollb
Copy link
Contributor Author

tollb commented Jan 26, 2019

I wonder if this is why gnumeric's toolbars are all blank on my machine.

I didn't observe an issue with blank toolbars on my system, before or after the patch, so it may be unrelated.

Copy link
Member

@vcunat vcunat left a comment

Choose a reason for hiding this comment

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

Seems good, thanks!

Darwin: the hook has a conditional code for dconf that looks "equivalent", and in the past month the build of gnumeric has been blocked by a failing dependency anyway.

@vcunat vcunat merged commit 8596144 into NixOS:master Jan 27, 2019
vcunat added a commit that referenced this pull request Jan 27, 2019
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

5 participants