[PATCH] Adding libldm: Manager for Windows dynamic disks including software RAID. It creates device mapper entries for dynamic disks allowing them to be mounted.

  • Open
  • quality assurance status badge
Details
2 participants
  • Lukasz Olszewski
  • Ludovic Courtès
Owner
unassigned
Submitted by
Lukasz Olszewski
Severity
normal
L
L
Lukasz Olszewski wrote on 4 Sep 2022 23:42
(address . guix-patches@gnu.org)
CACwB4R5Nype6=UppoXOZZO234AM5CjA=Opad4hyJNM0WChqKcQ@mail.gmail.com
---
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

Toggle diff (141 lines)
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
L
L
Lukasz Olszewski wrote on 11 Sep 2022 00:02
(address . 57590@debbugs.gnu.org)
CACwB4R6sE13TGmwTUoi08ptKgGpxi77hwYZiSZ9hhkyfwTwxvg@mail.gmail.com
Adding licensing headers to two new files.

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

Toggle diff (54 lines)
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@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@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
L
L
Ludovic Courtès wrote on 17 Oct 2022 10:03
(name . Lukasz Olszewski)(address . dev@lukaszolszewski.info)(address . 57590@debbugs.gnu.org)
87tu42ho62.fsf@gnu.org
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@lukaszolszewski.info> skribis:

Toggle quote (4 lines)
> ---
> 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.)

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

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

Toggle quote (3 lines)
> + (arguments
> + '(#:tests? #f

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

Toggle quote (2 lines)
> + #:parallel-build? #t

This is unnecessary.

Toggle quote (6 lines)
> + #: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.

Toggle quote (4 lines)
> + (replace 'bootstrap
> + (lambda _
> + (invoke "autoreconf" "-fiv"))))))

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

Toggle quote (2 lines)
> + (license license:gpl3)))

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

Toggle quote (10 lines)
> +(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…).

Toggle quote (8 lines)
> +(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?

Toggle quote (10 lines)
> +(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’.
L
L
Lukasz Olszewski wrote on 17 Oct 2022 13:12
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 57590@debbugs.gnu.org)
CACwB4R4RUGdOCmo=7tsMAF-2Kj_tVZ5sFSn1tSv_mESKUfJ7hg@mail.gmail.com
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@gnu.org> wrote:

Toggle quote (20 lines)
> 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@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.


Toggle quote (7 lines)
> > +++ 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.


Toggle quote (11 lines)
> > + (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?


Toggle quote (10 lines)
> > + #: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.


Toggle quote (7 lines)
> > + (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.


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

Ok, will do.


Toggle quote (14 lines)
> > +(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.


Toggle quote (11 lines)
> > +(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


Toggle quote (17 lines)
> > +(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.


Toggle quote (3 lines)
> Could you send updated patches?
>

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


Toggle quote (6 lines)
>
> Thanks in advance!
>
> Ludo’.
>

Regards,
Lukasz

Toggle quote (1 lines)
>
Attachment: file
L
L
Ludovic Courtès wrote on 18 Oct 2022 17:20
(name . Lukasz Olszewski)(address . dev@lukaszolszewski.info)(address . 57590@debbugs.gnu.org)
878rldma4w.fsf@gnu.org
Hi,

Lukasz Olszewski <dev@lukaszolszewski.info> skribis:

Toggle quote (11 lines)
>> > +++ 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.

Toggle quote (7 lines)
>> > + #:parallel-build? #t
>>
>> This is unnecessary.
>>
>
> Are parallel builds enabled by default?

Yes, that’s why.

Toggle quote (10 lines)
>> > + (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’.
?