Red Hat Bugzilla – Bug 57456
Improve & clearify 'include' directive
Last modified: 2007-04-18 12:38:41 EDT
Since /etc/logrotate.conf will be overwritten occasionally (see bug #14081), it is impossible to modify this file without losing reliability. To make global adaptations possible (e.g. another olddir directory, another rotation cycle) without changing this file I suggest: 1. when including an entire directory (e.g.'include /etc/logrotate.d') sort the filelist as it is done by 'ls'. Then a file /etc/logrotate.d/0options can be created which contains global options. Modules like the initscripts or vixie-cron are using a similar mechanism. or 2. Add something like 'include /etc/logrotate-site.conf' before the 'include /etc/logrotate.d' in /etc/logrotate.conf. Then this file should be marked as %config(noreplace) and the user can make modifications there. Packages like a2ps or lynx are using a similar approach. When using this method, options in global sections should be handled either as errors (e.g. ----- /etc/logrotate.d/samba | olddir /var/log/.old/samba | /var/log/samba/*.log { ... } would be erroneously) or the changed options should be reset when leaving the file. In any case, the mechanism how entire directories are read (sorted at 1. and random at 2.) should be mentioned in the manpage explicitly.
These days rpm seems to generate .rpmnew files instead of .rpmsave's, so I'm not really sure that your suggestions make sense anymore. If .rpmsave's are still a problem, we would probably need to discuss that ancient bug 14081 (I think there is a communications gap there as to why .rpmsave's make sense) before implementing this solution.
I don't speak about rpmsave or rpmnew files (and no, because '%config' and not '%config(noreplace)' is used for /etc/logrotate.conf, this file will be renamed to ...rpmsave and replaced by the RH logrotate.conf). I speak about the order in which the files are read. Currently this order is determined by the inode-number which is very difficulty to influence. E.g. consider a file /etc/logrotate.d/0options where local configurations are placed into. When will this file be read? Before /etc/logrotate.d/psacct? This randomness makes it very difficultly to make local adaptations.
My point was that the only reason you are worried about the sort order of the directory is because you don't understand the whole reason behind doing .rpmsave , and while I'll gladly take a patch, I'm not going to spend time coding on a way for you to avoid working with rpm. If you would like to discuss the reasons behind .rpmsave's, their advantages and disadvantages, or perhaps using noreplace on logrotate.conf, I'd be glad to have a conversation via e-mail with you.
I am not happy with your decision to keep /etc/logrotate.conf marked as %config only, but I accept it because you may have good reasons to replace it when changes are necessarily. The real flaw is the undetermined behavior of the 'include' statement. I will give a full example: --- /etc/logrotate.conf -- ## default file from logrotate-package | ... | include "/etc/logrotate.d" | ... --- /etc/logrotate.d/0options -- ## local configuration | olddir /var/log/.old --- /etc/logrotate.d/rpm --- ## from rpm-package | /var/log/rpmpkgs { | weekly | notifempty | missingok | } logrotate accepts both the 0options and rpm file and handles them while executing 'logrotate /etc/logrotate.conf'. But it is completely unclear where the rotated /var/log/rpmpkgs file will be placed. When '0options' has a lower inode-number, it will be moved into /var/log/.old, else into /var/log. This is ambiguous and the logrotate-documentation says nothing about this case. An expected behavior would be a sorted executing of the included files or prohibition of global options in included files. I will see if/when I can provide a patch...
Created attachment 40987 [details] Lets read 'include' the files of a directory in alphabetic order.
Patch should provide behavior as described in 1) of the initial report. BTW: While checking for resource-leaks introduced by my patch I have found other ones. I will attach the dmalloc result... (locations are refering to logrotate-3.6-1 + my patch).
Created attachment 40988 [details] summarized dmalloc result
Thanks, patch committed to CVS. The memory leaks are not a huge concern, since logrotate is not a long-running program. But again, patches welcome. :)