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

Test installing project on CI #90

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

EliahKagan
Copy link
Contributor

This changes the CI workflow to install the project itself to get its dependency, rather than installing the dependency from requirements.txt. This is the more important thing to test, because it verifies that the project is installable and effectively declares the dependencies it needs.

This changes the CI workflow to install the project itself to get
its dependency, rather than installing the dependency from
requirements.txt. This is the more important thing to test, because
it verifies that the project is installable and effectively
declares the dependencies it needs.
@EliahKagan EliahKagan marked this pull request as ready for review September 11, 2023 06:36
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

I was never aware the . syntax even existed and wonder if ultimately is just finds requirements.txt by convention, or if there is any other source of dependency information.

@Byron Byron merged commit d22ac20 into gitpython-developers:master Sep 11, 2023
@EliahKagan
Copy link
Contributor Author

The . syntax installs the project defined in the current directory. It doesn't know anything about requirements.txt, but it does know about setup.py, as well as the more modern setup.cfg and pyproject.toml.

It also can handle build backends that are not currently installed. For example, starting in Python 3.12, setuptools is not automatically installed, so in fresh Python 3.12 virtual environments a setup.py script cannot be run directly, but pip install . will still manage to run it. (This is the "Installing backend dependencies" step sometimes shown in pip output.) This works for backends other than setuptools, too. For example, a project that uses poetry and defines everything in pyproject.toml in a way only poetry understands can be installed with pip install . even if poetry is absent; pip installs the poetry build backend.

So setup.py is actually providing the dependencies that pip install . installs for gitdb:

gitdb/setup.py

Line 23 in d22ac20

install_requires=['smmap>=3.0.1,<6'],

That happens to be the same as in requirements.txt, but it doesn't have to be. If it weren't, installing from requirements.txt could make something succeed that would fail when the package is installed from PyPI (hence why I regard pip install . to be a better way to install required dependencies before running tests).

Although pip install . doesn't use requirements.txt, it will still be used if setup.py reads and uses it, as is the case in GitPython.

The other benefit of pip install . is that it installs the project, not just its dependencies. Because this is installing a package--albeit one that it is building on the fly from the working directory--it more closely matches what happens when the project is installed from PyPI. A middle ground can be achieved by also passing -e/--editable, which "installs a package" but the package is just the local files, rather than copies of them. This is very useful for local development, because changes are immediately reflected in the "installed package," but it's not necessary for CI.

(Of course, another way to test behavior of a built sdist or wheel is to actually build one and install from it in a new environment, which tools like tox and nox automate. I do not claim that using pip install . achieves that level of similarity to installing from PyPI.)

@EliahKagan EliahKagan deleted the ci-install branch September 11, 2023 07:29
@Byron
Copy link
Member

Byron commented Sep 11, 2023

Thanks a lot for the explanation, it's much appreciated.

The other benefit of pip install . is that it installs the project, not just its dependencies. Because this is installing a package--albeit one that it is building on the fly from the working directory--it more closely matches what happens when the project is installed from PyPI. A middle ground can be achieved by also passing -e/--editable, which "installs a package" but the package is just the local files, rather than copies of them.

The -e flag seems like the secret ingredient that I have been missing for a long time now. I simply wasn't able to run GitPython locally anymore once the hacks that I added early on (a decade ago?) to allow that were removed.

EliahKagan added a commit to EliahKagan/smmap that referenced this pull request Sep 17, 2023
This updates smmap's CI configuration in ways that are in line with
recent updates to gitdb's. In most cases there is no difference in
the changes, and the reason for the updates is more to avoid
confusing differences than from the value of the changes
themselves. In one case, there is a major difference (fetch-depth).

- gitpython-developers/gitdb#89 (same)

- gitpython-developers/gitdb#90 (same)
  It's just the project, not dependencies, but otherwise the same.

- gitpython-developers/gitdb#92 (opposite)
  This is the major difference. We don't need more than the tip of
  the branch in these tests. Keeping the default fetch-depth of 1
  by not setting it explicitly avoids giving the impression that
  the tests here are doing something they are not (and also serves
  as a speed optimization).

- gitpython-developers/gitdb#93 (same)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants