OCR-D / ocrd_all

Master repository which includes most other OCR-D repositories as submodules
MIT License
72 stars 17 forks source link

ocrd_all: make all creates error #303

Closed stefanCCS closed 1 year ago

stefanCCS commented 2 years ago

Hi, I have tried to update the current (23.02.2022) ocrd_all using git pull and make clean andmake all. Unfortunately, I get this error:

cd ocrd_anybaseocr ; make patch-pix2pixhd
make[2]: Entering directory '/home/ocrdadmin/ocrd_all/ocrd_anybaseocr'
git submodule update --init
From https://github.com/NVIDIA/pix2pixHD
 * branch            e524de235b251adddee6ca2bcbd31115a834077c -> FETCH_HEAD
error: Your local changes to the following files would be overwritten by checkout:
        data/custom_dataset_data_loader.py
        data/data_loader.py
        models/base_model.py
        models/networks.py
        models/pix2pixHD_model.py
        options/base_options.py
Please commit your changes or stash them before you switch branches.
Aborting
Unable to checkout 'e524de235b251adddee6ca2bcbd31115a834077c' in submodule path 'ocrd_anybaseocr/pix2pixhd'
Makefile:72: recipe for target 'pix2pixhd' failed
kba commented 2 years ago

We should probably add a clean target to ocrd_anybaseocr and delegate to it on make clean in ocrd_all, like we do for deps-ubuntu. What do you think @stweil @bertsky

stweil commented 2 years ago

This is one of several nagging issues with the current build. Ideally running make should not change any of the submodules after they were checked out or at least restore them after building. Currently some files are modified, others are added:

modified:   cor-asv-ann (untracked content)
modified:   dinglehopper (untracked content)
modified:   ocrd_anybaseocr (modified content, untracked content)
modified:   ocrd_olahd_client (untracked content)
modified:   ocrd_tesserocr (modified content, untracked content)
modified:   sbb_textline_detector (untracked content)
stweil commented 2 years ago

@stefanCCS, these commands should help:

git -C ocrd_anybaseocr/ocrd_anybaseocr/pix2pixhd reset --hard
git -C ocrd_anybaseocr/ocrd_anybaseocr/pix2pixhd clean -fxd
touch ocrd_anybaseocr

They restore changed files, remove added files and force a new build for ocrd_anybaseocr.

stweil commented 2 years ago

@kba, this issue is one of several others which I also have observed where incremental builds fail to work.

bertsky commented 2 years ago

In this particular case, I think we can keep ocrd_all completely out of the picture: It was the non-existing packaging of pix2pixHD and the horrible integration of the pix2pixHD subcomponent in ocrd_anybaseocr which led us to this solution. But now that we need a proper branch for that anyway, why not apply our packaging patches in another branch, then merge both in an OCR-D fork's master, and refer to that?

(@kba if we can agree on this plan then I recommend transferring the issue to ocrd_anybaseocr, which I have no permissions for.)

bertsky commented 2 years ago

It was the non-existing packaging of pix2pixHD and the horrible integration of the pix2pixHD subcomponent in ocrd_anybaseocr which led us to this solution. But now that we need a proper branch for that anyway, why not apply our packaging patches in another branch, then merge both in an OCR-D fork's master, and refer to that?

In that vein, I have created https://github.com/NVIDIA/pix2pixHD/pull/297, so we can rebase ocrd_anybaseocr on that.

bertsky commented 2 years ago

Alas, due to pypa/pip#6658, we cannot just make setup contain a pix2pixhd dependency as a relative path. So I think I'll just upload a pkg for it myself.

bertsky commented 2 years ago

Done:

kba commented 2 years ago

So I think I'll just upload a pkg for it myself.

https://pypi.org/project/pix2pixhd/ :tada:

kba commented 2 years ago

(@kba if we can agree on this plan then I recommend transferring the issue to ocrd_anybaseocr, which I have no permissions for.

I could but you solved everything AFAICS. Is there still a need for it?

stweil commented 2 years ago

Currently some files are modified, others are added

pip install . creates a build directory in each submodule which uses that command. Some submodules already had an entry for that build artifact, but in several others that entry is missing. I therefore created several pull requests to fix that:

https://github.com/ASVLeipzig/cor-asv-ann/pull/10 https://github.com/OCR-D/ocrd_olahd_client/pull/4 https://github.com/qurator-spk/dinglehopper/pull/66 https://github.com/qurator-spk/sbb_textline_detection/pull/59

As soon as those pull requests were merged and ocrd_all was updated to use the new code we should have a cleaner git status after make all.

bertsky commented 2 years ago

I could but you solved everything AFAICS. Is there still a need for it?

No, not anymore.

stefanCCS commented 1 year ago

@kba : Feel free to close this issue, as "make all" has worked already later in 2022.