[Bug 833855] New: Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

Bug ID: 833855
        QA Contact: [e-mail]
          Severity: medium
           Version: rawhide
          Priority: medium
                CC: [e-mail],
                    [e-mail]
          Assignee: [e-mail]
           Summary: Review Request: console-setup - Tools for configuring
                    the console using X Window System keymaps
        Regression: ---
      Story Points: ---
    Classification: Fedora
                OS: Linux
          Reporter: [e-mail]
              Type: ---
     Documentation: ---
          Hardware: All
        Mount Type: ---
            Status: NEW
         Component: Package Review
           Product: Fedora

Spec URL: http://vcrhonek.fedorapeople.org/console-setup/console-setup.spec
SRPM URL:
http://vcrhonek.fedorapeople.org/console-setup/console-setup-1.76-1.fc17.src...
Description: This package provides the console with the same keyboard
configuration scheme that X Window System has. Besides the keyboard, the
package configures also the font on the console. It includes a rich collection
of fonts and supports several languages that would be otherwise unsupported on
the console (such as Armenian, Georgian, Lao and Thai).
Fedora Account System Username: vcrhonek

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

Jef Spaleta  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Jef Spaleta  ---
First comment ahead of formal review.  The package contains a mismash of
different licensed content.

Since the binary package includes shell script executable named setupcon which
from the COPYRIGHT file and from examination of the script itself is licensed
under MIT.

GPLv2+ and MIT and Public Domain  seems to be most appropriate for the spec
license field.

-jef

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

Jef Spaleta  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
              Flags|                            |fedora-review?

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

--- Comment #2 from Jef Spaleta  ---
Summary:
Fix items 1 and 2 below and I'll be able to accept it.

Items that need to be addressed:

1) License field needs to be updated to read
GPLv2+ and MIT and Public Domain  due to license on the setupcon script.
This is a blocker

2) No need to for the package to own /etc/default
/etc/default is owned by glibc  which is part of the requirement chain through
the requirement on the kdb package. All other directory ownership looks good.
This is a blocker

Other items
3) rpmlint against the rpm has one E associated with incorrect FSF address from
the ckbcomp utility boilerplate. You are encouraged to notify the upstream for
this package to fix it, and you can correct it with a patch in your
package..but are not required to do so.

4) The supplied manpages still refer to /usr/local/etc/default instead of
/etc/default/ in multiple place. See if you can enhance the paths patch to fix
that.

5) I dont understand the notes in the spec concerning xkeyboard-config as a
possible requirement. Can you explain to me what the problem is in a little
more detail? Not a blocker, but I don't have enough info to make a
recomendation.

6)
setupcon -v  gives me some interesting feedback concerning keymap deny errors
for the loadkeys operation which suggests to me that the loadkeys operation its
not working as expected. Not sure what's going on there..but its not falling
over with a crash condition. You want to sort this out as part of this review?
I can supply the output on my F16 system.

Other Must checklist items:
rpmlint: console-setup-1.76-1.fc16.noarch.rpm 
console-setup.noarch: W: spelling-error Summary(en_US) keymaps -> key maps,
key-maps, makeups
console-setup.noarch: E: incorrect-fsf-address /usr/bin/ckbcomp

source checksum verifies
upstream checksum is:
38cbc433c40d80f164097e7acaf57b3e  console-setup_1.76.tar.gz

applied patch to fix up file paths seems reasonable.  There might be a more
clever way to do that patch, but that's not a blocker for inclusion.

package naming looks good according to guidelines.

Compiles cleanly on F16 and against koji scratch build

no locale files
no libraries
no devel files
no large docs
docs looks good

spec file is legible and has consistent use of macros.

setupcon  appears to run without faulting on a console tty

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

Jef Spaleta  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|[e-mail]    |[e-mail]

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

--- Comment #3 from Vitezslav Crhonek  ---
(In reply to comment #2)

Thanks Jef, I updated the files in
http://vcrhonek.fedorapeople.org/console-setup/
> Summary:
> Fix items 1 and 2 below and I'll be able to accept it.
>
> Items that need to be addressed:
> License field needs to be updated to read
>
> GPLv2+ and MIT and Public Domain  due to license on the setupcon script.
> This is a blocker

Fixed.
> No need to for the package to own /etc/default
>
> /etc/default is owned by glibc  which is part of the requirement chain
> through the requirement on the kdb package. All other directory ownership
> looks good.
> This is a blocker

Fixed.
> Other items
> 3) rpmlint against the rpm has one E associated with incorrect FSF address
> from the ckbcomp utility boilerplate. You are encouraged to notify the
> upstream for this package to fix it, and you can correct it with a patch in
> your package..but are not required to do so.

Fixed and upstream contacted.
> The supplied manpages still refer to /usr/local/etc/default instead of
>
> /etc/default/ in multiple place. See if you can enhance the paths patch to
> fix that.

Fixed. I also reviewed the manpages and add paths to
consolefonts/consoletrans/keymaps shipped within kbd package.
> I dont understand the notes in the spec concerning xkeyboard-config as a
>
> possible requirement. Can you explain to me what the problem is in a little
> more detail? Not a blocker, but I don't have enough info to make a
> recomendation.

Main purpose of adding this package is to unify keymaps between X Window System
and text console, see:
https://bugzilla.redhat.com/show_bug.cgi?id=680990

X Window System keymaps are shipped in xkeyboard-config package and
console-setup should use it by default to achieve the goal.

Upstream tarball of console-setup contains additional set of X Window System
keymaps, which I removed in spec file (ckbcomp would prefer them in another
case), but maybe they could be useful for someone, so we can consider to have
it in optional subpackage.
> setupcon -v  gives me some interesting feedback concerning keymap deny
> errors for the loadkeys operation which suggests to me that the loadkeys
> operation its not working as expected. Not sure what's going on there..but
> its not falling over with a crash condition. You want to sort this out as
> part of this review?
> I can supply the output on my F16 system.

Sure, please let me see the output. Works fine for me with default
configuration on F17 system:

# setupcon -v
Can not find the active virtual consoles, assuming ACTIVE_CONSOLES="/dev/tty1
/dev/tty2 /dev/tty3 /dev/tty4 /dev/tty5 /dev/tty6"
The charmap is UTF-8
BackSpace is ^?
Executing utf_start /dev/tty1.
Configuring /dev/tty1 in Unicode mode.
Executing utf_start /dev/tty2.
Configuring /dev/tty2 in Unicode mode.
Executing utf_start /dev/tty3.
Configuring /dev/tty3 in Unicode mode.
Executing utf_start /dev/tty4.
Configuring /dev/tty4 in Unicode mode.
Executing utf_start /dev/tty5.
Configuring /dev/tty5 in Unicode mode.
Executing utf_start /dev/tty6.
Configuring /dev/tty6 in Unicode mode.
Loading 512-char 8x16 font from file
/usr/share/consolefonts/Uni2-TerminusBold16.psf.gz
Loading Unicode mapping table...
setfont: graphics console /dev/tty2 skipped
Loading 512-char 8x16 font from file
/usr/share/consolefonts/Uni2-TerminusBold16.psf.gz
Loading Unicode mapping table...
Loading 512-char 8x16 font from file
/usr/share/consolefonts/Uni2-TerminusBold16.psf.gz
Loading Unicode mapping table...
Loading 512-char 8x16 font from file
/usr/share/consolefonts/Uni2-TerminusBold16.psf.gz
Loading Unicode mapping table...
Loading 512-char 8x16 font from file
/usr/share/consolefonts/Uni2-TerminusBold16.psf.gz
Loading Unicode mapping table...
For /dev/tty1:
Executing kbd_mode -u.
For /dev/tty2:
Executing kbd_mode -u.
For /dev/tty3:
Executing kbd_mode -u.
For /dev/tty4:
Executing kbd_mode -u.
For /dev/tty5:
Executing kbd_mode -u.
For /dev/tty6:
Executing kbd_mode -u.
Executing loadkeys /tmp/tmpkbd.Iyr2Zn.
Loading /tmp/tmpkbd.Iyr2Z
> Other Must checklist items:
> rpmlint: console-setup-1.76-1.fc16.noarch.rpm 
> console-setup.noarch: W: spelling-error Summary(en_US) keymaps -> key maps,
> key-maps, makeups
> console-setup.noarch: E: incorrect-fsf-address /usr/bin/ckbcomp

I changed it to key maps to make rpmlint happy, it probably doesn't matter.
> source checksum verifies
> upstream checksum is:
> 38cbc433c40d80f164097e7acaf57b3e  console-setup_1.76.tar.gz
>
> applied patch to fix up file paths seems reasonable.  There might be a more
> clever way to do that patch, but that's not a blocker for inclusion.
>
> package naming looks good according to guidelines.
>
> Compiles cleanly on F16 and against koji scratch build
>
> no locale files
> no libraries
> no devel files
> no large docs
> docs looks good
>
> spec file is legible and has consistent use of macros.
>
> setupcon  appears to run without faulting on a console tty

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

Jef Spaleta  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+

--- Comment #4 from Jef Spaleta  ---
Okay changes look good.

Package Approved

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

--- Comment #5 from Jef Spaleta  ---
Followup on the setupcon script

Running setupcon -v  as root seems to run fine.

Running setupcon -v as user throws the loadkey errors.

Reading the manpage and the README in the docs/ its not clear that setupcon is
suppose to be a privledged operation.  See attached output from setupcon -v
while as user

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

--- Comment #6 from Jef Spaleta  ---
Created attachment 594637
  --> https://bugzilla.redhat.com/attachment.cgi?id=594637&action=edit
stderr output of setupcon -v   run as normal user

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

--- Comment #7 from Jef Spaleta  ---
Hey, just a heads up it seems you might want to talk with the systemd-devels
about console-setup integration with systemd.

Relevant message from relevant thread:
http://lists.freedesktop.org/archives/systemd-devel/2012-June/005688.html

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

Vitezslav Crhonek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |fedora-cvs?

--- Comment #8 from Vitezslav Crhonek  ---
New Package SCM Request
=======================
Package Name: console-setup
Short Description: Tools for configuring the console using X Window System
keymaps
Owners: vcrhonek
Branches: f16 f17
InitialCC:

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

--- Comment #9 from Jon Ciesla  ---
Git done (by process-git-requests).

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

Vitezslav Crhonek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|---                         |NEXTRELEASE
        Last Closed|                            |2012-07-03 07:19:32

--- Comment #10 from Vitezslav Crhonek  ---
(In reply to comment #5)
> Followup on the setupcon script
>
> Running setupcon -v  as root seems to run fine.
>
> Running setupcon -v as user throws the loadkey errors.
>
> Reading the manpage and the README in the docs/ its not clear that setupcon
> is suppose to be a privledged operation.  See attached output from setupcon
> -v while as user

In most cases the setupcon script should be invoked by privileged user (because
loading new keymap with "loadkeys" requires it), but regular user can use it
too with certain options (e.g. "setupcon -f -v" will just change font on users
current tty).

[Bug 833855] Review Request: console-setup - Tools for configuring the console using X Window System keymaps

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=833855

--- Comment #11 from Jef Spaleta  ---
(In reply to comment #10)

Yeah that's what I figured, but I didn't read anything in any of the obvious
documentation about typical privledge operation. So just trying to be sure I
was understanding what I was seeing.

-jef