#3781 closed defect (fixed)

RSB crashes in case the host as an unreadable directory in "/"

Reported by: Sebastian Huber Owned by: Sebastian Huber
Priority: normal Milestone: 5.1
Component: tool/rsb Version: 5
Severity: normal Keywords:
Cc: Blocked By:
Blocking:

Description

butrfeld@elektra:~/rtemsSMP/src/rsb/rtems$ ../source-builder/sb-set-builder --source-only-download 5/rtems-sparc
RTEMS Source Builder - Set Builder, 5 (29fab0500e22)
Traceback (most recent call last):
  File "../source-builder/sb/cmd-set-builder.py", line 26, in <module>
    setbuilder.run()
  File "/users/staff/butrfeld/rtemsSMP/src/rsb/source-builder/sb/setbuilder.py", line 674, in run
    if not check.host_setup(opts):
  File "/users/staff/butrfeld/rtemsSMP/src/rsb/source-builder/sb/check.py", line 127, in host_setup
    if not path_check(opts):
  File "/users/staff/butrfeld/rtemsSMP/src/rsb/source-builder/sb/check.py", line 115, in path_check
    elif not path.exists(p):
  File "/users/staff/butrfeld/rtemsSMP/src/rsb/source-builder/sb/path.py", line 131, in exists
    return _exists(shell(paths))
  File "/users/staff/butrfeld/rtemsSMP/src/rsb/source-builder/sb/path.py", line 124, in _exists
    return basename(p) in ['.'] + listdir(dirname(p))
  File "/users/staff/butrfeld/rtemsSMP/src/rsb/source-builder/sb/path.py", line 118, in listdir
    return os.listdir(hp)
OSError: [Errno 13] Permission denied: '/adm'

The root directory "/" looks like this:

butrfeld@elektra:/$ ls -ls
total 89
 4 drwxr-x---+   6 root root  4096 Nov  5  2018 adm
 4 drwxr-xr-x    2 root root  4096 Apr 10 06:17 bin

Change History (7)

comment:1 Changed on Aug 9, 2019 at 10:49:09 AM by Sebastian Huber

In source-builder/sb/check.py:

def path_check(opts, silent = False):
    if 'PATH' in os.environ:
        paths = os.environ['PATH'].split(os.pathsep)
        for p in paths:
            if len(p.strip()) == 0:
                if not silent:
                    log.notice('error: environment PATH contains an empty path')
                return False
            elif not options.host_windows and (p.strip() == '.' or p.strip() == '..'):
                if not silent:
                    log.notice('error: environment PATH invalid path: %s' % (p))
                return False
            elif not path.exists(p):
                if not silent and opts.warn_all():
                    log.notice('warning: environment PATH not found: %s' % (p))
            elif not path.isdir(p):
                if not silent and opts.warn_all():
                    log.notice('warning: environment PATH not a directory: %s' % (p))
    return True

The path.exists() seems to be rather complicated, why is it necessary in addition to path.isdir()? What is the overall goal of this check? Is it not the responsibility of the user to take care of his $PATH variable?

comment:2 Changed on Nov 27, 2019 at 1:52:12 PM by Sebastian Huber

This bug is still open. It is a show stopper on some systems.

comment:3 Changed on Nov 27, 2019 at 1:55:58 PM by Sebastian Huber

Owner: set to Sebastian Huber
Status: newaccepted

I suggest to remove this check. I think the RSB tries to be smarter than necessary here.

comment:4 Changed on Nov 29, 2019 at 6:34:35 AM by Sebastian Huber

I am not sure if these hard errors are really useful, e.g.

$ export PATH=::$PATH
$ ./source-builder/sb-set-builder --prefix=/build/rtems/5 5/rtems-arm --source-only-download
RTEMS Source Builder - Set Builder, 5 (f3b44b25d135)
error: environment PATH contains an empty path
error: host build environment is not set up correctly
Build FAILED

Why don't we just issue a warning? If something goes wrong the warning will show up in the logs.

comment:5 Changed on Dec 1, 2019 at 11:14:09 PM by Chris Johns

I wonder how many issues have been fixed by having this check present that users cleaned up?

If we remove this test and we get no increase in issues then the test did nothing and our users all had clean environments. An increase means it did serve a purpose. I also think your point about the RSB nannying our users this way may be a step too far so making the tests just warnings and no fatal would be a good compromise. It would let us request the output with debugging a specific user problem.

comment:6 Changed on Dec 2, 2019 at 8:48:07 AM by Sebastian Huber

Nannying fits quite well. The empty path check is not superfluous. If I remove it, then RSB fails with:

$ export PATH=::$PATH
$ ../source-builder/sb-set-builder 5/rtems-arm --source-only-download
error: unknown application load error
error: unknown application load error

comment:7 Changed on Dec 3, 2019 at 5:53:07 AM by Sebastian Huber <sebastian.huber@…>

Resolution: fixed
Status: acceptedclosed

In 784f518/rtems-source-builder:

Be more resilient against $PATH errors

Close #3781.

Note: See TracTickets for help on using tickets.