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

Modify "recommended packages" output to include revision info #3925

Closed
snoyberg opened this issue Mar 15, 2018 · 6 comments
Closed

Modify "recommended packages" output to include revision info #3925

snoyberg opened this issue Mar 15, 2018 · 6 comments

Comments

@snoyberg
Copy link
Contributor

Currently, when you run stack build with an impossible build plan, it may give you advice like:

  * Recommended action: try adding the following to your extra-deps
    in /Users/michael/Documents/markdown/stack.yaml:

- blaze-html-0.9.0.1
- blaze-markup-0.8.2.0
- conduit-extra-1.3.0
- xml-conduit-1.8.0
- xss-sanitize-0.3.5.7

It would be better to include explicit revision information, so that we are recommending people use reproducible build plans.

@StevenXL
Copy link

@snoyberg I'd like to take this one on please. I'll be frank. I have not looked at the code yet, but the newcomer friendly hopefully means I'm not biting off more than I can chew.

@snoyberg
Copy link
Contributor Author

snoyberg commented Mar 28, 2018 via email

@StevenXL
Copy link

Great. I'll have a nice chunk of time to work on this tomorrow (Friday).

@StevenXL
Copy link

@snoyberg I took a look at the code, and I think that the changes that I have to make include modifying the pprintExceptions function.

Part of the arguments to pprintExceptions is [ConstructPlanException]. ConstructPlanException is a sum type with three data constructors - DependencyCycleDetected, DependencyPlanFailures, and UnknownPackage.

For this PR, we only care about DependencyPlanFailures, but none of the data constructors contain more information other than PackageName and Version, which we can't use to get a revision.

So that's my understanding of the current state of the code. What I think I need to do is someone get some PackageIdentifierRevision values into pprintExceptions. (Note: I was hoping that the ParentMap argument to pprintExceptions would be helfpul, since I can get from PackageName -> PackageIdentifier, but PackageIdentifier simply contains PackageName and Version, so we're back where we started).

I'm not sure where to get values of PackageIdentifierRevision from. My first inclination is for pprintExceptions to have an another argument - SourceMap, which we'd pass from the constructPlan function (which must be called with a SourceMap and is the only function to invoke pprintExceptions). The SourceMapis a mapping from PackageName to PackageSource and the PSIndex data constructor for PackageSource contains a PackageIdentifierRevision. Then, in the pprintExceptions function we'd try to lookup the revision information in the sourcemap, and if it is there we'd use it, if not we'd keep the packagename-version behavior.

Does this approach seem like a viable first-pass iteration that we'd be able to improve upon in code review?

@snoyberg
Copy link
Contributor Author

snoyberg commented Apr 3, 2018

Given that the packages don't currently exist in the build plan, they won't exist in the SourceMap either. My guess is that you could try simply adding the revision information to the DependencyPlanFailures data itself, and see if the location where that's populated already has revision info available.

@tdietert
Copy link
Contributor

tdietert commented Jun 6, 2018

Working on it 👍

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

4 participants