Hello Paul, Am Montag, den 05.01.2015, 21:55 +0100 schrieb Paul Gevers: > Hi Jörg, > > On 30-12-14 21:17, Jörg Frings-Fürst wrote: > > first sorry for my late answer. I was i little bit ooo and then the > > 31C3. > > No problem. Also sorry I didn't replied sooner. > > > Am Montag, den 29.12.2014, 11:13 +0100 schrieb Paul Gevers: > >> On 26-12-14 20:48, Jörg Frings-Fürst wrote: > >> Ironically, upstream just (15 hours ago) seems to have released 1.33.15 > >> as tar ball. > > > > I have seen it too. But the download diretory is "Xmlrpc-c Super > > Stable/". I thinks its only a new Super Stable release. > > Sure, saw that. I guess you want to follow the stable release tree? >
Yes. And eventually the Advanced release in Experimental. > >>> * New debian/patches/200-test_port.diff: > >>> - Change port for testing from 8080 to 7890 (Closes: #722503). > >> > >> I am missing some background, either in the bug or in the patch file. > >> Why do you think hard coding 7890 is any better than 8080? And why do > >> you consider this "Forwarded: not-needed"? Michael suggested to try > >> multiple ports instead of relying on one port and I think upstream > >> should be interested in such a patch. > > > > My first opinion was to change nothing. On debian the builds was running > > in a clean system without network. So there are no changes required. > > > > On a build on a normal system port 8080 often be used by proxies or http > > servers. So I switched the port for the build tests to 7890. > > > > I think that the using of multiple ports in this case the coast are to > > high. > > > > In my opinion is the best way to set the port for testing at build time > > as configure variables (--testport=7890 or so). But this must be > > discussed with upstream. So have set "Forwarded: not-needed". > > Well, some of this information was/is very welcome in the patch header. > Maybe with a link where to the e-mail thread where you started the > discussion about it (so that in the far future we can check what > actually became of the request). > > >>> * New debian/patches/005-xmlrpc_example.diff: > >>> - Backport from upstream release 1.34.0 (Closes: #524550). > >> > >> If you still want me to upload your current package, could you add a > >> source URL to the DEP5 header? The bts seems to say it should be here: > >> http://sourceforge.net/p/xmlrpc-c/code/2491 but with that commit I don't > >> see any content there. At least mention the proper revision in the > >> headers. Also, the patch seems to fix more than just the bug in the bts. > >> Please document what it is supposed to fix. > >> > > > > I have rewriten the patch, so that only the wrong example is removed. > > Ack. > > >>> * New missing debian/xmlrpc-c-config.man and debian/xmlrpc.man. > >> > >> You created these files with help2man. I prefer it when you do this at > >> build time, so that the man page stays up-to-date. I think you can tweak > >> the settings of help2man to not add the date and your name. > >> > > Yes the manfiles was basically made with help2man, but also massive > > edited. Therefore I don't build them at build time. > > Then I really suggest you remove the first line of the man page file. > Did you send this manpage upstream then? Ideally you should be able to > drop this file again once upstream excepts it. Also I suggest to either > drop the statement about "may be used by others" or improve the wording > such that you actually really mention a common license name that applies > to your work on this man page. > Are this ok? "This manual page was written by Jörg Frings-Fürst for the Debian project and is licensed under BSD-3." > >> Just wondering (haven't check yet), but you only add symbols files for > >> amd64 and i386. Is this working correctly with the other archs? Or are > >> they going to be more strict as a result? > >> > > My error. I have differences between amd64 and i386. So I have renamed > > the symbols file for libxmlrpc-c++8 to *.amd86 and *.i386. > > > > I have renamed libxmlrpc-c++8.symbols.amd64 to libxmlrpc-c++8.symbols. > > Wouldn't it be possible to merge the symbols files so that the common > part can still be used (haven't checked the content of the files myself > yet). > No, the symbols file are different between i386 and the other archs. > I think you don't need to add the version to the dpkg-gensymbols call, > and if you do, why strip the Debian part of the version? Doesn't > dh_makeshlibs call dpkg-gensymbols itself? So if you try to override > anything, shouldn't the dpkg-gensymbols calls be BEFORE the > dh_makeshlibs call? This doesn't look right to me. Have you seen > https://wiki.debian.org/UsingSymbolsFiles where it describes a way to > create a symbols file that contains as much history as possible? > If the symbols file contains versions with the Debian revision lintian displays a error[1]. No dpkg-gensymbols must run manually. Without the separate call no symbol files found. With I can found the diffs in the buildlog. And yes dpgk-gensymbols must be running befor dh_makeshlibs. Changed. > >> Please separate your commits to git to ease review and understanding. > > > > ok > > Thanks for doing this a bit. However, your commit dbfaa7a7cc is horrible > to review though, because you mix upstream changes due to the new > release with your changes. I recommend to have the new upstream release > in a clean commit and then add your changes nicely documented afterwards. > If you want I can make a "git reset" and push the changes separately. > >> I don't understand why you created a "new upstream release". As far as I > >> see it the only change is the version number and the distclean target in > >> the Makefile. No further changes in the files that you decided to keep > >> around. > >> > > > > Are the upstream change from 1.33.14 to 1.33.15 not enough? > > As I said, there weren't ANY. But as you now package a full new version, > this point is moot. > > > A new version is uploaded to mentors. > > Again, I look at the git version and assume it is the same. > > Some new remarks: > > It looks like you are mixing ideas in your d/rules file. You export > DEB_BUILD_MAINT_OPTIONS = hardening=+all, but at the same time you > declare all the *FLAGS manually (which you shouldn't). Also you don't > use or export the *FLAGS. I think you should check the last section of > dpkg-buildflags(1). (I could be wrong here). I think it would make > 500-mk_gennnmtab.patch unnecessary. > First I think hardening all is requested for xmlrpc-c. Only with "export DEB_BUILD_MAINT_OPTIONS = hardening=+all" a get a FTBFS: /usr/bin/ld: AbyssServer.osh: relocation R_X86_64_PC32 against symbol `_ZN8xmlrpc_c11AbyssServer10ReqHandler9terminateEv' can not be used when making a shared object; recompile with -fPIC /usr/bin/ld: final link failed: Bad value collect2: error: ld returned 1 exit status With [C|CPP|CXX]FLAGS += -fPIC the build runs, but I get by testing with blhc CPPFLAGS missing (-D_FORTIFY_SOURCE=2): [..] Then with [C|CPP|CXX]FLAGS += -fPIC -D_FORTIFY_SOURCE=2 blhc display on errors. Only lintian says: I: libxmlrpc-c++8: hardening-no-fortify-functions usr/lib/x86_64-linux-gnu/libxmlrpc_packetsocket.so.8.39 It looks like an error at the makefiles. Useless CFLAGS_PERSONAL & LDFLAGS removed. > Going to bed now, more review coming. > > Paul > > CU Jörg [1] https://lintian.debian.org/tags/symbols-file-contains-current-version-with-debian-revision.html -- New: GPG Fingerprint: 63E0 075F C8D4 3ABB 35AB 30EE 09F8 9F3C 8CA1 D25D GPG key (long) : 09F89F3C8CA1D25D GPG Key : 8CA1D25D CAcert Key S/N : 0E:D4:56 Old (revoked since 2014-12-31): pgp Fingerprint: 7D13 3C60 0A10 DBE1 51F8 EBCB 422B 44B0 BE58 1B6E pgp Key: BE581B6E Jörg Frings-Fürst D-54526 Niederkail Threema: SYR8SJXB IRC: j_...@freenode.net j_...@oftc.net
signature.asc
Description: This is a digitally signed message part