diff mbox series

[bug#61686] gnu: Add dropwatch.

Message ID 43b683614e136e70894ba395a6759dc714fe2faf.1677006911.git.mirai@makinata.eu
State New
Headers show
Series [bug#61686] gnu: Add dropwatch. | expand

Commit Message

Bruno Victal Feb. 21, 2023, 7:19 p.m. UTC
* gnu/packages/networking.scm (dropwatch): New variable.
---

Note: the kernel option NET_DROP_MONITOR must be set for this package to work.

The upstream sources use SPDX (though they're not completely REUSE compliant)
$ reuse lint
# MISSING LICENSES

'GPL-2.0-or-later' found in:
* Makefile.am
* autogen.sh
* configure.ac
* doc/Makefile.am
* src/Makefile.am
* src/dwdump.c
* src/lookup.c
* src/lookup.h
* src/lookup_bfd.c
* src/lookup_kas.c
* src/main.c
* src/net_dropmon.h


# MISSING COPYRIGHT AND LICENSING INFORMATION

The following files have no copyright and licensing information:
* .travis.yml
* README.md
* doc/dropwatch.1
* doc/dwdump.1
* spec/dropwatch.spec
* tests/Makefile.am
* tests/rundropwatch.sh

The following files have no copyright information:
* autogen.sh
* configure.ac
* doc/Makefile.am
* src/Makefile.am
* src/dwdump.c
* src/net_dropmon.h


# SUMMARY

* Bad licenses:
* Deprecated licenses:
* Licenses without file extension:
* Missing licenses: GPL-2.0-or-later
* Unused licenses:
* Used licenses: GPL-2.0-or-later
* Read errors: 0
* Files with copyright information: 6 / 19
* Files with license information: 12 / 19

Unfortunately, your project is not compliant with version 3.0 of the REUSE Specification :-(



 gnu/packages/networking.scm | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Nicolas Goaziou Feb. 24, 2023, 10:44 a.m. UTC | #1
Hello,

Bruno Victal <mirai@makinata.eu> writes:

> * gnu/packages/networking.scm (dropwatch): New variable.

Thanks.
> ---
> Note: the kernel option NET_DROP_MONITOR must be set for this package
> to work.

Do you think it is worth mentioning in the description? AFAICT, Guix
does not set this option by default.

> +(define-public dropwatch
> +  (package
> +    (name "dropwatch")
> +    (version "1.5.4")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://github.com/nhorman/dropwatch.git")

You can drop the ".git" suffix here.

> +                    (commit (string-append "v" version))))

You also need to add

  (file-name (git-file-name name version))

for proper naming in the store.

> +              (sha256
> +               (base32
> +                "1r653y7bx763fpxl1vrflx8bzcrbds98zk4z7yhfikjngrqn1f2d"))))
> +    (build-system gnu-build-system)
> +    ;; XXX: bfd support isn't finished.
> +    ;; https://github.com/nhorman/dropwatch/issues/76#issuecomment-1328345444
> +    (arguments
> +     (list #:configure-flags #~(list "--without-bfd")))
> +    (native-inputs (list autoconf automake pkg-config))
> +    (inputs (list libnl libpcap readline))
> +    (home-page "https://github.com/nhorman/dropwatch")
> +    (synopsis "Kernel dropped packet monitor")
> +    (description
> +     "dropwatch is an interactive utility for monitoring and

Nicpick: dropwatch —> Dropwatch

> +recording packets that are dropped by the kernel. It provides the commands

You need separate sentences with two spaces, per Texinfo syntax.

> +@command{dropwatch} and @command{dwdump}.")
> +    (license license:gpl2+)))

Regards,
Bruno Victal Feb. 25, 2023, 7:21 p.m. UTC | #2
Hi Nicolas,

On 2023-02-24 10:44, Nicolas Goaziou wrote:
>> Note: the kernel option NET_DROP_MONITOR must be set for this package
>> to work.
> 
> Do you think it is worth mentioning in the description? AFAICT, Guix
> does not set this option by default.

With e701f67246680c81dd111cf7136b936bf7d8718b, this is now set by default.

>> +(define-public dropwatch
>> +  (package
>> +    (name "dropwatch")
>> +    (version "1.5.4")
>> +    (source (origin
>> +              (method git-fetch)
>> +              (uri (git-reference
>> +                    (url "https://github.com/nhorman/dropwatch.git")
> 
> You can drop the ".git" suffix here.

Is there a reason for dropping the suffix for GitHub? AFAIK it "works" but it's not
the same url that they give out for cloning.

>> +                    (commit (string-append "v" version))))
> 
> You also need to add
> 
>   (file-name (git-file-name name version))
> 
> for proper naming in the store.

Is this always required? I assumed the default if unspecified was "good enough" and
that snippet was used for repositories with problematic names.

>> +    (home-page "https://github.com/nhorman/dropwatch")
>> +    (synopsis "Kernel dropped packet monitor")
>> +    (description
>> +     "dropwatch is an interactive utility for monitoring and
> 
> Nicpick: dropwatch —> Dropwatch

Noted.

> 
>> +recording packets that are dropped by the kernel. It provides the commands
> 
> You need separate sentences with two spaces, per Texinfo syntax.

This is actually not strictly necessary for strings embedded in scm files. The texinfo extractor is smart enough to automatically space the strings for us here.
You can verify this behavior with 'guix search dropwatch' and see that it automatically inserts two spaces correctly.


Cheers,
Bruno
Tobias Geerinckx-Rice Feb. 25, 2023, 8:01 p.m. UTC | #3
Hi Bruno,

On 2023-02-25 20:21, Bruno Victal wrote:
>> Do you think it is worth mentioning in the description? AFAICT, Guix
>> does not set this option by default.
> 
> With e701f67246680c81dd111cf7136b936bf7d8718b, this is now set by 
> default.

…on all architectures: it was inconsistently built-in on ARM, and 
missing from x86.  It's now modular everywhere.

\o/

To my pleasant surprise, it's automatically loaded when I start 
dropwatch as a regular user.

\o/

> Is there a reason for dropping the suffix for GitHub? AFAIK it "works"
> but it's not the same url that they give out for cloning.

It works, no quotes.  No technical reason to drop it, but many people 
do, perhaps so it matches the ‘home page’ many GitHubbed projects now 
lack.  It's also provably shorter.

If you feel strongly about keeping it, there's precedent in Guix.  It's 
just very rare (2.2%).  Consistency has its charms.

>> You also need to add
>> 
>>   (file-name (git-file-name name version))
>> 
>> for proper naming in the store.
> 
> Is this always required?

Yes, for git checkouts.

> I assumed the default if unspecified was "good enough" and
> that snippet was used for repositories with problematic names.

Nope.  It'll clone & build fine, but please set a meaningful name for 
humans instead of:

   /gnu/store/ja3blcdqia00k15vpayfc9kzsjijfpw0-git-checkout

Which is also a bonus footgun when someone inexperienced tries to update 
the package without changing the hash, because there's no version+commit 
information in the name.

>>> +    (home-page "https://github.com/nhorman/dropwatch")
>>> +    (synopsis "Kernel dropped packet monitor")

‘Monitor [for] dropped network packets’, or ‘Monitor [for] network 
packets dropped by the kernel’, maybe?  The above (RedHat) version just 
doesn't parse naturally for me.

>> You need separate sentences with two spaces, per Texinfo syntax.
> 
> This is actually not strictly necessary

Still.

Kind regards,

T G-R

Sent from a Web browser.  Excuse or enjoy my brevity.
diff mbox series

Patch

diff --git a/gnu/packages/networking.scm b/gnu/packages/networking.scm
index 44e2de5f08..daac734535 100644
--- a/gnu/packages/networking.scm
+++ b/gnu/packages/networking.scm
@@ -56,6 +56,7 @@ 
 ;;; Copyright © 2022 Reza Alizadeh Majd <r.majd@pantherx.org>
 ;;; Copyright © 2022 Nicolas Graves <ngraves@ngraves.fr>
 ;;; Copyright © 2023 Andreas Enge <andreas@enge.fr>
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -4619,3 +4620,30 @@  (define-public vnstat
 that vnStat won't actually be sniffing any traffic and also ensures light use
 of system resources regardless of network traffic rate.")
    (license license:gpl2+)))
+
+(define-public dropwatch
+  (package
+    (name "dropwatch")
+    (version "1.5.4")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/nhorman/dropwatch.git")
+                    (commit (string-append "v" version))))
+              (sha256
+               (base32
+                "1r653y7bx763fpxl1vrflx8bzcrbds98zk4z7yhfikjngrqn1f2d"))))
+    (build-system gnu-build-system)
+    ;; XXX: bfd support isn't finished.
+    ;; https://github.com/nhorman/dropwatch/issues/76#issuecomment-1328345444
+    (arguments
+     (list #:configure-flags #~(list "--without-bfd")))
+    (native-inputs (list autoconf automake pkg-config))
+    (inputs (list libnl libpcap readline))
+    (home-page "https://github.com/nhorman/dropwatch")
+    (synopsis "Kernel dropped packet monitor")
+    (description
+     "dropwatch is an interactive utility for monitoring and
+recording packets that are dropped by the kernel. It provides the commands
+@command{dropwatch} and @command{dwdump}.")
+    (license license:gpl2+)))