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.

Sign up for GitHub

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

Jump to bottom

wrapQtAppsHook: wrap binaries in all outputs #92710

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from
Open

wrapQtAppsHook: wrap binaries in all outputs #92710

wants to merge 1 commit into from

Conversation

B4dM4n
Copy link
Contributor

@B4dM4n B4dM4n commented Jul 8, 2020

Motivation for this change

wrapQtAppsHook currently only wraps the binaries from the first output. All other outputs are skipped.

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 some 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.

@B4dM4n B4dM4n requested a review from ttuegel as a code owner July 8, 2020 12:00
@ttuegel
Copy link
Member

ttuegel commented Jul 13, 2020

wrapQtAppsHook only targets the first output by design. What behavior are you seeing that this fixes?

@B4dM4n
Copy link
Contributor Author

B4dM4n commented Jul 13, 2020

I was working on #85306, because I wanted to use qdbusviewer from qt5.qttools (qttools.nix).

Adding nativeBuildInputs = [ wrapQtAppsHook ] didn't work, because:

  1. The bin output is not the first output
  2. The binaries are installed in multiple outputs

This took me a while to figure out and was a bit surprising.

Allowing wrapQtAppsHook to run once for each output seemed to help in both cases and should not impact any package.

If only running for the first output the expected behavior, it might be be worth a comment in wrap-qt-apps-hook.sh or the nixpkgs manual, instead of this PR.

@solson
Copy link
Member

solson commented Oct 1, 2020

qdbusviewer is still broken today. Can we move towards merging this or figuring out the correct fix for qdbusviewer and any similarly broken apps?

For reference, qdbusviewer fails like this:

$ qdbusviewer
qt.qpa.plugin: Could not load the Qt platform plugin "xcb" in "" even though it was found.
This application failed to start because no Qt platform plugin could be initialized. Reinstalling the application may fix this problem.

Available platform plugins are: wayland-egl, wayland, wayland-xcomposite-egl, wayland-xcomposite-glx, eglfs, linuxfb, minimal, minimalegl, offscreen, vnc, xcb, wayland-org.kde.kwin.qpa.

Aborted (core dumped)

@sikmir
Copy link
Member

sikmir commented Oct 13, 2020

assistant, designer, linguist, pixeltool, qcollectiongenerator, qhelpgenerator are broken as well. Is there any chance to land this PR?

SuperSandro2000
SuperSandro2000 reviewed Nov 27, 2020
View reviewed changes
pkgs/development/libraries/qt-5/hooks/wrap-qt-apps-hook.sh
Comment on lines +22 to +32
for prefix in "${qtFixupPrefixSeen[@]}"
do
if [ "${prefix:?}" == "$1" ]
then
return 1
fi
done

qtFixupPrefixSeen+=("$1")
return 0
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for prefix in "${qtFixupPrefixSeen[@]}"
do
if [ "${prefix:?}" == "$1" ]
then
return 1
fi
done
qtFixupPrefixSeen+=("$1")
return 0
}
for prefix in "${qtFixupPrefixSeen[@]}": do
if [ "${prefix:?}" == "$1" ]: then
return 1
fi
done
qtFixupPrefixSeen+=("$1")
return 0
}

@SuperSandro2000
Copy link
Member

@ttuegel do you think we can merge this?

@stale
Copy link

stale bot commented Jun 3, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 3, 2021
@sikmir
Copy link
Member

sikmir commented Jun 3, 2021

Problem still exists.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 3, 2021
@solson
Copy link
Member

solson commented Jun 3, 2021

qdbusviewer is still broken today.

assistant, designer, linguist, pixeltool, qcollectiongenerator, qhelpgenerator are broken as well.

qdbusviewer and everything in the quoted list work for me now. Did something change in the interim?

@sikmir
Copy link
Member

sikmir commented Jun 4, 2021

qdbusviewer is still broken today.

assistant, designer, linguist, pixeltool, qcollectiongenerator, qhelpgenerator are broken as well.

qdbusviewer and everything in the quoted list work for me now. Did something change in the interim?

Doesn't work for me:

$ result-bin/bin/qdbusviewer 
qt.qpa.plugin: Could not find the Qt platform plugin "xcb" in ""
This application failed to start because no Qt platform plugin could be initialized. Reinstalling the application may fix this problem.

fish: Job 1, 'result-bin/bin/qdbusviewer' terminated by signal SIGABRT (Abort)

@solson
Copy link
Member

solson commented Jun 4, 2021

Oh, maybe it's working for me because I have qt5.qttools.dev in my environment.systemPackages, which I believe I might have originally added as a workaround. I'll try removing that later and see if I can reproduce the problem.

@stale
Copy link

stale bot commented Jan 9, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 9, 2022
@sikmir
Copy link
Member

sikmir commented Jan 9, 2022

Up.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 9, 2022
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@ttuegel ttuegel Awaiting requested review from ttuegel ttuegel is a code owner

Assignees
No one assigned
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: qt/kde 10.rebuild-darwin: 101-500 10.rebuild-linux: 501+ 10.rebuild-linux: 1001-2500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@B4dM4n @ttuegel @solson @sikmir @SuperSandro2000