Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve CCUtil::main_setup (fixes issue #4230) #4239

Merged
merged 2 commits into from
May 12, 2024

Conversation

stweil
Copy link
Contributor

@stweil stweil commented May 12, 2024

Conda installations patch TESSDATA_PREFIX in the binary. That does not work for std::string because the length won't be patched, so use a normal C string which can be patched.

Simplify also the code which checks the last character of datadir.

Conda installations patch TESSDATA_PREFIX in the binary.
That does not work for std::string because the length
won't be patched, so use a normal C string which can be
patched.

Simplify also the code which checks the last character
of datadir.

Signed-off-by: Stefan Weil <[email protected]>
src/ccutil/ccutil.cpp Outdated Show resolved Hide resolved
@stweil stweil merged commit 6a31e36 into tesseract-ocr:main May 12, 2024
5 of 6 checks passed
@stweil stweil deleted the issue4230 branch May 12, 2024 07:39
@amitdo
Copy link
Collaborator

amitdo commented May 13, 2024

Conda installations patch TESSDATA_PREFIX in the binary.

What ???

https://docs.conda.io/projects/conda-build/en/stable/resources/make-relocatable.html

That's crazy!

@amitdo
Copy link
Collaborator

amitdo commented May 13, 2024

Do we really want to bend our code just because some distro is doing crazy things?

@stweil
Copy link
Contributor Author

stweil commented May 13, 2024

They really patch all binaries which contain a lengthy magic string (which they configure as prefix). In most cases that allows an installation to a different prefix.

@stweil
Copy link
Contributor Author

stweil commented May 13, 2024

Example: lib/pkgconfig/tesseract.pc is generated with this first line:

prefix=/Users/stweil/src/github/conda-forge/tesseract-feedstock/miniforge3/conda-bld/tesseract_1715530534651/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeh

The installed file has this first line:

prefix=/opt/homebrew/Caskroom/miniconda/base

@stweil
Copy link
Contributor Author

stweil commented May 13, 2024

Do we really want to bend our code just because some distro is doing crazy things?

I think we would not bend our code in most cases, but in this case our workaround only requires a single code line with an explaining comment. That is less work for us than handling future issue reports if we don't adapt the Tesseract code.

@egorpugin
Copy link
Contributor

egorpugin commented May 13, 2024

In general don't follow it.
We swap to string, to fs::path? Then what?
It's their thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants