Skip to content

cups-pdf(-to-pdf): init#182360

Merged
RaitoBezarius merged 4 commits intoNixOS:masterfrom
Yarny0:cups-pdf
Dec 28, 2022
Merged

cups-pdf(-to-pdf): init#182360
RaitoBezarius merged 4 commits intoNixOS:masterfrom
Yarny0:cups-pdf

Conversation

@Yarny0
Copy link
Contributor

@Yarny0 Yarny0 commented Jul 21, 2022

Description of changes

Implements the package request in #173745

cups-pdf is a cups printer backend that turns print jobs into pdf files.

  • This does not package cups-pdf from https://www.cups-pdf.de/, but a fork cups-pdf-to-pdf from https://github.com/alexivkin/CUPS-PDF-to-PDF . It claims to try harder to preserve searchable text in pdf files where the original cups-pdf rasterizes pdf files producing large pdf files with embedded images.

    https://github.com/alexivkin/CUPS-PDF-to-PDF/blob/c14428c2ca8e95371daad7db6d11c84046b1a2d4/README.md
    https://bugs.launchpad.net/ubuntu/+source/cups-pdf/+bug/366949

  • A NixOS module is included that, if enabled, installs a cups-pdf printer backend and a cups queue for that printer so that one can print to pdf without further ado. The module is quite flexible: It permits to install several cups-pdf backend "instances" (so one can use different configurations), and it can be told not to install a cups queue, so the user can install one or even several queues (again with varying configurations) if needed.

    Note that cups calls its backend processes with an ordinary user account, but cups-pdf wants to be called with root privileges because it assigns the print job's owner as ownership to the produced files. Since we don't want to change the cups configuration so that it calls every program with root privileges, this NixOS module installs a suid root wrapper. The wrapper is configured such that only cups (only the lp group) can call it.

  • A NixOS test is also added that uses cups-pdf to generate pdf files, then checks if they contain the printed text.

    Note that currently, cups-pdf-to-pdf produces small and searchable pdf files, but the text is "garbled", i.e., characters are reassigned randomly. This is a known issue and I don't know how to fix it:

    Again garbled text also with the current code base alexivkin/CUPS-PDF-to-PDF#7

    So the test checks the pdf by rasterizing it, then applies OCR.

  • As there is already a package request (see above), this change might warrant a mention in the release notes.

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/)
  • 22.11 Release Notes (or backporting 22.05 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.
Result of nixpkgs-review run on x86_64-linux 1
1 package blacklisted:
  • nixos-install-tools
1 package built:
  • cups-pdf-to-pdf
References

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: printing Drivers, CUPS & Co. 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` labels Jul 21, 2022
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jul 21, 2022
@Yarny0 Yarny0 mentioned this pull request Jul 21, 2022
Copy link

Choose a reason for hiding this comment

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

Consider registrating as cups-pdf-to-pdf instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(see above)

@Yarny0
Copy link
Contributor Author

Yarny0 commented Jul 23, 2022

Dear @nrdsp ,

adding a package option should be easy. Would you prefer we offer such an option? Just tell me and I'll push an updated branch. Do you think we should also package the original cups-pdf package? I remember building this package also before I discovered the improved fork, and I faintly remember it was quite easy and no difference to the forked package. So if you like cups-pdf packaged as well, just tell me and I'll add it to the branch, together with another vm test.

Copy link

@nrdsp nrdsp left a comment

Choose a reason for hiding this comment

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

The package seems to build on my machine, even if it complained about some functions being deprecated (opened issue upstream).

@nrdsp
Copy link

nrdsp commented Jul 23, 2022

Dear @nrdsp ,

adding a package option should be easy. Would you prefer we offer such an option? Just tell me and I'll push an updated branch. Do you think we should also package the original cups-pdf package? I remember building this package also before I discovered the improved fork, and I faintly remember it was quite easy and no difference to the forked package. So if you like cups-pdf packaged as well, just tell me and I'll add it to the branch, together with another vm test.

I would rather use the cups-pdf-to-pdf package than the original now that I'm aware of its existence. The option to choose from both packages should be easy to implement in the future if the need arises.

@bobby285271 bobby285271 added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jul 27, 2022
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1070

@Yarny0 Yarny0 force-pushed the cups-pdf branch 2 times, most recently from 425b383 to e758e01 Compare August 8, 2022 17:51
@Yarny0
Copy link
Contributor Author

Yarny0 commented Aug 8, 2022

Thanks @SuperSandro2000 for your suggestions. The most recent branch should now incorporate all of them.

@bobby285271 bobby285271 removed the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 9, 2022
@SuperSandro2000 SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 15, 2022
@Yarny0
Copy link
Contributor Author

Yarny0 commented Aug 18, 2022

SuperSandro2000 added the 2.status: merge conflict label 3 days ago

Rebased onto current nixos-unstable ( 762b003 ) and resolved the conflict (just had to rerun md-to-db.sh). cups-pdf vm test still passes.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 18, 2022
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-in-distress/3604/52

Copy link
Member

Choose a reason for hiding this comment

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

Isn't this relative to root or your current user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean with this question.

The cups-pdf documentation of this config directive doesn't say anything specific about relative paths. From looking at the code, I suspect a relative path would just be interpreted relative to the CWD. According to the documentation (and code), ${HOME} and ${USER} are special tokens that will be expanded to the print job owner's home directory and name, respectively. I expanded the documentation of that option a bit to explain it better.

@Yarny0
Copy link
Contributor Author

Yarny0 commented Nov 21, 2022

Thanks again for your review, @SuperSandro2000 . I hope I could answer all your questions. The brach is also rebased onto current nixos-unstable. The vm test still passes.

@RaitoBezarius
Copy link
Member

Can you move the release notes to 23.05 ?

@Yarny0 Yarny0 force-pushed the cups-pdf branch 2 times, most recently from d065a54 to fee623a Compare December 1, 2022 19:29
@Yarny0
Copy link
Contributor Author

Yarny0 commented Dec 1, 2022

Can you move the release notes to 23.05 ?

Done. Also rebased onto current nixos-unstable. Test still passes.

Copy link
Member

Choose a reason for hiding this comment

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

This fails on Darwin, which is definitely something that can be used there I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmmm, I don't know much about Darwin. Following the first hint in https://nixos.org/manual/nixpkgs/stable/#sec-darwin , I replaced gcc with $CC. Hopefully, this calls clang and the command line still works. On x86_64-linux, the package still builds fine with this change (and the test passes).

Can you try if this works on Darwin.

Copy link
Member

Choose a reason for hiding this comment

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

@RaitoBezarius
Copy link
Member

Alas, release notes conflicts :'( ; please fix the conflicts and I think we are good to merge.

@Yarny0
Copy link
Contributor Author

Yarny0 commented Dec 4, 2022

Rebased onto current nixos-unstable, merge conflict is resolved. Test still passes.

Note that cups-pdf refuses to run without root privileges.
To use the binary, one has to either convince cups to
call it with root privileges, or install it suid root.

Also note that currently, this cups-pdf-fork produces
small pdfs with selectable text, as promised.
However, copying the text produces "garbled" text
(characters are randomly reassigned).
This is a known issue and I don't know how to fix it:

alexivkin/CUPS-PDF-to-PDF#7
Some implementation notes:

* cups-pdf, and cups-pdf-to-pdf,
  support multiple instances with differing configurations.
  This can be accomplished by creating multiple configuration
  files with names `cups-pdf-{instance-name}.conf`.
  The Nixos module supports this feature by providing
  the option `instances` which is an attrset
  mapping instance names to instance configurations.
  To simplify module usage,
  an instance `pdf` is created by default.

* To use a cups-pdf instance, one also needs
  a cups queue that connects to the backend.
  The module does this automatically by default,
  using the `hardware.printers.ensurePrinters`.
  It uses one of the ppd files which is
  included in the cups-pdf package.
  If this isn't desired (e.g. because printer queues
  should be created by hand, or configured differently),
  the `installPrinter` option can be turned off
  (for each instance separately).

* In our configuration, cups calls external programs
  using the `cups` account and the `lp` group.
  cups-pdf refuses to operate without root privileges,
  likely because it needs to change the
  ownership of it output pdf files so that
  (only) the print job's owner can access them.

  The module installs a suid root wrapper for the backend
  program that can only be called by the `lp` group.
  The cups-pdf package is replaced by a wrapper
  package which calls the suid root wrapper.
  So cups can call its backend programs as usual.
@Yarny0
Copy link
Contributor Author

Yarny0 commented Dec 9, 2022

Rebased onto nixos-unstable again, due to a new merge conflict. Test still passes.

@RaitoBezarius
Copy link
Member

BTW, naive question but, you wrote:

Also note that currently, this cups-pdf-fork produces
small pdfs with selectable text, as promised.
However, copying the text produces "garbled" text
(characters are randomly reassigned).
This is a known issue and I don't know how to fix it:

Does that mean this package provide zero value with respect to its main feature?

@Yarny0
Copy link
Contributor Author

Yarny0 commented Dec 17, 2022

BTW, naive question but, you wrote:

[...]

Does that mean this package provide zero value with respect to its main feature?

This depends on what is considered the main feature, but if the main feature is text-searchable pdf-files, then sadly yes.
The original cups-pdf produces rasterized pdfs, thus the resolution is limited by the rasterization when viewed/printed on high-dpi devices. Even without the possibility to extract text, the pdfs from CUPS-PDF-to-PDF are usually vector graphics and ofter require less space. Depending on the use-case, this certainly can be an improvement (it is for me).

@RaitoBezarius
Copy link
Member

This is a good candidate for a more general nixpkgs warning (NixOS/rfcs#127), but I think it's fine.

@RaitoBezarius RaitoBezarius merged commit 861c7b1 into NixOS:master Dec 28, 2022
@Yarny0 Yarny0 deleted the cups-pdf branch January 15, 2023 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: printing Drivers, CUPS & Co. 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants