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

#3812 Remove system requirements from cache #4354

Merged
merged 12 commits into from Jan 25, 2019

Conversation

Projects
None yet
5 participants
@uilianries
Copy link
Member

commented Jan 21, 2019

Related Issue: #3812
Changelog: Feature: Remove system requirements conan folders (not installed binaries) from cache

Docs: conan-io/docs#1038

@PYVERS: Macos@py27, Windows@py36, Linux@py27, py34

closes #3812

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

uilianries added some commits Jan 21, 2019

#3812 Remove cached system_reqs
- Add option to remove system_reqs from package cache.

Signed-off-by: Uilian Ries <uilianries@gmail.com>
#3812 Add tests for remove --system_reqs
- conan remove --system_reqs should remove global requirement folder.
- Should we support by package id?

Signed-off-by: Uilian Ries <uilianries@gmail.com>
#3812 remove logger from cache
Signed-off-by: Uilian Ries <uilianries@gmail.com>

@ghost ghost assigned uilianries Jan 21, 2019

@ghost ghost added the stage: review label Jan 21, 2019

#3812 Update system_reqs description
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Show resolved Hide resolved conans/client/command.py Outdated
Show resolved Hide resolved conans/client/command.py Outdated
#3812 --system-reqs can raise error
- Conan remove --system-reqs forwards the exception message
  when an error occurs.
- Re-order remove command arguments
- Add test to check system-reqs error message

Signed-off-by: Uilian Ries <uilianries@gmail.com>

@danimtb danimtb added this to the 1.12 milestone Jan 22, 2019

@danimtb danimtb assigned jgsogo and unassigned uilianries Jan 22, 2019

@uilianries

This comment has been minimized.

Copy link
Member Author

commented Jan 22, 2019

Maybe we could support by package id too:

conan remove --system-reqs foo/0.1@user/channel:<package_id>

Is foo/0.1@user/channel:<package_id> supported by Conan?

@danimtb

This comment has been minimized.

Copy link
Member

commented Jan 22, 2019

It is not supported and I am not sure why we would need that. Maybe because of the global_system_requirements=True?

@SSE4

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2019

@uilianries also, it's not clear from tests, does it support pattern matching like regular remove (conan remove boost* -f)?

@uilianries

This comment has been minimized.

Copy link
Member Author

commented Jan 22, 2019

@danimtb that's because the command works globally, so if you want to remove only one lock per package id it won't be possible. A possible scenario that I think is when you have multiple docker containers running, but you have the same /conan/data shared for all containers. So if want to kill a container and start it again, you will need to remove the system_reqs.txt for the specific package id, otherwise it will affect all other containers.

@SSE4 No, it only supports full package reference. But I think you are right, otherwise we are breaking the command interface. Lemme update it.

@danimtb

This comment has been minimized.

Copy link
Member

commented Jan 22, 2019

@uilianries ok, now I get it. I guess that you can implement that with conan remove <ref> -p <id> --system-reqs

@jgsogo

jgsogo approved these changes Jan 22, 2019

Copy link
Member

left a comment

Two open questions:

  • if there will be patter matching
  • if there will be per package-id removal
Show resolved Hide resolved conans/client/command.py
Show resolved Hide resolved conans/test/integration/system_reqs_test.py Outdated
#3812 Remove system-reqs by package id
- Add support to parse package reference when `remove --system-reqs -p
 <ref>` is executed and remove ONLY the cache for the specific
  package id.

Signed-off-by: Uilian Ries <uilianries@gmail.com>

@ghost ghost assigned uilianries Jan 22, 2019

uilianries added some commits Jan 22, 2019

#3812 Ignore errors when remove system-reqs
- Package ID is NOT supported for system-reqs
- Ignore error, like permission or path not found

Signed-off-by: Uilian Ries <uilianries@gmail.com>
#3812 Remove system-reqs can raise errors
- Add exception for permission denied and wrong pkg ref
- Add new test to check possible errors in system-reqs

Signed-off-by: Uilian Ries <uilianries@gmail.com>
#3812 Fix test for Windows platform
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Show resolved Hide resolved conans/client/cache.py Outdated

uilianries added some commits Jan 24, 2019

#3812 Replace rmtree by rmdir
Signed-off-by: Uilian Ries <uilianries@gmail.com>
@lasote

lasote approved these changes Jan 25, 2019

@lasote lasote merged commit 2311d1b into conan-io:develop Jan 25, 2019

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details

@ghost ghost removed the stage: review label Jan 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.