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

Feature/improve package info handling #161

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

reorx
Copy link
Contributor

@reorx reorx commented Jun 25, 2018

Goals:

re-org the relationship of the functions below:

  • save_package_info: receives data from installation process, use make_package_info to create the dict, save it to file
  • make_package_info: in charge of creating the package_info dict from scratch
  • get_package_info: if the json file exists, get and parse its data; if not, call make_package_info to get the dict

Eventually, get_package_info's behavior becomes consistent and predictable, so now we can use it in list_everything easily and intuitively.

Other changes:

  • Add an uninstall-pipsi.py script
  • Add -e to pip when running get-pipsi.py .
  • Remove duplicated list_everything call

TODO:

  • Define a class to handle read/write about package_info

reorx added 16 commits June 25, 2018 16:35
for the frequent need to install/uninstall pipsi,
also useful for users who just want to uninstall/reinstall.
fix no scripts display when `list --versions`
re-org the relationship of the functions below:
- save_package_info: receives data from installation process, use `make_package_info` to create the dict, save it to file
- make_package_info: in charge of creating the package_info dict from scratch
- get_package_info: if the json file exists, get and parse its data; if not, call `make_package_info` to get the dict

Eventually, `get_package_info`'s behavior becomes consistent and predictable, so now we can use it in `list_everything` easily and intuitively.
pass the right scripts to `save_package_info`
as we already have get_package_info to provide us all we need, this function is just confusing and useless
to make it not so confusing when `normalize_package` exists
- move make_package_info to PackageInfo.create_from_venv_path
- serialize/deserialize package info by PackageInfo methods to_json and create_from_json
- get attributes by dot instead of slice
remove `from os.path import ...`, all functions are now referred by `os.path.xxx()`
because pipsi does not handle multiple version of a same package now,
ultimately `pipsi install xxx=1.0.0` and `pipsi uninstall xxx=1.0.0` are seen as `xxx` package under the hood,
so we should prompt `xxx is not installed`, `do you want to uninstall xxx` instead of `xxx=1.0.0` with version number.
rename to `list_package_infos`, change its return to list of PackageInfo,
change the output rendering in `list_cmd`
@smancill
Copy link

I confirm this was the only way I got pipsi working.

The command I used was a combination of #162 and this branch:

$ curl https://raw.githubusercontent.com/reorx/pipsi/41ec097047ede9af0381d86080ca4b59cbe51062/get-pipsi.py | python3 - --src 'git+https://github.com/reorx/pipsi.git@41ec097047ede9af0381d86080ca4b59cbe51062#egg=pipsi'

@reorx
Copy link
Contributor Author

reorx commented Aug 24, 2018

@mitsuhiko @RonnyPfannschmidt guys, reviewing of this project has been very inactivate recently, and I'm sad to see that. Pipsi is a very great tool, I personally use it a lot, I think it can make better difference, and it still has a lot of improvement space. I'm wondering if I can be one of the maintainers, I have at lease one day free to do open source coding each week. I really want to push pipsi forward, to release a stable and mature version for all the people who care about it. How do you think? Looking forward to your reply :)

@sahensley
Copy link

After installing Pipsi from this branch, I now have a functioning Pipsi installation which is backed by Python 3.6.6. I can also verify that the virtual environments are being made via the venv module instead of virtualenv.

I'd love to see @reorx added as a maintainer and possibly a few others to help keep this project afloat.

@sciyoshi
Copy link

Thanks @reorx, the package_info.json issue causes problems when pipsi shares its venv directory with other tools like pew. 👍 to merging this as well as expanding the set of maintainers if that's something @mitsuhiko would support.

@sciyoshi
Copy link

@reorx I did notice an issue however:

Packages and scripts installed through pipsi:
  Package "awscli":
Traceback (most recent call last):
  File "/Users/sciyoshi/.local/bin/pipsi", line 11, in <module>
    load_entry_point('pipsi', 'console_scripts', 'pipsi')()
  File "/Users/sciyoshi/.local/venvs/pipsi/lib/python3.6/site-packages/click/core.py", line 722, in __call__
    return self.main(*args, **kwargs)
  File "/Users/sciyoshi/.local/venvs/pipsi/lib/python3.6/site-packages/click/core.py", line 697, in main
    rv = self.invoke(ctx)
  File "/Users/sciyoshi/.local/venvs/pipsi/lib/python3.6/site-packages/click/core.py", line 1066, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/sciyoshi/.local/venvs/pipsi/lib/python3.6/site-packages/click/core.py", line 895, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/sciyoshi/.local/venvs/pipsi/lib/python3.6/site-packages/click/core.py", line 535, in invoke
    return callback(*args, **kwargs)
  File "/Users/sciyoshi/.local/venvs/pipsi/lib/python3.6/site-packages/click/decorators.py", line 27, in new_func
    return f(get_current_context().obj, *args, **kwargs)
  File "/Users/sciyoshi/.local/venvs/pipsi/src/pipsi/pipsi/__init__.py", line 607, in list_cmd
    click.echo('    ' + script)
TypeError: must be str, not list

It seems like the file gets re-written with scripts that look like this: {"name": "httpie", "version": "0.9.9", "scripts": [["/Users/sciyoshi/.local/venvs/httpie/bin/http", "/Users/sciyoshi/.local/bin/http"]]}

@reorx
Copy link
Contributor Author

reorx commented Sep 28, 2018

@sciyoshi hmm, I find that too, thanks for reporting, to make pipsi list work temporarily, you can change line 607 to click.echo(' {}'.format(script)). I'm digging further into this problem to see what causes scripts to be a list in list.

change link_script method return to be list of dest path, this makes
less confusion, and won't affect script when calling upgrade command
sort paths before removing so that files could be removed prior to dirs
@reorx
Copy link
Contributor Author

reorx commented Sep 28, 2018

@sciyoshi the new push should fix that issue, please check it out. For the infected package_info.json files, please manually fix them by removing the redundant [ and ] ([["/Users/sciyoshi/.local/venvs/httpie/bin/http", "/Users/sciyoshi/.local/bin/http"]] should become ["/Users/sciyoshi/.local/venvs/httpie/bin/http", "/Users/sciyoshi/.local/bin/http"]).

It was a bug in upgrade command that caused the json file to be wrong.

@sciyoshi sciyoshi mentioned this pull request Sep 28, 2018
@sciyoshi
Copy link

sciyoshi commented Sep 28, 2018

  File "/Users/sciyoshi/.local/bin/pipsi", line 11, in <module>
    load_entry_point('pipsi', 'console_scripts', 'pipsi')()
  File "/Users/sciyoshi/.local/venvs/pipsi/lib/python3.6/site-packages/click/core.py", line 722, in __call__
    return self.main(*args, **kwargs)
  File "/Users/sciyoshi/.local/venvs/pipsi/lib/python3.6/site-packages/click/core.py", line 697, in main
    rv = self.invoke(ctx)
  File "/Users/sciyoshi/.local/venvs/pipsi/lib/python3.6/site-packages/click/core.py", line 1066, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/sciyoshi/.local/venvs/pipsi/lib/python3.6/site-packages/click/core.py", line 895, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/sciyoshi/.local/venvs/pipsi/lib/python3.6/site-packages/click/core.py", line 535, in invoke
    return callback(*args, **kwargs)
  File "/Users/sciyoshi/.local/venvs/pipsi/lib/python3.6/site-packages/click/decorators.py", line 27, in new_func
    return f(get_current_context().obj, *args, **kwargs)
  File "/Users/sciyoshi/.local/venvs/pipsi/src/pipsi/pipsi/__init__.py", line 570, in upgrade
    if repo.upgrade(package, editable):
  File "/Users/sciyoshi/.local/venvs/pipsi/src/pipsi/pipsi/__init__.py", line 466, in upgrade
    old_scripts = set(info.scripts)
TypeError: unhashable type: 'list'

I also got this issue when upgrading, which I was able to fix by removing and reinstalling.

@reorx
Copy link
Contributor Author

reorx commented Oct 2, 2018

@sciyoshi could you try upgrading using the the updated version of this PR (ensure scripts in package_info.json is not list of list first, see my previous comment)? The latest two commits should have fixed the upgrading issue.

@aiguofer
Copy link

I was having issues upgrading pipsi installed packages (using master). I can confirm that this branch fixes those issues.

@cs01
Copy link
Contributor

cs01 commented Oct 21, 2018

Just wanted to note that I have started pipx which combines the functionality of pipsi and npx. I have implemented several of the features requested in pipsi, including upgrading all packages with one command (pipx upgrade-all) and reinstalling all packages w/ a different Python executable with one command (pipx reinstall-all PYTHON). pipx works with python3.6+ only, and uses venvs only.

I am actively responding to issues and PRs. Would love to hear what you think -- https://github.com/cs01/pipx.

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.

6 participants