Spec URL: http://directory.fedoraproject.org/sources/adminutil.spec SRPM URL: http://directory.fedoraproject.org/sources/adminutil-1.1.0-1.src.rpm Description: adminutil is used to administer Fedora Directory Server, usually in conjunction with the admin server. It is broken into two libraries - libadminutil contains the basic functionality, and libadmsslutil contains SSL versions and wrappers around the basic functions. The PSET functions allow applications to store their preferences and configuration parameters in LDAP, without having to know anything about LDAP. The configuration is cached in a local file, allowing applications to function even if the LDAP server is down. The other code is typically used by CGI programs used for directory server management, containing GET/POST processing code as well as resource handling (ICU ures API). Source tarball: http://directory.fedoraproject.org/sources/adminutil-1.1.0.tar.bz2
missing BuildRequires: cyrus-sasl-devel Shared Objects are not versioned. with correct BuildRequires builds in mock. rpmlint is quiet
(In reply to comment #1) > missing BuildRequires: cyrus-sasl-devel Ok. > Shared Objects are not versioned. I don't know what this means. I'm using standard autotools with libtool. Is there some libtool flag I need to pass in? > > with correct BuildRequires builds in mock. rpmlint is quiet
(In reply to comment #2) > > Shared Objects are not versioned. > > I don't know what this means. I'm using standard autotools with libtool. Is > there some libtool flag I need to pass in? /me reads info libtool - the -version-info flag - is this what you mean?
i think that what im refering to is `-version-number MAJOR[:MINOR[:REVISION]]' If OUTPUT-FILE is a libtool library, compute interface version information so that the resulting library uses the specified major, minor and revision numbers. This is designed to permit libtool to be used with existing projects where identical version numbers are already used across operating systems. New projects should use the `-version-info' flag instead. the .so should be .so.1.1.0 not .so.0.0.0
(In reply to comment #4) > i think that what im refering to is > > `-version-number MAJOR[:MINOR[:REVISION]]' ... > numbers are already used across operating systems. New projects > should use the `-version-info' flag instead. > > the .so should be .so.1.1.0 not .so.0.0.0 The description says "New projects should use the -version-info flag instead." I see a real mix of usage in /usr/lib - some libs use -version-number, some use -version-info, some are just .so with no version (or version in the name e.g. libnspr4.so). Is there a page on the fedoraproject wiki that describes library naming and versioning conventions?
The libtool doc is wrong. The versioning scheme implemented by -version-info not only doesn't match how ELF works, it doesn't match how _any_ extant executable format works. It's a fiction. Use -version-number.
New version, with several bug fixes and review issues addressed. Spec URL: http://directory.fedoraproject.org/sources/adminutil.spec SRPM URL: http://directory.fedoraproject.org/sources/adminutil-1.1.1-1.src.rpm Source tarball: http://directory.fedoraproject.org/sources/adminutil-1.1.1.tar.bz2
source files match upstream: 3f3aedc553e9f4eb9c0a1e9e6e047bf4 adminutil-1.1.1.tar.bz2 3f3aedc553e9f4eb9c0a1e9e6e047bf4 ../SOURCES/adminutil-1.1.1.tar.bz2 package meets naming and versioning guidelines. specfile is properly named, is cleanly written and uses macros consistently. dist tag is present. build root is correct. %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) license field matches the actual license. license is open source-compatible. LGPL License text included in package. latest version is being packaged. compiler flags are appropriate. %clean is present. package builds in mock ( devel x86_64). package installs properly debuginfo package looks complete. rpmlint says W: adminutil-devel no-documentation owns the directories it creates. doesn't own any directories it shouldn't. no duplicates in %files. file permissions are appropriate. no scriptlets present. code, not content. documentation is small, so no -docs subpackage is necessary. %docs are not necessary for the proper functioning of the package. no libtool .la droppings. not a GUI app. Needs work: final provides and requires are sane: all devel packages with .pc packages need to Requires: pkgconfig pkgconfig should not be BuildRequires it is provided by the devel packages that are BuildRequires and provide .pc files i filed bug# 240516 # cyrus-sasl-devel is required because mozldap uses it BuildRequires: cyrus-sasl-devel mozldap-devel should require it if this is the case. Fix these small issues and i will approve adminutil
New version, fixes issues mentioned in comment#8 Spec URL: http://directory.fedoraproject.org/sources/adminutil.spec SRPM URL: http://directory.fedoraproject.org/sources/adminutil-1.1.1-2.src.rpm Source tarball: http://directory.fedoraproject.org/sources/adminutil-1.1.1.tar.bz2
This needs to go now that you fixed mozldap-devel # cyrus-sasl-devel is required because mozldap uses it BuildRequires: cyrus-sasl-devel pkgconfig should not be a Requires of the main package but of the -devel package no need to BR pkgconfig
Created attachment 155166 [details] diffs Does this look ok?
Yes thats it.
New version, fixes issues mentioned in comment#10 Spec URL: http://directory.fedoraproject.org/sources/adminutil.spec SRPM URL: http://directory.fedoraproject.org/sources/adminutil-1.1.1-2.src.rpm Source tarball: http://directory.fedoraproject.org/sources/adminutil-1.1.1.tar.bz2 Ready to import?
* What are the lines of "BuildRequires" on -devel package? * Is this package parellel make unsafe?
(In reply to comment #14) > * What are the lines of "BuildRequires" on -devel package? They are the packages required to build the adminutil-devel package? > * Is this package parellel make unsafe? Not that I know of. Why?
(In reply to comment #15) > (In reply to comment #14) > > * What are the lines of "BuildRequires" on -devel package? > They are the packages required to build the adminutil-devel package? There seems to be some confusion. "BuildRequires" is the packages which are required to rebuild this (adminutil) package and they should be written at the part of the summary of main package (as you wrote them). There is no need to write duplicate BuildRequires. What is actually needed is to write "Requires" for -devel package. For example, adminutil.pc.in contains the line: ----------------------------------------------- Requires: nspr, nss, svrcore, mozldap, icu ----------------------------------------------- This means -devel package should have "Requires" (not BuildRequires) nspr-devel, nss-devel, ........ Check the section "Requires" of http://fedoraproject.org/wiki/Packaging/Guidelines > > > * Is this package parellel make unsafe? > > Not that I know of. Why? Please refer to the section "Parallel make" of http://fedoraproject.org/wiki/Packaging/Guidelines
Created attachment 155254 [details] spec file diffs Do these diffs address the issues in comment #16?
I cannot check if your comment 17 is sufficient until I can rebuild this package with mock successfully. However, I have one question about if -devel package should have "Requires: icu". Would you explain why this is needed? ("Requires: icu" in pkgconfig .pc file is satisfied with libicu-devel)
Created attachment 155258 [details] removed requires: icu Removed Requires: icu Does this look ok?
Created attachment 155265 [details] rpmlint for (installed) adminutil Still not enough * undefined non-weak symbols - libadmsslutil.so.1.1.1 contains undefined non-weak symbols. This is not allowed for packages to provide -devel subpackages because linkage against this library fails due to these symbols. * Timestamps - -devel package contains many header files (i.e. text files which are not created or modified during rebuild) and keeping timestamps on these files are recommend. Please use: --------------------------------------------------- make install DESTDIR=$RPM_BUILD_ROOT INSTALL="%{__install} -p" --------------------------------------------------- * macros consistency - Please determine to use macros or not to use macros for binaries. Please don't use both "rm" and "%{__rm}" If you want to use %{__rm}, please also use %{__make}, for example.
One question: Is the license of lib/libadminutil/acclanglist.c compatible with LGPL (not GPL) ? What does the following mean? (sorry, I am not legal specialist) ------------------------------------------------- Non-GPL Code permitted under this exception must only link to the code of this Program through those well defined interfaces identified in the file named EXCEPTION found in the source code files (the "Approved Interfaces"). --------------------------------------------------
Created attachment 155295 [details] more diffs
(In reply to comment #20) > Still not enough > * undefined non-weak symbols > - libadmsslutil.so.1.1.1 contains undefined non-weak > symbols. This is not allowed for packages to provide > -devel subpackages because linkage against this > library fails due to these symbols. Fixed - added linkage against libadminutil.la > * Timestamps > - -devel package contains many header files (i.e. text > files which are not created or modified during rebuild) > and keeping timestamps on these files are recommend. > Please use: > --------------------------------------------------- > make install DESTDIR=$RPM_BUILD_ROOT INSTALL="%{__install} -p" > --------------------------------------------------- Done. > * macros consistency > - Please determine to use macros or not to use macros > for binaries. Please don't use both "rm" and "%{__rm}" > If you want to use %{__rm}, please also use %{__make}, > for example. Done.
(In reply to comment #21) > One question: > Is the license of lib/libadminutil/acclanglist.c > compatible with LGPL (not GPL) ? What does > the following mean? (sorry, I am not legal specialist) Sorry. That file was copied from Fedora Directory Server to adminutil. I have modified the license to use LGPL instead of the FDS license (GPL + exception). This is ok since we (Red Hat) control the copyright.
Well, * Patching Makefile.am requires - to call "autoreconf" in %build stage - to add "Requires: libtool" - And please write "Patch0: ..." and "%patch0 ....." * For Requires problem of mozldap-devel against cyrus-sasl-devel, please see my comment (bug 240516 comment 2)
(In reply to comment #25) > Well, > > * Patching Makefile.am requires > - to call "autoreconf" in %build stage > - to add "Requires: libtool" > - And please write "Patch0: ..." and "%patch0 ....." I will commit the changes upstream (I am the upstream maintainer), so no patch required. There is a shell script called autogen.sh which makes sure the system has the correct version of the autotools before running autoreconf. > > * For Requires problem of mozldap-devel against cyrus-sasl-devel, > please see my comment (bug 240516 comment 2)
(In reply to comment #26) > (In reply to comment #25) > > Well, > > > > * Patching Makefile.am requires > > - to call "autoreconf" in %build stage > > - to add "Requires: libtool" > > - And please write "Patch0: ..." and "%patch0 ....." > > I will commit the changes upstream (I am the upstream maintainer), > so no patch > required. There is a shell script called autogen.sh which > makes sure the system > has the correct version of the autotools before running autoreconf. Okay. Then if you release new tarball (1.1.2), it is not a problem. > > * For Requires problem of mozldap-devel against cyrus-sasl-devel, > > please see my comment (bug 240516 comment 2) So please add "BuildRequires: cyrus-sasl-devel" to this package (adminutil).
(In reply to comment #27) > (In reply to comment #26) > > (In reply to comment #25) > > > Well, > > > > > > * Patching Makefile.am requires > > > - to call "autoreconf" in %build stage > > > - to add "Requires: libtool" > > > - And please write "Patch0: ..." and "%patch0 ....." > > > > I will commit the changes upstream (I am the upstream maintainer), > > so no patch > > required. There is a shell script called autogen.sh which > > makes sure the system > > has the correct version of the autotools before running autoreconf. > > Okay. Then if you release new tarball (1.1.2), it is not a problem. Is it necessary to re-version from 1.1.1 to 1.1.2? 1.1.1 has not yet been officially released. I'd like to get all of these changes for fedora extras into 1.1.1. > > > * For Requires problem of mozldap-devel against cyrus-sasl-devel, > > > please see my comment (bug 240516 comment 2) > So please add "BuildRequires: cyrus-sasl-devel" to this package > (adminutil). But I thought since adminutil does not use any cyrus-sasl include files nor link directly to any cyrus-sasl libs, it doesn't need BuildRequires: cyrus-sasl-devel?
(In reply to comment #28) > (In reply to comment #27) > > (In reply to comment #26) > > > (In reply to comment #25) > > Okay. Then if you release new tarball (1.1.2), it is not a problem. > > Is it necessary to re-version from 1.1.1 to 1.1.2? 1.1.1 has not yet been > officially released. I'd like to get all of these changes for fedora extras > into 1.1.1. I didn't know it... > > > > > * For Requires problem of mozldap-devel against cyrus-sasl-devel, > > > > please see my comment (bug 240516 comment 2) > > So please add "BuildRequires: cyrus-sasl-devel" to this package > > (adminutil). > > But I thought since adminutil does not use any > cyrus-sasl include files nor link > directly to any cyrus-sasl libs, it doesn't need > BuildRequires: cyrus-sasl-devel? Then please fix configure or the option of configre. Actually the lastest tarball I have locally checks the existence of sasl.h and mock build fails without cyrus-sasl-devel. ----------------------------------------------------- configure: checking for sasl... checking for --with-sasl... no checking for --with-sasl-inc... no checking for --with-sasl-lib... no checking for sasl.h... no configure: error: sasl not found, specify with --with-sasl. error: Bad exit status from /var/tmp/rpm-tmp.16648 (%build) RPM build errors: Bad exit status from /var/tmp/rpm-tmp.16648 (%build) ---------------------------------------------------------
(In reply to comment #29) > > But I thought since adminutil does not use any > > cyrus-sasl include files nor link > > directly to any cyrus-sasl libs, it doesn't need > > BuildRequires: cyrus-sasl-devel? > > Then please fix configure or the option of configre. > Actually the lastest tarball I have locally checks the existence of > sasl.h and mock build fails without cyrus-sasl-devel. > ----------------------------------------------------- > configure: checking for sasl... > checking for --with-sasl... no > checking for --with-sasl-inc... no > checking for --with-sasl-lib... no > checking for sasl.h... no > configure: error: sasl not found, specify with --with-sasl. > error: Bad exit status from /var/tmp/rpm-tmp.16648 (%build) Ok. This is the problem. The actual package with just the shared libs does not need sasl to build, because it doesn't build an executable. However, a developer may build tests, and since the tests are executables, they must be linked against sasl: gcc -g -O2 -o .libs/psetread tests/psetread-psetread.o ./.libs/libadmsslutil.so /share/adminutil/built/.libs/libadminutil.so -L/usr/lib64/dirsec -L/usr/lib64 ./.libs/libadminutil.so -lplc4 -lplds4 -lnspr4 -lssl3 -lnss3 -lsoftokn3 -lssldap60 -lldap60 -lprldap60 -lldif60 -licui18n -licuuc -licudata -Wl,--rpath -Wl,/home/rich/11srv/lib /usr/lib64/libldap60.so: undefined reference to `sasl_client_step' ... /usr/lib64/libldap60.so: undefined reference to `sasl_encode' collect2: ld returned 1 exit status make: *** [psetread] Error 1 So I suppose the real solution here will be to add another configure option e.g. --enable-tests. Then I'll have to see if there is some way I can conditionally include the m4/sasl.m4 file in configure.ac.
Created attachment 155392 [details] diffs Add --enable-tests configure option. By default this is on. If enable-tests is on, then sasl will be used, otherwise, sasl will not be used. However, for rpm packaging, we don't care about the tests, so we use --disable-tests, which skips the sasl stuff.
Well, I want to check if your patch in comment 31 works on mockbuild, however it seems that all my domestic mirrors (i.e. mirror servers in Japan) don't work well now...
Created attachment 155467 [details] mock build log for -3 Okay, patch on comment 31 seems okay for me.
Created attachment 155491 [details] cvs commit log New version, should be the final one Spec URL: http://directory.fedoraproject.org/sources/adminutil.spec SRPM URL: http://directory.fedoraproject.org/sources/adminutil-1.1.1-3.src.rpm Source tarball: http://directory.fedoraproject.org/sources/adminutil-1.1.1.tar.bz2 I'm going to be out next week. Margaret, if this is approved, can you do the cvs-import and create the branches? Or someone else? Otherwise, it will have to wait until the 5th at the earliest.
Dennis, ping?
I will unset the reviewer of this bug if no response is received from the current submitter within ONE WEEK.
By submitter do you mean me? Is there any other information you need from me?
(In reply to comment #37) > By submitter do you mean me? Is there any other information you need from me? s|submitter|reviewer|, sorry... Again: I will unset the reviewer of this bug if no response is received from the current reviewer within 6 days.
Builds in mock. Looks good to me Approved Mamoru thanks for your input
New Package CVS Request ======================= Package Name: adminutil Short Description: Utility library for Fedora Directory Server administration Owners: rmeggins,mlum Branches: FC-5 FC-6 F-7 devel InitialCC: Note that if no new branches are being created for FC-5, that's fine.
cvs done. Yes, no new FC-5 branches, I did the rest.
Thanks. I'm following these instructions: http://fedoraproject.org/wiki/PackageMaintainers/NewPackageProcess I did the cvs add, commit, and make tag in FC-6, F-7, and devel. I did a make build in FC-6 and it succeeded. However, F-7 and devel fail to build: http://koji.fedoraproject.org/koji/taskinfo?taskID=44132 BuildError: package adminutil not in list for tag dist-f8 http://koji.fedoraproject.org/koji/taskinfo?taskID=44119 BuildError: package adminutil not in list for tag dist-fc7-updates-candidate
Woohoo! Built successfully on all branches! Thanks to everyone for the help.