[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=551838 --- Comment #15 from Mamoru Tasaka mtas...@ioa.s.u-tokyo.ac.jp 2010-01-09 12:14:10 EST --- (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... -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=551838
--- Comment #10 from Roshan Singh singh.rosha...@gmail.com 2010-01-09
09:17:46 EST ---
(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.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=551838 --- Comment #11 from Mamoru Tasaka mtas...@ioa.s.u-tokyo.ac.jp 2010-01-09 09:24:07 EST --- (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 ? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=551838 --- Comment #12 from Roshan Singh singh.rosha...@gmail.com 2010-01-09 09:31:41 EST --- (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. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=551838 --- Comment #13 from Mamoru Tasaka mtas...@ioa.s.u-tokyo.ac.jp 2010-01-09 11:48:50 EST --- (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 21 | tee build.log -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=551838 --- Comment #14 from Roshan Singh singh.rosha...@gmail.com 2010-01-09 12:07:14 EST --- 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 ? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=551838
--- Comment #9 from Mamoru Tasaka mtas...@ioa.s.u-tokyo.ac.jp 2010-01-08
13:22:11 EDT ---
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?
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=551838
--- Comment #7 from Mamoru Tasaka mtas...@ioa.s.u-tokyo.ac.jp 2010-01-07
11:57:00 EDT ---
(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.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=551838 --- Comment #8 from Roshan Singh singh.rosha...@gmail.com 2010-01-07 22:56:31 EDT --- 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 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=551838 Mamoru Tasaka mtas...@ioa.s.u-tokyo.ac.jp changed: What|Removed |Added AssignedTo|nob...@fedoraproject.org|mtas...@ioa.s.u-tokyo.ac.jp Flag||fedora-review? --- Comment #5 from Mamoru Tasaka mtas...@ioa.s.u-tokyo.ac.jp 2010-01-06 10:34:36 EDT --- 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 78AC_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 88echo creating script directory: $HOME/.opendchub/scripts 89mkdir $HOME/.opendchub/scripts 90chmod 700 $HOME/.opendchub/scripts; 91for i in Samplescripts/*; do 92cp $i $HOME/.opendchub/scripts; 93done 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.rosha...@gmail.com 0.8.1-3 - Changed Group to a more appropriate one * Sun Jan 3 2009 Roshan Kumar Singh singh.rosha...@gmail.com 0.8.1-2 - Removed glibc-devel and added perl-devel to BR and changed configure * Sat Jan 2 2009 Roshan Kumar Singh singh.rosha...@gmail.com 0.8.1-1 - First RPM for opendchub - -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=551838 Mamoru Tasaka mtas...@ioa.s.u-tokyo.ac.jp changed: What|Removed |Added Status|NEW |ASSIGNED -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=551838
--- Comment #6 from Roshan Singh singh.rosha...@gmail.com 2010-01-06
22:23:47 EDT ---
(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.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=551838 --- Comment #2 from Roshan Singh singh.rosha...@gmail.com 2010-01-03 09:28:27 EDT --- 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 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=551838 --- Comment #3 from ELMORABITY Mohamed melmorab...@fedoraproject.org 2010-01-03 10:25:20 EDT --- 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 ^^. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=551838 --- Comment #4 from Roshan Singh singh.rosha...@gmail.com 2010-01-03 11:11:17 EDT --- (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 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 551838] Review Request: opendchub - A hub software for Direct Connect
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=551838 ELMORABITY Mohamed melmorab...@fedoraproject.org changed: What|Removed |Added CC||melmorab...@fedoraproject.o ||rg --- Comment #1 from ELMORABITY Mohamed melmorab...@fedoraproject.org 2010-01-02 10:46:52 EDT --- 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 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
