Bug 223724 - Review Request: fvwm - window manager
Review Request: fvwm - window manager
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Patrice Dumas
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-01-21 18:42 EST by Adam Goode
Modified: 2015-10-26 14:56 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-20 23:06:46 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
pertusus: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)
use fvwm-xdg-menu.py to generate the Utilities menu (1.60 KB, patch)
2007-01-25 06:23 EST, Patrice Dumas
no flags Details | Diff

  None (edit)
Description Adam Goode 2007-01-21 18:42:06 EST
Spec URL: http://www.spicenitz.org/fedora/fvwm.spec
SRPM URL: http://www.spicenitz.org/fedora/fvwm-2.5.21-1.src.rpm
Description: ICCCM-compliant multiple virtual desktop window manager
Comment 1 Patrice Dumas 2007-01-23 20:26:24 EST
* regarding the license, I have spotted some files without license
  but with a mention to an author or even a copyright notice. 
  When there is a copyright and no license, the default license
  is a restrictive license (no modification nor redistribution)
  which is non free and GPL incompatible.
  
it is the case for
bin/fvwm-config
bin/fvwm-convert-2.2 (although the comments seem to indicate that
                      it is public domain)
bin/fvwm-convert-2.4

utils/fvwm_make_*.sh  (although these are examples and so are likely
                       in the public domain)

modules/FvwmWharf/ASSound/ASSound.c

* I think that the summary could be shortened, ICCM compliance and
  for the X Window system are not very informative. Maybe something along

Summary:        Highly configurable multiple virtual desktop window manager

* fvwm-menu-headlines should use htmlview instead of netscape

* fvwm-bug requires %{_sbindir}/sendmail

* fvwm-menu-xlock requires xlock

* fvwm-menu-directory could use mimeopen -n instead of EDITOR to 
  open files.

* in fvwm/ConfigFvwmSetup the default menu is badly suited for 
  fedora. I propose removing rxvt, replacing the utility submenu 
  a dynamically generated menu using the freedesktop standard.
  For that, I found a python script using pyxdg which could 
  be suitable:
http://www.cl.cam.ac.uk/~pz215/fvwm-scripts/scripts/xdg-menu.py
  However, I found 2 issues with that script, when calling it with
  xdg-menu.py /etc/xdg/menus/applications.menu > somefile
1. the accented characters make it break
2. the top level menu hasn't a reproducible name which could be handy
   to reference it in a Popup entry. 
  I reported those issues upstream, with a patch to add a -m option
  to specify the toplevel menu name

Then AddToMenu MenuFvwmUtilities  could become something along:

PipeRead "fvwm-xdg-menu-top -m MenuFvwmUtilities /etc/xdg/menus/applications.menu"


* there is a missing
Requires: xterm
Comment 2 Patrice Dumas 2007-01-25 06:22:03 EST
Upstream for the xdg-menu.py fixed the issues I reported, and
changed the name of the script:
http://www.cl.cam.ac.uk/~pz215/fvwm-scripts/scripts/fvwm-xdg-menu.py

I submitted a minor patch for -f such that directory is created for 
icons if it doesn't exists.

fvwm-xdg-menu.py
Requires:    ImageMagick

I attach a patch for fvwm/ConfigFvwmSetup
Comment 3 Patrice Dumas 2007-01-25 06:23:35 EST
Created attachment 146525 [details]
use fvwm-xdg-menu.py to generate the Utilities menu
Comment 4 Adam Goode 2007-01-27 22:52:27 EST
Thanks for the excellent comments! Working...
Comment 5 Peter Lemenkov 2007-02-04 02:50:33 EST
(In reply to comment #4)
> Thanks for the excellent comments! Working...

Could you please update your spec according to Patrice's proposals.
Comment 6 Adam Goode 2007-02-28 23:46:08 EST
http://www.spicenitz.org/fedora/fvwm.spec
http://www.spicenitz.org/fedora/fvwm-2.5.21-2.src.rpm

- Shorten description
- Enable auto-generate menus in the Setup Form config generator
- Use htmlview instead of netscape
- Use mimeopen instead of EDITOR
- Add more Requires


Note that the copyright notices have been added upstream. I didn't include a
patch for it here.
Comment 7 Patrice Dumas 2007-03-01 16:21:21 EST
rpmlint output is ignorable:
W: fvwm devel-file-in-non-devel-package /usr/bin/fvwm-config


I am not convinced that it is right to provide the perl 
provides since they are not in the perl module dir. It means
filtering out all the provides, and from the requires:
perl(FVWM::*) perl(FvwmCommand) perl(General::FileSystem) perl(General::Parse)


Suggestions: 

add a trailing / to directories in %files to show visually
 that these are directories, like
%{_datadir}/%{name}/

Give permission on install command line call, even if the result
is righht by now:

install -D -p -m755 %{SOURCE2} \
        $RPM_BUILD_ROOT%{_bindir}/fvwm-xdg-menu

Comment 8 Adam Goode 2007-03-08 23:10:35 EST
http://www.spicenitz.org/fedora/fvwm.spec
http://www.spicenitz.org/fedora/fvwm-2.5.21-3.src.rpm

- Rebuild configure with autoconf >= 2.60 (for datarootdir)
- Filter out local Perl libraries from provides and requires

Also, the Suggestions above are implemented.
Comment 9 Patrice Dumas 2007-03-09 04:18:42 EST
In general regenerating configure isn't something that shouldn't be 
done here, but upstream. In my opinion you should just do

sed -e -i 's/@datarootdir@/%{_datadir}/' fvwm-config

and be done with it (and maybe fill a bug upstream). Or did I
miss somewhere else where it is used? (in fvwm-perllib it is 
unusefull).

Comment 10 Patrice Dumas 2007-03-09 04:20:01 EST
(In reply to comment #9)
> In general regenerating configure isn't something that shouldn't be 
> done here, but upstream. In my opinion you should just do
> 
> sed -e -i 's/@datarootdir@/%{_datadir}/' fvwm-config

well
sed -e -i 's:@datarootdir@:%{_datadir}:' fvwm-config
 
would certainly be better :-)
Comment 11 Adam Goode 2007-03-09 19:54:59 EST
I filed a bug upstream. Also, looking at FVWM mailing list archives, it seems
the problem is known.

http://www.fvwm.org/cgi-bin/fvwm-bug/incoming?id=4208

I'll see how hard it is to fix the Perl .in files to support old autoconf for now...
Comment 12 Patrice Dumas 2007-03-10 18:46:59 EST
(In reply to comment #11)
> I filed a bug upstream. Also, looking at FVWM mailing list archives, it seems
> the problem is known.
> 
> http://www.fvwm.org/cgi-bin/fvwm-bug/incoming?id=4208
> 
> I'll see how hard it is to fix the Perl .in files to support old autoconf for
now...

Unless I am wrong it does not need to be fixed in
fvwm-perllib, FVWM/Module.pm,  /usr/libexec/fvwm/2.5.21/FvwmDebug
/usr/libexec/fvwm/2.5.21/FvwmPerl /usr/libexec/fvwm/2.5.21/FvwmTabs
/usr/libexec/fvwm/2.5.21/FvwmWindowMenu
since the corresponding variables are never used.
Comment 13 Adam Goode 2007-03-13 22:52:07 EDT
Even though the variable filled with @datarootdir@ is never used, @datarootdir@
to Perl looks like the unbound array variable "@datarootdir". Thus, compilation
fails. I need to remove @datarootdir@ no matter where it appears.
Comment 14 Patrice Dumas 2007-03-14 05:15:50 EDT
Indeed with use strict it doesn't compile. What I used in 
texi2html is the following snippet:

if ('@datarootdir@' ne '@' . 'datarootdir@')
{
    $datarootdir = eval '"@datarootdir@"';
}
else
{
    $datarootdir = "/usr/local/share";
}

The else could also be $datarootdir = ''; or $datarootdir = undef;

The aim is different, it is to allow to run an unconfigured script.


Anyway in the case of the fvwm package it seems to me that a 'simple' 
sed substitution would be best, along

find . -name '*.in' -prune -o \( -type f -a -exec sed -i -e
's:@datarootdir@:%{_datadir}:' {} \; \)

Comment 15 Adam Goode 2007-03-15 23:30:24 EDT
http://www.spicenitz.org/fedora/fvwm.spec
http://www.spicenitz.org/fedora/fvwm-2.5.21-4.src.rpm

 - Don't patch configure, just patch a few files
Comment 16 Patrice Dumas 2007-03-20 18:58:06 EDT
* rpmlint output may be ignored:
W: fvwm devel-file-in-non-devel-package /usr/bin/fvwm-config
* free software, license included
* follow naming and packaging guideline
* source match upstream
c11efef91420e686d54f772e7162e879  ../SOURCES/fvwm-2.5.21.tar.bz2
e1f2ce60f0a49958f12d05dec44420a2  ../SOURCES/fvwm-xdg-menu.py
* sane provides
* add a .desktop session file
* follow freedesktop standards
* build and work fine on i386


APPROVED
Comment 17 Adam Goode 2007-03-20 21:55:30 EDT
New Package CVS Request
=======================
Package Name: fvwm
Short Description: window manager
Owners: adam@spicenitz.org
Branches: FC-5 FC-6
InitialCC: 
Comment 18 Adam Goode 2009-02-05 20:01:09 EST
Package Change Request
======================
Package Name: fvwm
New Branches: EL-4 EL-5
Owners: agoode pertusus
Comment 19 Kevin Fenzi 2009-02-05 22:14:59 EST
cvs done.
Comment 20 Martin Cermak 2015-06-05 05:12:24 EDT
Package Change Request
======================
Package Name: fvwm
New Branches: el7
Owners: agoode pertusus jhogarth peter
InitialCC: 

Plan to build fvwm for epel7.
Comment 21 Jon Ciesla 2015-06-05 08:16:22 EDT
Git done (by process-git-requests).
Comment 22 Martin Cermak 2015-06-05 11:42:53 EDT
(In reply to Jon Ciesla from comment #21)
> Git done (by process-git-requests).

May I get commit right to the newly created branch, please? Maybe I forgot to name myself among Owners?
Comment 23 Martin Cermak 2015-06-08 07:26:13 EDT
Update requested at https://admin.fedoraproject.org/pkgdb/package/fvwm/
Comment 24 Peter Lemenkov 2015-06-08 08:05:02 EDT
(In reply to Martin Cermak from comment #23)
> Update requested at https://admin.fedoraproject.org/pkgdb/package/fvwm/

I've just granted mcermak access to EL7 branch.
Comment 25 Martin Cermak 2015-06-08 08:08:38 EDT
Thanks, Peter.
Comment 26 Fedora Update System 2015-06-08 08:28:54 EDT
fvwm-2.6.5-1.el7 has been submitted as an update for Fedora EPEL 7.
https://admin.fedoraproject.org/updates/fvwm-2.6.5-1.el7
Comment 27 Fedora Update System 2015-10-26 14:56:55 EDT
fvwm-2.6.5-1.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.

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