Bug 567053
Summary: | Review Request: uzbl - Lightweight WebKit browser following the UNIX philosophy | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Daiki Ueno <dueno> |
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | fedora-package-review, marbolangos, mtasaka, notting, splinux25 |
Target Milestone: | --- | Flags: | mtasaka:
fedora-review+
dennis: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | uzbl-0-0.9.20100221gitabbffe5c3.fc12 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2010-03-26 04:08:47 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: |
Description
Daiki Ueno
2010-02-21 09:44:24 UTC
Some notes: * Epoch - Please don't introduce Epoch * Make build log more verbose - Build log messages like: ------------------------------------------------------------- 38 + make -j4 39 COMPILING src/callbacks.c 40 COMPILING src/events.c 41 COMPILING src/inspector.c 42 COMPILING src/uzbl-core.c 43 ... done. ------------------------------------------------------------- is not useful. We cannot check what is actually happening here, especially we cannot check if Fedora specific compilation flags are correctly honored or not: https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags Please build log more verbose so that we can check if compiler flags are correctly honored. ! Note As far as I checked this package, actually Fedora specific compilation flags are currently not correctly honored, so this needs fixing. * Build failure - Your srpm does not build on F-13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2005910 This is because F-13 changed the options to pass to linker: http://lists.fedoraproject.org/pipermail/devel/2010-February/130519.html https://fedoraproject.org/wiki/UnderstandingDSOLinkChange You can check this by passing "--no-added" option to linker (for this package, you can do by doing $ make CC="gcc -Wl,--no-added") * %makeinstall - Please avoid using this unless impossible: https://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used * Macros - Please use rpmmacros. /usr should be %{_prefix} https://fedoraproject.org/wiki/Packaging/RPMMacros * Desktop files - GUI application should install proper desktop files: https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files * Directories for document files - Currently this package installs document files under: - /usr/share/doc/uzbl-0 - /usr/share/uzbl/docs - /usr/share/uzbl/examples Please consider to unify these (especially "AUTHORS" or so are installed both under the first two directories, so these should be fixed. Ueno-san, would you update this bug? Thanks for the review. The issues should be fixed in -0.4: http://www.unixuser.org/~ueno/software/fedora/uzbl/uzbl-0-0.4.20100221gitabbffe5c3.src.rpm For 0.4: * build process - From build.log: ------------------------------------------------------------------------- 36 Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.yQ4cHw 43 + make -j8 54 cc -o uzbl-core callbacks.o events.o inspector.o uzbl-core.o -pthread -lwebkit-1.0 -lgtk-x11-2.0 -lgdk-x11-2.0 -latk-1.0 -lpangoft2-1.0 -lgdk_pixbuf-2.0 -lpangocairo-1.0 -lcairo -lpango-1.0 -lfreetype -lfontconfig -lsoup-2.4 -lgio-2.0 -lgobject-2.0 -lgmodule-2.0 -lgthread-2.0 -lrt -lglib-2.0 -lX11 -pthread 56 Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.axM2dB 69 + make DESTDIR=/builddir/build/BUILDROOT/uzbl-0-0.4.20100221gitabbffe5c3.i386 PREFIX=/usr install 76 cc -o uzbl-core callbacks.o events.o inspector.o uzbl-core.o -pthread -lwebkit-1.0 -lgtk-x11-2.0 -lgdk-x11-2.0 -latk-1.0 -lpangoft2-1.0 -lgdk_pixbuf-2.0 -lpangocairo-1.0 -lcairo -lpango-1.0 -lfreetype -lfontconfig -lsoup-2.4 -lgio-2.0 -lgobject-2.0 -lgmodule-2.0 -lgthread-2.0 -lrt -lglib-2.0 -lX11 -pthread ------------------------------------------------------------------------- So actually uzbl is compiled twice, once in %build and once in %install. It seems that Makefile does not check if the needed binaries are already rebuilt or not. So either - fix Makefile to prevent duplicate compilation - or just call "make" once (in %install, as binaries need to be installed anyway) * Desktop file - Installed desktop file contains: ------------------------------------------------------------------------- 5 Exec=uzbl 6 Icon=uzbl ------------------------------------------------------------------------- However none of these are installed. * "Example" files - Well, now I tried "uzbl-browser", and it complains: ------------------------------------------------------------------------- $ env LANG=C uzbl-browser ; echo $? cp: cannot stat `/usr/share/uzbl/examples/config/config': No such file or directory Could not copy default config to /home/tasaka1/.config/uzbl/config 3 ------------------------------------------------------------------------- I think the directory name "examples" is really confusing if this directory is actually needed on runtime, however it seems that this directory cannot be moved. Thanks for the review. The issues should be fixed in -0.5: http://www.unixuser.org/~ueno/software/fedora/uzbl/uzbl-0-0.5.20100221gitabbffe5c3.src.rpm On "examples" path, I ended up with keeping the default location as the upstream expects. For -0.5: Almost okay * Timestamps https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps - When using "cp" or "install" commands, please also add "-p" option to keep timestamps on installed files. * Scriptlets for icon cache - Please follow below: https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache * Directory ownership issue https://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Common_Mistakes - Currently the following directories themselves are not owned by any packages. ---------------------------------------------------------------- %{_datadir}/%{name}/ ---------------------------------------------------------------- Thanks. I've just uploaded -0.6 with fixes for them. http://www.unixuser.org/~ueno/software/fedora/uzbl/uzbl-0-0.6.20100221gitabbffe5c3.src.rpm For -0.6: Almost okay. * Directory ownership issue - Your spec file contains: ------------------------------------------------------- 66 %files 70 %{_datadir}/%{name}/ 71 %{_datadir}/%{name}/* ------------------------------------------------------- Now build.log shows warnings like: ------------------------------------------------------- 111 warning: File listed twice: /usr/share/uzbl/examples 112 warning: File listed twice: /usr/share/uzbl/examples/config 113 warning: File listed twice: /usr/share/uzbl/examples/config/config 114 warning: File listed twice: /usr/share/uzbl/examples/config/cookies 115 warning: File listed twice: /usr/share/uzbl/examples/data .... .... ------------------------------------------------------- Note that as https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Common_Mistakes shows, "%{_datadir}/%{name}/" list in %files contains the directory %{_datadir}/%{name} _and_ all directories/files/etc under this directory. Ah, I misread the explanation on the Wiki :-) Just put -0.7: http://www.unixuser.org/~ueno/software/fedora/uzbl/uzbl-0-0.7.20100221gitabbffe5c3.src.rpm Sorry, one more issue * %define -> %global - Now we prefer to use %global instead of %define https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define Please fix this when importing this package into Fedora CVS. ---------------------------------------------------------- This package (uzbl) is APPROVED by mtasaka ---------------------------------------------------------- Thanks. Done in -0.8: http://www.unixuser.org/~ueno/software/fedora/uzbl/uzbl-0-0.8.20100221gitabbffe5c3.src.rpm I'll do CVS admin request shortly. New Package CVS Request ======================= Package Name: uzbl Short Description: Lightweight WebKit browser following the UNIX philosophy Owners: ueno Branches: F-13 F-12 F-11 CVS Done uzbl-0-0.9.20100221gitabbffe5c3.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/uzbl-0-0.9.20100221gitabbffe5c3.fc13 uzbl-0-0.9.20100221gitabbffe5c3.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/uzbl-0-0.9.20100221gitabbffe5c3.fc12 Closing (maybe F-11 webkitgtk is too old for this package to compile) Yes, I should have tested F-11 build before importing the package to CVS. Can I remove the F-11 branch (or is it OK to leave the CVS tree for now)? You can just leave F-11 branch as it is. uzbl-0-0.9.20100221gitabbffe5c3.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report. |