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

Git versioning and modern packaging #1065

Merged
merged 9 commits into from
Dec 15, 2023
Merged

Conversation

bertsky
Copy link
Collaborator

@bertsky bertsky commented Jun 28, 2023

This brings our packaging up to speed, including our surprisingly challenging case of mutually pinned "sub-distributions". I had to comment out the actual requirement ocrd → ocrd_network to break a dependency circle, which is nececssary because otherwise pip goes completely crazy and downloads some arbitrary other versions and then complains of broken requirements. I also improved the makefile.

@bertsky bertsky requested review from kba, MehmedGIT and joschrew June 28, 2023 14:23
@bertsky
Copy link
Collaborator Author

bertsky commented Jun 28, 2023

CI failure is macos only – no idea what's going on there.

@bertsky
Copy link
Collaborator Author

bertsky commented Dec 7, 2023

@kba I resolved the conflict by keeping an empty setup() script. This conflict will re-emerge whenever we release a new version on master. (One of the benefits after merging this PR is that there will only be a single place where the version is specified – in the repo.)

Copy link
Member

@kba kba left a comment

Choose a reason for hiding this comment

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

Thanks for preparing this, I've now finally tested it, esp. love the version-from-git-tag mechanic.

Everything works fine except make install-dev. If I try pip install -e in ocrd_models, I get an error like

setuptools.extern.packaging._tokenizer.ParserSyntaxError: Expected end or semicolon (after version specifier)
    ocrd_utils == 2.59.2.dev7+g9fbddff75.d20231214 == 2.59.2.dev7+g9fbddff75.d20231214                       

I think the issue is that finalize_options is called twice, I don't know why but this simple workaround fixes it for me:

-self.distribution.install_requires = [                                
-    req + vers if req.startswith('ocrd') else req 
-    for req in self.distribution.install_requires                     
-]                                                                     
+self.distribution.install_requires = [                                
+    req + vers if req.startswith('ocrd') and '==' not in req else req 
+    for req in self.distribution.install_requires                     
+]                                                                     

If that's okay with you, I'll commit that and we can merge this before the next release.

BTW: Do you think there's a way to have a single place to define the build class and import it in the setup.pys? Not a big issue since I don't expect us to have to change setup.py anymore in the future, but it would be neat.

@bertsky
Copy link
Collaborator Author

bertsky commented Dec 14, 2023

If I try pip install -e in ocrd_models, I get an error like [...]

Very strange. I don't recall seeing that – perhaps it's one of the recent setuptools changes...

I think the issue is that finalize_options is called twice, I don't know why but this simple workaround fixes it for me: [...]
If that's okay with you, I'll commit that and we can merge this before the next release.

Sure, sounds reasonable – please go ahead.

BTW: Do you think there's a way to have a single place to define the build class and import it in the setup.pys? Not a big issue since I don't expect us to have to change setup.py anymore in the future, but it would be neat.

I did think about inheriting this, but gave up quickly IIRC.

@kba
Copy link
Member

kba commented Dec 14, 2023

OK, added the fix, now everything seems in order. I'll merge and release tomorrow.

@kba
Copy link
Member

kba commented Dec 15, 2023

macos still fails but I'll debug that separately.

@kba kba merged commit 65c6f03 into OCR-D:master Dec 15, 2023
@bertsky bertsky deleted the git-versioning branch June 6, 2024 14:11
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.

2 participants