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

Use ghc+integer-gmp for alpine linux build release #2615

Merged
merged 14 commits into from
Jan 22, 2022

Conversation

jneira
Copy link
Member

@jneira jneira commented Jan 20, 2022

@jneira
Copy link
Member Author

jneira commented Jan 20, 2022

@jneira
Copy link
Member Author

jneira commented Jan 20, 2022

The build has progressed but it has failed for linux and ghc-9.2.1 with

src/Ide/Version.hs:24:21: error:
Error:     • Exception when trying to run compile-time code:
        PATH: getEnv: does not exist (no environment variable)
      Code: gitCommitCount
    • In the untyped splice: $gitCommitCount
   |
24 |   let commitCount = $gitCommitCount
   |                     ^^^^^^^^^^^^^^^
cabal: Failed to build hls-1.5.1.0 (which is required by exe:hls from
hls-1.5.1.0).

🤔

@hasufell
Copy link
Member

hasufell commented Jan 20, 2022

The build has progressed but it has failed for linux and ghc-9.2.1 with

src/Ide/Version.hs:24:21: error:
Error:     • Exception when trying to run compile-time code:
        PATH: getEnv: does not exist (no environment variable)
      Code: gitCommitCount
    • In the untyped splice: $gitCommitCount
   |
24 |   let commitCount = $gitCommitCount
   |                     ^^^^^^^^^^^^^^^
cabal: Failed to build hls-1.5.1.0 (which is required by exe:hls from
hls-1.5.1.0).

thinking

Sounds like this part is failing: https://github.com/acfoltzer/gitrev/blob/450a7abf444260e95791649489705e54650c0232/src/Development/GitRev.hs#L65

The environment is borked (doesn't contain the PATH env var)?

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

it does (https://github.com/jneira/haskell-language-server/runs/4889676070?check_suite_focus=true)

Path: /__t/stack/2.7.3/x64:/github/home/.ghcup/bin:/github/home/.ghcup/ghc/9.2.1/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin

the build is successful for all ghcs < 9.2.1, using the same docker image for all, and the build for ghc-9.2.1 is successful in Ubuntu so i would say it is a weird interaction between both

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

well the workflow between the Ubuntu build and the alpine one 8s not exactly the same but I don't see any change which could cause the bug
will try to use Ubuntu in the same workflow to check it though (and as possible workaround)

also we could trace the value of the PATH env bar in our code but getExecutable is pretty standard

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

the build for ghc-9.2.1 is successful in Ubuntu so I would say it is a weird interaction between both

well the ghc version for alpine is not exactly the same than the Ubuntu one

anyone has an alpine machine to test the build, try to reproduce and trace the cause easier?

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

As first sanity check i am gonna invalidate the cache created in ubuntu

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

As expected changing alpine for ubuntu in the ghc-9.2.1 build has been succesful so the workflow is not the culprit: https://github.com/jneira/haskell-language-server/runs/4893355328?check_suite_focus=true

At this point i think we could consider use ubuntu for ghc-9.2.1 (it is experimental anyways) until we fix the bug for alpine + ghc-9.2.1. That way we will unblock the incoming release.

//cc @hasufell @pepeiborra @michaelpj

@michaelpj
Copy link
Collaborator

@Bodigrim made a release of mod with integer-simple (Bodigrim/mod#13 (comment)), so if we bump the index-state we may find that cabal can now get a build plan when using integer-simple.

@michaelpj
Copy link
Collaborator

Personally, the commit count suffix on the version string doesn't seem that useful to me, we could consider just binning that.

@hasufell
Copy link
Member

@Bodigrim made a release of mod with integer-simple (Bodigrim/mod#13 (comment)), so if we bump the index-state we may find that cabal can now get a build plan when using integer-simple.

Probably won't, because I already updated ghcup alpine bindists to integer-gmp.

@pepeiborra
Copy link
Collaborator

I support dropping the dependency if it helps.

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

All gitrev definitions needs find git in the env var PATH throught findExecutable "git" . We are actually using it to get the git commit here:

haskellLanguageServerVersion :: IO String
haskellLanguageServerVersion = do
path <- getExecutablePath
let gitHashSection = case $(gitHash) of
x | x == "UNKNOWN" -> ""
x -> " (GIT hash: " <> x <> ")"
return $ "haskell-language-server version: " <> haskellLanguageServerNumericVersion
<> " (GHC: " <> VERSION_ghc
<> ") (PATH: " <> path <> ")"
<> gitHashSection

and i would like to keep the commit when doing hls --version

The alternative githash dont use findExecutable though but does a raw (proc "git" args) { cwd = Just root } so maybe it would work 🤷

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

Meanwhile i've reproduced the ghc+alpine bug in a fresh alpine-3.12 vm installing ghc with ghcup:

imagen

wonderful

System.Environment.getEnv "PATH" also fails (and with whatever env var known to be set like $HOME, $USER, etc)
Opened https://gitlab.haskell.org/ghc/ghc/-/issues/20977

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

It worths to note that ghc-9.2.1 in alpine seems to be fragile:

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

As it breaks `cabal install`
@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

Ok, githash worked, yay! thanks @hasufell for the suggestion
I stand a little bit disturbed by the bug but whatever

@hasufell
Copy link
Member

@jneira I've updated the ghc 9.2.1 alpine bindist in ghcup. getEnv should work now.

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

@hasufell many thanks! i've reran the build workflow and it should use the new ghc bindist: https://github.com/haskell/haskell-language-server/actions/runs/1728847536

EDIT: it did:

[ Info  ] downloading: https://downloads.haskell.org/~ghcup/unofficial-bindists/ghc/9.2.1/ghc-9.2.1-x86_64-alpine-linux-integer-gmp.tar.xz as file /tmp/ghcup-b0002d8433a56e77/ghc-9.2.1-x86_64-alpine-linux-integer-gmp.tar.xz

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

@jneira I've updated the ghc 9.2.1 alpine bindist in ghcup. getEnv should work now.

i've double checked in my vm and it works as intended 👍

@jneira
Copy link
Member Author

jneira commented Jan 21, 2022

Well i think we can keep githash cause as commented by @hasufell it is being actively maintained so the pr could be merged if you agree

Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

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

The githash change looks fine to me, but I haven't reviewed the CI changes. Maybe @Anton-Latukha can take a look.

@jneira jneira merged commit 92a8cc0 into haskell:master Jan 22, 2022
@Anton-Latukha
Copy link
Collaborator

Anton-Latukha commented Jan 23, 2022

Well, as I came around to look at it... To review it, I needed/need to fully learn & understand the build workflow, so time.

Overall, glad to see build being ported to a formalized pattern.

Seems good. Would submit PRs if would find something.

path: ${{ steps.compress_server_binary.outputs.path }}

- name: (GHC 8.10) Build the wrapper
- name: Build the wrapper
if: matrix.ghc == '8.10.7'
run: cabal build exe:hls-wrapper -O2 $LINUX_CABAL_ARGS
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this need to be $CABAL_ARGS ?

Copy link
Member Author

Choose a reason for hiding this comment

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

omg 😧

Copy link
Member Author

Choose a reason for hiding this comment

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

that should have failed, as @hasufell suggested, setting the appropriate bash flag

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.

5 participants