Package: release.debian.org
Severity: normal
User: release.debian....@packages.debian.org
Usertags: unblock

Dear Release Team, Dear D-I Release Manager,

please unblock package screen/4.5.0-6.

Daniel Kahn Gillmor (beside others upstream) noticed that in 4.5.0 the
-L commandline option was subtly broken in a way that -L is documented
to have an optional parameter, but in fact, the parameter becomes
non-optional if further commandline options (i.e. parameters are
starting with a dash) are given as the next parameter after -L (if
existing) is unconditionally treated as log file name and screen bails
out if a log file name starts with a dash.

This leads to breakage in screen's commandline API: Some option
combinations are no more possible (due to potential positional
parameters after all options) without explicitly giving an log file
name as parameter to -L.

But in previous screen versions -L didn't accept any parameter, so any
potential parameter would have been treated like a positional
parameter. This makes screen behaving severly different with the same
parameters depending on the version. Namely it works as expected in
all versions except 4.5.0 since upstream has fixed that API breakage
in 4.5.1 (already in experimental).

So I cherry-picked the first and simplest commit in 4.5.1 targeting
this issue. It was later rewritten for the final 4.5.1 release to add
additional options and further logic, but it already fixed most of the
API breakage. While testing the patch I noticed that upstream forgot
to revert anticipatorily incremented/decremented counters (ac, av) if
the next argument starts with a dash. I've added two lines ("av--;"
and "ac++;") and that's the only difference between the patch in
4.5.0-6 and upstream's variant at
http://git.savannah.gnu.org/cgit/screen.git/commit/?h=screen-v4&id=c14e05e7c36c64d85198ed0fc89177427ece48d4

I must admit, I haven't tested it inside D-I, but since I haven't
heard of any D-I breakage due to that unplanned CLI API change in
4.5.0, I don't expect any breakage when I'm fixing that API breakage.

Full debdiff between 4.5.0-5 in testing and 4.5.0-6 in unstable:

diff -Nru screen-4.5.0/debian/changelog screen-4.5.0/debian/changelog
--- screen-4.5.0/debian/changelog       2017-04-04 01:14:01.000000000 +0200
+++ screen-4.5.0/debian/changelog       2017-05-23 01:57:09.000000000 +0200
@@ -1,3 +1,12 @@
+screen (4.5.0-6) unstable; urgency=low
+
+  * Cherry-pick c14e05e7 to fix -L parsing regression. (Closes: #863095)
+    + Modify patch to revert anticipatorily incremented/decremented
+      counters if next argument starts with a dash.
+    + Refresh line-numbers in 80_session_creation_docs.patch.
+
+ -- Axel Beckert <a...@debian.org>  Tue, 23 May 2017 01:57:09 +0200
+
 screen (4.5.0-5) unstable; urgency=low
 
   * Replace all occurrences of /var/run/ in packaging with /run/. (Closes:
diff -Nru 
screen-4.5.0/debian/patches/64-cherry-pick-c14e05e7-to-fix-cli-api-regression.patch
 
screen-4.5.0/debian/patches/64-cherry-pick-c14e05e7-to-fix-cli-api-regression.patch
--- 
screen-4.5.0/debian/patches/64-cherry-pick-c14e05e7-to-fix-cli-api-regression.patch
 1970-01-01 01:00:00.000000000 +0100
+++ 
screen-4.5.0/debian/patches/64-cherry-pick-c14e05e7-to-fix-cli-api-regression.patch
 2017-05-23 01:56:25.000000000 +0200
@@ -0,0 +1,65 @@
+Origin: c14e05e7c36c64d85198ed0fc89177427ece48d4
+Author: Alexander Naumov <alexander_nau...@opensuse.org>
+Description: Ignore logfile's name that begins with the "-" symbol
+ This fixes the API:
+ .
+ To enable logging we use -L option. But in case of
+ default logfile name (screenlog.0) we will need to
+ define it anyway. Because screen will try to interpret
+ next option as a parameter for -L option (which is
+ logfile name). It will fails ALWAYS, because next
+ parameter will always start with "-" symbol...
+ what is not permited for logfile name of course.
+ .   
+ For example:
+ .
+ $ screen -L -D -m ./configure
+ .
+ In this case logfile name is screenlog.0, because "-D"
+ will not be interpreted by screen as a name of logfile.
+Bug-Debian: https://bugs.debian.org/863095
+Bug: https://savannah.gnu.org/bugs/?50440
+Reviewd-By: Axel Beckert <a...@debian.org>
+
+--- a/doc/screen.1
++++ b/doc/screen.1
+@@ -262,8 +262,8 @@
+ tells
+ .I screen
+ to turn on automatic output logging for the windows. By default, logfile's 
name
+-is screenlog.1. You can sets new name: add it right after -L option e.g. 
"screen
+--L my_logfile".
++is screenlog.0. You can set new name: add it right after -L option e.g. 
"screen
++-L my_logfile". Keep in mind that name can not start with "-" symbol.
+ .TP 5
+ .B \-m
+ causes
+--- a/doc/screen.texinfo
++++ b/doc/screen.texinfo
+@@ -334,7 +334,9 @@
+ 
+ @item -L
+ Tell @code{screen} to turn on automatic output logging for the
+-windows.
++windows. By default, logfile's name is screenlog.0. You can set new name:
++add it right after -L option e.g. "screen -L my_logfile". Keep in mind
++that name can not start with "-" symbol.
+ 
+ @item -m
+ Tell @code{screen} to ignore the @code{$STY} environment variable.  When
+--- a/screen.c
++++ b/screen.c
+@@ -669,8 +669,11 @@
+           case 'L':
+             if (--ac != 0) {
+               screenlogfile = SaveStr(*++av);
+-              if (screenlogfile[0] == '-')
+-                Panic(0, "-L: logfile name can not start with \"-\" symbol");
++              if (screenlogfile[0] == '-') {
++                screenlogfile = SaveStr("screenlog.%n");
++                av--;
++                ac++;
++              }
+               if (strlen(screenlogfile) > PATH_MAX)
+                 Panic(0, "-L: logfile name too long. (max. %d char)", 
PATH_MAX);
+             }
diff -Nru screen-4.5.0/debian/patches/80_session_creation_docs.patch 
screen-4.5.0/debian/patches/80_session_creation_docs.patch
--- screen-4.5.0/debian/patches/80_session_creation_docs.patch  2017-04-03 
01:09:40.000000000 +0200
+++ screen-4.5.0/debian/patches/80_session_creation_docs.patch  2017-05-23 
01:56:25.000000000 +0200
@@ -54,7 +54,7 @@
  with @code{screen -r}.  Those marked @samp{attached} are running and
  have a controlling terminal.  If the session runs in multiuser mode,
  it is marked @samp{multi}.  Sessions marked as @samp{unreachable} either
-@@ -410,14 +411,15 @@
+@@ -412,14 +413,15 @@
  when only one @code{screen} is detached. Otherwise lists available sessions.
  
  @item -RR
diff -Nru screen-4.5.0/debian/patches/series screen-4.5.0/debian/patches/series
--- screen-4.5.0/debian/patches/series  2017-04-03 01:09:40.000000000 +0200
+++ screen-4.5.0/debian/patches/series  2017-05-23 01:56:25.000000000 +0200
@@ -12,6 +12,7 @@
 61-default-PATH_MAX-if-undefined-for-hurd.patch
 62-reverse-cherry-pick-5460f5d2-to-fix-privilege-escalation.patch
 63-fix-garbage-on-serial-terminal.patch
+64-cherry-pick-c14e05e7-to-fix-cli-api-regression.patch
 # 80-99: experimental patches, new features etc.
 80_session_creation_docs.patch
 81_session_creation_util.patch


So please ...

unblock screen/4.5.0-6

-- System Information:
Debian Release: 9.0
  APT prefers unstable
  APT policy: (990, 'unstable'), (600, 'testing'), (500, 'unstable-debug'), 
(500, 'buildd-unstable'), (110, 'experimental'), (1, 'experimental-debug'), (1, 
'buildd-experimental')
Architecture: amd64
 (x86_64)

Kernel: Linux 4.9.0-1-amd64 (SMP w/8 CPU cores)
Locale: LANG=C.UTF-8, LC_CTYPE=C.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)

Reply via email to