Spec URL: http://www.linux-ati-drivers.homecall.co.uk/html2text.spec SRPM URL: http://www.linux-ati-drivers.homecall.co.uk/html2text-1.3.2a-2.fc9.src.rpm Description: html2text is a command line utility that converts HTML documents into plain text. Each HTML document is read from standard input or a (local or remote) URI, and formatted into a stream of plain text characters that is written to standard output or into an output-file. The program preserves the original positions of table fields and accepts also syntactically incorrect input, attempting to interpret it "reasonably". The rendering is largely customisable through an RC file. dependency for alien 456756 https://bugzilla.redhat.com/show_bug.cgi?id=456756 I am also seeking a sponsor.
It looks like this depends on 456756 - NOT the other way around as things are currently. I am not changing it yet - I'm not sponsored and don't know if I should be doing edits like this one.
FYI - I'm not sponsored yet but am going to take a look at this one and provide a review.
(In reply to comment #1) > It looks like this depends on 456756 - NOT the other way around as things are > currently. I am not changing it yet - I'm not sponsored and don't know if I > should be doing edits like this one. html2text doesn't depend on alien! http://packages.debian.org/sid/html2text html2text is a dependency for debhelper ! http://packages.debian.org/sid/debhelper
You are correct of course. I did not mean to imply it depended on alien itself in order to build. I meant that this ticket's approval would depend on the existence of an alien rpm - that request is in the ticket I mentioned. I did not mean to imply that this package should declare a dependency on alien in the .spec file - it does not depend it. html2text should perhaps be blocked by the alien ticket instead - seems that the only way to get the effect in bugzilla is to have the other bug block this one (seems backwards to me at the moment - strange bugzilla ) - no need for change...my mistake.
** UNOFFICIAL REVIEW - NEED SPONSOR *** INCOMPLETE BUT OK - rpmlint output not included for spec file My run shows it to be ok though: 0 packages and 1 specfiles checked; 0 errors, 0 warnings NEEDSWORK- rpmlint output not included for SRPM My run shows it to produce one warning. html2text.src: W: invalid-license GPL Review the short hand codes for licenses and choose the correct GPL version. See: http://fedoraproject.org/wiki/Licensing See: http://fedoraproject.org/wiki/Licensing#SoftwareLicenses In the case of html2text this field looks like it should say: GPL+ OK - The package must be named according to the Package Naming Guidelines OK - The .spec file name matches the base package %{name}, format: %{name}.spec OK - The package must be licensed with a Fedora approved license and meets the Licensing Guidelines OK - legal appears to be ok - GPL code according to README file. OK - The License field in the package spec file must match the actual license After the above rpmlint issue is cleared this will be OK OK - Source package includes license text in README, README is listed as doc file. OK - The spec file must be written in American English OK - The spec file for the package MUST be legible. OK - The sources used to build the package must match the upstream source md5sum is: 6097fe07b948e142315749e6620c9cfc NEEDSWORK - The package must successfully compile and build into binary rpms on at least one supported architecture. I was unable to get this to build. 1) Fetched spec file to rpmbuild/SPEC, 2) placed SRPM in rpmbuild/SRPM 3) extracted tar.gz source from SRPM and placed into rpmbuild/SOURCES 3) cd to rpmbuild/SPEC 4) rpmbuild -ba html2text.spec error: File .../rpmbuild/SOURCES/sgml.C.patch: No such file or directory This is probably a simple fix - I will leave this one as an exercise for the submitter. This is enough comment for now...possibly more to come.
jeff, you have not tested correctly. You should have - either used rpmbuild --rebuild <the src.rpm provided by the submitter> or (better) - setup mock and use it for testing In your test case you have ignored the need to place the patches in the same source directory as the tarball.
Ok, thanks for the correction. I will re-test using rpmbuild --rebuild.
i strongly advice to make mock a friend of yours. The package must _anyway_ be able to be built in mock + using it helps in finding eventual BR problems.
OK - The package must successfully compile and build into binary rpms on at least one supported architecture. + LOCAL x86_64 + OK - rebuilt using rpmbuild --rebuild html2text-1.3.2a-2.fc9.src.rpm + OK - built using mock target fedora-rawhide-x86_64 + Remote all archs + SUBMITTED to koji (results pending) koji build --scratch dist-f10 ~/rpmbuild/SRPMS/html2text-1.3.2a-2.fc9.src.rpm
Correction - mock build still pending also.
OK - koji passes on 5 platforms http://koji.fedoraproject.org/koji/taskinfo?taskID=749033 Mock still pending on my local machine (yum updates take time)
All the SRPM's I submitted were built in mock .
(In reply to comment #12) > All the SRPM's I submitted were built in mock . Excellent. Then the one on my machine will pass. The one on my machine is actually redundant since koji tested on 5 targets including x86_64.
koji [scratch] build is more or less an alternative to building locally. since koji uses mock, there is no need to do tests both locally in mock and in koji. I for one do koji tests either if I have doubts on my setup (for instance when I happen to damage it) of if I want to test on architectures to which I do not have direct access (PPC for instance).
NEEDSWORK - rpm lint on binary rpms shows... rpmlint html2text-*.rpm html2text.x86_64: W: file-not-utf8 /usr/share/doc/html2text-1.3.2a/README html2text.x86_64: W: invalid-license GPL html2text-debuginfo.x86_64: W: invalid-license GPL 2 packages and 0 specfiles checked; 0 errors, 3 warnings. OK - local mock build with Fedora 10 target (yes it's redundant - but it was already running when I decided to do my koji build - I figured why not let it go as a double check on x86_64) OK - All build dependencies must be listed in BuildRequires (koji/mock) would have failed so this is OK. OK?? - The spec file MUST handle locales properly The spec file makes no use of the find_lang. I'm not sure is this is an issue since it does not look like this app supplies localization files for any language OK - Every binary RPM package which stores shared library file ... This package produces NO shared libs OK - If the package is designed to be relocatable, the packager must state this fact in the request for review, NOT stated in review and no mention of /usr in spec file OK - A package must own all directories that it creates OK - A package must not contain any duplicate files in the %files listing. OK - Permissions on files must be set properly (rpmls shows appropriate perms) OK - Each package must have a %clean section, which contains rm -rf %{buildroot} or $BUILD_ROOT OK - Each package must consistently use macros, as described in the [wiki:Self:Packaging/Guidelines#macros Uses both styles but uses one in the general rpm spec directive context and shell style ex: $BUILD_ROOT in build/install contexts - usage is consistent in each context. OK - The package must contain code, or permissable content. OK - Large documentation files should go in a -doc subpackage No large doc files in this case OK - If a package includes something as %doc, it must not affect the runtime of the application. Only doc text files are listed as %doc for this package OK - Header files must be in a -devel package. No header files included in binary rpm OK - Static libraries must be in a -static package No libraries of any kind in this package OK - Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' None included OK - If a package contains library files with a suffix.... None included OK - In the vast majority of cases, devel packages must require the base package... No devel package OK - Packages must NOT contain any .la libtool archives... No libs produced at all OK - Packages containing GUI applications ... Not a gui app OK - Packages must not own files or directories already owned by other packages. Watched install then inspected directories and files ... Also rpmlint will usually complain if this is not the case?---I believe? OK - At the beginning of %install, each package MUST run rm -rf %{buildroot} uses $RPM_BUILD_ROOT style OK - All filenames in rpm packages must be valid UTF-8. That's all for the MUSTS.
Ok, now for some SHOULDs.. OK - If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. README contains license text OK? - description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. Description is present in english and german. The program does not seem to have separate localiztion file (.po) Examined ".C" files - no sign of other languages either. Given only one supported language maybe should only describe in that language in the .spec file?? I have seen other places though (libical) describe in multiple langs even though not localized for them. Probably not an issue. OK - The reviewer should test that the package builds in mock Tested Fedora 10 target on mock local machine - OK Tested Fedora 10 target via koji - OK on all 5 platforms OK - The package should compile and build into binary rpms on all supported architectures. Tested Fedora 10 target via koji - OK on all 5 platforms OK - The reviewer should test that the package functions as described. A package should not segfault instead of running, for example. Tested with several internet sites - OK Tested with with several html files - initially fetched by wget of a web site url - OK OK - If scriptlets are used, those scriptlets must be sane Look reasonable to me. NEEDSWORK - No uninstall support? OK - Usually, subpackages other than devel should require ... No subpackages. OK - The placement of pkgconfig(.pc) files depends... No such files included. OK - If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin.... No outside dependencies. Ok that covers the SHOULDs
I will change the GPL to GPL+ for these errors ? html2text.x86_64: W: invalid-license GPL html2text-debuginfo.x86_64: W: invalid-license GPL And convert the README to UTF8 html2text.x86_64: W: file-not-utf8 /usr/share/doc/html2text-1.3.2a/README But can you explain what needs to be done for this NEEDSWORK - No uninstall support?
Actually, just tested and it looks like you need no additional work to get this. rpm provides it for free - in some cases postuninstall steps may need mention in the spec file - but not here.
iconv can be used to convert the file to utf-8
There is a major problem with the spec. If you examine the build logs available from koji (for instance http://koji.fedoraproject.org/koji/getfile?taskID=749035&name=build.log ) you can notice that the actual build process does not use the mandatory fedora compiling flags: + make -j4 g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g html2text.C g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g html.C g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g HTMLControl.C g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g HTMLParser.C This violates http://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags and must be fixed.
Fixed spec file & SRPM http://www.linux-ati-drivers.homecall.co.uk/html2text.spec http://www.linux-ati-drivers.homecall.co.uk/html2text-1.3.2a-3.fc9.src.rpm Is this better ? + make -j3 'DEBUG=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic' g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic html2text.C g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic html.C g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic HTMLControl.C g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic HTMLParser.C
Could you please be as kind as to explain the reasoning behind the following section from your spec: for file in html2text.1.gz html2textrc.5.gz; do basefile=`basename $file .gz` gunzip -c $file > __fedora_docs/$basefile touch -r $file __fedora_docs/$basefile done install -m0644 -p __fedora_docs/html2text.1 $RPM_BUILD_ROOT%{_mandir}/man1 install -m0644 -p __fedora_docs/html2textrc.5 $RPM_BUILD_ROOT%{_mandir}/man5 %{_mandir}/man1/html2text.1* %{_mandir}/man5/html2textrc.5* I would have used: install -m0644 -p html2text.1.gz $RPM_BUILD_ROOT%{_mandir}/man1 install -m0644 -p html2textrc.5.gz $RPM_BUILD_ROOT%{_mandir}/man5 %{_mandir}/man*/html2text*
(In reply to comment #22) > Could you please be as kind as to explain the reasoning behind the following > section from your spec: It comes from me. I think it is better to install raw man pages such that the rpm scripts choose how to compress (or not compress) it. As a side note, it would be better if __fedora_docs was renamed __dist_docs.
Looks like your changes to the Makefile will add all DEBUG flags (in this case -g and -O2) so now you are always building a debug version. This debug version does, however, now include the $RPM_OPT_FLAGS I think you may want to separate these out in the Makefile. Leave your DEBUG variable, add support for turning it on and off. Add new variable (OPT_FLAGS?) to handle optmization related flags I'm kind of new at this package stuff though...so let's see what other feedback you get - there may be further conventions for handling this for Fedora packaging...
Also, I did do a koji rebuild so you can see this in the log files... http://koji.fedoraproject.org/koji/taskinfo?taskID=750104 Example section from the x86 build (note the -g meaning a debug build) g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic html2text.C g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic html.C g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic HTMLControl.C g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic HTMLParser.C
Of course debug support being always on may also be what you want - perhaps to make it easier if users encounter bugs and want to report them. In that case this may be ok...let's see what the expert packagers say.
(In reply to comment #25) > Also, I did do a koji rebuild so you can see this in the log files... > http://koji.fedoraproject.org/koji/taskinfo?taskID=750104 > > > Example section from the x86 build (note the -g meaning a debug build) > > g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g -pipe -Wall > -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 > -m64 -mtune=generic html2text.C > g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g -pipe -Wall > -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 > -m64 -mtune=generic html.C > g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g -pipe -Wall > -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 > -m64 -mtune=generic HTMLControl.C > g++ -c -DVERSION=1.3.2a -DAUTO_PTR_BROKEN -O2 -g -pipe -Wall > -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 > -m64 -mtune=generic HTMLParser.C > The -g comes from the makefile . linux/4.3.0/include -I/usr/lib/gcc/x86_64-redhat-linux/4.3.0/../../../../include/c++/4.3.0 -I/usr/lib/gcc/x86_64-redhat-linux/4.3.0/../../../../include/c++/4.3.0/backward -I/usr/lib/gcc/x86_64-redhat-linux/4.3.0/../../../../include/c++/4.3.0/bits -I/usr/lib/gcc/x86_64-redhat-linux/4.3.0/../../../../include/c++/4.3.0/debug -I/usr/lib/gcc/x86_64-redhat-linux/4.3.0/../../../../include/c++/4.3.0/ext -I/usr/lib/gcc/x86_64-redhat-linux/4.3.0/../../../../include/c++/4.3.0/x86_64-redhat-linux/bits DEBUG=-O2 -g INCLUDES = $(LIBSTDCXX_INCLUDES) DEFINES = -DVERSION=$(VERSION) $(SYS_POLL_MISSING) $(BOOL_DEFINITION) $(EXPLICIT) $(AUTO_PTR_BROKEN) CPPFLAGS = $(INCLUDES) $(DEFINES) CXXFLAGS = $(CPPFLAGS) $(DEBUG) LDFLAGS = $(DEBUG) LOADLIBES = $(LIBSTDCXX_LIBS) $(SOCKET_LIBRARIES)
I saw that line in the makefile... As long as Fedora packaging has no rule about it it's your call as author. My question remains: Is it your intent to always have debugging symbols turned on?
(In reply to comment #28) > I saw that line in the makefile... > As long as Fedora packaging has no rule about it it's your call as author. > My question remains: Is it your intent to always have debugging symbols turned on? Yes, they are automatically stripped and put in a debuginfo package by the rpmbuild scripts.
Approved. You will still need a sponsor before you can request cvs access and commit anything however.
You shouldn't be approving NEEDSPONSOR tickets if you're not a sponsor. Approval of the review ticket should include an offer to sponsor the person: https://fedoraproject.org/wiki/Package_Review_Process (You can, of course, do as much pre-review work as you like on such tickets.) Alternately, you could consider requesting that you be made a sponsor: https://fedoraproject.org/wiki/PackageMaintainers/SponsorProcess
Not that much important, but I suggest replacing __fedora_docs with __dist_docs. http://fedoraproject.org/wiki/PackageMaintainers/Packaging_Tricks#Avoiding_using_fedora_or_redhat
New Package CVS Request ======================= Package Name: html2text Short Description: HTML-to-text converter Owners: leigh123linux Branches: F-9 F-10 InitialCC: leigh123linux
cvs done.
html2text-1.3.2a-4.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.
html2text-1.3.2a-4.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
I'd like to have that in EPEL-4/5. Are you interested, and, if not would you accept that I maintain the EPEL branch?
(In reply to comment #37) > I'd like to have that in EPEL-4/5. Are you interested, and, if not would you > accept that I maintain the EPEL branch? Yes, How do I get CVS for the EPEL branch?
Just make the regular CVS request and ask for EL-4 and EL-5 branches.
New Package CVS Request ======================= Package Name: html2text Short Description: HTML-to-text converter Owners: leigh123linux Branches: EL-4 EL-5 InitialCC: leigh123linux
(In reply to comment #39) > Just make the regular CVS request and ask for EL-4 and EL-5 branches. Thanks.
I am getting a error when I "make build" [leigh@localhost EL-5]$ make build /usr/bin/plague-client build html2text html2text-1_3_2a-5_el5 el5 Error connecting to build server: '(111, 'Connection refused')' make: *** [plague] Error 1 I edited ~/.plague-client.cfg to [Certs] user-ca-cert = ~/.fedora-upload-ca.cert server-ca-cert = ~/.fedora-server-ca.cert user-cert = ~/.fedora.cert [User] email = leigh123linux [Server] use_ssl = yes upload_user = leigh123linux allow_uploads = no address = https://127.0.0.1:8887 What am I doing wrong ?
I got there :-) [Certs] user-ca-cert = ~/.fedora-upload-ca.cert server-ca-cert = ~/.fedora-server-ca.cert user-cert = ~/.fedora.cert [User] email = leigh123linux [Server] use_ssl = yes upload_user = leigh123linux allow_uploads = no address = https://buildsys.fedoraproject.org:8887
Package Change Request ====================== Package Name: html2text New Branches: F-11 Owners: leigh123linux
Package Change Request ====================== Package Name: html2text New Branches: epel7 Owners: tdawson
Git done (by process-git-requests).
Package Change Request ====================== Package Name: html2text New Branches: el6 Owners: tdawson This package was orphaned for epel6. I'm already the maintainer for epel7, and I'd like to be the maintainer for epel6. But the package database isn't letting me do that. Maybe doing this way will work.
Done.