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

confirming dependencies when uninstall package #4245

Closed
wants to merge 23 commits into from

Conversation

aodag
Copy link
Contributor

@aodag aodag commented Jan 21, 2017

when i uninstall package, i want to know packages depends to that.
this PR adds checking packages depends to uninstalling, and confirming.

  • make confirm_dependencies to helper function
  • remove uninstalled package from installed_packages
  • change algorithm to detecting dependencies
  • add functional tests for various situations

Copy link
Member

@xavfernandez xavfernandez left a comment

Choose a reason for hiding this comment

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

We'll need a few tests for this

[dist for d in dist.requires() if d.key == self.req.name])
if not dep_keys:
return True
print("%s is depended from:" % self.req)
Copy link
Member

Choose a reason for hiding this comment

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

You should be using logger.info to provide feedback to the user.

return True
logger.info("%s is depended from:" % self.req)
for dep in dep_keys:
print(dep)
Copy link
Member

Choose a reason for hiding this comment

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

erm, same comment for this print :o

@xavfernandez
Copy link
Member

A rebase on master and a few tests would also be needed 👍

@aodag aodag force-pushed the feature-uninstall-deps branch from cec6f4e to 32f6645 Compare February 12, 2017 10:39
@dstufft dstufft added the needs rebase or merge PR has conflicts with current master label Apr 1, 2017
@aodag
Copy link
Contributor Author

aodag commented Apr 8, 2017

this PR is affected from 893f0b0.

@aodag aodag changed the title confirming dependencies when uninstall package WIP: confirming dependencies when uninstall package Apr 8, 2017
@xavfernandez
Copy link
Member

xavfernandez commented Apr 8, 2017

Yes, we are trying to simplify the RequirementSet and RequirementInstall object.
confirm_dependencies could also be moved as an helper to pip/commands/uninstall.py

@xavfernandez
Copy link
Member

Please also note that we don't use CHANGES.txt anymore for the changelog, cf https://pip.pypa.io/en/latest/development/#adding-a-news-entry

@aodag aodag force-pushed the feature-uninstall-deps branch from 7b48f3e to 66ca197 Compare April 8, 2017 19:06
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Apr 8, 2017
@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label May 19, 2017
@aodag aodag force-pushed the feature-uninstall-deps branch from aa54594 to 17bc4b7 Compare July 14, 2017 06:35
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Jul 14, 2017
@pradyunsg pradyunsg added the type: enhancement Improvements to functionality label Jul 14, 2017
@aodag
Copy link
Contributor Author

aodag commented Jul 14, 2017

@xavfernandez please review this PR!

@aodag aodag changed the title WIP: confirming dependencies when uninstall package confirming dependencies when uninstall package Jul 23, 2017
@zaazbb
Copy link

zaazbb commented Aug 20, 2017

need confirming dependencies when upgrade package also.
see #4681

@pradyunsg pradyunsg dismissed xavfernandez’s stale review August 20, 2017 08:15

There has been a test added.

@aodag
Copy link
Contributor Author

aodag commented Aug 20, 2017

@zaazbb OK, I'm sure to need confirming on upgrading packaging too.
I'll consider this PR can include that feature.

@pradyunsg
Copy link
Member

pradyunsg commented Aug 20, 2017

I'd suggest you make another PR for that -- that PR would need discussion. :)

@aodag
Copy link
Contributor Author

aodag commented Aug 21, 2017

@pradyunsg i think so.
yes, this PR is completed.

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will eligible for code review and hopefully merging!

@pradyunsg pradyunsg added type: feature request Request for a new feature and removed type: enhancement Improvements to functionality labels Oct 24, 2017
@pradyunsg
Copy link
Member

Pinging @aodag! :)

@pradyunsg
Copy link
Member

Linking to #4001

@aodag aodag changed the title confirming dependencies when uninstall package WIP: confirming dependencies when uninstall package Oct 26, 2017
@aodag aodag changed the title WIP: confirming dependencies when uninstall package confirming dependencies when uninstall package Dec 9, 2017
@aodag
Copy link
Contributor Author

aodag commented Dec 9, 2017

@pfmoore i rewrote to check once for everything and added tests for some situations.

@aodag
Copy link
Contributor Author

aodag commented Dec 13, 2017

travis-ci terminated the test for pypy3.

The job exceeded the maximum time limit for jobs, and has been terminated.

@aodag
Copy link
Contributor Author

aodag commented Feb 15, 2018

What else can I do?

@aodag
Copy link
Contributor Author

aodag commented Mar 8, 2018

@pfmoore what should i do somthing?

@pradyunsg
Copy link
Member

Hey @aodag!

Sorry for the lack of a response lately. My suggestion would be to wait for a bit for #5000 and then, once it's merged, reuse relavent parts of it to run a check, once, before the installation.

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label May 23, 2018
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Jun 9, 2018
@aodag
Copy link
Contributor Author

aodag commented Jun 9, 2018

BrownTruck, you are my best friend.

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Jul 27, 2018
@pradyunsg pradyunsg closed this May 23, 2019
@pradyunsg
Copy link
Member

Closing this since it's bitrotten. Please file a new PR if there's still interest in this.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 22, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation needs rebase or merge PR has conflicts with current master type: feature request Request for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants