Re: locate.updatedb TMPDIR
On Sun, Feb 09, 2020 at 04:33:13PM +0100, Ingo Schwarze wrote:
>
> this is absolutely not OK.
>
> How did you test this?
I changed weekly directly on the remote machine to detect and narrow
down the issue and generated the diff later on my local machine source
tree.
>$ doas cat /var/log/weekly.part
> /etc/weekly[79]: no closing quote
Hah! Thanks for spotting this! Below the correct diff.
OK?
FYI, this effectively reverts Revision 1.9 committed by mickey about 20
years ago.
> > Note, this might break existing setups, where one has set TMPDIR in
> > /etc/weekly.local to point elsewhere. This might be handled with a
> > -current upgrade entry?
Please find below another possible diff for current.html
OK?
Index: etc/weekly
===
RCS file: /cvs/src/etc/weekly,v
retrieving revision 1.29
diff -u -p -r1.29 weekly
--- etc/weekly 30 Dec 2019 16:49:51 - 1.29
+++ etc/weekly 9 Feb 2020 19:51:31 -
@@ -48,7 +48,7 @@ if [ -f /var/db/locate.database ]; then
if TMP=`mktemp /var/db/locate.database.XX`; then
trap 'rm -f $TMP; exit 1' 0 1 15
UPDATEDB="/usr/libexec/locate.updatedb"
- echo "${UPDATEDB} --fcodes=- --tmpdir=${TMPDIR:-/tmp}" | \
+ echo "${UPDATEDB} --fcodes=-" | \
nice -5 su -m nobody 1>$TMP
if [ $? -ne 0 ]; then
echo "Rebuilding locate database failed"
Index: faq/current.html
===
RCS file: /cvs/www/faq/current.html,v
retrieving revision 1.1025
diff -u -p -r1.1025 current.html
--- faq/current.html8 Feb 2020 10:19:11 - 1.1025
+++ faq/current.html9 Feb 2020 19:51:35 -
@@ -205,8 +205,17 @@ Old binaries should be deleted and scrip
adapted.
-# rm -f /usr/sbin/{dig,host,nslookup}
+# rm -f /usr/sbin/{dig,host,nslookup}
+
+
+2020/02/09 - /etc/weekly locate.updatedb TMPDIR
+
+TMPDIR is no longer propagated for locate.updatedb
+in https://man.openbsd.org/weekly.8";>weekly(8).
+Custom TMPDIR values for locate.updatedb set in
+root crontab or /etc/weekly.local should be moved into
+/etc/locate.rc.
Re: locate.updatedb TMPDIR
On Sun, Feb 09, 2020 at 04:18:52PM +0100, Ingo Schwarze wrote: > Hi Todd, > > Todd C. Miller wrote on Sun, Feb 09, 2020 at 07:52:10AM -0700: > > > I'm fine with this. > > I don't really object, but i'm not sure it is needed either. > > It's certainly obvious that command line arguments override defaults. > That's what they always do. It's their whole point, in general. > > Also, command line arguments (at least almost) always override > configuration file settings. If they don't, i would usually call > that a serious design error. There may be exceptions, like > configuration settings to enable or disable optional command > line functionality, but that is very unusual. Personally, I won't necessarily expect that, as I have seen way to many tools doing all kinds of weird things with ordering, e.g. defaults, overridden by local user config, overridden by current env, overridden by global config, overridden by command line flags, etc. In this particular case I had to lookup the order in the code to narrow down the actual issue. Hence I thought it would be a good idea to document this. However, I'm fine with assuming sane defaults on OpenBSD, so not needed then. > > However, it doesn't look like we document /etc/locate.rc anywhere. > > I'm not sure it deserves its own man page > > but I think it should at least be documented in locate.updatedb > > in some fashion. > > It is mentioned there, but not really documented. > > OK? OK jung@ > Ingo > > > Index: locate.updatedb.8 > === > RCS file: /cvs/src/usr.bin/locate/locate/locate.updatedb.8,v > retrieving revision 1.18 > diff -u -r1.18 locate.updatedb.8 > --- locate.updatedb.8 10 Dec 2009 00:45:43 - 1.18 > +++ locate.updatedb.8 9 Feb 2020 15:14:57 - > @@ -51,10 +51,6 @@ > .Pa /etc/weekly > script. > .Pp > -The contents of the newly built database can be controlled by the > -.Pa /etc/locate.rc > -file as well as the command line arguments. > -.Pp > The options are as follows: > .Bl -tag -width Ds > .It Fl -fcodes > @@ -75,6 +71,14 @@ > .It Fl -tmpdir > Sets the directory temporary files are stored in. > .El > +.Pp > +The default settings are controlled by the configuration file > +.Pa /etc/locate.rc . > +It is a > +.Xr sh 1 > +script that can be used to set variables. > +The names of the variables match the names of the command line > +options, but in all caps. > .Sh FILES > .Bl -tag -width /var/db/locate.database -compact > .It Pa /etc/locate.rc >
Re: locate.updatedb TMPDIR
Hi Joerg,
this is absolutely not OK.
How did you test this?
$ doas cat /var/log/weekly.part
/etc/weekly[79]: no closing quote
With that fixed, i agree with the direction of the change.
Yours,
Ingo
Joerg Jung wrote on Sun, Feb 09, 2020 at 12:33:42AM +0100:
> I have a machine with a large data storage attached, but it has only 2GB
> of /tmp (which I consider enough usually). On this machine weekly
> locate.updatedb fails, due to /tmp being full. To fix this I would like
> to point locate to a different TMPDIR.
>
> But it seems one can not just set TMPDIR from /etc/locate.rc to point
> elsewhere, since command line args in /etc/weekly override
> /etc/locate.rc settings.
>
> While it's possible to add a weekly.local to set TMPDIR I believe it
> should be better set in the actual configuration file instead of
> ignoring locate.rc.
>
> Thus the diff below proposes to remove command line argument to be able
> to use /etc/locate.rc instead.
>
> Note, this might break existing setups, where one has set TMPDIR in
> /etc/weekly.local to point elsewhere. This might be handled with a
> -current upgrade entry?
>
> Comments? OK?
>
> Thanks,
> Regards,
> Joerg
>
>
> Index: etc/weekly
> ===
> RCS file: /cvs/src/etc/weekly,v
> retrieving revision 1.29
> diff -u -p -r1.29 weekly
> --- etc/weekly30 Dec 2019 16:49:51 - 1.29
> +++ etc/weekly8 Feb 2020 23:13:02 -
> @@ -48,7 +48,7 @@ if [ -f /var/db/locate.database ]; then
> if TMP=`mktemp /var/db/locate.database.XX`; then
> trap 'rm -f $TMP; exit 1' 0 1 15
> UPDATEDB="/usr/libexec/locate.updatedb"
> - echo "${UPDATEDB} --fcodes=- --tmpdir=${TMPDIR:-/tmp}" | \
> + echo "${UPDATEDB} --fcodes=- | \
> nice -5 su -m nobody 1>$TMP
> if [ $? -ne 0 ]; then
> echo "Rebuilding locate database failed"
Re: locate.updatedb TMPDIR
On Sun, 09 Feb 2020 16:18:52 +0100, Ingo Schwarze wrote: > It is mentioned there, but not really documented. OK millert@. - todd
Re: locate.updatedb TMPDIR
Hi Todd, Todd C. Miller wrote on Sun, Feb 09, 2020 at 07:52:10AM -0700: > I'm fine with this. I don't really object, but i'm not sure it is needed either. It's certainly obvious that command line arguments override defaults. That's what they always do. It's their whole point, in general. Also, command line arguments (at least almost) always override configuration file settings. If they don't, i would usually call that a serious design error. There may be exceptions, like configuration settings to enable or disable optional command line functionality, but that is very unusual. > However, it doesn't look like we document /etc/locate.rc anywhere. > I'm not sure it deserves its own man page > but I think it should at least be documented in locate.updatedb > in some fashion. It is mentioned there, but not really documented. OK? Ingo Index: locate.updatedb.8 === RCS file: /cvs/src/usr.bin/locate/locate/locate.updatedb.8,v retrieving revision 1.18 diff -u -r1.18 locate.updatedb.8 --- locate.updatedb.8 10 Dec 2009 00:45:43 - 1.18 +++ locate.updatedb.8 9 Feb 2020 15:14:57 - @@ -51,10 +51,6 @@ .Pa /etc/weekly script. .Pp -The contents of the newly built database can be controlled by the -.Pa /etc/locate.rc -file as well as the command line arguments. -.Pp The options are as follows: .Bl -tag -width Ds .It Fl -fcodes @@ -75,6 +71,14 @@ .It Fl -tmpdir Sets the directory temporary files are stored in. .El +.Pp +The default settings are controlled by the configuration file +.Pa /etc/locate.rc . +It is a +.Xr sh 1 +script that can be used to set variables. +The names of the variables match the names of the command line +options, but in all caps. .Sh FILES .Bl -tag -width /var/db/locate.database -compact .It Pa /etc/locate.rc
Re: locate.updatedb TMPDIR
On Sun, 09 Feb 2020 00:46:52 +0100, Joerg Jung wrote: I'm fine with this. However, it doesn't look like we document /etc/locate.rc anywhere. I'm not sure it deserves its own man page but I think it should at least be documented in locate.updatedb in some fashion. - todd > Index: usr.bin/locate/locate/locate.updatedb.8 > === > RCS file: /cvs/src/usr.bin/locate/locate/locate.updatedb.8,v > retrieving revision 1.18 > diff -u -p -r1.18 locate.updatedb.8 > --- usr.bin/locate/locate/locate.updatedb.8 10 Dec 2009 00:45:43 - > 1.18 > +++ usr.bin/locate/locate/locate.updatedb.8 8 Feb 2020 23:44:41 - > @@ -54,6 +54,7 @@ script. > The contents of the newly built database can be controlled by the > .Pa /etc/locate.rc > file as well as the command line arguments. > +Command line arguments override rc file and defaults. > .Pp > The options are as follows: > .Bl -tag -width Ds >
Re: locate.updatedb TMPDIR
On Sun, 09 Feb 2020 00:33:42 +0100, Joerg Jung wrote: > I have a machine with a large data storage attached, but it has only 2GB > of /tmp (which I consider enough usually). On this machine weekly > locate.updatedb fails, due to /tmp being full. To fix this I would like > to point locate to a different TMPDIR. > > But it seems one can not just set TMPDIR from /etc/locate.rc to point > elsewhere, since command line args in /etc/weekly override > /etc/locate.rc settings. > > While it's possible to add a weekly.local to set TMPDIR I believe it > should be better set in the actual configuration file instead of > ignoring locate.rc. > > Thus the diff below proposes to remove command line argument to be able > to use /etc/locate.rc instead. I think this is fine. The existing situation where /etc/weekly overrides /etc/locate.rc violates the principle of least surprise. - todd
Re: locate.updatedb TMPDIR
On Sun, Feb 09, 2020 at 12:33:42AM +0100, Joerg Jung wrote:
> Hi,
>
> I have a machine with a large data storage attached, but it has only 2GB
> of /tmp (which I consider enough usually). On this machine weekly
> locate.updatedb fails, due to /tmp being full. To fix this I would like
> to point locate to a different TMPDIR.
>
> But it seems one can not just set TMPDIR from /etc/locate.rc to point
> elsewhere, since command line args in /etc/weekly override
> /etc/locate.rc settings.
Below is a second diff, which tries to document that behaviour.
> While it's possible to add a weekly.local to set TMPDIR I believe it
> should be better set in the actual configuration file instead of
> ignoring locate.rc.
>
> Thus the diff below proposes to remove command line argument to be able
> to use /etc/locate.rc instead.
>
> Note, this might break existing setups, where one has set TMPDIR in
> /etc/weekly.local to point elsewhere. This might be handled with a
> -current upgrade entry?
>
> Comments? OK?
>
> Thanks,
> Regards,
> Joerg
>
>
> Index: etc/weekly
> ===
> RCS file: /cvs/src/etc/weekly,v
> retrieving revision 1.29
> diff -u -p -r1.29 weekly
> --- etc/weekly30 Dec 2019 16:49:51 - 1.29
> +++ etc/weekly8 Feb 2020 23:13:02 -
> @@ -48,7 +48,7 @@ if [ -f /var/db/locate.database ]; then
> if TMP=`mktemp /var/db/locate.database.XX`; then
> trap 'rm -f $TMP; exit 1' 0 1 15
> UPDATEDB="/usr/libexec/locate.updatedb"
> - echo "${UPDATEDB} --fcodes=- --tmpdir=${TMPDIR:-/tmp}" | \
> + echo "${UPDATEDB} --fcodes=- | \
> nice -5 su -m nobody 1>$TMP
> if [ $? -ne 0 ]; then
> echo "Rebuilding locate database failed"
>
Index: usr.bin/locate/locate/locate.updatedb.8
===
RCS file: /cvs/src/usr.bin/locate/locate/locate.updatedb.8,v
retrieving revision 1.18
diff -u -p -r1.18 locate.updatedb.8
--- usr.bin/locate/locate/locate.updatedb.8 10 Dec 2009 00:45:43 -
1.18
+++ usr.bin/locate/locate/locate.updatedb.8 8 Feb 2020 23:44:41 -
@@ -54,6 +54,7 @@ script.
The contents of the newly built database can be controlled by the
.Pa /etc/locate.rc
file as well as the command line arguments.
+Command line arguments override rc file and defaults.
.Pp
The options are as follows:
.Bl -tag -width Ds
locate.updatedb TMPDIR
Hi,
I have a machine with a large data storage attached, but it has only 2GB
of /tmp (which I consider enough usually). On this machine weekly
locate.updatedb fails, due to /tmp being full. To fix this I would like
to point locate to a different TMPDIR.
But it seems one can not just set TMPDIR from /etc/locate.rc to point
elsewhere, since command line args in /etc/weekly override
/etc/locate.rc settings.
While it's possible to add a weekly.local to set TMPDIR I believe it
should be better set in the actual configuration file instead of
ignoring locate.rc.
Thus the diff below proposes to remove command line argument to be able
to use /etc/locate.rc instead.
Note, this might break existing setups, where one has set TMPDIR in
/etc/weekly.local to point elsewhere. This might be handled with a
-current upgrade entry?
Comments? OK?
Thanks,
Regards,
Joerg
Index: etc/weekly
===
RCS file: /cvs/src/etc/weekly,v
retrieving revision 1.29
diff -u -p -r1.29 weekly
--- etc/weekly 30 Dec 2019 16:49:51 - 1.29
+++ etc/weekly 8 Feb 2020 23:13:02 -
@@ -48,7 +48,7 @@ if [ -f /var/db/locate.database ]; then
if TMP=`mktemp /var/db/locate.database.XX`; then
trap 'rm -f $TMP; exit 1' 0 1 15
UPDATEDB="/usr/libexec/locate.updatedb"
- echo "${UPDATEDB} --fcodes=- --tmpdir=${TMPDIR:-/tmp}" | \
+ echo "${UPDATEDB} --fcodes=- | \
nice -5 su -m nobody 1>$TMP
if [ $? -ne 0 ]; then
echo "Rebuilding locate database failed"
