Bug 448337 - Review Request: enlightenment - a next generation desktop shell
Summary: Review Request: enlightenment - a next generation desktop shell
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2008-05-26 00:24 UTC by Pavel Shevchuk
Modified: 2008-09-01 22:43 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2008-09-01 22:43:58 UTC
Type: ---
mtasaka: fedora-review+
kevin: fedora-cvs+

Attachments (Terms of Use)

Description Pavel Shevchuk 2008-05-26 00:24:06 UTC
Spec URL: http://rpm.scwlab.com/e-for-rawhide/enlightenment.spec
SRPM URL: http://rpm.scwlab.com/e-for-rawhide/enlightenment-0.16.999.043-1.fc9.src.rpm

Enlightenment 0.17 is the next generation of UNIX graphical environments. It is
not just a window manager, but it is also a desktop shell. A desktop shell
means, a window manager plus a file manager, plus configuration utilitys all in
one. They are not separated as usual.

Vera font is being stripped out because:
1) It's original bitstream version, it doesn't include national symbols
2) Its license is different from E license and i don't want License field to exceed 80 chars :)
3) Bundling font is just stupid.

I moved bit of documentation from /usr/share/enlightenment/doc to /usr/share/doc/enlightenment-devel because it's better place for it.

rpmlint rants about two suid binaries. One of them is sudo-like thing used by e to shutdown/reboot, other is cpufreq plugin. sudo-thing could be replaced with proper consolekit/policykit integration, i added this to my TODO.

<emo mode>This sole package was reason for me to spend 2 weeks on learning rpm, become sponsored packager and then spend another month on packaging needed E libraries. I already received few emails from people anticipating E inclusion in fedora.</emo mode>

Comment 1 Dr. Gregory R. Kriehn 2008-05-26 01:17:55 UTC
Have you considered enabling additional functionality, such as using pam,
gettext, curl, openssl, SDL, libXcursor, libXdamage, libXcomposite, etc?

Please see the following spec file as an example:

SPEC:  http://optics.csufresno.edu/fedora/extras/9/SPECS/enlightenment.spec

There is a lot of functionality that you have disabled by default, but would be
easy to include, and I believe should be.  Comments?

Comment 2 Pavel Shevchuk 2008-05-26 01:29:06 UTC
I guess i have everything enabled already. 

Locale change works, curl and openssl are enabled in ecore, sdl packend need is 
questionable (but i may enable it in future), cursors get themed, composite 
radiobutton is available, screen lock works.

Comment 3 Mamoru TASAKA 2008-07-02 17:56:44 UTC
For 0.16.999.043-1:

! Question
  - Would you explain what .edj files are? Can this be freely

* Multiple issues on -devel subpackage
  ** Requires
     - efeet-devel is also needed for Requires of -devel subpackage
       (see e.h)

  ** pkgconfig file
$ pkg-config --libs enlightenment
     - This is perhaps wrong (pkg-config --libs foo should usually
       return -Ldir -lfoo -baa .....)

  ** "config.h" inclusion
     - From %_includedir/%name/e.h:
     4  #ifndef E_H
     5  #define E_H
     7  #include "config.h"
       but 1. config.h is not installed and 2. config.h must not be
       needed for installed header files.

* %prep
  - Perhaps the following is simpler:
%setup -q
# strip out bundled vera font, it doesn't support national glyphs
rm -r data/fonts
sed -i \
	-e '\|CONFIG_FILES="\$CONFIG_FILES data/fonts/Makefile"|d' \
	-e 's|data/fonts/Makefile||' \
	configure ; chmod +x configure
sed -i -e 's|fonts||' data/Makefile.in
sed -i -e '\|\.ttf|d' data/{init,themes}/default.edc

* Duplicate files
  - Please check if the following files are really needed.
    They are also installed as %doc.

* Directory ownership issue
  - Please make it sure that all directories created when installing
    this package are correctly owned by this package.
[tasaka1@localhost ~]$ rpm -qf /usr/lib/enlightenment/preload/e_precache.so 
[tasaka1@localhost ~]$ rpm -qf /usr/lib/enlightenment/preload/              
file /usr/lib/enlightenment/preload is not owned by any package
[tasaka1@localhost ~]$ rpm -qf /usr/lib/enlightenment/        
file /usr/lib/enlightenment is not owned by any package

* setuid files
  - Please explain why two binaries must have setuid bits.
    Also please consider if pam mechanism or so can replace this.

Comment 4 Mamoru TASAKA 2008-07-09 13:37:39 UTC

Comment 5 Pavel Shevchuk 2008-07-09 17:26:23 UTC
Pong. Sorry, i'm very busy at work now. I will fix pkg this weekend. 

Comment 6 Pavel Shevchuk 2008-07-26 16:18:43 UTC
* Sat Jul 26 2008 Pavel "Stalwart" Shevchuk <stlwrt@gmail.com> - 0.16.999.043-2
- Added missing efreet-devel require to enlightenment-devel
- Removed broken enlightenment_sys
- Removed more improperly placed docs
- Fixed directory ownership

Spec: http://rpm.scwlab.com/e-for-rawhide/2/enlightenment.spec
SRPM: http://rpm.scwlab.com/e-for-rawhide/2/
RPMs for rawhide-x86_64: http://rpm.scwlab.com/e-for-rawhide/2/

>Would you explain what .edj files are? Can this be freely (re)distributable?
.edj are enlightened skins, which consist of images and some bytecode. They're 
generated from stuff in source tarball which has same license as C code

>efeet-devel is also needed for Requires of -devel subpackage (see e.h)

>pkgconfig file
is like this upstream and it somehow works when building additional modules, 
will report to developers

>"config.h" inclusion
no idea, will also be reported

>%prep - Perhaps the following is simpler
thanks, replaced my lame code with yours proper

>Duplicate files

>Directory ownership issue

>Please explain why two binaries must have setuid bits.
cpufreq module changes governors directly, not depending on bulky power 
management daemons like gnome/kde
removed enlightenment_sys bits which don't seem to work (deprecated?)

Comment 7 Mamoru TASAKA 2008-07-29 19:05:12 UTC
Sorry for late reply..

For -2:
* Directory ownership issue
  - %{_includedir}/%{name}/ is not yet owned by any packages.

* pkgconfig issue
  - For now would you fix enlightenment.pc.in like following?
Version: @VERSION@
Libs: -L$libdir

* config.h inclusion
  - This issue must be fixed.
    A quick workaround is to include config.h as unique and arch-dependent 
    name and modify e.h so that e.h includes such arch-dependent header file.
    Also see %_includedir/openssl/opensslconf.h in openssl-devel.

    So a example of quick workaround follows:
    1. Modify e.h (in the source tarball it is src/bin/e.h) as:
 * vim:ts=8:sw=3:sts=8:noexpandtab:cino=>5n-3f0^-2{2
#ifndef E_H
#define E_H

#include "e_config_arch.h"

#define USE_IPC
#if 0
    2. Prepare e_config_arch.h (as %SOURCE? , say %SOURCE1) as

#if defined(__i386__)
#include "e_config-i386.h"
#elif defined(__ia64__)
#include "e_config-ia64.h"
#elif defined(__powerpc64__)
#include "e_config-ppc64.h"
#elif defined(__powerpc__)
#include "e_config-ppc.h"
#elif defined(__s390x__)
#include "e_config-s390x.h"
#elif defined(__s390__)
#include "e_config-s390.h"
#elif defined(__sparc__) && defined(__arch64__)
#include "e_config-sparc64.h"
#elif defined(__sparc__)
#include "e_config-sparc.h"
#elif defined(__x86_64__)
#include "e_config-x86_64.h"
#error "This enlightment header files do not work your architecture?"

   3. Then:
%setup -q
install %SOURCE1 src/bin/
%configure --disable-static
cp -p config.h e_config-%{_arch}.h
make %{?_smp_mflags}

make install DESTDIR=$RPM_BUILD_ROOT INSTALL='install -p'
install -cpm 644 %SOURCE0 e_config-%{_arch}.h %{_includedir}/%{name}/


* multilib conflict
  - And some reviewers say that shell scripts in foo-devel package should
    not differ between different architectures (see:
    https://fedoraproject.org/wiki/PackagingDrafts/MultilibTricks )

    Would you _consider_ to fix %name-config.in (in src tarball) like below?
    ! Note this is _optional_ and _not mandatory_


if [ -z $ARCH ] ; then ARCH=$(uname -m) ; fi
case $ARCH in
	x86_64 | ia64 | s390 )

      echo $libdirs
(and same below)

Comment 8 Mamoru TASAKA 2008-08-08 15:33:10 UTC

Comment 9 Pavel Shevchuk 2008-08-08 15:52:57 UTC
Will fix on this weekend

Comment 10 Mamoru TASAKA 2008-08-14 14:09:54 UTC
ping again?

Comment 11 Mamoru TASAKA 2008-08-25 16:07:16 UTC
ping again?

Comment 12 Pavel Shevchuk 2008-08-31 22:26:10 UTC
Sorry for huge delay =(

* Sat Aug 30 2008 Pavel "Stalwart" Shevchuk <stlwrt@gmail.com> - 0.16.999.043-3
- One more directory ownership fix
- Backported enlightenment.pc.in from trunk
- Strip unneeded config.h include from e.h

Spec: http://rpm.scwlab.com/fedora/e17/9/i386/enlightenment-0.16.999.043-3/enlightenment.spec
SRPM: http://rpm.scwlab.com/fedora/e17/9/i386/enlightenment-0.16.999.043-3/enlightenment-0.16.999.043-3.fc9.src.rpm
YUM repository for f9, i386 and x86_64:
name=Stalwart Fedora - E17

I hacked up patch for config.h just to realize that config.h isn't actually needed to build stuff that depends on e17. =( So i just strip config.h out of e.h now.
I'm not sure deriving from upstream enlightenment-config is really needed, so i didn't in -3. If you insist, i could do -4

Comment 13 Mamoru TASAKA 2008-09-01 14:46:35 UTC

       This package (enlightenment) is APPROVED by mtasaka

Comment 14 Pavel Shevchuk 2008-09-01 19:28:15 UTC
New Package CVS Request
Package Name: enlightenment
Short Description: A next generation desktop shell
Owners: stalwart
Branches: F-8 F-9
Cvsextras Commits: yes

Comment 15 Kevin Fenzi 2008-09-01 21:13:45 UTC
cvs done.

Comment 16 Pavel Shevchuk 2008-09-01 22:43:58 UTC
Kudos to Kevin, arigatos to Mamoru. E17 is now in rawhide ;)

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