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

Remove nix bits from repo and CI #5373

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove nix bits from repo and CI #5373

wants to merge 1 commit into from

Conversation

wez
Copy link
Owner

@wez wez commented May 5, 2024

These workflows fail often and I have no time or desire to learn nix to troubleshoot and resolve them.

I have not been able to reach those that originally contributed them by mentions on commits on GH, and since I hate to have something that is essentially broken and opaque to me sitting in the repo, with no maintainers, I plan to remove it.

See also my original position, and my later acceptance that was hoping that the community would be more active in looking after this:

If this is to remain, I think I will need to have someone commit to looking after this stuff, and to commit to finding a replacement in the event that they no longer have the capacity to maintain it.

These workflows fail often and I have no time or desire to learn
nix to troubleshoot and resolve them.

I have not been able to reach those that originally contributed
them by mentions on commits on GH, and since I hate to have
something that is essentially broken and opaque to me sitting
in the repo, with no maintainers, I plan to remove it.

See also my original position, and my later acceptance that
was hoping that the community would be more active in looking
after this:

* #3547 (comment)
* #3547 (comment)

If this is to remain, I think I will need to have someone commit
to looking after this stuff, and to commit to finding a replacement
in the event that they no longer have the capacity to maintain it.
@wez
Copy link
Owner Author

wez commented May 5, 2024

cc: @happenslol, @gabyx

@bew
Copy link
Sponsor Contributor

bew commented May 5, 2024

Hi @wez o/

May I ask where you mentioned people about Nix issues? 🤔
A quick look at the recent git log on main shows the last one mentioning nix is from about a month ago, did I miss something?

As for how to communicate and interact, tagging individuals may not be the most effective as you say, since not everyone has much time all the time...
An idea could be to have a tracking issue (that is never never closed) that relevant people can subscribe to, and when you're doing an update with dependencies or you have an issue with Nix (and mentioning people does not work, if you want to keep doing that) you could post a small msg of what we should be looking at.
Alternative is opening a fresh issue tagged distribution: nix or similar, and people watching the repo can react there.

What do you think?


Opened PR #5374 that should fix the build

@happenslol
Copy link
Contributor

Fair enough. I'm in a place where I can't commit to maintaining this personally right now, so I won't push to keep this in the repo. I do think this would break a few setups though.

@wez
Copy link
Owner Author

wez commented May 5, 2024

I think there are a couple of logistical problems around the category of "stuff that wez cannot directly maintain in this repo":

  • There does need to be someone that is willing and able to maintain it
  • GH won't let me assign someone as a reviewer who does not have write access to the repo
  • Cc's and mentions are not high enough signal

More specifically for nix, I wonder if there is better automation possible.

For #5374, is there a way to automatically update the nix stuff in that same way?
If so, having it run at PR time to suggest the fix would be great
There could be a periodic run on main to fix and update stuff that otherwise sneaks in (eg: like me pushing a commit directly).

Is that what https://github.com/wez/wezterm/blob/main/.github/workflows/update-flake.yml is intended to do?
That workflow itself has never run successfully, in part because I had to enable certain actions for it, but having resolved that just now, it failed for other reasons.
https://github.com/wez/wezterm/actions/workflows/update-flake.yml has the history for it.

@happenslol
Copy link
Contributor

happenslol commented May 5, 2024

I don't know how realistic it is to automate package upgrades. Nix basically requires every input to the build to be locked to a specific revision, and upstream changes in the mainline nix package registry might change things around at any time, so imho manual maintenance is definitely required.

I can understand that you don't want to have stuff in the repo that can be out of date some of the time, and of course you can't be expected to deal with nix things yourself just because of it's some of your users' preferred package manager. From my point of view, having nix in the repo provides the following benefits:

  • Being able to use the latest main build in a nix setup
  • Having a local development shell with all dependencies pre-installed
  • Being able to run wezterm without installing using nix run ...

All of these can be achieved some way or another without having the files in the repo - it just removes a lot of friction and cuts down on fragmentation to have a "canonical" nix flake. From the start, I only opened the PR because I created the flake for myself and figured other people might want to use it too if the work is already done; not because I thought wezterm ought to support nix officially.

I appreciate that you're still looking to make it work somehow, but for me personally it would be fine to merge this. The only reason I'm hesitant is because the original PR got some attention, so I'd assume that some people are using it. Their current setups will keep working, but whenever they attempt to update to the latest main revision after the merge, they'd have to switch back to either a custom flake or the packaged version from nixpkgs.

That's my personal opinion, and I don't wanna give of the impression that I have any authority on this - I'm just some nix user. The other people in the thread and the other PRs/issues might have other ideas.

@gabyx
Copy link
Contributor

gabyx commented May 6, 2024

@happenslol : Fully agree with all what you said. I understand @wez for wanting the stuff deleted, however I find it extremely sad when Nix tooling is gone, I am currently fully using this Nix flake.nix file on my system and will continuing to do so.
I am not sure if I have the time to maintain this, but I could give it a try.

A tag on the PRs like distribution: nix would help a lot.

@Cu3PO42
Copy link

Cu3PO42 commented May 9, 2024

I also currently use the Flake and love having it here, that said, I also follow all arguments listed above against having it here.

More specifically for nix, I wonder if there is better automation possible.
[...]
Is that what https://github.com/wez/wezterm/blob/main/.github/workflows/update-flake.yml is intended to do?

This workflow updates any of the Flake's inputs. Currently these are

  • nixpkgs, the main upstream package repository, from which system libraries like Wayland, X11 are pulled,
  • rust-overlay, where the relevant version of Rust is obtained from,
  • flake-utils, a small utility library that essentially never updates and could be removed if required,
  • freetype2,
  • harfbuzz,
  • libpng,
  • zlib.

The last four are those dependencies that are usually included via a submodule. Unfortunately, they will not be touched by the workflow, which is designed to work for references to branches rather than specific commits or tags. For example, nixpkgs is included via the nixpkgs-unstable (unstable as in Debian unstable, not as in broken) branch, but the flake.lock pins it to a particular commit. The workflow will update to a newer commit. Since the last four libraries are included via a particular commit, they are not touched.

The workflow is currently failing because the Flake lives in the nix/ subdirectory and the action wasn't configured for this.

  with:
            pr-title: 'Update flake.lock' # Title of PR to be created
            pr-labels: | # Labels to be set on the PR
              dependencies
              automated
            pr-assignees: happenslol,gabyx,emiller88
+            path-to-flake-dir: 'nix/'

should be the required fix.


I took a look at the history of the flake.nix and tried to identify which changes were made in response to breakages. All of the ones that seem relevant to regular maintenance were updates of included commits or hash updates. I do believe these could be automated to a higher degree, but I'm not sure there's off-the-shelf tooling for it available right now and introducing more code doesn't seem like a step in the right direction in this instance.

As an immediate step to reduce the instances in which this happens, we could try to remove some of these inputs. nixpkgs includes versions of libpng, zlib, and harfbuzz that are at least as new as the ones that are currently explicitly included. Of course this means Nix users would have slightly different builds, leading to potentially slightly different issues. I see how that would be less than ideal as well.

The other instances in which hash changes occured were in the context of Cargo dependencies on versions downloaded via Git. These are necessary every time you add or update a dependency on a Git repo. This can, unfortunately not be avoided but could also be automated with the right tooling. Unfortunately I'm not aware of any right now.

@mirror-kt
Copy link
Contributor

If you are interested in transferring flake to org and managing it, I would be happy to help.

By the way, it may be a good idea to enable allowBuiltinFetchGit, although it may sacrifice some definitive buildability. (See here).
My understanding is that an option to let cargo, not nix, retrieve the git-specified crate, so there is no need to maintain outputHashes on the nix side.

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.

None yet

6 participants