Make stack clean be more thorough by default #4385

vanceism7 · 2018-11-02T19:55:50Z

Fixes #3863 and potentially fixes #2278

Please include the following checklist in your PR:

Any changes that could be relevant to users have been recorded in the ChangeLog.md
The documentation has been updated, if necessary.

I tested out stack clean and stack purge. It seems like theyre doing what they should be. Also added a small spot in the guide.md explaining the commands a little more indepth.

dbaynard

Thanks for the PR! I like the approach you've taken.

In my comment on the issue I asked whether my example description was accurate, but never received an answer. I shall ask again, now that you have submitted this PR.

Specifically, I'll get another person to review this.

doc/GUIDE.md

dbaynard · 2018-11-07T16:14:56Z

doc/GUIDE.md

+### `stack purge`
+`stack purge` Deletes the local stack working directory, including extra-deps, git dependencies and the compiler output. 
+It does not delete any snapshot packages, compilers or installed programs. -- This essentially
+reverts your project to a completely fresh state, as if it had never been built. 


This last bit could be more precise. It isn't obvious what a 'completely fresh state' is. I'd have to study that part of the code to offer something that could be written here instead.

I suspect the list I wrote in my comment on the issue may not be comprehensive, so I'll get somebody else to check.

dbaynard · 2018-11-07T16:15:44Z

src/Stack/Clean.hs

@@ -54,12 +55,17 @@ dirsToDelete cleanOpts = do
 -- | Options for @stack clean@.
 data CleanOpts
    = CleanShallow [PackageName]
-    -- ^ Delete the "dist directories" as defined in 'Stack.Constants.distRelativeDir'
+    -- ^ Delete the "dist directories" as defined in 'Stack.Constants.Config.distRelativeDir'


dbaynard · 2018-11-07T16:18:25Z

src/Stack/Options/CleanParser.hs

@@ -19,4 +19,6 @@ cleanOptsParser = CleanShallow <$> packages <|> doFullClean
        flag'
            CleanFull
            (long "full" <>
-             help "Delete all work directories (.stack-work by default) in the project")
+             help "Delete the local stack working directory (.stack-work by default).")


Ah — this is a subtle change in meaning (probably my fault). It is possible for a project to have multiple .stack-work directories. The old description indicates all are deleted; this implies only one is deleted.

Oh my bad, I wasn't aware that you could have multiple .stack-work directories. Ill fix that

I'm not sure how stack actually behaves, mind.

Oh ok, perhaps I'll wait until this is reviewed further before I run off changing things haha

dbaynard · 2018-11-07T16:24:32Z

Also to fix #2278 the documentation changes would need merging to stable not master — though I'd prefer to see the whole PR merged in to stable for the next minor release, if the API addition is acceptable.

doc/GUIDE.md

dbaynard · 2018-11-14T16:23:23Z

It would be good to get an integration test, for this, too, but don't worry too much about it, @vanceism7, if you don't have a chance — it should be fairly straightforward.

dbaynard · 2018-11-14T17:00:47Z

Also I have a directory tree diff listing what has been deleted by each of these commands, which I still need to check. It's in progress! Thanks for bearing with me 👍

vanceism7 · 2018-11-14T21:44:37Z

Oh no problem, thanks for taking the time to address this issue @dbaynard. Id be happy to put the integration test in, but I'll be a bit busy for the next week or so. If there's no rush, I'll try to get to that soon, otherwise, I'm fine if someone else finishes that up too.

Thanks!

vanceism7 · 2018-11-28T23:04:58Z

Hey just posting an update, I'm back and finished with my prior obligations so I'll work on getting those integration tests in

dbaynard · 2019-02-10T16:40:30Z

We've had various issues with stack clean — I'll take a look at this shortly and see whether this can be merged. Thanks for your patience, @vanceism7!

Co-Authored-By: vanceism7 <[email protected]>

dbaynard · 2019-02-10T21:50:38Z

I've rebased, and fixed a conflict (I thought there might be more). Given this doesn't actually affect the operation of clean (just the interface) if it passes CI & integration tests are ok then I'll merge this ASAP.

- The integration test ensures the correct directories are present, then deleted, for a project with only one package, not in a subdirectory.

dbaynard · 2019-02-10T23:10:14Z

Note: the integration test fails. The behaviour of stack clean has changed on master, at some point — see #4480.

vanceism7 · 2019-02-10T23:44:18Z

Oh ok np, I'll fix that integration test as soon as I can

dbaynard · 2019-02-11T14:07:24Z

@vanceism7 To be clear, it's not failing because of your changes! There's an issue with clean on master.

snoyberg · 2019-03-06T13:15:47Z

I've opened up #4607, which fixes the clean problems. I've pushed a new commit (1cabb98) to a separate branch, which merges this branch and the branch for #4607, together with a minor doc fix. Assuming #4607 is merged, I believe 1cabb98 can be merged into this PR.

Make stack clean be more thorough by default (supersedes #4385)

dbaynard requested changes Nov 7, 2018

View reviewed changes

dbaynard added the awaiting review label Nov 7, 2018

mattaudesse reviewed Nov 8, 2018

View reviewed changes

doc/GUIDE.md Outdated Show resolved Hide resolved

mattaudesse approved these changes Nov 11, 2018

View reviewed changes

dbaynard mentioned this pull request Nov 14, 2018

Weird "can't load .so/.DLL" error I believe because of a git repo as a dependency. #740

Closed

dbaynard mentioned this pull request Dec 15, 2018

Global --offline flag to prevent internet access #1594

Open

dbaynard mentioned this pull request Jan 7, 2019

stack clean fails due to missing ghc-pkg (on latest from master) #4480

Closed

mihaimaruseac changed the title ~~Fix #3863 - Stack clean should be more thorough by default~~ Make stack clean be more thorough by default Jan 22, 2019

vanceism7 and others added 10 commits February 10, 2019 21:46

Implemented stack purge

cb1f824

Updated comment for clean shallow. (No fix was needed)

d4d7352

Updated docs for stack clean

bac2fcf

Updated help messages for stack clean and purge

caa2b9b

fix docs wording

23d534d

Co-Authored-By: vanceism7 <[email protected]>

fixed wording

854593b

Co-Authored-By: vanceism7 <[email protected]>

Clarify differences between clean and purge

dad8f92

Improve description of --full

ccba7f2

Clarify clean and purge help text

9ccfdab

Added stack purge integration test

97f280b

Check correct directory deletion behaviour for simple project

893a8b7

- The integration test ensures the correct directories are present, then deleted, for a project with only one package, not in a subdirectory.

dbaynard force-pushed the fix-3863 branch from f2ccde6 to 893a8b7 Compare February 10, 2019 23:08

dbaynard added blocked and removed awaiting review labels Feb 10, 2019

snoyberg merged commit 893a8b7 into commercialhaskell:master Mar 8, 2019

snoyberg added a commit that referenced this pull request Mar 8, 2019

Merge pull request #4609 from commercialhaskell/vanceism7-fix-3863

77aa9c3

Make stack clean be more thorough by default (supersedes #4385)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make stack clean be more thorough by default #4385

Make stack clean be more thorough by default #4385

vanceism7 commented Nov 2, 2018 •

edited

Loading

dbaynard left a comment

dbaynard Nov 7, 2018

dbaynard Nov 7, 2018

dbaynard Nov 7, 2018

vanceism7 Nov 7, 2018

dbaynard Nov 7, 2018

vanceism7 Nov 7, 2018

dbaynard commented Nov 7, 2018

dbaynard commented Nov 14, 2018

dbaynard commented Nov 14, 2018

vanceism7 commented Nov 14, 2018

vanceism7 commented Nov 28, 2018

dbaynard commented Feb 10, 2019

dbaynard commented Feb 10, 2019

dbaynard commented Feb 10, 2019

vanceism7 commented Feb 10, 2019

dbaynard commented Feb 11, 2019

snoyberg commented Mar 6, 2019

Make stack clean be more thorough by default #4385

Make stack clean be more thorough by default #4385

Conversation

vanceism7 commented Nov 2, 2018 • edited Loading

dbaynard left a comment

Choose a reason for hiding this comment

dbaynard Nov 7, 2018

Choose a reason for hiding this comment

dbaynard Nov 7, 2018

Choose a reason for hiding this comment

dbaynard Nov 7, 2018

Choose a reason for hiding this comment

vanceism7 Nov 7, 2018

Choose a reason for hiding this comment

dbaynard Nov 7, 2018

Choose a reason for hiding this comment

vanceism7 Nov 7, 2018

Choose a reason for hiding this comment

dbaynard commented Nov 7, 2018

dbaynard commented Nov 14, 2018

dbaynard commented Nov 14, 2018

vanceism7 commented Nov 14, 2018

vanceism7 commented Nov 28, 2018

dbaynard commented Feb 10, 2019

dbaynard commented Feb 10, 2019

dbaynard commented Feb 10, 2019

vanceism7 commented Feb 10, 2019

dbaynard commented Feb 11, 2019

snoyberg commented Mar 6, 2019

vanceism7 commented Nov 2, 2018 •

edited

Loading