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

uninstall files in chunks of 500. #2263

Merged
merged 1 commit into from
Jan 10, 2014

Conversation

rolandwalker
Copy link
Contributor

This is intended to address #2122.

I saved a stat by not calling file.exist?, instead passing the -f flag to rm. I would argue that is (strongly) preferable.

I changed the .run! method to .run (allowing rm to return an error code) purely to make the tests pass. Here, I think this is tolerable but not preferable. We should change it back after ironing out tests.

The test glitch occurs in 'snags permissions on ornery dirs, but returns them afterwords'. In the past, ima_installed_file was never getting deleted. It appears that because the dir above was unreadable, ima_installed_file failed the .exist? test, and was silently skipped. Now that the .exist? is gone, rm is actually attempted (and fails).

Of course, tests are still clean with .run! if the test suite is run under sudo. However, the correct thing should be to update the "snags" test. @phinze, I wasn't entirely sure what you were getting at in "snags", so I didn't alter it.

This is intended to address Homebrew#2122.
@phinze
Copy link
Contributor

phinze commented Jan 10, 2014

Ooo good call!

Yeah the tests around pkg uninstall are super stubby since I couldn't come up with a realistic way of making an isolated test. The "best" way would probably for us to create some pkgs with different features to drop in test/support/binaries - but I haven't yet had the time to go down that path.

This looks like a step in the right direction for now. Merging. 🚜

phinze added a commit that referenced this pull request Jan 10, 2014
@phinze phinze merged commit fead909 into Homebrew:master Jan 10, 2014
@rolandwalker rolandwalker deleted the faster_uninstall branch February 8, 2014 21:49
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants