Bug 196379 - Review Request: kdeartwork-extras: Artwork Extras, including xscreensaver-based screensavers
Review Request: kdeartwork-extras: Artwork Extras, including xscreensaver-bas...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-06-22 16:21 EDT by Rex Dieter
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

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


Attachments (Terms of Use)

  None (edit)
Description Rex Dieter 2006-06-22 16:21:57 EDT
Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/kdeartwork-extras.spec
SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.stable/kdeartwork-extras-3.5.3-6.src
Description:
KDE Artwork Extras, including xscreensaver-based screensavers
Comment 1 Rex Dieter 2006-06-22 16:24:22 EDT
Targetted for fc6+
Comment 2 Parag AN(पराग) 2006-06-23 01:18:03 EDT
Rex i got error in mock build
error: unpacking of archive failed on file
/builddir/build/SOURCES/kdegraphics-3.5.3.tar.bz2;449b7b15: cpio: read failed -
Invalid argument
Error installing srpm: kdegraphics-extras-3.5.3-3.src.rpm
Comment 3 Parag AN(पराग) 2006-06-23 01:23:28 EDT
oops wrong copy/paste 
anyway i am not able to download SRPM can you check SRPM link
Comment 4 Rex Dieter 2006-06-23 07:20:07 EDT
url got truncated (missed .rpm):

fixed SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.stable/kdeartwork-extras-3.5.3-6.src.rpm

unpacking WORKSFORME.
Comment 5 Parag AN(पराग) 2006-06-23 07:48:36 EDT
rpmlint -i kdegraphics-extras-3.5.3-3.src.rpm
error checking signature of kdegraphics-extras-3.5.3-3.src.rpm
Comment 6 Rex Dieter 2006-06-23 07:53:49 EDT
WORKSFORME:
$rpmlint -i kdegraphics-extras-3.5.3-3.src.rpm

You apparently don't have by gpg key, fetch from
http://kde-redhat.sourceforge.net/
Comment 7 Mike McGrath 2006-07-26 15:19:33 EDT
WORKSFORME:
and builds in mock.
Comment 8 Mamoru TASAKA 2006-09-29 12:53:46 EDT
Is this targeted for FC6 or FC7?
Comment 9 Rex Dieter 2006-09-29 12:56:30 EDT
fc6 (and newer)
Comment 10 Mamoru TASAKA 2006-09-29 13:09:14 EDT
Then, where is the newest SPEC file and SRPM?
Comment 11 Rex Dieter 2006-09-29 13:15:33 EDT
Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/kdeartwork-extras.spec
SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.stable/kdeartwork-extras-3.5.4-2.src

%changelog
* Tue Aug 29 2006 Rex Dieter <rexdieter[AT]users.sf.net> 3.5.4-2
- used unversioned Requires: kdebase

* Tue Jul 25 2006 Rex Dieter <rexdieter[AT]users.sf.net> 3.5.4-1
- kde-3.5.4
Comment 12 Mamoru TASAKA 2006-09-29 14:54:09 EDT
I will review this later.
Comment 13 Mamoru TASAKA 2006-09-30 03:09:53 EDT
Well,

1. From http://fedoraproject.org/wiki/Packaging/Guidelines :

* rpmlint is not silent.
W: kdeartwork-extras macro-in-%changelog _libdir
W: kdeartwork-extras macro-in-%changelog _libexecdir
   Use %% to avoild having macros expanded.

* Requires:
  - Does this package (kdeartwork-extras) really require
    xscreensaver related packages? It is someting like
    "optional", isn't it? Perhaps there are some users
    who don't want to install xscreensaver-gl-extras.

    I also wonder why this package require xscreensaver-base.
    Perhaps KDE has its own screensaver mechanism isn't it?
    (I use GNOME and xscreensave as screensaver, so I don't
     know well about KDE).

* BuildRequires:
  - Is xscreensaver-base really required?

2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
   = Nothing.

3. Other things I have noticed:
A. request:
   * Desktop files corresponding to the following hacks are missing
     - glschool (in xscreensaver-gl-extras)
     - topblock (in xscreensaver-gl-extras)
     Would you add the desktop files for these hacks?

   * IMO, the hacks in the following package can also be used for 
     this package. 
     rss-glx (in extras and maintained by Nils Philippsen)
     xdaliclock (in extras and maintained by Chris Ricker)

     Would you add the corresponding desktop files or ask the
     maintainers to support this package?
Comment 14 Mamoru TASAKA 2006-09-30 03:24:31 EDT
Another one question.

Does "sourc"ing Qt config files (/etc/profile.d/qt.sh) still needed?
At least mockbuild surely "source" all files under /etc/profile.d and
explicit "sourc"ing qt.sh is unnecessary.

I can see many packages which use qt for rebuilding and don't "source"
qt.sh explicitly under:
http://buildsys.fedoraproject.org/logs/fedora-development-extras/
Comment 15 Rex Dieter 2006-09-30 13:50:02 EDT
> Does "sourc"ing Qt config files (/etc/profile.d/qt.sh) still needed?

Strictly speaking, no, not for mock anyway.  otoh, it's presence certainly is
not wrong nor does it cause any harm.

> Does this package (kdeartwork-extras) really require
> xscreensaver related packages?

Yes, most of the bundled screensavers actually use xscreensaver as the backend.

> It is someting like
>    "optional", isn't it? Perhaps there are some users
>    who don't want to install xscreensaver-gl-extras.

Without more package splitting, no.  The manual work required to make that work
properly simply isn't worth it, imo.

>   - Is xscreensaver-base really required?

Yes.  %%configure checks for the presence of the xscreensaver bits at buildtime.

>    * Desktop files corresponding to the following hacks are missing

Personally, I'm not interested in adding those by hand.  IMO, it's an upstream
issue (ie, I'd prefer to let them "fix" it).

>    * IMO, the hacks in the following package can also be used for 
>     this package. 

I'll ping upstream about these too.

> W: kdeartwork-extras macro-in-%changelog _libdir
> W: kdeartwork-extras macro-in-%changelog _libexecdir
>   Use %% to avoild having macros expanded.

Oops, will fix that.

Comment 16 Mamoru TASAKA 2006-10-01 02:30:26 EDT
(In reply to comment #15)

> >   - Is xscreensaver-base really required?
> 
> Yes.  %%configure checks for the presence of the xscreensaver bits at buildtime.
Actually I tried mockbuild with xscreensaver-base removed from
BuildRequires and
* mockbuild suceeded without any failure
* the results of "rpm -qlp" and "rpm -ql --requires" make no difference

It seems that configure checks where xscreensaver hacks are installed and
does not check if xscreensaver daemon is installed.

I still wonder why this package requires xscreensaver-base (for Requires:).
KDE uses its own screensaver daemon, doesn't it? Does this package
require xscreensaver daemon or xscreensaver configuration program?
 
> > W: kdeartwork-extras macro-in-%changelog _libdir
> > W: kdeartwork-extras macro-in-%changelog _libexecdir
> >   Use %% to avoild having macros expanded.
> 
> Oops, will fix that.
> 
Comment 17 Rex Dieter 2006-10-01 08:46:56 EDT
Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/kdeartwork-extras.spec
SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.stable/kdeartwork-extras-3.5.4-3.src

%changelog
* Sat Sep 30 2006 Rex Dieter <rexdieter[AT]users.sf.net> 3.5.4-3
- use %%_foo style macros in %%changelog


> I still wonder why this package requires xscreensaver-base (for Requires:).

Because many of the kde screensavers actually use xscreensaver.  In the absense
of the xscreensaver bits, the screensavers will be listed as being available,
but they won't actually work or do anything.
Comment 18 Rex Dieter 2006-10-01 08:49:37 EDT
> Actually I tried mockbuild with xscreensaver-base removed from
> BuildRequires and
> * mockbuild suceeded without any failure

But do all the xscreensaver-based screensavers actually *work*?  I'd venture
possibly not, because %configure checks for the hacks location at buildtime, and
in the absense of that, it may just end up guessing.
Comment 19 Mamoru TASAKA 2006-10-01 09:15:36 EDT
Umm..  I must explain why I wonder for xscreensaver-base dependency
as xscreensaver maintainer in FE.

(In reply to comment #17))
> Because many of the kde screensavers actually use xscreensaver
What do you mean by "many of the kde screensavers"? 

This means that many KDE screensaver "hacks" (not "daemon") uses xscreensaver
"daemon" (i.e. /usr/bin/xscreensaver) ? 

(In reply to comment #18)
> But do all the xscreensaver-based screensavers actually *work*?  
> I'd venture possibly not, because %configure checks for the hacks 
> location at buildtime, and
> in the absense of that, it may just end up guessing.

Yes, configure of this package tries to check for the path of xscreensaver
hacks, and NO hacks are included in xscreensaver-base. So mockbuild without
xscreensaver-base correctly finds correctly all xscreensaver hacks 
(as all hacks are installed in the two hacks).

xscreensaver-base includes only the files related to xscreensaver "daemon".
And all the hacks packaged in xscreensaver-{extras,gl-extras} can actually
work without xscreensaver-base because all hacks in xscreensaver does NOT use
xscreensaver daemon, they are completely stand-alone.
This is why xscreensaver-extras, xscreensaver-gl-extras does not require
xscreensaver-base (this is very intentional), and gnome-screensaver 
can use xscreensaver-related hacks without xscreensaver-base.

Comment 20 Mamoru TASAKA 2006-10-01 09:19:03 EDT
Oops..

> (as all hacks are installed in the two hacks)
I meant: (as all hacks are installed in the two packages)
Comment 21 Rex Dieter 2006-10-01 14:23:32 EDT
Thanks for the explanation (though I personally would have named the pkgs
differently so that was more clear).

Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/kdeartwork-extras.spec
SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.stable/kdeartwork-extras-3.5.4-4.src

%changelog
* Sun Oct 01 2006 Rex Dieter <rexdieter[AT]users.sf.net> 3.5.4-4
- extras: drop (Build)Requires: xscreensaver-base
Comment 22 Mamoru TASAKA 2006-10-02 10:35:55 EDT
Source doesn't seem to have a copy of GPL license. Please
ask upstream to include GPL copy.

Other things are okay.

----------------------------------------------------------------------
  This package (kdeartwork-extras) is APPROVED by me.
Comment 23 Rex Dieter 2006-10-02 10:49:38 EDT
Thanks.
Comment 24 Rex Dieter 2006-10-02 11:13:35 EDT
imported...

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