Update warning for pyproject.toml license spec

beeware / briefcase

Tools to support converting a Python project into a standalone native application.

https://briefcase.readthedocs.io/

BSD 3-Clause "New" or "Revised" License

2.47k stars 350 forks source link

Update warning for pyproject.toml license spec #1851

Closed rmartin16 closed 4 weeks ago

rmartin16 commented 1 month ago

Changes

Rework a bit the license warning from https://github.com/beeware/briefcase/pull/1812 to include additional information
This change also drops a warning for each time the license field is specified in pyproject.toml

Notes

The presentation of the original warning was a little jarring to me since it spread across my entire terminal. Additionally, though, it wasn't using the "WARNING" banner typically reserved for messages like this.
Moreover, I didn't really understand the change I was being asked to make.

PR Checklist:

[X] All new features have been tested
[X] All new features have been documented
[X] I have read the CONTRIBUTING.md file
[X] I will abide by the code of conduct

freakboy3742 commented 4 weeks ago

The Windows failure definitely isn't transient; it's persisted through multiple re-runs, include full reruns after a rebase.

In a desparate search for anything that could be causing this, I noticed that pytest-xdist is installed; I removed it, and the problem went away. I don't know why xdist would be the problem, other than the fact that multi-threading a test suite could cause some interesting cleanup side effects.

Tagging @rmartin16 in case you have strong opinions to the contrary; my inclination is to drop xdist for the test suite if it's going to be more stable.

rmartin16 commented 4 weeks ago

my inclination is to drop xdist for the test suite if it's going to be more stable.

Would tox -e py-fast still work?

Moreover, though, what if you roll back the recent version bump for pytest-xdist? They reworked how process forking works in the latest version.

freakboy3742 commented 4 weeks ago

my inclination is to drop xdist for the test suite if it's going to be more stable.

Would tox -e py-fast still work?

I guess not - so there'd need to be other cleanups.

Moreover, though, what if you roll back the recent version bump for pytest-xdist? They reworked how process forking works in the latest version.

Well that'd do it... I'll try and see.

freakboy3742 commented 4 weeks ago

Moreover, though, what if you roll back the recent version bump for pytest-xdist? They reworked how process forking works in the latest version.

Well that'd do it... I'll try and see.

Looks like it doesn't - 3.5.0 generates the same error.

So - my inclination is to drop xdist. While parallel support is nice to have, it doesn't yield that much of a performance benefit; plus, dropping the extra dependency is one less cause of error, and one less dependency to keep up to date. Thoughts?

rmartin16 commented 4 weeks ago

Moreover, though, what if you roll back the recent version bump for pytest-xdist? They reworked how process forking works in the latest version.

Well that'd do it... I'll try and see.

Looks like it doesn't - 3.5.0 generates the same error.

well, my guess is coverage is doing something weird....at least, if my previous investigation showing this unclosed file is coming from coverage is correct.

So - my inclination is to drop xdist. While parallel support is nice to have, it doesn't yield that much of a performance benefit; plus, dropping the extra dependency is one less cause of error, and one less dependency to keep up to date. Thoughts?

I do normally use tox -e py-fast...since it is about 3x faster (although, tox -e py only takes 18s for me...).

One more plea perhaps...what if we just don't install pytest-xdist for 3.13? And if the problem still exists when 3.13 is released, we drop it completely then.

freakboy3742 commented 4 weeks ago

One more plea perhaps...what if we just don't install pytest-xdist for 3.13? And if the problem still exists when 3.13 is released, we drop it completely then.

I can live with that for now. It's also possible that this might be fixed in 3.13-b2, or with a coverage bump at some point; if we get close to 3.13-final and this is still lingering, we can re-evaluate then.

freakboy3742 commented 4 weeks ago

@rmartin16 Looks like the py3.13 exclusion on xdist has worked; If you're happy, feel free to merge.