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 pre-commit hook to update editable install versions #642

Closed
wants to merge 3 commits into from

Conversation

MHendricks
Copy link

This adds a pre-commit hook to setuptools_scm that will automatically update the .egg-info for a setuptools_scm enabled project. It uses the newer post-commit, post-merge, post-checkout, post-rewrite hooks that pre-commit supports.

This MR is working for me, but still needs some work before it would be ready to merge into setuptools_scm. I'm providing this MR as a proof of concept and to ask if its something that can/should be added to setuptools_scm, or if I should make my own package.

I have a few TODO's that will need looked into:

  • I'm using setup.py but that is deprecated, update this to use pyproject.toml and if desired, support the other deprecated methods of building the egg_info.
  • Improve the detection of rebase, squash commits so it can skip calling egg_info repeatedly after every time that process does a commit.
  • Should we create a console_scripts entry_point instead of using __main__.py? Ie python -m setuptools_scm post-commit
  • Potentially use argproperty or click to parse the required cli arguments of __main__.py or the console_script.
  • Testing.

@MHendricks MHendricks marked this pull request as ready for review October 21, 2021 01:35
Copy link
Contributor

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

I like the idea but wonder how to correctly integrate it with pep-620 as that will not support the in worktree Metadata one could update

.pre-commit-hooks.yaml Outdated Show resolved Hide resolved
src/setuptools_scm/__main__.py Outdated Show resolved Hide resolved
@MHendricks
Copy link
Author

I'm not sure what you mean by integrate it with pep-620, that seems to be dealing with the c api.

I figured out that post-commit lets you configure arguments to pass to its entry. Using this I was able to make it so you can configure the build command that is run so you can change it from the default of "python setup.py egg_info". See the updated README.rst for details.

I am curious if there is a newer way to build the egg_info than calling setup.py? That is how all of our pip packages are setup and from what I can tell setup.py is still required to support editable installs, but I haven't done much research on that yet.

Btw, it took me a while to figure out how to easily test uncommitted pre-commit hook plugin changes. I use this command in the directory of my test pip package. I have my working copy of setuptools_scm next to the test pip repo. It doesn't work with the custom args in .pre-commit-config.yaml though.

pre-commit try-repo ../setuptools_scm --verbose --hook-stage post-commit

@RonnyPfannschmidt
Copy link
Contributor

Apologies, there is a different pep for editable installs using wheels, once it is widespread, updating local egg info is likely not going to be enough

Im currently down with a cold and might Nedd until next weekend to get back at this

@MHendricks
Copy link
Author

No problem, hope you feel better.

I'm guessing you were talking about pep-660, and wanting to support removing setup.py. By letting the hook config specify the build command in .pre-commit-config.yaml, it should be fairly easy to migrate to other build mechanisms. If there is a better command we can change the default command to match. Though ideally it is as fast as possible and doesn't require knowing where the working copy may be editable installed to.

I've spent the extra time cleaning up and finishing code so the pull request is ready for a full review now. I've cleaned up my prototype code, updated the readme and created tests. Let me know what you think when you get a chance to look at it.

@MHendricks
Copy link
Author

Have you gotten a chance to look this over?

@RonnyPfannschmidt
Copy link
Contributor

i managed to take a initial look

i am under the impression this should register virtual envronments that have been used as install target, and decide based on build backend

the classical setuptools one just needs to regen the egg info

modern pakcages need a pip install -e for PEP660

@RonnyPfannschmidt RonnyPfannschmidt marked this pull request as draft December 29, 2021 22:43
@MHendricks
Copy link
Author

Ok, I've made it create a virtualenv and do a full pip install -e . command by default. You can still use the egg_info build process by passing a build command but this is no longer the default.

I'm creating a virtualenv named .setuptools_scm in the root of the repo by default but this can be configured as well. Let me know if you have any suggestions for the name of the virtualenv. I want to make sure to re-use the virtualenv so its not slowed down by creating it each time a git command is run.

I rebased and squashed all of the previous code into the Create pre-commit hook commit. All new code is in the second commit.

@RonnyPfannschmidt
Copy link
Contributor

i think there has been a misunderstanding, creating a setuptools_scm specific virtualenv to do the updates in is pretty much not going to help the users who have their own, potentially numerous virtualenvs that each need updates

@RonnyPfannschmidt
Copy link
Contributor

i believe some coordination with pip will be necessary

@MHendricks
Copy link
Author

Ok I did some more reading and I think I understand what you mean. PEP 660 requires putting the ProjectName.dist-info folder into the wheel. This means that each virtualenv's site-packages contains it's own version information and its not stored in a central location of the source repo's ProjectName.egg-info directory. While it doesn't matter that its a .dist-info instead of an .egg-info folder for this PR, it does matter that its not stored inside the source repo.

So, the only way to update the version for all of the editable installs the way I want is to somehow be able to find all of a users's virtualenv and system installs and run a editable install.

If that's the case, I really don't like that, it makes this so much more complicated and slow. My first thought is that when doing an editable install we would add that path to a file in the source repo and the hook would process each path, and have to deal with also updating the system pythons.

I'm not a fan of this. I'm not sure if uninstalling a editable package would allow us to automatically remove that path from the file and I'm sure there are other ways that could be broken, so we would have to add checks for if its currently editability installed for a given path.

@RonnyPfannschmidt
Copy link
Contributor

closing as infeasible or this library as of now - build backends for the editable wheels need to provide the hooks

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