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

Add with-compiler field for cabal scripts #6310

Closed
wants to merge 5 commits into from

Conversation

lukel97
Copy link
Contributor

@lukel97 lukel97 commented Oct 26, 2019

Adds a new data type and field grammar for parsing script metadata.
Now it is possible to have scripts like:

#!/usr/bin/env cabal
{- cabal:
build-depends: base
with-compiler: ghc-8.6.5
-}
main = putStrLn "hey"

Progresses towards #5698


Please include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

Please also shortly describe how you tested your change. Bonus points for added tests!

Adds a new data type and field grammar for parsing script metadata.
Now it is possible to have scripts like:

    #!/usr/bin/env cabal
    {- cabal:
    build-depends: base
    with-compiler: ghc-8.6.5
    -}
    main = putStrLn "hey"

Progresses towards haskell#5698
Cabal/doc/nix-local-build.rst Outdated Show resolved Hide resolved
Cabal/Distribution/PackageDescription/FieldGrammar.hs Outdated Show resolved Hide resolved
<$> blurFieldGrammar L.executable (executableFieldGrammar "script")
<*> optionalFieldAla "with-compiler" FilePathNT L.hcPath
{-# SPECIALIZE scriptFieldGrammar :: ParsecFieldGrammar' Script #-}
{-# SPECIALIZE scriptFieldGrammar :: PrettyFieldGrammar' Script #-}
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's no EOL newline. Not biggie, but if you can configure your editor for this project, it would be nice next time!

@lukel97
Copy link
Contributor Author

lukel97 commented Nov 9, 2019

@phadej @DanielG ping

@hvr
Copy link
Member

hvr commented Nov 10, 2019

At this point, we should really start "versioning" the "script spec format" and make use of the version field right after the cabal: marker, e.g.

{- cabal: 3.2
build-depends: base ^>= { 4.12.0, 4.13.0 }
with-compiler: ghc-8.6.5
-}

@phadej
Copy link
Collaborator

phadej commented Nov 10, 2019 via email

@23Skidoo
Copy link
Member

/cc @typedrat

@mpickering
Copy link
Collaborator

I would also like to be able to specify the compiler to use on the command line. For example

cabal File.hs -wghc-8.6.5

It seems the option is currently just ignored?

@hvr
Copy link
Member

hvr commented Dec 14, 2019

I would also like to be able to specify the compiler to use on the command line. For example

cabal File.hs -wghc-8.6.5

It seems the option is currently just ignored?

@mpickering It's not ignored; you're invoking the special shebang interface;

If you had used the normal designated CLI invocation for cabal scripts via cabal v2-run -wghc-8.6.5 File.hs you would have been fine.

Consider this simple script:

#! /usr/bin/env cabal

{- cabal:
build-depends: base
-}

import System.Environment

main = print =<< getArgs

Then, compare the outputs of the three invocations:

$ cabal ./foo.hs -wghc-7.4.2 -- --help

["-wghc-7.4.2","--","--help"]

$ cabal v2-run ./foo.hs -wghc-7.4.2 -- --help

["--help"]

$ ./foo.hs -wghc-7.4.2 -- --help

["-wghc-7.4.2","--","--help"]

Does it make sense now?

(NB: Only the one with v2-run is being explicitly compiled with ghc-7.4.2; the other two invocations pick up the default configured ghc version for compilation).

@mpickering
Copy link
Collaborator

Thanks. I did not know you could use v2-run with an executable script.

@typedrat typedrat self-requested a review December 21, 2019 17:56
Copy link
Collaborator

@typedrat typedrat left a comment

Choose a reason for hiding this comment

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

LGTM!

@phadej phadej self-assigned this Dec 21, 2019
@phadej
Copy link
Collaborator

phadej commented Dec 21, 2019

I think @hvr raised the valid concern of need to versioning. I'll assign this to myself and merge after 3.0.1.0 and 3.2.0.0 dust is settled and add cabal versioning to this. If we version libraries, we definitely should version the tool that reads the definitions, as they will evolve (and possibly in non backward compatible ways).

@gbaz
Copy link
Collaborator

gbaz commented Aug 12, 2021

@phadej is this something you still intended to get to?

@gbaz
Copy link
Collaborator

gbaz commented Feb 24, 2022

is this superceded by #7997 ?

@bacchanalia
Copy link
Collaborator

With #7997 you can specify with-compiler inside the {- project: ... -} meta block, so I think it does supercede this.

@Mikolaj
Copy link
Member

Mikolaj commented Feb 28, 2022

In that case, let me close.

@bubba: thank you for the PR and apologies that it dragged on for so long. If you think there's anything in the PR that can get rebased, scavenged and put to good use, please kindly open a new PR and we'll do our best not to lose it again.

@Mikolaj Mikolaj closed this Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants