search

duckinator / bork

A Python build and release management tool.
https://bork.readthedocs.io
MIT License
12 stars 2 forks source link

Switch to using `build` (instead of `pep517`) #233

Closed nbraud closed 3 years ago

nbraud commented 3 years ago

@duckinator I ran the tests, and it builds one of my local projects without issue. I also built bork as a zipapp (using itself, of course) and used the zipapp to rebuild bork and my project.

nbraud commented 3 years ago

I'm confused why the linting fails in the Zippapp bootstrap (and only then) ; I don't even see where does the import-order linter come from, let alone whether it is erroneously treating build as if it was part of the standard distribution.

duckinator commented 3 years ago

The current pylint setup is to enable everything, and disable what we don't want. I'm okay adding wrong-import-order to the list of disabled ones — I rarely find it useful. https://github.com/duckinator/bork/blob/master/.pylintrc#L21-L27

Also: the zipapp kludge is still needed, unfortunately, because the build library uses the pep517 library internally. We don't actually get rid of the pep517 dependency, we just get a better API to work with.

duckinator commented 3 years ago

@nbraud I'd say try reverting the commit removing the zipapp kludge and work from there.

nbraud commented 3 years ago

Also: the zipapp kludge is still needed, unfortunately, because the build library uses the pep517 library internally. We don't actually get rid of the pep517 dependency, we just get a better API to work with.

Oh sadness xD I guess it only worked because I was testing in an environment where pep517 was installed.

nbraud commented 3 years ago

PS: Looks like the erroneous lint still triggers in the boopstrat tests.

duckinator commented 3 years ago

You can go ahead and add wrong-import-order to the list of disabled checkers here: https://github.com/duckinator/bork/blob/master/.pylintrc#L21-L27

I honestly thought I'd disabled it at some point. Must've been another project. :stuck_out_tongue:

nbraud commented 3 years ago

OK, it looks like there are a few things going on:

nbraud commented 3 years ago

@duckinator Could you look into the breakage on macOS? I don't have any Apple devices to test with :3

duckinator commented 3 years ago

@nbraud looks like we're running into https://github.com/pypa/build/issues/108. Looks like the tl;dr is it's a resolved setuptools bug, and pinning setuptools>42 in pyproject.toml:

requires = ["setuptools > 42", "wheel"]

(We did similar with Emanate ages ago, for different reasons, in duckinator/emanate/pull/136)

nbraud commented 3 years ago

Looks like version constraints in pyproject.toml's build-system.requires are silently ignored. It's ridiculous that, in 20 fucking 21, none of the Python tooling actually obeys version constraints, but it's somewhat out-of-scope for Bork to fix. >_>'

For now, I kicked in a setup step on macOS that updates pip and setuptools. I guess I should do that on all platforms, just to be sure?

duckinator commented 3 years ago

It's ridiculous that, in 20 fucking 21, none of the Python tooling actually obeys version constraints

I know that pip is working on fixing this problem on their side. It was part of their big rewrite of the resolver, iirc.

I'll investigate and see if I can find what exact thing is responsible for ignoring the version here.

duckinator commented 3 years ago

For now, I kicked in a setup step on macOS that updates pip and setuptools. I guess I should do that on all platforms, just to be sure?

In theory we should be able to get away with just updating pip, which is probably good practice anyway. But yeah, since it's giving us problems, let's install new setuptools as well.

duckinator commented 3 years ago

Opened pypa/pip#9416 about being unable to pin requirement versions in pyproject.toml.

nbraud commented 3 years ago

Opened pypa/pip#9416 about being unable to pin requirement versions in pyproject.toml.

From the discussion there, it looks like pip honors the version constraints, but the projects being built in tests need that version constraint (for their build to succeed on macOS)

If you can reproduce the issue locally (I do not have a macOS box to test with), could you confirm that setuptools > 42 is sufficient to fix the issue? I can then go add the version constraint everywhere it's needed.

duckinator commented 3 years ago

@nbraud I do not have access to a mac, so can only reproduce it in CI. I'm not entirely sure how to approach this, tbh.

nbraud commented 3 years ago

@nbraud I do not have access to a mac, so can only reproduce it in CI. I'm not entirely sure how to approach this, tbh.

Oh, OK.

I'd suggest merging this anyhow, since the issue is shown to be outside of bork and the tests are working. Maybe we can enlist @AstraLuma's help to get this confirmed and fixed in the other projects?

duckinator commented 3 years ago

@nbraud sounds like a plan to me. merge when you think it's ready. :+1:

nbraud commented 3 years ago

bors r=duckinator

bors[bot] commented 3 years ago

Build succeeded: