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

Flip ghc-lib default to True #1385

Closed
wants to merge 1 commit into from

Conversation

shayne-fletcher
Copy link
Collaborator

Summary:

  • Flip cabal flag ghc-lib default to True (i.e. disable 'auto' mode; unconditionally link ghc-lib-parser)
  • Upgrade to ghc-lib-parser-ex-9.2.1.0 (which has the same link behavior with respect to ghc-lib-parser)
  • Fixes Consider defaulting to ghc-lib #1376

Test plan:

(1) Execute stack build --resolver nightly-2022-05-27 --cabal-verbose (i.e. ghc-9.2.2). Observe,

depends:
  ...
  ghc-lib-parser-9.2.3.20220527-Apb5ZlADTS72sl2ZpNOJTy
  ghc-lib-parser-ex-9.2.1.0-13IoJhA5FNUCTxsgEW3SsR
  ...

This shows that ghc-lib-parser is linked and not ghc libs despite the compiler version being >=9.2.2 && <9.3.

(2) Enable in stack.yaml the stanza

flags:
   hlint:
     ghc-lib: false
   ghc-lib-parser-ex:
     auto: true
     no-ghc-lib: false

and executestack build --resolver nightly-2022-05-27 --cabal-verbose. Observe now,

depends:
  ...
  ghc-9.2.2
  ghc-boot-9.2.2
  ghc-boot-th-9.2.2
  ...

This shows that with the default flags overridden we can recover the "old" behavior of linking the the native ghc libs rather than ghc-lib-parser when the compiler version is >= 9.2.2 && < 9.3

@shayne-fletcher
Copy link
Collaborator Author

shayne-fletcher commented Jun 5, 2022

it looks like macos CI is struggling with errors of this kind?

/Users/runner/.ghcup/ghc/9.2.2/lib/ghc-9.2.2/lib/../lib/x86_64-osx-ghc-9.2.2/rts-1.0.2/include/ffitarget.h:10:10: fatal error: 'ffitarget_x86.h' file not found
#include "ffitarget_x86.h"
         ^~~~~~~~~~~~~~~~~
1 error generated.

this is a known GHC issue (see more about this here). tl;dr a workaround while it's not fixed upstream is to execute build commands in an environment in which C_INCLUDE_PATH=$(xcrun --show-sdk-path)/usr/include/ffi. hope this helps!

stack.yaml Outdated Show resolved Hide resolved
@shayne-fletcher
Copy link
Collaborator Author

shayne-fletcher commented Jun 9, 2022

it looks like macos CI is struggling with errors of this kind?

/Users/runner/.ghcup/ghc/9.2.2/lib/ghc-9.2.2/lib/../lib/x86_64-osx-ghc-9.2.2/rts-1.0.2/include/ffitarget.h:10:10: fatal error: 'ffitarget_x86.h' file not found
#include "ffitarget_x86.h"
         ^~~~~~~~~~~~~~~~~
1 error generated.

this is a known GHC issue (see more about this here). tl;dr a workaround while it's not fixed upstream is to execute build commands in an environment in which C_INCLUDE_PATH=$(xcrun --show-sdk-path)/usr/include/ffi. hope this helps!
(e.g. i have script like,

cmd="cabal new-build all -j --ghc-option=-j"
ghc_version=$(ghc -V | tail -c 6)
if [[ "$ghc_version" == "9.2.2" ]]; then
  eval "C_INCLUDE_PATH=$(xcrun --show-sdk-path)/usr/include/ffi" "$cmd"
else
  eval "$cmd"
fi

disclaimer: i'm not much cop at bash so i have no idea if it's good code or not!)

this is resolved with ghc-9.2.3 however (https://gitlab.haskell.org/ghc/ghc/-/issues/20592#note_435963) so maybe the best resolution is to ghcup that one.

@ndmitchell
Copy link
Owner

Stack has moved to GHC 9.2.3 for its nightly. Shayne, can you push a rebase so I can get fresh signal on the CI. If github via ghcup has moved to 9.2.3 I think we're good to land this, otherwise I think we might have to wait.

@shayne-fletcher shayne-fletcher force-pushed the ghc-lib-parser-ex-9.2.1.0 branch from 8dda6a9 to 9a049f0 Compare July 10, 2022 12:46
@shayne-fletcher
Copy link
Collaborator Author

that's a shame. the ghc version continues to be ghc-9.2.2.

@shayne-fletcher
Copy link
Collaborator Author

Stack has moved to GHC 9.2.3 for its nightly. Shayne, can you push a rebase so I can get fresh signal on the CI. If github via ghcup has moved to 9.2.3 I think we're good to land this, otherwise I think we might have to wait.

sadly we are looking stuck. i guess there's no provision afforded us by github where we can arrange our builds be executed in an environment that sets C_INCLUDE_PATH?

@ndmitchell
Copy link
Owner

I have no doubt we can fix the CI builds. What I worry is about having CI as a signal for end users, where HLint might no longer compile.

@shayne-fletcher
Copy link
Collaborator Author

i've got a build running in #1396 that

         - os: macOS-latest
-          ghc: '9.2'
+          ghc: '9.2.3'

so far it's looking good. if it goes through would you consider this a fix?

@shayne-fletcher
Copy link
Collaborator Author

I have no doubt we can fix the CI builds. What I worry is about having CI as a signal for end users, where HLint might no longer compile.

i think i'm hearing your concern as being users encountering this error in the wild when building hlint? if so, yes, the only "fix" i know we have for that are workarounds that set C_INCLUDE_PATH (or equivalent) (there are plenty of these in https://gitlab.haskell.org/ghc/ghc/-/issues/20592).

i wonder if this isn't already a problem for users on the current version of hlint (e.g. this report digital-asset/ghc-lib#352). i simply don't know.

@shayne-fletcher
Copy link
Collaborator Author

shayne-fletcher commented Jul 10, 2022

i wonder if this isn't already a problem for users on the current version of hlint (e.g. this report digital-asset/ghc-lib#352). i simply don't know.

argh, of course it is not. mac-OS/ghc-9.2.2 with the current version of hlint will (default) build depend on ghc not ghc-lib-parser. that's why this missing include error manifests in this diff, and hasn't before now. ok, i guess we are on the same page now.

@shayne-fletcher
Copy link
Collaborator Author

i've got a build running in #1396 that

         - os: macOS-latest
-          ghc: '9.2'
+          ghc: '9.2.3'

so far it's looking good. if it goes through would you consider this a fix?

fwiw, macOS/ghc-9.2.3 builds with the changes of this diff have succeeded. making this modification to ci.yml in this PR would fix CI. do please let me know if you want me to amend this PR with it.

@shayne-fletcher shayne-fletcher force-pushed the ghc-lib-parser-ex-9.2.1.0 branch from 14d9f20 to 63aebe4 Compare August 8, 2022 23:41
@shayne-fletcher
Copy link
Collaborator Author

given #1410 has been accepted which contains this change, i am abandoning this PR.

@shayne-fletcher shayne-fletcher deleted the ghc-lib-parser-ex-9.2.1.0 branch August 12, 2022 13:38
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.

Consider defaulting to ghc-lib
2 participants