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

Enhance --allow-{newer,older} syntax #4575

Merged
merged 1 commit into from
Jul 3, 2017
Merged

Enhance --allow-{newer,older} syntax #4575

merged 1 commit into from
Jul 3, 2017

Conversation

hvr
Copy link
Member

@hvr hvr commented Jun 25, 2017 •
edited
Loading

This extends the capabilities of --allow-{newer,older} to allow for more
fine-grained scoping to control more precisely which packages and constraints
a relaxation is applied to. See updated documentation for more details.

@mention-bot
Copy link

@hvr, thanks for your PR! By analyzing the history of the files in this pull request, we identified @grayjay, @dcoutts and @ezyang to be potential reviewers.

23Skidoo
23Skidoo approved these changes Jul 3, 2017
View reviewed changes
Copy link
Member

@23Skidoo 23Skidoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo some comments.

cabal-install/Distribution/Client/Types.hs Outdated
data RelaxDepScope = RelaxDepScopeAny
-- ^ Apply relaxation in any package
| RelaxDepScopePackage !PackageName
-- ^ Apply relexation to in all versions of a package
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: relexation.

cabal-install/Distribution/Client/Dependency.hs
removeBound RelaxLower RelaxDepModNone = removeLowerBound
removeBound RelaxUpper RelaxDepModNone = removeUpperBound
removeBound relKind RelaxDepModCaret =
foldVersionRange'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be a good idea for the future to add a variant of foldVersionRange that takes a record, so that in the case where most transformations are identity, client code would be more compact.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the very same thought when I implemented that function! :-)

cabal-install/Distribution/Client/Types.hs Outdated
normalP = RelaxedDep `fmap` parse
-- "greedy" choices
scopeP = (pure RelaxDepScopeAny <* Parse.char '*' <* Parse.char ':')
Parse.<++ (pure RelaxDepScopeAny <* Parse.string "all:")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit confusing that the constructor which corresponds to all is called RelaxDepScopeAny.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll rename

cabal-install/Distribution/Client/Dependency.hs
Dependency pkgName (vrtrans verRange)
relaxPackageDeps vrtrans (RelaxDepsSome allowNewerDeps') gpd =
relaxAll (Dependency pkgName verRange) =
Dependency pkgName (removeBound relKind RelaxDepModNone verRange)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should allow something like --allow-newer=^all to relax only caret dependencies everywhere. Probably not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, the ^ thing conflicts with zsh shell expansion too:

$ echo ^all              
appveyor-retry.cmd appveyor.yml AUTHORS Cabal cabal-install cabal.project cabal.project.travis 
cabal-tests.log cabal-testsuite default.nix dist dist-newstyle ghc-packages id_rsa_cabal_website.aes256.enc LICENSE README.md stack.yaml TAGS travis travis-bootstrap.sh travis-common.sh travis-deploy.sh 
travis-install.sh travis-meta.sh travis-script.sh travis-solver-debug-flags.sh travis-stack.sh update-cabal-files.sh
$ echo foo:^all
zsh: no matches found: foo:^all

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I do plan to support --allow-newer=all, --allow-newer=^all, --allow-newer=foo:^all, etc. via a future PR. That's a perfectly valid use-case when I want to quickly see if there's overly constraining upper bounds in a specific package.

I'm not sure what to do about zsh getting in the way... which unquoted syntax does it not try to interpret? ;-)

Copy link
Member

@23Skidoo 23Skidoo Jul 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can keep using ^, it's not a very common operation, and zsh users can use quotes.

@hvr
Enhance --allow-{newer,older} syntax
a0d8035 
This extends the capabilities of `--allow-{newer,older}` to allow for more
fine-grained scoping to control more precisely which packages and constraints
a relaxation is applied to. See updated documentation for more details.
@hvr hvr changed the title WIP: Enhance --allow-{newer,older} syntax Enhance --allow-{newer,older} syntax Jul 3, 2017
@hvr hvr merged commit 013cdce into haskell:master Jul 3, 2017
@hvr hvr deleted the pr/extended-relax-deps branch July 3, 2017 21:11
hvr added a commit to hvr/cabal that referenced this pull request Aug 6, 2017
@hvr
Augment --allow-{newer,older} syntax to support wildcard
4f75e5c 
This builds on top of a0d8035 (haskell#4575)
and extends the syntax to support the token `all` or `*` to serve as
wild-card for the relaxation subject, i.e. the following non-exhaustive
list of forms is made possible (NB: the package name `all` is reserved
on Hackage):

    allow-newer: somepkg:*
    allow-newer: somepkg:all
    allow-newer: somepkg:^*
    allow-newer: somepkg:^all
    allow-newer: all:^all
    allow-newer: *:^all
    allow-newer: *:^*
    allow-newer: *:*
    allow-newer: all:all

Refer to the user's guide for details
@hvr hvr mentioned this pull request Aug 6, 2017
Merged
3 tasks
hvr added a commit to hvr/cabal that referenced this pull request Aug 6, 2017
@hvr
Augment --allow-{newer,older} syntax to support wildcard
918453f 
This builds on top of a0d8035 (haskell#4575)
and extends the syntax to support the token `all` or `*` to serve as
wild-card for the relaxation subject, i.e. the following non-exhaustive
list of forms is made possible (NB: the package name `all` is reserved
on Hackage):

    allow-newer: somepkg:*
    allow-newer: somepkg:all
    allow-newer: somepkg:^*
    allow-newer: somepkg:^all
    allow-newer: all:^all
    allow-newer: *:^all
    allow-newer: *:^*
    allow-newer: *:*
    allow-newer: all:all

Refer to the user's guide for details
hvr added a commit to hvr/cabal that referenced this pull request Aug 6, 2017
@hvr
Augment --allow-{newer,older} syntax to support wildcard
c9d2990 
This builds on top of a0d8035 (haskell#4575)
and extends the syntax to support the token `all` or `*` to serve as
wild-card for the relaxation subject, i.e. the following non-exhaustive
list of forms is made possible (NB: the package name `all` is reserved
on Hackage):

    allow-newer: somepkg:*
    allow-newer: somepkg:all
    allow-newer: somepkg:^*
    allow-newer: somepkg:^all
    allow-newer: all:^all
    allow-newer: *:^all
    allow-newer: *:^*
    allow-newer: *:*
    allow-newer: all:all

Refer to the user's guide for details
hvr added a commit to hvr/cabal that referenced this pull request Aug 9, 2017
@hvr
Augment --allow-{newer,older} syntax to support wildcard
8bcd719 
This builds on top of a0d8035 (haskell#4575)
and extends the syntax to support the token `all` or `*` to serve as
wild-card for the relaxation subject, i.e. the following non-exhaustive
list of forms is made possible (NB: the package name `all` is reserved
on Hackage):

    allow-newer: somepkg:*
    allow-newer: somepkg:all
    allow-newer: somepkg:^*
    allow-newer: somepkg:^all
    allow-newer: all:^all
    allow-newer: *:^all
    allow-newer: *:^*
    allow-newer: *:*
    allow-newer: all:all

Refer to the user's guide for details
@tfausak tfausak mentioned this pull request Oct 2, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@23Skidoo 23Skidoo 23Skidoo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hvr @mention-bot @23Skidoo