Spec URL: http://besser82.fedorapeople.org/libyui/SPECS/libyui.spec SRPM URL: http://besser82.fedorapeople.org/libyui/SRPMS/libyui-3.0.1-1.fc18.src.rpm Description: This is the user interface engine that provides the abstraction from graphical user interfaces (Qt, Gtk) and text based user interfaces (ncurses). Originally developed for YaST, libyui can now be used independently of YaST for generic (C++) applications. The SRPM successfully builds on koji. rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=5334190 F20: http://koji.fedoraproject.org/koji/taskinfo?taskID=5334220 F19: http://koji.fedoraproject.org/koji/taskinfo?taskID=5334213 F18: http://koji.fedoraproject.org/koji/taskinfo?taskID=5334223 Fedora Account System Username: besser82 Hello! This is my first package I want to contribute to Fedora, so I need a sponsor. I am in good contact to upstream, esp. to the lead-maintainer and have direct-push access to the libyui github-repos, too. BR, Björn Esser
Updated to V3.0.3 You can find SPEC/SRPM here: http://besser82.fedorapeople.org/pkg/libyui/ Or clone my git-repo from besser82:public_git/libyui.git and checkout the libyui branch. BR, Björn Esser
A first look at the spec file. Mostly minor issues (related to Fedora Packaging Guidelines): * rpmlint output is clean except for false positives about spelling errors and shared-lib-calls-exit which is known and covered by a git pull request already. > BuildRequires: boost-devel%{?_isa} > BuildRequires: cmake%{?_isa} >= 2.8 > BuildRequires: doxygen%{?_isa} > BuildRequires: fdupes%{?_isa} > BuildRequires: gcc-c++%{?_isa} > BuildRequires: graphviz%{?_isa} > BuildRequires: pkgconfig%{?_isa} Using %_isa in BuildRequires is more confusing than beneficial. First of all, these "BuildRequires" become the src.rpm's "Requires": $ rpm -qpR libyui-3.0.3*.src.rpm boost-devel(x86-64) cmake(x86-64) >= 2.8 doxygen(x86-64) fdupes(x86-64) gcc-c++(x86-64) graphviz(x86-64) pkgconfig(x86-64) rpmlib(FileDigests) <= 4.6.0-1 rpmlib(CompressedFileNames) <= 3.0.4-1 And usually one publishes a single src.rpm in a common "source" repository for all target architectures. The expanded %_isa value is the one from the build environment the src.rpm has been built on. That may be wrong and confusing. It could even be from a build machine with a completely different architecture. For example, "boost-devel(x86-64)" is not available for i686, where the requirement ought to be "boost-devel(x86-32)". It would be strictly required to rebuild the src.rpm to fix its "Requires". Even if no conditional arch-dependent build requirements are used in the spec file. Conclusively: Not using %_isa in BuildRequires is fine and much more common, too. > BuildRequires: gcc-c++%{?_isa} gcc-c++ (as well as gcc and a few more) are expected to be available in the "minimum build environment": https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires -> https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2 > %package devel > > Requires: glibc-devel%{?_isa} > Requires: libstdc++-devel%{?_isa} Same here. Except that are indirectly required by "gcc gcc-c++" already. > Requires: %{name}%{?_isa} = %{version} At Fedora, the base package dependency is more strict and includes %{release}, too. This is because either the base package or the subpackage may be patched, and a strict dependency enforces that they match eachother: https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package > Summary: Header files for %{name} More accurate would be something like Summary: Files needed for developing with %{name} since the package clearly contains more than just the headers. ;-) > %package devel > > URL: https://github.com/%{name}/%{name}/ No need to duplicate the URL tag here. It is inherited from the base package URL. > %package doc > > Group: Development/Libraries > > URL: https://github.com/%{name}/%{name}/ Same here. Plus, group "Documentation" would be appropriate: https://fedoraproject.org/wiki/Packaging:Guidelines#Documentation > %build > CFLAGS="${CFLAGS:-%optflags}" ; export CFLAGS ; > CXXFLAGS="${CXXFLAGS:-%optflags}" ; export CXXFLAGS ; > FFLAGS="${FFLAGS:-%optflags%{?_fmoddir: -I%_fmoddir}}" ; export FFLAGS ; > FCFLAGS="${FCFLAGS:-%optflags%{?_fmoddir: -I%_fmoddir}}" ; export FCFLAGS ; > %{?__global_ldflags:LDFLAGS="${LDFLAGS:-%__global_ldflags}" ; export LDFLAGS ;} > cmake .. \ Check out the %cmake macro which makes all this easier (rpm --eval %cmake): http://fedoraproject.org/wiki/Packaging:Cmake > %install > cd build > rm -rf %{buildroot} It is emptied automatically already: https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag > make DESTDIR=%{buildroot} install Nowadays there's a %make_install macro for that. > install -m0644 Using also option -p can be helpful for packages, not just if they contain old/ancient files: https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps > %clean > rm -rf %{buildroot} https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean > %dir %{_datadir}/%{name} > > -- Theme-Dir: /usr/share/libyui/theme It would make sense to include this [empty] directory in this package, because there is code in the lib that uses this path. A pkgconfig variable is defined for it, too. > %doc %dir %{_defaultdocdir}/%{name}-%{version} It's a directory and never marked as %doc. Files are to be marked as %doc. > %defattr(-,root,root) https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions > %files devel > %doc %dir %{_defaultdocdir}/%{name}-%{version} > %doc %{_defaultdocdir}/%{name}-%{version}/C* https://fedoraproject.org/wiki/Packaging:Guidelines#Duplicate_Files Same for the -doc subpackage. > %{_libdir}/cmake/%{name} https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership https://fedoraproject.org/wiki/Packaging:UnownedDirectories * Doing a test-build with a modified spec file using the %cmake macro, build.log output gets more verbose and reveals that somewhere "-O3 -g3" is being appended: https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
Spec URL: http://besser82.fedorapeople.org/pkg/libyui/libyui.spec SRPM URL: http://besser82.fedorapeople.org/pkg/libyui/libyui-3.0.3-1.fc18.src.rpm Thanks for your review, Michael! Uploaded fixed spec and new SRPM to location above. Please review again. Cheers, Björn.
Almost. ;) * You've missed using %cmake instead of defining all the *FLAGS environment variables yourself. * Some reviewers demand that package submitters increase the "Release" value for every update and add a %changelog entry, also during review: https://fedoraproject.org/wiki/Packaging:FrequentlyMadeMistakes It's good practice and makes running rpmdev-diff and rpmdiff more convenient. * As why you've requested a "hardened build", not sure: https://fedoraproject.org/wiki/Packaging:Guidelines#PIE * Unowned directories fun: libyui has been changed to not include /usr/share/doc/libyui-3.0.3/ -- that directory is "unowned" now: $ rpmls -p libyui-3.0.3-1.fc19.x86_64.rpm|grep ^d drwxr-xr-x /usr/lib64/yui drwxr-xr-x /usr/share/libyui drwxr-xr-x /usr/share/libyui/theme libyui-devel either needs to "Requires: cmake" (only for directory ownership) OR include a %dir entry for /usr/lib64/cmake/ (the latter is permitted): https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership If you choose to include the cmake directory in your package, be especially careful that the file permissions are exactly the same as in the cmake package. libyui-doc has been changed to not include the "html" directory: $ rpmls -p libyui-doc-3.0.3-1.fc19.noarch.rpm|grep ^d drwxr-xr-x /usr/share/doc/libyui-3.0.3/html/search The following patch shall fix the directory issues. Btw, it's okay to include full directory trees as you had done before. Example: 1) %{_includedir}/yui/ 2) %{_includedir}/yui 3) %dir %{_includedir}/yui %{_includedir}/yui/* Those three are equivalent. The Wiki _tries_ to explain it. --- libyui.spec.orig 2013-05-12 22:24:36.700740024 +0200 +++ libyui.spec 2013-05-12 22:26:51.794385741 +0200 @@ -118,6 +118,7 @@ %dir %{_libdir}/yui %dir %{_datadir}/%{name} %dir %{_datadir}/%{name}/theme +%dir %{_defaultdocdir}/%{name}-%{version} %doc %{_defaultdocdir}/%{name}-%{version}/C* @@ -126,6 +127,7 @@ %dir %{_includedir}/yui %{_includedir}/yui/* %{_libdir}/pkgconfig/%{name}.pc +%dir %{_libdir}/cmake %dir %{_libdir}/cmake/%{name} %{_libdir}/cmake/%{name}/* %dir %{_datadir}/%{name}/buildtools @@ -133,7 +135,9 @@ %files doc -%doc %{_defaultdocdir}/%{name}-%{version}/*/* +%dir %{_defaultdocdir}/%{name}-%{version} +%doc %{_defaultdocdir}/%{name}-%{version}/examples/ +%doc %{_defaultdocdir}/%{name}-%{version}/html/ %changelog
Spec URL: http://besser82.fedorapeople.org/pkg/libyui/libyui.spec SRPM URL: http://besser82.fedorapeople.org/pkg/libyui/libyui-3.0.3-2.fc18.src.rpm Updated the package, see %changelog below. Now the build rpms should own all required dirs: drwxr-xr-x /usr/include/yui drwxr-xr-x /usr/lib64/cmake/libyui drwxr-xr-x /usr/lib64/yui drwxr-xr-x /usr/share/doc/libyui-3.0.3 drwxr-xr-x /usr/share/doc/libyui-3.0.3/examples drwxr-xr-x /usr/share/doc/libyui-3.0.3/html drwxr-xr-x /usr/share/doc/libyui-3.0.3/html/search drwxr-xr-x /usr/share/libyui drwxr-xr-x /usr/share/libyui/buildtools drwxr-xr-x /usr/share/libyui/theme /usr/lib64/cmake/ -> -devel Requires: cmake (needed for libyui*-builds anyways) %changelog * Mon May 13 2013 Björn Esser <bjoern.esser> - 3.0.3-2 - fixup as suggested in https://bugzilla.redhat.com/show_bug.cgi?id=959926#c2 - add Patch0 to obey conventions about the compiler flags set in the system rpm configuration. See: https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags - fixup as suggested in https://bugzilla.redhat.com/show_bug.cgi?id=959926#c4 - build a hardened version just in case - add Requires: %{libsuffix}-ui, because %{name} without UI-plugins is as useful as a car without gas and tires. - add -devel Provides: %{libsuffix}-ui to provide a FAKE %{libsuffix}-ui to satisfy dependencies during rpmbuild of the UI-plugins and made sure this is known by documenting this in %{name}-devel description. - add -devel Requires: cmake to solve the /usr/lib*/cmake/ ownership-problem, which is needed for libyui*-builds anyways. * Mon May 06 2013 Björn Esser <bjoern.esser> - 3.0.3-1 - Initial RPM release.
* Minor typo: s/pakage/package/ in the new -devel %description. * About the PIE build, I'm not convinced that a GUI framework should enable those build options (gtk3 and Qt don't do that either), but it's not a blocker. * The following is an example of macro-madness: ;-p Patch0: lib%{libsuffix}_honor_compiler_flags.patch Files (such as these Patch files) typically have a fixed file name, especially since the files get checked into git. Using macros here doesn't make anything more convenient. * It's good that you've made the yui-ui dependency use %_isa, but the idea is somewhat half-baked. Let's see: 1) It's a circular dependency, which can become troublesome. Package libyui-$PLUGIN requires libyui/libyui-devel to build, and libyui requires any libyui-$PLUGIN to even install - or else no UI implementation would be available. You cannot build a first libyui-$PLUGIN, because no plugin package is available to satisfy the yui-ui dep in the libyui package when populating the buildroot. Therefore you would need to bootstrap your builds: - build a first libyui _without_ the yui-ui Requires - build a first libyui-$PLUGIN - rebuild libyui _with_ the yui-ui Req added back And the consequences are that you would need repeat that procedure whenever you want to build an incompatible libyui. Assume you've built an upgrade from libyui.so.5 to libyui.so.6. Any available plugin package would still need the old libyui.so.5, so there would be a broken dependency in the buildroot upon trying to (re)build the plugin packages. 2) It makes no sense to provide a "fake" yui-ui in libyui-devel. *Any* such provider could be used by a dependency resolver to satisfy a requirement found in another package. So, if libyui requires yui-ui, installing libyui-devel would satisfy the dependency _without_ providing a UI. Not just theoretically, it's real. This is what happens during a test-installation: # yum list libyui\* --enablerepo=plague-needsign Loaded plugins: langpacks, refresh-packagekit Available Packages libyui.x86_64 3.0.3-2.fc19 plague-needsign libyui-devel.x86_64 3.0.3-2.fc19 plague-needsign libyui-doc.noarch 3.0.3-2.fc19 plague-needsign libyui-ncurses.x86_64 2.43.8-1.fc19 plague-needsign libyui-ncurses-devel.x86_64 2.43.8-1.fc19 plague-needsign libyui-ncurses-doc.noarch 2.43.8-1.fc19 plague-needsign # yum list libyui\* --enablerepo=plague-needsign Loaded plugins: langpacks, refresh-packagekit Available Packages libyui.x86_64 3.0.3-2.fc19 plague-needsign libyui-devel.x86_64 3.0.3-2.fc19 plague-needsign libyui-doc.noarch 3.0.3-2.fc19 plague-needsign libyui-ncurses.x86_64 2.43.8-1.fc19 plague-needsign libyui-ncurses-devel.x86_64 2.43.8-1.fc19 plague-needsign libyui-ncurses-doc.noarch 2.43.8-1.fc19 plague-needsign [root@localhost ~]# yum install libyui --enablerepo=plague-needsign Loaded plugins: langpacks, refresh-packagekit plague-needsign | 2.9 kB 00:00 !!! Resolving Dependencies --> Running transaction check ---> Package libyui.x86_64 0:3.0.3-2.fc19 will be installed --> Processing Dependency: yui-ui(x86-64) for package: libyui-3.0.3-2.fc19.x86_64 --> Running transaction check ---> Package libyui-devel.x86_64 0:3.0.3-2.fc19 will be installed --> Finished Dependency Resolution Dependencies Resolved ================================================================================ Package Arch Version Repository Size ================================================================================ Installing: libyui x86_64 3.0.3-2.fc19 plague-needsign 192 k Installing for dependencies: libyui-devel x86_64 3.0.3-2.fc19 plague-needsign 102 k Transaction Summary ================================================================================ Install 1 Package (+1 Dependent package) Total download size: 294 k Installed size: 1.2 M Is this ok [y/N]: n Exiting on user command
Spec URL: http://besser82.fedorapeople.org/pkg/libyui/libyui.spec SRPM URL: http://besser82.fedorapeople.org/pkg/libyui/libyui-3.0.3-3.fc18.src.rpm Fixed up critical stuff. %changelog * Tue May 14 2013 Björn Esser <bjoern.esser> - 3.0.3-3 - removed macro from Patch0. - fixed typo -> s/pakage/package/ - removed Provides/Requires: yui_ui <snip>
Spec URL: http://besser82.fedorapeople.org/pkg/libyui/libyui.spec SRPM URL: http://besser82.fedorapeople.org/pkg/libyui/libyui-3.0.4-1.fc18.src.rpm New upstream version. Thanks for your review. %changelog * Wed May 15 2013 Björn Esser <bjoern.esser> - 3.0.4-1 - new upstream version - add Patch1 to skip generation of pdf-docs if doxygen-latex is installed. <snip>
Created attachment 748422 [details] one basic way to toggle the yui-ui dependency Okay, currently the yui-ui circular dep is missing, but you've said you will think about ways how to add it. Beyond that: APPROVED
New Package SCM Request ======================= Package Name: libyui Short Description: GUI-abstraction library Owners: besser82 Branches: f19
Git done (by process-git-requests).
libyui-3.0.4-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/libyui-3.0.4-1.fc19
Package Change Request ====================== Package Name: libyui New Branches: f18 Owners: besser82
libyui-3.0.5-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/libyui-3.0.5-1.fc19
libyui-3.0.5-1.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/libyui-3.0.5-1.fc18
libyui-3.0.5-1.fc18 has been pushed to the Fedora 18 stable repository.
libyui-3.0.5-1.fc19 has been pushed to the Fedora 19 stable repository.
Package Change Request ====================== Package Name: libyui New Branches: el6 Owners: besser82 Want to build on el6, too.