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

Outdated documentation about adding a remote git repo in stack.yaml #4425

Closed
Lysxia opened this issue Nov 30, 2018 · 10 comments
Closed

Outdated documentation about adding a remote git repo in stack.yaml #4425

Lysxia opened this issue Nov 30, 2018 · 10 comments

Comments

@Lysxia
Copy link
Contributor

Lysxia commented Nov 30, 2018

(Version 1.9.1.1, Git revision a953002 (6170 commits) x86_64)

The manual gives two different ways of adding a dependency via git (that one can find in a search using the keyword git):

I couldn't get the first one to work (even though it's the first result). The second is fine.

Steps to reproduce

  1. Take any stack project (or create one).
  2. Add some package foo that can be found on a remote git repo as a dependency to the package
  3. Use either of the above two methods to add foo to stack.yaml
  4. stack build: the first method results in an error, the second doesn't.

So one of these seems outdated. It's odd that the error is not about invalid syntax, but stack simply seems to ignore that location: field under packages:.

@mihaimaruseac
Copy link
Contributor

Checking whether this is a regression or just outdated documentation

@dbaynard
Copy link
Contributor

It’s definitely a deprecation — see https://www.fpcomplete.com/blog/2017/07/stacks-new-extensible-snapshots#four-package-locations (implemented in #3249).

…packages still supports local file paths, Git repos, and HTTP(S) URLs, but for the latter two requires you to explicitly state extra-dep: as either true or false.

Then further down

  • Everyone could transition from their current extra-dep: true syntax over the next few versions before we remove the old support
    I'm not normally in favor of breaking backwards compatibility in Stack, but miscategorized extra deps has resulted in much confusion, so I'd be happy to see it go, even if it requires rewriting stack.yaml files over time.

So

  1. @Lysxia does it work with an explicit extra-dep statement?
  2. @snoyberg it wasn't clear from a brief glance at the changelog when/whether the old syntax has been deprecated. It appears from this issue that is has — is that correct?

Whatever happens, we need to change the docs. Depending on 1 we either update to reflect the requirement for an explicit extra-dep or just indicate that this is old behaviour.

It would be good to ensure that stack recognises the old stack.yaml format — and possibly even correct the error. @Lysxia What error do you actually see?

@Lysxia
Copy link
Contributor Author

Lysxia commented Dec 10, 2018

Indeed it works with an extra-dep key. Without it here's a sample error:

Error: While constructing the build plan, the following exceptions were encountered:

Unknown package: generic-recursion-schemes

Some different approaches to resolving this:

  * Consider trying 'stack solver', which uses the cabal-install solver to attempt to find some working build configuration. This can be convenient when dealing with many complicated constraint
    errors, but results may be unpredictable.


Plan construction failed.

That is if the package is not on Hackage. If it is, you get either the usual error if it is not in your resolver, or stack installs whatever it finds in your resolver.

A big source of confusion for me was the fact that stack seems to just ignore that field (without the extra-dep key). I would expect at least a warning about it.

@dbaynard
Copy link
Contributor

OK, so two things are needed to resolve the issue fully

  1. Correct the docs to indicate that using packages without extra-dep is deprecated, that using packages for dependency is old behaviour, and recommending the correct formulation.
  2. Parse the old syntax and error with how to resolve the issue.

@Lysxia Would you like to tackle one, or perhaps both (please no animal names for functions)?

@Lysxia
Copy link
Contributor Author

Lysxia commented Dec 10, 2018

Sadly I don't have time to work on this in the near future, although I would have liked to. But thanks for the offer. ;)

@snoyberg
Copy link
Contributor

Sorry, I didn't notice this issue before. I wasn't even aware of the first doc. I'm going to send a PR to have it point to the YAML configuration doc instead.

mattaudesse added a commit that referenced this issue Mar 26, 2019
Remove incorrect Git repo documentation (fixes #4425)
@mattaudesse
Copy link
Member

Resolved in #4656 - thanks @snoyberg for fixing and @Lysxia for reporting.

@mattaudesse
Copy link
Member

Oops, sorry for jumping the gun - I see we still want to support the deprecated format; reopening.

@mattaudesse mattaudesse reopened this Mar 26, 2019
@snoyberg
Copy link
Contributor

I don't think we want to do that. We had a deprecation cycle and then removed support for the old format. The next release of Stack (2.0) is taking that even further with implicit snapshots.

@Lysxia
Copy link
Contributor Author

Lysxia commented Mar 27, 2019

Thanks @snoyberg ! I agree that was probably the last trace of the old format.

@Lysxia Lysxia closed this as completed Mar 27, 2019
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

5 participants