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.

Sign up for GitHub

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

Jump to bottom

Start using the pantry package #4254

Merged
merged 249 commits into from
Aug 26, 2018
Merged

Start using the pantry package #4254

merged 249 commits into from
Aug 26, 2018

Conversation

snoyberg
Copy link
Contributor

This is a massive diff. Any reviews people have are most welcome. The basic idea is:

  • All code related to downloading packages, unpacking packages, parsing cabal files, and interacting with Hackage have been moved into a new package, pantry
  • This new package currently resides inside the Stack repo, but will be moved out in the future. (Same with the curator package present as well.)
  • The rest of the codebase has to adapt :)

For those looking to review, I'd recommend:

  • Don't bother looking at individual commits, too much noise
  • Check out the changes inside the doc directory and ChangeLog.md first
  • Review the code in subs/pantry next
  • As much as you'd like, review the changes in src

snoyberg added 30 commits July 16, 2018 16:17
@snoyberg
Begin the move to Pantry
e3b4cf8 
This will be a new library for storing package information. This first
bit overhauls the Hackage index update code, and stores information in a
SQLite database instead of the old caches. This turns out to be
significantly faster for `stack update` calls.

Fixes #3586

Note that it would be nicer to just resume the caching from where we'd
last left off, or to parse the revision numbers from the cabal files
themselves. See the discussion in haskell/hackage-server#779 to see why
that isn't possible.
@snoyberg
Move more types into the pantry subdir
5379771
@snoyberg
Use Cabal types in pantry
7050741
@snoyberg
Implement getPackageVersions
0527a63
@snoyberg
loadFromIndex
79b6913
@snoyberg
Remove unneeded updateAllIndices wrapper
23972ae
@snoyberg
Remove need for resolvePackagesAllowMissing
bf6e2c5
@snoyberg
Silence overwhelming SQL logging
67cb34d
@snoyberg
Remove unpackPackageIdents
acd0213
@snoyberg
Drop need for resolvePackages
d7beab7
@snoyberg
Implement the unpack command
4d3c0fa
@snoyberg
Update changelog and close #4137
7aecad8
@snoyberg
Do an efficient incremental cache update
33ef253 
Thanks to @phadej for the inspiration for this in his comment:
haskell/hackage-server#779 (comment)
@snoyberg
More accurate log message
3cfea9a
@snoyberg
Better retention of cabal file size
26dd4c3
@snoyberg
Calculate the Hackage tarball metadata
aa9b6db
@snoyberg
Move around the duplicate update logic
d427696
@snoyberg
Start adding the Tree data types
b8ce1e0
@snoyberg
Fix the 1024 null bytes, and clean up docs/naming
f1a3a97
@snoyberg
Make it shallower
f2990ea
@snoyberg
Prepare for archive code
f80a284
@snoyberg
Bookkeeping around Hackage trees (revisions, ugh)
8c9d41e
@snoyberg
Loading trees from Hackage works
65e0ede
@snoyberg
Unpacking a tree works!
8ccbf05
@snoyberg
Archive caching
c438a10
@snoyberg
Start refactoring the PackageLocation stuff
742a822
@snoyberg
Remove the CabalLoader concept
9d50562
@snoyberg
Start moving PackageLocation into pantry, lots of stuff is b0rken
f276dee
@snoyberg
Move over to Cabal types
55ac92b
@snoyberg
RawPackageLocation
db366f4
snoyberg and others added 12 commits August 23, 2018 06:50
@snoyberg
Attempt to use PRAGMA busy_timeout
aa670e6 
Ideally, fixes #4247. Instead of immediately failing on the database
being busy (via usage by another process), Stack will now have a 2
second pause and then try again. The vast majority of interactions with
the database are much faster than 2 seconds. The only exception is the
population of the Hackage index cache, but that's hopefully rare enough,
and touching few enough tables, to not be a problem.
@snoyberg
More reliable persistent-sqlite extra-dep
7e38e09
@snoyberg
Ugly, temporary hack for store build issues
fb6f819 
Use a hacked version of store which hopefully requires much less memory.
@qrilka
Migrate some remains of code to use Cabal types/functions
3211dd5
@snoyberg
Check in pantry.cabal
2b846ff 
Pantry doesn't support archives and repos without cabal files. I'm
generally opposed to checking in generated files, but it may be the best
choice here. The alternative would end up with some crazy hacks to
support the non-deterministic nature of generating cabal files from
package.yaml files.
@snoyberg
Support symlinks to parent dir
784535e
@qrilka
Move parsing functions into pantry
7cfef74
@qrilka
Remove unused pragmas
ba15062
@snoyberg
Add lower bound on hpack
0e1e10e
@snoyberg
Normalize safely across Windows and Unix
a52bacb
@snoyberg
Fix warnings
0621362
@qrilka
parsePackageNameFromFilePath out of pantry, new pantryexceptions
c8b2031
@dbaynard
Copy link
Contributor

Thanks, the changes look good. I'm grateful for the explanation of the cache-breaking behaviour.

@dbaynard dbaynard mentioned this pull request Aug 23, 2018
Closed
@snoyberg
Copy link
Contributor Author

Thanks, the changes look good. I'm grateful for the explanation of the cache-breaking behaviour.

I'm very grateful for the great review, thanks!

mihaimaruseac
mihaimaruseac reviewed Aug 23, 2018
View reviewed changes
Copy link
Contributor

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

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

Skimmed over much of src/ changes, they look good to me but I'm still not 100% up to date with the entire codebase

doc/yaml_configuration.md Outdated

Since 1.6.0
This flag was introduced in Stack 1.6, and removed in Stack 1.11 with
hte move to Pantry. You will receive a warning if this configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

the instead of hte

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks!

@mihaimaruseac
Copy link
Contributor

There's an argument to wait on merging this into master until after I've implemented #3922 to avoid breaking caches twice. However, my aversion to this is because:..

I'd say we should merge this first, it's already quite large.

@snoyberg
Copy link
Contributor Author

Integration tests are passing, so this is ready to merge. There are some merge conflicts with master, which I'm resolving. Once that's done and CI is green, anyone should feel free to merge this.

@snoyberg snoyberg merged commit 99ecb78 into master Aug 26, 2018
@snoyberg snoyberg deleted the pantry branch August 26, 2018 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbaynard dbaynard dbaynard left review comments

@mihaimaruseac mihaimaruseac mihaimaruseac left review comments

Assignees
No one assigned
Labels
awaiting review component: pantry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@snoyberg @dbaynard @mihaimaruseac @qrilka @borsboom @bergmark