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

nix: update flake inputs #8943

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

evanrichter
Copy link
Contributor

also removed non-existent crane flake input overrides

This was just a simple nix flake update

* removed non-existent crane flake input overrides
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Thanks!

@the-mikedavis the-mikedavis added A-dependencies Area: Dependency S-waiting-on-review Status: Awaiting review from a maintainer. labels Nov 29, 2023
@nanzhong
Copy link
Contributor

nanzhong commented Dec 4, 2023

Just wanted to give a heads up that that this will currently fail to build via the flake (at least on macos) until #8962 is applied (rescript grammar failing to build), this is related to nixpkgs-unstable bumping to clang 16 in the stdenv.

Not sure if this has been brought up before, but would it make sense to add a CI step to actually build helix so that build failures like this can be caught via automation?

@pascalkuthe
Copy link
Member

we ofcourse have a ci step that builds helix (including grammars) but that runs on the distros that GH runner provide so we can't catch "grammar won't work with distro/compiler configuration X" this way (and we can't change that without making CI really complicated an slow)

@pascalkuthe pascalkuthe merged commit 455b206 into helix-editor:master Dec 4, 2023
6 checks passed
@nanzhong
Copy link
Contributor

nanzhong commented Dec 4, 2023

Are you referring to https://github.com/helix-editor/helix/blob/master/.github/workflows/cachix.yml? Just to clarify I'm talking about building helix using the flake. To me that workflow looks like it's doing just that (though it's not matrixed on os like the build workflow), but it only runs on the master branch post merge. My question was more whether that should run on PRs as well (and tangentially whether it should also be matrixed on OS) to verify that the flake actually builds; I think in theory that should have caught this failure.

I guess whether this additional overhead is worth it is dependent on how much nix is intended to be supported or expected to be used for helix.

@pascalkuthe
Copy link
Member

Ah no I didn't mean nix specifically. I meant we build helix in CI. The official build process for development uses cargo and that is what we test (on the GJ runner). To me nix is just another distro/package manger and shouldn't affect whether we merge PRs.

evanrichter added a commit to evanrichter/helix that referenced this pull request Dec 5, 2023
* removed non-existent crane flake input overrides
Desdaemon pushed a commit to Desdaemon/helix that referenced this pull request Dec 11, 2023
* removed non-existent crane flake input overrides
dgkf pushed a commit to dgkf/helix that referenced this pull request Jan 30, 2024
* removed non-existent crane flake input overrides
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
* removed non-existent crane flake input overrides
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
* removed non-existent crane flake input overrides
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
* removed non-existent crane flake input overrides
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependencies Area: Dependency S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants