GNU bug report logs - #40032
[PATCH] gnu: Add nvme-cli

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 40032 in the body.
You can then email your comments to 40032 AT debbugs.gnu.org in the normal way.

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

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

From: Vincent Legoll <vincent.legoll <at> gmail.com>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add nvme-cli
Date: Thu, 12 Mar 2020 02:13:06 +0100

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

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

[0001-gnu-Add-nvme-cli.patch (text/x-patch, attachment)]

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

From: Leo Famulari <leo <at> famulari.name>
To: Vincent Legoll <vincent.legoll <at> gmail.com>
Cc: 40032 <at> debbugs.gnu.org
Subject: Re: [bug#40032] [PATCH] gnu: Add nvme-cli
Date: Thu, 12 Mar 2020 14:15:13 -0400

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.

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

From: Vincent Legoll <vincent.legoll <at> gmail.com>
To: Leo Famulari <leo <at> famulari.name>
Cc: 40032 <at> debbugs.gnu.org
Subject: Re: [bug#40032] [PATCH] gnu: Add nvme-cli
Date: Sat, 14 Mar 2020 00:14:41 +0100

I'll investigate further and amend the patch

On Thu, Mar 12, 2020 at 7:15 PM Leo Famulari <leo <at> 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

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

From: Vincent Legoll <vincent.legoll <at> gmail.com>
To: Leo Famulari <leo <at> famulari.name>
Cc: 40032 <at> debbugs.gnu.org
Subject: Re: [bug#40032] [PATCH] gnu: Add nvme-cli
Date: Sun, 15 Mar 2020 18:07:44 +0100

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 <at> gmail.com> wrote:
>
> I'll investigate further and amend the patch
>
> On Thu, Mar 12, 2020 at 7:15 PM Leo Famulari <leo <at> 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

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

From: Leo Famulari <leo <at> famulari.name>
To: Vincent Legoll <vincent.legoll <at> gmail.com>
Cc: 40032 <at> debbugs.gnu.org
Subject: Re: [bug#40032] [PATCH] gnu: Add nvme-cli
Date: Sun, 15 Mar 2020 13:22:42 -0400

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?

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

From: Vincent Legoll <vincent.legoll <at> gmail.com>
To: Leo Famulari <leo <at> famulari.name>
Cc: 40032 <at> debbugs.gnu.org
Subject: Re: [bug#40032] [PATCH] gnu: Add nvme-cli
Date: Sun, 15 Mar 2020 18:36:54 +0100

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

Like the following ?

On Sun, Mar 15, 2020 at 6:22 PM Leo Famulari <leo <at> famulari.name> wrote:
>
> 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

[0001-gnu-Add-nvme-cli.patch (text/x-patch, attachment)]

Message #25 received at 40032-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Vincent Legoll <vincent.legoll <at> gmail.com>
Cc: 40032-done <at> debbugs.gnu.org
Subject: Re: [bug#40032] [PATCH] gnu: Add nvme-cli
Date: Sun, 15 Mar 2020 13:57:22 -0400

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 <at> 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.

Message #28 received at 40032-done <at> debbugs.gnu.org (full text, mbox):

From: Vincent Legoll <vincent.legoll <at> gmail.com>
To: Leo Famulari <leo <at> famulari.name>
Cc: 40032-done <at> debbugs.gnu.org
Subject: Re: [bug#40032] [PATCH] gnu: Add nvme-cli
Date: Sun, 15 Mar 2020 19:10:18 +0100

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 <at> famulari.name> wrote:
>
> 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 <at> 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

Message #31 received at 40032-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Vincent Legoll <vincent.legoll <at> gmail.com>
Cc: 40032-done <at> debbugs.gnu.org
Subject: Re: [bug#40032] [PATCH] gnu: Add nvme-cli
Date: Sun, 15 Mar 2020 14:21:00 -0400

On Sun, Mar 15, 2020 at 07:10:18PM +0100, Vincent Legoll wrote:
> Thanks for all the fixes, I'll try to get better at doing that myself...

No worries! We're glad to have your patches :)

Previous Next

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