[PATCH 1/6] gnu: Add picocom.

[PATCH 1/6] gnu: Add picocom.

[David Craven]

* gnu/packages/terminals.scm (picocom): New variable.
---
 gnu/packages/terminals.scm | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/gnu/packages/terminals.scm b/gnu/packages/terminals.scm
index d5c24d2..7ab4559 100644
--- a/gnu/packages/terminals.scm
+++ b/gnu/packages/terminals.scm
@@ -259,3 +259,38 @@ multi-seat support, a replacement for @command{mingetty}, and more.")
                    license:gpl2+))
     (supported-systems (filter (cut string-suffix? "-linux" <>)
                                %supported-systems))))
+
+(define-public picocom
+  (package
+    (name "picocom")
+    (version "2.1")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append
+                    "https://github.com/npat-efault/picocom"
+                    "/archive/" version ".tar.gz"))
+              (file-name (string-append name "-" version ".tar.gz"))
+              (sha256
+               (base32
+                "1v891cx18vx3lnpfaq90f5y6njgigkn4qsikhrmyzshnz32jy5bb"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:make-flags '("CC=gcc")
+       #:tests? #f
+       #:phases
+       (modify-phases %standard-phases
+         (delete 'configure)
+         (replace 'install
+           (lambda* (#:key outputs #:allow-other-keys)
+             (let* ((out (assoc-ref outputs "out"))
+                    (bin (string-append out "/bin"))
+                    (man (string-append out "/share/man")))
+               (install-file "picocom" bin)
+               (install-file "picocom.1" man)))))))
+    (home-page "https://github.com/npat-efault/picocom")
+    (synopsis "Minimal dumb-terminal emulation program")
+    (description "It was designed to serve as a simple, manual, modem
+configuration, testing, and debugging tool.  It has also serves well
+as a low-tech serial communications program to allow access to all
+types of devices that provide serial consoles.")
+    (license license:gpl2+)))
-- 
2.9.0

[PATCH 2/6] gnu: %default-extra-linux-options: Remove redundant options.

[David Craven]

* gnu/packages/linux.scm (%default-extra-linux-options): Remove
  redundant options.
---
 gnu/packages/linux.scm | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index c8be533..d8d14d4 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -187,18 +187,8 @@ for ARCH and optionally VARIANT, or #f if there is no such configuration."
     (search-path %load-path file)))
 
 (define %default-extra-linux-options
-  `(("CONFIG_NET_9P" . m)
-    ("CONFIG_NET_9P_VIRTIO" . m)
-    ("CONFIG_VIRTIO_BLK" . m)
-    ("CONFIG_VIRTIO_NET" . m)
-    ;; https://lists.gnu.org/archive/html/guix-devel/2014-04/msg00039.html
-    ("CONFIG_DEVPTS_MULTIPLE_INSTANCES" . #t)
-    ("CONFIG_VIRTIO_PCI" . m)
-    ("CONFIG_VIRTIO_BALLOON" . m)
-    ("CONFIG_VIRTIO_MMIO" . m)
-    ("CONFIG_FUSE_FS" . m)
-    ("CONFIG_CIFS" . m)
-    ("CONFIG_9P_FS" . m)))
+  `(;; https://lists.gnu.org/archive/html/guix-devel/2014-04/msg00039.html
+    ("CONFIG_DEVPTS_MULTIPLE_INSTANCES" . #t)))
 
 (define (config->string options)
   (string-join (map (match-lambda
-- 
2.9.0

Re: [PATCH 2/6] gnu: %default-extra-linux-options: Remove redundant options.

[Ludovic Courtès]

Hi David,

David Craven <david@craven.ch> skribis:

> * gnu/packages/linux.scm (%default-extra-linux-options): Remove
>   redundant options.

This patches appears to break things:

--8<---------------cut here---------------start------------->8---
$ sudo ./pre-inst-env guix system reconfigure ~/src/configuration/pluto-configuration.scm --fallback

[...]

Backtrace:
In ice-9/boot-9.scm:
 157: 13 [catch #t #<catch-closure 8c9aa0> ...]
In unknown file:
   ?: 12 [apply-smob/1 #<catch-closure 8c9aa0>]
In ice-9/boot-9.scm:
  63: 11 [call-with-prompt prompt0 ...]
In ice-9/eval.scm:
 432: 10 [eval # #]
In ice-9/boot-9.scm:
2401: 9 [save-module-excursion #<procedure 8e7800 at ice-9/boot-9.scm:4045:3 ()>]
4050: 8 [#<procedure 8e7800 at ice-9/boot-9.scm:4045:3 ()>]
1724: 7 [%start-stack load-stack #<procedure 8faae0 at ice-9/boot-9.scm:4041:10 ()>]
1729: 6 [#<procedure 8fd9c0 ()>]
In unknown file:
   ?: 5 [primitive-load "/gnu/store/8lmcdhipd7ccjpaqd3a8pbb2yvn9d7v0-linux-modules-builder"]
In ice-9/eval.scm:
 432: 4 [eval # ()]
 453: 3 [eval # ()]
 411: 2 [eval # ()]
In srfi/srfi-1.scm:
 578: 1 [map #<procedure lookup (a)> ("ahci" "usb-storage" "uas" ...)]
In unknown file:
   ?: 0 [scm-error misc-error #f ...]

ERROR: In procedure scm-error:
ERROR: module not found "virtio_pci.ko" "/gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/modules"
builder for `/gnu/store/x0jqyr639karxakl0q2807ka608g4k3n-linux-modules.drv' failed with exit code 1
--8<---------------cut here---------------end--------------->8---

“make check-system TESTS=basic” fails similarly.

Indeed, most of the virtio modules disappeared:

--8<---------------cut here---------------start------------->8---
$ find "/gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/modules" -name virtio\*
/gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/modules/4.7.5-gnu/kernel/drivers/char/hw_random/virtio-rng.ko
/gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/modules/4.7.5-gnu/kernel/drivers/scsi/virtio_scsi.ko
/gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/modules/4.7.5-gnu/kernel/drivers/gpu/drm/virtio
/gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/modules/4.7.5-gnu/kernel/drivers/gpu/drm/virtio/virtio-gpu.ko
/gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/modules/4.7.5-gnu/kernel/drivers/virtio
/gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/modules/4.7.5-gnu/kernel/drivers/virtio/virtio_input.ko
--8<---------------cut here---------------end--------------->8---

I think we need to revert this patch.  Could you please look at it?

TIA!

Ludo’.

Re: [PATCH 2/6] gnu: %default-extra-linux-options: Remove redundant options.

[David Craven]

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

I'll look at it I'm ca 1h afk. That's possible, the kernel configs may use
y instead of m, but I checked that they're all there. I think that the
configs should probably be changed, or wdyt?

On Sep 29, 2016 10:41 AM, "Ludovic Courtès" <ludo@gnu.org> wrote:

> Hi David,
>
> David Craven <david@craven.ch> skribis:
>
> > * gnu/packages/linux.scm (%default-extra-linux-options): Remove
> >   redundant options.
>
> This patches appears to break things:
>
> --8<---------------cut here---------------start------------->8---
> $ sudo ./pre-inst-env guix system reconfigure ~/src/configuration/pluto-configuration.scm
> --fallback
>
> [...]
>
> Backtrace:
> In ice-9/boot-9.scm:
>  157: 13 [catch #t #<catch-closure 8c9aa0> ...]
> In unknown file:
>    ?: 12 [apply-smob/1 #<catch-closure 8c9aa0>]
> In ice-9/boot-9.scm:
>   63: 11 [call-with-prompt prompt0 ...]
> In ice-9/eval.scm:
>  432: 10 [eval # #]
> In ice-9/boot-9.scm:
> 2401: 9 [save-module-excursion #<procedure 8e7800 at
> ice-9/boot-9.scm:4045:3 ()>]
> 4050: 8 [#<procedure 8e7800 at ice-9/boot-9.scm:4045:3 ()>]
> 1724: 7 [%start-stack load-stack #<procedure 8faae0 at
> ice-9/boot-9.scm:4041:10 ()>]
> 1729: 6 [#<procedure 8fd9c0 ()>]
> In unknown file:
>    ?: 5 [primitive-load "/gnu/store/8lmcdhipd7ccjpaqd3a8pbb2yvn9d7
> v0-linux-modules-builder"]
> In ice-9/eval.scm:
>  432: 4 [eval # ()]
>  453: 3 [eval # ()]
>  411: 2 [eval # ()]
> In srfi/srfi-1.scm:
>  578: 1 [map #<procedure lookup (a)> ("ahci" "usb-storage" "uas" ...)]
> In unknown file:
>    ?: 0 [scm-error misc-error #f ...]
>
> ERROR: In procedure scm-error:
> ERROR: module not found "virtio_pci.ko" "/gnu/store/
> kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/modules"
> builder for `/gnu/store/x0jqyr639karxakl0q2807ka608g4k3n-linux-modules.drv'
> failed with exit code 1
> --8<---------------cut here---------------end--------------->8---
>
> “make check-system TESTS=basic” fails similarly.
>
> Indeed, most of the virtio modules disappeared:
>
> --8<---------------cut here---------------start------------->8---
> $ find "/gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/modules"
> -name virtio\*
> /gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/
> modules/4.7.5-gnu/kernel/drivers/char/hw_random/virtio-rng.ko
> /gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/
> modules/4.7.5-gnu/kernel/drivers/scsi/virtio_scsi.ko
> /gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/
> modules/4.7.5-gnu/kernel/drivers/gpu/drm/virtio
> /gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/
> modules/4.7.5-gnu/kernel/drivers/gpu/drm/virtio/virtio-gpu.ko
> /gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/
> modules/4.7.5-gnu/kernel/drivers/virtio
> /gnu/store/kzia9krx5ax1pm2pq5476v5fv71x36ai-linux-libre-4.7.5/lib/
> modules/4.7.5-gnu/kernel/drivers/virtio/virtio_input.ko
> --8<---------------cut here---------------end--------------->8---
>
> I think we need to revert this patch.  Could you please look at it?
>
> TIA!
>
> Ludo’.
>

[-- Attachment #2: Type: text/html, Size: 3944 bytes --]

Re: [PATCH 2/6] gnu: %default-extra-linux-options: Remove redundant options.

[David Craven]

>> I think we need to revert this patch.  Could you please look at it?

m and y aren't equivalent after all. The default initrd requires these
to be modules.

So there are two options, revert the patch or change the y's to m's.

I think your're right. I can revert it and add a comment saying why
they are needed.

Re: [PATCH 2/6] gnu: %default-extra-linux-options: Remove redundant options.

[Ludovic Courtès]

David Craven <david@craven.ch> skribis:

>>> I think we need to revert this patch.  Could you please look at it?
>
> m and y aren't equivalent after all. The default initrd requires these
> to be modules.
>
> So there are two options, revert the patch or change the y's to m's.
>
> I think your're right. I can revert it and add a comment saying why
> they are needed.

Right, thanks for the quick response!

I think it’s best to have them as modules, because I imagine one would
want to avoid loading them when the system runs on the bare metal.

Ludo’.

[PATCH 3/6] gnu: make-linux-libre: Install device tree files.

[David Craven]

* gnu/packages/linux.scm (make-linux-libre): Install device tree files.
---
 gnu/packages/linux.scm | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index d8d14d4..243a71c 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -263,8 +263,8 @@ for ARCH and optionally VARIANT, or #f if there is no such configuration."
              (let ((build  (assoc-ref %standard-phases 'build))
                    (config (assoc-ref inputs "kconfig")))
 
-               ;; Use the architecture-specific config if available, and
-               ;; 'defconfig' otherwise.
+               ;; Use a custom kernel configuration file or a default
+               ;; configuration file.
                (if config
                    (begin
                      (copy-file config ".config")
@@ -283,13 +283,16 @@ for ARCH and optionally VARIANT, or #f if there is no such configuration."
            (lambda* (#:key inputs native-inputs outputs #:allow-other-keys)
              (let* ((out    (assoc-ref outputs "out"))
                     (moddir (string-append out "/lib/modules"))
-                    (kmod    (assoc-ref (or native-inputs inputs) "kmod")))
+                    (dtbdir (string-append out "/lib/dtbs"))
+                    (kmod   (assoc-ref (or native-inputs inputs) "kmod")))
+               ;; Install kernel image, kernel configuration and link map.
+               (for-each (lambda (file) (install-file file out))
+                         (find-files "." "^(\\.config|bzImage|zImage|vmlinuz|System\\.map)$"))
+               ;; Install device tree files
+               (for-each (lambda (file) (install-file file dtbdir))
+                         (find-files "." "\\.dtb$"))
+               ;; Install kernel modules
                (mkdir-p moddir)
-               (for-each (lambda (file)
-                           (copy-file file
-                                      (string-append out "/" (basename file))))
-                         (find-files "." "^(bzImage|zImage|vmlinuz|System\\.map)$"))
-               (copy-file ".config" (string-append out "/config"))
                (zero? (system* "make"
                                (string-append "DEPMOD=" kmod "/bin/depmod")
                                (string-append "MODULE_DIR=" moddir)
-- 
2.9.0

[PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[David Craven]

* gnu/packages/linux.scm (linux-libre-beagle-bone-black): Remove.
---
 gnu/packages/linux.scm | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 243a71c..6f7986d 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -339,13 +339,6 @@ It has been modified to remove all non-free binary blobs.")
                     #:defconfig "multi_v7_defconfig"
                     #:extra-version "arm-generic"))
 
-(define-public linux-libre-beagle-bone-black
-  (make-linux-libre %linux-libre-version
-                    %linux-libre-hash
-                    '("armhf-linux")
-                    #:defconfig "omap2plus_defconfig"
-                    #:extra-version "beagle-bone-black"))
-
 \f
 ;;;
 ;;; Pluggable authentication modules (PAM).
-- 
2.9.0

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[John Darrington]

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

Why are we removing this?


On Mon, Sep 26, 2016 at 12:34:45PM +0200, David Craven wrote:
     * gnu/packages/linux.scm (linux-libre-beagle-bone-black): Remove.
     ---
      gnu/packages/linux.scm | 7 -------
      1 file changed, 7 deletions(-)
     
     diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
     index 243a71c..6f7986d 100644
     --- a/gnu/packages/linux.scm
     +++ b/gnu/packages/linux.scm
     @@ -339,13 +339,6 @@ It has been modified to remove all non-free binary blobs.")
                          #:defconfig "multi_v7_defconfig"
                          #:extra-version "arm-generic"))
      
     -(define-public linux-libre-beagle-bone-black
     -  (make-linux-libre %linux-libre-version
     -                    %linux-libre-hash
     -                    '("armhf-linux")
     -                    #:defconfig "omap2plus_defconfig"
     -                    #:extra-version "beagle-bone-black"))
     -
      \f
      ;;;
      ;;; Pluggable authentication modules (PAM).
     -- 
     2.9.0

-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[David Craven]

It's for the original beagle bone I think, not the beagle bone black,
so it's named wrong at best. I'm using the generic kernel for my bbb.
Unless someone has a bb board and wants to test this, it's just code
no one is using.

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Vincent Legoll]

Hello,

@David: the following question is nothing against you, I just
took the opportunity to ask, sorry for the thread hijacking.

On Mon, Sep 26, 2016 at 3:23 PM, John Darrington wrote:
> Why are we removing this?

Is it just me finding this changelog format usefulness to be
very low ?

I've come to understand this is a standardized way of doing it,
but... Can someone point me to an explanation about why they
are done like this ?

I read a few on this ML since a few weeks and am almost daily
wondering "WTF ?" There's often no explanation about the reason
why the changes are made...

Thanks

-- 
Vincent Legoll

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Ricardo Wurmus]

Vincent Legoll <vincent.legoll@gmail.com> writes:

> @David: the following question is nothing against you, I just
> took the opportunity to ask, sorry for the thread hijacking.
>
> On Mon, Sep 26, 2016 at 3:23 PM, John Darrington wrote:
>> Why are we removing this?
>
> Is it just me finding this changelog format usefulness to be
> very low ?
>
> I've come to understand this is a standardized way of doing it,
> but... Can someone point me to an explanation about why they
> are done like this ?
>
> I read a few on this ML since a few weeks and am almost daily
> wondering "WTF ?" There's often no explanation about the reason
> why the changes are made...

It is acceptable to add an explanation between the summary line and the
changelog block.  We’ve done so in the past.

See one of commits e465d9e19087ab150f7e31f21c09e4a147b93b36,
dfdaea32e90fd226aa98028a196fca3d2d371a94,
7f39e131805857614a171b51cbf432b4285ba1e6,
4c0416ae1709e08c311d32af4823fac9d73364ee, … for examples.

~~ Ricardo

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[John Darrington]

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

On Mon, Sep 26, 2016 at 04:24:10PM +0200, Vincent Legoll wrote:
     Hello,
     
     @David: the following question is nothing against you, I just
     took the opportunity to ask, sorry for the thread hijacking.
     
     On Mon, Sep 26, 2016 at 3:23 PM, John Darrington wrote:
     > Why are we removing this?
     
     Is it just me finding this changelog format usefulness to be
     very low ?
     

I entirely agree.   

Our current convention for commit messages is aesthetically pleasing,
but useless.   It is redundant to put in the message WHAT has changed.
That can be easily determined by running "git show".   What is much
more usefull is the reason WHY we changed it.


Recently, on another project,  I came across a line of code which I
was pretty sure was pointless.  I was about to delete it - but I 
decided to check first.  So I ran "git blame" to find out which 
commit had added it.  Then I ran git show to get the full details
of that commit.   Fortunately, the person who had committed it 5 years
ago had mentioned exactly why it was needed.  - and it was there
for a very valid reason too.   Had we been using the Guix convention,
this would probably have been deleted and consequences would have
followed.

I vote that we make the reason for a commit to be compulsory.  All
other details optional.


J'



-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Vincent Legoll]

> I entirely agree.

I'm not alone :-)

> Our current convention for commit messages is aesthetically pleasing,
> but useless.   It is redundant to put in the message WHAT has changed.
> That can be easily determined by running "git show".   What is much
> more usefull is the reason WHY we changed it.

+1

> Recently, on another project,  I came across a line of code which I
> was pretty sure was pointless.  I was about to delete it - but I
> decided to check first.  So I ran "git blame" to find out which
> commit had added it.  Then I ran git show to get the full details
> of that commit.   Fortunately, the person who had committed it 5 years
> ago had mentioned exactly why it was needed.  - and it was there
> for a very valid reason too.   Had we been using the Guix convention,
> this would probably have been deleted and consequences would have
> followed.

Been there, done that...

> I vote that we make the reason for a commit to be compulsory.  All
> other details optional.

+1, more details is better than less, LKML-style...

-- 
Vincent Legoll

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Leo Famulari]

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

On Mon, Sep 26, 2016 at 06:49:34PM +0200, John Darrington wrote:
> Our current convention for commit messages is aesthetically pleasing,
> but useless.   It is redundant to put in the message WHAT has changed.
> That can be easily determined by running "git show".   What is much
> more usefull is the reason WHY we changed it.

I find the GNU changelog style to be very useful when reading the Git
log. 

There are certain types of commits that we make over and over again,
especially with packages (updates, adding inputs, etc), and I find it
helpful when their commit messages follow a common format.

I agree that commits that don't conform to one of those types should
have some explanation. This is why I often rewrite commit messages when
I am committing on behalf of somebody else.

But there is a value to "aesthetic" consistency. It's just like having
standards for code style in my opinion. It helps a lot when reading
others' commits.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[John Darrington]

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

On Mon, Sep 26, 2016 at 01:31:09PM -0400, Leo Famulari wrote:
     On Mon, Sep 26, 2016 at 06:49:34PM +0200, John Darrington wrote:
     > Our current convention for commit messages is aesthetically pleasing,
     > but useless.   It is redundant to put in the message WHAT has changed.
     > That can be easily determined by running "git show".   What is much
     > more usefull is the reason WHY we changed it.
     
     I find the GNU changelog style to be very useful when reading the Git
     log. 
     
     There are certain types of commits that we make over and over again,
     especially with packages (updates, adding inputs, etc), and I find it
     helpful when their commit messages follow a common format.
     
     I agree that commits that don't conform to one of those types should
     have some explanation. This is why I often rewrite commit messages when
     I am committing on behalf of somebody else.
     
     But there is a value to "aesthetic" consistency. It's just like having
     standards for code style in my opinion. It helps a lot when reading
     others' commits.

I don't object to a having a common style.   I just wish that the information
that we put in our messages a) gave the rationale for the change; and b) didn't
merely repeat information which git show could have told me.

J'


-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[David Craven]

> I don't object to a having a common style.   I just wish that the information
> that we put in our messages a) gave the rationale for the change; and b) didn't
> merely repeat information which git show could have told me.

So is anyone against this change or just my commit message? Would
someone like to suggest a better one?

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Vincent Legoll]

> So is anyone against this change or just my commit message? Would
> someone like to suggest a better one?

The discussion about changelog style is orthogonal to this patch, LGTM,
with your explanation to John's question, we can see why this change is
done.

Let's not hold this hostage to $OTHER_SUBJECT...

-- 
Vincent Legoll

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[David Craven]

> Recently, on another project,  I came across a line of code which I
> was pretty sure was pointless.

The guy who wrote that line should have included a comment in that
case. The git history isn't a replacement for code commenting. Code
needs to be readable without looking at it's history.

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Leo Famulari]

On Mon, Sep 26, 2016 at 08:06:13PM +0200, David Craven wrote:
> > I don't object to a having a common style.   I just wish that the information
> > that we put in our messages a) gave the rationale for the change; and b) didn't
> > merely repeat information which git show could have told me.
> 
> So is anyone against this change or just my commit message? Would
> someone like to suggest a better one?

If it's true that this kernel variant is not for the BBB, and you are
using a generic kernel successfully on the BBB, then I see no problem
with this change. But I'm not an expert on this subject.

Be sure to add a comment explaining the change in the commit message ;)

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Ludovic Courtès]

John Darrington <john@darrington.wattle.id.au> skribis:

> I don't object to a having a common style.   I just wish that the information
> that we put in our messages a) gave the rationale for the change; and b) didn't
> merely repeat information which git show could have told me.

The rationale should really go to comments in the code:
<https://www.gnu.org/prep/standards/html_node/Change-Log-Concepts.html>.

A notable exception is reverts or removals: for these, I think it’s best
to have an explanation in the code, so that someone browsing the history
can understand why that happened.

Ludo’.

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Tobias Geerinckx-Rice]

[-- Attachment #1.1: Type: text/plain, Size: 828 bytes --]

Hullo,

On 26/09/16 18:49, John Darrington wrote:
> I vote that we make the reason for a commit to be compulsory.

+1, s/commit/non-trivial commit/

I agree that it should be more strongly encouraged, if just to make
people aware that it's actually allowed — plenty aren't.

If no-one submits a proposal, I will.

> All other details optional.

-1, but I think *that* would require more than a mere vote anyway ;-)

<blah>
I found the strict commit format annoying at first. Then I started
following the guix-commit list for fun and edutainment. Then I tried
following another commit list. I have seen the light.

That curated list of changes might ‘duplicate’ the diff in certain
trivial cases, but it saves more people far more time than it cost me to
write.
</blah>

Kind regards,

T G-R


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Mark H Weaver]

John Darrington <john@darrington.wattle.id.au> writes:

> On Mon, Sep 26, 2016 at 04:24:10PM +0200, Vincent Legoll wrote:
>      Hello,
>      
>      @David: the following question is nothing against you, I just
>      took the opportunity to ask, sorry for the thread hijacking.
>      
>      On Mon, Sep 26, 2016 at 3:23 PM, John Darrington wrote:
>      > Why are we removing this?
>      
>      Is it just me finding this changelog format usefulness to be
>      very low ?
>      
>
> I entirely agree.   
>
> Our current convention for commit messages is aesthetically pleasing,
> but useless.   It is redundant to put in the message WHAT has changed.
> That can be easily determined by running "git show".   What is much
> more usefull is the reason WHY we changed it.
>
>
> Recently, on another project,  I came across a line of code which I
> was pretty sure was pointless.  I was about to delete it - but I 
> decided to check first.  So I ran "git blame" to find out which 
> commit had added it.  Then I ran git show to get the full details
> of that commit.   Fortunately, the person who had committed it 5 years
> ago had mentioned exactly why it was needed.

Our conventions for commit logs, which follow the GNU Coding Standards
for Change Logs (see section 6.8 of the GNU Coding Standards), is that
explanations belong in the comments of the code itself, not in the
commit log.  If that had been done in the example you give above, you
would have known why the line was needed in a small fraction of the time
that it must have taken you to perform all the steps above.

However, I agree that commits that _remove_ code should include the
rationale in the commit log, if the reason is not obvious and if there's
no sensible place to put the explanation in the code.

What do you think?

      Mark

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[John Darrington]

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

On Tue, Sep 27, 2016 at 08:14:38PM -0400, Mark H Weaver wrote:
     
     Our conventions for commit logs, which follow the GNU Coding Standards
     for Change Logs (see section 6.8 of the GNU Coding Standards), is that
     explanations belong in the comments of the code itself, not in the
     commit log.  If that had been done in the example you give above, you
     would have known why the line was needed in a small fraction of the time
     that it must have taken you to perform all the steps above.

That can work in simple instances - where the change is a one liner or at least
where the change is confined to a few consecutive lines in one file.  But not
all changes are that simple.  Sometimes it is necessary to make a change which 
involves lots of small changes in lots of different places in lots of different
files.   In such cases it would be rediculous to append a comment to each line
changed.  In fact I have seen such code from the 1980s where almost every 4th line
has a comment like: 
  mode++; /* JMD 09/2/1981: Increment the mode count otherwise the crud wangler overflows 
                    in the case where grunger regurtitates */

The occasional such comment is ok, but when it's every third line it does nothing
for readability nor comprehensibility.
     
     However, I agree that commits that _remove_ code should include the
     rationale in the commit log, if the reason is not obvious and if there's
     no sensible place to put the explanation in the code.
     
     What do you think?

Well I certainly agree that adding a reason for a change is a good idea.  Even if
we do also have text in the message which is superfluous.

J'

Note to self:  Must write an improved version of gnulib's git-log-to-changelog script
which will satisfy the die-harders.


-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Ludovic Courtès]

Mark H Weaver <mhw@netris.org> skribis:

> Our conventions for commit logs, which follow the GNU Coding Standards
> for Change Logs (see section 6.8 of the GNU Coding Standards), is that
> explanations belong in the comments of the code itself, not in the
> commit log.  If that had been done in the example you give above, you
> would have known why the line was needed in a small fraction of the time
> that it must have taken you to perform all the steps above.
>
> However, I agree that commits that _remove_ code should include the
> rationale in the commit log, if the reason is not obvious and if there's
> no sensible place to put the explanation in the code.
>
> What do you think?

+1

Ludo’.

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Ludovic Courtès]

Hi,

Vincent Legoll <vincent.legoll@gmail.com> skribis:

> Is it just me finding this changelog format usefulness to be
> very low ?

The change log is just a log of changes, not the place where we explain
the why and how—this rather goes to comments.  See
<https://www.gnu.org/prep/standards/html_node/Change-Log-Concepts.html>
for the rationale.

Ludo’.

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Vincent Legoll]

Hello,

On Wed, Sep 28, 2016 at 11:11 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> <https://www.gnu.org/prep/standards/html_node/Change-Log-Concepts.html>

I read this and still am on the opposite side.

The "changelog entries are like an undo list" is a WTF, to me this is what
`patch -R` is for...

My take on that (the newbie wanting to grasp it) is that I do the following:
- read git changelog entries titles in gitk (or a ML, or a gitweb)
- if oneliner title looks interesting for a subject I'm currently
trying to learn
- I read the full changelog entry
- if that is still interesting me, then I go read the code

Is there something equivalent with the GNU-styled CLs, that does not require
me to read every patch ? Because that's not scaling very well, especially in
languages that I'm not proficient in.

My example is LKML, the changelogs are an extremely valuable tool to understand
things, to debug, etc... I just love this level of details put into changelogs.

Tastes & colors...

-- 
Vincent Legoll

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Ludovic Courtès]

Hi,

Vincent Legoll <vincent.legoll@gmail.com> skribis:

> On Wed, Sep 28, 2016 at 11:11 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>> <https://www.gnu.org/prep/standards/html_node/Change-Log-Concepts.html>
>
> I read this and still am on the opposite side.
>
> The "changelog entries are like an undo list" is a WTF, to me this is what
> `patch -R` is for...

Not quite, because the change log shows the language-level view of the
changes, which can be hard to infer by looking at a diff.  It makes it
easy to answer questions such as “when did we change this function?”,
“when did we introduce that variable”, etc.

> My take on that (the newbie wanting to grasp it) is that I do the following:
> - read git changelog entries titles in gitk (or a ML, or a gitweb)
> - if oneliner title looks interesting for a subject I'm currently
> trying to learn
> - I read the full changelog entry
> - if that is still interesting me, then I go read the code

I see.  The commit title is definitely what you’re interested in here.
I understand it can be frustratingly short, but then again it’s no
substitute for the full discussions or code explanations, IMO.

> Is there something equivalent with the GNU-styled CLs, that does not require
> me to read every patch ? Because that's not scaling very well, especially in
> languages that I'm not proficient in.
>
> My example is LKML, the changelogs are an extremely valuable tool to understand
> things, to debug, etc... I just love this level of details put into changelogs.

AIUI Linux-style commit logs are not change logs, but explanations.  I
still strongly believe that explanations believe in code.

Ludo’.

Re: [PATCH 4/6] gnu: linux-libre-beagle-bone-black: Remove kernel variant.

[Vincent Legoll]

Hello,

>> The "changelog entries are like an undo list" is a WTF, to me this is what
>> `patch -R` is for...
>
> Not quite, because the change log shows the language-level view of the
> changes,

I don't understand "language-level view of the changes"

> It makes it easy to answer questions such as “when did we change this
> function?”, “when did we introduce that variable”, etc.

This looks suspicously close to *-blame functionality, looks script
(or any other
automated way) -material to me.

Personally I've never done this by searching changelogs (and here I show my
lack of participation in GNU-standardized projects)...

> I see.  The commit title is definitely what you’re interested in here.

Yes, but this is a layered (recursive) approach, I dig a level when the previous
one looked interesting, or backtrack. This allows to cheaply skip irrelevant
material (with a small miss rate).

> I understand it can be frustratingly short

No, for my usage it's totally useless, I never had to search that way, way less
efficient that a git blame (+ refinements)

> but then again it’s no substitute for the full discussions or code explanations
[...]
> AIUI Linux-style commit logs are not change logs, but explanations.  I
> still strongly believe that explanations believe in code.

Let's agree to disagree

Cheers

-- 
Vincent Legoll

[PATCH 5/6] gnu: make-u-boot-package: Add files-to-install argument.

[David Craven]

* gnu/packages/u-boot.scm (make-u-boot-package): Add files-to-install.
  (u-boot-vexpress, u-boot-malta, u-boot-beagle-bone-black): Use
  files-to-install.
---
 gnu/packages/u-boot.scm | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/gnu/packages/u-boot.scm b/gnu/packages/u-boot.scm
index cdd52d8..60fd48f 100644
--- a/gnu/packages/u-boot.scm
+++ b/gnu/packages/u-boot.scm
@@ -82,7 +82,7 @@ tree binary files. These are board description files used by Linux and BSD.")
 also initializes the boards (RAM etc).")
     (license license:gpl2+)))
 
-(define (make-u-boot-package board triplet)
+(define (make-u-boot-package board triplet files-to-install)
   "Returns a u-boot package for BOARD cross-compiled for TRIPLET."
   (package
     (inherit u-boot)
@@ -114,23 +114,20 @@ also initializes the boards (RAM etc).")
                        (closedir dir))
                      #f)))))
          (replace 'install
-           (lambda* (#:key outputs make-flags #:allow-other-keys)
-             (let* ((out (assoc-ref outputs "out"))
-                    (libexec (string-append out "/libexec"))
-                    (uboot-files (find-files "." ".*\\.(bin|efi|spl)$")))
-               (mkdir-p libexec)
-               (for-each
-                (lambda (file)
-                  (let ((target-file (string-append libexec "/" file)))
-                    (mkdir-p (dirname target-file))
-                    (copy-file file target-file)))
-                uboot-files)))))))))
+           (lambda* (#:key outputs #:allow-other-keys)
+             (for-each (lambda (file)
+                         (let* ((out (assoc-ref outputs "out"))
+                                (source-file (string-append (getcwd) "/" file))
+                                (target-file (string-append out "/" file)))
+                           (mkdir-p (dirname target-file))
+                           (copy-file source-file target-file)))
+                       '(,@files-to-install)))))))))
 
 (define-public u-boot-vexpress
-  (make-u-boot-package "vexpress_ca9x4" "arm-linux-gnueabihf"))
+  (make-u-boot-package "vexpress_ca9x4" "arm-linux-gnueabihf" '("u-boot.bin")))
 
 (define-public u-boot-malta
-  (make-u-boot-package "malta" "mips64el-linux-gnuabi64"))
+  (make-u-boot-package "malta" "mips64el-linux-gnuabi64" '("u-boot.bin")))
 
 (define-public u-boot-beagle-bone-black
-  (make-u-boot-package "am335x_boneblack" "arm-linux-gnueabihf"))
+  (make-u-boot-package "am335x_boneblack" "arm-linux-gnueabihf" '("MLO" "u-boot.img")))
-- 
2.9.0

Re: [PATCH 5/6] gnu: make-u-boot-package: Add files-to-install argument.

[Danny Milosavljevic]

Hi David,

On Mon, 26 Sep 2016 12:34:46 +0200
David Craven <david@craven.ch> wrote:

>  (define-public u-boot-vexpress
> -  (make-u-boot-package "vexpress_ca9x4" "arm-linux-gnueabihf"))
> +  (make-u-boot-package "vexpress_ca9x4" "arm-linux-gnueabihf" '("u-boot.bin")))
>  
>  (define-public u-boot-malta
> -  (make-u-boot-package "malta" "mips64el-linux-gnuabi64"))
> +  (make-u-boot-package "malta" "mips64el-linux-gnuabi64" '("u-boot.bin")))
>  
>  (define-public u-boot-beagle-bone-black
> -  (make-u-boot-package "am335x_boneblack" "arm-linux-gnueabihf"))
> +  (make-u-boot-package "am335x_boneblack" "arm-linux-gnueabihf" '("MLO" "u-boot.img")))
                                                                   ^^^^
Ughh... are we sure we want to do that? I don't think a regular user would know what to put there. If the U-Boot build is from a clean slate it's not like there are stray files lying around or anything - we can just copy them all if we want.

Also, I don't think that we will have these define-public things for different systems in the long run. I posted a generated patch adding *all* of the supported U-Boot systems and Ludo vetoed it (and I agree by now - it's not necessary).

My preferred approach would be to allow the user to put the following into /etc/config.scm :

(bootloader (u-boot-configuration
              (device "/dev/mmcblk0")
              (package (make-u-boot-package "malta")))

(Nothing was left off)

What do you think?

Re: [PATCH 5/6] gnu: make-u-boot-package: Add files-to-install argument.

[David Craven]

> What do you think?

I think there is a big difference here between supported boards and
unsupported boards. It doesn't make sense to add shortcuts for all
possible boards. Boards that are tested and supported should have
shortcuts.

I'd like cuirass to build native disk images for our supported boards
so that we can just write it to an sd card.

It would also be nice if we had a cross-compiled image for qemu. This
also would solve the guix system vm problem, since we can get a
cross-compiled substitute that will allow building a native image
which can then be started in qemu.

I'm planning on adding support for bbb, zybo and qemu, because that's
the hardware I've got.

Re: [PATCH 5/6] gnu: make-u-boot-package: Add files-to-install argument.

[David Craven]

FYI: I got guixsd to boot, need a tty that works over serial so that I
can actually login...

Re: [PATCH 5/6] gnu: make-u-boot-package: Add files-to-install argument.

[Danny Milosavljevic]

Hi David,

On Mon, 26 Sep 2016 15:41:12 +0200
David Craven <david@craven.ch> wrote:

> FYI: I got guixsd to boot, need a tty that works over serial so that I
> can actually login...

\o/ very nice :)

Do you mean client software? Then GNU screen can do that.

$ screen /dev/ttyUSB0 115200

Or do you mean terminal client hardware?

Then FTDI USB serial adapters can do that.

Or do you mean server software?

mingetty

Re: [PATCH 5/6] gnu: make-u-boot-package: Add files-to-install argument.

[David Craven]

> Or do you mean server software?
>
> mingetty

mingetty doesn't support serial or does it? the man page doesn't
document a baud rate flag, and setting the tty to ttyS1 causes a crash
- not a tty. agetty or the getty from toybox have a flag to set the
baud rate.

Re: [PATCH 5/6] gnu: make-u-boot-package: Add files-to-install argument.

[Danny Milosavljevic]

On Mon, 26 Sep 2016 16:07:16 +0200
David Craven <david@craven.ch> wrote:

> mingetty doesn't support serial or does it? the man page doesn't
> document a baud rate flag, and setting the tty to ttyS1 causes a crash
> - not a tty. agetty or the getty from toybox have a flag to set the
> baud rate.

Right.

Debian on LIME2 (Allwinner A20) does:

root       883     1  0 Aug24 ttyS0    00:00:00 /sbin/agetty --keep-baud 115200 38400 9600 ttyS0 vt102

Re: [PATCH 5/6] gnu: make-u-boot-package: Add files-to-install argument.

[David Craven]

> Ughh... are we sure we want to do that? I don't think a
> regular user would know what to put there. If the U-Boot
> build is from a clean slate it's not like there are stray files
> lying around or anything - we can just copy them all if we
> want.

I thought about this patch and I think we should add it. The regex
solution results in a lot of garbage inside the output directory.
Regular users don't compile u-boot, and anyone with the skills to port
guixsd to a new board will know what to put. It also serves as
documentation, from reading the package definition I can tell what the
relevant files are without examining the output directory (which is
full of garbage - so I have to google it anyway).

Re: [PATCH 5/6] gnu: make-u-boot-package: Add files-to-install argument.

[Danny Milosavljevic]

Hi David,

On Fri, 7 Oct 2016 10:19:42 +0200
David Craven <david@craven.ch> wrote:
> > Ughh... are we sure we want to do that? I don't think a
> > regular user would know what to put there. If the U-Boot
> > build is from a clean slate it's not like there are stray files
> > lying around or anything - we can just copy them all if we
> > want.  
> 
> I thought about this patch and I think we should add it. The regex
> solution results in a lot of garbage inside the output directory.
> Regular users don't compile u-boot, and anyone with the skills to port
> guixsd to a new board will know what to put. It also serves as
> documentation, from reading the package definition I can tell what the
> relevant files are without examining the output directory (which is
> full of garbage - so I have to google it anyway).

Yeah, if there are a lot of garbage files I also think that it's okay to do it like you propose. 

(It's just something that we have been struggling with in the ARM world - people think their board is the only one and then you have some custom configuration which only works for their board for no good reason. In this case the U-Boot binaries (contents) are rightfully board-specific. But there's no good reason that the names and splitting-up-into-files are (or the installation process - it's 2016, a computer should automate the installation). I'll mention it upstream)

I wonder how U-Boot itself finds out the target names - but that's for a future patch. (I think one could examine ALL-y to find out)

So I agree. LTGM!

[PATCH 6/6] gnu: make-u-boot-package: Don't cross compile on arm.

[David Craven]

* gnu/packages/u-boot.scm (make-u-boot-package): Don't cross compile on
  arm.
---
 gnu/packages/u-boot.scm | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/gnu/packages/u-boot.scm b/gnu/packages/u-boot.scm
index 60fd48f..d8b8291 100644
--- a/gnu/packages/u-boot.scm
+++ b/gnu/packages/u-boot.scm
@@ -88,20 +88,24 @@ also initializes the boards (RAM etc).")
     (inherit u-boot)
     (name (string-append "u-boot-" (string-downcase board)))
     (native-inputs
-     `(("cross-gcc" ,(cross-gcc triplet))
-       ("cross-binutils" ,(cross-binutils triplet))
-       ,@(package-native-inputs u-boot)))
+     `(,@(package-native-inputs u-boot)
+       ,@(if triplet
+             `(("cross-gcc" ,(cross-gcc triplet))
+               ("cross-binutils" ,(cross-binutils triplet)))
+             '())))
     (arguments
      `(#:test-target "test"
        #:make-flags
-       (list "HOSTCC=gcc" (string-append "CROSS_COMPILE=" ,triplet "-"))
+       (list "HOSTCC=gcc" ,@(if triplet
+                                `((string-append "CROSS_COMPILE=" ,triplet "-"))
+                                '()))
        #:phases
        (modify-phases %standard-phases
          (replace 'configure
            (lambda* (#:key outputs make-flags #:allow-other-keys)
              (let ((config-name (string-append ,board "_defconfig")))
                (if (file-exists? (string-append "configs/" config-name))
-                   (zero? (apply system* "make" `(,@make-flags ,config-name)))
+                   (zero? (apply system* "make" (cons config-name make-flags)))
                    (begin
                      (display "Invalid board name. Valid board names are:")
                      (let ((dir (opendir "configs"))
@@ -124,10 +128,19 @@ also initializes the boards (RAM etc).")
                        '(,@files-to-install)))))))))
 
 (define-public u-boot-vexpress
-  (make-u-boot-package "vexpress_ca9x4" "arm-linux-gnueabihf" '("u-boot.bin")))
+  (make-u-boot-package
+   "vexpress_ca9x4"
+   (if (string-prefix? "armhf" (%current-system)) #f "arm-linux-gnueabihf")
+   '("u-boot.bin")))
 
 (define-public u-boot-malta
-  (make-u-boot-package "malta" "mips64el-linux-gnuabi64" '("u-boot.bin")))
+  (make-u-boot-package
+   "malta"
+   (if (string-prefix? "mips64el" (%current-system)) #f "mips64el-linux-gnuabi64")
+   '("u-boot.bin")))
 
 (define-public u-boot-beagle-bone-black
-  (make-u-boot-package "am335x_boneblack" "arm-linux-gnueabihf" '("MLO" "u-boot.img")))
+  (make-u-boot-package
+   "am335x_boneblack"
+   (if (string-prefix? "armhf" (%current-system)) #f "arm-linux-gnueabihf")
+   '("MLO" "u-boot.img")))
-- 
2.9.0

Re: [PATCH 1/6] gnu: Add picocom.

[Leo Famulari]

On Mon, Sep 26, 2016 at 12:34:42PM +0200, David Craven wrote:
> * gnu/packages/terminals.scm (picocom): New variable.

> +    (arguments
> +     `(#:make-flags '("CC=gcc")
> +       #:tests? #f

Please add a comment saying that there is no test suite (that's what I
understand from skimming the GitHub web page).

> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure)
> +         (replace 'install
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out"))
> +                    (bin (string-append out "/bin"))
> +                    (man (string-append out "/share/man")))

This path should be "/share/man/man1".

Otherwise looks good to me!