Bug 220967
Summary: | Review Request: libscigraphica - A library of gtk+ widgets for SciGraphica | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Deji Akingunola <dakingun> |
Component: | Package Review | Assignee: | Paul F. Johnson <paul> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | mtasaka |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-02-02 16:57:33 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779, 220968 |
Description
Deji Akingunola
2006-12-29 18:40:29 UTC
I'll give this a shot tonight Requires: %name = %{version} needs to be Requires: %{name} = %{version}-%{release} pkgconfig BuildRequires: gettext perl(XML::Parser) pkgconfig should really be BuildRequires: gettext-devel perl(XML::Parser) you don't need pkgconfig as a main package BR as it's not used, but is in the devel package. gtk+extras-devel doesn't seem to be in FE (rawhide). Is it correctly named? It is, I've built it (libscigraphica) in mock. In response to the other comment, I believe pkgconfig is used in configure process. I'll put the corrected files up in a moment at Spec URL: ftp://czar.eas.yorku.ca/pub/scigraphica/libscigraphica.spec SRPM URL: ftp://czar.eas.yorku.ca/pub/scigraphica/libscigraphica-2.1.1-2.src.rpm Thanks. (In reply to comment #2) > Requires: %name = %{version} > > needs to be > > Requires: %{name} = %{version}-%{release} pkgconfig > > BuildRequires: gettext perl(XML::Parser) pkgconfig > > should really be > > BuildRequires: gettext-devel perl(XML::Parser) > > you don't need pkgconfig as a main package BR as it's not used, but is in the The build actually only relies on binaries in gettext itself, it doesn't compile against gettext libs or use anything fron gettext-devel. Also, as mentioned earlier, libscigraphica checks for pkgconfig when running the configure script. > devel package. > I've added it. Thanks Builds cleanly in mock rpmlint is quiet (warning on the devel package, but it can be ignored) Review Bad: Release: 2 - must have the .%{?dist} tag Requires: %name = %{version}-%{release} - inconsistent naming - should be %{name} %{_libdir}/scigraphica/ - this takes the ownership of the directory which should be owned by scigraphical Good: UTF-8, American English, contains documentation for the main package pkgconfig in R for the -devel package MD5SUM matches upstream No .la or .a files packaged Unsure: Does the -devel need ldconfig to be run? (In reply to comment #6) > Bad: > Release: 2 - must have the .%{?dist} tag > Requires: %name = %{version}-%{release} - inconsistent naming - should be %{name} Fixed > %{_libdir}/scigraphica/ - this takes the ownership of the directory which should > be owned by scigraphical > Maybe not. It just seems to design mistake on the part of upstream to name that directory scigraphica, its a superset of things scigraphica itself installs under it. I've modified scigraphica not to own that directory, but just install it own stuff under it. > Unsure: > Does the -devel need ldconfig to be run? I believe the package guildline says it has to be done for the package and its subpackage that contains a *.so. Corrected spec and srpms below, thanks. Spec URL: ftp://czar.eas.yorku.ca/pub/scigraphica/libscigraphica.spec SRPM URL: ftp://czar.eas.yorku.ca/pub/scigraphica/libscigraphica-2.1.1-3.src.rpm The scigraphica directory must belong to the application and not the library. Well, (In reply to comment #7) > (In reply to comment #6) > > Unsure: > > Does the -devel need ldconfig to be run? > I believe the package guildline says it has to be done > for the package and its > subpackage that contains a *.so. It is not necessary. As when you try "ldconfig -v" and it returns like: ------------------------------------------------- libGL.so.1 -> libGL.so.1.2 ------------------------------------------------- ldconfig checks libraries with somajor if the library has somajor/sominor. and .so files are not (necessary to be) checked. (In reply to comment #8) > The scigraphica directory must belong to the > application and not the library. Umm.. Ownership distribution between libraries <-> applications is of second importance or less. First of all, "all directories created during installation must belong to some package". You can install libscigraphica without any applications which may use this library. %{_libdir}/scigraphica/ _must_ be owned by this package. There are other issues. ---------------------------------- /usr/share/aclocal/libscigraphica-2.0.m4 ---------------------------------- This should be in -devel package and owning aclocal m4 files means -devel package should need "Requires: automake" libscigraphica-2.0.pc says: ----------------------------------- Requires: glib-2.0 gtk+-2.0 gtkextra-2.0 ----------------------------------- This means that -devel package should require ????-devel as well as pkgconfig (Please check "Requires" section of http://fedoraproject.org/wiki/Packaging/Guidelines) And.. this package contains many documentations (xml/header files, etc) and image files and keeping timestamps on these files are strongly recommended. Please keep timestamps on these files. (I have not tried, however for many cases ----------------------------------------------------- make DESTDIR=%{buildroot} INSTALL="%{__install} -c -p" install ------------------------------------------------------ works. Another issue. -------------------------------------------------------------- + make make all-recursive make[1]: Entering directory `/builddir/build/BUILD/libscigraphica-2.1.1' Making all in pixmaps make[2]: Entering directory `/builddir/build/BUILD/libscigraphica-2.1.1/pixmaps' make[2]: Nothing to be done for `all'. make[2]: Leaving directory `/builddir/build/BUILD/libscigraphica-2.1.1/pixmaps' Making all in scigraphica make[2]: Entering directory `/builddir/build/BUILD/libscigraphica-2.1.1/scigraphica' Making all in widgets make[3]: Entering directory `/builddir/build/BUILD/libscigraphica-2.1.1/scigraphica/widgets' if /bin/sh ../../libtool --mode=compile gcc -DHAVE_CONFIG_H -I. -I. -I../.. -I. -I../../pixmaps -I../../scigraphica -I../../scigraphica/dialogs -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/gtkextra-2.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/python2.5 -I/usr/include/python2. 5/Numeric/ -DWITH_NUMERIC_PYTHON -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -g -O -W -Wall -DWITH_WARNINGS -MT gtkplotart.lo -MD -MP -MF ".deps/gtkplotart.Tpo" \ ......... This means that gcc optimizes this code finally by "-O", not "-O2"? If so, this should be fixed. (In reply to comment #9) > > (In reply to comment #8) > > The scigraphica directory must belong to the > > application and not the library. > Umm.. Ownership distribution between libraries <-> applications is > of second importance or less. > > First of all, "all directories created during installation must > belong to some package". You can install libscigraphica without > any applications which may use this library. %{_libdir}/scigraphica/ > _must_ be owned by this package. > So Paul what do think about this? I actually know people that use libscigraphica without needing scigraphica. > There are other issues. > ---------------------------------- All fixed. Happy new year to one and all. Spec URL: ftp://czar.eas.yorku.ca/pub/scigraphica/libscigraphica.spec SRPM URL: ftp://czar.eas.yorku.ca/pub/scigraphica/libscigraphica-2.1.1-4.src.rpm (In reply to comment #10) > Another issue. > 5/Numeric/ -DWITH_NUMERIC_PYTHON -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 > -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 > -mtune=generic -fasynchronous-unwind-tables -g -O -W -Wall -DWITH_WARNINGS -MT > gtkplotart.lo -MD -MP -MF ".deps/gtkplotart.Tpo" \ > ......... > > This means that gcc optimizes this code finally by "-O", not > "-O2"? If so, this should be fixed. This is caused by the configure scripts assuming it building a CVS checkout (the developers have enabled some extra debugging options for CVS builds, and mistakingly shipped a configure script generated in a CVS checkout). Regenerating the configure script should have worked, but it failed because some expected file are not shipped in the released package. I've fixed it and some other issues I noticed while looking at this. Spec URL: ftp://czar.eas.yorku.ca/pub/scigraphica/libscigraphica.spec SRPM URL: ftp://czar.eas.yorku.ca/pub/scigraphica/libscigraphica-2.1.1-5.src.rpm Well, some minor things and issues. Minor: * BuildRequies pkgconfig, gtk2-devel is not necessary. These are (finally) required by gtk+extra-devel ? Group: Development/Libraries (main) Usually "Development" is used for -devel package and the group of this type is "System Environment/Libraries". Should be fixed: * %{_datadir}/pixmaps/%{name}/ is not owned by any package. (In reply to comment #13) > Well, some minor things and issues. > Fixed. Spec URL: ftp://czar.eas.yorku.ca/pub/scigraphica/libscigraphica.spec SRPM URL: ftp://czar.eas.yorku.ca/pub/scigraphica/libscigraphica-2.1.1-6.src.rpm Seemingly okay. I leave this package as the judgment by Paul. One question. What is the .c souce codes included in -devel package? -------------------------------------------------- /usr/include/scigraphica-2.0/scigraphica/algorithms/Axb_core.c /usr/include/scigraphica-2.0/scigraphica/algorithms/lm_core.c -------------------------------------------------- Well, Paul, are you reviewing this? Umm, again ping, Paul? Builds fine in mock, rpmlint is clean (devel gives a warning about no-docs which can be ignored) Review Good Consistent use of macros No permission conflicts / incorrect ownership Docs included No static libs included devel file uses pkgconfig correctly library - doesn't need desktop icon rpm requirements sane APPROVED Imported into cvs and built. Thanks |