Bug 189482 - Review Request: pcb - An interactive printed circuit board editor
Review Request: pcb - An interactive printed circuit board editor
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-04-20 09:06 EDT by Peter Jones
Modified: 2009-09-09 12:23 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-07-10 13:50:54 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Peter Jones 2006-04-20 09:06:23 EDT
Spec URL: http://people.redhat.com/pjones/packages/pcb.spec
SRPM URL: http://people.redhat.com/pjones/packages/pcb-0.20060414-1.src.rpm
Description: PCB is an interactive printed circuit board editor for the X window system.
Comment 1 Jochen Schmitt 2006-04-20 11:22:25 EDT
Good:

+ Local build worked fine.

Bad:
- source0 doesn't contains a fulled qualified URL.
- BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
- No menu entry for GUIs exists
- Mock build fails with:

checking for cygwish83... no
checking for cygwish80... no
configure: error: Did not find the wish executible.  You need to make sure
        that tcl is installed on your system and that wish is in your path
error: Bad exit status from /var/tmp/rpm-tmp.14070 (%build)


I asume, that tcl should be a BuildRequires.

- rpmlint on source rpm complaints:
W: pcb summary-ended-with-dot An interactive printed circuit board editor.
W: pcb strange-permission pcb.spec 0600

- rpmlint on binary rpm complaints:
W: pcb summary-ended-with-dot An interactive printed circuit board editor.
E: pcb info-files-without-install-info-postin /usr/share/info/pcb.info.gz
E: pcb info-files-without-install-info-postun /usr/share/info/pcb.info.gz
E: pcb non-executable-script /usr/share/pcb/tools/PCB2HPGL 0644
W: pcb devel-file-in-non-devel-package /usr/share/pcb/tools/gerbertotk.c
E: pcb info-files-without-install-info-postin /usr/share/info/pcb.info-2.gz
E: pcb info-files-without-install-info-postun /usr/share/info/pcb.info-2.gz
E: pcb standard-dir-owned-by-package /usr/share/man/man1
E: pcb non-executable-script /usr/share/pcb/tools/Merge_dimPCBPS 0644
E: pcb non-executable-script /usr/share/pcb/tools/tgo2pcb.tcl 0644
E: pcb info-files-without-install-info-postin /usr/share/info/pcb.info-1.gz
E: pcb info-files-without-install-info-postun /usr/share/info/pcb.info-1.gz
E: pcb non-executable-script /usr/share/pcb/tools/MergePCBPS 0644

- Package doesn't contains a verbatin written copy of the license text.
Comment 2 Peter Jones 2006-04-20 19:21:29 EDT
Ok, all of these things are fixed, and a new src.rpm and spec are uploaded. 
Additionally, rpmlint runs clean on both src an binary rpm.
Comment 3 Peter Jones 2006-04-20 19:23:30 EDT
Oh, actually, there's still no menu entry.  Are there docs on that someplace?
Comment 5 Christian Iseli 2006-04-20 19:43:50 EDT
Please do not import packages into CVS before they are marked FE-ACCEPT.
Comment 6 Brian Pepple 2006-04-20 19:50:47 EDT
Looks like you've got some unnecessary BuildRequires.

Duplicate BuildRequires: m4 (by bison), glib2-devel (by gtk2-devel), pkgconfig
(by glib2-devel), libjpeg-devel (by gd-devel), freetype-devel (by gd-devel),
zlib-devel (by gd-devel), libpng-devel (by gd-devel), netpbm-progs (by tetex-latex)
Comment 7 Peter Jones 2006-04-20 20:05:51 EDT
OK, desktop file added, and also fixed the extra buildrequires.
Comment 8 Jochen Schmitt 2006-04-23 15:39:19 EDT
Bad:

- Use of buildroot is not consistant
- Duplicate BuildRequires: libjpeg-devel (by gd-devel)
- Mock build failed:
checking for wish8.3... no
checking for wish80... no
checking for wish8.0... no
checking for cygwish83... no
checking for cygwish80... no
configure: error: Did not find the wish executible.  You need to make sure
        that tcl is installed on your system and that wish is in your path
error: Bad exit status from /var/tmp/rpm-tmp.58193 (%build)


Comment 9 Hans de Goede 2006-06-18 03:43:06 EDT
It has been almost 2 full months since the last comment by Jochen, Peter can you
post a new version which does build in Mock, or are you no longer interested?
Comment 10 Chitlesh GOORAH 2006-07-08 08:19:19 EDT
Hello there,

It's nearly 3 months, now.
PCB is an application, I personnally will be dependent on since Im an electronic
student.

Im asking whether I can take over it, can I ?
I have already a spec file 
 - with the latest release 20060422
 - rpmlint does not complain
 - builds successfully under mock (i386)

SPEC : http://beta.glwb.info/pcb/pcb.spec
SRPM : http://beta.glwb.info/pcb/pcb-0.20060422-1.src.rpm
Comment 11 Hans de Goede 2006-07-08 08:57:38 EDT
Since almost a month has past since my last ping without any reply, I think its
reasonable for you to take over. I'll do a review as / when time permits. I
dunno if its wise todo this here though, maybe it would be better to close this
ticket and submit a new review request, anyone any opinions / advice on this?
Comment 12 Hans de Goede 2006-07-09 06:41:20 EDT
Since no one has commented lets just handle things in this Review Request. Here
is a full Review for Chitlesh's version, not Peter's! :

MUST:
=====
* rpmlint output is:
E: pcb info-dir-file /usr/share/info/dir
This must be fixed, add "rm -f ${RPM_BUILD_ROOT}%{_infodir}/dir" in %install
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License (GPL) ok, license fileS included
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on devel-x86_64
* BR: ok
* No locales
* No shared libraries
* Not relocatable
* Package owns / or requires all dirs (with some strangeness see Must fix
below)
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code.
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* no -devel package needed, no libs / .la files.
* .desktop file as required and properly installed


MUST fix:
=========
* rpmlint output is:
E: pcb info-dir-file /usr/share/info/dir
This must be fixed, add "rm -f ${RPM_BUILD_ROOT}%{_infodir}/dir" in %install.
Without this fixed installing it gives:
Preparing...                ########################################### [100%]
        file /usr/share/info/dir from install of pcb-0.20060422-1 conflicts with
file from package info-4.8-11
You probaly didn't see this on your system because the dir file getting
generated depends on the system configuration on which the package is build,
hence the -f to rm, so that if the dir file isn't there it doesn't cause an error.
* You are missing the following requires for the texinfo script:
Requires(post): /sbin/install-info
Requires(preun): /sbin/install-info


Should fix:
===========
* Add an icon and install it under:
  %{_datadir}/icons/hicolor/32x32/apps
  Where 32x32 is the size of the icon, please do ls /usr/share/icons/hicolor/
  to see the available valid sizes, if the icon doesn't match any pick the 
  closest.
* Once the icon is in the proper plase you must add %post(un) script to update 
  the icon-cache see:
http://fedoraproject.org/wiki/ScriptletSnippets#head-fc74f078205565f961f6d836b77c3428619c689d
Comment 13 Chitlesh GOORAH 2006-07-09 08:17:00 EDT
Does this mean that I am the _official_ packager of pcb now ?

SPEC : http://beta.glwb.info/pcb/pcb.spec
SRPM : http://beta.glwb.info/pcb/pcb-0.20060422-2.src.rpm

%changelog
- fixed E: pcb info-dir-file /usr/share/info/dir
- added /sbin/install-info as requires for %%post and %%preun
- added icon and treated GTK+ icon cache as required

I was not quite sure how to deal with the icon, so i put it in %post.
Comment 14 Hans de Goede 2006-07-09 16:21:21 EDT
(In reply to comment #13)
> Does this mean that I am the _official_ packager of pcb now ?
> 

I guess it does, there is no official procedure for this, but packages have been
taken over while in review like tihs before.

> SPEC : http://beta.glwb.info/pcb/pcb.spec
> SRPM : http://beta.glwb.info/pcb/pcb-0.20060422-2.src.rpm
> 
> %changelog
> - fixed E: pcb info-dir-file /usr/share/info/dir
> - added /sbin/install-info as requires for %%post and %%preun
> - added icon and treated GTK+ icon cache as required
> 
> I was not quite sure how to deal with the icon, so i put it in %post.
Erm, thats wrong, really wrong, on other peoples systems %{SOURCE2} won't even
be available the commented line at the end of %install is how it should be done,
you probably need to put a:
mkdir -p $RPM_BUILD_ROOT%{_datadir}/icons/hicolor/32x32/apps
above that line to make it to work.

Once the icon installation is ok (and completly removed from %post) the package
is ok and I'll approve it.


Comment 15 Chitlesh GOORAH 2006-07-09 17:23:32 EDT
thanks,
Updated:
SPEC : http://beta.glwb.info/pcb/pcb.spec
SRPM : http://beta.glwb.info/pcb/pcb-0.20060422-3.src.rpm
Comment 16 Hans de Goede 2006-07-10 00:31:01 EDT
Looks good now, approved!
Comment 17 Chitlesh GOORAH 2008-11-29 13:19:01 EST
New Package CVS Request
=======================
Package Name: pcb
Short Description: An interactive printed circuit board editor
Owners: chitlesh
Branches: EL-5
Comment 18 Kevin Fenzi 2008-12-01 17:02:24 EST
cvs done.
Comment 19 Chitlesh GOORAH 2009-09-09 06:26:40 EDT
Package Change Request
======================
Package Name: pcb
Short Description: An interactive printed circuit board editor
New Branches: F-12
Owners:chitlesh
Comment 20 Kevin Fenzi 2009-09-09 12:23:17 EDT
cvs done.

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