Bug 856860 - css files not installed
css files not installed
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: shellinabox (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Simone Caronni
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-12 21:27 EDT by Joel
Modified: 2012-11-16 18:58 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-10-30 21:05:21 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
patch for spec file (1.78 KB, patch)
2012-09-12 21:27 EDT, Joel
no flags Details | Diff
replacement systemd service (345 bytes, text/plain)
2012-09-12 21:28 EDT, Joel
no flags Details
replacement systconfig file (519 bytes, text/plain)
2012-09-12 21:28 EDT, Joel
no flags Details

  None (edit)
Description Joel 2012-09-12 21:27:36 EDT
Created attachment 612310 [details]
patch for spec file

Description of problem:

The provided css files are not installed.  


The attached spec patch creates /usr/share/shellinabox and installs the .css files into there.

The attached shellinaboxd.service tells systemd to start shellinabox with /usr/share/shellinabox as the current working directory

The attached shellinaboxd.sysconfig has an example OPT line that enables a right-click menu choice to switch between light on black or black on white.  It also disables the turn-off-ssl right-click menu choice.

I think it would be reasonable to make that the default but I didn't got that far in this submission.

Joel
Comment 1 Joel 2012-09-12 21:28:21 EDT
Created attachment 612311 [details]
replacement systemd service
Comment 2 Joel 2012-09-12 21:28:46 EDT
Created attachment 612312 [details]
replacement systconfig file
Comment 3 Simone Caronni 2012-09-13 04:47:28 EDT
Thanks for the info and examples. I thought I was the only consumer for shellinabox on RHEL/Fedora :)

I've started applying the changes you listed plus something more:

1- Install provided CSS files and relevant directory.
2- Expanded sysconfig file with additional example.
3- Expanded sysconfig file with my local ssh setup as example (we embed the Shellinabox window into some cgi scripts without ssl etc.).

There are a few things to consider before proceeding:

1) What about "styles.css" "and print-styles.css"?

According to the man page:

 --css=filename
 Sometimes, it is not necessary to replace the entire style sheet using the
 --static-file option. But instead a small incremental  change  should  be
 made to the visual appearance of the terminal. The --css option provides a
 means to append additional style rules to the end of the default styles.css
 sheet. More than one --css option can be given on the same command line.

When using the original "make install" commands they are not installed, though.

2) The "WorkingDirectory" in the systemd file is fine, but how can we make it available also to the init file for RHEL/CentOS? I've looked at the man page but there's no parameter to set it at runtime.

The Makefile installs the css files by default in $docdir.

3) The makefile install and the manpages are really debian-centric. Should we patch the man page to reflect the path of the installed files?

4) The list of options is now very long. Would setting user, group, cert directory etc. be worth splitting into multiple variables in the sysconfig file like it is in the default debian files? i.e.:

PORT=4200
DATADIR=/var/run/shellinabox
USER=shellinabox
GROUP=shellinabox

And then in the systemd file:

ExecStart=/usr/sbin/shellinaboxd -u $USER -g $GROUP --cert=$DATADIR --port=$PORT
Comment 4 Simone Caronni 2012-09-13 05:14:39 EDT
(In reply to comment #3)
> 3) The makefile install and the manpages are really debian-centric. Should
> we patch the man page to reflect the path of the installed files?

Forget this, the file configuration section is added only if DPKGBUILD is set.
 
> 4) The list of options is now very long. Would setting user, group, cert
> directory etc. be worth splitting into multiple variables in the sysconfig
> file like it is in the default debian files? i.e.:
> 
> PORT=4200
> DATADIR=/var/run/shellinabox
> USER=shellinabox
> GROUP=shellinabox
> 
> And then in the systemd file:
> 
> ExecStart=/usr/sbin/shellinaboxd -u $USER -g $GROUP --cert=$DATADIR
> --port=$PORT

This definitely makes sense, here are the udpates:

http://pkgs.fedoraproject.org/cgit/shellinabox.git/log/
Comment 5 Joel 2012-09-13 10:45:56 EDT
Seems like a good idea.  

For the extended options in shellinaboxd.sysconfig

Should the lines include the baseline for example something like this:

# OPTS="--user-css Normal:+black-on-white.css,Reverse:-white-on-black.css ${BASEOPTS}"

with BASEOPTS defined as currently set?
Comment 6 Joel 2012-09-17 14:09:05 EDT
Regarding RHEL/CentOS, one could have a script in /usr/share/shellinabox that CDed to the correct directory before starting the daemon.  Or one could create an init script ;-)
Comment 7 Simone Caronni 2012-09-17 14:28:04 EDT
Hello,

I'm currently on holiday, I'll be back next Monday and take care of the bug.
The RHEL/CentOS init scripts are already in place.

Regards,
--Simone
Comment 8 Joel 2012-09-19 12:24:21 EDT
Hope you are having a wonderful holiday!  Thanks for your support.
Comment 9 Simone Caronni 2012-09-24 10:40:45 EDT
Back from holiday.

I checked some other init scripts, entering a directory before launching a command is ok in SysV init scripts.

So I've set up WorkingDirectory in service files as your patch and added the cd command to the init scripts.

Regarding the split of multiple options and one long $BASEOPTS I think is better to keep them separate. Other packages follow the same approach.

Putting any additional variable to split the lines can be done by the user in the sysconfig file, if there's any need.

Binaries are here: http://koji.fedoraproject.org/koji/packageinfo?packageID=13996
I will push an update when the other pending updates I have in Bodhi reach stable.

Thank you very much for all your input!

--Simone
Comment 10 Joel 2012-09-24 11:52:07 EDT
Thanks!

Did you forget the 
WorkingDirectory=/usr/share/shellinabox

line in /lib/systemd/system/shellinaboxd.service ?

Just looked in fc16 rpm and that seems to be missing.

Also, did you decide not to include the print-styles.css file?

Joel
Comment 11 Simone Caronni 2012-09-25 02:43:48 EDT
(In reply to comment #10)
> Did you forget the 
> WorkingDirectory=/usr/share/shellinabox
> 
> line in /lib/systemd/system/shellinaboxd.service ?
> 
> Just looked in fc16 rpm and that seems to be missing.

Yes, my fault. Too many things at the same time.
 
> Also, did you decide not to include the print-styles.css file?

Well, it is not installed by default and not required for the basic operation of the daemon, unless you customize the interface and expose those files somewhere and reference them in the config file.

But since they're referenced by the man page I've added them to the %doc directory, much like the "examples" directory in other packages.

Are you already a Fedora packager? Would you like to have commit access to the various branches?
Comment 12 Simone Caronni 2012-09-25 05:08:45 EDT
Builds available:

http://koji.fedoraproject.org/koji/packageinfo?packageID=13996
Comment 13 Joel 2012-09-25 11:32:57 EDT
I'm not a fedora packager, although I've been making rpms since the 90's (was in RHCN).  Some day I should become one...
Comment 14 Joel 2012-09-25 12:15:55 EDT
Simone, You are going to hate me but there is another bug:

In /lib/systemd/system/shellinaboxd.service

The ExecStart line needs to have curly braces around CERTDIR and PORT after the = sign to get them to expand.  Don't put the curly braces around the others as OPTS will be a single argument rather than a set of arguments.

New Corrected Line:

ExecStart=/usr/sbin/shellinaboxd -u $USER -g $GROUP --cert=${CERTDIR} --port=${PORT} $OPTS


Joel
Comment 15 Simone Caronni 2012-09-26 06:07:53 EDT
Strange, it works on my RHEL boxes, but not on Fedora 17.

Builds on the way, here is the commit:

http://pkgs.fedoraproject.org/cgit/shellinabox.git/commit/?id=deba4cbc4f87ccf488f47d374d08cfacd5850a9c

I don't hate anybody, so anything that doesn't work please tell. :)

Thank you very much.
Comment 16 Simone Caronni 2012-09-28 03:15:01 EDT
Hello,

I've found a thing that doesn't work. On Fedora (17, at least) the /var/run folder is a symlink to the /run mounted tmpfs filesystem.

tmpfs on /run type tmpfs (rw,nosuid,nodev,seclabel,mode=755)

Upon a reboot, the folders in it gets deleted and so shellinaboxd refuses to start because he dose not create his cert dir automatically. Any idea on how to solve this? systemd flags/directives maybe?

On RHEL is of course fine as /var/run is not on tmpfs.

--Simone
Comment 17 Joel 2012-09-28 15:26:59 EDT
You are right.  Stupid me to suggest /var/run.  That is listed as a place for temporaries.  We should do /var/shellinabox.  

Joel
Comment 18 Simone Caronni 2012-10-01 02:57:59 EDT
/var/lib seems to be a better choice [1]; an excerpt from the FHS:

"This hierarchy holds state information pertaining to an application or the system. State information is data that programs modify while they run, and that pertains to one specific host. Users must never need to modify files in /var/lib to configure a package's operation.

State information is generally used to preserve the condition of an application (or a group of inter-related applications) between invocations and between different instances of the same application. State information should generally remain valid after a reboot, should not be logging output, and should not be spooled data.

An application (or a group of inter-related applications) must use a subdirectory of /var/lib for its data. There is one required subdirectory, /var/lib/misc, which is intended for state files that don't need a subdirectory; the other subdirectories should only be present if the application in question is included in the distribution. [38]

/var/lib/<name> is the location that must be used for all distribution packaging support. Different distributions may use different names, of course."

I think these builds are fine and I would like to push them. Please note that you have to remove the old shellinabox user or change its path as its home directory has changed.

[1] http://www.pathname.com/fhs/pub/fhs-2.3.html#VARLIBVARIABLESTATEINFORMATION
Comment 19 Fedora Update System 2012-10-03 02:44:19 EDT
shellinabox-2.14-14.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/shellinabox-2.14-14.el5
Comment 20 Fedora Update System 2012-10-03 02:45:10 EDT
shellinabox-2.14-14.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/shellinabox-2.14-14.el6
Comment 21 Fedora Update System 2012-10-03 02:45:31 EDT
shellinabox-2.14-14.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/shellinabox-2.14-14.fc16
Comment 22 Fedora Update System 2012-10-03 02:45:51 EDT
shellinabox-2.14-14.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/shellinabox-2.14-14.fc17
Comment 23 Fedora Update System 2012-10-03 02:46:12 EDT
shellinabox-2.14-14.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/shellinabox-2.14-14.fc18
Comment 24 Fedora Update System 2012-10-03 13:02:22 EDT
Package shellinabox-2.14-14.el6:
* should fix your issue,
* was pushed to the Fedora EPEL 6 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=epel-testing shellinabox-2.14-14.el6'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-EPEL-2012-13070/shellinabox-2.14-14.el6
then log in and leave karma (feedback).

Note You need to log in before you can comment on or make changes to this bug.