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.

Sign up for GitHub

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

Jump to bottom

Fix nix build CI #3404

Merged
merged 15 commits into from
Dec 17, 2022
Merged

Fix nix build CI #3404

merged 15 commits into from
Dec 17, 2022

Conversation

wavewave
Copy link
Contributor

@wavewave wavewave commented Dec 16, 2022 •
edited
Loading

flakes don't need the special installer anymore
cachix/install-nix-action@46187b3

This fixes the nix build CI, which has been broken for a while with this permission error:

Screen Shot 2022-12-15 at 11 29 50 PM

@wavewave wavewave marked this pull request as draft December 16, 2022 05:55
@wavewave wavewave changed the title WIP: nix build CI Fix nix build CI Dec 16, 2022
@wavewave wavewave force-pushed the wavewave/ci-nix-build branch from c8507a0 to 4c518f8 Compare December 16, 2022 07:02
@wavewave
Copy link
Contributor Author

This branch merged #3400 and has a fix for hls-cabal-plugin (in an opposite way to #3383, by removing Cabal-syntax, not cabal, for making stackage test pass)
I think whether to choose Cabal or Cabal-syntax can be determined once we have the CI on master green.

@wavewave wavewave marked this pull request as ready for review December 16, 2022 07:44
@wavewave wavewave requested a review from fendor as a code owner December 16, 2022 07:44
@wavewave
Copy link
Contributor Author

also cherry-picked the commit "implicit-hie < 0.1.3" due to subtle cradle detection behavior change in implicit-hie, resulting in test failure, which was found in #3317

@michaelpj
Copy link
Collaborator

If you're superseding #3400 then please close that one.

Happy to merge, but I do want a comment about the fourmolu issue.

@michaelpj michaelpj added the merge me Label to trigger pull request merge label Dec 16, 2022
fendor
fendor reviewed Dec 16, 2022
View reviewed changes
cabal.project Outdated
stylish-haskell +ghc-lib,
-- implicit-hie 0.1.3.0 introduced an unexpected behavioral change.
-- https://github.com/Avi-D-coder/implicit-hie/issues/50
implicit-hie < 0.1.3
Copy link
Collaborator

@fendor fendor Dec 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

See #3317 (comment), this applies here, too. @michaelpj what do you think? Fix in a follow-up PR? Must not be forgotten for #3409

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, good point. I was thinking it was okay to be temporary, but indeed it can mess up our ability to release. Better to put a fake dependency in ghcide if we have to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added implicit-hie to ghcide.

@michaelpj michaelpj removed the merge me Label to trigger pull request merge label Dec 16, 2022
@wavewave wavewave mentioned this pull request Dec 16, 2022
Merged
@wavewave wavewave requested a review from pepeiborra as a code owner December 16, 2022 20:25
@wavewave
Copy link
Contributor Author

hmm. tests on GHC 8.10.7 failed after putting implicit-hie to ghcide dependency. Is it due to some flakiness?

@michaelpj
Copy link
Collaborator

Yeah, almost certainly. I've rerun the jobs.

@wavewave
Copy link
Contributor Author

oh thank you!

@wavewave
Copy link
Contributor Author

It was really due to the flaky tests. Now it's back to green. :-)

@fendor fendor added the merge me Label to trigger pull request merge label Dec 17, 2022
fendor
fendor approved these changes Dec 17, 2022
View reviewed changes
Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mergify mergify bot merged commit f5db150 into haskell:master Dec 17, 2022
@wavewave wavewave deleted the wavewave/ci-nix-build branch December 19, 2022 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelpj michaelpj michaelpj approved these changes

@fendor fendor fendor approved these changes

@berberman berberman Awaiting requested review from berberman berberman is a code owner

@guibou guibou Awaiting requested review from guibou guibou is a code owner

@Anton-Latukha Anton-Latukha Awaiting requested review from Anton-Latukha

@Ailrun Ailrun Awaiting requested review from Ailrun

@pepeiborra pepeiborra Awaiting requested review from pepeiborra

Assignees
No one assigned
Labels
merge me Label to trigger pull request merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wavewave @michaelpj @fendor