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

Use more compatible flag name #155

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
9 participants
@tfausak
Copy link

tfausak commented Sep 8, 2017

This flag name was changed without comment in 6e1ff78. It has caused a few problems:

Those problems either caused or accelerated some (attempted) changes in Cabal:

Those problems also caused a change in Stack:

In short: Cabal never had any trouble with this. Stack did. The current master version of Stack can handle flags like this, but no released versions of it can. It's not clear when the next version of Stack will be released. In the meantime, no Stack users can use this package. This commit changes the offending flag to something that Stack can handle. By using a flag that Stack can handle, Stack users can once again use this package, and it can return to Stackage. There are no apparent downsides to using a more compatible flag name.

Use more compatible flag name
This flag name was changed without comment in 6e1ff78. It has caused a few problems:

- commercialhaskell/stack#3345
- commercialhaskell/stackage#2755
- commercialhaskell/stackage#2759
- commercialhaskell/stackage#2842
- haskell/cabal#4686
- #150

Those problems either caused or accelerated some (attempted) changes in Cabal:

- haskell/cabal#4654
- haskell/cabal#4687
- haskell/cabal#4696

Those problems also caused a change in Stack:

- commercialhaskell/stack#3349

In short: Cabal never had any trouble with this. Stack did. The current master version of Stack can handle flags like this, but no released versions of it can. It's not clear when the next version of Stack will be released. In the meantime, no Stack users can use this package. This commit changes the offending flag to something that Stack can handle. By using a flag that Stack can handle, Stack users can once again use this package, and it can return to Stackage. There are no apparent downsides to using a more compatible flag name.
@lwm

This comment has been minimized.

Copy link

lwm commented Sep 11, 2017

@tfausak, thank you for taking some time on this one 💯

@tfausak

This comment has been minimized.

Copy link

tfausak commented Sep 30, 2017

@hvr what can I do to help move this along?

@hvr hvr added the wontfix label Sep 30, 2017

@hvr

This comment has been minimized.

Copy link
Collaborator

hvr commented Sep 30, 2017

In the meantime, no Stack users can use this package. [..] Stack users can once again use this package

I don't think that's true. Stack users can use this package just fine. LTS 9.6 has https://www.stackage.org/lts-9.6/package/cassava-0.4.5.1 wheras Nightly 2017-09-29 has https://www.stackage.org/nightly-2017-09-29/package/cassava-0.5.0.0


PSA: The bug has been fixed in Stack-1.6.1 !

Please upgrade if you are an affected Stack user


@hvr hvr closed this Sep 30, 2017

@hvr hvr added the invalid label Sep 30, 2017

@tfausak

This comment has been minimized.

Copy link

tfausak commented Sep 30, 2017

That's... deliberately missing the point. The flag only changed in version 0.5.1.0, so of course Stack users can install versions before that. Stack users cannot install the latest version of this package because the flag name is invalid for all released versions of Stack.

> stack --version
Version 1.5.1 x86_64 hpack-0.17.1

> stack --resolver lts-9.0 build cassava-0.5.1.0
Error: While constructing the build plan, the following exceptions were encountered:
In the dependencies for cassava-0.5.1.0:
    bytestring-0.10.8.1 must match >=0.9.2 && <0.10.4 (latest applicable is 0.10.2.0)
needed for unknown reason - stack invariant violated.
Plan construction failed.

> stack --resolver lts-9.0 build --flag cassava:-bytestring--LT-0_10_4 cassava-0.5.1.0
option --flag: Invalid flag name: bytestring--LT-0_10_4

Why is it important that the flag name be like this?

@hvr

This comment has been minimized.

Copy link
Collaborator

hvr commented Sep 30, 2017

Let me turn the burden of proof around, and ask why does it take so long for Stack to make a point release even though this bug in Stack has been known for over a month already, when I made it quite clear in commercialhaskell/stack#3345 (comment) that Stack's the place where this ought to be fixed. So I'd suggest you try shaming Stack dev into releasing a point release instead.

@tfausak

This comment has been minimized.

Copy link

tfausak commented Sep 30, 2017

I agree that it would be nice for Stack to cut a release that supports these flag names. However I think it's clear that it's easier to change a flag name in Cassava than it is to release a new version of Stack.

Regardless, that doesn't explain why this flag name has to be the way it is. What was wrong with the old flag (or my proposed flag)?

@tfausak

This comment has been minimized.

Copy link

tfausak commented Sep 30, 2017

For anyone trying to install cassava-0.5.1.0 with Stack 1.5.1, I released my fork on Hackage as Cassava-0.5.1.0. You can install it like so:

> stack --resolver lts-9.0 build --flag Cassava:-bytestring-LT-0_10_4 Cassava-0.5.1.0 text-short-0.1.1

Other than the flag name, it's identical to cassava-0.5.1.0.

@mgsloan

This comment has been minimized.

Copy link

mgsloan commented Oct 19, 2017

@hvr Even if stack fixes this, older versions of stack will still not work with this package. Not all distributions and users keep up to date.

EDIT: NVM, it already got fixed commercialhaskell/stack#3345 , just not yet released. Here's what I said here, now ignore:

I'm not certain we will "fix" this at all. IMHO there should be a single set of rules for package names and flag names. Less to remember. So, IMHO, this is an oversight in cabal's spec and implementation. In order to make it so that most current build configurations will build with prior versions of stack, I think it makes sense to keep the restriction in stack.

Ideally, to maintain compatibility between the tools, cabal should add the restriction. I doubt this will break many existing packages, because it is such a strange corner case.

I have seen you give no justification for this flag name. It seems that its purpose is solely to break compilation with stack. While at times the relationship is tenuous, I think overall there has been an amicable relationship between hackage / cabal-install and stackage / stack. From my perspective, this is a petty and childish action on your part which potentially compromises future amicable collaboration. I know that many others view it similarly, and are puzzled why someone clearly so intelligent as you would resort to such tactics. I'll gladly continue collaborating with other community members who are more reasonable.

Please reconsider your decision to flagrantly disregard the stack use-case. It is not too late to restore good will.

I realize that you may draw an analogy to many packages not having upper bounds. However, many intelligent people, perhaps most, will agree there are good reasons to omit upper bounds. On the other hand, there are no good reasons to have a double hyphen in a flag name. The only purpose here is to break usage of this package with stack.

@mgsloan

This comment has been minimized.

Copy link

mgsloan commented Oct 19, 2017

After some discussion with others via email and slack, the following is now clear: @hvr has a tool that generates these flag names, and likely did not initially realize that they would cause problems for stack

So, my attribution of definitive malice to this choice seems to be inaccurate. Sorry for tossing around accusations. With the comments here, it was quite hard to not attribute malice. From the perspective of a stack developer, this is stubbornness to just change an inconsequential name. This change fixes builds and has no other effects. It is easy to attribute this to stubbornness or at least unfriendliness.

Side note: typical stack configurations are currently quite forwards and backwards compatible. The very first released version of stack can still build the stack of a few months ago, and vice versa. Oldest stack probably builds today's stack as well, just haven't tested that recently. While this certainly isn't guaranteed, it really sucks to have some packages unnecessarily break this property.

I agree that this is a bug in stack. However, please consider changing the flag name, because it breaks builds for folks using older versions of stack. Thanks!

@tonymorris

This comment has been minimized.

Copy link

tonymorris commented Oct 31, 2017

Programmers :)

@mitchellwrosen

This comment has been minimized.

Copy link

mitchellwrosen commented Dec 9, 2017

Man... was releasing Cassava-0.5.1.0 (note: capital C) really the right move here? Heh. If you had to grab this package in a pinch I'm sure a non-Hackage dependency would have sufficed until you could get this issue sorted out. But now we have two different [cC]assava packages forever. Confused emoji face.

@mitchellwrosen

This comment has been minimized.

Copy link

mitchellwrosen commented Dec 9, 2017

@tfausak says via email, posted w/ his permission:

If cassava fixes the flag name, I'll gladly deprecate Cassava in favor of it. It'll be just like the cabal package.

@Cipherwraith

This comment has been minimized.

Copy link

Cipherwraith commented Dec 12, 2017

Its unfortunate that there is an impasse of such massive proportions between the stack devs and @hvr.

@ekmett

This comment has been minimized.

Copy link

ekmett commented Dec 31, 2017

Dumb question:

Would changing the flag actually fix anything if stack has to parse the old files anyways?

It seems based on cursory observation that once any version of any package with a flag name that stack didn't understand made its way into the index, then stack became completely hosed, no?

I'd be happy to be wrong here.

@ekmett

This comment has been minimized.

Copy link

ekmett commented Jan 2, 2018

https://twitter.com/taylorfausak/status/948280024129527808 makes a fairly compelling case for renaming the flag in my eyes. It'd mean even old versions of stack could build new snapshots.

@ejmg

This comment has been minimized.

Copy link

ejmg commented Jan 5, 2018

wasted nearly 45 mins diagnosing this bug because of a divergence between my shell instances and upgrading stack, silly this PR wasn't taken for something so simple yet time draining. Thanks!

@tonymorris

This comment has been minimized.

Copy link

tonymorris commented Jan 5, 2018

@ejmg

This comment has been minimized.

Copy link

ejmg commented Jan 5, 2018

Hey, thanks for the feedback. Tons of people use Stack because it's a pretty great tool, popular trade books endorse it, and large companies like Facebook actively advocate its adoption, so I'm sure your feedback will really turn the tide here.

To everyone else I'm spamming on this thread, I'm sorry my comment didn't more appropriately join the community discussion on whether certain flag naming choices in a major library should accommodate a major build tool. It was my first project with the language and I got frustrated after the lost time from a small bug/realizing my code was actually fine.

@tonymorris

This comment has been minimized.

Copy link

tonymorris commented Jan 5, 2018

Yeah, I remember in the late 1990s when a large company endorsed that we all use Windows 98 and plenty of people were using it. Turns out that was a bad idea!

Sorry to hear your introduction was frustrating. Try not using stack and see how that goes. We're all waiting to help you.

nh2 added a commit to nh2/keter that referenced this pull request Jan 16, 2018

setup-keter: Install latest stack from haskellstack.org instead of ap…
…t sources.

The apt sources for stack are no longer maintained.
They have no version newer than stack 1.5.1.

haskellstack.org is now the official install location
even for apt-base distros.

With stack 1.5.1, installing keter fails with:

  AesonException "Error in $.packages.cassava.constraints.flags['bytestring--lt-0_10_4']: Invalid flag name: \"bytestring--lt-0_10_4\""

This is caused by haskell-hvr/cassava#155.
Using the latest stack, works around that issue.

@nh2 nh2 referenced this pull request Jan 16, 2018

Open

Fix cassava doubledash #180

@vedksah vedksah referenced this pull request Jul 10, 2018

Open

Uncurated Hackage Layer #6

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