GNU bug report logs - #57590
[PATCH] Adding libldm: Manager for Windows dynamic disks including software RAID. It creates device mapper entries for dynamic disks allowing them to be mounted.

Previous Next

To reply to this bug, email your comments to 57590 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Lukasz Olszewski <dev <at> lukaszolszewski.info>
To: guix-patches <at> gnu.org
Subject: [PATCH] Adding libldm: Manager for Windows dynamic disks including
 software RAID. It creates device mapper entries for dynamic disks allowing
 them to be mounted.
Date: Sun, 4 Sep 2022 23:42:55 +0200

---
 gnu/packages/libldm.scm | 70 +++++++++++++++++++++++++++++++++++++++++
 gnu/services/libldm.scm | 47 +++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)
 create mode 100644 gnu/packages/libldm.scm
 create mode 100644 gnu/services/libldm.scm

diff --git a/gnu/packages/libldm.scm b/gnu/packages/libldm.scm
new file mode 100644
index 0000000000..38fb5e218e
--- /dev/null
+++ b/gnu/packages/libldm.scm
@@ -0,0 +1,70 @@
+(define-module (gnu packages libldm)
+  #:use-module (gnu packages)
+  #:use-module (guix packages)
+  #:use-module ((guix licenses)
+                #:prefix license:)
+  #:use-module (guix build-system gnu)
+  #:use-module (gnu packages base)
+  #:use-module (gnu packages autotools)
+  #:use-module (gnu packages m4)
+  #:use-module (gnu packages pkg-config)
+  #:use-module (gnu packages gnome)
+  #:use-module (gnu packages glib)
+  #:use-module (gnu packages compression)
+  #:use-module (gnu packages readline)
+  #:use-module (gnu packages linux)
+  #:use-module (gnu packages xml)
+  #:use-module (gnu packages docbook)
+  #:use-module (gnu packages gtk)
+  #:use-module (guix git-download))
+
+(define-public libldm
+  (package
+    (name "libldm")
+    (version "0.2.5")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/mdbooth/libldm.git")
+                    (commit (string-append "libldm-" version))))
+              (file-name (git-file-name name version))
+              (sha256
+               (base32
+                "08iz3kq4ci79abpyxwwqmzi3bayyk4s29n8h1jqgdgk5yskwgnrn"))))
+    (build-system gnu-build-system)
+    (inputs (list json-glib
+                  glib
+                  zlib
+                  readline
+                  lvm2
+                  libgudev))
+    (native-inputs (list which
+                         m4
+                         libtool
+                         autoconf-wrapper
+                         automake
+                         pkg-config
+                         `(,glib "bin")
+                         gtk-doc
+                         libxml2
+                         libxslt
+                         docbook-xsl))
+    (arguments
+     '(#:tests? #f
+       #:parallel-build? #t
+       #:phases (modify-phases %standard-phases
+                  (add-before 'configure 'set-env
+                    (lambda _
+                      (setenv "CONFIG_SHELL"
+                              (which "")) #t))
+                  (add-before 'bootstrap 'run-gtkdocize
+                    (lambda _
+                      (invoke "gtkdocize")))
+                  (replace 'bootstrap
+                    (lambda _
+                      (invoke "autoreconf" "-fiv"))))))
+    (home-page "https://github.com/mdbooth/libldm")
+    (synopsis "Manager for Microsoft Windows dynamic disks")
+    (description
+     "Libldm is a library for managing Microsoft Windows dynamic
disks, which use Microsoft's LDM metadata.  It can inspect them, and
also create and remove device-mapper block devices which can be
mounted.")
+    (license license:gpl3)))
diff --git a/gnu/services/libldm.scm b/gnu/services/libldm.scm
new file mode 100644
index 0000000000..00e19540f0
--- /dev/null
+++ b/gnu/services/libldm.scm
@@ -0,0 +1,47 @@
+(define-module (gnu services libldm)
+  #:use-module (guix records)
+  #:use-module (guix gexp)
+  #:use-module (guix diagnostics)
+  #:use-module (guix i18n)
+  #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-26)
+  #:use-module (ice-9 match)
+  #:use-module (guix gexp)
+  #:use-module (gnu services)
+  #:use-module (guix modules)
+  #:use-module (gnu services base)
+  #:use-module (gnu services shepherd)
+  #:use-module (gnu packages libldm)
+  #:export (libldm-configuration libldm-configuration? libldm-service-type))
+
+(define-record-type* <libldm-configuration>
+                     libldm-configuration
+                     make-libldm-configuration
+                     libldm-configuration?
+                     (package
+                       libldm-configuration-package
+                       (default libldm))
+                     (action libldm-configuration-action
+                             (default '("create" "all"))))
+
+(define (libldm-shepherd-service config)
+  "Return a <shepherd-service> for libldm with CONFIG"
+  (let* ((libldm (libldm-configuration-package config))
+         (action (libldm-configuration-action config)))
+    (list (shepherd-service (documentation
+                             "Run ldmtool to create Windows dynamic
disc device nodes at startup.")
+                            (provision '(libldmd))
+                            (one-shot? #t)
+                            (start #~(make-forkexec-constructor
(append (list (string-append #$libldm
+
         "/bin/ldmtool"))
+
  '(#$@action))))
+                            (stop #~(make-kill-destructor))))))
+
+(define libldm-service-type
+  (service-type (name 'libldm)
+                (extensions (list (service-extension
+                                   shepherd-root-service-type
+                                   libldm-shepherd-service)))
+                (default-value (libldm-configuration))
+                (description
+                 "Run ldmtool to create device nodes for Windows
dynamic discs so they can be mounted")))

base-commit: 41bce2414a286836b4071d90b660fb457ee76e32
-- 
2.36.1

Message #8 received at 57590 <at> debbugs.gnu.org (full text, mbox):

From: Lukasz Olszewski <dev <at> lukaszolszewski.info>
To: 57590 <at> debbugs.gnu.org
Subject: [PATCH] Adding libldm: Manager for Windows dynamic disks including
 software RAID. It creates device mapper entries for dynamic disks allowing
 them to be mounted.
Date: Sun, 11 Sep 2022 00:02:04 +0200

Adding licensing headers to two new files.

---
 gnu/packages/libldm.scm | 19 +++++++++++++++++++
 gnu/services/libldm.scm | 19 +++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/gnu/packages/libldm.scm b/gnu/packages/libldm.scm
index 38fb5e218e..b8f68d433d 100644
--- a/gnu/packages/libldm.scm
+++ b/gnu/packages/libldm.scm
@@ -1,3 +1,22 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2022 Lukasz Olszewski <dev <at> lukaszolszewski.info>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+
 (define-module (gnu packages libldm)
   #:use-module (gnu packages)
   #:use-module (guix packages)
diff --git a/gnu/services/libldm.scm b/gnu/services/libldm.scm
index 00e19540f0..875ebf69da 100644
--- a/gnu/services/libldm.scm
+++ b/gnu/services/libldm.scm
@@ -1,3 +1,22 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2022 Lukasz Olszewski <dev <at> lukaszolszewski.info>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+
 (define-module (gnu services libldm)
   #:use-module (guix records)
   #:use-module (guix gexp)
--
2.36.1

Message #11 received at 57590 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Lukasz Olszewski <dev <at> lukaszolszewski.info>
Cc: 57590 <at> debbugs.gnu.org
Subject: Re: bug#57590: [PATCH] Adding libldm: Manager for Windows dynamic
 disks including software RAID. It creates device mapper entries for
 dynamic disks allowing them to be mounted.
Date: Mon, 17 Oct 2022 10:03:49 +0200

Hi Lukasz,

Apologies for the delay!

I think the patch series is close to being ready; we’ll need a few
changes before we’re done.

Lukasz Olszewski <dev <at> lukaszolszewski.info> skribis:

> ---
>  gnu/packages/libldm.scm | 70 +++++++++++++++++++++++++++++++++++++++++
>  gnu/services/libldm.scm | 47 +++++++++++++++++++++++++++

Please make one patch adding the package, and another one adding the
service.

In each patch, please make sure to add the new file to gnu/local.mk (you
can check the Git history for examples.)

> +++ b/gnu/packages/libldm.scm
> @@ -0,0 +1,70 @@
> +(define-module (gnu packages libldm)

We’ll need the license/copyright header as you noted.

> +    (arguments
> +     '(#:tests? #f

Please add a comment explaining why tests are skipped.  That should be a
last resort.

> +       #:parallel-build? #t

This is unnecessary.

> +       #:phases (modify-phases %standard-phases
> +                  (add-before 'configure 'set-env
> +                    (lambda _
> +                      (setenv "CONFIG_SHELL"
> +                              (which "")) #t))

I don’t think this can work because (which "") returns #f but ‘setenv’
expects a string.

> +                  (replace 'bootstrap
> +                    (lambda _
> +                      (invoke "autoreconf" "-fiv"))))))

Is it necessary?  The default ‘bootstrap’ phase does something similar.

> +    (license license:gpl3)))

This should be ‘license:gpl3+’ because source file headers carry the “or
any later version” wording.

> +(define-record-type* <libldm-configuration>
> +                     libldm-configuration
> +                     make-libldm-configuration
> +                     libldm-configuration?
> +                     (package
> +                       libldm-configuration-package
> +                       (default libldm))
> +                     (action libldm-configuration-action
> +                             (default '("create" "all"))))

Indentation is off here (I noticed that ‘guix style’ got it wrong so I’m
fixing it now…).

> +(define (libldm-shepherd-service config)
> +  "Return a <shepherd-service> for libldm with CONFIG"
> +  (let* ((libldm (libldm-configuration-package config))
> +         (action (libldm-configuration-action config)))
> +    (list (shepherd-service (documentation
> +                             "Run ldmtool to create Windows dynamic
> disc device nodes at startup.")

Maybe s/disc/disk/ throughout for consistency?

> +(define libldm-service-type
> +  (service-type (name 'libldm)
> +                (extensions (list (service-extension
> +                                   shepherd-root-service-type
> +                                   libldm-shepherd-service)))
> +                (default-value (libldm-configuration))
> +                (description
> +                 "Run ldmtool to create device nodes for Windows
> dynamic discs so they can be mounted")))

Please add a period at the end, and write @command{ldmtool}.

One last thing: could you add documentation for the service in
doc/guix.texi, maybe under “Virtualization” or in some new section?
Please include a paragraph giving some context and an example.

Could you send updated patches?

Thanks in advance!

Ludo’.

Message #14 received at 57590 <at> debbugs.gnu.org (full text, mbox):

From: Lukasz Olszewski <dev <at> lukaszolszewski.info>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 57590 <at> debbugs.gnu.org
Subject: Re: bug#57590: [PATCH] Adding libldm: Manager for Windows dynamic
 disks including software RAID. It creates device mapper entries for dynamic
 disks allowing them to be mounted.
Date: Mon, 17 Oct 2022 13:12:13 +0200

[Message part 1 (text/plain, inline)]

Hi,

No problem at all regarding the delay. Unfortunately I've been busier than
usual in last few weeks (this is likely to continue for few more weeks).

Regarding the comments please see inline below.

On Mon, 17 Oct 2022, 10:03 Ludovic Courtès, <ludo <at> gnu.org> wrote:

> Hi Lukasz,
>
> Apologies for the delay!
>
> I think the patch series is close to being ready; we’ll need a few
> changes before we’re done.
>
> Lukasz Olszewski <dev <at> lukaszolszewski.info> skribis:
>
> > ---
> >  gnu/packages/libldm.scm | 70 +++++++++++++++++++++++++++++++++++++++++
> >  gnu/services/libldm.scm | 47 +++++++++++++++++++++++++++
>
> Please make one patch adding the package, and another one adding the
> service.
>
> In each patch, please make sure to add the new file to gnu/local.mk (you
> can check the Git history for examples.)
>

Ok, will do.


> > +++ b/gnu/packages/libldm.scm
> > @@ -0,0 +1,70 @@
> > +(define-module (gnu packages libldm)
>
> We’ll need the license/copyright header as you noted.
>

I've posted a later patch that included those, but it was posted as a patch
on top of a patch so perhaps it wasn't well visible. I'll integrate it in
the next version.


> > +    (arguments
> > +     '(#:tests? #f
>
> Please add a comment explaining why tests are skipped.  That should be a
> last resort.
>
> > +       #:parallel-build? #t
>
> This is unnecessary.
>

Are parallel builds enabled by default? Or is there a convention not to
enable then unless some requirements are met?


> > +       #:phases (modify-phases %standard-phases
> > +                  (add-before 'configure 'set-env
> > +                    (lambda _
> > +                      (setenv "CONFIG_SHELL"
> > +                              (which "")) #t))
>
> I don’t think this can work because (which "") returns #f but ‘setenv’
> expects a string.
>

I'll have to test it without. If setenv indeed fails now then it should
continue to work without it.


> > +                  (replace 'bootstrap
> > +                    (lambda _
> > +                      (invoke "autoreconf" "-fiv"))))))
>
> Is it necessary?  The default ‘bootstrap’ phase does something similar.
>

I've copied this phase from another package. If I remember correctly the
configure phase failed without. I'll have to test again to check.


> > +    (license license:gpl3)))
>
> This should be ‘license:gpl3+’ because source file headers carry the “or
> any later version” wording.
>

Ok, will do.


> > +(define-record-type* <libldm-configuration>
> > +                     libldm-configuration
> > +                     make-libldm-configuration
> > +                     libldm-configuration?
> > +                     (package
> > +                       libldm-configuration-package
> > +                       (default libldm))
> > +                     (action libldm-configuration-action
> > +                             (default '("create" "all"))))
>
> Indentation is off here (I noticed that ‘guix style’ got it wrong so I’m
> fixing it now…).
>

OK, I'll keep the above.


> > +(define (libldm-shepherd-service config)
> > +  "Return a <shepherd-service> for libldm with CONFIG"
> > +  (let* ((libldm (libldm-configuration-package config))
> > +         (action (libldm-configuration-action config)))
> > +    (list (shepherd-service (documentation
> > +                             "Run ldmtool to create Windows dynamic
> > disc device nodes at startup.")
>
> Maybe s/disc/disk/ throughout for consistency?
>

Ok


> > +(define libldm-service-type
> > +  (service-type (name 'libldm)
> > +                (extensions (list (service-extension
> > +                                   shepherd-root-service-type
> > +                                   libldm-shepherd-service)))
> > +                (default-value (libldm-configuration))
> > +                (description
> > +                 "Run ldmtool to create device nodes for Windows
> > dynamic discs so they can be mounted")))
>
> Please add a period at the end, and write @command{ldmtool}.
>
> One last thing: could you add documentation for the service in
> doc/guix.texi, maybe under “Virtualization” or in some new section?
> Please include a paragraph giving some context and an example.
>

OK, will do.


> Could you send updated patches?
>

If I don't manage to do it this week, then on the weekend.


>
> Thanks in advance!
>
> Ludo’.
>

Regards,
Lukasz

>

[Message part 2 (text/html, inline)]

Message #17 received at 57590 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Lukasz Olszewski <dev <at> lukaszolszewski.info>
Cc: 57590 <at> debbugs.gnu.org
Subject: Re: bug#57590: [PATCH] Adding libldm: Manager for Windows dynamic
 disks including software RAID. It creates device mapper entries for
 dynamic disks allowing them to be mounted.
Date: Tue, 18 Oct 2022 17:20:15 +0200

Hi,

Lukasz Olszewski <dev <at> lukaszolszewski.info> skribis:

>> > +++ b/gnu/packages/libldm.scm
>> > @@ -0,0 +1,70 @@
>> > +(define-module (gnu packages libldm)
>>
>> We’ll need the license/copyright header as you noted.
>>
>
> I've posted a later patch that included those, but it was posted as a patch
> on top of a patch so perhaps it wasn't well visible. I'll integrate it in
> the next version.

I did see it (thanks!).  It would be great though if you could send a
single “v2” patch that includes everything.

>> > +       #:parallel-build? #t
>>
>> This is unnecessary.
>>
>
> Are parallel builds enabled by default?

Yes, that’s why.

>> > +                  (replace 'bootstrap
>> > +                    (lambda _
>> > +                      (invoke "autoreconf" "-fiv"))))))
>>
>> Is it necessary?  The default ‘bootstrap’ phase does something similar.
>>
>
> I've copied this phase from another package. If I remember correctly the
> configure phase failed without. I'll have to test again to check.

Yes please.

Thanks,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.