Add js-options field to cabal file to allow passing custom preprocessing options for js (#10721) #10722
Conversation
|
Please let me know if this is an acceptable feature at all, if so I'm gonna add any needed doc changes / changelogs. |
Swordlash
changed the title
| @@ -621,6 +621,7 @@ buildInfoFieldGrammar = | |||
| <*> monoidalFieldAla "cc-options" (alaList' NoCommaFSep Token') L.ccOptions | |||
| <*> monoidalFieldAla "cxx-options" (alaList' NoCommaFSep Token') L.cxxOptions | |||
| ^^^ availableSince CabalSpecV2_2 [] | |||
| <*> monoidalFieldAla "js-options" (alaList' NoCommaFSep Token') L.jsOptions | |||
There was a problem hiding this comment.
See above, needs to be guarded by availableSince.
There was a problem hiding this comment.
I thought about it but got confused by a fact js-sources are not guarded. Is it an omission?
Which value should I put here?
There was a problem hiding this comment.
@mpickering ? Should I add version 3.16?
|
Can you please add some tests?
|
|
@mpickering: are you satisfied with the PR now? Let me rebase in order to prod CI. |
|
@mergify rebase |
✅ Branch has been successfully rebased |
|
Hah, it seems the whitespace check is failing. |
|
|
||
| if impl(javascript) | ||
| js-sources: jsbits/lib.js | ||
| js-options: -optJSP-DPRINT_DEF |
There was a problem hiding this comment.
This is wrong. Now it's just a new name for ghc-options.
If js-options is introduced as a separate field, they should be prepended by -optJSP automatically (like cpp-options or cxx-options are prepended with appropriate -opt flag).
Also these are jsp options, i.e. JavaScript Preprocessor options. (c.f. cc-options and cpp-options).
There was a problem hiding this comment.
This is wrong. Now it's just a new name for ghc-options.
If what I need was achievable through ghc-options, there would be no ticket!
Your suggestion for remame and prepend makes sense, I was merely following a hint of how to implement it. If there is a better way to make those options be passed rather than by separate field, let me know.
There was a problem hiding this comment.
If what I need was achievable through ghc-options, there would be no ticket!
That's a different issue.
As @hsyl20 pointed on GHC tracker
So it looks like the ghc-options isn't passed by Cabal when building the js-sources
and that's the bug.
If there say ghc-options: -Werror is not passed by cabal when building js-sources, that's out right bug, which is not fixed by this PR.
Note: if ghc-options is not passed when compiling Cmm or asm, that's also a bug. This is not unique, IMHO Cabal should simply dump all possible options to GHC and not try to filter them; e.g. #9801 is another issue where Cabal omits options, which in fact can affect GHC when compiling "other stuff". TL;DR, Cabal tries to be too smart here.
In other words: jsp-options should be passed to GHC even when it's compiling ordinary Haskell files. Who knows, maybe GHC will allow inline JS which needs to be preprocessed. (It kind of allows inline C with CApiFFI!).
The PR fixes the issue (#10721) by adding a new
js-optionsfield to cabal file that are passed tocomponentJsGhcOptions.Please advise on the testing strategy, if any.
Thank you @hsyl20 for investigation and suggestion how to fix it.