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

Add dev.py implementing dev command shorthands #3128

Merged
merged 2 commits into from
Nov 3, 2024
Merged

Conversation

ankith26
Copy link
Member

@ankith26 ankith26 commented Sep 28, 2024

This is an attempt at simplifying the steps a pygame-ce dev has to do to get out a complete working change that passes our CI.

Some goals

  • help new devs figure stuff out faster. This script does an auto-install of all dependencies needed, so virtually no setup is needed to run this script. For people who don't like the idea of a script messing with the global package space, this script also provides a flag to do everything inside a venv.
  • make the dev workflow more efficient, with shorthand commands for all the common steps one usually needs.
  • get everyone onboard the editable install strategy, with one convenient command.
  • serve as a self-documenting system (with help commands)

@ankith26 ankith26 requested a review from a team as a code owner September 28, 2024 14:22
@ankith26
Copy link
Member Author

ankith26 commented Sep 28, 2024

Building pygame

python dev.py build

Builds and installs pygame in editable mode by default. Can pass a flag to do a regular install. Either way, this command is bound to be faster and efficient than a pip install . or an equivalent because it doesn't do build isolation and uses the same builddir every run.

Also has a flag to build in debug mode (optimizations turned off and debug symbols turned on) easily. Finally, this command also enables more warnings as errors by default to mimic CI as closely as possible. Again, there's a flag to opt out of this.

Building docs

python dev.py docs builds the docs, python dev.py build --full does a full generation.

Running code format

python dev.py format should just format the code for the peeps who don't like or can't use the pre-commit strategy. This still uses pre-commit internally, but does auto-install, and doesn't install any git hooks. The command should therefore be pretty much exactly like the legacy setup.py format, functionally.

Running lint

python dev.py lint runs pylint tests on the codebase, mimic-ing CI.

Generating and testing type stubs

python dev.py stubs runs mypy stubtest and also generates the automated bits of the stubs that we use.

Running tests

python dev.py test is a simple shorthand for python -m pygame.tests, though as a little enhancement it can also take a list of modules to run tests on that subset only.

@ankith26 ankith26 marked this pull request as draft September 28, 2024 14:43
@ankith26
Copy link
Member Author

Converting it to a draft because there's still stuff to do.

  • Most importantly, get more feedback on this PR.
  • Make CI use this script directly where possible

dev.py Outdated Show resolved Hide resolved
dev.py Show resolved Hide resolved
dev.py Show resolved Hide resolved
@ankith26 ankith26 force-pushed the ankith26-dev-py branch 2 times, most recently from 0ef397a to d92a9e3 Compare October 2, 2024 09:13
@ankith26 ankith26 marked this pull request as ready for review October 2, 2024 09:25
@ankith26
Copy link
Member Author

ankith26 commented Oct 2, 2024

This PR is ready for review and suggestions. I have updated the CI to add a dev.py all test

Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lovely, these commands all work great locally on windows for me.

Should simplify the setup, and onboarding for new developers - especially once we've documented all these on the developer wiki pages.

@ankith26
Copy link
Member Author

ankith26 commented Oct 6, 2024

Just added one commit, for supporting the --sdl3 option. So one can do py dev.py build --sdl3

@damusss
Copy link
Contributor

damusss commented Oct 8, 2024

@ankith26 So the last time I tested it I noticed it was nice but a little bit slower than usual because every command checks if every single module is installed.
I think when dev.py finishes executing it should save a .dev-cache.txt file (gitignored ofc) that keeps track of the installed version of the needed dependencies.
This way when you start dev.py again instead of checking with pip (very slow) it only needs to check the contents of that file, and install things only if they aren't high enough or not present.
What do you think?

@ankith26
Copy link
Member Author

I would like to explore ways of speeding it up further, but I don't think .dev-cache.txt is the way to go. Basically, the user may make changes to the environment by using pip directly, and then the cache would be useless and incorrect.

What I could try to do, is somehow add a faster way to check that the version requirements match via pip itself, and therefore eliminate the networking call which I think is the reason for most of the initial slowdown

@ankith26
Copy link
Member Author

In my latest commit I just implemented a solution that should speed up the startup time of the script.

Basically I was trying to unconditionally upgrade pip in the script, and this can be slow or even error out because it does networking. Now I added logic that tests for the pip version and only attempts the upgrade if it's below some version threshold set.

This way, this script now does 0 networking operations if all dependencies are in place and have expected versions.

@ankith26 ankith26 force-pushed the ankith26-dev-py branch 7 times, most recently from 8488e97 to 1e2aa89 Compare October 23, 2024 11:09
dev.py Outdated Show resolved Hide resolved
pyproject.toml Outdated
"cython<=3.0.11",
"sphinx<=7.2.6",
"sphinx<=8.1.3",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these just general package updates? I would like Sphinx in a different PR, usually each major version requires some docs reworking.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did check the docs after this change and everything seemed okay on a glance, but yeah maybe it should be a separate PR. The reason these bumps are needed is to get this PR pass msys2/multiarch tests because there we use preinstalled dependencies and if the version constraint does not match it triggers a reinstall, which can error on those exotic platforms

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't fully understand, but a random msys2 environment having preinstalled deps should not be setting our version constraints for us.

dev.py Outdated Show resolved Hide resolved
dev.py Outdated Show resolved Hide resolved
@Starbuck5
Copy link
Member

This is great stuff! I put down a few comments, the one I care most about is the sphinx one. But once those are resolved I will approve.

Copy link
Member

@Starbuck5 Starbuck5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pushing some of the changes out of this PR.

Let's get this in!

@ankith26 ankith26 added this to the 2.5.3 milestone Nov 3, 2024
@ankith26 ankith26 merged commit de88561 into main Nov 3, 2024
24 checks passed
@ankith26 ankith26 deleted the ankith26-dev-py branch November 3, 2024 11:43
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

Successfully merging this pull request may close these issues.

4 participants