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 cabal scripting support #5483

Merged
merged 7 commits into from Aug 2, 2018

Conversation

Projects
3 participants
@typedrat
Collaborator

typedrat commented Jul 31, 2018

Closes #3843. Implements effectively the design given, with some changes:

  • cabal new-run allows the user to run scripts in files without shebangs
  • The {- cabal: -} syntax is now that of the executable block

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.
@23Skidoo

This comment has been minimized.

Show comment
Hide comment
@23Skidoo

23Skidoo Jul 31, 2018

Member

Nice! Needs docs though.

Member

23Skidoo commented Jul 31, 2018

Nice! Needs docs though.

@typedrat typedrat moved this from In Progress to Needs Review in Have new-build become build (GSOC2018) Aug 1, 2018

@23Skidoo

This comment has been minimized.

Show comment
Hide comment
@23Skidoo

23Skidoo Aug 1, 2018

Member

I tried this with a multi-module script and got the following error:

$ cabal-typedrat new-run Shake/Shake.hs
Resolving dependencies...
Build profile: -w ghc-8.4.3 -O1
In order, the following will be built (use -v for more details):
 - fake-package-0 (exe:script) (configuration changed)
Configuring executable 'script' for fake-package-0..
Preprocessing executable 'script' for fake-package-0..
cabal-typedrat: can't find source for Shake/Cabal in .,
[...]/dist-newstyle/build/x86_64-linux/ghc-8.4.3/fake-package-0/x/script/build/script/autogen,
[...]/dist-newstyle/build/x86_64-linux/ghc-8.4.3/fake-package-0/x/script/build/global-autogen

In the Shake/Shake.hs file I have

{- cabal:
build-depends: base, aeson ^>= 1.4, shake ^>= 0.16
other-modules: Shake.Cabal
               Shake.DBSchema
               Shake.Flags
               Shake.GetHsDeps
               Shake.NewBuild
               Shake.Oracles
               Shake.TeamCity
               Shake.Utils
ghc-options: -with-rtsopts=-I0
default-language: Haskell2010
-}
Member

23Skidoo commented Aug 1, 2018

I tried this with a multi-module script and got the following error:

$ cabal-typedrat new-run Shake/Shake.hs
Resolving dependencies...
Build profile: -w ghc-8.4.3 -O1
In order, the following will be built (use -v for more details):
 - fake-package-0 (exe:script) (configuration changed)
Configuring executable 'script' for fake-package-0..
Preprocessing executable 'script' for fake-package-0..
cabal-typedrat: can't find source for Shake/Cabal in .,
[...]/dist-newstyle/build/x86_64-linux/ghc-8.4.3/fake-package-0/x/script/build/script/autogen,
[...]/dist-newstyle/build/x86_64-linux/ghc-8.4.3/fake-package-0/x/script/build/global-autogen

In the Shake/Shake.hs file I have

{- cabal:
build-depends: base, aeson ^>= 1.4, shake ^>= 0.16
other-modules: Shake.Cabal
               Shake.DBSchema
               Shake.Flags
               Shake.GetHsDeps
               Shake.NewBuild
               Shake.Oracles
               Shake.TeamCity
               Shake.Utils
ghc-options: -with-rtsopts=-I0
default-language: Haskell2010
-}
@typedrat

This comment has been minimized.

Show comment
Hide comment
@typedrat

typedrat Aug 1, 2018

Collaborator

That’s not something I even considered on the docket. Easy enough to implement I suppose.

Collaborator

typedrat commented Aug 1, 2018

That’s not something I even considered on the docket. Easy enough to implement I suppose.

::
$ cabal new-run script.hs

This comment has been minimized.

@23Skidoo

23Skidoo Aug 1, 2018

Member

Can you please also add an example of how to pass command-line arguments to a script started with new-run? I expect something like cabal new-run script.hs -- --foo bar.

@23Skidoo

23Skidoo Aug 1, 2018

Member

Can you please also add an example of how to pass command-line arguments to a script started with new-run? I expect something like cabal new-run script.hs -- --foo bar.

This comment has been minimized.

@typedrat

typedrat Aug 2, 2018

Collaborator

That's exactly right, I thought it was regular enough with the rest of new-run that it didn't need to be called out explicitly, but you're right that it could use a mention.

@typedrat

typedrat Aug 2, 2018

Collaborator

That's exactly right, I thought it was regular enough with the rest of new-run that it didn't need to be called out explicitly, but you're right that it could use a mention.

#!/usr/bin/env cabal
{- cabal:
build-depends: base ^>= 4.11
, shelly ^>= 1.8.1

This comment has been minimized.

@23Skidoo

23Skidoo Aug 1, 2018

Member

Probably needs default-language: Haskell2010 as well, since otherwise you always get a warning. Unless we decide to splice that in automagically.

@23Skidoo

23Skidoo Aug 1, 2018

Member

Probably needs default-language: Haskell2010 as well, since otherwise you always get a warning. Unless we decide to splice that in automagically.

This comment has been minimized.

@hvr

hvr Aug 1, 2018

Member

@23Skidoo actually this makes me think of something... I think we should encode the spec-version somehow... e.g.

#!/usr/bin/env cabal
{-# LANGUAGE Haskell2010, GADTs #-}
{- cabal:2.4
build-depends: ...
-}

and we might want to tolerate not having to specify default-language w/ scripts, as default-{extensions,language} makes less sense imho for scripts, as we have actual language pragmas (I've intentionally added one in the example above), and you certainly wouldn't want to replicate those...

@hvr

hvr Aug 1, 2018

Member

@23Skidoo actually this makes me think of something... I think we should encode the spec-version somehow... e.g.

#!/usr/bin/env cabal
{-# LANGUAGE Haskell2010, GADTs #-}
{- cabal:2.4
build-depends: ...
-}

and we might want to tolerate not having to specify default-language w/ scripts, as default-{extensions,language} makes less sense imho for scripts, as we have actual language pragmas (I've intentionally added one in the example above), and you certainly wouldn't want to replicate those...

This comment has been minimized.

@23Skidoo

23Skidoo Aug 1, 2018

Member

What about multi-file scripts? Easier to set default-language in one place than use a pragma in each file.

@23Skidoo

23Skidoo Aug 1, 2018

Member

What about multi-file scripts? Easier to set default-language in one place than use a pragma in each file.

This comment has been minimized.

@hvr

hvr Aug 1, 2018

Member

@23Skidoo we can still support default-language; I just think we shouldn't force people to use it by emitting nasty warnings like we do for .cabal files...

@hvr

hvr Aug 1, 2018

Member

@23Skidoo we can still support default-language; I just think we shouldn't force people to use it by emitting nasty warnings like we do for .cabal files...

This comment has been minimized.

@typedrat

typedrat Aug 1, 2018

Collaborator

It skips that check on purpose.

@typedrat

typedrat Aug 1, 2018

Collaborator

It skips that check on purpose.

This comment has been minimized.

@23Skidoo

23Skidoo Aug 2, 2018

Member

Won't we then need to parse LANGUAGE pragmas to make sure that they are actually specified when default-language is absent?

@23Skidoo

23Skidoo Aug 2, 2018

Member

Won't we then need to parse LANGUAGE pragmas to make sure that they are actually specified when default-language is absent?

This comment has been minimized.

@hvr

hvr Aug 2, 2018

Member

@23Skidoo depends on the semantics you want to have for scripts...

However, I suggest we postpone this design/bikeshedding issue to a followup-task in order not to hold up merging this PR; I think all @typedrat needs to do is tweaks the docs and we're ready to go?

@hvr

hvr Aug 2, 2018

Member

@23Skidoo depends on the semantics you want to have for scripts...

However, I suggest we postpone this design/bikeshedding issue to a followup-task in order not to hold up merging this PR; I think all @typedrat needs to do is tweaks the docs and we're ready to go?

@typedrat typedrat merged commit 6b6d588 into haskell:master Aug 2, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@typedrat typedrat moved this from Needs Review to Done in Have new-build become build (GSOC2018) Aug 2, 2018

@hvr hvr added this to the 2.4 milestone Aug 2, 2018

@hvr

This comment has been minimized.

Show comment
Hide comment
@hvr

hvr Aug 2, 2018

Member

@23Skidoo any objections to cherry picking this to 2.4?

Member

hvr commented Aug 2, 2018

@23Skidoo any objections to cherry picking this to 2.4?

@23Skidoo

This comment has been minimized.

Show comment
Hide comment
@23Skidoo

23Skidoo Aug 2, 2018

Member

@hvr What about the spec-version thing?

Member

23Skidoo commented Aug 2, 2018

@hvr What about the spec-version thing?

@hvr

This comment has been minimized.

Show comment
Hide comment
@hvr

hvr Aug 2, 2018

Member

@23Skidoo well, technically this is still all a tech-preview in 2.4, thanks to the new-* prefix... so we don't need this to be "final", no? :-)

As for what to do about the spec-version, I intend to write up a design ticket to discuss how to pin down the semantics in a robust way so that scripts don't bitrot if cabal keeps changing its semantics.

In any case, I consider the benefit of making this feature available to users in 2.4 in a tech-preview to gain more feedback & experience to outweight the potential risk of having to break those early adopters in 3.0...

Member

hvr commented Aug 2, 2018

@23Skidoo well, technically this is still all a tech-preview in 2.4, thanks to the new-* prefix... so we don't need this to be "final", no? :-)

As for what to do about the spec-version, I intend to write up a design ticket to discuss how to pin down the semantics in a robust way so that scripts don't bitrot if cabal keeps changing its semantics.

In any case, I consider the benefit of making this feature available to users in 2.4 in a tech-preview to gain more feedback & experience to outweight the potential risk of having to break those early adopters in 3.0...

@23Skidoo

This comment has been minimized.

Show comment
Hide comment
@23Skidoo

23Skidoo Aug 7, 2018

Member

@hvr OK, sure, let's backport this to 2.4.

Member

23Skidoo commented Aug 7, 2018

@hvr OK, sure, let's backport this to 2.4.

@hvr

This comment has been minimized.

Show comment
Hide comment
@hvr

hvr Aug 7, 2018

Member

@23Skidoo turns out it's been already done via 3e6a2c4 :-)

Member

hvr commented Aug 7, 2018

@23Skidoo turns out it's been already done via 3e6a2c4 :-)

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