Code cleanup following extensible snapshots

[snoyberg]

In the process of adding extensible snapshots, I've both discovered some places where the code in Stack could do with some cleanup, and introduced enough changes that I've made a brand new mess. Right now, these things are all in my head, and I've been trying to pick them off one by one, but I'd rather get it down in an issue to ensure I'm not blocking anything from proceeding.

Naming conventions

There is confusion about a number of terms in the codebase right now. For example:

• Local vs project, and local vs snapshot
• Targets and wanteds
• Extra deps, local dependencies
• The term "location" referring to both the database and where a package comes from

We should come up with a standard and consistent terminology, put it in some architecture document, and standardize data types and functions in the codebase.

FIXME Edit this and propose a new nomenclature.

PackageSource

PR #3351 fixes some egregious problems with the PackageSource datatypes. Originally: we had "upstream" packages (those from the package index/Hackage) and "local" packages. That line is blurred a lot now with extensible snapshots, and we really need to break up this datatype much better than what #3351 does. In particular, we should put much more information on how to build the relevant packages in that datatype, to avoid needing to reparse the .cabal files multiple times. As I see if, we have two dimensions on which package sources vary:

• Whether the package is in the snapshot, is a local database dependency package, or a package that is part of the project.
• What can change between various builds in order to invalidate a previous build: the files themselves, flags passed in, and components to be built

Not all combinations are possible: a snapshot package, once built, is totally immutable. Any attempt to change it will promote it to the local database (or, if we implement implicit snapshots, create a new implicit snapshot).

Once this is done, TaskType can be cleaned up as well, and likely a large number of FIXMEs asking about the difference between Local and Snap databases will automatically be resolved.

withCabalLoader

A nice to have: instead of opening and closing a bunch of file handles, open up the file handles here and stick them in an MVar. But if we avoid the reparsing of cabal files all over the place, this may be less important.

---

I reserve the right to expand this issue in the future 😄

[snoyberg]

Here's a bit of a diff from a first stab I took (and then put down):

-data PackageSource
-  = PSFiles LocalPackage InstallLocation
-  -- ^ Package which exist on the filesystem (as opposed to an index tarball)
-  | PSIndex InstallLocation (Map FlagName Bool) [Text] PackageIdentifierRevision
-  -- ^ Package which is in an index, and the files do not exist on the
-  -- filesystem yet.
-    deriving Show
-
+data PackageSource = PackageSource
+  { psDatabase :: !InstallLocation
+  , psFiles :: !PackageFiles
+  , psLPI :: !(LoadedPackageInfo (PackageLocation FilePath))
+  }
+  deriving Show
+
+data PackageFiles
+  = PFIndex !PackageIdentifierRevision
+  | PFImmutable !(Path Abs Dir)
+  | PFMutable !(Path Abs Dir) -- FIXME add in local file path info
+  deriving Show

[snoyberg]

And I'll share the stupid little whiteboarding I did a few days ago, which may not be very useful for anyone but at least lets me erase the board.

Also, everyone can now see just how bad my handwriting really is.

[snoyberg]

With the move to Pantry, most of this is already done. We can continue working on cleanups without this issue open.