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

Rpm package python bindings #380

Merged
merged 2 commits into from
Jul 10, 2023
Merged

Rpm package python bindings #380

merged 2 commits into from
Jul 10, 2023

Conversation

engelmi
Copy link
Member

@engelmi engelmi commented Jul 7, 2023

This PR

  • adds the pyhirte python module as a package to the hirte.spec
  • fixes a typing issue in enable/disable unit in pyhirte.ext

Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

I'm not a python packaging expert, but I fear that we will not be able to build the package in Fedora if we use deprecated macros.
If we can't use the recommended python RPM macros with current build structure, which mixes C and python projects, then I'd suggest to have a different spec file for python package or completely different project.

hirte.spec.in Outdated Show resolved Hide resolved
hirte.spec.in Show resolved Hide resolved
hirte.spec.in Show resolved Hide resolved
Added the pyhirte python module to the hirte spec
for building an rpm package. For this, switched to
setup.py (instead of pyproject.toml) due to issues
with rpm pyproject macros.

Signed-off-by: Michael Engel <mengel@redhat.com>
Signed-off-by: Michael Engel <mengel@redhat.com>
@engelmi
Copy link
Member Author

engelmi commented Jul 7, 2023

I'm not a python packaging expert, but I fear that we will not be able to build the package in Fedora if we use deprecated macros.
If we can't use the recommended python RPM macros with current build structure, which mixes C and python projects, then I'd suggest to have a different spec file for python package or completely different project.

@mwperina Mixing C and Python projects does not impact each other here. I migrated (again) from pyproject.toml to setup.py (which is deprecated) so that I was able to use these deprecated macros (e.g. py3_install) - even though I would prefer to use the new and recommended pyproject.toml. Using the pyproject.toml works fine building it via python -m build, but using the new rpm macros (e.g. pyproject_wheel) resulted in an UNKNOWN wheel - making the rpm build fail. Maybe upgrading from python3.9 to 3.11 would solve this, but I haven't tried it.

@dougsland
Copy link
Contributor

I'm not a python packaging expert, but I fear that we will not be able to build the package in Fedora if we use deprecated macros.
If we can't use the recommended python RPM macros with current build structure, which mixes C and python projects, then I'd suggest to have a different spec file for python package or completely different project.

@mwperina Mixing C and Python projects does not impact each other here. I migrated (again) from pyproject.toml to setup.py (which is deprecated) so that I was able to use these deprecated macros (e.g. py3_install) - even though I would prefer to use the new and recommended pyproject.toml. Using the pyproject.toml works fine building it via python -m build, but using the new rpm macros (e.g. pyproject_wheel) resulted in an UNKNOWN wheel - making the rpm build fail. Maybe upgrading from python3.9 to 3.11 would solve this, but I haven't tried it.

IMO, If upgrade doesn't work, we can always "fly" in the first moment with something which works. Lets remember, we started with manually written python module and now we have a fancy auto generation module. Users will always appreciate initial features available. Or in other words, merge early, merge often.

@mwperina
Copy link
Member

I'm not a python packaging expert, but I fear that we will not be able to build the package in Fedora if we use deprecated macros.
If we can't use the recommended python RPM macros with current build structure, which mixes C and python projects, then I'd suggest to have a different spec file for python package or completely different project.

@mwperina Mixing C and Python projects does not impact each other here.

Well, partially it is, because we are using non-standard project structure, that might be the reason why the preferred python macros doesn't work

I migrated (again) from pyproject.toml to setup.py (which is deprecated) so that I was able to use these deprecated macros (e.g. py3_install) - even though I would prefer to use the new and recommended pyproject.toml. Using the pyproject.toml works fine building it via python -m build, but using the new rpm macros (e.g. pyproject_wheel) resulted in an UNKNOWN wheel - making the rpm build fail. Maybe upgrading from python3.9 to 3.11 would solve this, but I haven't tried it.

IMO, If upgrade doesn't work, we can always "fly" in the first moment with something which works. Lets remember, we started with manually written python module and now we have a fancy auto generation module. Users will always appreciate initial features available. Or in other words, merge early, merge often.

This is not about upgrade, we want hirte packages for latest Fedora and El9, so we need to be able to adapt to different python versions (3.9 on EL9, 3.11 on FC37/38, 3.12 on rawhide).

If will try to take a look at the end of this week if we can make the spec working with non-deprecated macros ...

@engelmi
Copy link
Member Author

engelmi commented Jul 10, 2023

Well, partially it is, because we are using non-standard project structure, that might be the reason why the preferred python macros doesn't work

Yes, its a non-standard project structure. However, in the spec I simply copy the pyhirte directory to root temporarily for the build/packaging - bringing it into the required structure (with lots of unnecessary files for the pyhirte package). Not sure if that is a good or bad approach, but I wouldn't want to have a pyproject.toml in the root directory (tracked by git).

This is not about upgrade, we want hirte packages for latest Fedora and El9, so we need to be able to adapt to different python versions (3.9 on EL9, 3.11 on FC37/38, 3.12 on rawhide).

Would El9 etc. reject the rpm macros? The built .whl is the same for both approaches. Not sure about the versions, though. I think for 3.12 the setup.py support is removed.

If will try to take a look at the end of this week if we can make the spec working with non-deprecated macros ...

That would be great!

Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

Let's merge now and if needed let's improve later

@engelmi engelmi merged commit ef6bd14 into eclipse-bluechi:main Jul 10, 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 this pull request may close these issues.

3 participants