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

add OCR-D/ocrd_fileformat, fix #80 #83

Merged
merged 4 commits into from
Apr 23, 2020
Merged

add OCR-D/ocrd_fileformat, fix #80 #83

merged 4 commits into from
Apr 23, 2020

Conversation

kba
Copy link
Member

@kba kba commented Apr 22, 2020

No description provided.

Makefile Outdated
@@ -474,9 +481,9 @@ endif
docker-minimum-git docker-medium-git docker-maximum-git: PIP_OPTIONS = -e

# Minimum-size selection: use Ocropy binarization, use Tesseract from PPA
docker-minimum docker-minimum-git: DOCKER_MODULES = core ocrd_im6convert ocrd_cis ocrd_tesserocr tesserocr workflow-configuration ocrd_repair_inconsistencies
docker-minimum docker-minimum-git: DOCKER_MODULES = core ocrd_im6convert ocrd_cis ocrd_tesserocr tesserocr workflow-configuration ocrd_repair_inconsistencies ocrd_fileformat
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am friend of alphabetic sorting of such lists, but that can be done next time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

238f514 core should be before cor-asv-... IMHO

@bertsky
Copy link
Collaborator

bertsky commented Apr 22, 2020

LGTM.

Can you please add ocrd-pagetopdf as well (this version for now)? I know it should be in ocr-fileformat itself, but that would never be able to replace the full-featured direct conversion.

@kba
Copy link
Member Author

kba commented Apr 22, 2020

https://app.circleci.com/pipelines/github/OCR-D/ocrd_all/119/workflows/3520cde8-ea4f-4eb8-8e8e-ab229cb8991a/jobs/127 Looks like the submodule is not checked out recursively - @bertsky @stweil what am I doing wrong?

Can you please add ocrd-pagetopdf as well (this version for now)? I know it should be in ocr-fileformat itself, but that would never be able to replace the full-featured direct conversion.

Sure see #84

@bertsky
Copy link
Collaborator

bertsky commented Apr 22, 2020

https://app.circleci.com/pipelines/github/OCR-D/ocrd_all/119/workflows/3520cde8-ea4f-4eb8-8e8e-ab229cb8991a/jobs/127 Looks like the submodule is not checked out recursively - @bertsky @stweil what am I doing wrong?

You made no mistake: ocrd_fileformat gets cloned recursively:

git submodule sync --recursive ocrd_fileformat
if git submodule status --recursive ocrd_fileformat | grep -qv '^ '; then \
	git submodule update --init --recursive ocrd_fileformat && \
	touch ocrd_fileformat; fi
Submodule 'ocrd_fileformat' (https://github.com/OCR-D/ocrd_fileformat) registered for path 'ocrd_fileformat'
Cloning into '/build/ocrd_fileformat'...
Submodule path 'ocrd_fileformat': checked out 'de5b53282243c0b9696bf1ca9a22c327793ef37d'
Submodule 'ocr-fileformat' (https://github.com/UB-Mannheim/ocr-fileformat) registered for path 'ocrd_fileformat/ocr-fileformat'
Cloning into '/build/ocrd_fileformat/ocr-fileformat'...
Submodule path 'ocrd_fileformat/ocr-fileformat': checked out '86a41edf80ad235a95a612b981ffb68ae49417db'

But then subsequently:

. /usr/local/bin/activate && make -C ocrd_fileformat install-fileformat install
make[1]: Entering directory '/build/ocrd_fileformat'
make -C  ocr-fileformat PREFIX=/usr/local vendor install
make[2]: Entering directory '/build/ocrd_fileformat/ocr-fileformat'
make -C vendor check
make[3]: Entering directory '/build/ocrd_fileformat/ocr-fileformat/vendor'
make[3]: *** [check] Error 1
make[2]: *** [check] Error 2
make[1]: *** [install-fileformat] Error 2

So obviously there's a bug in ocr-fileformat: make vendor does not work, and does not even report a proper error.

@bertsky
Copy link
Collaborator

bertsky commented Apr 22, 2020

Can you please add ocrd-pagetopdf as well (this version for now)? I know it should be in ocr-fileformat itself, but that would never be able to replace the full-featured direct conversion.

Sure see #84

Thanks @stweil @kba!

@stweil
Copy link
Collaborator

stweil commented Apr 23, 2020

So obviously there's a bug in ocr-fileformat: make vendor does not work, and does not even report a proper error.

It checks whether all required executables are available, and the error message is indeed not good. Maybe unzip is missing. I added it to the requirements, so hopefully the Docker build should work now.

@stweil
Copy link
Collaborator

stweil commented Apr 23, 2020

@kba, @bertsky, would you prefer a different build fix, or can we merge now?

@bertsky
Copy link
Collaborator

bertsky commented Apr 23, 2020

would you prefer a different build fix, or can we merge now?

I suppose we could give ocrd_fileformat's Makefile a target deps-ubuntu with all required packages. But we could also do that in a later PR and see how we fare without.

@stweil stweil merged commit 8020860 into master Apr 23, 2020
@stweil stweil deleted the add-fileformat branch April 23, 2020 08:02
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