Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 223724 - Review Request: fvwm - window manager
Summary: Review Request: fvwm - window manager
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Patrice Dumas
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2007-01-21 23:42 UTC by Adam Goode
Modified: 2015-10-26 18:56 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-03-21 03:06:46 UTC
Type: ---
Embargoed:
pertusus: fedora-review+
gwync: fedora-cvs+


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

Description Adam Goode 2007-01-21 23:42:06 UTC
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-24 01:26:24 UTC
* 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 11:22:03 UTC
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 11:23:35 UTC
Created attachment 146525 [details]
use fvwm-xdg-menu.py to generate the Utilities menu

Comment 4 Adam Goode 2007-01-28 03:52:27 UTC
Thanks for the excellent comments! Working...

Comment 5 Peter Lemenkov 2007-02-04 07:50:33 UTC
(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-03-01 04:46:08 UTC
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 21:21:21 UTC
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-09 04:10:35 UTC
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 09:18:42 UTC
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 09:20:01 UTC
(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-10 00:54:59 UTC
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 23:46:59 UTC
(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-14 02:52:07 UTC
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 09:15:50 UTC
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-16 03:30:24 UTC
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 22:58:06 UTC
* 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-21 01:55:30 UTC
New Package CVS Request
=======================
Package Name: fvwm
Short Description: window manager
Owners: adam
Branches: FC-5 FC-6
InitialCC: 

Comment 18 Adam Goode 2009-02-06 01:01:09 UTC
Package Change Request
======================
Package Name: fvwm
New Branches: EL-4 EL-5
Owners: agoode pertusus

Comment 19 Kevin Fenzi 2009-02-06 03:14:59 UTC
cvs done.

Comment 20 Martin Cermak 2015-06-05 09:12:24 UTC
Package Change Request
======================
Package Name: fvwm
New Branches: el7
Owners: agoode pertusus jhogarth peter
InitialCC: 

Plan to build fvwm for epel7.

Comment 21 Gwyn Ciesla 2015-06-05 12:16:22 UTC
Git done (by process-git-requests).

Comment 22 Martin Cermak 2015-06-05 15:42:53 UTC
(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 11:26:13 UTC
Update requested at https://admin.fedoraproject.org/pkgdb/package/fvwm/

Comment 24 Peter Lemenkov 2015-06-08 12:05:02 UTC
(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 12:08:38 UTC
Thanks, Peter.

Comment 26 Fedora Update System 2015-06-08 12:28:54 UTC
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 18:56:55 UTC
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.