Skip to content
Snippets Groups Projects

Allow /etc/debsums-ignore to be deleted rather than empty

Closed Josh Triplett requested to merge josh/debsums:no-empty-file into master

Modify the cron jobs to handle that file not existing, and don't ship it by default. This avoids shipping an empty configuration file in /etc.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • assigned to @abe

  • Generally fine and a good idea. Actually I wasn't aware of that file as I don't use the cron jobs.

    Nevertheless this even adds more code duplication, which I'd prefer to reduce. I was already thinking about factoring out all common code of the cron jobs into a single file.

    Edited by Axel Beckert
  • One thing which likely also needs to be handled is proper config file removal, even if it's empty by default.

  • Personally, I would prefer if the cron job just appeared in /usr/share/doc/debsums/ as an example, which makes it easy to tell people to drop it into daily, weekly, or whatever they want. That also removes the need to make it configurable when to run.

  • Personally, I would prefer if the cron job just appeared in /usr/share/doc/debsums/ as an example, which makes it easy to tell people to drop it into daily, weekly, or whatever they want. That also removes the need to make it configurable when to run.

  • While that would have been an option initially, IMHO this is a too invasive change nowadays as we don't know how many people use that feature. Changing it now IMHO cries for rather ugly breakage.

  • Suppose that we removed the feature but didn't remove the conffile, so that new installations didn't have it but existing installations (that might be using it) don't have it removed?

    Optionally, we could also remove unmodified versions of the cron jobs if and only if the user hasn't configured them to be enabled.

    (All that said, I don't want to let the perfect be the enemy of the good here, and I'd be happy to update this patch to remove debsums-ignore if empty if you prefer. However, I also think it'd make sense to just leave it, since an empty file has the same semantic meaning as a missing one.)

  • However, I also think it'd make sense to just leave it, since an empty file has the same semantic meaning as a missing one.

    Not according to policy §9.1.2. There the sole thing looked at is existence or non-existence of /etc/staff-group-for-usr-local.

    Anyways, I intent to merge this merge request next time I work on debsums. I don't want to merge it now as I then would likely forget to work out proper config file removal, etc.

  • Not according to policy §9.1.2. There the sole thing looked at is existence or non-existence of /etc/staff-group-for-usr-local.

    I don't mean in general, I mean specifically for /etc/debsums-ignore after this MR.

    I don't want to merge it now as I then would likely forget to work out proper config file removal, etc.

    I'd be happy to update this MR to include config file removal, if that'd help.

  • I don't mean in general, I mean specifically for /etc/debsums-ignore after this MR.

    Ah ok, a misunderstanding then.

    I'd be happy to update this MR to include config file removal, if that'd help.

    Yes, please!

    Feel free to include a proper debian/changelog entry, too. (Currently wondering if this MR is actually a feature request or a bug fix — because that would make it either version 2.3.0 or version 2.2.4 according to Semantic Versioning.)

    Although I just thought that we can also have this feature (empty and non-existent file are equal) while still shipping /etc/debsums-ignore in the package, maybe even with some explanatory comment in it. (Would need some comment parsing feature first. Which would mean another level of indirection in /etc/cron.*/debsums, so that's clearly a separate feature.)

    But then again I think I remember that you're following the school of least clutter in /etc/ while I prefer self-documenting configuration files by default, even if the defaults without config file are sane. So you are probably not happy with that idea of mine. ;-)

  • I'm all for shipping self-documenting configuration files, I'd just like them to be in /usr/share/doc/$package as examples rather than in /etc. :)

  • The commit is now cherry-picked as bbda06b2 and pushed. I added proper rm_conffile handling, too.

    Edited by Axel Beckert
  • closed

  • Thank you, I really appreciate it!

  • You're welcome.

    We're though still not there yet. I used debian/maintscript with rm_conffile, but this does not yet work as expected in case /etc/debsums-ignore has been modified:

    Preparing to unpack debsums_2.2.5_all.deb ...
    Unpacking debsums (2.2.5) over (2.2.4) ...
    Setting up debsums (2.2.5) ...
    Installing new version of config file /etc/cron.daily/debsums ...
    Installing new version of config file /etc/cron.monthly/debsums ...
    Installing new version of config file /etc/cron.weekly/debsums ...
    Obsolete conffile /etc/debsums-ignore has been modified by you.
    Saving as /etc/debsums-ignore.dpkg-bak ...

    So the file gets unconditionally renamed if modified. That's not what we want.

Please register or sign in to reply
Loading