Bug 513452 - Review Request: mutter-moblin - Moblin Netbook plugin for Mutter
Summary: Review Request: mutter-moblin - Moblin Netbook plugin for Mutter
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Tom "spot" Callaway
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 506486 506721 506804 506848 507377 508318 511895
Blocks: FedoraMoblin20 518445 518446 518447 518448 518449 518450
TreeView+ depends on / blocked
 
Reported: 2009-07-23 17:35 UTC by Peter Robinson
Modified: 2009-08-20 13:07 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-08-20 13:06:45 UTC
Type: ---
Embargoed:
tcallawa: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Peter Robinson 2009-07-23 17:35:31 UTC
Description of problem:


Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:

Comment 1 Peter Robinson 2009-07-23 17:40:58 UTC
SPEC: http://pbrobinson.fedorapeople.org/mutter-moblin.spec
SRPM: http://pbrobinson.fedorapeople.org/mutter-moblin-0.30.3-1.fc11.src.rpm

Description:
Moblin Netbook plugin for Mutter

Comment 2 Peter Robinson 2009-08-06 14:50:49 UTC
SPEC: as before
SRPM: http://pbrobinson.fedorapeople.org/mutter-moblin-0.32.2-1.fc11.src.rpm

New upstream release with some various cleanups to the spec file.

Comment 3 Peter Robinson 2009-08-07 14:53:09 UTC
SPEC: as before
SRPM: http://pbrobinson.fedorapeople.org/mutter-moblin-0.32.3-1.fc11.src.rpm

New upstream release with some various cleanups to the spec file.

Comment 4 D. Marlin 2009-08-11 22:36:33 UTC
This package will not currently build due to a missing dependency:

from root.log
  DEBUG util.py:256:  No Package Found for bickley-devel

so a full review will have to wait for bickley.


configure will be run twice, once in the prep stage (through autogen.sh) and once in the build stage.

  # run autogen.sh until we have a proper release, but don't run configure twice.
  sed -i 's|echo|exit 0|g' autogen.sh
  ./autogen.sh

The 'sed' change does not appear to work for this autogen.sh script.  I think the following command line change will:

  # run autogen.sh until we have a proper release, but don't run configure twice.
  NOCONFIGURE=yes ./autogen.sh


It appears that the libraries are being included in the files lists twice (duplicate):

  %files -f %{name}.lang
  %defattr(-,root,root,-)
  %doc COPYING README
  %{_libdir}/lib*.so.0*
  %{_libdir}/mutter/plugins/moblin-netbook.so.*
  %{_libdir}/lib*.so.0*				# duplicate
  %{_libexecdir}/moblin-panel*
    :

  %files devel
  %defattr(-,root,root,-)
  %{_libdir}/lib*.so
  %{_libdir}/mutter/plugins/moblin-netbook.so
  %{_libdir}/pkgconfig/*.pc
  %{_libdir}/lib*.so				# duplicate
  %{_includedir}/*

Comment 5 Peter Robinson 2009-08-18 22:21:52 UTC
New upstream release that has split out bickley and a lot of other bits. Both good and bad!

SPEC: As before
SRPM: http://pbrobinson.fedorapeople.org/mutter-moblin-0.34.0-1.fc12.src.rpm
koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1613225

The above points are fixed.

Comment 6 Tom "spot" Callaway 2009-08-19 17:44:20 UTC
Two items of note:

1. Instead of using sed to try to stop configure from running twice (it doesn't work), do this:

# run autogen.sh until we have a proper release, but don't run configure twice.
NOCONFIGURE=true ./autogen.sh

That works because the provided autogen.sh just calls out to gnome-autogen.sh, which has some intelligence.

2. Do you want to pass --enable-scaled-background to configure?

== Review ==

Good:

- rpmlint checks return:
mutter-moblin.x86_64: W: shared-lib-calls-exit /usr/lib64/libmoblin-panel.so.0.0.0 exit.5
mutter-moblin-devel.x86_64: W: no-documentation
mutter-moblin-devel.x86_64: W: dangling-relative-symlink /usr/lib64/mutter/plugins/moblin-netbook.so moblin-netbook.so.3400.0.0

All safe to ignore.

- package meets naming guidelines
- package meets packaging guidelines
- license (GPLv2+) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream (c781f9a6558167658fc7891d1ba8c041db1a75113ef220441c05f11a877436ed)
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- locales handled properly
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- devel package ok
- no .la files
- post/postun ldconfig ok
- devel requires base package n-v-r 

Address the first two issues before commit, but this is APPROVED.

Comment 7 Peter Robinson 2009-08-19 18:55:02 UTC
> Two items of note:
> 
> 1. Instead of using sed to try to stop configure from running twice (it doesn't
> work), do this:
> 
> # run autogen.sh until we have a proper release, but don't run configure twice.
> NOCONFIGURE=true ./autogen.sh
> 
> That works because the provided autogen.sh just calls out to gnome-autogen.sh,
> which has some intelligence.

Thanks. That is very good to know, I've seen so many different work arounds its hard to know which is best.

> 2. Do you want to pass --enable-scaled-background to configure?

I was discussing that one last night with someone, I agree I think its the best option.

I will update them both before commit.

Thanks!

Comment 8 Peter Robinson 2009-08-19 18:58:07 UTC
New Package CVS Request
=======================
Package Name: mutter-moblin
Short Description: Moblin Netbook plugin for Mutter
Owners: pbrobinson
Branches: F-11
InitialCC:

Comment 9 Jason Tibbitts 2009-08-19 21:41:21 UTC
CVS done.

Comment 10 Peter Robinson 2009-08-19 23:29:41 UTC
imported and built.

Comment 11 Peter Robinson 2009-08-20 13:06:45 UTC
Closed as its now in rawhide. Thanks spot!

Will be adding very shortly the packages for the moblin panel that were split out of this package in the 0.34.0 release. They add functionality but the core UI is testable without them. I'll add them with a dep on this as a tracker.


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