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

ocrd_all: make all creates error #303

Closed
stefanCCS opened this issue Mar 23, 2022 · 13 comments
Closed

ocrd_all: make all creates error #303

stefanCCS opened this issue Mar 23, 2022 · 13 comments

Comments

@stefanCCS
Copy link

Hi,
I have tried to update the current (23.02.2022) ocrd_all using git pull and make clean and make 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
Copy link
Member

kba commented Mar 23, 2022

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
Copy link
Collaborator

stweil commented Mar 23, 2022

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
Copy link
Collaborator

stweil commented Mar 23, 2022

@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
Copy link
Collaborator

stweil commented Mar 23, 2022

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

@bertsky
Copy link
Collaborator

bertsky commented Mar 23, 2022

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
Copy link
Collaborator

bertsky commented Mar 23, 2022

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 NVIDIA/pix2pixHD#297, so we can rebase ocrd_anybaseocr on that.

@bertsky
Copy link
Collaborator

bertsky commented Mar 24, 2022

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
Copy link
Collaborator

bertsky commented Mar 24, 2022

@bertsky bertsky linked a pull request Mar 24, 2022 that will close this issue
@kba
Copy link
Member

kba commented Mar 24, 2022

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

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

@kba
Copy link
Member

kba commented Mar 24, 2022

(@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
Copy link
Collaborator

stweil commented Mar 24, 2022

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:

ASVLeipzig/cor-asv-ann#10
OCR-D/ocrd_olahd_client#4
qurator-spk/dinglehopper#66
qurator-spk/sbb_textline_detection#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
Copy link
Collaborator

bertsky commented Mar 24, 2022

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

No, not anymore.

@stefanCCS
Copy link
Author

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

@stweil stweil closed this as completed Mar 15, 2023
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 a pull request may close this issue.

4 participants