Skip to content

Comments

treewide: remove issue #56943 workarounds#215715

Merged
Artturin merged 8 commits intoNixOS:stagingfrom
Artturin:removegirworkarounds2
Feb 18, 2023
Merged

treewide: remove issue #56943 workarounds#215715
Artturin merged 8 commits intoNixOS:stagingfrom
Artturin:removegirworkarounds2

Conversation

@Artturin
Copy link
Member

checked with diffoscope+difflog and launching the programs

i have fixed that issue in my other gir PRs###### Description of changes

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: xfce The Xfce Desktop Environment 8.has: documentation This PR adds or changes documentation labels Feb 10, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Feb 10, 2023
@Artturin Artturin force-pushed the removegirworkarounds2 branch 4 times, most recently from e7be096 to fc9640a Compare February 17, 2023 18:24
checked with diffoscope+difflog and launching the programs

i have fixed that issue in my other gir PRs
and fix double wrapping

it still fails to launch though but its not a regression

$ ./result/bin/dogtail-logout
Traceback (most recent call last):
  File "/nix/store/az8mw2pmhbwlm08176wd62hn0hfzwv8s-python3.10-dogtail-0.9.11/bin/.dogtail-logout-wrapped", line 8, in <module>
    from dogtail.tree import *
  File "/nix/store/az8mw2pmhbwlm08176wd62hn0hfzwv8s-python3.10-dogtail-0.9.11/lib/python3.10/site-packages/dogtail/tree.py", line 5, in <module>
    from dogtail import predicate
  File "/nix/store/az8mw2pmhbwlm08176wd62hn0hfzwv8s-python3.10-dogtail-0.9.11/lib/python3.10/site-packages/dogtail/predicate.py", line 6, in <module>
    from gi.repository import GLib
ModuleNotFoundError: No module named 'gi'
pygobject3 has to be propagated for the import line in the python file
in bin

$ ./result/bin/curtail
Traceback (most recent call last):
  File "/nix/store/fqjld9nchfwqd9x1pywmjfdsqrk4lxds-curtail-1.3.1/bin/.curtail-wrapped", line 37, in <module>
    import gi
ModuleNotFoundError: No module named 'gi'
@Artturin Artturin force-pushed the removegirworkarounds2 branch from fc9640a to ab3ba31 Compare February 17, 2023 18:26
Copy link
Member

@jtojnar jtojnar 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, thanks.

@jtojnar
Copy link
Member

jtojnar commented Feb 17, 2023

@Artturin
Copy link
Member Author

Actually, we might want to add some tests to master/pkgs/build-support/setup-hooks/wrap-gapps-hook/default.nix

already did for gdk-pixbuf in https://github.com/NixOS/nixpkgs/pull/208537/files#diff-fa727d655e0d18c0ba8bb953613ed08fb7853f04b226341ee2fe6b17957cf523R74

for gobject-introspection there's already typelib-x tests written by you

@jtojnar
Copy link
Member

jtojnar commented Feb 17, 2023

Those do not have strictDeps = true, though?

@Artturin
Copy link
Member Author

Artturin commented Feb 17, 2023

Those do not have strictDeps = true, though?

NixOS/nixpkgs@8be7ab6 (#208537)

run tests with strictDeps

the ones using testLib.runTest are using strictDeps

EDIT disregard, they're testing the package which was not built with strictDeps, i'll add strictDeps

luckily the lib output does not contain binaries therefore its safe to
remove it from propagatedBuildInputs
'nix build -f . "wrapGAppsHook.tests"' pass
@Artturin
Copy link
Member Author

alright strictDeps enabled and dconf.lib moved to depsTargetTargetPropagated

@ofborg ofborg bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. labels Feb 17, 2023
@Artturin
Copy link
Member Author

nix build -f . "pkgsCross.aarch64-multiplatform.wrapGAppsHook.tests" --keep-going passes other than

basic-contains-gdk-pixbuf> FAIL: basic-contains-gdk-pixbuf: The file “/nix/store/faxhx93d6qvizzwnw2g548pzj5sxmrwn-basic-aarch64-unknown-linux-gnu/bin/foo” should include a line containing “GDK_PIXBUF_MODULE_FILE” that also contains “/nix/store/sir0dnzpqpfl1fmx2lpv2lwjcvy17dzc-librsvg-aarch64-unknown-linux-gnu-2.55.1/lib/gdk-pixbuf-2.0/2.10.0/loaders.cache”.

due to

# Not generated when cross compiling.
postInstall = lib.optionalString (stdenv.hostPlatform == stdenv.buildPlatform) ''
# Merge gdkpixbuf and librsvg loaders
cat ${lib.getLib gdk-pixbuf}/lib/gdk-pixbuf-2.0/2.10.0/loaders.cache $GDK_PIXBUF/loaders.cache > $GDK_PIXBUF/loaders.cache.tmp
mv $GDK_PIXBUF/loaders.cache.tmp $GDK_PIXBUF/loaders.cache
'';

@Artturin Artturin requested a review from jtojnar February 17, 2023 23:03
@jtojnar
Copy link
Member

jtojnar commented Feb 17, 2023

Would not it make more sense to keep the test failing until it is properly fixed (e.g. using emulator to generate the loaders), rather than mask the broken runtime behaviour?

@ofborg ofborg bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. labels Feb 17, 2023
@Artturin Artturin force-pushed the removegirworkarounds2 branch from e04770b to d5b6b62 Compare February 18, 2023 05:37
@Artturin
Copy link
Member Author

Would not it make more sense to keep the test failing until it is properly fixed (e.g. using emulator to generate the loaders), rather than mask the broken runtime behaviour?

fixed it

@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. and removed 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. labels Feb 18, 2023
Copy link
Member

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

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

Great job.

@Artturin Artturin merged commit cdcca1c into NixOS:staging Feb 18, 2023
@Artturin Artturin deleted the removegirworkarounds2 branch February 18, 2023 18:12

# Not generated when cross compiling.
postInstall = lib.optionalString (stdenv.hostPlatform == stdenv.buildPlatform) ''
postInstall = lib.optionalString (stdenv.hostPlatform.emulatorAvailable buildPackages) ''
Copy link
Member

@lilyinstarlight lilyinstarlight Jul 29, 2023

Choose a reason for hiding this comment

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

After this PR, I also need to add this diff for librsvg to build with cross again:

diff --git a/pkgs/development/libraries/librsvg/default.nix b/pkgs/development/libraries/librsvg/default.nix
index 9974826573a..87ddcd28403 100644
--- a/pkgs/development/libraries/librsvg/default.nix
+++ b/pkgs/development/libraries/librsvg/default.nix
@@ -147,11 +147,11 @@ stdenv.mkDerivation (finalAttrs: {
     mv $GDK_PIXBUF/loaders.cache.tmp $GDK_PIXBUF/loaders.cache
 
     mkdir -p "$out/share/bash-completion/completions/"
-    $out/bin/rsvg-convert --completion bash > "$out/share/bash-completion/completions/rsvg-convert"
+    ${stdenv.hostPlatform.emulator buildPackages} $out/bin/rsvg-convert --completion bash > "$out/share/bash-completion/completions/rsvg-convert"
     mkdir -p "$out/share/zsh/site-functions/"
-    $out/bin/rsvg-convert --completion zsh > "$out/share/zsh/site-functions/_rsvg-convert"
+    ${stdenv.hostPlatform.emulator buildPackages} $out/bin/rsvg-convert --completion zsh > "$out/share/zsh/site-functions/_rsvg-convert"
     mkdir -p "$out/share/fish/vendor_completions.d/"
-    $out/bin/rsvg-convert --completion fish > "$out/share/fish/vendor_completions.d/rsvg-convert.fish"
+    ${stdenv.hostPlatform.emulator buildPackages} $out/bin/rsvg-convert --completion fish > "$out/share/fish/vendor_completions.d/rsvg-convert.fish"
   '';
 
   postFixup = lib.optionalString withIntrospection ''

Is this the correct approach for fixing that?

If so, I'll open a PR to fix librsvg cross (I'll probably use a let rsvg-convert = "${stdenv.hostPlatform.emulator buildPackages} $out/bin/rsvg-convert" in ... to shorten it a little, though)

Copy link
Member

Choose a reason for hiding this comment

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

We already have that on staging: #244602

Copy link
Member

Choose a reason for hiding this comment

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

Ah, thanks! I did a quick search but missed that. Sorry for the noise 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: xfce The Xfce Desktop Environment 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants