[bug#55891] [PATCH] gnu: Add iec16022

[bug#55891] [PATCH] gnu: Add iec16022

[Remco van 't Veer]

* gnu/packages/iec16022.scm: New file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
---
 gnu/local.mk              |  1 +
 gnu/packages/iec16022.scm | 56 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)
 create mode 100644 gnu/packages/iec16022.scm

diff --git a/gnu/local.mk b/gnu/local.mk
index abd6a30d66..8b3ad229aa 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -313,6 +313,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/packages/image-processing.scm	\
   %D%/packages/image-viewers.scm	\
   %D%/packages/imagemagick.scm			\
+  %D%/packages/iec16022.scm		\
   %D%/packages/inklingreader.scm		\
   %D%/packages/inkscape.scm			\
   %D%/packages/installers.scm			\
diff --git a/gnu/packages/iec16022.scm b/gnu/packages/iec16022.scm
new file mode 100644
index 0000000000..b8b0fe0176
--- /dev/null
+++ b/gnu/packages/iec16022.scm
@@ -0,0 +1,56 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2022 Remco van 't Veer <remco@remworks.net>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu packages iec16022)
+  #:use-module (guix packages)
+  #:use-module (guix git-download)
+  #:use-module (guix build-system gnu)
+  #:use-module (guix licenses)
+  #:use-module (guix gexp)
+  #:use-module (gnu packages autotools)
+  #:use-module (gnu packages popt))
+
+(define-public iec16022
+  (package
+    (name "iec16022")
+    (version "0.3.1")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/rdoeffinger/iec16022")
+                    (commit "c9a9fd926fd1e6cca9782fe3f8c2bab66010ca51")))
+              (file-name (git-file-name name version))
+              (sha256
+               (base32
+                "1k9hfcjcm64w0n20979k2bx5lmxqhsh6df11qxyjdy4aq0l6i62z"))))
+    (build-system gnu-build-system)
+    (arguments
+     '(#:phases (modify-phases %standard-phases
+                  (add-before 'bootstrap 'disable-autogen-configure
+                    (lambda _
+                      (substitute* "autogen.sh"
+                        (("^\\./configure") "# nop")))))))
+    (inputs (list popt))
+    (native-inputs (list autoconf automake libtool))
+    (synopsis "Command-line tool for producing Data Matrix barcodes")
+    (description
+     "Iec16022 is a non-interactive software package for producing Data Matrix
+barcodes as specified by ISO/IEC 16022.  It produces ASCII, UTF-8, EPS, Bin,
+Stamp and PNG images.")
+    (home-page "https://github.com/rdoeffinger/iec16022")
+    (license gpl2+)))
-- 
2.36.1

[bug#55891] [PATCH] gnu: Add iec16022

[Maxime Devos]

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

Remco van 't Veer schreef op vr 10-06-2022 om 17:04 [+0200]:
> +    (arguments
> +     '(#:phases (modify-phases %standard-phases
> +                  (add-before 'bootstrap 'disable-autogen-configure
> +                    (lambda _
> +                      (substitute* "autogen.sh"
> +                        (("^\\./configure") "# nop")))))))

As per <https://logs.guix.gnu.org/guix/2022-06-09.log#194339>, I
recommend doing eliminating the ' in favour of 'list':

(arguments
  (list #:phases
        #~(modify-phases [...])))

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#55891] [PATCH] gnu: Add iec16022

[Maxime Devos]

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

Remco van 't Veer schreef op vr 10-06-2022 om 17:04 [+0200]:
> * gnu/packages/iec16022.scm: New file.

We usually don't do per-package modules in Guix, maybe it would fit
next to the barcode packages in (gnu packages aidc)?

Though aidc refers to a particular company, so maybe rename (gnu
packages aidc) to (gnu packages barcodes) first?

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#55891] [PATCH] gnu: Add iec16022

[Maxime Devos]

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

Remco van 't Veer schreef op vr 10-06-2022 om 17:04 [+0200]:
> +     "Iec16022 is a non-interactive software package for producing Data Matrix
> +barcodes as specified by ISO/IEC 16022.  It produces ASCII, UTF-8, EPS, Bin,
> +Stamp and PNG images.")

Can be improved by using the @acronym TeXinfo markup for ISO, IEC,
ASCII, UTF-8, EPS and PNG.  Or maybe that would be TMI here, dunno.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#55891] [PATCH] gnu: Add iec16022

[Maxime Devos]

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

Remco van 't Veer schreef op vr 10-06-2022 om 17:04 [+0200]:
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://github.com/rdoeffinger/iec16022")
> +                    (commit "c9a9fd926fd1e6cca9782fe3f8c2bab66010ca51")))
> +              (file-name (git-file-name name version))
> +              (sha256

Contains a bug: on line

https://github.com/rdoeffinger/iec16022/blob/c9a9fd926fd1e6cca9782fe3f8c2bab66010ca51/image.c#L284

, it allocates a buffer, which can fail (by returning NULL).
It then passes the possibly NULL buffer to compress2.
Also, it writes files without checking for errors
(e.g. EIO, ENOSPC or EDQUOT).

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#55891] [PATCH v2] gnu: Add iec16022.

[Remco van 't Veer]

* gnu/packages/aidc.scm (iec16022): New variable.
---

Changes in this v2:
* Improved (arguments ..) style
* Moved definition to aidc.scm

 gnu/packages/aidc.scm | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/gnu/packages/aidc.scm b/gnu/packages/aidc.scm
index 5482261a59..90341320b3 100644
--- a/gnu/packages/aidc.scm
+++ b/gnu/packages/aidc.scm
@@ -26,6 +26,7 @@ (define-module (gnu packages aidc)
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (guix packages)
   #:use-module (guix download)
+  #:use-module (guix gexp)
   #:use-module (guix git-download)
   #:use-module (gnu packages autotools)
   #:use-module (gnu packages check)
@@ -36,6 +37,7 @@ (define-module (gnu packages aidc)
   #:use-module (gnu packages image)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages pkg-config)
+  #:use-module (gnu packages popt)
   #:use-module (gnu packages python)
   #:use-module (gnu packages python-xyz)
   #:use-module (gnu packages pretty-print)
@@ -257,3 +259,34 @@ (define-public qrcodegen-cpp
 project also offers Java, Javascript, Python, C, and Rust implementations.")
     (home-page "https://www.nayuki.io/page/qr-code-generator-library")
     (license license:expat)))
+
+(define-public iec16022
+  (package
+    (name "iec16022")
+    (version "0.3.1")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/rdoeffinger/iec16022")
+                    (commit "c9a9fd926fd1e6cca9782fe3f8c2bab66010ca51")))
+              (file-name (git-file-name name version))
+              (sha256
+               (base32
+                "1k9hfcjcm64w0n20979k2bx5lmxqhsh6df11qxyjdy4aq0l6i62z"))))
+    (build-system gnu-build-system)
+    (arguments
+     (list #:phases
+           #~(modify-phases %standard-phases
+               (add-before 'bootstrap 'disable-autogen-configure
+                 (lambda _
+                   (substitute* "autogen.sh"
+                     (("^\\./configure") "# nop")))))))
+    (inputs (list popt))
+    (native-inputs (list autoconf automake libtool))
+    (synopsis "Command-line tool for producing Data Matrix barcodes")
+    (description
+     "Iec16022 is a non-interactive software package for producing Data Matrix
+barcodes as specified by ISO/IEC 16022.  It produces ASCII, UTF-8, EPS, Bin,
+Stamp and PNG images.")
+    (home-page "https://github.com/rdoeffinger/iec16022")
+    (license license:gpl2+)))
-- 
2.36.1

[bug#55891] [PATCH] gnu: Add iec16022

[Remco van 't Veer]

2022/06/10 17:42, Maxime Devos:

> Remco van 't Veer schreef op vr 10-06-2022 om 17:04 [+0200]:
>> +    (arguments
>> +     '(#:phases (modify-phases %standard-phases
>> +                  (add-before 'bootstrap 'disable-autogen-configure
>> +                    (lambda _
>> +                      (substitute* "autogen.sh"
>> +                        (("^\\./configure") "# nop")))))))
>
> As per <https://logs.guix.gnu.org/guix/2022-06-09.log#194339>, I
> recommend doing eliminating the ' in favour of 'list':
>
> (arguments
>   (list #:phases
>         #~(modify-phases [...])))

done

[bug#55891] [PATCH] gnu: Add iec16022

[Remco van 't Veer]

2022/06/10 17:45, Maxime Devos:

> Remco van 't Veer schreef op vr 10-06-2022 om 17:04 [+0200]:
>> * gnu/packages/iec16022.scm: New file.
>
> We usually don't do per-package modules in Guix, maybe it would fit
> next to the barcode packages in (gnu packages aidc)?
>
> Though aidc refers to a particular company, so maybe rename (gnu
> packages aidc) to (gnu packages barcodes) first?
>
> Greetings,
> Maxime.

Done, minus rename, I don't feel comfortable doing that.

[bug#55891] [PATCH] gnu: Add iec16022

[Remco van 't Veer]

2022/06/10 17:47, Maxime Devos:

> Remco van 't Veer schreef op vr 10-06-2022 om 17:04 [+0200]:
>> +     "Iec16022 is a non-interactive software package for producing Data Matrix
>> +barcodes as specified by ISO/IEC 16022.  It produces ASCII, UTF-8, EPS, Bin,
>> +Stamp and PNG images.")
>
> Can be improved by using the @acronym TeXinfo markup for ISO, IEC,
> ASCII, UTF-8, EPS and PNG.  Or maybe that would be TMI here, dunno.

TMI IMHO.  No other package descriptions bother to use @acronym for
thing like ASCII, PNG etc.

[bug#55891] [PATCH] gnu: Add iec16022

[Remco van 't Veer]

2022/06/10 17:55, Maxime Devos:

> Remco van 't Veer schreef op vr 10-06-2022 om 17:04 [+0200]:
>> +    (source (origin
>> +              (method git-fetch)
>> +              (uri (git-reference
>> +                    (url "https://github.com/rdoeffinger/iec16022")
>> +                    (commit "c9a9fd926fd1e6cca9782fe3f8c2bab66010ca51")))
>> +              (file-name (git-file-name name version))
>> +              (sha256
>
> Contains a bug: on line
>
> https://github.com/rdoeffinger/iec16022/blob/c9a9fd926fd1e6cca9782fe3f8c2bab66010ca51/image.c#L284
>
> , it allocates a buffer, which can fail (by returning NULL).
> It then passes the possibly NULL buffer to compress2.
> Also, it writes files without checking for errors
> (e.g. EIO, ENOSPC or EDQUOT).

Good catch!  I'll need to brush up on my C skills to fix these.
Meanwhile I'd like to ship including bugs, WDYT?

[bug#55891] [PATCH v3] gnu: Add iec16022.

[Remco van 't Veer]

* gnu/packages/aidc.scm (iec16022): New variable.
---
Changes in this v3:
* Added copyright line

 gnu/packages/aidc.scm | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/gnu/packages/aidc.scm b/gnu/packages/aidc.scm
index 5482261a59..2847b591ab 100644
--- a/gnu/packages/aidc.scm
+++ b/gnu/packages/aidc.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2018, 2019 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2019 Guillaume Le Vaillant <glv@posteo.net>
 ;;; Copyright © 2020 Leo Famulari <leo@famulari.name>
+;;; Copyright © 2022 Remco van 't Veer <remco@remworks.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -26,6 +27,7 @@ (define-module (gnu packages aidc)
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (guix packages)
   #:use-module (guix download)
+  #:use-module (guix gexp)
   #:use-module (guix git-download)
   #:use-module (gnu packages autotools)
   #:use-module (gnu packages check)
@@ -36,6 +38,7 @@ (define-module (gnu packages aidc)
   #:use-module (gnu packages image)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages pkg-config)
+  #:use-module (gnu packages popt)
   #:use-module (gnu packages python)
   #:use-module (gnu packages python-xyz)
   #:use-module (gnu packages pretty-print)
@@ -257,3 +260,34 @@ (define-public qrcodegen-cpp
 project also offers Java, Javascript, Python, C, and Rust implementations.")
     (home-page "https://www.nayuki.io/page/qr-code-generator-library")
     (license license:expat)))
+
+(define-public iec16022
+  (package
+    (name "iec16022")
+    (version "0.3.1")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/rdoeffinger/iec16022")
+                    (commit "c9a9fd926fd1e6cca9782fe3f8c2bab66010ca51")))
+              (file-name (git-file-name name version))
+              (sha256
+               (base32
+                "1k9hfcjcm64w0n20979k2bx5lmxqhsh6df11qxyjdy4aq0l6i62z"))))
+    (build-system gnu-build-system)
+    (arguments
+     (list #:phases
+           #~(modify-phases %standard-phases
+               (add-before 'bootstrap 'disable-autogen-configure
+                 (lambda _
+                   (substitute* "autogen.sh"
+                     (("^\\./configure") "# nop")))))))
+    (inputs (list popt))
+    (native-inputs (list autoconf automake libtool))
+    (synopsis "Command-line tool for producing Data Matrix barcodes")
+    (description
+     "Iec16022 is a non-interactive software package for producing Data Matrix
+barcodes as specified by ISO/IEC 16022.  It produces ASCII, UTF-8, EPS, Bin,
+Stamp and PNG images.")
+    (home-page "https://github.com/rdoeffinger/iec16022")
+    (license license:gpl2+)))
-- 
2.36.1

[bug#55891] [PATCH] gnu: Add iec16022

[Maxime Devos]

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

Remco van 't Veer schreef op vr 10-06-2022 om 19:39 [+0200]:
> > Contains a bug: on line
> > 
> > https://github.com/rdoeffinger/iec16022/blob/c9a9fd926fd1e6cca9782fe3f8c2bab66010ca51/image.c#L284
> > 
> > , it allocates a buffer, which can fail (by returning NULL).
> > It then passes the possibly NULL buffer to compress2.
> > Also, it writes files without checking for errors
> > (e.g. EIO, ENOSPC or EDQUOT).
> 
> Good catch!  I'll need to brush up on my C skills to fix these.
> Meanwhile I'd like to ship including bugs, WDYT?

I think it's ok, _if_ the bugs are eventually at least reported
upstream if we (i.e., you) cannot fix them timely (needs some contrived
circumstances to trigger the bug).

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#55891] [PATCH] gnu: Add iec16022

[Remco van 't Veer]

2022/06/10 23:47, Maxime Devos:

> Remco van 't Veer schreef op vr 10-06-2022 om 19:39 [+0200]:
>> > Contains a bug: on line
>> >
>> > https://github.com/rdoeffinger/iec16022/blob/c9a9fd926fd1e6cca9782fe3f8c2bab66010ca51/image.c#L284
>> >
>> > , it allocates a buffer, which can fail (by returning NULL).
>> > It then passes the possibly NULL buffer to compress2.
>> > Also, it writes files without checking for errors
>> > (e.g. EIO, ENOSPC or EDQUOT).
>>
>> Good catch!  I'll need to brush up on my C skills to fix these.
>> Meanwhile I'd like to ship including bugs, WDYT?
>
> I think it's ok, _if_ the bugs are eventually at least reported
> upstream if we (i.e., you) cannot fix them timely (needs some contrived
> circumstances to trigger the bug).

https://github.com/rdoeffinger/iec16022/pull/17

[bug#55891] [PATCH] gnu: Add iec16022

[Remco van 't Veer]

2022/06/10 23:47, Maxime Devos:

> I think it's ok, _if_ the bugs are eventually at least reported
> upstream if we (i.e., you) cannot fix them timely (needs some
> contrived circumstances to trigger the bug).

The maintainer's stance on not checking malloc response (from
https://github.com/rdoeffinger/iec16022/pull/17#issuecomment-1152892833):

> > I'll open an issue for the unchecked [mc]alloc and write calls
> > instead if you do and maybe have another stab at fixing these issues
> > when my C-programming skills have improved.
>
> I agree on the unchecked writes. For the unchecked [mc]alloc I am more
> tempted towards removing the existing checks, since I am not convinced
> checking won't always do more harm than good for those.

He elaborated on this at:

  https://github.com/rdoeffinger/iec16022/pull/17#issuecomment-1153103749

Boiling down to: when memory is that low exiting with a proper error
messages and doing cleanup probably won't work anyway and the OOM killer
will have swept in before the situation exists.  Which makes sense,
IMHO, but I am no expert.

I've created an issue for the unchecked writes:

  https://github.com/rdoeffinger/iec16022/issues/18

HTH,
Remco

[bug#55891] [PATCH] gnu: Add iec16022

[Remco van 't Veer]

Is there anything else I can do to keep this ticket moving?  Bug about
unchecked IO is reported and discussion about unchecked malloc has
stalled.

Also note, this package is also being shipped by debian, archlinux,
fedora, and others.  Not that it matters but just to point out it's a
widely spread utility.

Cheers,
Remco

[bug#55891] [PATCH] gnu: Add iec16022

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 1388 bytes --]

On 01-07-2022 08:58, Remco van 't Veer wrote:

> Is there anything else I can do to keep this ticket moving?  Bug about
> unchecked IO is reported and discussion about unchecked malloc has
> stalled.

You could search for an alternative solution that is acceptable to 
upstream, or if upstream completely refuses bugfixes, add some patch to 
the Guix package definition.  Also, I have not yet seen a v2 for 
resolving the other (non-upstream-code) issues I noted.

Another method would be to help out with reviewing and fixing things 
(with other packages), this frees up time and is seen as a good act, 
hence making other people more inclined to help out with any remaining 
issues in your patches (including fixing issues such as those of the 
kind I have mentioned before, but also actually applying the patch and 
committing it).

You could also perform _all_ the checks in (guix)Submitting Patches 
(e.g., the bundling check, actually building it for other systems as 
well with QEMU, reproducibility check).

> Also note, this package is also being shipped by debian, archlinux,
> fedora, and others.  Not that it matters but just to point out it's a
> widely spread utility.

I am not convinced by your implied method of persuasion by social 
pressure / argumentum ad populum. If it doesn't matter, why use it as an 
argument?

Greetings,
Maxime.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

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

[bug#55891] [PATCH] gnu: Add iec16022

[Remco van 't Veer]

Hi Maxime,

Sorry for the late reply, I was out camping.

2022/07/31 01:48, Maxime Devos:

> On 01-07-2022 08:58, Remco van 't Veer wrote:
>
>> Is there anything else I can do to keep this ticket moving?  Bug about
>> unchecked IO is reported and discussion about unchecked malloc has
>> stalled.
>
> You could search for an alternative solution that is acceptable to
> upstream, or if upstream completely refuses bugfixes, add some patch
> to the Guix package definition.  Also, I have not yet seen a v2 for
> resolving the other (non-upstream-code) issues I noted.

The patches I did submit upstream were not accepted and I agree with the
arguments of the maintainer so I won't add these to this package
definition.

  https://github.com/rdoeffinger/iec16022/pull/17

Also, I did make a v2 but forgot to cc you, sorry about that:

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55891#20

Also did a v3 because the copyright line was missing (and failed to cc
you again):

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55891#35

> Another method would be to help out with reviewing and fixing things
> (with other packages), this frees up time and is seen as a good act,
> hence making other people more inclined to help out with any remaining
> issues in your patches (including fixing issues such as those of the
> kind I have mentioned before, but also actually applying the patch and
> committing it).

That's a great tip and I will try and leap over my newby/shyness
feelings to get into that.  Getting feedback like yours has certainly
thought me a lot about what to look out for.  Thank you for your
feedback so far, also on the other patches I've submitted!

> You could also perform _all_ the checks in (guix)Submitting Patches
> (e.g., the bundling check, actually building it for other systems as
> well with QEMU, reproducibility check).

Just ran through them again and all seems to be fine.  I am not sure how
to prove I did.

>> Also note, this package is also being shipped by debian, archlinux,
>> fedora, and others.  Not that it matters but just to point out it's a
>> widely spread utility.
>
> I am not convinced by your implied method of persuasion by social
> pressure / argumentum ad populum. If it doesn't matter, why use it as
> an argument?

What I should have written is I am moving and application from
ubuntu/debian to guix and am missing this package.  We already use a
custom channel to make development possible but in doing to I was
tickled by (guix)Creating a Channel mentioning:

  Before publishing a channel, please consider contributing your package
  definitions to Guix proper (*note Contributing::).  Guix as a project
  is open to free software of all sorts, and packages in Guix proper are
  readily available to all Guix users and benefit from the project’s
  quality assurance process.

Cheers,
Remco