#729 Issue `closed`: Confused about the meaning of the has_binary and get_path functions¶

Labels: enhancement, documentation, cleanup, fixed / solved / done

jsmeix opened issue at 2015-12-02 16:11:¶

In https://github.com/rear/rear/pull/728 I did

has_binary wipefs && REQUIRED_PROGS=( "${REQUIRED_PROGS[@]}" wipefs )

based on what I found in 26_crypt_layout.sh

if ! has_binary cryptsetup; then
    return
fi
...
REQUIRED_PROGS=( "${REQUIRED_PROGS[@]}" cryptsetup )

In _input-output-functions.sh there is

# Check if any of the binaries/aliases exist
has_binary() {
        for bin in $@; do
                if type $bin >&8 2>&1; then
                        return 0
                fi
        done
        return 1
}

get_path() {
        type -p $1 2>&8
}

Because plain "type" without any option is used
the has_binary function also results true e.g. for aliases
(as mentioned in its comment) and in general for
anything that can be called from a bash prompt
(also functions).

Accordingly I assume the has_binary function is not the right one
to find out if its argument is an executable file that can be added
to the REQUIRED_PROGS array.

It seems get_path is better but even that has strange effects
because "type -p" may output nothing with zero exit code
in some cases:

# type -a test
test is a shell builtin
test is /usr/bin/test
test is /usr/bin/X11/test

# type -p test && echo y
y

# type -P test && echo y
/usr/bin/test
y

I think the only fail-safe way to test if something is an executable file that can be added to the REQUIRED_PROGS array is "test -P".

jsmeix commented at 2015-12-02 16:12:¶

@gdha @schlomo
could you tell more about the intended usage of the has_binary and get_path functions?

schlomo commented at 2015-12-02 16:45:¶

has_binary helps you to know if there is a program that you can call. You don't care if it is a shell alias or a file on disk or a shell function, you just want to know if you can call it.

get_path returns you the file path of a program on disk if it is in $PATH

jsmeix commented at 2015-12-03 13:52:¶

Simply put:

REQUIRED_PROGS=( "${REQUIRED_PROGS[@]}" kill )

does not work (no "bin/kill" in recovery system)
while

REQUIRED_PROGS=( "${REQUIRED_PROGS[@]}" /usr/bin/kill )

works.

Details:

My point is that get_path (i.e. "type -p") returns the file path of its argument on disk if it is in $PATH but only when its argument is of type "file".

In contrast to "type -P" that returns the file path of its argument on disk if such a file exists in $PATH.

Accordingly I wonder if "type -p" or "type -P" should be used for the elements in the REQUIRED_PROGS array.

Currently get_path (i.e. "type -p") is used in 39_copy_binaries_libraries.sh

# calculate binaries from needed progs
declare -a BINARIES=( $(
for bin in "${PROGS[@]}" "${REQUIRED_PROGS[@]}"; do
        file="$(get_path "$bin")"
        if [[ -x "$file" ]]; then
                echo $file
                echo "Found $file" >&8
        fi
done | sort -u) )

For normal cases when for an element in REQUIRED_PROGS only an executable file exists, that file gets copied into the recovery system.

But for an exceptional case when for an element in REQUIRED_PROGS both a function or builtin and an executable file exists, then nothing gets copied into the recovery system.

I wonder if that behaviour is right or wrong.

In general when for an element in REQUIRED_PROGS both a function or builtin and an executable file exists one could assume that then those function or builtin is (somehow) automatically available in the recovery system.

This is in particular true for the usual functions in rear (but those functions are normally not specified in REQUIRED_PROGS).

But should we rely on it?

E.g. when a builtin exists but that does not provide features of an executable file so that the executable file is explicitly required.

I think the actual root issue here is that when for an element in REQUIRED_PROGS not only an executable file exists but something else too (e.g. a function or a builtin).

I think the actual problem is what to do if an element in REQUIRED_PROGS does not identify a unique executable.

I think it is impossible to automatically do the right thing in such cases.

Therefore I would like to implement a warning message that is shown if an element in REQUIRED_PROGS does not identify a unique executable.

I would like to show a warning message if "type -a" does not result one single line for an element in REQUIRED_PROGS like:

'kill' specified in REQUIRED_PROGS is ambiguous:
kill is a shell builtin
kill is /usr/bin/kill
kill is /bin/kill
Specify it with full path when you need the executable file.

But on the other hand I know your ( @schlomo ) opinion about warning messages and I agree with you in general.

But currently I have no better idea here.

I think silently ignoring ambiguities (as it is now) and let the user find out later the hard way when it does not work is worse than a warning message in this particular case.

schlomo commented at 2015-12-03 14:05:¶

IMHO kill is not really a valid example. As ReaR actually requires Bash >= 3 to work we always "ship" the Bash internal commands.

If a user would have a personal shell function mykill and include that in REQUIRED_PROGS then indeed it would not work.

Maybe we can fix that problem when it comes up? Or do you have a specific problem that needs fixing.

Using type -P might indeed be a good idea. Since which Bash version is that supported?

jsmeix commented at 2015-12-03 14:40:¶

I have no specific problem that needs fixing
so that there is not any kind of pressure here.
Accordingly I did not set the label "bug" for this issue
but only "enhancement" (and "documentation" if needed).

I think "kill" is a valid example at least on my SLE12 system because:

# help kill
kill: kill [-s sigspec | -n signum | -sigspec] pid | jobspec ... or kill -l [sigspec]
...
    Options:
      -s sig
...
      -n sig
...
      -l

versus

# man kill
...
SYNOPSIS
       kill [-signal|-s signal|-p] [-q value] [-a] [--] pid|name...
       kill -l [number] | -L
...
OPTIONS
       -s, --signal signal
...
       -l, --list [number]
...
       -L, --table
...
       -a, --all
...
       -p, --pid
...
       -q, --queue value

I.e. the kill builtin and /usr/bin/kill support different options.

Meanwhile I also think "type -P" is the right thing.

Reasoning:

When the user specified

REQUIRED_PROGS=( "${REQUIRED_PROGS[@]}" kill )

it is reasonable to assume that his explicit "kill" therein does not mean
the builtin but the executable file:

# type -P kill
/usr/bin/kill

Even if his explicit "kill" therein did mean the builtin, having /usr/bin/kill additionally in the recovery system cannot do any harm because a simple "kill" calls the builtin while /usr/bin/kill would have to be explicitly called with its path.

In the end I think at least in 39_copy_binaries_libraries.sh it is possible to do automatically "the right thing" via

# calculate binaries from needed progs
declare -a BINARIES=( $(
for bin in "${PROGS[@]}" "${REQUIRED_PROGS[@]}"; do
        file="$(type -P "$bin")"
        if [[ -x "$file" ]]; then
                echo $file
                echo "Found $file" >&8
        fi
done | sort -u) )

Currently I do not know if there could be regressions at unexpected places if we change get_path in _input-output-functions.sh to

get_path() {
        type -P $1 2>&8
}

According to your ( @schlomo ) above description in https://github.com/rear/rear/issues/729#issuecomment-161360329

get_path returns you the file path of a program on disk if it is in $PATH

it matches exactly "type -P" (but not "type -p" in any case):

# help type
type: type [-afptP] name [name ...]
...
    If the -p flag is used, `type' either returns the name of the disk
    file that would be executed, or nothing if `type -t NAME' would not
    return `file'.
...
    The -P flag forces a PATH search for each NAME, even if it is an alias,
    builtin, or function, and returns the name of the disk file that would
    be executed.

This is the "help type" output of bash 3.1.17 in SLE10.

jsmeix commented at 2015-12-03 14:48:¶

In the bash-2.05b sources (we have that in SLE9) I found builtins/type.def that contains:

... type [-afptP] name [name ...]
...
If the -p flag is used, `type' either returns the name of the disk
file that would be executed, or nothing if `type -t NAME' would not
return `file'.
...
The -P flag forces a PATH search for each NAME, even if it is an alias,
builtin, or function, and returns the name of the disk file that would
be executed.

schlomo commented at 2015-12-03 14:53:¶

@jsmeix did we already define the oldest OS version we support? That would give you where to check.

BTW, I just realized another probable reason for having get_path and has_binary:

get_path should be used to copy programs to the rescue system. Here we need the actual path of the file in order to copy it.
has_binary should be used to decide if a a feature is available, e.g. wipefs. For that we don't care if the program is an alias, a function or an actual file on disk. We just want to know if we can call that program and don't care about the implementation. Very useful to allow users to supply a missing program with a Bash function in ReaR that will do the same job.

I fully understand your arguments. And I am not against using "type -P" if it works also on the oldest supported OS (Thanks for checking SLE9!!). Feel free to make a pull request, as usual.

Let's close this issue till it becomes an actual problem (it helps us all if "open" issues are issues where we still need to do something).

Feel free to reopen it if we should do something.

jsmeix commented at 2015-12-03 15:10:¶

I made my pull request simultaneosly with your comment.

If you like, accept the pull request.

I think using "type -P" instead of "type -p" for get_path()
is a real improvement towards more fail-safe operation.

Even if it is only needed in exceptional cases - but usually
the special use cases are the ones that cause trouble.

jsmeix commented at 2015-12-03 15:12:¶

I think I remember official support for some old distributions was dropped. I think @gdha has mentioned it in the documentation...

jsmeix commented at 2015-12-03 15:16:¶

I found in doc/rear-release-notes.txt

Supported Operating Systems
Rear-1.17 is supported on the following Linux based
operating systems:
  Fedora 20, 21 and 22
  RHEL 5, 6 and 7
  CentOS 5, 6 and 7
  ScientificLinux 6 and 7
  SLES 11 and 12
  OpenSuSe 11, 12 and 13
  Debian 6, 7 and 8
  Ubuntu 12, 13, 14 and 15
Rear-1.17 dropped officially support for the following
Linux based operating systems:
  Fedora <20
  RHEL 3 and 4
  SLES 9 and 10
  OpenSuSe <11
  Debian <6
  Ubuntu <12
If you require support for unsupported Linux Operating System
you must acquire a rear support contract (per system).

jsmeix commented at 2015-12-03 16:50:¶

With https://github.com/rear/rear/pull/730 the isue should be fixed.

[Export of Github issue for rear/rear.]

#729 Issue closed: Confused about the meaning of the has_binary and get_path functions¶