Bug#864181: Fwd: Bug#864181: os-prober: dmraid detection not functional.

[Philip Hands]

Mike Mestnik  writes:

> In that case the proposed patch is wrong, dmraid is run every time the
> file exists.  Not only is the conditional in test wrong, but the file
> is created when it should be being removed.

You appear to be reading the || after the -f test as &&

To render those lines into english, one would have:

  Either the file exists OR
 we create it now

Cheers, Phil.
-- 
|)|  Philip Hands  [+44 (0)20 8530 9560]  HANDS.COM Ltd.
|-|  http://www.hands.com/http://ftp.uk.debian.org/
|(|  Hugo-Klemm-Strasse 34,   21075 Hamburg,GERMANY


signature.asc
Description: PGP signature

Bug#864181: Fwd: Bug#864181: os-prober: dmraid detection not functional.

[Mike Mestnik]

In that case the proposed patch is wrong, dmraid is run every time the
file exists.  Not only is the conditional in test wrong, but the file
is created when it should be being removed.

Bug#864181: Fwd: Bug#864181: os-prober: dmraid detection not functional.

[Philip Hands]

Mike Mestnik  writes:

> This does look better, I love the use of operators over if statements.
> I don't think using a temp file is necessary here.  I also wish the
> regex wouldn't ever match a device containing the name of another
> device, in that it should match the surrounding bits.
>
> { dmraid -r -c 2>/dev/null || true } | grep -q "$device" && return 0

The use of the temporary file is there to stop the repeated running of
dmraid, which was the sole purpose of the commit that introduced the
bug, so if one gets rid of that one might as well just revert that
commit.

I agree that there appears to be the potential for $device to match
inappropriately, but I'd need to know something about what dmraid
outputs in order to improve on the grep (I don't have a sataraid machine
to test it on).  Adding a -F option might be a good start.

Cheers, Phil.
-- 
|)|  Philip Hands  [+44 (0)20 8530 9560]  HANDS.COM Ltd.
|-|  http://www.hands.com/http://ftp.uk.debian.org/
|(|  Hugo-Klemm-Strasse 34,   21075 Hamburg,GERMANY


signature.asc
Description: PGP signature

Bug#864181: Fwd: Bug#864181: os-prober: dmraid detection not functional.

[Mike Mestnik]

This does look better, I love the use of operators over if statements.
I don't think using a temp file is necessary here.  I also wish the
regex wouldn't ever match a device containing the name of another
device, in that it should match the surrounding bits.

{ dmraid -r -c 2>/dev/null || true } | grep -q "$device" && return 0

Bug#864181: os-prober: dmraid detection not functional.

[Philip Hands]

Mike Mestnik  writes:

> Package: os-prober
> Version: 1.75
> Severity: normal
>
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> Here is the code as found in os-prober:17
> : >"$OS_PROBER_TMP/dmraid-map"
> DMRAID=$(type dmraid >/dev/null 2>&1 || true)
> if [ "$DMRAID" ]; then
> dmraid -r -c >"$OS_PROBER_TMP/dmraid-map"
> fi
>
> The problem is that $DMRAID will always be empty because stdout is
> redirected.

That was introduced by commit 9d89a525.

It seems untidy to have that done outside of on_sataraid().

I've just pushed an alternative approach here:

  
https://anonscm.debian.org/cgit/d-i/os-prober.git/commit/?h=pu/bug-864181=ebf32d6e0ba1d77a0644b57e59070bfa542cb62b

If you compare that to the parent of 9d89a525 you get:

=-=-=-
diff --git a/os-prober b/os-prober
index 0e51682..e0e1a1b 100755
--- a/os-prober
+++ b/os-prober
@@ -18,7 +18,12 @@ on_sataraid () {
type dmraid >/dev/null 2>&1 || return 1
local parent="${1%/*}"
local device="/dev/${parent##*/}"
-   if dmraid -r -c | grep -q "$device"; then
+
+   local mapcache="$OS_PROBER_TMP/dmraid-map"
+   [ -f "$mapcache" ] ||
+   dmraid -r -c >"$mapcache" 2>/dev/null || true
+
+   if grep -q "$device" "$mapcache"; then
return 0
fi
return 1
=-=-=-

which strikes me as rather neater.

Cheers, Phil.

P.S. on reflection, perhaps the test should be -e rather than -f, but it
_really_ shouldn't matter.
-- 
|)|  Philip Hands  [+44 (0)20 8530 9560]  HANDS.COM Ltd.
|-|  http://www.hands.com/http://ftp.uk.debian.org/
|(|  Hugo-Klemm-Strasse 34,   21075 Hamburg,GERMANY


signature.asc
Description: PGP signature

Bug#864181: os-prober: dmraid detection not functional.

[Mike Mestnik]

Package: os-prober
Version: 1.75
Severity: normal

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Here is the code as found in os-prober:17
: >"$OS_PROBER_TMP/dmraid-map"
DMRAID=$(type dmraid >/dev/null 2>&1 || true)
if [ "$DMRAID" ]; then
dmraid -r -c >"$OS_PROBER_TMP/dmraid-map"
fi

The problem is that $DMRAID will always be empty because stdout is redirected.

- -- System Information:
Debian Release: 8.7
  APT prefers stable
  APT policy: (500, 'stable'), (490, 'testing'), (480, 'unstable'), (470, 
'experimental')
Architecture: i386 (x86_64)

Kernel: Linux 4.2.0-19-generic (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages os-prober depends on:
ii  grub-common  2.02~beta3-5
ii  libc62.24-9

os-prober recommends no packages.

os-prober suggests no packages.

- -- no debconf information

-BEGIN PGP SIGNATURE-
Version: GnuPG v1

iQIcBAEBAgAGBQJZNHjXAAoJEOPRqa2O3KuKcVMP/0Ad2IjgqvOCVuQgu3aSmc4P
390JFx8OPEBNh0C8OAEy+1d4EhKi2n50nvMUBy8Kg3iNpKBdwEgTAnGe3P35wkHZ
QKW4deSvHLg0jutuodiLzANTdc7UKGwpF1js3oEB73Yrg1WOyj+0uLGMGnwnu3a7
uYWzuwyZlRaBenMAFD52Uov6zUNo7i/LTUwUIXI4qzGtycHkYVZhLPe6XolnfK/w
BMugsTxWC4a7Y5d0WK/eQn1qEkqaB5NHV3OWgVnzhKqSAZa3ucnSyETAHgp/aJZT
S2+WNsNEC3t1nZdQz5gmzK1bGn6AmmSIS1RMO2n20Ih/e+7gbfbqSo3WETwFuX+o
LGefi+PFp5Jv9524e2T2DTPwfTFfvaes2+L5NFlvWV6oYf2rXLdt6Ky5wWJJBhg6
illjQVOGAwwkbEdB3xlv+zjx91vgrbQKhE2XN2eHcM0xIhd84BEuvnyOBK7BL07Z
PuF0+FfAHNYi/jra6Q+0Ddtuc2QS/tkEJ+kYCn5TU+c6d0265sBCM3lQueNj4Tdz
4lPsd6AHOYL7l03XQ0i0+IifnYWAV97l2oauKToIujaBcTfJ9VoDS++Jik1UrXwE
o9KNQJQ8gq7wRpeKTZr+fmiUulfvXId2ETXXnSTbfJvv2hJJpjtGBnq1jHsWb219
8DgV9lKM1ys/brYwq/NU
=aTuF
-END PGP SIGNATURE-