Bug#844560: check for config file existence is wrong

[Mattia Rizzolo]

On Fri, Nov 18, 2016 at 04:08:32PM +0000, Ian Jackson wrote:
> Hi.  Thanks for your attention to this mostly-cosmetic bug...

Given that you seem to be more "involved" than the average
show^Wreport-rebug-and-flee I'm going to bother you a bit more :)

> > If that the problem it could prably be solved imho in a nicer way by nesting
> > another if in the else branch to check for simple existence (by [ -e $file ])
> > and print a proper message if the file exists but it's not regular, and keep
> > the current message only for the real enoent case.
> 
> I think this way lies madness.  We already have a mechanism for
> figuring out what went wrong: bash tries to open the file, and the
> kernel figures out why it can't be opened, and tells bash an errno
> value.  Trying to replicate this logic in your script will result in
> an ever-increasing set of cases.

oh, well, I can't really think of more real cases other than
    * enoent
    * eperm/eaccess
    * eio
    * emfile
ok, that's already too many.  but really, other than enoent and eperm
(and the "not a regular file" which is not a errno thing afaik) none of
them is a real case; nvm, that's me being sloopy :)

> How about
> 
>   . "$RCFILE" || echo "W: $RCFILE could not be sourced" >&2
> 
> ?
> 
>   . "$RCFILE" ||:

bah, this one's too boring :P

> If you don't like those suggestions, how about simply changing -f to
> -e ?  That way /dev/null will just work.  And the existing message
> about nonexistence would be truthful.

I'm thinking to do both of these two suggestions.  See the attached
commit, would this suite you?  Do you have more suggestions to make this
nicer? :)

> If the `.' fails for some reason other than nonexistence (eg, the file
> exists but has wrong permissions, or it's actually a socket, or
> something) then it's probably best to bomb out, anyway.

sadly the whole thing doesn't run with set -e (yet... there are just too
many places where return codes are not checked to make me confortable on
just adding it without having enough time to deal with it).  And we
never crashed on "RCFILE not loadable", so I'm not going to make it
fatal just now; I consider such a change an interface change that I'd
rather not do, I've done already a bunch for this dev cycle.


PS: interesting reject of email coming from gmail's SMTP ;)

-- 
regards,
                        Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540      .''`.
more about me:  https://mapreri.org                             : :'  :
Launchpad user: https://launchpad.net/~mapreri                  `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-
-------------- next part --------------
commit ae56c537e2d2eab5b31ad82d588665b450428760
gpg: Signature made lun 21 nov 2016 23:37:46 CET
gpg:                using RSA key 0x0816B9E18C762BAD
gpg: Good signature from "Mattia Rizzolo <mattia at mapreri.org>" [unknown]
gpg:                 aka "Mattia Rizzolo <mapreri at gmail.com>" [unknown]
gpg:                 aka "Mattia Rizzolo <mattia at debian.org>" [unknown]
gpg:                 aka "Mattia Rizzolo <mapreri at ubuntu.com>" [unknown]
gpg:                 aka "Mattia Rizzolo <mapreri at ubuntu-it.org>" [unknown]
gpg:                 aka "[jpeg image of size 4218]" [unknown]
gpg:                 aka "[jpeg image of size 4456]" [unknown]
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.
Primary key fingerprint: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540
     Subkey fingerprint: 8B78 6878 6C33 E5C6 4C4D  0A48 0816 B9E1 8C76 2BAD
Author:     Mattia Rizzolo <mattia at debian.org>
AuthorDate: Mon Nov 21 23:37:23 2016 +0100
Commit:     Mattia Rizzolo <mattia at debian.org>
CommitDate: Mon Nov 21 23:37:41 2016 +0100

    loadconfig: be more honest about why it's not possible to load a conf file, instead of just saying "it doesn't exist"
    
    Closes: #844560

diff --git a/pbuilder-loadconfig b/pbuilder-loadconfig
index 6f03c96..713fddd 100644
--- a/pbuilder-loadconfig
+++ b/pbuilder-loadconfig
@@ -24,11 +24,11 @@ for RCFILE in \
     "$PBUILDER_PKGDATADIR"/pbuilderrc \
     "$PBUILDER_SYSCONFDIR"/pbuilderrc \
     "$HOME"/.pbuilderrc; do
-    if [ -f "$RCFILE" ]; then 
-	. "$RCFILE"
+    # log() is not available
+    if [ -e "$RCFILE" ]; then
+        . "$RCFILE" || echo "W: $SRCFILE could not be loaded" >&2
     else
-	# log() is not available
-	echo "W: $RCFILE does not exist" >&2
+        echo "W: $RCFILE does not exist" >&2
     fi
     # deprecated in v0.216
     if [ -n "$PKGNAME_LOGFILE_EXTENTION" ] ; then
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pbuilder-maint/attachments/20161121/630525a4/attachment.sig>