diff mbox series

[bug#61851] gnu: tesseract-ocr-tessdata-fast: Install tesseract config files.

Message ID fed85bc978d9469832e5aaad737a8816d5f49fa7.1677531307.git.jlicht@fsfe.org
State New
Headers show
Series [bug#61851] gnu: tesseract-ocr-tessdata-fast: Install tesseract config files. | expand

Commit Message

Jelle Licht Feb. 27, 2023, 8:55 p.m. UTC
From: Jelle Licht <jlicht@fsfe.org>

* gnu/packages/ocr.scm (tesseract-ocr-tessdata-fast)[source]: Add recursive?
flag. Adjust hash accordingly.
[arguments]<#:phases>: Remove unneeded workaround.
---

 gnu/packages/ocr.scm | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

Simon South Feb. 27, 2023, 10:43 p.m. UTC | #1
Jelle,

Respectfully, and speaking only as an interested observer, I think this
may not be the right fix.

Guix's Tesseract is indeed missing its config files, causing (among
other things) the examples in the online documentation[0] to not work,
e.g.:

  ssouth@hamlet ~/tesseract-ocr-test [env]$ tesseract images/eurotext.png - -l eng hocr
  read_params_file: Can't open hocr
  The (quick) [brown] {fox} jumps!
  Over the $43,456.78 <lazy> #90 dog
  (...)

But the root issue appears to be a misconfiguration of the
TESSDATA_PREFIX search path in the tessdata-ocr package, which causes
Tesseract's own config files to be installed in a folder other than the
one it's configured to search.

Fixing this places Tesseract's config files and the trained-data files
together beneath /usr/share/tessdata, allowing Tesseract to work as
expected:

  ssouth@hamlet ~/tesseract-ocr-test [env]$ tesseract images/eurotext.png - -l eng hocr
  <?xml version="1.0" encoding="UTF-8"?>
  <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
  (...)

This approach has the advantage of keeping the
tesseract-ocr-tessdata-fast package "pure" and focused only on
trained-data files, which will be important for the patch I'm working on
that will split it into multiple packages, one for each language and
script, to allow greater flexibility.

I'll respond to this email with a draft (!) patch to tesseract-ocr that
should achieve the same result as yours, making the config files
available for use.  Does this also fix the problem for you?  If so,
would you consider submitting this change instead?
Jelle Licht Feb. 28, 2023, 12:31 a.m. UTC | #2
Hi Simon,

Simon South <simon@simonsouth.net> writes:

> Jelle,
>
> Respectfully, and speaking only as an interested observer, I think this
> may not be the right fix.

Cunningham's law strikes again :) [1].

>
> Guix's Tesseract is indeed missing its config files, causing (among
> other things) the examples in the online documentation[0] to not work,
> e.g.:
>
>   ssouth@hamlet ~/tesseract-ocr-test [env]$ tesseract images/eurotext.png - -l eng hocr
>   read_params_file: Can't open hocr
>   The (quick) [brown] {fox} jumps!
>   Over the $43,456.78 <lazy> #90 dog
>   (...)
>
> But the root issue appears to be a misconfiguration of the
> TESSDATA_PREFIX search path in the tessdata-ocr package, which causes
> Tesseract's own config files to be installed in a folder other than the
> one it's configured to search.
>
> Fixing this places Tesseract's config files and the trained-data files
> together beneath /usr/share/tessdata, allowing Tesseract to work as
> expected:
>
>   ssouth@hamlet ~/tesseract-ocr-test [env]$ tesseract images/eurotext.png - -l eng hocr
>   <?xml version="1.0" encoding="UTF-8"?>
>   <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
>   (...)

I will believe you without any doubt, but there's this spooky comment
left in the tesseract-ocr 'adjust-TESSDATA_PREFIX-macro phase:

--8<---------------cut here---------------start------------->8---
  ;; Use a deeper TESSDATA_PREFIX hierarchy so that a more
  ;; specific search-path than '/share' can be specified.  The
  ;; build system uses CPPFLAGS for itself, so we can't simply set
  ;; a make flag.
--8<---------------cut here---------------end--------------->8---

This makes me believe the current situation was a deliberate choice, but
I personally don't understand what the original problem was/is.

> This approach has the advantage of keeping the
> tesseract-ocr-tessdata-fast package "pure" and focused only on
> trained-data files, which will be important for the patch I'm working on
> that will split it into multiple packages, one for each language and
> script, to allow greater flexibility.
>
> I'll respond to this email with a draft (!) patch to tesseract-ocr that
> should achieve the same result as yours, making the config files
> available for use.  Does this also fix the problem for you?  If so,
> would you consider submitting this change instead?

It seems to work for my stuff! I'm bringing Maxim to weigh in on this, as
they are the (un?)lucky expert according to my git-foo.

Thanks for paying attention!
- Jelle

[1] https://meta.wikimedia.org/wiki/Cunningham%27s_Law
Simon South Feb. 28, 2023, 3 p.m. UTC | #3
Jelle Licht <jlicht@fsfe.org> writes:
> Cunningham's law strikes again :)

Ha, interesting.  That one's new to me.

> This makes me believe the current situation was a deliberate choice...

Yes, it was, and I realize now I didn't provide much in the way of
rationale in my previous email.  So here's the background information
for anyone interested:

Tesseract normally expects to find its data files in /usr/share/tessdata
and subfolders thereof.  We'd like to use Guix's native-search-paths
functionality to pull together data from (for instance) multiple
language-specific data packages, and Tesseract conveniently honours a
TESSDATA_PREFIX environment variable that specifies its data folder's
location, so it seems we are all set.

What should TESSDATA_PREFIX be set to?  Tesseract's documentation[0]
says

  TESSDATA_PREFIX environment variable should be set to the parent
  directory of “tessdata” directory.

So "share" then, presumably, to have the data files located at
"share/tessdata".  The man page[1] seems to confirm this:

  To use a non-standard language pack named foo.traineddata, set the
  TESSDATA_PREFIX environment variable so the file can be found at
  TESSDATA_PREFIX/tessdata/foo.traineddata...

This creates a problem, though, since defining a native-search-path of
just "share" will pull in files from virtually every single Guix
package.  The solution then is to introduce an intermediate folder,
"tesseract-ocr", that sidesteps this problem, and to configure Tesseract
appropriately at build time so it installs its data files to
"share/tesseract-ocr/tessdata" instead.  This is why the existing code
was written the way it was and what the comment you pointed out is
referring to.

However there's a problem with this, too: Patching Makefile.am the way
the code does results in only some of Tesseract's data files being
placed in "share/tesseract-ocr/tessdata"; you can see in the package
output there is still a "share/tessdata" folder that contains
Tesseract's config files.  Since these aren't also placed beneath
"share/tesseract-ocr/tessdata" Tesseract can't find them at runtime.

The solution to this seems to be to remove this phase and instead use
the "--datadir" configure flag to specify the desired data-folder path.
Doing this results in all of Tesseract's data files being installed
beneath "share/tesseract-ocr/tessdata" and the resulting package works
as you'd expect.

However the problem with this is... none of it is necessary in the first
place!  It turns out Tesseract's documentation is simply WRONG and the
program actually expects TESSDATA_PREFIX to contain the complete path to
the "tessdata" data folder, not the path of the folder directly above
it.  So Tesseract can be built as-is, the native-search-path can be
safely defined as "share/tessdata", and everything just works.

This is what the patch I passed on yesterday does.
Maxim Cournoyer Feb. 28, 2023, 3:35 p.m. UTC | #4
Hi Simon,

Simon South <simon@simonsouth.net> writes:

> Jelle Licht <jlicht@fsfe.org> writes:
>> Cunningham's law strikes again :)
>
> Ha, interesting.  That one's new to me.
>
>> This makes me believe the current situation was a deliberate choice...
>
> Yes, it was, and I realize now I didn't provide much in the way of
> rationale in my previous email.  So here's the background information
> for anyone interested:
>
> Tesseract normally expects to find its data files in /usr/share/tessdata
> and subfolders thereof.  We'd like to use Guix's native-search-paths
> functionality to pull together data from (for instance) multiple
> language-specific data packages, and Tesseract conveniently honours a
> TESSDATA_PREFIX environment variable that specifies its data folder's
> location, so it seems we are all set.
>
> What should TESSDATA_PREFIX be set to?  Tesseract's documentation[0]
> says
>
>   TESSDATA_PREFIX environment variable should be set to the parent
>   directory of “tessdata” directory.
>
> So "share" then, presumably, to have the data files located at
> "share/tessdata".  The man page[1] seems to confirm this:
>
>   To use a non-standard language pack named foo.traineddata, set the
>   TESSDATA_PREFIX environment variable so the file can be found at
>   TESSDATA_PREFIX/tessdata/foo.traineddata...
>
> This creates a problem, though, since defining a native-search-path of
> just "share" will pull in files from virtually every single Guix
> package.  The solution then is to introduce an intermediate folder,
> "tesseract-ocr", that sidesteps this problem, and to configure Tesseract
> appropriately at build time so it installs its data files to
> "share/tesseract-ocr/tessdata" instead.  This is why the existing code
> was written the way it was and what the comment you pointed out is
> referring to.
>
> However there's a problem with this, too: Patching Makefile.am the way
> the code does results in only some of Tesseract's data files being
> placed in "share/tesseract-ocr/tessdata"; you can see in the package
> output there is still a "share/tessdata" folder that contains
> Tesseract's config files.  Since these aren't also placed beneath
> "share/tesseract-ocr/tessdata" Tesseract can't find them at runtime.
>
> The solution to this seems to be to remove this phase and instead use
> the "--datadir" configure flag to specify the desired data-folder path.
> Doing this results in all of Tesseract's data files being installed
> beneath "share/tesseract-ocr/tessdata" and the resulting package works
> as you'd expect.
>
> However the problem with this is... none of it is necessary in the first
> place!  It turns out Tesseract's documentation is simply WRONG and the
> program actually expects TESSDATA_PREFIX to contain the complete path to
> the "tessdata" data folder, not the path of the folder directly above
> it.  So Tesseract can be built as-is, the native-search-path can be
> safely defined as "share/tessdata", and everything just works.
>
> This is what the patch I passed on yesterday does.

Thanks for explaining, that makes sense!  Would you be so kind as to
open an issue with upstream about the misleading doc?  That'd complete
it and avoid any confusion in the future.
Simon South Feb. 28, 2023, 4:40 p.m. UTC | #5
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
> Would you be so kind as to open an issue with upstream about the
> misleading doc?

I would've submitted a patch already were the project not using GitHub.
I don't have a GitHub account and don't intend to get one.

Would anyone else be willing to be open an issue on this?
Maxim Cournoyer Feb. 28, 2023, 9:41 p.m. UTC | #6
Hello,

Simon South <simon@simonsouth.net> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>> Would you be so kind as to open an issue with upstream about the
>> misleading doc?
>
> I would've submitted a patch already were the project not using GitHub.
> I don't have a GitHub account and don't intend to get one.
>
> Would anyone else be willing to be open an issue on this?

No problem; see: https://github.com/tesseract-ocr/tesseract/issues/4025.
Jelle Licht March 16, 2023, 8:38 p.m. UTC | #7
Hey folks,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hello,
>
> Simon South <simon@simonsouth.net> writes:
>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>>> Would you be so kind as to open an issue with upstream about the
>>> misleading doc?
>>
>> I would've submitted a patch already were the project not using GitHub.
>> I don't have a GitHub account and don't intend to get one.
>>
>> Would anyone else be willing to be open an issue on this?
>
> No problem; see: https://github.com/tesseract-ocr/tesseract/issues/4025.

So it seems the issue was confirmed. In addition there seems to be some
inconsistencies between build system with regards to how the data dir is
interpreted by tesseract:

https://github.com/tesseract-ocr/tesseract/issues/4026

I think it makes sense for us to apply [a version of] Simon's patch.  QA
also seems to show green lights, ignoring the unrelated recent
openmpi-related failures.

WDYT?
 - Jelle
Maxim Cournoyer March 21, 2023, 3:13 a.m. UTC | #8
Hello,

Jelle Licht <jlicht@fsfe.org> writes:

> Hey folks,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> Hello,
>>
>> Simon South <simon@simonsouth.net> writes:
>>
>>> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>>>> Would you be so kind as to open an issue with upstream about the
>>>> misleading doc?
>>>
>>> I would've submitted a patch already were the project not using GitHub.
>>> I don't have a GitHub account and don't intend to get one.
>>>
>>> Would anyone else be willing to be open an issue on this?
>>
>> No problem; see: https://github.com/tesseract-ocr/tesseract/issues/4025.
>
> So it seems the issue was confirmed. In addition there seems to be some
> inconsistencies between build system with regards to how the data dir is
> interpreted by tesseract:
>
> https://github.com/tesseract-ocr/tesseract/issues/4026
>
> I think it makes sense for us to apply [a version of] Simon's patch.  QA
> also seems to show green lights, ignoring the unrelated recent
> openmpi-related failures.
>
> WDYT?

I've now applied it, after writing a proper change log commit message,
and running the xvnc and lightdm system tests to get some confidence
(they make use of tesseract-ocr).

Thank you for looking into it!
diff mbox series

Patch

diff --git a/gnu/packages/ocr.scm b/gnu/packages/ocr.scm
index c1cd4f061e..e07d40bda4 100644
--- a/gnu/packages/ocr.scm
+++ b/gnu/packages/ocr.scm
@@ -82,18 +82,14 @@  (define-public tesseract-ocr-tessdata-fast
               (method git-fetch)
               (uri (git-reference
                     (url "https://github.com/tesseract-ocr/tessdata_fast")
+                    (recursive? #t) ; for tessconfigs
                     (commit version)))
               (file-name (git-file-name name version))
               (sha256
                (base32
-                "1m310cpb87xx8l8q7jy9fvzf6a0m8rm0dmjpbiwhc2mi6w4gn084"))))
+                "1hqdsy3zdy5b9l641fvhnawkw6wpb8nkvjql78q8g47js8109mhm"))))
     (build-system copy-build-system)
-    (arguments (list #:install-plan #~'(("." "share/tesseract-ocr/tessdata"))
-                     #:phases #~(modify-phases %standard-phases
-                                  (add-after 'unpack 'delete-broken-links
-                                    (lambda _
-                                      (delete-file "configs")
-                                      (delete-file "pdf.ttf"))))))
+    (arguments (list #:install-plan #~'(("." "share/tesseract-ocr/tessdata"))))
     (home-page "https://github.com/tesseract-ocr/tessdata_fast")
     (synopsis "Fast integer versions of trained LSTM models")
     (description "This repository contains fast integer versions of trained