Spec URL: http://roshansingh.fedorapeople.org/opendchub/opendchub.spec SRPM URL: http://roshansingh.fedorapeople.org/opendchub/opendchub-0.8.1-1.fc12.src.rpm Description: Opendchub is the hub software for Direct Connect P2P network. It runs as a daemon.
Hi, here is an informal review of your package, while waiting to be sponsored ^^. Just a few remarks about BR, the general look of the .spec seems pretty good ^^. * since glibc-devel is a dependancy of gcc, and since gcc is part of the minimal build system in Fedora (see https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2), there is no need to set glibc-devel as a BuildRequires. * However, after checking the configure.in, I think you'll need to set perl-devel as a BuildRequires. * By the way, according to the file configure.in, the BR libcap-devel is only required if the option « --enable-switch_user » is explicitely called. Otherwise it is useless. If you think that this option is useful, it may be a good thing to enable it, to offer as many enabled features as possible in your binary. You should check that you have defined all the required BuildRequires with mock: http://fedoraproject.org/wiki/Extras/MockTricks
Hi, Thanks for reviewing my package. I have removed glibc-devel. > * However, after checking the configure.in, I think you'll need to set > perl-devel as a BuildRequires. perl-ExtUtils-Embed needs perl-devel as its dependency, so i have not included it in BuildRequires. > * By the way, according to the file configure.in, the BR libcap-devel is only > required if the option « --enable-switch_user » is explicitely called. > Otherwise it is useless. > If you think that this option is useful, it may be a good thing to enable it, > to offer as many enabled features as possible in your binary. Thank you for noticing this. I have included it: %configure --enable-switch_user I have checked it with mock, it is running fine. However it was first time that I was using mock. I would like you to take a look :-). Link to updated spec and srpm: SPEC: http://roshansingh.fedorapeople.org/opendchub/opendchub.spec SRPM: http://roshansingh.fedorapeople.org/opendchub/opendchub-0.8.1-2.fc12.src.rpm
> perl-ExtUtils-Embed needs perl-devel as its dependency, so i have not included > it in BuildRequires. You're right, excellent :) mock runs find indeed, and rpmlint is silent on your SRPM, your binary RPM and the debuginfo one. I don't see any problem, I think your RPM is OK :-) Just a little comment about the Group tag: isn't it more appropriate to set it to " Applications/Internet"? Just my personal opinion about your Description also, but I find it a little bit "laconic" ^^. Maybe you could use the description provided on the opendchub home page to make it more precise ^^.
(In reply to comment #3) > Applications/Internet"? > Just my personal opinion about your Description also, but I find it a little > bit "laconic" ^^. Maybe you could use the description provided on the opendchub > home page to make it more precise ^^. I am the maintainer of the project as well. Thanks for the suggestion, I was also not sure in which group I should put it to. I think this should fix all the issues. New spec and srpm: SPEC: http://roshansingh.fedorapeople.org/opendchub/opendchub.spec SRPM: http://roshansingh.fedorapeople.org/opendchub/opendchub-0.8.1-3.fc12.src.rpm
Well, * perl module dependency https://fedoraproject.org/wiki/Packaging/Perl#Perl_Requires_and_Provides - When writing perl module rpm dependency as (Build)Requires, use the virtual provides on the package, not the rpm name itself (i.e. use "BuildRequires: perl(ExtUtils::Embed)" * creating directory under home directory - configure.in reads: ----------------------------------------------------------- 60 dnl Check if config directory exists. 61 if test ! -d $HOME/.opendchub; then 62 echo "creating config directory: $HOME/.opendchub" 63 mkdir $HOME/.opendchub 64 chmod 700 $HOME/.opendchub; 65 fi 66 75 if test "$ENABLE_PERL" = "yes"; then 76 AC_CHECK_PROG(ENABLE_PERL, perl, yes, no) 77 if test "$ENABLE_PERL" = "no"; then 78 AC_MSG_WARN(Perl wasn't found. Scripting will be disabled.) 79 else 85 dnl Check if script directory exists. 86 dnl Creates it and copies sample scripts to it if it doesn't. 87 if test ! -d $HOME/.opendchub/scripts; then 88 echo "creating script directory: $HOME/.opendchub/scripts" 89 mkdir $HOME/.opendchub/scripts 90 chmod 700 $HOME/.opendchub/scripts; 91 for i in Samplescripts/*; do 92 cp $i $HOME/.opendchub/scripts; 93 done 94 fi 95 fi 96 else 97 echo "Perl script support is disabled." 98 fi ----------------------------------------------------------- and build.log says: ----------------------------------------------------------- 129 Switch user support is enabled. 130 creating config directory: /builddir/.opendchub 131 checking for perl... yes 132 creating script directory: /builddir/.opendchub/scripts ----------------------------------------------------------- Well, - build process should not create any directories under home directory. If these directories are needed, they should be created when the program is actually executed. Also Fedora forbids to create directories under home directory during build process: https://fedoraproject.org/wiki/Packaging/Guidelines#Scriplets_are_only_allowed_to_write_in_certain_directories - Furthermore, these created directories are not installed when installing this software with rpm anyway. So this behavior (i.e creating directories under home directory and installing some scripts under there during build) should be suppressed. - By the way maybe Samplescripts/ should be included as %doc. * %changelog - It is recommended (and useful when using Fedora CVS) to insert one line between each %changelog entry like: --------------------------------------------------------- * Sun Jan 3 2009 Roshan Kumar Singh <singh.roshan08> 0.8.1-3 - Changed Group to a more appropriate one * Sun Jan 3 2009 Roshan Kumar Singh <singh.roshan08> 0.8.1-2 - Removed glibc-devel and added perl-devel to BR and changed configure * Sat Jan 2 2009 Roshan Kumar Singh <singh.roshan08> 0.8.1-1 - First RPM for opendchub ---------------------------------------------------------
(In reply to comment #5) > * perl module dependency > https://fedoraproject.org/wiki/Packaging/Perl#Perl_Requires_and_Provides > - When writing perl module rpm dependency as (Build)Requires, > use the virtual provides on the package, not the rpm name itself > (i.e. use "BuildRequires: perl(ExtUtils::Embed)" Done! The very next line in the above links says "packages link to libperl.so, usually to provide embedded perl functionality. All of these packages must also use the versioned MODULE_COMPAT Requires. " So I think this is needed as well Requires: perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version)) > > * creating directory under home directory [snip] > Well, > - build process should not create any directories under home > directory. If these directories are needed, they should be > created when the program is actually executed. The binary also creates the directories. I will remove this thing in the next release. > So this behavior (i.e creating directories under home directory > and installing some scripts under there during build) should > be suppressed. I have done this by %build %configure --enable-switch_user rm -rf $HOME/.opendchub Is it okay ? Other changes have also been made. I will renew the SPEC after you confirm the changes.
(In reply to comment #6) > (In reply to comment #5) > Done! The very next line in the above links says "packages link to libperl.so, > usually to provide embedded perl functionality. All of these packages must also > use the versioned MODULE_COMPAT Requires. " So I think this is needed as well > > Requires: perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version)) - Because none of the binaries in this package are linked against libperl.so and this package does not contain any perl scripts, this Requires is not needed. > > * creating directory under home directory > I have done this by > > %build > %configure --enable-switch_user > rm -rf $HOME/.opendchub > > Is it okay ? - This is still undesired when rebuilding this srpm with usual rpmbuild because this actually removes ~/.opendchub, which may already be used by the user. You should apply a patch against configure.in not to create these directories and call autotools.
I have made the changes. SPEC: http://roshansingh.fedorapeople.org/opendchub/opendchub.spec SRPM: http://roshansingh.fedorapeople.org/opendchub/opendchub-0.8.1-4.fc12.src.rpm
For -4: * BR - Now this srpm calls autotools, "BR: automake" is needed ! Note After adding "BR: automake", build.log shows: -------------------------------------------------------- 145 + make -j4 146 (CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/sh /builddir/build/BUILD/opendchub-0.8.1/missing --run autoheader) 147 autoheader: WARNING: Using auxiliary files such as `acconfig.h', `config.h.bot' -------------------------------------------------------- It is not preferable that autotools are automatically called after configure. So "$ autoheader" should also be called before configure. ! Miscs - Would you divide a long line (for example with containing more than 80 characters) into two lines?
(In reply to comment #9) > For -4: > > * BR > - Now this srpm calls autotools, "BR: automake" is needed What about aclocal and autoconf, should they be also added to BR ? > After adding "BR: automake", build.log shows: > -------------------------------------------------------- > 145 + make -j4 > 146 (CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/sh > /builddir/build/BUILD/opendchub-0.8.1/missing --run autoheader) > 147 autoheader: WARNING: Using auxiliary files such as `acconfig.h', > `config.h.bot' > -------------------------------------------------------- > It is not preferable that autotools are automatically called > after configure. So "$ autoheader" should also be called > before configure. I added autoheader to %build before configure, even then autoheader is being called again.
(In reply to comment #10) > (In reply to comment #9) > > For -4: > > * BR > > - Now this srpm calls autotools, "BR: automake" is needed > What about aclocal and autoconf, should they be also added to BR ? - /usr/bin/aclocal is in automake rpm, and automake has "Requires: autoconf" > I added autoheader to %build before configure, even then autoheader is being > called again. - Would you try aclocal -> autoheader -> autoconf -> automake ?
(In reply to comment #11) > > > - Now this srpm calls autotools, "BR: automake" is needed > > What about aclocal and autoconf, should they be also added to BR ? > > - /usr/bin/aclocal is in automake rpm, and automake has > "Requires: autoconf" ok. > > > I added autoheader to %build before configure, even then autoheader is being > > called again. > > - Would you try aclocal -> autoheader -> autoconf -> automake ? still the same. BTW how do you save the build.log. I use rpmbuild -ba to create rpm.
(In reply to comment #12) > (In reply to comment #11) > > > I added autoheader to %build before configure, even then autoheader is being > > > called again. > > > > - Would you try aclocal -> autoheader -> autoconf -> automake ? > > still the same. - Then forcely modify the timestamps of autotools related files to prevent autotools from being called after configure like: ---------------------------------------------------- %prep %setup -q %patch0 -p0 -b .configure aclocal autoconf automake autoheader touch -r configure \ config.h.in \ Makefile.in \ aclocal.m4 ---------------------------------------------------- > BTW how do you save the build.log. I use rpmbuild -ba to create > rpm. - You can use "tee" command like $ rpmbuild -ba foo.spec 2>&1 | tee build.log
Okay it is working fine now. In the %doc section i think it will be better to copy the entire Samplescript directory instead of copying the files inside it, %doc AUTHORS ChangeLog COPYING NEWS README TODO Samplescripts Is it okay ? What is you nick on IRC ?
(In reply to comment #14) > %doc AUTHORS ChangeLog COPYING NEWS README TODO Samplescripts > > Is it okay ? - No problem. > What is you nick on IRC ? - I have never used IRC...
The updated files are: SPEC: http://roshansingh.fedorapeople.org/opendchub/opendchub.spec SRPM: http://roshansingh.fedorapeople.org/opendchub/opendchub-0.8.1-5.fc12.src.rpm
Okay. ----------------------------------------------------- This package (opendchub) is APPROVED by mtasaka -----------------------------------------------------
New Package CVS Request ======================= Package Name: opendchub Short Description: A hub software for Direct Connect Owners: roshansingh Branches: F-11 F-12
CVS done (by process-cvs-requests.py)
Today while I was importing the srpm there was some network problem and while upload it quit with and error message. I retried, it went ahead, but there seems to be a problem. I am able to run 'make build' successfully on the devel branch but am not able to do it in F-11 and F-12. What should I do. There was a patch in the srpm which is not uploaded in F-12 only. However when I run make build Koji is complaining about the patch to be absent in both the branches F-11 and F-12. What should I do.
Well, - If you prefer to use "cvs-import.sh" (although I have never used this) * bump the release number to "5%{?dist}.1" * create new srpm again * and retry to import If you don't want to use cvs-import.sh (and I don't use cvs-import.sh), judging from http://cvs.fedoraproject.org/viewvc/rpms/opendchub/ : For F-12: patch is actually missing * cvs add <patch> * bump the release number to "5%{?dist}.1" * cvs commit -> make tag -> make build For F-11: patch is there, but not tagged, while spec file is already tagged. * bump the release number to "5{?dist}.1" * cvs commit -> make tag -> make build
Please submit push requests on bodhi for F-12/11.
opendchub-0.8.1-5.fc12.1 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/opendchub-0.8.1-5.fc12.1
opendchub-0.8.1-5.fc11.1 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/opendchub-0.8.1-5.fc11.1
opendchub-0.8.1-5.fc11.1 has been pushed to the Fedora 11 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update opendchub'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2010-0702
opendchub-0.8.1-5.fc12.1 has been pushed to the Fedora 12 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update opendchub'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F12/FEDORA-2010-0705
Closing.
opendchub-0.8.1-5.fc12.1 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.
opendchub-0.8.1-5.fc11.1 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.