[bug#47540] gnu: Add prips.

From 22cc953d3e663bb842d5a0b970577a3d54f6fef1 Mon Sep 17 00:00:00 2001
 From: methuselah-0 <david.larsson@selfhosted.xyz>
Date: Thu, 1 Apr 2021 15:10:45 +0200
Subject: [PATCH] gnu: Add prips.

* gnu/packages/admin.scm (prips): New variable.
---
  gnu/packages/admin.scm | 34 ++++++++++++++++++++++++++++++++++
  1 file changed, 34 insertions(+)

    (package
      (name "alive")

Hi,

Some comments on the patch.

On Thu, 2021-04-01 at 15:11 +0200, david larsson wrote:
> [...]
> +(define-public prips
> +  (package
> +    (name "prips")
> +    (version "1.1.1")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append "https://devel.ringlet.net/files/sys/"
> +                           name "/" name "-" version ".tar.xz"))
> +       (sha256
> +        (base32 
> "1a33vbl4w603mk6mm5r3vhk87fy3dfk5wdpch0yd3ncbkg3fmvqn"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     '(#:make-flags (list "CC=gcc")

As "CC=gcc" is a constant, I would write '("CC=gcc") here,
though admittedly this is largely a matter of taste.

Another problem: when cross-compiling, "gcc" will be a
compiler for the build system, not the host system (assuming
I got the terminology right).  Instead, write

     `(#:make-flags (list (string-append "CC" ,(cc-for-target)))
       ...)

(The quasiquote ` is important.)
(TODO to self: fix other packages that incorrectly set "CC=gcc" ...)

> +       #:phases (modify-phases
> +                    %standard-phases

%standard-phases shouldn't be on a separate line here.

> +                  (delete 'configure)
> +                  (delete 'check)

Prips has some tests, so don't delete this phase.

> +                  (replace 'install
> +                    (lambda _
> +                      (let*
> +                          ((bin-dir  (string-append %output "/bin"))
> +                           (bin-file (string-append bin-dir "/prips")))

The ((bin-dir ...)) should be on the same line as 'let*'.

> +                        (mkdir-p bin-dir)
> +                        (copy-file "prips" bin-file)
> +                        (chmod bin-file #o700)))))))

Why are you making bin/prips writable?  Shouldn't this be
(chmod bin-file #o600) instead?

> +    (synopsis "Tool that prints the IP addresses in a given range")
> +    (description "Prips can be used to print all of the IP addresses in
> + a given range.  This allows the enhancement of tools only work

‘Enhancement’ is rather vague and leaning towards marketing-speak.
I do not have an alternative suggestion however.

> + on one host at a time (e.g. whois).")
> +    (home-page "https://devel.ringlet.net/sysutils/prips/")
> +    (license license:gpl2)))

I looked at the source code and it seems prips is actually gpl2+.

Greetings,
Maxime.

On 2021-04-01 21:44, Maxime Devos wrote:

>> +    (arguments
>> +     '(#:make-flags (list "CC=gcc")
> 
> As "CC=gcc" is a constant, I would write '("CC=gcc") here,
> though admittedly this is largely a matter of taste.
> Another problem: when cross-compiling, "gcc" will be a
> compiler for the build system, not the host system (assuming
> I got the terminology right).  Instead, write
> 
>      `(#:make-flags (list (string-append "CC" ,(cc-for-target)))
>        ...)
> 
> (The quasiquote ` is important.)

Fixed.

>> +       #:phases (modify-phases
>> +                    %standard-phases
> 
> %standard-phases shouldn't be on a separate line here.

Fixed.

> 
>> +                  (delete 'configure)
>> +                  (delete 'check)
> 
> Prips has some tests, so don't delete this phase.

I tried to fix but this fails.

> 
>> +                  (replace 'install
>> +                    (lambda _
>> +                      (let*
>> +                          ((bin-dir  (string-append %output "/bin"))
>> +                           (bin-file (string-append bin-dir 
>> "/prips")))
> 
> The ((bin-dir ...)) should be on the same line as 'let*'.
> 
>> +                        (mkdir-p bin-dir)
>> +                        (copy-file "prips" bin-file)
>> +                        (chmod bin-file #o700)))))))
> 
> Why are you making bin/prips writable?  Shouldn't this be
> (chmod bin-file #o600) instead?

I don't know for sure why that happened, but it's fixed to #o600 now.

>> +    (synopsis "Tool that prints the IP addresses in a given range")
>> +    (description "Prips can be used to print all of the IP addresses 
>> in
>> + a given range.  This allows the enhancement of tools only work
> 
> ‘Enhancement’ is rather vague and leaning towards marketing-speak.
> I do not have an alternative suggestion however.

This was an exact quote from website or something.

>> +    (license license:gpl2)))
> 
> I looked at the source code and it seems prips is actually gpl2+.

Fixed.

> Greetings,
> Maxime.

Thanks for your review!

This is what it looks like now, and failing the check phase:

(define-public prips
   (package
     (name "prips")
     (version "1.1.1")
     (source
      (origin
        (method url-fetch)
        (uri (string-append "https://devel.ringlet.net/files/sys/"
                            name "/" name "-" version ".tar.xz"))
        (sha256
         (base32 
"1a33vbl4w603mk6mm5r3vhk87fy3dfk5wdpch0yd3ncbkg3fmvqn"))))
     (build-system gnu-build-system)
     (arguments
      `(#:make-flags (list (string-append "CC=" ,(cc-for-target)))
        #:phases (modify-phases %standard-phases
                   (delete 'configure)
                                         ;(delete 'check)
                   (replace 'install
                     (lambda _
                       (let*
                           ((bin-dir  (string-append %output "/bin"))
                            (bin-file (string-append bin-dir "/prips")))
                         (mkdir-p bin-dir)
                         (copy-file "prips" bin-file)
                         (chmod bin-file #o600)))))))
     (synopsis "Tool that prints the IP addresses in a given range")
     (description "Prips can be used to print all of the IP addresses in
  a given range.  This allows the enhancement of tools only work
  on one host at a time (e.g. whois).")
     (home-page "https://devel.ringlet.net/sysutils/prips/")
     (license license:gpl2+)))

The logs:
---------------

starting phase `check'
make: *** No rule to make target 'check'.  Stop.

Test suite failed, dumping logs.
command "make" "check" "-j" "2" "CC=gcc" failed with status 2



Best regards,
David

---------------

Here's a new patch which has the fixes you mentioned, except I still 
haven't readded the configure and test phases as I don't know how to 
make them work.

Best regards,
David

[...]
> +(define-public prips
> +  (package
> +    (name "prips")
> +    (version "1.1.1")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append "https://devel.ringlet.net/files/sys/"
> +                           name "/" name "-" version ".tar.xz"))
> +       (sha256
> +        (base32 "1a33vbl4w603mk6mm5r3vhk87fy3dfk5wdpch0yd3ncbkg3fmvqn"))))
> +    (build-system gnu-build-system)

Add ‘(native-inputs `(("perl-test-harness" ,perl-test-harness)))’ such that
the makefile can find the 'prove' binary.

> +    (arguments
> +     `(#:make-flags (list (string-append "CC=" ,(cc-for-target)))

Add ‘#:test-target "test"’ here, as the makefile has a 'test' target
instead of a 'check' target.

> +       #:phases (modify-phases %standard-phases
> +                  (delete 'configure)
> +                  (delete 'check)

and remove the (delete 'check).

The package now builds successfully for me.

Greetings,
Maxime.