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

CI: Nix: make Nix dev env passing optional #2488

Conversation

Anton-Latukha
Copy link
Collaborator

@Anton-Latukha Anton-Latukha commented Dec 14, 2021

This makes Nix dev env configuration in HLS development a non-blocking process.

Now CI would simply report the status of the build, but wouldn't block the workflow/merges.

This allows both HLS to be developed & for Nix dev env to be maintained at its own pase.


derived from #2480

This makes Nix dev env configuration in HLS development a non-blocking process.
Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

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

well, I think the problematic nix builds are only triggered for master (and it does not block the pr) and if the pr is touching nix config files, thanks to skip-action rules about file globs triggering the job

And I think failing in the changing nix files case should be kept.

@Anton-Latukha
Copy link
Collaborator Author

Anton-Latukha commented Dec 14, 2021

Well, then yesterday the skip rule seemed to skip the build in the #2470.

The failing build in the master is the proof that in the PR it was not checked. But also:

& then master branch failed to build.

At the time - I looked through reviews & the doc in #2476, everything was addressed & the change was minor. Decided that it is good enough to be done with, so marked green & set merge me. Then CI failed. I thought "what the heck, build failure on minor doc update" & so I noticed that red status of build on master caused the same build failure in #2476, so opened #2480 with "& all PRs".

So from what I've seen, skipping a workflow means that PR does not need to check it. But when after that master runs the workflows & fails - the fail makes PRs run the same check, or something like that.

I'm solid that was the case. Because CI failed with that error & when I was doing a report & stuff only after that PR got blocked in review 8)

Making build optional allows to be soft management about it & punting the build until it fixed. The downside to it

@Ailrun
Copy link
Member

Ailrun commented Dec 14, 2021

@Anton-Latukha I think it's more like a (arguable) bug of skip-duplicate-actions. The "non-skip" behaviour happened probably because of merge commit, which "modifies" other files than the document files. I'm not sure whether skipping the check in that case always makes sense or not.

@Anton-Latukha
Copy link
Collaborator Author

Well, it is also a skipping action bug. It is both at the same time.

I just generally found allowing the build to report error & PRs still be mergeable - a good soft management approach.

But also essentially containing Nix checks to PRs that do changes to Nix configuration & to the main branch - is almost the same, but better 😆

@Anton-Latukha
Copy link
Collaborator Author

Ok then.

@jneira
Copy link
Member

jneira commented Dec 14, 2021

Yeah take a look to that pr jobs:

https://github.com/haskell/haskell-language-server/actions/workflows/nix.yml?query=branch%3Aadd-emacs-config-example

The three first commits skipped correctly the nix build, but the merge commit with master (42a9d59) included commits with changes to .nix files:

From the pre job of https://github.com/haskell/haskell-language-server/runs/4510223222?check_suite_focus=true:

Run fkirc/[email protected]
Stop backtracking at commit https://github.com/haskell/haskell-language-server/commit/42a9d59288ae180c6a02fe59ede8958fc78092c7 because '.github/workflows/test.yml,cabal-ghc901.project,cabal-ghc921.project,cabal.project,configuration-ghc-901.nix,docs/troubleshooting.md,flake.nix,ghcide/ghcide.cabal,ghcide/src/Development/IDE/Core/Compile.hs,ghcide/src/Development/IDE/Core/ProgressReporting.hs,ghcide/src/Development/IDE/Core/Rules.hs,ghcide/src/Development/IDE/Core/Shake.hs,ghcide/test/exe/Progress.hs,haskell-language-server.cabal,plugins/hls-class-plugin/src/Ide/Plugin/Class.hs,stack-8.10.6.yaml,stack-8.10.7.yaml,stack-8.6.5.yaml,stack-8.8.4.yaml,stack-9.0.1.yaml,stack.yaml' are not skippable against paths '**.nix' or paths_ignore ''
Do not skip execution because we did not find a transferable run

And it triggered the nix build incorrectly cause the pr itself did not change any of those files. So i think doing a rebase instead of merge would not triggered it.

I would prefer continue being as strict as possible without bother contributors. If the nix build is broken on master we can rebase master onto the pr branch before merging the pr itslef (instead with a merge commit), or even merge the pr forcily if the unique failing job is the nix build and it is already failing in master.
If doing those exceptions becomes too frequent and nix build fixes are excesively delayed we could add this config changes at that point.

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.

3 participants