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

Skipper: init at 3.2.25.1701 #99233

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

@NicolasGuilloux
Copy link

@NicolasGuilloux NicolasGuilloux commented Oct 1, 2020

Motivation for this change

Skipper: init at 3.2.25.1701 (fixes partially #99232).

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.
@NicolasGuilloux
Copy link
Author

@NicolasGuilloux NicolasGuilloux commented Oct 1, 2020

TODO:

  • Understand why it tries to read into /var/log/installer/syslog
  • Test binary
  • Clean the git history
  • Checks that everything fits with the Contributing requirements

Since it is my first package, I would greatly appreciate some feedback :) Cheers guys !

@NicolasGuilloux NicolasGuilloux changed the title Add Skipper and Pulpo package Skipper: init at 3.2.25.1701 Oct 5, 2020
@NicolasGuilloux
Copy link
Author

@NicolasGuilloux NicolasGuilloux commented Oct 5, 2020

I cleaned everything. If anyone wants to test or merge it, it is good to go! ;)

I still have that annoying attempt to access to /var/log/installer/syslog without success, but it does not harm the stability of the software.

@NicolasGuilloux
Copy link
Author

@NicolasGuilloux NicolasGuilloux commented Oct 7, 2020

Do I have to ping somebody about this PR? I think the PR is ready to be merged.

maintainers/maintainer-list.nix Show resolved Hide resolved
pkgs/applications/editors/inventic/skipper.nix Outdated Show resolved Hide resolved
pkgs/applications/editors/inventic/skipper.nix Outdated Show resolved Hide resolved
pkgs/applications/editors/inventic/skipper.nix Outdated Show resolved Hide resolved
postgresql
qt5.qtbase
qt5.qtscript
stdenv.cc.cc.lib
Copy link
Member

@SuperSandro2000 SuperSandro2000 Nov 24, 2020

That looks strange to me and so far I only have seen it when doing cross compile.

pkgs/applications/editors/inventic/skipper.nix Outdated Show resolved Hide resolved
pkgs/applications/editors/inventic/skipper.nix Outdated Show resolved Hide resolved
'';

postFixup = ''
wrapQtApp $out/opt/Skipper
Copy link
Member

@SuperSandro2000 SuperSandro2000 Nov 24, 2020

Please avoid double wrapping.

Copy link
Author

@NicolasGuilloux NicolasGuilloux Mar 17, 2021

I tried to remote it, but when I start the package, it fails to launch with the following error:

##WARNING: Library libxcb-xinerama.so.0 is missing on your system. Please install it by using 'sudo apt-get install libxcb-xinerama0'
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.

[1]    28569 abort (core dumped)  skipper

With the postFixup, everything works well. Do you know why?

Copy link
Member

@SuperSandro2000 SuperSandro2000 Mar 18, 2021

wrapQtApp uses internally makeWrapper and we can pass the arguments it uses to the same wrapper that already sets LD_LIBRARY_PATH.

pkgs/applications/editors/inventic/skipper.nix Outdated Show resolved Hide resolved
pkgs/applications/editors/inventic/skipper.nix Outdated Show resolved Hide resolved
@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Nov 25, 2020

Result of nixpkgs-review pr 99233 run on x86_64-linux 1

1 package built:
  • skipper

@NicolasGuilloux
Copy link
Author

@NicolasGuilloux NicolasGuilloux commented Mar 17, 2021

Hi @SuperSandro2000

Sorry for the delay. I was pretty busy and I completely forgot to fix your feedbacks :(
Anyway, I fixed what you asked, except the postFixup. I don't know why it fails if I remove it. I though the hook added in the nativeBuildInputs would take care of that.

Anyway, about the package itself, I added a .desktop entry. Tell me if there is a better way of doing it.
I also updated the package to the latest version.

Cheers! :D

@@ -20944,6 +20944,8 @@ in

rhvoice = callPackage ../applications/audio/rhvoice { };

skipper = callPackage ../applications/editors/inventic/skipper.nix {};
Copy link
Member

@SuperSandro2000 SuperSandro2000 Mar 18, 2021

Suggested change
skipper = callPackage ../applications/editors/inventic/skipper.nix {};
skipper = callPackage ../applications/editors/inventic/skipper.nix { };

wrapQtApp $out/opt/Skipper
'';

meta = with stdenv.lib; {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Mar 18, 2021

Suggested change
meta = with stdenv.lib; {
meta = with lib; {

postFixup = ''
wrapQtApp $out/opt/Skipper
'';
Copy link
Member

@SuperSandro2000 SuperSandro2000 Mar 18, 2021

Suggested change
postFixup = ''
wrapQtApp $out/opt/Skipper
'';

'';

postFixup = ''
wrapQtApp $out/opt/Skipper
Copy link
Member

@SuperSandro2000 SuperSandro2000 Mar 18, 2021

wrapQtApp uses internally makeWrapper and we can pass the arguments it uses to the same wrapper that already sets LD_LIBRARY_PATH.

mv ./* $out/opt
makeWrapper $out/opt/Skipper $out/bin/skipper \
--prefix LD_LIBRARY_PATH : ${lib.makeLibraryPath runtimeDependencies}
Copy link
Member

@SuperSandro2000 SuperSandro2000 Mar 18, 2021

Suggested change
--prefix LD_LIBRARY_PATH : ${lib.makeLibraryPath runtimeDependencies}
--prefix LD_LIBRARY_PATH : ${lib.makeLibraryPath runtimeDependencies} \
''${qtWrapperArgs[@]}

@NicolasGuilloux
Copy link
Author

@NicolasGuilloux NicolasGuilloux commented Apr 5, 2021

I don't know if I can ping you @SuperSandro2000, sorry for the spam :(

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Apr 5, 2021

Do you need help with something? I am just a bit behind my github notifications and will catch up with them in the coming days.

, zlib }:

let
desktopFileTemplate = ./skipper.desktop;
Copy link
Member

@SuperSandro2000 SuperSandro2000 Apr 5, 2021

We have a function called makeDesktopItem which should be used instead of this extra file.

mkdir -p $out/opt
chmod +x ./Skipper
mv ./* $out/opt
Copy link
Member

@SuperSandro2000 SuperSandro2000 Apr 5, 2021

nixpkgs has not directory named opt in $out. Please move binaries to $out/bin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants