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: Fix #78803 #81475

Merged
merged 1 commit into from Mar 4, 2020
Merged

Conversation

worldofpeace
Copy link
Contributor

Add to gappsWrapperArgs in preFixupPhases.

Motivation for this change

Fix #78803 #75844

Test with packages in abe3475

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.

@jtojnar
Copy link
Contributor

jtojnar commented Mar 2, 2020

Possibly also fixes #57638?

@worldofpeace
Copy link
Contributor Author

Possibly also fixes #57638?

I tried to add a function to help with #57638, but I still saw duplicates (usually 2).

@jtojnar
Copy link
Contributor

jtojnar commented Mar 2, 2020

Hmm, we would probably need to emulate qtUnseenHostPath.

@worldofpeace
Copy link
Contributor Author

Hmm, we would probably need to emulate qtUnseenHostPath.

I tried this exactly, at least I think I did.

@jtojnar
Copy link
Contributor

jtojnar commented Mar 2, 2020

Anyway, using preFixupPhases looks fine to me for #78803 within the limitations of the current builder.

Not sure why are you removing the prefix check, though.

@worldofpeace
Copy link
Contributor Author

worldofpeace commented Mar 3, 2020

@jtojnar Should I replace $prefix with ${prefix:?} throughout?
I wasn't entirely sure why it wasn't done everywhere.

@jtojnar
Copy link
Contributor

jtojnar commented Mar 3, 2020

Only the first use should include :? since if the variable does not exist, the execution will stop there.

@worldofpeace
Copy link
Contributor Author

Only the first use should include :? since if the variable does not exist, the execution will stop there.

Fixed that.

@worldofpeace worldofpeace changed the base branch from master to staging March 4, 2020 00:31
Add to gappsWrapperArgs in preFixupPhases.
@worldofpeace
Copy link
Contributor Author

We're now on staging.

@worldofpeace worldofpeace merged commit add141c into NixOS:staging Mar 4, 2020
@worldofpeace worldofpeace deleted the fix-wrapgapps branch March 4, 2020 00:33
@worldofpeace
Copy link
Contributor Author

backported to 20.03 in e8e569c (staging-20.03)

jtojnar added a commit to jtojnar/nixpkgs that referenced this pull request Mar 4, 2020
Since we split wrapGAppsHook and move its variable initialization to preFixupPhases in NixOS#81475, it was getting run before glibPreFixupPhase which sets GSETTINGS_SCHEMAS_PATH variable gappsWrapperArgsHook depends on. Let's introduce this ugly hack to ensure glibPreFixupPhase will run before gappsWrapperArgsHook.
worldofpeace pushed a commit that referenced this pull request Mar 4, 2020
Since we split wrapGAppsHook and move its variable initialization to preFixupPhases in #81475, it was getting run before glibPreFixupPhase which sets GSETTINGS_SCHEMAS_PATH variable gappsWrapperArgsHook depends on. Let's introduce this ugly hack to ensure glibPreFixupPhase will run before gappsWrapperArgsHook.

(cherry picked from commit 8e4f502)
worldofpeace added a commit to worldofpeace/nixpkgs that referenced this pull request Mar 24, 2020
Since NixOS#81475 this caused the wrapper to be empty of entries from
wrapGAppsHook because the wrapGAppsHook function doesn't add
them anymore, and was moved to gappsWrapperArgsHook. Instead
of just running that in postBuild it's more future proof to make this
use stdenv.mkDerivation because we want to mess around with the
generic builder.
worldofpeace added a commit to worldofpeace/nixpkgs that referenced this pull request Mar 24, 2020
Since NixOS#81475 this caused the wrapper to be empty of entries from
wrapGAppsHook because the wrapGAppsHook function doesn't add
them anymore, and was moved to gappsWrapperArgsHook. Instead
of just running that in postBuild it's more future proof to make this
use stdenv.mkDerivation because we want to mess around with the
generic builder.
worldofpeace added a commit that referenced this pull request Mar 24, 2020
Since #81475 this caused the wrapper to be empty of entries from
wrapGAppsHook because the wrapGAppsHook function doesn't add
them anymore, and was moved to gappsWrapperArgsHook. Instead
of just running that in postBuild it's more future proof to make this
use stdenv.mkDerivation because we want to mess around with the
generic builder.

(cherry picked from commit db41c78)
worldofpeace added a commit that referenced this pull request Mar 24, 2020
Since #81475 this caused the wrapper to be empty of entries from
wrapGAppsHook because the wrapGAppsHook function doesn't add
them anymore, and was moved to gappsWrapperArgsHook. Instead
of just running that in postBuild it's more future proof to make this
use stdenv.mkDerivation because we want to mess around with the
generic builder.

(cherry picked from commit a9e7e93)
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.

wrapGAppsHook: make double wrapping workaround work again
2 participants