Skip to content

Adds a new version of the hook to download releases#336

Merged
JohnnyMorganz merged 2 commits into
JohnnyMorganz:masterfrom
IamTheFij:py-hook
Jan 9, 2022
Merged

Adds a new version of the hook to download releases#336
JohnnyMorganz merged 2 commits into
JohnnyMorganz:masterfrom
IamTheFij:py-hook

Conversation

@IamTheFij
Copy link
Copy Markdown
Contributor

This version will auto download the current release from Github and then
run that instead of looking at system or compiling with cargo. It should
be fast and transparent for users.

I've found myself wanting to do thing similar to this more than once with pre-commit as well as generally wanting to automate downloading of releases from Github or Gitea in other scripts, so I wrote something to make this happen. The Python package release-gitter allows downloading releases based on a file template and detecting the current os and arch.

What this looks like in a project like StyLua is a pyproject.toml file that tells pip (the installer that pre-commit uses) that this repo is a Python package. It uses a custom package builder that calls release-gitter to, instead of bundling a bunch of Python files, downloads the specified StyLua release from Github and bundles that instead.

The pyproject.toml is where release-gitter is configured.

At the top, [build-system] is the part where we tell it to use release-gitter and the pseudo_builder.

In [tool.release-gitter] we specify the arguments to be passed to release-gitter. For StyLua these are pretty simple. We list the file to extract from the zip, the format template for identifying the zip we weant, an exec command to make stylua executable once downloaded, and then a map that maps platform.system() names to the system names used in the StyLua assets.

It gets everything else from either git remote or Cargo.toml.

This version will auto download the current release from Github and then
run that instead of looking at system or compiling with cargo. It should
be fast and transparent for users.
@JohnnyMorganz
Copy link
Copy Markdown
Owner

Thank you for your explanation of what's happening here!

The only thing I'm wary about is, although you've written a nice explanation, it is still something which I am not fully confident with doing myself (mainly the pyproject stuff, first time seeing that!). But, given that it's only for a small part of the overall project (just a helper for one of the precommit hooks), I'm willing to still merge it

I was just wondering, is the downloaded binary cached on the user system? I imagine for frequent commits, continuously redownloading from GitHub will be annoying, and it might hit rate limits

Comment thread .pre-commit-hooks.yaml Outdated
- lua

- id: stylua-release
name: StyLua Release
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Should probably be aligned with the other names

Suggested change
name: StyLua Release
name: StyLua (release)

Also is release the right name here? Maybe github instead? I'm not really sure myself

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm. Yea, I think (Github) or (Github Release) makes sense to give a better idea of where it's actually coming from.

@JohnnyMorganz
Copy link
Copy Markdown
Owner

JohnnyMorganz commented Jan 7, 2022

Also, does this respect the version tag provided by precommit? Or will it always download the latest binary

@IamTheFij
Copy link
Copy Markdown
Contributor Author

IamTheFij commented Jan 7, 2022 via email

@IamTheFij
Copy link
Copy Markdown
Contributor Author

I wasn’t able to look up the git tag used because it appears that pre-commit uses a shallow clone, but it pulls the version from Cargo.toml.

I managed to make this an option. Adding version-git-tag = true to the pyproject file will use the latest git tag rather than Cargo.toml.

@JohnnyMorganz
Copy link
Copy Markdown
Owner

Thanks for this, I haven't tested it myself, but I assume it works alright

@JohnnyMorganz JohnnyMorganz merged commit 3b2c724 into JohnnyMorganz:master Jan 9, 2022
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.

2 participants