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

Makefile: sequential install does not work #351

Closed
bertsky opened this issue Nov 20, 2019 · 8 comments
Closed

Makefile: sequential install does not work #351

bertsky opened this issue Nov 20, 2019 · 8 comments
Assignees
Labels

Comments

@bertsky
Copy link
Collaborator

bertsky commented Nov 20, 2019

When I do make install, this will currently run one pip install for each package in core sequentially. But since each of those depend on each other, and the independently called pip does not know about the others, it will try to satisfy those from the package index instead of the local installation. This can create inconsistencies when the features / API change, but the version does not.

For example this happened recently:

Traceback (most recent call last):
  File "/usr/local/bin/ocrd", line 5, in <module>
    from ocrd.cli import cli
  File "/usr/local/lib/python3.6/site-packages/ocrd/cli/__init__.py", line 3, in <module>
    from ocrd.cli.ocrd_tool import ocrd_tool_cli
  File "/usr/local/lib/python3.6/site-packages/ocrd/cli/ocrd_tool.py", line 7, in <module>
    from ocrd.decorators import parameter_option
  File "/usr/local/lib/python3.6/site-packages/ocrd/decorators.py", line 7, in <module>
    from ocrd_utils import (
ImportError: cannot import name 'parse_json_string_or_file'

This is ocrd.decorators from master (2.0.0) trying to use ocrd_utils from PyPI (2.0.0).

IMO we need to bring all packages together into one pip call. I know this is problematic, since pip semantics is all-or-nothing instead of one-by-one. But maybe we can at least come up with something that usually does work correctly, and breaks visibly when it does not?

Ideas/options:

  • calculate the hashes of all subpackages and then use --require-hashes
  • install one by one, but then reinstall all at once with --no-index
  • build wheel files locally, then install these all at once
@bertsky
Copy link
Collaborator Author

bertsky commented Nov 20, 2019

* build wheel files locally, then install these all at once

This does not work. Installation from wheels behaves exactly the same as installation from directories.

Simpler (and successful in the current situation):

  • install one by one, but then install again one by one with --no-deps

@bertsky
Copy link
Collaborator Author

bertsky commented Nov 20, 2019

@kba do you want me to make a PR based on the latter solution? (We need this urgently for ocrd_all – perhaps I should first add a workaround there?)

@bertsky
Copy link
Collaborator Author

bertsky commented Nov 22, 2019

We need this urgently for ocrd_all – perhaps I should first add a workaround there?)

See here

@kba
Copy link
Member

kba commented Nov 25, 2019

I don't really understand how this is a problem since the BUILD_ORDER variable defines the install order from ocrd_utils (which does not depend on another ocrd pkg) to ocrd (which includes all the other packagages). Looping through those packages and pip installing them should satisfy the requirements and not download from PyPI.

If you have a PR that fixes the problems for you, I'm open.

@bertsky
Copy link
Collaborator Author

bertsky commented Nov 25, 2019

Looping through those packages and pip installing them should satisfy the requirements and not download from PyPI.

Yes, I guess this only becomes a problem when using --force-reinstall (as used by ocrd_all/Makefile to ensure the CLI timestamps are updated).

The PR would look exactly like the workaround linked above (redoing make install with --no-deps). You still want it, or disregard as too special and close?

@kba
Copy link
Member

kba commented Nov 25, 2019

Seems a useful pattern. Add it as another target install_alternative or similar?

@bertsky
Copy link
Collaborator Author

bertsky commented Nov 29, 2019

Seems a useful pattern. Add it as another target install_alternative or similar?

Not so sure. Probably too confusing. I say we should close.

@kba
Copy link
Member

kba commented Nov 29, 2019

OK, thanks.

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

No branches or pull requests

2 participants