Re: [Gnu-arch-users] [HEY TOM!] unhelpful error for unreadable directories

[Tom Lord]

    > From: Aaron Bentley <address@hidden>

    > Tom Lord wrote:
    > >     > From: Aaron Bentley <address@hidden>

    > >     >> But this subdir stuff:  you're not talking about "saving the 
day".
    > >     >> You're talking about saving somebody, what, say an average of 10m
    > >     >> puzzling over a rare, confusing error message?

    > >     > No, since this error will typically happen during or before
    > >     > their tla import, these will tend to be new users.  And they'll
    > >     > conclude that tla is borked and chuck it.

    > > FUD is not an argument.

    > You were saying the error was a trivial thing.  Maybe it's rare, but 
    > I've encountered it too.  Maybe it's 10 minutes once you know what to 
    > expect, but it took longer than 10 minutes for me to sort out.  And when 
    > it came up in IRC, the other guy who was helping went in an utterly 
    > wrong direction.  (He thought it meant that the tree root was missing 
    > {arch})

    > Maybe I overstated it, but I was reacting to what I felt was an 
    > understatement.

Let's be clear about what we disagree about.   

We agree that the existing error message for this case is crap.  It
doesn't provide enough information to figure out what the heck is
going on.   The existing code is:

  ctl_file = file_name_in_vicinity (0, path, "{arch}");

  if (0 > vu_lstat (&errn, ctl_file, &stat_buf))
    {
      if (errn == ENOENT)
        {
          lim_free (0, ctl_file);
          return 0;
        }
      else
        {
          panic ("i/o error in arch_inventory_traversal/is_nested_tree");
          return 0;             /* notreached */
        }
    }

and an improvement would be to replace the panic with:


   /* untested code */

   safe_printfmt (2, "unable to lstat %s\n   error: (%d) %s\n",
                  ctl_file, errn, errno_to_string (errn));
   exit (2);
   return 0;

or something similar.

We disagree (apparently) about adding new system calls to try to
second guess for the OS why the lstat failed or might fail --
specifically testing for searchability of `path'.  I say "don't do
that" because if you do you'll be creating a new catastrophic failure
mode -- the command will fail in some environments when it would not
fail if the extra test were not present.  This is analogous to a
"check oil" light on a car that shuts down the engine and refuses to
let you start it again until the control computer of the car has been
manually reset.  You also say "newbies will chuck it, otherwise" which
I consider to be unsubstantiated FUD.

Your own anecdote contradicts your FUD.   The newbie didn't chuck it
even with the old crappy error message.   The newbie asked for help on
IRC.   You could try to say "most newbies won't do that" or "irc isn't
always helpful" but at that point we're getting into pure speculation
about what might or might not happen.    In essense, you're just
making up a problem out of thin air and trying to make it sound
important by asserting it has something to do with new adopters.


    > Arch is powerful, and I want it to succeed.  But in spots, it
    > has poor usability, and I think that could hold it back, so I'm
    > trying to improve that.

And nobody disagrees or is trying to stop or discourage you -- just to
make sure that your proposed improvements are, in fact, improvements.


    >>> And if they get an error message that tla was unable to test for the 
    >>> existance of {arch} in subdirectory that isn't supposed to have one

    >>> a) they may conclude that tla requires an {arch} in every subdir

    >> Then write the error message carefully to avoid that confusion.

    > That's why I asked for your help.

Which I gave.   We got hung up when you seemed to insist on adding new
fs tests to that code in spite of the reasons I gave against them,
using FUD as your rationale.


    > >     > b) they may conclude that tla is borked and chuck it

    > > I stopped reading here.

    > That's disappointing, as I did spend some time and thought on it.

I'll go back to it then.   From that earlier message:

    >> And at what cost?  Eventually, your proposed test will fail
    >> _incorrectly_ and tla will abort when it doesn't have to.  

    > No.  I was proposing testing for unsearchability only if I
    > couldn't stat $foo/{arch}.

No, you weren't.  You started off that way several messages back but
when I pointed out problems with it you changed your proposal thusly:

    > One way to address the problem would be to put a test for
    > readability before the stat.  That would be a pessimization, but
    > it would have the same behaviour as doing the test after the
    > stat if the stat failed.

I pointed out the problems with your revised proposal and that's when
you jumped into FUD.



    > I came up with the following message, for the case where the stat errno 
    > is EACCES:
    > "Cannot access contents of directory $foo"

    > Unfortunately, this assumes that $foo is a directory, and I don't have a 
    > guarantee that it is.  I think that:

    > "Permission denied attempting to check attributes of $foo/{arch}"

    > will be misleading, but if that's the best we can do, it's
    > better than what we currently have.  I suppose we could add "(is
    > $foo searchable?)"  for the EACCES case...


That message is ok.  I like the more literal one I gave above, as
well.  I'm against the proposed parenthetical comment on the grounds
that: (a) states the obvious -- programmers working on unix systems
should be aware enough of the permissions system that that question
occurs to them; (b) overreaches -- it may be incorrect in environments
with non-standard access control systems or otherwise exotic
filesystems.


-t