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

⚠️ CI failed on Check Manifest ⚠️ #28695

Closed
scikit-learn-bot opened this issue Mar 26, 2024 · 6 comments · Fixed by #28757
Closed

⚠️ CI failed on Check Manifest ⚠️ #28695

scikit-learn-bot opened this issue Mar 26, 2024 · 6 comments · Fixed by #28757

Comments

@scikit-learn-bot
Copy link
Contributor

scikit-learn-bot commented Mar 26, 2024

CI is still failing on Check Manifest (Apr 09, 2024)

@github-actions github-actions bot added the Needs Triage Issue requires triage label Mar 26, 2024
@lesteve
Copy link
Member

lesteve commented Mar 26, 2024

Very likely related to switching to meson as the main build backend #28506.

@jeremiedbb
Copy link
Member

Looks like check-manifest doesn't support alternative build backends (mgedmin/check-manifest#163)

@jeremiedbb jeremiedbb added Build / CI and removed Needs Triage Issue requires triage labels Mar 26, 2024
@betatim
Copy link
Member

betatim commented Mar 26, 2024

It looks like meson dist doesn't support being run on something that isn't a git repository. Is that also how you read the error message? If yes, it makes me wonder if we can contribute something to meson to make this work (no need to change check manifest)?

@lesteve
Copy link
Member

lesteve commented Mar 26, 2024

Another option would be to switch to check-sdist as mentioned in mgedmin/check-manifest#163 (comment).

I quickly checked and it seems like pipx run check-sdist is happy and says:

Successfully built scikit_learn-1.5.dev0.tar.gz
SDist matches git

Full disclosure, I don't have a good grasp of what either check-manifest nor check-sdist is doing.

Probably a good thing to do would be something like numpy/numpy#23981 (comment) to make sure there are no major differences in the produced sdist between before and after switching to Meson as our main build backend.

Side-comment: to build a sdist it seems like people do:

python -m build --sdist

for example numpy:
https://github.com/numpy/numpy/blob/c3694335faffe35e3d961feffefdc0a4b9a665da/.github/workflows/wheels.yml#L220-L223

@ogrisel
Copy link
Member

ogrisel commented Mar 27, 2024

Looking at the check-sdist source code I think it should be quite helpful. +1 for replacing check-manifest by check-sdist on my side.

@lesteve
Copy link
Member

lesteve commented Apr 3, 2024

I opened #28757 to tackle this.

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

Successfully merging a pull request may close this issue.

5 participants