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

stack unpack downloads deprecated package versions #1391

Closed
borsboom opened this issue Nov 21, 2015 · 24 comments
Closed

stack unpack downloads deprecated package versions #1391

borsboom opened this issue Nov 21, 2015 · 24 comments
Milestone

Comments

@borsboom
Copy link
Contributor

Steps to reproduce:

Run stack unpack stack

Expected:

It unpacks the latest non-deprecated version (as of this filing, stack-0.1.8.0).

Actual:

It unpacks stack-9.9.9, which is deprecated and unbuildable.

$ stack unpack stack
stack-9.9.9: download
Unpacked stack-9.9.9 to /Users/manny/fpco/tmp/stack-9.9.9/
@DNNX
Copy link
Contributor

DNNX commented Nov 22, 2015

I just scanned hackage index trying to find what other packages are affected. As it turns out, at the moment 15 out of 9069 packages have max deprecated-versions >= max non-deprecated-versions. In other words, stack unpacking any of those 15 packages would download a deprecated version:

call    0.2
fptest  0.2.2.0
hydrogen-cli-args   0.9
hydrogen-data   0.7.1
hydrogen-prelude    0.9
hydrogen-syntax 0.7.1
json-autotype   1.0.9
leksah-server   0.8.0.8
leksah  0.8.0.8
ltk 0.8.0.8
peyotls-codec   0.3.1.1
pipes-cereal    0.1.0.0
sasl    0.0.0.2
stack   9.9.9
word12  1.0.0

I hope it will be useful for anyone who decides to fix this bug (at least for testing).

I initially wanted to fix it myself, but it seems that it is not that trivial. We need to either make a request to Hackage API each time a user runs stack unpack, or enrich the package index with information about package deprecations in order to have it at our disposal at runtime. Currently package index just don't have this info. As far as I understand, package index is created by another tool (not stack), so maybe another issue should be created in that tool's Github repo as well.

@mgsloan
Copy link
Contributor

mgsloan commented Nov 22, 2015

Hmm, it's surprising to me that stack unpack would unpack the latest version rather than the snapshot version. Maybe that's a good way to fix this? Granted, that doesn't help for the packages that aren't in the snapshot / when you're using a compiler resolver.

@borsboom
Copy link
Contributor Author

Yeah, I could go either way on this. Not really clear to me whether it's preferable to use the latest version or the snapshot version, so I'd tend toward staying with the current behaviour unless there's a compelling reason to change it (and I'm not sure that this issue qualifies as compelling enough).

I just confirmed that the package index in ~/.stack/indices/Hackage/00-index.tar does include the preferred-versions file (e.g., in stack/preferred-versions), and so does the all-cabal-hashes repo ((e.g., stack's preferred-versions).

@luigy
Copy link
Contributor

luigy commented Dec 8, 2015

hmm there are a couple ways mentioned above to approach this

Z. query hackage api each time
all the information needed should already be present so I'm not to fond of this, but it is def the simplest

S Z. extend package-cache map to contain this information
Currently a package-cache is keyed by package-indentifier, but preferred-versions applies to the whole package. Perhaps something like this? - not too familiar if this will cause significant performance regression

newtype PackageCacheMap 
      = PackageCacheMap (Map PackageName (Maybe PreferredVersions, Map Version PackageCache))

also from skimming the code looks like a change like this will rebuild cache correctly once users upgrade

and maybe have a flag to support @mgsloan comment on fetching it using the snapshot?

@tolysz
Copy link
Collaborator

tolysz commented Dec 10, 2015

Just playing with the stack --stack-yaml some/stack.yaml unpack some-package
It looks like it ignores the version specified inside the .yaml (in my case in the packages section) and it just takes some random one, possibly, latest.

@mgsloan
Copy link
Contributor

mgsloan commented Dec 11, 2015

@tolysz Good point. The case of unpacking a local package (in particular, location: git) makes me think that stack unpack should default to unpacking the version specified by the stack.yaml. Then we can add a stack unpack --latest command to get the current behavior.

I'm thinking it's fine to change the meaning of stack unpack, since I doubt it gets much use in scripts. It'd probably be good to have the command output clearly state when it's not unpacking the latest version, though, incase folks still expect that.

@tolysz
Copy link
Collaborator

tolysz commented Dec 11, 2015

Or even stack unpack --hackage ...

@mgsloan
Copy link
Contributor

mgsloan commented Dec 12, 2015

I prefer --latest over --hackage, because all the packages come from hackage.

@Ericson2314
Copy link

I prefer --latest over --hackage, because all the packages come from hackage.

Not if stack.yaml specifies otherwise.

@Ericson2314
Copy link

For ghcjs/ghcjs#442 @luigy had the excellent idea of using stack unpack as an initial step to leverage Stack in the boot process. For us to do that however, this needs to be fixed. Any pointers on where to begin?

@mgsloan
Copy link
Contributor

mgsloan commented Feb 24, 2016

Note that you can do stack unpack aeson-0.11.0.0 to get a specific version, the main things this ticket is about:

  • Automatically selecting the version based on stackage snapshot.
  • Avoiding deprecated versions when pulling down latest hackage versions.

Here's where the code for unpack lives:

-- | Intended to work for the command line command.

@Ericson2314
Copy link

@mgsloan to be clear we want to replace the submodules with a stack.yaml: while submodules work great with the forks I hope GHCJS will use, it's annoying to point to an upstream repo that may move/die/be-force-pushed/etc vs just refer to a (released) version number with stack.yaml.

Thanks for that tip on the code. I guess I need to hook that up with the "normal" fetching stuff in https://github.com/commercialhaskell/stack/blob/master/src/Stack/Config.hs

@luigy
Copy link
Contributor

luigy commented Feb 24, 2016

hehe it looked promising to use stack for a bigger part of it before looking into the sorcery inside ghcjs-boot which will require a bit more custom support from inside stack. To give in a bit more context, we wanted in the least to pin ghcjs boot packages into stackage snapshots.

In the end if unpack gets snapshot support I still think stack would be a convenient way to fetch boot packages in order to continue with the boot process, but probably too much of a dependency to incur? We were just tossing ideas to improve maintainability of ghcjs

Any pointers on where to begin?

I should have some code laying around for the second option I mentioned here

@luigy
Copy link
Contributor

luigy commented Feb 24, 2016

did we come up in agreement wether to use hackage or resolver(if snapshot) as the default?
I can take this one if @Ericson2314 didn't beat me to it

@Ericson2314
Copy link

I'm operating on the assumption that the package version stack would use is the one it should unpack. so whatever the version is in resolver U extra-deps U local-packages (rightwards arguments taking precedence in map union).

@Ericson2314
Copy link

Since stack.yaml doesn't make one list which cabal files are in which locations, I think the code I listed does some searching. To be fair I'm fine with any sort of default, but I hope everyone would agree that there should be at least an option to unpack the version stack would build in practice.

@Ericson2314
Copy link

@luigy if you want to take it it would be very much appreciated. All this GHCJS is some serious yack-shaving on my capstone project for school I'm worried I should soon be getting back too. :)

@mgsloan
Copy link
Contributor

mgsloan commented Feb 24, 2016

I'd be open to adding some code to stack for the purpose of supporting ghcjs-boot's usecase, ideally remaining as generic as possible.

luigy added a commit to luigy/stack that referenced this issue Feb 25, 2016
* fetches version present in snapshot otherwise falls back to hackage
* caches `preferred-versions` from index
* only versions within `preferred-versions` are fetched unless explicitly asked by package identifier
* added --latest flag to fetch latest version from hackage regardless of resolver
@rrnewton
Copy link
Contributor

(CC @RyanGlScott, @vollmerm)
We just came across the stack unpack behavior and were surprised that it was ignoring our stack.yaml.

Likewise, we expected to be able to pass --resolver=lts-5.13 to force it, as most of the stack commands we use respect --resolver. But that wasn't an option.

Reading this issue it sounds like there's some consensus around giving stack the ability to unpack stack-dictated package versions.

@mgsloan
Copy link
Contributor

mgsloan commented Apr 18, 2016

Agreed that we really should make this change. Hopefully we can get #1858 finished up for this release.

@mgsloan mgsloan added this to the P2: Should milestone Apr 18, 2016
@mgsloan mgsloan removed this from the P3: Optional milestone Apr 18, 2016
@tolysz
Copy link
Collaborator

tolysz commented Aug 28, 2016

Ideally, we would need to get the latest revision of .cabal for a given version.

@mgsloan
Copy link
Contributor

mgsloan commented Sep 26, 2016

We don't want to use the latest revisions, because that can make stack builds non reproducible.

@mgsloan
Copy link
Contributor

mgsloan commented Oct 24, 2016

NOTE: Existing work

The PR for this has languished for a long time, so I've closed it - https://github.com/commercialhaskell/stack/pull/1839/files . Future efforts should use this as reference, as a lot of work already got done by @luigy here.

One thing to note that that PR doesn't do is to also consider extra-deps. Hopefully with the implementation of #1265 there will be a straightforward way to implement a query of "What is the package version specified by a given stack.yaml configuration"

@snoyberg
Copy link
Contributor

This has already been fixed in Stack, closing.

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