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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

tmux: add proposed upstream fix for split-window regression #289789

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

alois31
Copy link
Contributor

@alois31 alois31 commented Feb 18, 2024

In tmux 3.4, a regression was introduced that breaks the deprecated (but still supported for backwards compatibility) -p option to split-window. Several external tools (for example fzf.kak, fzf-tmux and nnn) break due to this. Add the proposed upstream fix from tmux/tmux#3840.

Description of changes

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:
  • 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.05 Release Notes (or backporting 23.05 and 23.11 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.

In tmux 3.4, a regression was introduced that breaks the deprecated (but still
supported for backwards compatibility) -p option to split-window. Several
external tools (for example fzf.kak, fzf-tmux and nnn) break due to this. Add
the proposed upstream fix from tmux/tmux#3840.
@alois31 alois31 marked this pull request as ready for review February 18, 2024 17:59
Copy link
Contributor

@x10an14 x10an14 left a comment

Choose a reason for hiding this comment

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

If this way of testing is equivalent/accepted, then yes, this works for me on 1xx86_64-linux NixOS (the config change/patch was introduced through home-manager).

diff --git a/home-manager/programs/terminal/tmux.nix b/home-manager/programs/terminal/tmux.nix
index aae6d8f..46836cf 100644
--- a/home-manager/programs/terminal/tmux.nix
+++ b/home-manager/programs/terminal/tmux.nix
@@ -53,4 +53,23 @@ in {
       '')
     ];
   };
+  nixpkgs.overlays = [
+    (final: prev: {
+      tmux = prev.tmux.overrideAttrs (old: {
+        patches =
+          (old.patches or [])
+          ++ [
+            (
+              # For context, see:
+              # https://github.com/NixOS/nixpkgs/pull/289789/files
+              # TODO: Remove when fixed
+              pkgs.fetchpatch {
+                url = "https://github.com/tmux/tmux/commit/2d1afa0e62a24aa7c53ce4fb6f1e35e29d01a904.diff";
+                hash = "sha256-mDt5wy570qrUc0clGa3GhZFTKgL0sfnQcWJEJBKAbKs=";
+              }
+            )
+          ];
+      });
+    })
+  ];
 }

EDIT: Formatting

@otavio otavio requested a review from NickCao February 19, 2024 19:14
otavio added a commit to otavio/nix-config that referenced this pull request Feb 19, 2024
Refs: NixOS/nixpkgs#289789
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
otavio added a commit to otavio/nix-config that referenced this pull request Feb 19, 2024
Refs: NixOS/nixpkgs#289789
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
otavio added a commit to otavio/nix-config that referenced this pull request Feb 19, 2024
Refs: NixOS/nixpkgs#289789
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
otavio added a commit to otavio/nix-config that referenced this pull request Feb 19, 2024
Refs: NixOS/nixpkgs#289789
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
@otavio
Copy link
Contributor

otavio commented Feb 19, 2024

I can confirm that this solution worked for me as well, resolving the issue.

@fpletz fpletz merged commit 7558558 into NixOS:master Feb 20, 2024
28 checks passed
@x10an14
Copy link
Contributor

x10an14 commented Feb 20, 2024

Are we certain we wanted to merge this? What mechanism is in place for this not just breaking once the tmux package gets upgraded in nixpkgs next time?

We trust in the people performing the upgrade testing the software before pushing PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants