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

calibre: wrap manually #66483

Merged
merged 1 commit into from Aug 12, 2019
Merged

Conversation

worldofpeace
Copy link
Contributor

Also move wrapGAppsHook to nativeBuildInputs

Motivation for this change
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 nix-review --run "nix-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.
Notify maintainers

cc @

Also move wrapGAppsHook to nativeBuildInputs
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

nix-review passes on NixOS
diff LGTM (although not super familiar with Qt application packaging)
binaries seem to work
leaf package

I did notice that some of the applications don't get passed the right executable name (but the wrapped name), although this doesn't affect how they work:

[nix-shell:/home/jon/.cache/nix-review/pr-66483]$ ./results/calibre/bin/web2disk
Usage: .web2disk-wrapped URL

Where URL is for example https://google.com

Whenever you pass arguments to .web2disk-wrapped that have spaces in them, enclose the arguments in quotation marks. For example: "/some path/with spaces"

Options:
  --version             show program's version number and exit
...

@worldofpeace
Copy link
Contributor Author

Yeah that's an unfortunate side-effect of how wrappers work.

@lilyball
Copy link
Member

The executable name is because wrapProgram renames the original program from foo to .foo-wrapped. I wonder if it's worth changing wrapProgram to instead change it to something like .wrapped/foo, so the basename doesn't change. Conflicts would still have to be handled, right now they append extra _s to the name, so we could still end up with .wrapped/foo_, though I suppose .wrapped_/foo is also a possibility (though it might be weird to see).

@jonringer
Copy link
Contributor

In a lot of wrapped programs, it displays correctly. I just wonder if the mechanism is something like file in python where it doesn't reference $0

@lilyball
Copy link
Member

wrapProgram will pass the original name as argv 0 to the program, but programs that use the platform-specific mechanism of reading the running executable name end up displaying the wrong name.

@worldofpeace worldofpeace merged commit 8a139bf into NixOS:master Aug 12, 2019
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

3 participants