bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

[Maxim Cournoyer]

Hello,

Recently, I noticed that 'make check-system TESTS=btrfs-root-os' stopped
working at all, even on master without my changes.

So, I've git bisected the faulty commit, and got:

--8<---------------cut here---------------start------------->8---
git bisect start
# good: [35cb2ac56cce1496c622958b53e82ba5d0c0ec48] gnu: Add queen.
git bisect good 35cb2ac56cce1496c622958b53e82ba5d0c0ec48
# bad: [a46174f457f7e558e67b7103074ee9ba92ada5dc] build: file-systems: Specify a "nfsvers" option by default for NFS.
git bisect bad a46174f457f7e558e67b7103074ee9ba92ada5dc
# bad: [532b318020748c035b43f83611899e4ba0875ae1] gnu: Add r-xmisc.
git bisect bad 532b318020748c035b43f83611899e4ba0875ae1
# bad: [7046106ef90e7855311dbb1f1ae9fed3df5cf875] gnu: upx: Clean up.
git bisect bad 7046106ef90e7855311dbb1f1ae9fed3df5cf875
# good: [a9546f8bb1fde766dc8ccd435b03aac1f720bd0f] gnu: go-github-com-certifi-gocertifi: Update to 2018.01.18-1.a5e0173
git bisect good a9546f8bb1fde766dc8ccd435b03aac1f720bd0f
# good: [53ddc6a71f6d7968734007f0b5325c66d9c6a33b] gnu: emacs-markdown-mode: Use HTTPS home page.
git bisect good 53ddc6a71f6d7968734007f0b5325c66d9c6a33b
# good: [1788bb9016a98d9c9412bc1611156d33f3f731d0] gnu: openscenegraph: Update to 3.6.5.
git bisect good 1788bb9016a98d9c9412bc1611156d33f3f731d0
# bad: [35f985cb52d7bc3d7e03d5db0a63da4dcf4d3322] gnu: guile-present: Use HTTPS home page.
git bisect bad 35f985cb52d7bc3d7e03d5db0a63da4dcf4d3322
# good: [d39885a8a9e0e03c2bf6277d475d384168bba642] gnu: python-reno: Update home page.
git bisect good d39885a8a9e0e03c2bf6277d475d384168bba642
# bad: [21f5de6d32a7d0e75ec2df69dc753a75a15657cd] gnu: openssh: Don't use NAME in source URI.
git bisect bad 21f5de6d32a7d0e75ec2df69dc753a75a15657cd
# bad: [139a9887843d0ea7dd724c4dd0d00470337370b3] gnu: shepherd: Add "guile3.0-shepherd" variant.
git bisect bad 139a9887843d0ea7dd724c4dd0d00470337370b3
# bad: [205c1e04e04b9a9338c7219ff82bd13f000fb8c8] gnu: shepherd: Update to 0.7.0.
git bisect bad 205c1e04e04b9a9338c7219ff82bd13f000fb8c8
# first bad commit: [205c1e04e04b9a9338c7219ff82bd13f000fb8c8] gnu: shepherd: Update to 0.7.0.
--8<---------------cut here---------------end--------------->8---

So, it seems that updating shepherd to 0.7.0 caused this regression
(commit 205c1e04e04b9a9338c7219ff82bd13f000fb8c8).

It probably causes other tests to fail in (gnu tests install), but I
haven't tried any.

Maxim

bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

[Ludovic Courtès]

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> So, it seems that updating shepherd to 0.7.0 caused this regression
> (commit 205c1e04e04b9a9338c7219ff82bd13f000fb8c8).

Could it be something similar to <https://bugs.gnu.org/39671>, fixed in
commit 00500449b0d53bb9219608a3cbee8faf7a3edae1?

Thanks,
Ludo’.

bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

[Maxim Cournoyer]

Hello Ludovic!

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> So, it seems that updating shepherd to 0.7.0 caused this regression
>> (commit 205c1e04e04b9a9338c7219ff82bd13f000fb8c8).
>
> Could it be something similar to <https://bugs.gnu.org/39671>, fixed in
> commit 00500449b0d53bb9219608a3cbee8faf7a3edae1?
>
> Thanks,
> Ludo’.

I've reviewed the problem linked above, and the various places we still
have setenvs before forking a process with Shepherd, and haven't found a
problematic usage so far.

I've ran the following again:

--8<---------------cut here---------------start------------->8---
guix build --verbosity=10 /gnu/store/l6p6jnwn844hyfwfazg39i7mj4v3z9ac-btrfs-root-os.drv |& strings
--8<---------------cut here---------------end--------------->8---

The |& strings part prevents SeaBIOS from resetting my terminal emulator
and loosing parts of the scroll buffer.  Not really needed here, but
handy sometimes.

The failed derivation build ends with:

marionette is ready
;;; (uname #("Linux" "gnu" "5.4.24-gnu" "#1 SMP 1" "x86_64"))
builder for `/gnu/store/s3ba2v7cl8x0v7wi8xg3xrpm5216i3jf-installation.drv' failed with exit code 1


Right after outputing what seems to be the result of evaluating
'uname'.  Opening up the builder code referenced in that derivation,
/gnu/store/jv133gqxz1wj053vjzpnk6rkr6409lv1-btrfs-root-os-builder, I
could find:

--8<---------------cut here---------------start------------->8---
(test-assert "uname"
    (match
	(marionette-eval
	 (quote
	  (uname))
	 marionette)
      (#("Linux" host-name version _ architecture)
       (and
	(string=? host-name "liberigilo")
	(string-prefix? "5.4.24" version)
	(string-prefix? architecture %host-type)))))
--8<---------------cut here---------------end--------------->8---

Comparing with the above uname output, we can see that it validates the
hostname matches against "liberigilo", yet it is "gnu"!  Perhaps
Shepherd 0.7.0 introduced some problem with the hostname service?

Maxim

bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

[Maxim Cournoyer]

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

[...]

> Comparing with the above uname output, we can see that it validates the
> hostname matches against "liberigilo", yet it is "gnu"!  Perhaps
> Shepherd 0.7.0 introduced some problem with the hostname service?

Another data point: the same happen when running './pre-inst-env make
check-system TESTS=installed-os', so this is not Btrfs specific.

bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

[Ludovic Courtès]

Hi!

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Comparing with the above uname output, we can see that it validates the
> hostname matches against "liberigilo", yet it is "gnu"!  Perhaps
> Shepherd 0.7.0 introduced some problem with the hostname service?

The “basic” test expects the name “liberigilo”, which is why all the
tests in (gnu tests install) use that name (yeah, it should probably be
a parameter somewhere…).

Could it be that the btrfs test uses “gnu” instead?

(The Shepherd doesn’t know or care about the host name.)

HTH!
Ludo’.

bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 1243 bytes --]

The build log of <https://ci.guix.gnu.org/build/2356870/details> has:

--8<---------------cut here---------------start------------->8---
guix system: [1m[0mbootloader successfully installed on '[1m/dev/vdb[0m'
+ sync
[  194.803139] udevd[313]: failed to execute '/gnu/store/pxw38aa6wwxyd2ws9pkzj4m68qrmjscd-eudev-3.2.9/lib/udev/${exec_prefix}/bin/udevadm' '${exec_prefix}/bin/udevadm trigger -s block -p ID_BTRFS_READY=0': No such file or directory
--8<---------------cut here---------------end--------------->8---

The broken udev rule is ‘64-btrfs.rules’ in eudev:

--8<---------------cut here---------------start------------->8---
# reconsider pending devices in case when multidevice volume awaits
ENV{ID_BTRFS_READY}=="1", RUN+="${exec_prefix}/bin/udevadm trigger -s block -p ID_BTRFS_READY=0"
--8<---------------cut here---------------end--------------->8---

… where “${exec_prefix}” is expanded from “@bindir@”, a classical
issue that we should report upstream.  :-)

In the meantime, the following patch should solve the problem.  I’m
running the test now, and if it passes and you’re fine with it, I’ll
commit it.

It’s pleasant we have tests to catch such issues!

Thanks,
Ludo’.


[-- Attachment #2: Type: text/x-patch, Size: 2761 bytes --]

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 64ea566dbd..3884e15233 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2013, 2014, 2015, 2016 Andreas Enge <andreas@enge.fr>
 ;;; Copyright © 2012 Nikita Karetnikov <nikita@karetnikov.org>
 ;;; Copyright © 2014, 2015, 2016, 2017, 2018, 2019, 2020 Mark H Weaver <mhw@netris.org>
@@ -2870,6 +2870,26 @@ device nodes from /dev/, handles hotplug events and loads drivers at boot
 time.")
     (license license:gpl2+)))
 
+;; TODO: Merge with eudev on the next rebuild cycle.
+(define-public eudev/btrfs-fix
+  (package/inherit
+   eudev
+   (version (string-append (package-version eudev) "-1"))
+   (arguments
+    (substitute-keyword-arguments (package-arguments eudev)
+      ((#:phases phases '%standard-phases)
+       `(modify-phases ,phases
+          (add-before 'configure 'patch-bindir-in-btrfs-rules
+            (lambda* (#:key outputs #:allow-other-keys)
+              ;; The "@bindir@" substitution incorrectly expands to a literal
+              ;; "${exec_prefix}" (see <https://bugs.gnu.org/39926>).  Work
+              ;; around it.
+              (let ((out (assoc-ref outputs "out")))
+                (substitute* "rules/64-btrfs.rules.in"
+                  (("@bindir@")
+                   (string-append out "/bin")))
+                #t)))))))))
+
 (define-public eudev-with-hwdb
   (deprecated-package "eudev-with-hwdb" eudev))
 
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 706b3ae7ec..0c8978790d 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2013, 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015, 2016 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2015, 2016 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2015 Sou Bunnbu <iyzsong@gmail.com>
@@ -1918,7 +1918,7 @@ archive}).  If that is not the case, the service will fail to start."
   udev-configuration make-udev-configuration
   udev-configuration?
   (udev   udev-configuration-udev                 ;<package>
-          (default eudev))
+          (default eudev/btrfs-fix))
   (rules  udev-configuration-rules                ;list of <package>
           (default '())))
 

bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

[Ludovic Courtès]

Hi again!

Ludovic Courtès <ludo@gnu.org> skribis:

> The broken udev rule is ‘64-btrfs.rules’ in eudev:
>
> # reconsider pending devices in case when multidevice volume awaits
> ENV{ID_BTRFS_READY}=="1", RUN+="${exec_prefix}/bin/udevadm trigger -s block -p ID_BTRFS_READY=0"
>
> … where “${exec_prefix}” is expanded from “@bindir@”, a classical
> issue that we should report upstream.  :-)
>
> In the meantime, the following patch should solve the problem.  I’m
> running the test now, and if it passes and you’re fine with it, I’ll
> commit it.

I pushed the patch as 24a4a635fd76a91463c75cfe615929ec482cc2f6.

It doesn’t solve the “btrfs-root-os” bug though.  :-/

I noticed that it’s the installation.drv that fails, not the basic.drv,
which is why we see “gnu” as the host name.  The weird thing is that it
fails after ‘reboot’, so I’m not sure what that means.  To be continued…

Thanks,
Ludo’.

bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

[Maxim Cournoyer]

Hello Ludovic!

Ludovic Courtès <ludo@gnu.org> writes:

> Hi again!
>
> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> The broken udev rule is ‘64-btrfs.rules’ in eudev:
>>
>> # reconsider pending devices in case when multidevice volume awaits
>> ENV{ID_BTRFS_READY}=="1", RUN+="${exec_prefix}/bin/udevadm trigger -s block -p ID_BTRFS_READY=0"
>>
>> … where “${exec_prefix}” is expanded from “@bindir@”, a classical
>> issue that we should report upstream.  :-)
>>
>> In the meantime, the following patch should solve the problem.  I’m
>> running the test now, and if it passes and you’re fine with it, I’ll
>> commit it.
>
> I pushed the patch as 24a4a635fd76a91463c75cfe615929ec482cc2f6.
>
> It doesn’t solve the “btrfs-root-os” bug though.  :-/
>
> I noticed that it’s the installation.drv that fails, not the basic.drv,
> which is why we see “gnu” as the host name.  The weird thing is that it
> fails after ‘reboot’, so I’m not sure what that means.  To be continued…
>
> Thanks,
> Ludo’.

I've reported this problem to upstream here:
https://github.com/gentoo/eudev/issues/182.  Thanks for working around
it.

Maxim

bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

[Ludovic Courtès]

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
> [...]
>
>> Comparing with the above uname output, we can see that it validates the
>> hostname matches against "liberigilo", yet it is "gnu"!  Perhaps
>> Shepherd 0.7.0 introduced some problem with the hostname service?
>
> Another data point: the same happen when running './pre-inst-env make
> check-system TESTS=installed-os', so this is not Btrfs specific.

Fixed by be0a672c30ad1401019abb5cb83d59be171813d0.

I wonder how this could have worked before: ‘reboot’ never gets a reply,
so it’s supposed to hang until it’s killed when the ‘user-processes’
service is stopped.

Thanks,
Ludo’.

bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

[Maxim Cournoyer]

Hi Ludo,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>>
>> [...]
>>
>>> Comparing with the above uname output, we can see that it validates the
>>> hostname matches against "liberigilo", yet it is "gnu"!  Perhaps
>>> Shepherd 0.7.0 introduced some problem with the hostname service?
>>
>> Another data point: the same happen when running './pre-inst-env make
>> check-system TESTS=installed-os', so this is not Btrfs specific.
>
> Fixed by be0a672c30ad1401019abb5cb83d59be171813d0.
>
> I wonder how this could have worked before: ‘reboot’ never gets a reply,
> so it’s supposed to hang until it’s killed when the ‘user-processes’
> service is stopped.
>
> Thanks,
> Ludo’.

Hey, weird!  Anyway, thanks for fixing it!  How did you debug this?  I'm curious.

Maxim

bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

[Ludovic Courtès]

Hi!

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hi Maxim,
>>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>
>>> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>>>
>>> [...]
>>>
>>>> Comparing with the above uname output, we can see that it validates the
>>>> hostname matches against "liberigilo", yet it is "gnu"!  Perhaps
>>>> Shepherd 0.7.0 introduced some problem with the hostname service?
>>>
>>> Another data point: the same happen when running './pre-inst-env make
>>> check-system TESTS=installed-os', so this is not Btrfs specific.
>>
>> Fixed by be0a672c30ad1401019abb5cb83d59be171813d0.
>>
>> I wonder how this could have worked before: ‘reboot’ never gets a reply,
>> so it’s supposed to hang until it’s killed when the ‘user-processes’
>> service is stopped.
>>
>> Thanks,
>> Ludo’.
>
> Hey, weird!  Anyway, thanks for fixing it!  How did you debug this?  I'm curious.

I first noticed that the installation script ran to completion and that
the VM seems to stop properly.  Then I saw our code that did:

  (zero? (marionette-eval '(system #$script) marionette))

So I just tried adding:

  (pk (marionette-eval '(system "/run/current-system/profile/sbin/reboot")
                       marionette))

to the ‘basic’ test.  I noticed it would return 15, and then I got it.

Nothing fancy, mostly stubbornness and perseverance.  :-)

Ludo’.

bug#39926: Regression introduced by Shepherd 0.7.0 ('make check-system TESTS=btrfs-root-os' fails)

[Maxim Cournoyer]

Hi!

Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> Hey, weird!  Anyway, thanks for fixing it!  How did you debug this?  I'm curious.
>
> I first noticed that the installation script ran to completion and that
> the VM seems to stop properly.  Then I saw our code that did:
>
>   (zero? (marionette-eval '(system #$script) marionette))
>
> So I just tried adding:
>
>   (pk (marionette-eval '(system "/run/current-system/profile/sbin/reboot")
>                        marionette))
>
> to the ‘basic’ test.  I noticed it would return 15, and then I got it.
>
> Nothing fancy, mostly stubbornness and perseverance.  :-)

I see! Good old pk :-). Thanks for sharing the process used!

Maxim