zellij: 0.40.1 -> 0.41.1

[r-vdp]

https://github.com/zellij-org/zellij/releases/tag/v0.41.0

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[jorikvanveen]

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 353630

---

x86_64-linux

✅ 1 package built:

• zellij

[a-kenji]

Maintainers should be aware of the following:

Zellij 0.41.0 forces vendored libcurl dependency by default

Source:
zellij-org/zellij#3720

[thegrubster]

There has been a new patch release due to some issues with the generic binaries in 0.41, although I am unsure if this is relevant for nix

https://github.com/zellij-org/zellij/releases/tag/v0.41.1

[a-kenji]

Thank you @thegrubster,

There has been a new patch release due to some issues with the generic binaries in 0.41, although I am unsure if this is relevant for nix

Yes, this seems relevant.

[tjni]

Maybe we can patch zellij-utils/Cargo.toml to depend on isahc without the static-curl feature.

[nadir-ishiguro]

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 353630

---

x86_64-linux

✅ 1 package built:

• zellij

[wahjava]

Looks good on aarch64-linux.

[r-vdp]

Maybe we can patch zellij-utils/Cargo.toml to depend on isahc without the static-curl feature.

I actually tried this, but it doesn't seem to work. I added

  postPatch = ''
    substituteInPlace zellij-utils/Cargo.toml \
      --replace-fail 'isahc = "1.7.2"' 'isahc = { version = "1.7.2", default-features = false, features = ["http2", "text-decoding"] }'
  '';

But I'm still not seeing curl as a dependency:

➜ nix-store --query --references result/
/nix/store/3bvxjkkmwlymr0fssczhgi39c3aj1l7i-glibc-2.40-36
/nix/store/m8gwqmn8k3jm0gbcia358mz4y00lgmbc-openssl-3.3.2
/nix/store/s94fwp43xhzkvw8l8nqslskib99yifzi-gcc-13.3.0-lib

➜ ldd ./result/bin/zellij
        linux-vdso.so.1 (0x00007f81611ab000)
        libssl.so.3 => /nix/store/m8gwqmn8k3jm0gbcia358mz4y00lgmbc-openssl-3.3.2/lib/libssl.so.3 (0x00007f815f2f4000)
        libcrypto.so.3 => /nix/store/m8gwqmn8k3jm0gbcia358mz4y00lgmbc-openssl-3.3.2/lib/libcrypto.so.3 (0x00007f815ec00000)
        libc.so.6 => /nix/store/3bvxjkkmwlymr0fssczhgi39c3aj1l7i-glibc-2.40-36/lib/libc.so.6 (0x00007f815ea07000)
        libgcc_s.so.1 => /nix/store/s94fwp43xhzkvw8l8nqslskib99yifzi-gcc-13.3.0-lib/lib/libgcc_s.so.1 (0x00007f8161180000)
        libm.so.6 => /nix/store/3bvxjkkmwlymr0fssczhgi39c3aj1l7i-glibc-2.40-36/lib/libm.so.6 (0x00007f815f20d000)
        libdl.so.2 => /nix/store/3bvxjkkmwlymr0fssczhgi39c3aj1l7i-glibc-2.40-36/lib/libdl.so.2 (0x00007f815f206000)
        libpthread.so.0 => /nix/store/3bvxjkkmwlymr0fssczhgi39c3aj1l7i-glibc-2.40-36/lib/libpthread.so.0 (0x00007f815f201000)
        /nix/store/3bvxjkkmwlymr0fssczhgi39c3aj1l7i-glibc-2.40-36/lib/ld-linux-x86-64.so.2 => /nix/store/3bvxjkkmwlymr0fssczhgi39c3aj1l7i-glibc-2.40-36/lib64/ld-linux-x86-64.so.2 (0x00007f81611ad000)

Am I holding this wrong?

[tjni]

Does it need curl in buildInputs?

[tjni]

I verified that this is needed on darwin at least (due to zellij-org/zellij#3668), but I'm having some trouble building this on aarch64-linux through the darwin linux builder (I don't have access to a Linux machine at the moment).

[r-vdp]

Does it need curl in buildInputs?

Yeah, I didn't mention it but I also added curl to the build inputs of course :)

[tjni]

It looks like curl-sys looks for curl on Linux using pkg-config, so one thing we could try is depending on curl.dev in buildInputs.

[r-vdp]

@tjni I also just found that by looking at what other packages in nixpkgs are doing, it's working now!

[r-vdp]

It would be helpful if anyone could test this on darwin also.

[SuperSandro2000]

For curl-config, right?

[r-vdp]

So that cargo finds curl, using pkg-config. Without this, it doesn't.

[tjni]

I think we can go with this for now, but I think cross-compilation might be broken.

1. curl-sys uses https://docs.rs/pkg-config/latest/pkg_config/#cross-compilation, which seems to require setting certain environment variables for cross-compilation use case.
2. we need it to use at build-time the .pc files from curl used at runtime, so how do we do that?

I don't have enough experience in cross-compilation to know how to do this. Maybe @Artturin or someone else can help out here (perhaps as a follow up to this PR)?

Note: This is technically not needed on Darwin since pkg-config is not used on Darwin to find curl. It does seem to be used on Darwin and all platforms to find OpenSSL.

[r-vdp]

Would you make its inclusion conditional here, or do we consider that it doesn't matter to include this path in the sandbox also on darwin and it will simply not be used then?

For the cross-compilation, I also don't have much experience with this.

[tjni]

I've been thinking about it a little more, and I think we should just keep it like this for now. There's probably a nice solution for this somewhere, but I'll mull it over some more separately.

[tjni]

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 353630

---

aarch64-darwin

✅ 1 package built:

• zellij

[tjni]

LGTM for Darwin

[khaneliman]

Does 0.41.1 work without 21de802 and did we not need it before the version bump? Otherwise that should probably be squashed into the first commit if it's unique to the version bump to avoid a commit that technically is breaking a package (Doesn't truly matter, just a nit to think about for atomic commits.). Otherwise, LGTM.

[khaneliman]

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 353630

---

x86_64-linux

✅ 1 package built:

• zellij

---

aarch64-linux

✅ 1 package built:

• zellij

---

x86_64-darwin

✅ 1 package built:

• zellij

---

aarch64-darwin

✅ 1 package built:

• zellij

[r-vdp]

Does 0.41.1 work without 21de802 and did we not need it before the version bump? Otherwise that should probably be squashed into the first commit if it's unique to the version bump to avoid a commit that technically is breaking a package (Doesn't truly matter, just a nit to think about for atomic commits.). Otherwise, LGTM.

It does work, it just has a vendored curl binary, which is not ideal in terms of security updates.
I'd like to keep the commits separate, since they are kind of orthogonal concerns. But I also don't feel very strongly either way.

[khaneliman]

Does 0.41.1 work without 21de802 and did we not need it before the version bump? Otherwise that should probably be squashed into the first commit if it's unique to the version bump to avoid a commit that technically is breaking a package (Doesn't truly matter, just a nit to think about for atomic commits.). Otherwise, LGTM.

It does work, it just has a vendored curl binary, which is not ideal in terms of security updates. I'd like to keep the commits separate, since they are kind of orthogonal concerns. But I also don't feel very strongly either way.

If it works without it, that's fine to me.