Spec URL: http://terjeros.fedorapeople.org/e16/e16.spec SRPM URL: http://terjeros.fedorapeople.org/e16/e16-0.16.8.9-1.fc8.src.rpm Description: Enlightenment is a window manager for the X Window System that is designed to be powerful, extensible, configurable and pretty darned good looking! It is one of the more graphically intense window managers. Enlightenment goes beyond managing windows by providing a useful and appealing graphical shell from which to work. It is open in design and instead of dictating a policy, allows the user to define their own policy, down to every last detail. This package will install the Enlightenment window manager.
I'd be happy to review this package. Look for a full review in a bit.
OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. See below - License See below - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: 8d27553ae9c582a9d331ea4077063a14 e16-0.16.8.9.tar.gz 8d27553ae9c582a9d331ea4077063a14 e16-0.16.8.9.tar.gz.1 See below - BuildRequires correct OK - Spec handles locales/find_lang OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot OK - Package is code or permissible content. OK - Doc subpackage needed/used. OK - Packages %doc files don't affect runtime. OK - Package has rm -rf RPM_BUILD_ROOT at top of %install OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. See below - No rpmlint output. OK - final provides and requires are sane. SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should have dist tag OK - Should package latest version Issues: 1. Some of the source files appear to be GPLv2+ epp/cpperror.c epp/cppalloc.c epp/cppexp.c epp/cpphash.c epp/cpplib.c epp/cpplib.h epp/cppmain.c From a quick look, those source files all compile to the epp binary. The rest are BSDish, but not matching exactly any of the examples on the wiki. I am a bit concerned with the second paragraph: " The above copyright notice and this permission notice shall be included in all copies of the Software, its documentation and marketing & publicity materials, and acknowledgment shall be given in the documentation, materials and software packages that this Software was used." In addition there is a copy of the Bitsteam Vera Fonts with their own license inside the "winter.etheme" tar.gz thats in the e16 tar.gz. Can that be made somehow to use the already existing Vera package? I will have spot check it over... 2. Some possible missing BuildRequires: checking X11/SM/SMlib.h usability... no checking X11/SM/SMlib.h presence... no checking for X11/SM/SMlib.h... no checking for SmcOpenConnection in -lSM... no checking for XFT... no checking for XineramaQueryExtension in -lXinerama... no configure: WARNING: Xinerama support was requested but not found checking for XF86VidModeQueryExtension in -lXxf86vm... no configure: WARNING: Zoom support was requested but not found checking for XRRQueryExtension in -lXrandr... no configure: WARNING: RandR support was requested but not found checking for X11/extensions/Xrandr.h... no checking for XCompositeQueryExtension in -lXcomposite... no checking for X11/extensions/Xcomposite.h... no checking for X11/extensions/Xdamage.h... no checking for X11/extensions/Xfixes.h... no checking for X11/extensions/Xrender.h... no configure: WARNING: Composite support was requested but required component was not found checking for mass_quantities_of_bass_ale in -lFridge... no checking for mass_quantities_of_any_ale in -lFridge... no Warning: No ales were found in your refrigerator. We highly suggest that you rectify this situation immediately. 3. rpmlint says: e16.i386: E: zero-length /usr/share/e16/themes/winter/slideouts/slideouts.cfg e16.i386: E: zero-length /usr/share/e16/themes/winter/buttons/buttons.cfg Not sure if those can be removed, or if they are needed by that theme... e16.i386: E: invalid-soname /usr/lib/libe16_hack.so libe16_hack.so Does that so file need to be in /usr/lib? e16.i386: W: file-not-utf8 /usr/share/doc/e16-0.16.8.9/ChangeLog e16.i386: W: file-not-utf8 /usr/share/doc/e16-0.16.8.9/AUTHORS Might run 'iconv' on those?
> 1. Some of the source files appear to be GPLv2+ GPLv2+ added to license. > In addition there is a copy of the Bitsteam Vera Fonts with their own license > inside the "winter.etheme" tar.gz thats in the e16 tar.gz. Can that be made > somehow to use the already existing Vera package? Removed ttfonts dir, created a symlink and added bitstream-vera to req, seems to work. > I will have spot check it over... Ok, thanks. > 2. Some possible missing BuildRequires: Tried to add some X related devel packages, better now? > 3. rpmlint says: > > e16.i386: E: zero-length /usr/share/e16/themes/winter/slideouts/slideouts.cfg > e16.i386: E: zero-length /usr/share/e16/themes/winter/buttons/buttons.cfg > > Not sure if those can be removed, or if they are needed by that theme... Tried that, not a crash, but a freeze, better leave these. > e16.i386: E: invalid-soname /usr/lib/libe16_hack.so libe16_hack.so > > Does that so file need to be in /usr/lib? Don't know, however e16 seems to be happy without this lib, maybe needed by the epplets? > e16.i386: W: file-not-utf8 /usr/share/doc/e16-0.16.8.9/ChangeLog > e16.i386: W: file-not-utf8 /usr/share/doc/e16-0.16.8.9/AUTHORS > > Might run 'iconv' on those? Fixed. SPEC: http://terjeros.fedorapeople.org/e16/e16.spec SRPM: http://terjeros.fedorapeople.org/e16/e16-0.16.8.9-2.fc7.src.rpm
> Tried to add some X related devel packages, better now? Added some more, did not bump release: SPEC: http://terjeros.fedorapeople.org/e16/e16.spec SRPM: http://terjeros.fedorapeople.org/e16/e16-0.16.8.9-2.fc7.src.rpm
A license with virtually the same text is pending review at the FSF right now, so I'm slapping FE-Legal on this one until it is resolved... sorry.
> A license with virtually the same text is pending review at the FSF right now, > so I'm slapping FE-Legal on this one until it is resolved... sorry. Oki, thanks for the help so far.
New upstream version (which solves the libhach issue): - 0.16.8.10 - libhack has moved to e16 subdir Updated package here: SPEC: http://terjeros.fedorapeople.orgke16/e16.spec SRPM: http://terjeros.fedorapeople.org/e16/e16-0.16.8.10-1.fc7.src.rpm BTW: any progress on the license issue?
> SPEC: http://terjeros.fedorapeople.orgke16/e16.spec Sorry, it's SPEC: http://terjeros.fedorapeople.org/e16/e16.spec
Spot: Any word back on the license here?
ping. there hasnt been any activity since months now.
It's still pending legal review from the FSF. Hopefully they will get around to reviewing it soon...
OK, e16 license came back as free but GPL-incompatible (v2 and v3), due to the fact that notices and an acknowledgement must appear in "marketing and publicity materials", which puts it in the BSD-advertising-clause category. Since the epp bits compile into their own, independent binary, and there is no source code linking or sharing, this is fine to distribute as is. Please use: License: BSD with advertising and GPLv2+ It is worth noting that some of the e16 upstream components have fixed this problem in their license: (http://www.enlightenment.org/viewvc/misc/geist/COPYING?r1=1.2&r2=1.3&pathrev=HEAD&view=patch) I would strongly suggest reaching out to the copyright holders everytime we see this license and asking them if they're willing to make the same license changes. While its not a problem for this specific package, it could easily make other works which have GPL and e16 code linked together, totally undistributable by Fedora (or anyone, really). Lifting FE-Legal.
Great news spot, thanks! New package ready: - 0.16.8.12 - fix license (thanks spot and kevin!) - enable dbus and pango - simplity setup and modify desc - try to preserve timestamps spec: http://terjeros.fedorapeople.org/e16/e16.spec srpm: http://terjeros.fedorapeople.org/e16/e16-0.16.8.12-1.fc8.src.rpm
Actually, after more thought, I want the License to be: License: MIT with advertising and GPLv2+ Sorry for the confusion. This is what I get for doing Licensing after midnight. :)
Ok, updated package: spec: http://terjeros.fedorapeople.org/e16/e16.spec srpm: http://terjeros.fedorapeople.org/e16/e16-0.16.8.12-2.fc8.src.rpm
ok, first off... per spot's comment, would you be willing to ping upstream about changing this license? Not a blocker, but it would be nice to fix it up moving forward if possible. ;) rpmlint now says: e16.i386: E: zero-length /usr/share/e16/themes/winter/slideouts/slideouts.cfg e16.i386: E: zero-length /usr/share/e16/themes/winter/buttons/buttons.cfg Can leave those as we mentioned before... e16.i386: W: dangling-relative-symlink /usr/share/e16/themes/winter/ttfonts ../../../fonts/bitstream-vera You sure that link is right? I think it might need another ../ there? e16.i386: W: invalid-license MIT with advertising e16.src: W: invalid-license MIT with advertising e16-debuginfo.i386: W: invalid-license MIT with advertising Can be ignored. The build options look ok for the most part, but I see: Experimental options - DO NOT USE unless you know what you are doing Compile with ecore/ecore_x ... no GLX .......................... no ScreenSaver .................. no D-Bus ........................ yes Is D-Bus stable enough to use? If so, how about the others? Surely not a blocker. The only thing I see off hand to doublecheck is the link to the font. Can you check that and make sure it's right before checkin? Otherwise, this package looks ok, and is APPROVED.
(In reply to comment #16) > ok, first off... per spot's comment, would you be willing to ping upstream > about changing this license? Not a blocker, but it would be nice > to fix it up moving forward if possible. ;) Will send a mail to Kim. > > e16.i386: W: dangling-relative-symlink /usr/share/e16/themes/winter/ttfonts > ../../../fonts/bitstream-vera > > You sure that link is right? I think it might need another ../ there? It's correct (from a live system): $ namei /usr/share/e16/themes/winter/ttfonts f: /usr/share/e16/themes/winter/ttfonts d / d usr d share d e16 d themes d winter l ttfonts -> ../../../fonts/bitstream-vera d .. d .. d .. d fonts d bitstream-vera > The build options look ok for the most part, but I see: > > Experimental options - DO NOT USE unless you know what you are doing > Compile with ecore/ecore_x ... no > GLX .......................... no > ScreenSaver .................. no > D-Bus ........................ yes > > Is D-Bus stable enough to use? If so, how about the others? > Surely not a blocker. DBUS is not listed as experimental in "configure --help" while the others are, uhm... > The only thing I see off hand to doublecheck is the link to the font. > Can you check that and make sure it's right before checkin? Yes, > Otherwise, this package looks ok, and is APPROVED. Thanks, Kevin! New Package CVS Request ======================= Package Name: e16 Short Description: The Enlightenment window manager, DR16 Owners: terjeros Branches: F-7 F-8 F-9 InitialCC: Cvsextras Commits: yes
>DBUS is not listed as experimental in "configure --help" while the others are, >uhm... Weird. Might just be a bug in the build/configure output... might mention that upstream as well? cvs done.
(In reply to comment #18) > Weird. Might just be a bug in the build/configure output... > might mention that upstream as well? dbus is experimental (info from upstream), > cvs done. Thanks. Now imported.