[PATCH] gnu: Add nvme-cli

  • Done
  • quality assurance status badge
Details
2 participants
  • Leo Famulari
  • Vincent Legoll
Owner
unassigned
Submitted by
Vincent Legoll
Severity
normal

Debbugs page

Vincent Legoll wrote 5 years ago
(address . guix-patches@gnu.org)
CAEwRq=r4yWqyJ6GEK+Bj4UibDvXkqBpphVchw7UmeTe04r7+5g@mail.gmail.com
Lightly tested (list & list-subsys commands)
on real hardware.

Guix pack'ed the binary and copied out of the
VM to run (in debian)

--
Vincent Legoll
From 9c836b206d65c0e80706c0a3730e77f508b10f43 Mon Sep 17 00:00:00 2001
From: Vincent Legoll <vincent.legoll@gmail.com>
Date: Thu, 12 Mar 2020 01:51:12 +0100
Subject: [PATCH] gnu: Add nvme-cli

* gnu/packages/linux.scm (nvme-cli): New variable.
---
gnu/packages/linux.scm | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)

Toggle diff (42 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index b6048a8cfb..d7b31103c8 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -3544,6 +3544,35 @@ IDE driver subsystem. Many external USB drive enclosures with SCSI-ATA Command
Translation (@dfn{SAT}) are also supported.")
(license (license:non-copyleft "file://LICENSE.TXT"))))
+(define-public nvme-cli
+ (package
+ (name "nvme-cli")
+ (version "1.10.1")
+ (home-page "https://github.com/linux-nvme/nvme-cli")
+ (source (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url home-page)
+ (commit (string-append "v" version))))
+ (sha256
+ (base32 "12wp2wxmsw2v8m9bhvwvdbhdgx1md8iilhbl19sfzz2araiwi2x8"))
+ (file-name (git-file-name name version))))
+ (build-system gnu-build-system)
+ (arguments
+ `(#:make-flags (list "CC=gcc")
+ #:phases (modify-phases %standard-phases
+ (delete 'configure)
+ (replace 'install
+ (lambda _
+ (zero? (system* "make" "install-spec" "PREFIX="
+ (string-append "DESTDIR=" %output))))))
+ #:tests? #f))
+ (synopsis "NVM-Express user space tooling for Linux")
+ (description "Utility to provide standards compliant tooling for NVM-Express
+drives. It was made specifically for Linux as it relies on the IOCTLs defined
+by the mainline kernel driver.")
+ (license license:gpl2+)))
+
(define-public rfkill
(package
(name "rfkill")
--
2.25.1
Leo Famulari wrote 5 years ago
(name . Vincent Legoll)(address . vincent.legoll@gmail.com)(address . 40032@debbugs.gnu.org)
20200312181513.GA8595@jasmine.lan
On Thu, Mar 12, 2020 at 02:13:06AM +0100, Vincent Legoll wrote:
Toggle quote (2 lines)
> + #:tests? #f))

Why are the tests skipped? Please, always add a code comment here for
reviewers.
Vincent Legoll wrote 5 years ago
(name . Leo Famulari)(address . leo@famulari.name)(address . 40032@debbugs.gnu.org)
CAEwRq=qtHYZ7wXT-TWfZLh837mGMb+cCgjbKOrPagdsq5-r8MQ@mail.gmail.com
I'll investigate further and amend the patch

On Thu, Mar 12, 2020 at 7:15 PM Leo Famulari <leo@famulari.name> wrote:
Toggle quote (9 lines)
>
> On Thu, Mar 12, 2020 at 02:13:06AM +0100, Vincent Legoll wrote:
> > + #:tests? #f))
>
> Why are the tests skipped? Please, always add a code comment here for
> reviewers.



--
Vincent Legoll
Vincent Legoll wrote 5 years ago
(name . Leo Famulari)(address . leo@famulari.name)(address . 40032@debbugs.gnu.org)
CAEwRq=pMDGyhQvOBMex6PXYx+2W71kZGHGx3p_v=26APXbtw8w@mail.gmail.com
It looks like the tests require the presence of /sys/devices...

Should I just add that explanation as comment ?

On Sat, Mar 14, 2020 at 12:14 AM Vincent Legoll
<vincent.legoll@gmail.com> wrote:
Toggle quote (18 lines)
>
> I'll investigate further and amend the patch
>
> On Thu, Mar 12, 2020 at 7:15 PM Leo Famulari <leo@famulari.name> wrote:
> >
> > On Thu, Mar 12, 2020 at 02:13:06AM +0100, Vincent Legoll wrote:
> > > + #:tests? #f))
> >
> > Why are the tests skipped? Please, always add a code comment here for
> > reviewers.
>
>
>
> --
> Vincent Legoll



--
Vincent Legoll
Leo Famulari wrote 5 years ago
(name . Vincent Legoll)(address . vincent.legoll@gmail.com)(address . 40032@debbugs.gnu.org)
20200315172242.GA26892@jasmine.lan
On Sun, Mar 15, 2020 at 06:07:44PM +0100, Vincent Legoll wrote:
Toggle quote (4 lines)
> It looks like the tests require the presence of /sys/devices...
>
> Should I just add that explanation as comment ?

Yeah, I would say that they require sysfs, which is not accessible from
the build environment.

One could check with "ls -l /sys" in a build phase, or check the docs:


Can you send a revised patch?
Vincent Legoll wrote 5 years ago
(name . Leo Famulari)(address . leo@famulari.name)(address . 40032@debbugs.gnu.org)
CAEwRq=pKhf5YhFc6pG8=0o28HTMJ7f_uss4UHc11YkTKQtUc2A@mail.gmail.com
Like the following ?

On Sun, Mar 15, 2020 at 6:22 PM Leo Famulari <leo@famulari.name> wrote:
Toggle quote (17 lines)
>
> On Sun, Mar 15, 2020 at 06:07:44PM +0100, Vincent Legoll wrote:
> > It looks like the tests require the presence of /sys/devices...
> >
> > Should I just add that explanation as comment ?
>
> Yeah, I would say that they require sysfs, which is not accessible from
> the build environment.
>
> One could check with "ls -l /sys" in a build phase, or check the docs:
>
> https://guix.gnu.org/manual/en/html_node/Build-Environment-Setup.html
>
> Can you send a revised patch?



--
Vincent Legoll
From 4f794f64ca5438773fc9980bbf0cf5739144dc87 Mon Sep 17 00:00:00 2001
From: Vincent Legoll <vincent.legoll@gmail.com>
Date: Thu, 12 Mar 2020 01:51:12 +0100
Subject: [PATCH] gnu: Add nvme-cli

* gnu/packages/linux.scm (nvme-cli): New variable.
---
gnu/packages/linux.scm | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)

Toggle diff (43 lines)
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 688d9eefaf..e8cc01d288 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -3577,6 +3577,36 @@ IDE driver subsystem. Many external USB drive enclosures with SCSI-ATA Command
Translation (@dfn{SAT}) are also supported.")
(license (license:non-copyleft "file://LICENSE.TXT"))))
+(define-public nvme-cli
+ (package
+ (name "nvme-cli")
+ (version "1.10.1")
+ (home-page "https://github.com/linux-nvme/nvme-cli")
+ (source (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url home-page)
+ (commit (string-append "v" version))))
+ (sha256
+ (base32 "12wp2wxmsw2v8m9bhvwvdbhdgx1md8iilhbl19sfzz2araiwi2x8"))
+ (file-name (git-file-name name version))))
+ (build-system gnu-build-system)
+ (arguments
+ `(#:make-flags (list "CC=gcc")
+ #:phases (modify-phases %standard-phases
+ (delete 'configure)
+ (replace 'install
+ (lambda _
+ (zero? (system* "make" "install-spec" "PREFIX="
+ (string-append "DESTDIR=" %output))))))
+ #:tests? #f)) ; The tests require sysfs, which is not accessible from
+ ; the build environment
+ (synopsis "NVM-Express user space tooling for Linux")
+ (description "Utility to provide standards compliant tooling for NVM-Express
+drives. It was made specifically for Linux as it relies on the IOCTLs defined
+by the mainline kernel driver.")
+ (license license:gpl2+)))
+
(define-public rfkill
(package
(name "rfkill")
--
2.25.1
Leo Famulari wrote 5 years ago
(name . Vincent Legoll)(address . vincent.legoll@gmail.com)(address . 40032-done@debbugs.gnu.org)
20200315175722.GE26892@jasmine.lan
On Sun, Mar 15, 2020 at 06:36:54PM +0100, Vincent Legoll wrote:
Toggle quote (7 lines)
> From 4f794f64ca5438773fc9980bbf0cf5739144dc87 Mon Sep 17 00:00:00 2001
> From: Vincent Legoll <vincent.legoll@gmail.com>
> Date: Thu, 12 Mar 2020 01:51:12 +0100
> Subject: [PATCH] gnu: Add nvme-cli
>
> * gnu/packages/linux.scm (nvme-cli): New variable.

Thanks! Pushed as 323841bda4e5b8f9b30626ab768aaf711ee6aabf with the
following changes that I somehow forgot to mention. There was no need to
make you revise the patch again...

Toggle quote (3 lines)
> + #:phases (modify-phases %standard-phases
> + (delete 'configure)

I added a comment about why the phase is deleted.

Toggle quote (5 lines)
> + (replace 'install
> + (lambda _
> + (zero? (system* "make" "install-spec" "PREFIX="
> + (string-append "DESTDIR=" %output))))))

I replaced (zero? (system* ...)) with (invoke ...), which raises
exceptions on failure rather than returning a boolean #f. This is "the
Guix way" for a while now:


Toggle quote (3 lines)
> + (description "Utility to provide standards compliant tooling for NVM-Express
> +drives. It was made specifically for Linux as it relies on the IOCTLs defined

And I made this first sentence into a complete sentence.
Closed
Vincent Legoll wrote 5 years ago
(name . Leo Famulari)(address . leo@famulari.name)(address . 40032-done@debbugs.gnu.org)
CAEwRq=p6ObsJiWrs-EDw7vK5BfbGCjP4Yvwvvxvh-SrMQaGBEg@mail.gmail.com
Thanks for all the fixes, I'll try to get better at doing that myself...

On Sun, Mar 15, 2020 at 6:57 PM Leo Famulari <leo@famulari.name> wrote:
Toggle quote (36 lines)
>
> On Sun, Mar 15, 2020 at 06:36:54PM +0100, Vincent Legoll wrote:
> > From 4f794f64ca5438773fc9980bbf0cf5739144dc87 Mon Sep 17 00:00:00 2001
> > From: Vincent Legoll <vincent.legoll@gmail.com>
> > Date: Thu, 12 Mar 2020 01:51:12 +0100
> > Subject: [PATCH] gnu: Add nvme-cli
> >
> > * gnu/packages/linux.scm (nvme-cli): New variable.
>
> Thanks! Pushed as 323841bda4e5b8f9b30626ab768aaf711ee6aabf with the
> following changes that I somehow forgot to mention. There was no need to
> make you revise the patch again...
>
> > + #:phases (modify-phases %standard-phases
> > + (delete 'configure)
>
> I added a comment about why the phase is deleted.
>
> > + (replace 'install
> > + (lambda _
> > + (zero? (system* "make" "install-spec" "PREFIX="
> > + (string-append "DESTDIR=" %output))))))
>
> I replaced (zero? (system* ...)) with (invoke ...), which raises
> exceptions on failure rather than returning a boolean #f. This is "the
> Guix way" for a while now:
>
> https://lists.gnu.org/archive/html/guix-devel/2017-12/msg00278.html
>
> > + (description "Utility to provide standards compliant tooling for NVM-Express
> > +drives. It was made specifically for Linux as it relies on the IOCTLs defined
>
> And I made this first sentence into a complete sentence.



--
Vincent Legoll
Closed
Leo Famulari wrote 5 years ago
(name . Vincent Legoll)(address . vincent.legoll@gmail.com)(address . 40032-done@debbugs.gnu.org)
20200315182100.GB5561@jasmine.lan
On Sun, Mar 15, 2020 at 07:10:18PM +0100, Vincent Legoll wrote:
Toggle quote (2 lines)
> Thanks for all the fixes, I'll try to get better at doing that myself...

No worries! We're glad to have your patches :)
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 40032@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 40032
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help