Created attachment 572452 [details, diff] patch for sys-apps/smartmontools-7.0-r1 ebuild If USE=-update_drivedb, then the /var/db/smartmontools/drivedb.h first installed on the system never gets updated by subsequent merges of sys-apps/smartmontools: # genlop smartmontools | tail Tue Dec 5 05:43:39 2017 >>> sys-apps/smartmontools-6.6 Fri Dec 29 00:01:01 2017 >>> sys-apps/smartmontools-6.6 Thu Jan 25 02:16:04 2018 >>> sys-apps/smartmontools-6.6 Thu Jan 25 04:48:47 2018 >>> sys-apps/smartmontools-6.6 Tue Mar 27 04:05:44 2018 >>> sys-apps/smartmontools-6.6 Tue Mar 27 07:17:10 2018 >>> sys-apps/smartmontools-6.6 Tue Apr 9 09:30:51 2019 >>> sys-apps/smartmontools-7.0-r1 # ll /var/db/smartmontools/drivedb.h -rw-r--r-- 1 root root 176K Dec 5 2017 /var/db/smartmontools/drivedb.h The ebuild prints a warning message that recommends manually updating /var/db/smartmontools/drivedb.h. However, this is not a maintainable solution from a sysadmin's POV. On top of that, the warning message itself is easy to miss. If USE=update_drivedb, sys-apps/smartmontools-7.0-r1.ebuild installs a monthly cron job that runs /usr/sbin/update-smart-drivedb. The cron job is not necessarily desirable, so just setting the USE-flag is not a solution. I propose changing the logic in the ebuild to update /var/db/smartmontools/drivedb.h if the timestamp of the shipped file is newer than the existing file. This should result in reasonable behavior regardless of the state of the 'update_drivedb' USE-flag.
We chose this solution because we cannot add a file to the package database that is supposed to change between package upgrades. This would lead to incorrect file checksums in the package database.
(In reply to Lars Wendler (Polynomial-C) from comment #1) > We chose this solution because we cannot add a file to the package database > that is supposed to change between package upgrades. This would lead to > incorrect file checksums in the package database. The patch was written with that existing behavior in mind and shouldn't break it, unless I've misunderstood something.
Can you please rebase your patch against sys-apps/smartmontools-7.2?
Created attachment 684732 [details, diff] patch against sys-apps/smartmontools-7.2.ebuild Here's an improved version of the patch rebased against 7.2. If there's a better/canned way to do this without losing any corner-case coverage, please let me know.
Created attachment 684816 [details, diff] patch for sys-apps/smartmontools-7.2 The writability check in the previous patch was not implemented correctly. Here's a corrected version of the patch which should hopefully work as intended. Apologies for the mistake and additional noise.
I would also not print the warning is the cron file is installed
Created attachment 766807 [details, diff] smartmontools-7.3-r1.patch Attaching the patch from https://github.com/gentoo/gentoo/pull/24394. I'm not going with this (we're going to do CONFIG_PROTECT instead), but wanted to post it here for posterity.
(In reply to Sam James from comment #7) > Created attachment 766807 [details, diff] [details, diff] > smartmontools-7.3-r1.patch > > Attaching the patch from https://github.com/gentoo/gentoo/pull/24394. I'm > not going with this (we're going to do CONFIG_PROTECT instead), but wanted > to post it here for posterity. Is this the CONFIG_PROTECT approach: https://github.com/gentoo/gentoo/pull/24394/commits/6b01c050a531364777b85976a395b1fa703b2f83 I see it in the PR 24394 , but that also includes this change: https://github.com/gentoo/gentoo/pull/24394/commits/15bbf78a8f02fb2d02bc6df3e15165842b0f31f0
How about adding a pkg_config() too, so that the user wouldn't have to type the commands manually?
