diff mbox series

[bug#46376] gnu: tesseract-ocr: update to 4.1.1)

Message ID CAJsg1E_GNDOpWfHd-nUPVk7VuAgfSFcUnG-tEZQGy-utzrPcTA@mail.gmail.com
State Accepted
Headers show
Series [bug#46376] gnu: tesseract-ocr: update to 4.1.1) | expand

Checks

Context Check Description
cbaines/submitting builds success
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Andy Tai Feb. 8, 2021, 11:14 p.m. UTC 
updated patch attached


On Mon, Feb 8, 2021 at 1:36 PM Jelle Licht <jlicht@posteo.net> wrote:
>
>
> > * gnu/packages/ocr.scm (tesseract-ocr): update to 4.1.1
>                                           ^ This is usually capitalised.

capitalized

> > [snip]
> >  (define-public tesseract-ocr
> > +  ;; some useful commits beyond last official stable release in release branch
> > +  (let ((commit "97079fa353557af6df86fd20b5d2e0dff5d8d5df"))
> >    (package
> >      (name "tesseract-ocr")
> > -    (version "3.04.01")
> > +    (version "4.1.1")
>                 ^ Since we are not _actually_ using version 4.1.1, but a
>                   later commit, you could use
>                   `(git-version "4.1.1" revision commit)' here instead

changed as suggested
>
> >      (source
> >       (origin
> >         (method git-fetch)
> >         (uri (git-reference
> >                (url "https://github.com/tesseract-ocr/tesseract")
> > -              (commit version)))
> > -       (file-name (git-file-name name version))
> > +              (commit commit)
> > +              ;; Fetch git submodules
> > +              (recursive? #t)))
>
> Instead of stating what the code does, would you consider adding a
> comment why this is needed?
>

commented as suggested

> > +       (file-name (string-append name "-" version "-" commit "-checkout"))
>                      ^ You can use `(git-file-name name version)' here.
>

changed as suggested

> >         (sha256
> > -        (base32 "0h1x4z1h86n2gwknd0wck6gykkp99bmm02lg4a47a698g4az6ybv"))))
> > +        (base32 "0axwla82fpzp86lc553wp3hk0fz5dylw4as0jbf4hkqcyajlbzp4"))))
> >      (build-system gnu-build-system)
> >      (inputs
> > -     `(("leptonica" ,leptonica)))
> > +     `(  ("cairo" ,cairo)
> > +         ("icu" ,icu4c)
> > +         ("leptonica" ,leptonica)
> > +         ("pango" ,pango)))
> > +    (native-inputs
> > +     `(("autoconf" ,autoconf)
> > +       ("autoconf-archive" ,autoconf-archive)
> > +       ("automake" ,automake)
> > +       ("googletest" ,googletest)
> > +       ("libtool" ,libtool)
> > +       ("pkg-config" ,pkg-config)))
> >      (arguments
> >       '(#:configure-flags
> >         (let ((leptonica (assoc-ref %build-inputs "leptonica")))
> > -         (list (string-append "LIBLEPT_HEADERSDIR=" leptonica "/include")))))
> > +         (list (string-append "LIBLEPT_HEADERSDIR=" leptonica "/include")))
> > +       ;; some test, applybox_test fails to build
> > +       #:tests? #f))
>  2 nits: Is it possible to patch or disable only the failing tests?

tests failing to build probably due to some issue with parallel
builds; did not dig into it as probably will take much time; will be
TODO if time allows)

>  Is there a reason googletest is added if testing is subsequently disabled?
>

removed

> Thanks,
>  - Jelle



--
Andy Tai, atai@atai.org, Skype: licheng.tai, Line: andy_tai, WeChat: andytai1010
Year 2021 民國110年
自動的精神力是信仰與覺悟
自動的行為力是勞動與技能

Comments

Jelle Licht Feb. 9, 2021, 11:56 a.m. UTC | #1 
Hello Andy,

I have some additional questions about your updated patch.

Andy Tai <atai@atai.org> writes:

> updated patch attached
>> > -       (file-name (git-file-name name version))
>> > +              (commit commit)
>> > +              ;; Fetch git submodules
>> > +              (recursive? #t)))
>>
>> Instead of stating what the code does, would you consider adding a
>> comment why this is needed?
>>
>
> commented as suggested

I was unclear in my message: Of course there are some submodules that
are being fetched; why should we fetch them in the first place? From
a cursory glance, they seem required to do stuff such as running the
tests, which in this iteration of the patch are not being run.

FWIW, leaving out the `recursive? #t' still allows me to build
tesseract-ocr: could you try and see if it makes a difference in your
use of tesseract?

>> >      (inputs
>> > -     `(("leptonica" ,leptonica)))
>> > +     `(  ("cairo" ,cairo)
>> > +         ("icu" ,icu4c)
>> > +         ("leptonica" ,leptonica)
>> > +         ("pango" ,pango)))

I just built the package: no references are made to cairo, icu4c and
pango, and tesseract seems to build fine without them: is there a
specific reason why these were added to the inputs?

>> > +    (native-inputs
>> > +     `(("autoconf" ,autoconf)
>> > +       ("autoconf-archive" ,autoconf-archive)
>> > +       ("automake" ,automake)
>> > +       ("googletest" ,googletest)
>> > +       ("libtool" ,libtool)
>> > +       ("pkg-config" ,pkg-config)))
>> >      (arguments
>> >       '(#:configure-flags
>> >         (let ((leptonica (assoc-ref %build-inputs "leptonica")))
>> > -         (list (string-append "LIBLEPT_HEADERSDIR=" leptonica "/include")))))
>> > +         (list (string-append "LIBLEPT_HEADERSDIR=" leptonica "/include")))
>> > +       ;; some test, applybox_test fails to build
>> > +       #:tests? #f))
>>  2 nits: Is it possible to patch or disable only the failing tests?
>
> tests failing to build probably due to some issue with parallel
> builds; did not dig into it as probably will take much time; will be
> TODO if time allows)

I'll defer to someone with more experience with tesseract, as I have no
experience to speak of on whether this leads to us having a (subtly)
broken package.

Adding the following arguments might help to validate your assumption:
`#:make-flags (list "-j" "1")'.

Thanks,
 - Jelle
diff mbox series

Patch

From 7ab7fcbb95d933f6a7ba3e688131caa0435bdfdd Mon Sep 17 00:00:00 2001
From: Andy Tai <atai@atai.org>
Date: Sun, 7 Feb 2021 22:23:47 -0800
Subject: [PATCH] gnu: tesseract-ocr: Update to 4.1.1

* gnu/packages/ocr.scm (tesseract-ocr): Update to 4.1.1
---
 gnu/packages/ocr.scm | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/gnu/packages/ocr.scm b/gnu/packages/ocr.scm
index dc4930918a..0d357726b2 100644
--- a/gnu/packages/ocr.scm
+++ b/gnu/packages/ocr.scm
@@ -3,6 +3,7 @@ 
 ;;; Copyright © 2016, 2020 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2019 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2019 Alex Vong <alexvong1995@gmail.com>
+;;; Copyright © 2021 Andy Tai <atai@atai.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -26,7 +27,12 @@ 
   #:use-module (guix git-download)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system python)
+  #:use-module (gnu packages autotools)
+  #:use-module (gnu packages check)
   #:use-module (gnu packages compression)
+  #:use-module (gnu packages gtk)
+  #:use-module (gnu packages icu4c)
+  #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages python)
   #:use-module (gnu packages image))
 
@@ -52,25 +58,41 @@  it produces text in 8-bit or UTF-8 formats.")
     (license license:gpl3+)))
 
 (define-public tesseract-ocr
+  ;; some useful commits beyond last official stable release in release branch
+  (let ((commit "97079fa353557af6df86fd20b5d2e0dff5d8d5df")
+        (revision "1"))
   (package
     (name "tesseract-ocr")
-    (version "3.04.01")
+    (version (git-version "4.1.1" revision commit))
     (source
      (origin
        (method git-fetch)
        (uri (git-reference
               (url "https://github.com/tesseract-ocr/tesseract")
-              (commit version)))
+              (commit commit)
+              ;; source git repo with submodules; ensure they are fetched
+              (recursive? #t)))
        (file-name (git-file-name name version))
        (sha256
-        (base32 "0h1x4z1h86n2gwknd0wck6gykkp99bmm02lg4a47a698g4az6ybv"))))
+        (base32 "0axwla82fpzp86lc553wp3hk0fz5dylw4as0jbf4hkqcyajlbzp4"))))
     (build-system gnu-build-system)
     (inputs
-     `(("leptonica" ,leptonica)))
+     `(  ("cairo" ,cairo)
+         ("icu" ,icu4c)
+         ("leptonica" ,leptonica)
+         ("pango" ,pango)))
+    (native-inputs
+     `(("autoconf" ,autoconf)
+       ("autoconf-archive" ,autoconf-archive)
+       ("automake" ,automake)
+       ("libtool" ,libtool)
+       ("pkg-config" ,pkg-config)))
     (arguments
      '(#:configure-flags
        (let ((leptonica (assoc-ref %build-inputs "leptonica")))
-         (list (string-append "LIBLEPT_HEADERSDIR=" leptonica "/include")))))
+         (list (string-append "LIBLEPT_HEADERSDIR=" leptonica "/include")))
+       ;; some test, applybox_test fails to build
+       #:tests? #f))
     (home-page "https://github.com/tesseract-ocr/tesseract")
     (synopsis "Optical character recognition engine")
     (description
@@ -79,7 +101,7 @@  high accuracy.  It supports many languages, output text formatting, hOCR
 positional information and page layout analysis.  Several image formats are
 supported through the Leptonica library.  It can also detect whether text is
 monospaced or proportional.")
-    (license license:asl2.0)))
+    (license license:asl2.0))))
 
 (define-public zinnia
   (let* ((commit "581faa8f6f15e4a7b21964be3a5ec36265c80e5b")
-- 
2.30.0