testsuite fails when /sbin missing from PATH

We could add that to the environment of cmd_runner.Popen() or just change the PATH when running the test suite. Although the former is not really necessary as when running /sbin commands we probably always use sudo (except in tests, of course)

Let's add a cmd_runner.Popen optional arg which ensures that a specific directory is amended to PATH if missing? we would use this when calling commands which might be outside of a sane PATH such as /sbin and usr/sbin or in custom directories (I'm thinking of linaro-hwpack-install in the chroot, if we move it to a non-standard directory).

On Mon, 2011-01-31 at 13:35 +0000, Loïc Minier wrote:
> Let's add a cmd_runner.Popen optional arg which ensures that a specific
> directory is amended to PATH if missing? we would use this when calling
> commands which might be outside of a sane PATH such as /sbin and
> usr/sbin or in custom directories (I'm thinking of linaro-hwpack-install
> in the chroot, if we move it to a non-standard directory).

But then how is this any different than providing the full path to the
binary when it's not in a standard location?

It allows people to have the binary anywhere in the PATH, not only where we say it must be. They might have it in /usr/sbin, or /urs/local/sbin.

utilities in /sbin or /usr/sbin are a bit special because some systems are actually setup to not include this in the PATH, and it is not strictly speaking incorrect; outside of utilities typically in sbin and usr/sbin, we should assume that the utility is found in the PATH, or that the utility is missing (or that users should fix the PATH before calling).

Oh, of course. That makes sense.

One slight variation of what you propose would be to provide the extra path as a hint to cmd_runner.Popen(), which would first look for the utility there and if not found just move on and hope it's found somewhere in the $PATH. This way we don't have to mess with the environment, although it shouldn't be a problem as we should mess only with the environment of that single Popen invocation.

Loïc, could you show the actual failures that happen?

The thing we have to be careful of here is that sudo won't pass $PATH through.

Therefore if we want to amend path in Popen() when as_root=True it needs to be
done with sudo's support for explicit environment variable setting.

Thanks,

James

Guilherme, agreed just looking into a specific directory explicitly is nicer than saving environ, changing environ, restoring environ; good idea. However I think we should try current PATH first (i.e. not prepend the dir) as this allows overriding commands.

James, I think we can assume sudo will reset the PATH and will include sbin in it; if we want to support other things than sbin, we indeed need to make sure than we pass the directory across sudo too, but sudo chroot/explicit-dir/binary should work.

James, actual error -- steps to reproduce:
* start new clean natty schroot
* install linaro-image-tools build-deps
* enter chroot from clean env
env -i schroot -c natty-f1175172-fda1-42cc-a6d6-91b7346482d5 -r /bin/bash
* run testsuite

 python -m testtools.run tests.test_suite
Tests running...
tar: Removing leading `/' from member names
tar: /tmp/tmpFISN5j: file changed as we read it
======================================================================
ERROR: linaro_media_create.tests.test_media_create.TestCreatePartitions.test_run_sfdisk_commands
----------------------------------------------------------------------
Text attachment: traceback
------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/testtools/runtest.py", line 169, in _run_user
    return fn(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/testtools/testcase.py", line 499, in _run_test_method
    return self._get_test_method()()
  File "linaro_media_create/tests/test_media_create.py", line 598, in test_run_sfdisk_commands
    stderr=subprocess.PIPE)
  File "linaro_media_create/partitions.py", line 287, in run_sfdisk_commands
    as_root=as_root)
  File "linaro_media_create/cmd_runner.py", line 45, in run

    return Popen(args, stdin=stdin, stdout=stdout, stderr=stderr)
  File "linaro_media_create/cmd_runner.py", line 60, in __init__
    super(Popen, self).__init__(args, env=env, **kwargs)
  File "/usr/lib/python2.7/subprocess.py", line 672, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1206, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory
------------
======================================================================
ERROR: linaro_media_create.tests.test_media_create.TestCreatePartitions.test_run_sfdisk_commands_raises_on_non_zero_returncode
----------------------------------------------------------------------
Text attachment: traceback
------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/testtools/runtest.py", line 169, in _run_user
    return fn(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/testtools/testcase.py", line 499, in _run_test_method
    return self._get_test_method()()
  File "linaro_media_create/tests/test_media_create.py", line 607, in test_run_sfdisk_commands_raises_on_non_zero_returncode

    stderr=subprocess.PIPE)
  File "/usr/lib/python2.7/dist-packages/testtools/testcase.py", line 318, in assertRaises
    ret = callableObj(*args, **kwargs)
  File "linaro_media_create/partitions.py", line 287, in run_sfdisk_commands
    as_root=as_root)
  File "linaro_media_create/cmd_runner.py", line 45, in run
    return Popen(args, stdin=stdin, stdout=stdout, stderr=stderr)
  File "linaro_media_create/cmd_runner.py", line 60, in __init__
    super(Popen, self).__init__(args, env=env, **kwargs)
  File "/usr/lib/python2.7/subprocess.py", line 672, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1206, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory
------------
=======================================================...

Thanks.

These are all failures in test code, so this isn't too high priority I
don't think.

It would be good to fix it in a way that stops this class of problem
hitting us later in functional code.

Thanks,

James

It's a bit trickier as I originally thought; this works:
env -u PATH python -c 'import os; os.environ["PATH"] = "foo"; from subprocess import Popen; Popen(["/usr/bin/env"]).wait()'|grep PATH

this doesn't:
env -u PATH python -c 'import os; os.environ["PATH"] = "foo"; from subprocess import Popen; Popen(["env"]).wait()'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib/python2.6/subprocess.py", line 623, in __init__
    errread, errwrite)
  File "/usr/lib/python2.6/subprocess.py", line 1141, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

because Python uses the real os.environ when running / locating the command to Popen, so we should either ensure that os.environ is sane in some bootstrap code, or we should resolve all commands that we pass to Popen() ourselves after fixing the PATH.