Bug 3700

Hi, thanks for reporting this bug.
As there is no maintainer for this package I added the committers in CC.

CC: (none) => mageia, thierry.vignaud

Created attachment 1458 [details]
Patch by Gordon Lack

attaching a copy of Gordon Lack's comment containing his patch from  https://qa.mandriva.com/show_bug.cgi?id=63122#c1, because I don't know what will happen to Mdv

Update - This bug is still present in Beta 2.

Also, based on the documentation there are 2 different options for specifying excluded files or directories: -x which takes a path as an argument, and -X which takes a pattern. Since the argument that fails contains a wildcard I believe the correct option would be -X instead of -x, however both throw the same error.

CC: (none) => jalaramie

(In reply to comment #3)
> Update - This bug is still present in Beta 2.

> I
> believe the correct option would be -X instead of -x, however both throw the
> same error.

Well, yes and no. It is a two part fix.  Part 1 is:

$ dif /etc/sysconfig/tmpwatch_vorig /etc/sysconfig/tmpwatch
3c3
< TMPWATCH_EXCLUDES="-x /tmp/.ICE-unix -x /tmp/.X*-unix -x /tmp/.font-unix -x /tmp/.Test-unix"
---
> TMPWATCH_EXCLUDES="-x /tmp/.ICE-unix -X /tmp/.X*-unix -x /tmp/.font-unix -x /tmp/.Test-unix"

Part 2 is:
$ dif /etc/cron.daily/_vorig/tmpwatch_vorig /etc/cron.daily/tmpwatch
4a5
> set -f
5a7
> set +f
6a9
> set -f
7a11
> set +f

Created attachment 1825 [details]
patched /etc/sysconfig/tmpwatch

Created attachment 1826 [details]
patched /etc/cron.daily/tmpwatch

(In reply to comment #4)
> (In reply to comment #3)
> > Update - This bug is still present in Beta 2.
> 
> Part 2 is:
> $ dif /etc/cron.daily/_vorig/tmpwatch_vorig /etc/cron.daily/tmpwatch
> 4a5
> > set -f
> 5a7
> > set +f
> 6a9
> > set -f
> 7a11
> > set +f

This works for me as well. Now I have to go brush up my sh scripting skills to find out why it works. ;-)

Wouldnt be backslashing the * be a much simpler (and more evident) solution? - ending up with:

TMPWATCH_EXCLUDES="-x /tmp/.ICE-unix -X /tmp/.X\*-unix -x /tmp/.font-unix -x /tmp/.Test-unix"

CC: (none) => juergen.harms

(In reply to comment #8)
> Wouldnt be backslashing the * be a much simpler (and more evident) solution? -
> ending up with:
> 
> TMPWATCH_EXCLUDES="-x /tmp/.ICE-unix -X /tmp/.X\*-unix -x /tmp/.font-unix -x
> /tmp/.Test-unix"

I'll confirm that just changing /etc/sysconfig/tmpwatch to the above does keep /etc/cron.daily/tmpwatch from complaining. :)

I'll always vote for the "much simpler and more evident solution" every time. 8-)

>> I'll always vote for the "much simpler and more evident solution" every time.

So do I.  Hence my comment in the patch about *not* begin able to do it with quoting. (i.e. That's what comments are for - making evident).

The f-flag is in set for a reason.  This is it.

You can simplify the patch to the script by removing the consecutive set +f, set -f, but only because the two tmpwatch commands there are consecutive.  This, however, would remove the symmetry of each /usr/sbin/tmpwatch invocation.

And don't try dropping it all into a function either.  What you have here is a single variable (TMPWATCH_EXCLUDES) containing multiple parameters, and one of them is a wildcard which *must not be expanded by the shell* (tmpwatch itself does the expansion).  That is an extremely fragile setup.

CC: (none) => gordon.lack

(In reply to comment #10)
>  That is an extremely fragile setup.

IMHO the whole configuration is a bit squirrelly...

1. I've never seen a cron script put a config file in /etc/sysconfig before. I always thought that directory was for the config files for services.

2. There are 2 variables specified in the config file; 1 is disabled and TMPWATCH_EXCLUDES only applies to one invocation of tmpwatch. I guess theoretically you could add other invocations of tmpwatch that had excluded directories, but then you would have to edit 2 files instead of simply adding the excluded directory to the new invocation.

3. tmpwatch is invoked 3 times in the cron script, but TMPWATCH_OPTIONS is only set once. What if you wanted to use different options for each invocation?

I think the 2 file setup is overly complex and less flexible than a single cron script, but maybe I just don't understand why it was done that way.

/etc/sysconfig seems a reasonable place to put a config file for a *system* crontab.
This is on RedHat5:
=====
cron.daily/prelink:. /etc/sysconfig/prelink
cron.weekly/99-raid-check:# This script reads it's configuration from /etc/sysconfig/raid-check
cron.weekly/99-raid-check:[ -f /etc/sysconfig/raid-check ] || exit 0
cron.weekly/99-raid-check:. /etc/sysconfig/raid-check
=====

It's also a good idea to keep configuration separate from code - it allows you to edit a config file without worrying about code structure and also to keep code in a CMS without having to specify a config in it.

No idea what LSB says, though.

(In reply to comment #12)
> /etc/sysconfig seems a reasonable place to put a config file for a *system*
> crontab.
> This is on RedHat5:
> =====
> cron.daily/prelink:. /etc/sysconfig/prelink
> cron.weekly/99-raid-check:# This script reads it's configuration from
> /etc/sysconfig/raid-check
> cron.weekly/99-raid-check:[ -f /etc/sysconfig/raid-check ] || exit 0
> cron.weekly/99-raid-check:. /etc/sysconfig/raid-check
> =====
> 
> It's also a good idea to keep configuration separate from code - it allows you
> to edit a config file without worrying about code structure and also to keep
> code in a CMS without having to specify a config in it.

Oh I agree completely that config and code should be separate. In fact I have template init scripts and matching config files that put *every* variable that isn't part of the script logic in the config file, which is more than many init scripts do. It just seems in this particular case it serves no useful purpose and actually makes configuration less flexible since tmpwatch is invoked multiple times. I don't think it's a big deal worth putting a lot of thought or effort into. Just an observation.

> 
> No idea what LSB says, though.

I think LSB is a great idea and the best (only?) standard Linux has, and I support following it. (Honestly, I also think it's just sewing another arm on Frankenstein but that's a rant for another place and time)  ;-)

Jeff

The updated /etc/sysconfig/tmpwatch configuration file in package tmpwatch-2.10.3-2.mga2 avoids wildcards and sidesteps this issue on my system. But it also enables the first line: TMPWATCH_OPTIONS="-umc" Is that correct? Based on the man page it looks like those 3 options are mutually exclusive.

Jeff

(In reply to comment #14)
> The updated /etc/sysconfig/tmpwatch configuration file in package
> tmpwatch-2.10.3-2.mga2 avoids wildcards and sidesteps this issue on my system.
> But it also enables the first line: TMPWATCH_OPTIONS="-umc" Is that correct?
> Based on the man page it looks like those 3 options are mutually exclusive.
> 
> Jeff

http://pkgs.fedoraproject.org/gitweb/?p=tmpwatch.git;a=blob;f=tmpwatch.daily;h=5ab2e7462f5bd901bdcdb19c96145d8e6bc8317e;hb=8d2580d952fa93c4f08f7bfaf53433c1ab4ecd7f

As Fedora is the upstream and they are using the same flags then i hope they know what they do. But to be sure you may ask the upstream.

CC: (none) => sander.lepik

Hi,

This bug was filed against cauldron, but we do not have cauldron at the moment.

Please report whether this bug is still valid for Mageia 2.

Thanks :)

Cheers,
marja

Keywords: (none) => NEEDINFO

(In reply to comment #16)

> Please report whether this bug is still valid for Mageia 2.

resolved with the patch

Keywords: NEEDINFO => (none)
Status: NEW => RESOLVED
Resolution: (none) => FIXED