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.

Sign up for GitHub

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

Jump to bottom

It should not matter if flags given before command or after #3959

Closed
brandon-leapyear opened this issue Apr 9, 2018 · 2 comments · Fixed by #4067
Closed

It should not matter if flags given before command or after #3959

brandon-leapyear opened this issue Apr 9, 2018 · 2 comments · Fixed by #4067
Labels
component: options parsing type: bug
Milestone
P2: Should

Comments

@brandon-leapyear
Copy link
Contributor

brandon-leapyear commented Apr 9, 2018 •
edited
Loading

This happened in a wrapper script, when I'm setting default flags to pass to stack; e.g.

#!/bin/bash
STACK_ARGS=(
    --test --no-run-tests
    --bench --no-run-benchmarks
)
exec stack "${STACK_ARGS[@]}" "$@"

Steps to reproduce

  1. stack --test --no-run-tests build
  2. stack build --test --no-run-tests

Expected

Either both should build tests and not run them or neither should work (since stack doesn't take those flags on its own)

Actual

The first built and ran tests. The second built but didn't run tests.

In other words, stack --test build and stack --test --no-run-tests build do the same thing.

Stack version

Version 1.6.5, Git revision 24ab0d6ff07f28276e082c3ce74dfdeb1a2ca9e9 (5514 commits) x86_64 hpack-0.20.0

Method of installation

From https://github.com/commercialhaskell/stack/releases/download
@snoyberg snoyberg added this to the P2: Should milestone Apr 18, 2018
@snoyberg
Copy link
Contributor

Thank you for the report, I can confirm the bug.

@snoyberg snoyberg mentioned this issue Apr 18, 2018
Closed
markus1189 added a commit to markus1189/stack that referenced this issue Jun 6, 2018
@markus1189
Change parsers to use flag' instead of switch
d854357 
This fixes commercialhaskell#3959.

The problem is that the
[switch](https://hackage.haskell.org/package/optparse-applicative/docs/Options-Applicative-Builder.html#v:switch)
function returns a `Just False` which gets wrapped into `First`.  This
means that the value is *always* set to either `True` or `False` and
the `First` monoid means that during options parsing the order of the
flags suddenly matters as described in commercialhaskell#3959, because `First (Just
False)` is chosen.

The fix is to use the
[flag](https://hackage.haskell.org/package/optparse-applicative-0.14.2.0/docs/Options-Applicative-Builder.html#v:flag-39-)
function that does not set a default value, so we get a `First
Nothing` instead.
markus1189 added a commit to markus1189/stack that referenced this issue Jun 6, 2018
@markus1189
Change parsers to use flag' instead of switch
98d25ce 
This fixes commercialhaskell#3959.

The problem is that the [switch](https://hackage.haskell.org/package/optparse-applicative/docs/Options-Applicative-Builder.html#v:switch) function returns a `Just False` which gets wrapped into `First`.

This means that the value is *always* set to either `True` or `False`
and the `First` monoid means that during options parsing the order of
the flags suddenly matters as described in commercialhaskell#3959, because `First (Just
False)` is chosen when `mappend`ing with `First (Just True)`.

The fix is to use the [flag](https://hackage.haskell.org/package/optparse-applicative-0.14.2.0/docs/Options-Applicative-Builder.html#v:flag-39-) function that does not set a default value, so we get a `First Nothing` instead.
@markus1189 markus1189 mentioned this issue Jun 6, 2018
Merged
2 tasks
markus1189 added a commit to markus1189/stack that referenced this issue Jun 6, 2018
@markus1189
Change parsers to use flag' instead of switch
29a87d2 
This fixes commercialhaskell#3959.

The problem is that the [switch](https://hackage.haskell.org/package/optparse-applicative/docs/Options-Applicative-Builder.html#v:switch) function returns a `Just False` which gets wrapped into `First`.

This means that the value is *always* set to either `True` or `False`
and the `First` monoid means that during options parsing the order of
the flags suddenly matters as described in commercialhaskell#3959, because `First (Just
False)` is chosen when `mappend`ing with `First (Just True)`.

The fix is to use the [flag](https://hackage.haskell.org/package/optparse-applicative-0.14.2.0/docs/Options-Applicative-Builder.html#v:flag-39-) function that does not set a default value, so we get a `First Nothing` instead.
markus1189 added a commit to markus1189/stack that referenced this issue Jun 6, 2018
@markus1189
Change parsers to use flag' instead of switch
7d50f47 
This fixes commercialhaskell#3959.

The problem is that the [switch](https://hackage.haskell.org/package/optparse-applicative/docs/Options-Applicative-Builder.html#v:switch) function returns a `Just False` which gets wrapped into `First`.

This means that the value is *always* set to either `True` or `False`
and the `First` monoid means that during options parsing the order of
the flags suddenly matters as described in commercialhaskell#3959, because `First (Just
False)` is chosen when `mappend`ing with `First (Just True)`.

The fix is to use the [flag](https://hackage.haskell.org/package/optparse-applicative-0.14.2.0/docs/Options-Applicative-Builder.html#v:flag-39-) function that does not set a default value, so we get a `First Nothing` instead.
markus1189 added a commit to markus1189/stack that referenced this issue Jun 6, 2018
@markus1189
Change parsers to use flag' instead of switch
bf4711e 
This fixes commercialhaskell#3959.

The problem is that the [switch](https://hackage.haskell.org/package/optparse-applicative/docs/Options-Applicative-Builder.html#v:switch) function returns a `Just False` which gets wrapped into `First`.

This means that the value is *always* set to either `True` or `False`
and the `First` monoid means that during options parsing the order of
the flags suddenly matters as described in commercialhaskell#3959, because `First (Just
False)` is chosen when `mappend`ing with `First (Just True)`.

The fix is to use the [flag](https://hackage.haskell.org/package/optparse-applicative-0.14.2.0/docs/Options-Applicative-Builder.html#v:flag-39-) function that does not set a default value, so we get a `First Nothing` instead.
@snoyberg
Copy link
Contributor

snoyberg commented Jun 6, 2018

This should be addressed by #4067.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: options parsing type: bug
Projects
None yet
Milestone
P2: Should
Development

Successfully merging a pull request may close this issue.

Change parsers to use flag' instead of switch markus1189/stack
2 participants
@snoyberg @brandon-leapyear