Skip to content

FedoraProject package #196

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

Closed
LecrisUT opened this issue Feb 13, 2023 · 16 comments
Closed

FedoraProject package #196

LecrisUT opened this issue Feb 13, 2023 · 16 comments

Comments

@LecrisUT
Copy link
Collaborator

In preparation for distributing to FedoraProject, I have prepared a git repository of the .spec file:
https://github.com/LecrisUT/scikit-build-core-rpm

The linked copr project is at https://copr.fedorainfracloud.org/coprs/lecris/scikit-build-core/
If there is anything missing, let me know

@LecrisUT
Copy link
Collaborator Author

LecrisUT commented Feb 13, 2023

Didn't take long, already could use some help on this build against 0.1.6: https://copr.fedorainfracloud.org/coprs/lecris/scikit-build-core/build/5521479/

On Fedora 37 and Fedora 38 there are different errors. I'll try to build it with main head as well.

Edit: Against main, the issue is that the package version requirements are too high and they are not available in FredoraProject yet.

@henryiii
Copy link
Collaborator

henryiii commented Feb 14, 2023

The problem I saw was from using VCS version. I've been sort of planning to remove VCS versioning - while I usually like it, it usually involves setuptools_scm (via hatch-vcs in this case, but still), which changes the way setuptools works for all builds, not just the one you are attempting. This is particularly bad for a build backend, as some distributors have trouble separating dependencies required for build and install (Spack is the biggest offender here).

For the other archs, you need cmake & ninja as dependencies if you are on a platform that doesn't support binary wheels. In general, though, as a distributor, you probably always want your version of cmake and ninja as required dependencies, and you might want to disable the isolated tests (-m "not isolated") if they fail.

I'll try to set up a small guide for preparing a distribution soon. The [pyproject] dependencies and cmake and ninja should be required dependencies when distributed outside the PyPI ecosystem.

@henryiii
Copy link
Collaborator

henryiii commented Feb 14, 2023

Edit: Against main, the issue is that the package version requirements are too high

Which ones?

Also, Name: pyhton-scikit-build-core -> python (SP)

I think you can set the version via SETUPTOOLS_SCM_PRETEND_VERSION to ignore git.

@LecrisUT
Copy link
Collaborator Author

Edit: Against main, the issue is that the package version requirements are too high

Which ones?

I think this build has the relevant dependency issues. For some reason it sometimes passes this check and can go to pytest, but that might be a bug somewhere on copr. Explicitly: cattrs, pytest, pytest-subprocess. Can copy my copr/rpm git repo to try again with different dependencies and on test branches

For the other archs, you need cmake & ninja as dependencies

Is this for BuildRequirements or Requirements? I thought it works with make as well.

The problem I saw was from using VCS version

I think it would still be nice to have this option to fallback to and together with cmake dynamic versioning. Can it be as an optional dependency?

Also, Name: pyhton-scikit-build-core -> python (SP)

What's the SP?

I think you can set the version via SETUPTOOLS_SCM_PRETEND_VERSION to ignore git.

Is this about Forge builds? This is temporary to be able to test various branches and other fixes. When in FedoraProject, I will change it to static urls and static tar balls. Although it might be useful for future maintainers to have this example rpm spec file here to test their own forks with copr.

@henryiii
Copy link
Collaborator

Ahh, forgot about testing dependencies. Yes, very new there since things were added/fixed in those packages recently. Pytest could be relaxed, 7.2 is just the first version that properly shows tracebacks with ExceptionGroups in them. We used to have a workaround by avoiding pytest's tracebacks, and bumped the pin to drop it. But there's no harm with the older versions other than poorer tracebacks. cattrs is just for a few self-constancy tests that could easily be skipped. Pytest-subprocess is probably the most important one, since we use it pretty heavily and we contributed the features we are using.

I think CMake should be in the Requirements. Either ninja or make should be in the requirements, with Ninja pretty strongly preferred. It depends on if you are targeting building with isolation or without isolation - without isolation, you need cmake and ninja or make. With isolation, you don't need those or even the [pyproject] requirements - but if you are building in isolation, you are not using the host environment at all (isolated from it!), which means you are not using your packaged version of build dependencies, but are using a virtual environment and PyPI packages in isolation. So I'd say there's never a reason to repackage this without cmake and either make/ninja.

I mean the VCS version for scikit-build-core itself. The ability to read versions from other places is in development (in #197). For scikit-build-core, we currently use hatch-vcs (which uses setuptools-scm internally) to get the version. It only requires a git checkout (or that special environment variable) if it's coming from git - from archives or SDists, the version is burned into a contained file. So setting SETUPTOOLS_SCM_PRETEND_VERSION would only be important if you are building from git & don't have a full checkout. Again, might drop this just to keep the building of this as simple as possible for distributors. Spack is likely to throw a fit about it... ;)

pyhton-scikit-build-core -> python-scikit-build-core, minor misspelling in the Name metadata field.

@LecrisUT
Copy link
Collaborator Author

It depends on if you are targeting building with isolation or without isolation

For the FedoraProject, it should be non-isolated. Are there any special flags to force it to be non-isolated?

Either ninja or make should be in the requirements, with Ninja pretty strongly preferred.

Yes, that's possible in rpm specs. I need to refresh my knowledge on how to call it. But we still need them in BuildRequirements in order to run pytest right?

I mean the VCS version for scikit-build-core itself.

Got it. But was this one of the causes of the build failures?

pyhton-scikit-build-core

Oh yes. Typos will be the death of me.

@henryiii
Copy link
Collaborator

Are there any special flags to force it to be non-isolated?

Those should already be there in build scripts, but yes, there are flags for both pip and build to avoid isolation. Packaging should usually not be isolated, and users usually build isolated (but those users also won't need the repackaged version)

But we still need them in BuildRequirements in order to run pytest right?

Ideally, yes, you should have these when testing.

But was this one of the causes of the build failures?

Yes, I think this was one of the build failures, it was trying to get the version from VCS and it was a shallow checkout or something like that.

@LecrisUT
Copy link
Collaborator Author

Are there any special flags to force it to be non-isolated?

Those should already be there in build scripts, but yes, there are flags for both pip and build to avoid isolation. Packaging should usually not be isolated, and users usually build isolated (but those users also won't need the repackaged version)

Hopefully those can be passed in the spec macro:

%pyproject_wheel

I've disabled the isolation tests, and trying to see if it can sometimes get passed the python dependency requirements. But alas it doesn't seem to collaborate

@henryiii
Copy link
Collaborator

The only failure I see is the lack of pytest 7.2 (I can reduce the dependency to 7.0 without problems, it's just for a nicer traceback) and pytest-subprocess 1.5 (this should probably be updated in Fedora). Maybe I don't know where to look, though.

@LecrisUT
Copy link
Collaborator Author

Here is the page for pytest-subprocess:
https://src.fedoraproject.org/rpms/python-pytest-subprocess

We might be able to backport the updates there to Fedora 37, but at least getting it working on rawhide is a starting point.

henryiii added a commit that referenced this issue Feb 14, 2023
This might help a little with packaging such as in #196.

Signed-off-by: Henry Schreiner <[email protected]>
@henryiii
Copy link
Collaborator

Looks like there's an issue here: https://bugzilla.redhat.com/show_bug.cgi?id=2165202

It should be a fairly straightforward update, it's just got two features and a fix (all of which I contributed in support of scikit-build-core). https://pytest-subprocess.readthedocs.io/en/latest/history.html#id1

@LecrisUT
Copy link
Collaborator Author

Yeah, those issues are automatically generated as reminders for the maintainers. Might be updated soon though since it's only been 2 weeks ago. Although to get them back-ported to older versions it needs to be requested.

On the note of package maintainers, it would be helpful to add this spec file in this repo with appropriate webhook to copr. That way a copr build will be triggered at any tag release. We can then monitor for build status and update fedoraproject repo accordingly more easily. Similarly for scikit-build.

@henryiii
Copy link
Collaborator

FYI, 0.2.0 is out, with one fewer dependency (distlib) and slightly relaxed pytest requirement.

I'm okay to add something to this repo if it's possible to test it in some way.

@LecrisUT
Copy link
Collaborator Author

Oh I understand the errors now. Previously it was erroring at the build stage, and it could never get to the test stage. I'll trigger it again tomorrow or when a repo is pushed. For some reason though F37 is failing the build stage again (log)

As for adding the spec and copr, I've opened a PR #201 so we can discuss more there.

@henryiii
Copy link
Collaborator

Have you tried setting SETUPTOOLS_SCM_PRETEND_VERSION? Looks like it can't get the version from git when looking at the log.

@LecrisUT
Copy link
Collaborator Author

LecrisUT commented Feb 15, 2023

Unfortunately I don't see a way to prepend environment variables in the spec file. Curiously though only F37 is affected. I'll try to switch it to the version model if it solves it. Nope same issue.

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

2 participants