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

Package to FedoraProject #834

Closed
LecrisUT opened this issue Jan 6, 2023 · 15 comments
Closed

Package to FedoraProject #834

LecrisUT opened this issue Jan 6, 2023 · 15 comments

Comments

@LecrisUT
Copy link
Contributor

LecrisUT commented Jan 6, 2023

In order for other fedora RPMs to depend on this project, it should be built there as well

@hroncok
Copy link

hroncok commented Jan 10, 2023

FTR @hrnciar is working on this.

@henryiii henryiii mentioned this issue Jan 10, 2023
@LecrisUT
Copy link
Contributor Author

Is https://copr.fedorainfracloud.org/coprs/thrnciar/python-scikit-build/ the copr repo that @hrnciar is working on? Did anyone get in touch with fedoraproject team to open a repo there?

@hroncok
Copy link

hroncok commented Jan 11, 2023

Is https://copr.fedorainfracloud.org/coprs/thrnciar/python-scikit-build/ the copr repo that @hrnciar is working on?

Yes.

Did anyone get in touch with fedoraproject team to open a repo there?

We are the team. Working on it, see the official request at https://bugzilla.redhat.com/show_bug.cgi?id=2159976

@LecrisUT
Copy link
Contributor Author

Thank you, looking forward to it

@henryiii
Copy link
Contributor

henryiii commented Jan 11, 2023

Oops, the patch to requirements-dev is something we can fix. [test] should not bring in flake8 and such (but currently does).

@hrnciar
Copy link

hrnciar commented Jan 12, 2023

Hello,

during the package review, there were discovered two files scikit-build ships that seem like they contain another library.
Those files are:

/usr/lib/python3.11/site-packages/skbuild/resources/cmake/FindCython.cmake
/usr/lib/python3.11/site-packages/skbuild/resources/cmake/FindF2PY.cmake

This goes against a part of the Fedora Packaging Guidelines which says that packages shouldn't bundle another library.
Would it be possible not to ship those files? Or could you please provide the library name and version from where it comes from so I can add the correct Provides. Thank you.

In the bugzilla there is also this comment from @LecrisUT:

Would it make sense to move these files to system cmake? The system would call system cmake by default, and it would be helpful for debugging non python build executions alongside it.

Could you please elaborate on that, we don't have a lot of experience with cmake.

@LecrisUT
Copy link
Contributor Author

Could you please elaborate on that, we don't have a lot of experience with cmake.

So cmake searches the path defined by CMAKE_MODULE_PATH or the default system installation, i.e. /usr/lib64/cmake. I believe scikit-build overrides CMAKE_MODULE_PATH to include /usr/lib/python3.11/site-packages/skbuild/resources/cmake, but this will only make it buildable through setup.py. Instead as the package distribution side, we should move those scripts in the system installation of /usr/lib64/cmake to make this functions available globally. We should also fix the python script to first search for system provided cmake packages (unless appropriate variable is given e.g. use_bundled_cmake/use_bundled_cmake_extensions) and if fails (e.g. a test CMakeLists.txt importing the packages) default to bundled files of pip (delete site-packages version on rpm version for good practices).

Note, we should make sure there is no conflict in the provided files by other packages (dnf whatprovides FindCython.cmake for example). If there is, I think there are appropriate methods of making them cross compatible with provides macros and such.

@LecrisUT LecrisUT mentioned this issue Jan 12, 2023
5 tasks
@LecrisUT
Copy link
Contributor Author

@hrnciar Can you make a spec for scikit-build-core as well?

I re-read your previous concerns, which would apply for scikit-build-core as well. Overall we should delete those (maybe also add a patch to delete the logic in python) since the Fedora packaged versions already have the necessary fixes. Otherwise if the cmake package is a bit slower, it will be ok to include the cmake files because they are scripts and only used internally. Just delete them accordingly to not influence dnf whatprovides FindCython.cmake too much for example.

For the scripts that are provided by the package, namely PythonExtensions, those should be moved to system cmake location.

@hrnciar
Copy link

hrnciar commented Feb 10, 2023

Hello, I'd like to let you know that scikit-build is now packaged in Fedora. If anyone would be willing to help maintain the package, I'll be happy to add any co-maintainers.

@hrnciar Can you make a spec for scikit-build-core as well?

I am sorry, but I don't have the capacity for this. My main interest in packaging scikit-build was that I needed it for updating poetry which I maintain.

@LecrisUT
Copy link
Contributor Author

About scikit-build-core I have built it similarly from a Pypi project here, just need to clean it up to add missing dependencies and it could be published. Any notes on getting the ball rolling on adding this FedoraProject? I forgot the IRC/Matrix channels where to get info on this.

@hrnciar
Copy link

hrnciar commented Feb 13, 2023

Are there any missing dependencies? pyproject-rpm-macros automatically detects dependencies and since the copr build succeeded, I believe that all dependencies are available.

Adjust the spec file according to the comments there. E.g the license should be in SPDX license format. Apache-2.0 instead of ASL.

Then follow this process to add it to Fedora.

If you have any questions you can join Fedora Python channel on Matrix.

@LecrisUT
Copy link
Contributor Author

For scikit-build-core we need to bundle the optional dependencies form [pyproject], as well as [test] in BuildRequires to properly run pytest

Thanks for the notes

@LecrisUT
Copy link
Contributor Author

@hrnciar Should I help maintain this package since I am becoming a maintainer of scikit-build-core?

@hrnciar
Copy link

hrnciar commented Mar 21, 2023

I've added you as a scikit-build co-maintainer, thank you.

@LecrisUT
Copy link
Contributor Author

Oh, I forgot I was the one who opened this issue. Let me close it.

@hrnciar Check this pr for how we can automate copr builds and PRs to src.fedoraproject.org. I think there is a packit implementation meant for dist-git to also automate Koji and Bohdi submissions. What do you think of adding those?

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

No branches or pull requests

4 participants