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_f-f@freenode.net
j_f-f@oftc.net
Attachment:
signature.asc
Description: This is a digitally signed message part