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

wrapGAppsHook: add package’s own modules to GIO_EXTRA_MODULES #57603

Merged
merged 1 commit into from Mar 15, 2019

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Mar 13, 2019

Motivation for this change

When a package provides both executables and gio modules, it is quite
probable the executables will need those modules. wrapGAppsHook wraps
executables with GIO_EXTRA_MODULES picked up from dependencies
but forgets about the package being built. Let’s add to consideration.

Closes: #50254

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

When a package provides both executables and gio modules, it is quite
probable the executables will need those modules. wrapGAppsHook wraps
executables with GIO_EXTRA_MODULES picked up from dependencies
but forgets about the package being built. Let’s add to consideration.

Closes: NixOS#50254
@jtojnar
Copy link
Contributor Author

jtojnar commented Mar 13, 2019

$ cat $(nix-build -A gvfs)/libexec/gvfsd-fuse | rg GIO_EXTRA_MODULES
export GIO_EXTRA_MODULES='/nix/store/hvdwscbv3w6f5wr7liri89g473v6r6v9-dconf-0.30.1-lib/lib/gio/modules'${GIO_EXTRA_MODULES:+':'}$GIO_EXTRA_MODULES
export GIO_EXTRA_MODULES='/nix/store/hvdwscbv3w6f5wr7liri89g473v6r6v9-dconf-0.30.1-lib/lib/gio/modules'${GIO_EXTRA_MODULES:+':'}$GIO_EXTRA_MODULES
export GIO_EXTRA_MODULES='/nix/store/27yrqjjig6qz3hbkzpkwivmzakas5lw5-gvfs-1.38.1/lib/gio/modules'${GIO_EXTRA_MODULES:+':'}$GIO_EXTRA_MODULES

Copy link
Contributor

@hedning hedning left a comment

Choose a reason for hiding this comment

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

Looks good to me. A bit weird that eg. dconf is written twice, but that's another issue as far as I understand.

@jtojnar
Copy link
Contributor Author

jtojnar commented Mar 14, 2019

Yeah, our setup hooks are not well written 😁

However, a dependency specified twice will have its setup hook run twice, and that could easily change the build environment (though a well-written setup hook will therefore strive to be idempotent so this is in fact not observable).

https://nixos.org/nixpkgs/manual/#ssec-setup-hooks

Edit: split off an issue #57638

@worldofpeace
Copy link
Contributor

@jtojnar
Copy link
Contributor Author

jtojnar commented Mar 14, 2019

I would remove it but there are still many unwrapped apps (QT). Maybe we could add an gnome3.enableImpureClusterfuck option and disable it by default.

@worldofpeace
Copy link
Contributor

worldofpeace commented Mar 14, 2019

I would remove it but there are still many unwrapped apps (QT).

Aren't wrappers for QT apps incoming? So maybe removing it completely isn't far off.

should open an issue to streamline us dropping it?

Maybe we could add an gnome3.enableImpureClusterfuck option and disable it by default.

Well that does get the point across just fine 👍
But maybe it's not in our best interest to start hiding things, so when the time is right.

@jtojnar
Copy link
Contributor Author

jtojnar commented Mar 14, 2019

Aren't wrappers for QT apps incoming?

That still seems to be a far off. And they do not include GNOME hooks, IIRC.

@jtojnar jtojnar merged commit 1e0661c into NixOS:staging Mar 15, 2019
@jtojnar jtojnar deleted the wrap-gapps-hook-self-gio branch March 15, 2019 13:26
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