raimon49 / pip-licenses

Dump the license list of packages installed with pip.
MIT License
307 stars 43 forks source link

Add `--python` option #150

Closed ClementPinard closed 1 year ago

ClementPinard commented 1 year ago

Fixes #149 Still Work in Progress, happy to receive feedback !

Not particularly happy with the fact that you have to specify until the site-packages folder and not just the venv folder. suggestions welcome.

ClementPinard commented 1 year ago

here is how you can use it if you want licenses (and version numbers) from another environment. This would be particularly useful in a CI where you want exact license and versions of your project dependencies, which could be clashing with pip-licenses's dependencies (even though they are minimal)

source /path/to/venv/bin/activate
/path/to/pip-licenses --path $(python -c "import sys; print(','.join(filter(bool, sys.path)))")

this would be particularly interesting with pip-licenses installed in you system python

example with poetry

pip install pip-licences
source `poetry env info --path`/bin/activate
pip-licenses --path $(python -c "import sys; print(','.join(filter(bool, sys.path)))")
thejcannon commented 1 year ago

Not particularly happy with the fact that you have to specify until the site-packages folder and not just the venv folder.

I've seen other tools allow for a --python flag pointing to a python interpreter, then it grabs sys.path from that. So you'd point to <venv>/bin/python which is much more stomachable and also helps with custom Python shenanigans and might handle .pth files better.

ClementPinard commented 1 year ago

So I guess launching another python from subprocess ? and then getting back the printed sys.path ? That would be possible indeed ! I'll try to come up with something that uses both --python and --path as they could be added together in the end.

raimon49 commented 1 year ago

@ClementPinard I read #149 and using distributions([folder_path]) to explore outside folders is a cool and interesting idea. Will the implementation of the --path option be further updated as we see the discussion?

raimon49 commented 1 year ago

I am not sure if I should merge this PR and include it in the next release 4.2.0.

Is the implementation of the --path option complete?

ClementPinard commented 1 year ago

Hey, sorry for late answer. If no objections, I'm going to implement the --python option as suggested by @thejcannon very soon (today if I find the time)

From that we can work on potential review comments, and I think the feature will be complete.

ClementPinard commented 1 year ago

So I said I would keep both --path and --python. Turns out, when I had to give an example of how you would need a path option, I could not come up with something useful not already done by the --python .

I you need to get licenses from an extra folder, you might as well install the package with -e option instead of playing with sys.path

As a result, I decided to only keep the --python option, to not add a superfluous option in an already packed arg parser. Feel free to share your thoughts on this, and an example on how you would need it in a clean way :upside_down_face:

@raimon49 I now think this PR is finished, and open for review.

codecov[bot] commented 1 year ago

Codecov Report

Merging #150 (4129b76) into master (471fc3d) will decrease coverage by 0.24%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #150      +/-   ##
==========================================
- Coverage   99.74%   99.50%   -0.24%     
==========================================
  Files           1        1              
  Lines         392      408      +16     
==========================================
+ Hits          391      406      +15     
- Misses          1        2       +1     
Impacted Files Coverage Δ
piplicenses.py 99.50% <100.00%> (-0.24%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ClementPinard commented 1 year ago

I add a test that creates a temporary empty virtual env, and thus should only find pip and setuptools.

I had some troubles figuring out why it still found all packages in main virtual environment, and solved this problem by calling the alternative python executable with empty environment variables for PYTHONPATH and VIRTUAL_ENV

output = subprocess.run(
            [executable, "-c", script],
            capture_output=True,
            env={**os.environ, "PYTHONPATH": "", "VIRTUAL_ENV": ""},
        )

Would there be some usecase where keeping them is actually important ? FYI, not resetting PYTHONPATH breaks the tests because it is set to the main virtual environment, resetting VIRTUAL_ENV does not

raimon49 commented 1 year ago

I add a test that creates a temporary empty virtual env, and thus should only find pip and setuptools.

Oh, it had never occurred to me that this was the way to do it. I consider it a good enough test case.

Why are CI tests failing only in the Python 3.8 environment?

ClementPinard commented 1 year ago

Looks like I was not typehinting the right way. The venvBuilder context is a types.SimpleNamespace object (https://docs.python.org/3.8/library/types.html#types.SimpleNamespace), but I typehinted it as venv.SimpleNamespace I suspect venv did import SimpleNamespace but only after a certain version, because mypy did not complain for my python3.10 version

I put back the original type in the typehinbt and we should be good to go. If not, I'll just remove the typehint altogether.

raimon49 commented 1 year ago

@ClementPinard Thank you for your great contribution! I will merge this PR into the next release candidate version.

raimon49 commented 1 year ago

pip-licenses version 4.2.0 RC2 is now available. This PR includes.

If you have any concerns about the actual use of the system, please provide feedback. Thanks.

$ pip install "pip-licenses==4.2.0rc2"