Spec Name or Url: http://amsn.sourceforge.net/tjikkun/tcltls.spec SRPM Name or Url: http://amsn.sourceforge.net/tjikkun/tcltls-1.5.0-1.src.rpm Description: The TLS OpenSSL extension to Tcl
Good: + Local build worked fine. + Mock build worked fine. Bad: - fedora-qa complaints: tls1.5.0-src.tar.gz 11 kB 00:00 Source tls1.5.0-src.tar.gz is different from upstream - Package doesn't contains any documentation. - Package doesn't contain verbarin license text. - rpmlint compiaints: E: tcltls invalid-soname /usr/lib/libtls1.50.so libtls1.50.so - fedora-qa complaints. Each %files section should have a %defattr line (wiki: Packaging/ReviewGuidelines)
-Source tls1.5.0-src.tar.gz is different from upstream i checked and both are 113600 in size, and have MD5SUM of: 9eeab472475773b3810acc808ebec759 but sometimes the sf.net has a failed-mirror and the the download is not correct. Or maybe I am doing something wrong, I am quite new at packaging. The rest I have fixed. Thank you for taking the time to review. Spec Name or Url: http://amsn.sourceforge.net/tjikkun/tcltls.spec SRPM Name or Url: http://amsn.sourceforge.net/tjikkun/tcltls-1.5.0-2.src.rpm
Created attachment 126831 [details] protocoll from make test
Good: * RPM name is OK * Source tls1.5.0-src.tar.gz is the same as upstream * Builds fine in mock * rpmlint of tcltls looks OK * rpmlint of tcltls-devel looks OK * File list of tcltls looks OK * File list of tcltls-devel looks OK * INSERT RESULT OF RUN TEST Bad: - debuginfo RPM contains no source files. - make test fails please see attached file.
- debuginfo RPM contains no source files. How can I make it contain them? I searched for information but could not find anything. Is it automatically generated with /usr/lib/rpm/find-debuginfo.sh ? If so what can I do about it? - make test fails please see attached file. I fixed this but will upload when I know about the other problem. Once again thank you very much for taking the time for reviewing.
Add the -g flag during compiliation. Don't stript the executables.
Spec Name or Url: http://amsn.sourceforge.net/tjikkun/tcltls.spec SRPM Name or Url: http://amsn.sourceforge.net/tjikkun/tcltls-1.5.0-3.src.rpm I *think* everything is ok now, but you'll probably find something :)
Sorry: Local build coused: _BLKSIZE=1 -DHAVE_ST_BLKSIZE=1 -DHAVE_SIGNED_CHAR=1 -DHAVE_LANGINFO=1 -DHAVE_SYS_IOCTL_H=1 -I"/usr/include" -I"/usr/include" -g -O2 -fPIC -c `echo ./fixstrtod.c` -o fixstrtod.o rm -f libtls-1.50g.so cc -shared -Wl,-soname,libtls-1.50g.so -o libtls-1.50g.so tls.o tlsIO.o tlsBIO.o tlsX509.o fixstrtod.o -L/usr/lib -ltclstub8.4g -L/usr/lib -lssl -L/usr/lib -lcrypto /usr/bin/ld: cannot find -ltclstub8.4g collect2: ld returned 1 exit status make: *** [libtls-1.50g.so] Error 1 Fehler: Bad exit status from /var/tmp/rpm-tmp.63113 (%build)
Now where does that g come from? I don't understand.
(In reply to comment #9) > Now where does that g come from? I don't understand. It comes from the --enable-symbols flag that you pass to configure. You might want to consider updating the tcl.m4 in this package with the latest one from the tclconfig module in Tcl's cvs repository. That will also fix the build errors on x86_64 (it doesn't look for tclConfig.sh in /usr/lib64).
(In reply to comment #8) > _BLKSIZE=1 -DHAVE_ST_BLKSIZE=1 -DHAVE_SIGNED_CHAR=1 -DHAVE_LANGINFO=1 > -DHAVE_SYS_IOCTL_H=1 -I"/usr/include" -I"/usr/include" -g -O2 -fPIC -c > `echo ./fixstrtod.c` -o fixstrtod.o > rm -f libtls-1.50g.so > cc -shared -Wl,-soname,libtls-1.50g.so -o libtls-1.50g.so tls.o tlsIO.o tlsBIO.o > tlsX509.o fixstrtod.o -L/usr/lib -ltclstub8.4g -L/usr/lib -lssl -L/usr/lib -lcrypto This log exposes further issues: * -I/usr/include and -L/usr/lib almost never are correct * The compiler calls above don't honor RPM_OPT_FLAGS.
(In reply to comment #11) > This log exposes further issues: > * -I/usr/include and -L/usr/lib almost never are correct These are picked up from the tclConfig.sh build file in the Core Tcl package. It should be fixed in Core. > * The compiler calls above don't honor RPM_OPT_FLAGS. It seems that the extension is ignoring TCL_EXTRA_CFLAGS from tclConfig.sh, which is where RPM_OPT_FLAGS is stored from the build of Tcl. This is supposed to be picked up automatically, but isn't here. You can work around it by adding it to CFLAGS_DEFAULT in the Makefile: CFLAGS_DEFAULT = @CFLAGS_DEFAULT@ @TCL_EXTRA_CFLAGS@
(In reply to comment #1) > - rpmlint compiaints: > E: tcltls invalid-soname /usr/lib/libtls1.50.so libtls1.50.so This is a dynamically loaded extension to Tcl, not a shared library for linking against other applications. IMHO, it should be moved out of %{_libdir} and into its own %{_libdir}/tls1.50 directory, with the pkgIndex.tcl file updated accordingly, as is done with some other Tcl extensions in FE. Let me know if you want some help with this and the other build issues. I've packaged quite a number of Tcl extensions for FE and have come across most of these problems before.
Created attachment 126929 [details] Added patch for configure script Added the diff for the configure script so that you don't need to run autoconf in the spec file.
Created attachment 126930 [details] look for tclConfig.sh in %{_libdir} Modified tcl.m4 and configure script to look for tclConfig.sh in '--libdir' and /usr/lib64 on x86_64. This fixes the build error on x86_64.
Created attachment 126931 [details] Add $RPM_OPT_FLAGS to compile line Updates Makefile.in to add $TCL_EXTRA_CFLAGS from tclConfig.sh to the compile line. TCL_EXTRA_CFLAGS contains the $RPM_OPT_FLAGS value from the build of Tcl in Fedora Core. This removes the need for the tcltls-1.5-tclm4-debugoptimize patch.
Wart, thank you very much for all your posts! At the moment I am really busy at work but I will try to use the patches as soon as possible. This is (together with amsn) the first rpm i am building so I have a lot to learn (and thanks to everybody posting I have learned a lot).
(In reply to comment #17) > Wart, thank you very much for all your posts! At the moment I am really busy at > work but I will try to use the patches as soon as possible. This is (together > with amsn) the first rpm i am building so I have a lot to learn (and thanks to > everybody posting I have learned a lot). No problem. I had actually considered submitting this one myself for tclhttpd (to get https support), but got distracted by other packages.
Finally I had some time again, so here they are: http://amsn.hoentjen.eu/download/tcltls.spec http://amsn.hoentjen.eu/download/tcltls-1.5.0-5.src.rpm I put in all Wart's changes and moved the libtls so into the tls1.5 subdir. I hope I got it right now, Sander
You still need to remove '--enable-symbols' from the %configure statement so that it doesn't try to add the 'g' in -ltclstub8.4g. It would also be good, as Jochen said, to include the tests as part of the build. Just add the following to the spec file: %check make test And as noted, there are three failing tests. The first two look like they might be due to the use of a different openssl version from when the tests were written. The third should be posted upstream to see if it's a real problem or not.
Hum, i must have messed something up because it did have that before, i had the patch ready to go: http://amsn.hoentjen.eu/download/tcltls.spec http://amsn.hoentjen.eu/download/tcltls-1.5.0-6.src.rpm it is ok on my machine, now i am also trying it with mock, but that will take a while.
God: + Mock build works fine. Bad: - Source tls1.5.0-src.tar.gz is different from upstream
ok now i have one with correct MD5SUM http://amsn.hoentjen.eu/download/tcltls-1.5.0-7.src.rpm
This latest rpm works great with tclhttpd. I can verify that this package is functional.
sorry, but the tar.gz file in the source RPM doesn't match with the upstream version. fedora-qa tcltls-1.5.0-7.src.rpm Starting QA for local srpm tcltls-1.5.0-7.src.rpm Checking tcltls, version 1.5.0, release 7... Are the name, version and release correct according to Fedora's package naming guidelines ([y]/n) y View the last spec diff ? ([y]/n) n Checking source 0 Source tls1.5.0-src.tar.gz is different from upstream (wiki: QAChecklist item 2) Patches found: tcltls-configurein.pkgname.patch, tcltls-1.5-rpmoptflags.patch, tcltls-1.5-64bit.patch, tcltls-makefilein-soname.patch, tcltls-tclm4-soname.patch, tcltls-1.5-relpath.patch, tcltls-1.5-pkgindex-version.patch, tcltls-1.5-ciphertest-KRB5.patch. Look at the patches ? ([y]/n) Interrupted by user
I have the following md5sum from both upstream and the latest src rpm: 9eeab472475773b3810acc808ebec759 tls1.5.0-src.tar.gz However, the Source0: tag can't be used directly to get the sources due to the sourceforge download redirection. Try using "dl.sourceforge.net" instead of "download.sourceforge.net".
ok, as you said now with dl.sourceforge.net: http://amsn.hoentjen.eu/download/tcltls-1.5.0-8.src.rpm Hope this is finally the right one, I must be the worst packager ever..
Sorry, you may have a strange problem. I got fedora-qa tcltls-1.5.0-8.src.rpm Starting QA for local srpm tcltls-1.5.0-8.src.rpm Checking tcltls, version 1.5.0, release 8... Are the name, version and release correct according to Fedora's package naming guidelines ([y]/n) y Diff the spec files ? ([y]/n) n Checking source 0 tls1.5.0-src.tar.gz 12 kB 00:00 Source tls1.5.0-src.tar.gz is different from upstream (wiki: QAChecklist item 2) Patches found: tcltls-configurein.pkgname.patch, tcltls-1.5-rpmoptflags.patch, tcltls-1.5-64bit.patch, tcltls-makefilein-soname.patch, tcltls-tclm4-soname.patch, tcltls-1.5-relpath.patch, tcltls-1.5-pkgindex-version.patch, tcltls-1.5-ciphertest-KRB5.patch. Look at the patches ? ([y]/n)
for Information the md5sum from the downloaded file is: 2e39bbc3b1cc8f837fe7a0abec9bdaad tls1.5.0-src.tar.gz The source in in the rpm have the following md5sum: 9eeab472475773b3810acc808ebec759 tls1.5.0-src.tar.gz
(In reply to comment #29) > 2e39bbc3b1cc8f837fe7a0abec9bdaad tls1.5.0-src.tar.gz > > 9eeab472475773b3810acc808ebec759 tls1.5.0-src.tar.gz Are the file sizes the same? Can you untar the downloaded file and the tarball in the src rpm and run "diff -r" to check the differences?
It's seems to be a access problem to dl.sourceforge.com When I download the .tar.gz file with firefox, I get a proper file.
(In reply to comment #27) > Hope this is finally the right one, I must be the worst packager ever.. Nah, I've seen (and written!) much worse. :) This and #185951 look like your first Extras packages. If this is true then you need to add the FE-NEEDSPONSOR blocker bug to both so that potential sponsors can review and approve this package.
Thanks a lot the both of you, I will now add the FE-NEEDSPONSOR blocker.
Unassigning bug since the reviewer can't sponsor. This will let sponsors know that it's up for grabs.
If you're having trouble finding a sponsor then there are a couple of things you can do: * Comment on other packages to show that you have a good understanding of the packaging guidelines. This is the most important thing you can do. * Post to fedora-extras-list that you need a sponsor and refer back to these two bugs. Potential sponsors will then take a look at these bugs, as well as others that you've commented on to see if you've earned your sponsorship.
Wart, thank you for your comments and for teaching me stuff. I knew about those 2 things to get sponsored. It just seems i picked a really bad time to start this packaging stuff, since at the moment I am really busy with work/school and my computer is not playing nicely (time for replacement, but no money). Work is calming down a bit (I hope) so I think I will be able to do some reviews (making comments) soon. Sander
Assigning this to myself now that I've been granted sponsor status. Have you filled out the CLA in the accounts system yet?
I now have filled it out, and I applied for a cvsextras account.
I finally got around to a formal review. Most of the issues have already been addreessed above, but there's still a few remaining nits. MUST ==== * rpmlint output: W: tcltls-devel no-documentation ...which can be ignored because upstream doesn't provide development documentation. * Source matches upstream 9eeab472475773b3810acc808ebec759 SOURCES/tls1.5.0-src.tar.gz * BSD license ok, license file included * Spec file legible and in Am. English * No excessive BR: * No locales * No shared libraries in the default linker path * Not relocatable * Owns the directory that it creates * No duplicate %files * buildroot cleaned where needed * No -docs subpackage needed * header file included in -devel package * No .desktop file needed * Package runs without crashing (tested with tclhttpd) * Builds in mock on: core4-i386, core4-x86_64, core5-i386, core5-x86_64, devel-i386 * File permissions look ok ANOMOLIES ========= * Package name does not match upstream (tcltls vs tls). However, the general convention outside of Fedora for naming Tcl packages is "tcl<foo>", which is already quite similar to Fedora's naming convention for language subpackages "tcl-<foo>". In this case I feel it's better to use either tcl<foo> or tcl-<foo> for the package name as 'tls' is too generic. * The version number is 1.5.0, yet upstream puts files in %{_libdir}/tls1.50. You should probably tell upstream to fix their directory name. You can also do it yourself in this package, but I won't consider it a blocker. Fixing the directory name would also remove the need for the majorver and minorver %defines in the spec file. MUSTFIX ======= * A more succinct summary would be "OpenSSL extension for Tcl" * Summary for the -devel package reads better as: "Header files for the OpenSSL extension for Tcl" * Minor nit: Remove the commented-out entry in %files and in %prep * Use a macro in %configure: %configure --with-ssl-dir=%{_prefix} * Group should be "Development/Libraries" * The URL, Version, and Group tags in the -devel subpackage aren't necessary. They are inherited from the base package.
(In reply to comment #39) I fixed all, except for "Group tag in the -devel subpackage", since rpmbuild complains without it. I decided to change the tcltls name to tcl-tls because i was just thinking about that today, and i wanted to ask what was right, since in most other distros it is tcltls (that's why i chose it initially) but for fedora's naming tcl-tls seems most appropriate. http://amsn.hoentjen.eu/download/tcl-tls.spec http://amsn.hoentjen.eu/download/tcl-tls-1.5.0-9.src.rpm
(In reply to comment #40) > (In reply to comment #39) > I fixed all, except for "Group tag in the -devel subpackage", since rpmbuild > complains without it. > I decided to change the tcltls name to tcl-tls because i was just thinking about > that today, and i wanted to ask what was right, since in most other distros it > is tcltls (that's why i chose it initially) but for fedora's naming tcl-tls > seems most appropriate. In this case I think following the name used by other distros would be more appropriate. Please use tcltls instead of tcl-tls. After you make that change I'll APPROVE the package and we can move on to the next step.
ok, name change reverted. http://amsn.hoentjen.eu/download/tcltls.spec http://amsn.hoentjen.eu/download/tcltls-1.5.0-10.src.rpm
I double checked that the package still builds in mock. APPROVED I've updated your sponsored status so that you can now continue with the cvs import process. Don't hesitate to email me if you have any questions.
Package has been built for devel now, and I will request build for FC4 and 5 as soon as the branches are created
Reassigning to me as part of some bugzilla cleanup.