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
nixos/cupsd: new option for specifying printers.conf
declaratively.
#23684
nixos/cupsd: new option for specifying printers.conf
declaratively.
#23684
Conversation
@jraygauthier, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @abbradar and @urkud to be potential reviewers. |
Would also be a candidate for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally don't add features to the release after branch-off. Do you think it's highly desirable?
description = '' | ||
Contents of the printers configuration file of the CUPS daemon | ||
(<filename>printers.conf</filename>). When left null, cups will use | ||
the (<filename>/etc/cupsd/printers.conf</filename>) as the configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe /etc/cups/
was meant here? Also CUPS will use this path anyway, it's just that the file will or won't be there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disregard the second point, I forgot how our CUPS works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yes, cups
was meant. Will fix the doc. As to the behavior, as with other configuration files, as soon as the configuration is modified through the web interface, cups will move printers.oonf
aside to printers.conf.0
. Otherwise, cups will use the declarative file. This seems to be consistent with
the behavior of other configurations files.
It is indeed desirable to me, not sure about others. I can live with rebasing the changeset over 17.03 though as long as it is available in the next release (17.09?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@globin, how do you feel about porting this to 17.03? This is a feature but seems a very small one.
378ed78
to
52fee6a
Compare
I just noticed that cups will also move aside the configuration when stopped and the configuration is present. It makes this only semi useful. Do you think it would be a good that when declarative printers configuration is requested (non null), |
Seems fair -- we already do this in several other modules. EDIT: Please mention it in the docs! |
There's a bit of history there: |
52fee6a
to
c18acb9
Compare
An improved version which replaces the cups modified version with the declarative version when specified. About the documentation, do you have a suggestion as to where to bring this up? I had a look at nixos manual and there seems to be no section about printer configuration. Were you rather referring to the release notes? |
I am currently investigating whether it would be possible to symlink instead of copy. It would provide us with a way of knowing that the current file already is the declarative version and avoid the operation completly. |
c18acb9
to
04e0838
Compare
This latest version with the symlink seems to also play well with cups and its behavior is closer to what's usually done by nixos. It will even backup a previous non-declarative version of the configuration to cups's standard backup location. |
ln -s --backup=simple --suffix='.O' \ | ||
"${rootdir}/etc/cups/printers.conf" "/var/lib/cups/printers.conf" | ||
fi | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the difference to what is done in the following lines is that we do a backup here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not exactly. The difference that matters is that for this particular file we now systematically / always do the linking even when the file exist (in which case we do a backup).
@@ -305,6 +321,16 @@ in | |||
[ -L "$i" ] && rm "$i" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make that rm -- "$i"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is not part of the changeset. Is there any reason you mention this as part of this PR?
according to https://ss64.com/bash/rm.html:
The rm command accepts the -- option which will cause it to stop processing flag options from that point forward.
This allows the removal of file names that begin with a dash -.
rm -- -filename
Alternatively use an absolute or relative path reference.
rm /home/user/-filename
rm ./-filename
As I understand it, the --
add no value here as i
will always be filled with an absolute path of the form /var/lib/cups/*
.
Stalled. |
Motivation for this change
To allow for declarative specification of cups printers.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)