`tmux -d -x ... -y ...` does not respect sizing

[Anthony Sottile]

[Impact]
========

I use tmux for testing a CLI app on different terminal sizes. I noticed that after upgrading to 3.2a from 3.2, the -x and -y parameters I pass to tmux new-session -d does not take effect.

The below command outputs are from terminals with the same size:

$ tmux -V
tmux 3.2a
$ rm bar; tmux new-session -d -x 20 -y 10 'tput cols > bar'; sleep 0.1; cat bar
174

$ tmux -V
tmux 3.2
$ rm bar; tmux new-session -d -x 20 -y 10 'tput cols > bar'; sleep 0.1; cat bar
20

Also, the default size has also changed, where 3.2 always spawns a detached window of width 80 regardless of the terminal size, however 3.2a uses the width of the terminal I used to spawn it.

[Test Plan]
===========

You can reproduce this pretty easily with:

$ lxc launch images:ubuntu/jammy jtemp --vm
# apt upadte && apt install tmux
### change to a user - not really needed but I have a user configured.
$ tmux -V
$ tmux new-session -d -x 20 -y 20 bash
$ tmux send-keys 'tput cols'
$ tmux send-keys 'Enter'
$ sleep 1
$ tmux capture-pane -pt0 | grep -Eo '^[0-9]+'
$ tmux kill-session

on 20.04, I get the following:

```console
$ bash t.sh tmux 3.0a
20
```

on 22.04 I get the following (my parent window is 118 wide):

```console
$ bash t.sh
tmux 3.2a
118
```

[Regression Potential]
======================

There might be a corner case where a user might face a regression if they have done some workarounds to this issue but that should be really minimal (if not zero). The patch is really trivial and is really targeted (see the if clause) so the chances of actual regression is also really minimal.

[Other Information]
===================

I've gone ahead and bisected to find the patch that's needed to fix this -- it was originally committed here: https://github.com/tmux/tmux/commit/df3fe2aa72da0555106c6187e750418f0e59d901

applying that to the packaging of 3.2a should be pretty straightforward, I've verified that applying it fixes the problem:

```console
$ git checkout 3.2a
$ git cherry-pick df3fe2aa72da0555106c6187e750418f0e59d901
$ ./autogen.sh >& /dev/null && ./configure --prefix=$PWD/prefix >& /dev/null && make -j5 >& /dev/null && make install >& /dev/null && PATH=/tmp/tmux/prefix/bin:$PATH bash ../t.sh
tmux 3.2a
20
```

so all that should be needed is to apply that patch:

```diff
commit df3fe2aa72da0555106c6187e750418f0e59d901 (refs/bisect/skip-df3fe2aa72da0555106c6187e750418f0e59d901)
Author: Nicholas Marriott <email address hidden>
Date: Tue Jul 13 10:38:57 2021 +0000

    Only use client for sizing when not detached, GitHub issue 2772.

diff --git a/cmd-new-session.c b/cmd-new-session.c
index 666aeaac..033c707f 100644
--- a/cmd-new-session.c
+++ b/cmd-new-session.c
@@ -280,7 +280,8 @@ cmd_new_session_exec(struct cmd *self, struct cmdq_item *item)
        memset(&sc, 0, sizeof sc);
        sc.item = item;
        sc.s = s;
- sc.tc = c;
+ if (!detached)
+ sc.tc = c;

        sc.name = args_get(args, 'n');
        sc.argc = args->argc;

```

thanks!

[Anthony Sottile (asottile)]

I have made a PPA including that patch in case anyone wants to try it out:

- source code here: https://github.com/asottile/tmux-jammy
- PPA here: https://launchpad.net/~asottile/+archive/ubuntu/tmux-jammy

[Utkarsh Gupta (utkarsh)]

Thank you, Anthony. For reporting this and also finding the required patch. I've assigned this to myself and will get to it in sometime, hopefully. :)

[Launchpad Janitor (janitor)]

This bug was fixed in the package tmux - 3.3-2

---------------
tmux (3.3-2) unstable; urgency=medium

  * Cherry-pick fixes from upstream Git:
    + 0f6227f46b: don't silently delete or rename the most recent buffer
    + 18838fbc87: fix crash when using `run-shell' in config file
    + 3edda3c5e7: do not unintentionally turn off all mouse mode
  * Remove obsolete field Name from debian/upstream/metadata.
  * Upload to unstable.

 -- Romain Francoise <email address hidden> Sat, 04 Jun 2022 17:08:29 +0200

[Anthony Sottile (asottile)]

just curious -- will this land in jammy?

[Robie Basak (racb)]

It's possible, but does need some effort to ensure we don't regress tmux in an unrelated way for unaffected users. Full details on the process are at https://wiki.ubuntu.com/StableReleaseUpdates

Would you like to volunteer to drive this process to get tmux fixed in Jammy?

If so, please comment with a justification against https://wiki.ubuntu.com/StableReleaseUpdates#When and complete steps 1 through 4 in https://wiki.ubuntu.com/StableReleaseUpdates#Procedure - and go ahead with all the steps if you can. Note that that SRU team would need to make a final decision.

[Anthony Sottile (asottile)]

yeah -- I did the same process for https://bugs.launchpad.net/ubuntu/+source/tmux/+bug/1875109 (or actually, looks like someone did it for me) -- I tried to include all of the required parts in the original report here (reproduction, rationalization, patch) -- let me know what's missing!

[Bryce Harrington (bryce)]

@Anthony, you've got a thorough looking reproduction, and sounds like the patch was acceptable, and that's great you've had experience following the process on LP: #1875109. To your question on what is still needed here, it looks like step 3 from the link Robie presented would need attention next; i.e. adding an [Impact] section and [Where problems could occur].

For step #4, it looks like you've done the lion's share of the work for that via the PPA you provided in comment #1. The version number for this SRU to jammy needs to be 3.2a-4ubuntu1.1, and the changelog text should include a reference to this bug #, and a bit more explanation of the change itself; again you can look at the changelog from https://code.launchpad.net/~sergiodj/ubuntu/+source/tmux/+git/tmux/+merge/384181 to get a feel for the formating and level of detail expected.

Again, thanks for already going above and beyond, the above steps aren't expected of you but if you do tackle them it will speed the processing of this fix.

[Bryce Harrington (bryce)]

A couple other notes (these are minor) from looking at the package diff in your PPA:

- For an SRU, I'd recommend not refreshing the other patches (namely upstream-b1a8c0fe022e.diff, upstream-32f2d9d089ce.diff, and platform-quirks.diff) assuming the package builds fine without those refreshed. If you have a good reason to refresh them, that's fine, just include an explanation why in the changelog.

- Not at all required, but we often include additional header metadata in new patches. E.g. Author, Origin, Bug-Ubuntu, etc.) You can see the patch from https://code.launchpad.net/~sergiodj/ubuntu/+source/tmux/+git/tmux/+merge/384181 as an example. Again, not required for SRU, but good practice.

[Anthony Sottile (asottile)]

ah yeah for my PPA I didn't really make that as a "this is the patch formatted nicely for canonical" -- I just cherry-picked using gbp and refreshed the patches using that. that's why the commit and such are all the default values! but yeah totally happy with whatever version number it ends up with here!

[Impact] the patch adjusts how the client is attached to the newly created tmux session. earlier in the code the client is detected as being detached or not. the regression that occurred in tmux itself forgot to take into account the detached state when creating a new session. the patch is essentially the upstream fix for handling detached sessions

[Where problems could occur] the change is mostly around creating tmux sessions and creating detached tmux sessions. that would be where I would expect regressions to occur -- as unlikely as that seems

[Anthony Sottile (asottile)]

any updates on this? my fork has been working well but I'd like to not need it at some point!

[Utkarsh Gupta (utkarsh)]

Hello,

Sorry it took a while but I've added more details to the bug as per the SRU template and have prepped the update as well. MP should soon be auto-linked to the bug. Thanks for your patience and apologies for the delay. TIA.

[Andreas Hasenack (ahasenack)]

Hello Anthony, or anyone else affected,

Accepted tmux into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/tmux/3.2a-4ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Utkarsh Gupta (utkarsh)]

Hello,

Performing the verification:

$ lxc launch images:ubuntu/jammy jtemp --vm
$ lxc shell jtemp
# apt upadte && apt install tmux
### change to a user - not really needed but I have a user configured.
$ tmux -V
>> tmux 3.2a
$ apt show tmux | grep Version
>> Version: 3.2a-4
$ tmux new-session -d -x 20 -y 20 bash
$ tmux send-keys 'tput cols'
$ tmux send-keys 'Enter'
$ sleep 1
$ tmux capture-pane -pt0 | grep -Eo '^[0-9]+'
>> 118
$ tmux kill-session

## Enabling the -proposed repository.
$ sudo apt update && sudo apt upgrade
$ tmux -V
>> tmux 3.2a
$ apt show tmux | grep Version
>> Version: 3.2a-4ubuntu0.1
$ tmux new-session -d -x 20 -y 20 bash
$ tmux send-keys 'tput cols'
$ tmux send-keys 'Enter'
$ sleep 1
$ tmux capture-pane -pt0 | grep -Eo '^[0-9]+'
>> 20
$ tmux kill-session

Therefore, the fix works as expected. Thus marking the bug as verified. \o/

[Anthony Sottile (asottile)]

I have also verified the fix -- thanks!

[Ubuntu SRU Bot (ubuntu-sru-bot)]

All autopkgtests for the newly accepted tmux (3.2a-4ubuntu0.1) for jammy have finished running.
The following regressions have been reported in tests triggered by the package:

tmux/3.2a-4ubuntu0.1 (ppc64el)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/jammy/update_excuses.html#tmux

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

[Utkarsh Gupta (utkarsh)]

Re-triggered the failing autopkgtest and they're now passing. So no real regression, really. \o/

[Launchpad Janitor (janitor)]

This bug was fixed in the package tmux - 3.2a-4ubuntu0.1

---------------
tmux (3.2a-4ubuntu0.1) jammy; urgency=medium

  * d/p/lp1976110-respect-sizing.diff: Add patch to only
    use client for sizing when not detached. (LP: #1976110)

 -- Utkarsh Gupta <email address hidden> Wed, 23 Nov 2022 17:40:50 +0530

[Brian Murray (brian-murray)]

The verification of the Stable Release Update for tmux has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.