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

Implicit snapshots (aka: Unify extra-deps and extensible snapshots) #1265

Closed
borsboom opened this issue Nov 1, 2015 · 15 comments
Closed

Implicit snapshots (aka: Unify extra-deps and extensible snapshots) #1265

borsboom opened this issue Nov 1, 2015 · 15 comments
Assignees
Milestone

Comments

@borsboom
Copy link
Contributor

borsboom commented Nov 1, 2015

There's a lot of overlap between extra-deps and extensible snapshots. Both give you a to add some "extra stuff" to a snapshot. The main difference is that extra-deps are installed into <project>/.stack-work and treated as slightly weird local packages, while extensible snapshots will go into ~/.stack/snapshots and are immutable. There's a fair amount of confusion around extra-deps (and packages that have extra-dep: true) , so instead of having two ways to do similar things, they should be unified into a single coherent approach.

Particular issues currently:

  • Most people don't want extra-deps to behave like local packages, especially as regards GHC options
  • extra-deps cannot be shared between projects (or copies of the same project)
  • The current stack.yaml configuration is confusing

Now that snapshots can share packages between them, there's no longer a good reason why extra-deps have to be local to the project. Instead, they could form an implicit extended snapshot.

So here, after discussing with @snoyberg, is my proposal for steps to take:

  1. Ensure that extensible snapshots (Extensible snapshots #863) can support everything required to support extra-deps. That would include:

    • Setting custom ghc-options for packages
    • Getting package source from a remote Git repository or local directory [edit: local directory for extended snapshot would not be a good idea, since contents could change]
  2. Modify stack so that an implicit custom snapshot is created for extra-deps, without changing the existing stack.yaml configuration. The implicit custom snapshot's name would be generated by taking a hash the package source location, name, version, flags, and ghc-options (at least) for each package

  3. Rationalize the stack.yaml configuration. In particular, do away with the extra-deps section. All additions to the custom snapshot will be added using a flag in the packages section (similar to the existing extra-dep flag, but perhaps with a different name like extend-snapshot). The existing configuration would be deprecated but still supported for some time.

    Edit: alternatively, keep a separate section for the extended snapshot (akin to current extra-deps section), but have it accept exactly the same syntax as the packages section (but not accepting local directories). Pro: a bit more clear; Con: more work to switch packages between local and extended snapshot (you have to move stuff around in stack.yaml instead of just toggling a flag).

ping @mgsloan @zudov @bitemyapp

@3noch
Copy link
Member

3noch commented Nov 2, 2015

👍

@mgsloan
Copy link
Contributor

mgsloan commented Dec 11, 2015

For (3), I'm thinking doing the following makes sense:

  • Add a new dependencies section, which uses the same syntax as packages.
  • Deprecate use of extra-deps.
  • Deprecate use of extra-dep: True in packages list, and disallow it in the dependencies list. Hopefully this will clear up some of the confusion in Stack should be more clear about packages vs. dependencies #1217
  • Make it so that the packages and dependencies list can specify a hackage package version. Does it make sense to be able to specify a hackage package as a local package? Seems a little weird, but it may well be useful, and it seems consistent given that we allow using source control repos as local packages.

Major release 0.2 (or perhaps 1.0!) would remove the deprecated options. How hard would it be to automatically upgrade the stack.yaml? Comment preservation is tricky!

@bitemyapp
Copy link
Contributor

@mgsloan

Does it make sense to be able to specify a hackage package as a local package? Seems a little weird, but it may well be useful, and it seems consistent given that we allow using source control repos as local packages.

Errr, vendoring? If I read you correctly, this is something I've done many times, although it wasn't "local", it was a git repository with modifications divergent from an available Hackage version. I had to do this for Snap 1.0 and it was only due to Stack that I was able to manage the mess.

Deprecate use of extra-dep: True in packages list, and disallow it in the dependencies list. Hopefully this will clear up some of the confusion in #1217

That's satisfactory for what I was concerned about, although we'll want a big-bold-headline explaining package vs. dependency since the former can be read either way.

Everything else seems fine.

@mgsloan
Copy link
Contributor

mgsloan commented Dec 12, 2015

If I read you correctly, this is something I've done many times, although it wasn't "local", it was a git repository with modifications divergent from an available Hackage version

packages:
- hackage: lens-4.13

vs

packages:
- location:
    git: https://github.com/ekmett/lens.git
    commit: v4.13

The main reason to add it is consistency. I've discussed this with @borsboom and he's in favor. Probably the way it will work is that packages will treat strings as dir paths, whereas dependencies will treat strings as hackage versions. Paths won't be allowed in dependencies. In all other ways they will work the same.

@bitemyapp
Copy link
Contributor

@mgsloan

Paths won't be allowed in dependencies. In all other ways they will work the same.

What's the replacement for the "is a package, local path, has extra-dep: true" case in this design? There was a lot of that too in my projects.

@mgsloan
Copy link
Contributor

mgsloan commented Dec 12, 2015

What's the replacement for the "is a package, local path, has extra-dep: true" case in this design? There was a lot of that too in my projects.

Good question! I actually didn't realize that was possible. This capability is indeed problematic if the extra-deps are going to implicitly create a custom snapshot (as we'd need the hash to change if the package changes). Then again, this is also an issue with git repo commits that aren't SHAs.

Does stack reinstall these when you change files in them?

@bitemyapp
Copy link
Contributor

@mgsloan didn't seem to, but I'd have to test to be sure. That was why we'd set extra-deps on it.

/cc Paging colleague @k-bx

@k-bx
Copy link
Contributor

k-bx commented Dec 14, 2015

I don't actually see any extra-dep: true local package in our repo, only ones pointing to github's commit. So I guess shouldn't be an issue (and shouldn't be an issue to put it to github anyway).

@erikkaplun
Copy link
Contributor

a big +1 to this one!

@mgsloan
Copy link
Contributor

mgsloan commented Jan 3, 2016

Note that flags and ghc-options should implicitly promote packages to extra-deps, as described in #849. Currently, you need to manually add extra-deps when specifying a flag:

Invalid flag specification:
- Attempted to set flag on snapshot package time-locale-compat, please add to extra-deps

@mgsloan
Copy link
Contributor

mgsloan commented Jan 6, 2016

Note that after this, having the solver add to extra-deps due to flags will be unnecessary: harendra-kumar@86334c6

@sjakobi
Copy link
Member

sjakobi commented Aug 15, 2016

I just talked to @borsboom about this issue:

His idea was to start in the loadResolver function. A stack.yaml with custom extra-deps, flags or ghc-options etc. would be treated as an extended snapshot of the underlying resolver, and could be identified for example with a hash of the extensions (extra-deps, flags etc).
This way, for example in the case where the project is duplicated on the same machine, both copies will use the same snapshot, and any dependencies of the projects have to be compiled only once.

EDIT: He proposed to call the custom resolver that results from a stack.yaml an "implicit snapshot". I think that's a fitting name.

@borsboom borsboom changed the title Unify extra-deps and extensible snapshots Implicit snapshots (aka: Unify extra-deps and extensible snapshots) Sep 19, 2016
@borsboom borsboom modified the milestones: P1: Must, P2: Should Sep 19, 2016
@snoyberg
Copy link
Contributor

snoyberg commented Jul 5, 2017

See #3249. That PR explicitly decided not to create an implicit snapshot based on extra-deps, due to:

  1. The potential for a rather large disk space usage for each time dependencies change
  2. Lack of support for extra-deps depending on project packages in that setup

@snoyberg
Copy link
Contributor

Closing now that #3249 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants