Spec URL: http://ispbrasil.com.br/ifstat/ifstat.spec SRPM URL: http://ispbrasil.com.br/ifstat/ifstat-1.1-1.2.fc9.src.rpm Description: ifstat(1) is a little tool to report interface activity like vmstat/iostat do. In addition, ifstat can poll remote hosts through SNMP if you have the ucd-snmp library. It will also be used for localhost if no other known method works (You need to have snmpd running for this though). ifstat seems to be cool, and I like it in fedora.
little change. http://ispbrasil.com.br/ifstat/ifstat.spec http://ispbrasil.com.br/ifstat/ifstat-1.1-1.3.fc9.src.rpm
Could you please replace the "makeinstall" call with make install (plus proper arguments) ? The .spec file bundled in the tarball can give you some very good hints on how to do it properly. See https://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used for reasoning behind my request.
please l@@k again I have added a destdir patch, so I can use "make install" I have also disabled debug info package, do you agree about this ? rpmlint is showing a warning about file-not-utf8, can you help about this ? ifstat.x86_64: W: file-not-utf8 /usr/share/doc/ifstat-1.1/README here are the spec and the src.rpm http://ispbrasil.com.br/ifstat/ifstat.spec http://ispbrasil.com.br/ifstat/ifstat-1.1-1.4.fc9.src.rpm
I am not sure that the destdir patch is needed, I'll try to check tomorrow. And no, you should NOT disable building the debuginfo package. Exactly the opposite, you should make sure it contains useful info (i.e. the debug symbols stripped from the binaries; rpmbuild will do it for you automatically if you create a correct spec and you take care to not strip the generated binaries.) As of the file-not-utf8 one, that is easy. Just use iconv to convert it. Something similar to iconv README -f ISO8859-1 -o README.utf8 touch -r README README.utf8 mv README.utf8 README will convert the file and preserve the timestamp. BTW, you should use %{__make} install DESTDIR=$RPM_BUILD_ROOT CFLAGS="$RPM_OPT_FLAGS" INSTALL="install -p" in order to preserve the timestamps of the files. Or , alternatively, you could add the "-p" parameter to the install commands from your patch
the destdir patch is needed because the default makefile is buggy, after you look you will see. (without patch the makefile install ifstat in $(bindir) instead of $(DESTDIR)$(bindir)) about the debuginfo the package produces a empty debuginfo file, I have disabled because this rpmlint /usr/src/redhat/RPMS/x86_64/ifstat-debuginfo-1.1-1.4.fc9.x86_64.rpm ifstat-debuginfo.x86_64: E: empty-debuginfo-package 1 packages and 0 specfiles checked; 1 errors, 0 warnings. about file-not-utf8, I have added a small patch to fix this instead using iconv. here are new spec file and src.rpm http://ispbrasil.com.br/ifstat/ifstat.spec http://ispbrasil.com.br/ifstat/ifstat-1.1-1.5.fc9.src.rpm
I have no intention to review this request, however: (In reply to comment #5) > about the debuginfo the package produces a empty debuginfo file, I have > disabled As wolfy said in the comment 4, this is not allowed. Instead you _must_ create a useful debuginfo rpm correctly. The culprit which prevents from creating correct debuginfo rpm is in Makefile.in: ---------------------------------------------------------- 67 install-$(TARGET): $(TARGET) 68 $(INSTALL) -d -m 755 $(DESTDIR)$(bindir) 69 $(INSTALL) -p -s -m 755 $(TARGET) $(DESTDIR)$(bindir)/$(TARGET) <-------------- 70 $(INSTALL) -d -m 755 $(DESTDIR)$(mandir)/man1 71 $(INSTALL) -p -m 644 ifstat.1 $(DESTDIR)$(mandir)/man1/ifstat.1 ---------------------------------------------------------- On the line 69, "install" command uses "-s" option, which strips installed binary, which needs fixing. By the way build log shows: ---------------------------------------------------------- + /usr/bin/make -j2 gcc -I. -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -O3 -fomit-frame-pointer -g0 -DHAVE_CONFIG_H -DNETSNMP_ENABLE_IPV6 -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -Ulinux -Dlinux=linux -I/usr/include/rpm -D_REENTRANT -D_GNU_SOURCE -DDEBUGGING -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm -I/usr/lib/perl5/5.10.0/i386-linux-thread-multi/CORE -I/usr/include/net-snmp -c ifstat.c -o ifstat.o ---------------------------------------------------------- Compilar flags "-O3" "-fomit-frame-pointer" makes debugging almost impossible so these flags should be removed (note. the flag "-O3" is replaced by the latter "-O2", however it is better that -O3 is removed anyway)
Please be as kind as to NOT alter the author's name via a so called patch. Use the proper method, i.e. iconv.
(In reply to comment #7) > Please be as kind as to NOT alter the author's name via a so called patch. Use > the proper method, i.e. iconv. I have no intention to change the author name, but using iconv make it's unreadable, also using iconv makes spec files bigger, I have only converted the name of author to UTF8 removing "¨" [1] take a look http://gael.roualland.free.fr/ifstat/HISTORY I have changed from Gaël Roualland <gael.roualland.com> to Gael Roualland <gael.roualland.com> the name still the same, but writen in UTF8 1 - http://en.wikipedia.org/wiki/Umlaut_(diacritic)
(In reply to comment #6) > 69 $(INSTALL) -p -s -m 755 $(TARGET) $(DESTDIR)$(bindir)/$(TARGET) > <-------------- BIG thanks to everyone, I am learning alot of things, I have removed -s , removed "--enable-optim", from configure option, and added --enable-debug, now I belive the debug_info package is being created.
lasted changes here http://ispbrasil.com.br/ifstat/ifstat.spec http://ispbrasil.com.br/ifstat/ifstat-1.1-1.6.fc9.src.rpm
(Just commenting again that as I guess I have no time to review this ticket for now, I appreciate if someone else would review this ticket)
* I don't see anything wrong with the suggested iconv line. Hint: Run iconv on all files that need to be converted, then create a diff to replace your patch. If you disagree with the reviewer, don't jump to another solution, but find out whether you really tried the same fix. * --enable-debug in general is the wrong thing to do, especially if it sets more than just compiler flags like -g. This program also sets -DDEBUG which bears the risk of enabling and compiling optional code that is not wanted. This particular program doesn't evaluate the DEBUG define yet in its own source files, but other programs use such flags to toggle assertions/debugging output and diagnostic code paths. What you want instead is to simply ensure that the Fedora CFLAGS are accepted by the Makefile and are not modified in unwanted ways. Read the build log. * Re: comment 4 > %{__make} install DESTDIR=$RPM_BUILD_ROOT CFLAGS="$RPM_OPT_FLAGS" > INSTALL="install -p" Correct except for the CFLAGS definition, which must NOT be present here. Nothing during "make install" is supposed to compile anything. Also note that the %makeinstall macro may be used where a patch like ifstat-destdir.patch were far bigger. That macro is a little bit dangerous in some cases, so prefer the recommended make install invocation. * Licence is "GPLv2+" not "GPLv2". See header of the .c/.h files. Note the "or [...] any later version".
Reis, would you update your srpm?
updated. http://ispbrasil.com.br/ifstat/ifstat.spec http://ispbrasil.com.br/ifstat/ifstat-1.1-1.7.fc9.src.rpm
Seems good except for: * Timestamp on original tarball is not kept https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps Please re-download the original tarball by: $ wget -N http://gael.roualland.free.fr/ifstat/ifstat-1.1.tar.gz for example. * Files under %_mandir are automatically marked as %doc. * Please change the release number to X%{?dist} (2%{?dist}, for example) (I guess you want to do so after the review is passed) Well, + This package itself is now okay (but please fix above) + You have also other review requests, which seems good to some extent Okay. -------------------------------------------------------------- This package (itstat) is APPROVED by mtasaka -------------------------------------------------------------- Please follow the procedure written on: http://fedoraproject.org/wiki/PackageMaintainers/Join from "Install the Client Tools (Koji) ". Now I am sponsoring you. If you want to import this package into Fedora 8/9/10, you also have to look at http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT (after once you rebuilt this package on koji Fedora rebuilding system). If you have questions, please ask me. Removing NEEDSPONSOR.
fixed. http://ispbrasil.com.br/ifstat/ifstat.spec http://ispbrasil.com.br/ifstat/ifstat-1.1-8.fc9.src.rpm
Okay, then please follow join wiki.
http://koji.fedoraproject.org/koji/taskinfo?taskID=910891 now I need to ask for a cvs account ?
Now you should follow Join wiki from "Add Package to CVS and Set Owner". (as mentioned at http://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_and_Set_Owner follow http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure )
New Package CVS Request ======================= Package Name: ifstat Short Description: Interface statistics Owners: itamarjp Branches: F-9 F-10 InitialCC: mtasaka
cvs done.
ifstat-1.1-8.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/ifstat-1.1-8.fc9
ifstat-1.1-8.fc9 has been pushed to the Fedora 9 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 ifstat'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-9312
ifstat-1.1-8.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.