<> {{{#!irc [20:05] so who's around for the FTBFS session? [20:05] * Rail is [20:05] thanks pleia2 [20:05] sure thing :) [20:05] * norax_ is [20:05] * sebner waves [20:05] * funkyHat is ^·^ [20:05] * ikt dances [20:06] i mean hi [20:06] is geser around as well? :) [20:06] * funkyHat dances on ikt [20:06] sistpoty: yes [20:06] excellent, then let's get started [20:06] I'm already looking for a good example [20:06] :) [20:06] first off, our recent archive rebuild showed a lot of packages, that fail to build from source (that's what FTBFS stands for) [20:07] you can see the results at http://people.ubuntuwire.org/~wgrant/rebuild-ftbfs-test/test-rebuild-20090909.html [20:07] http://launchpadlibrarian.net/32020496/buildlog_ubuntu-karmic-i386.libofa_0.9.3-3_FAILEDTOBUILD.txt.gz looks like a good candidate [20:08] yes, let's take this one [20:08] first off, a number of packages have already been fixed on the list [20:09] so please first check if the version in the archive is not already newer than the one in this list [20:10] however there are more good sources to look at in this list... [20:10] the "PTS" link goes straight to the debian package tracking system [20:10] hi all how are you guys doing? [20:10] maybe unstable already has a newer version [20:11] the "BTS" link goes to the debian bug tracking system [20:11] eventually there's already a bug and/or a patch there [20:11] let's check [20:12] sorry, I am late. which bug are you on currently ? [20:12] c_korn, http://launchpadlibrarian.net/32020496/buildlog_ubuntu-karmic-i386.libofa_0.9.3-3_FAILEDTOBUILD.txt.gz [20:12] c_korn: libofa from http://people.ubuntuwire.org/~wgrant/rebuild-ftbfs-test/test-rebuild-20090909.html [20:12] thanks [20:12] quit [20:13] now in this case, we're lucky, because at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=504902 there already is a patch :) [20:13] lo all [20:13] one should also look at the bugs in LP for that package in case an other contributor fixed it already and waits on sponsoring [20:14] https://launchpad.net/ubuntu/+source/libofa/+bugs [20:15] maybe we'll pick another package, where it's up to us to do the work? [20:17] * sistpoty looks for a good one [20:17] like http://launchpadlibrarian.net/31983196/buildlog_ubuntu-karmic-i386.libcommoncpp2_1.7.3-1_FAILEDTOBUILD.txt.gz? [20:18] * funkyHat doesn't see where libofa has been fixed ? [20:18] funkyHat: there is a Debian bug with a patch which "just" need to be packaged and sponsored into Ubuntu [20:19] geser: that one is excellent :) [20:19] or tor.. [20:19] funkyHat: here: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=504902 [20:19] while in itself easy to do (and should be done) it's not an good example how to fix it yourself [20:20] Ok. Perhaps I will have a go at fixing that one then, should be good practice [20:20] ok, so at first we'll be looking again at BTS and PTS, and at lp bugs of libcommoncpp2 [20:21] funkyHat: ping me and I'll sponsor it then [20:22] sebner: to main? [20:22] geser: ah, didn't check where it's based :\ [20:22] so no fixes in BTS, no newer version in unstable and no open bugs in launchpad [20:22] for libcommoncpp2 [20:22] I guess I could offer sponsoring to main for FTBFSes ;) [20:23] however there's another thing we could check for libcommoncpp2 [20:23] if you type apt-cache showsrc libcommoncpp2 [20:23] you'll see a field called Vcs-Browser: http://svn.debian.org/wsvn/pkg-voip/libcommoncpp2/?op=log [20:23] (I'm not sure if there's a format I should use for the bug report, but I won't disturb the class any more) [20:23] and Vcs-Svn: svn://svn.debian.org/pkg-voip/libcommoncpp2/trunk/ [20:24] (or use the links on the PTS page for libcommoncpp2) [20:24] funkyHat: along the lines "patch to fix FTBFS", please subscribe me if you've got a patch ;) [20:25] at this link (or the VCS link from PTS) is where development of the debianization happens [20:25] at a glance, there doesn't seem to be anything related to the FTBFS [20:25] finally, let's grab the sourcepackage and get working [20:26] while the source package downloads, let's try to see where the first error is that gcc reported [20:27] to make it more clear, I've pastebinned it (as taken from the build log) [20:27] http://paste.ubuntu.com/273724/ [20:28] so we'll need to look add ciddr.cpp, lines 205 and 335 [20:28] (they reside in the "src" subdirectory) [20:28] everyone got that file open right now? [20:29] yep [20:30] I've pastebinned the relevant part again: http://paste.ubuntu.com/273729/ [20:31] now gcc tells us about an "invalid conversion from 'const char*' to 'char*'" [20:32] a const pointer means, that you cannot change the object (memory) it points to [20:32] in c++ (and in c as well) you can only get rid of that const by casting [20:32] however casting often enough is not safe [20:33] e.g. if the memory is read-only memory (e.g. if it's a string constant like const char *s = "hello world";) [20:33] if you'd cast away the const for read only memory, the program would simply segfault when trying to write to it [20:33] the posix c string functions are a little bit nasty... you pass them a const char * [20:34] and you obtain a char * out of these, pointing to the memory that the parameter referred to [20:34] erm, I'm referring to strrchr for example (strchr and a few other follow that scheme) [20:35] this means, they implicitely get rid of the const (which might be dangerous) [20:35] in c there is sadly no alternative to it [20:35] however in c++, you can overload functions [20:35] and that's one of the gcc-4.4 changes [20:35] there exist two overloaded strrchr functions [20:35] one which gets a char * as parameter and returns a char * [20:36] and one which gets a const char * as parameter and returns a const char * [20:36] gcc then selects the correct one based on the parameter passed into it (not by the return type) [20:37] so with gcc-4.4 you can no longer accidentally get rid of the const by calling strrchr [20:37] in this example (line 205) , cp is declared const, hence the return value (ep) must also be const [20:38] however as ep is written to later, we cannot simply declare it const as well [20:39] let's try to see what the entire IPV4Cidr::set method does [20:41] the only place, where ep is written to, is straight afterwards, so let's take a closer look at this snippet [20:41] http://paste.ubuntu.com/273732/ [20:41] still following me so far? [20:41] yes [20:41] Just about! [20:41] ya [20:42] ok, good [20:42] this snippet tries to find the last occurance of a '/' in cp [20:42] in case you missed this write access, gcc will inform you during your test build (-> FTBFS :) [20:42] and then sets it to '\0' [20:43] which means it simply truncates cp at (i.e. before) the last '/'. [20:43] now comes the fun: this means that it writes to cp, which is passed as *const* into the method. tststs [20:44] at this point, we can either choose the unelegant and simple way, or try to get it right (which might mean pain, pain, pain) [20:44] it looks like we found a bug [20:44] the simple way would be to assume that it worked before, and obviously the const'ness of cp is a red herring [20:45] so we can cast it away [20:45] with: const_cast(whatwewanttocast) [20:46] or in this case line 205: ep = strchr(const_cast(cp), '/'); [20:46] sistpoty: from a look at this function, should line 211 (cp = cbuf) be moved before line 202? [20:47] geser: that could be [20:47] however the program could also require the side effect that cp is in fact changed [20:48] (that's always hard to judge from a glimpse) [20:48] that's a problem :( [20:48] yes [20:48] but anyway: this function doesn't seem to the right thing in case someone passes a string of e.g. "192.168.1/24" [20:49] geser: indeed. [20:49] yes, then it segfaults [20:49] of course if you're unsure, there's always one very good option to choose: [20:49] sistpoty: I mean even if it's passed in a memory from e.g. strcpy [20:49] ask upstream :) [20:51] the function takes our string, copies it (line 200), then strips it off at '/' and fills up the copy (still with the '/' with ".0" till it has 4 octects [20:52] when now someone passes "192.168.1/24" it turns cp into "192.168.1" and makes cbuf contain "192.168.1/24.0" [20:52] yes, that looks like it [20:52] don't know what inet_aton will make out of it [20:53] (parse it and convert it to a non-string representation) [20:53] (and probably fail at the parsing) [20:54] this is probably now a good time to look if upstream released a new version and look if it's fixed there else contact upstream with what we found out and let upstream handle it [20:56] their homepage announces: GNU Common C++ 2.0 Beta Candidate [20:56] http://www.gnu.org/software/commoncpp/ [20:59] hm... has anyone found a download link for the 2.0 beta yet? [20:59] * sistpoty only sees 1.7.3 [20:59] * geser too [21:00] and cvs seems to be 404 [21:00] seems so [21:01] ftp://www.mirrorservice.org/sites/ftp.gnu.org/gnu/commoncpp/ [21:01] * BlackFate away [21:02] ah, so it's called ucommon nowadays? [21:04] indeed it is, and I've also found our nice method again in the ucommon source package [21:05] it's in src/socket.cpp, line 788 [21:06] which a) has majored a bit, and b) seems to confirm geser's first idea how to fix it [21:07] sistpoty: from a look at this function, should line 211 (cp = cbuf) be moved before line 202? [21:08] so the string as is should be copied to cbuf, and only cbuf should get adjusted [21:09] let's try to do this [21:10] so we change line 205 to ep = strchr(cbuf, '/'); [21:11] any objection? [21:12] anyone still around? :) [21:12] here [21:12] * geser is [21:12] just let's try if it builds :) [21:12] here, but this is over my head, I'll try to follow what I understand :) [21:13] well, for sure it won't because we didn't adjust the error in line 305 yet [21:13] as one has seen here fixing FTBFS trains ones detective skill :) [21:13] funkyHat: just ask if anythings unclear [21:13] let's take a look at line 305 [21:13] any suggestions how to fix this? [21:14] why does it need fixing? I seem to overlook something there [21:15] line 335 ? [21:15] c_korn: yes, in the same file [21:15] cidr.cpp:335: error: invalid conversion from 'const char*' to 'char*' [21:15] (from the original build log) [21:15] line 335 makes more sense [21:16] erm, sorry :) [21:16] hm, ep gets written again [21:16] looks like a copy&paste bug to me :) [21:17] so anyone with a suggestion? [21:17] and when you compare the function name and what it does with the one we just fixed, then the fix should be pretty obvious [21:18] copy and paste fix ? ;) [21:18] nicolasvw: righto, let's use the buffer again [21:19] and finally now it's time to do a test-build [21:19] let's add a changelog entry and build it in pbuilder [21:20] hint: if you're working on a package, which might need a number of patches [21:20] it might be easier to install the build-dependencies on your system [21:20] and do a fakeroot make -f debian/rules binary [21:20] to test-build [21:20] as this can then (ideally) reuse the already built files and will only built your new changes (and dependencies) again [21:21] -- i.e. if the upstream build system supports it [21:21] of course as last action, you should then always test-build it in a clean (=pbuilder) environment [21:22] so anyone with a buildresult yet? [21:23] an other option is to start directly in a pbuilder (pbuilder login) but one has to don't forget to copy the changes outside the pbuilder before one exits it [21:23] (or use the --bindmounts option to pbuilder) [21:23] ? [21:23] what I currently do is using a pbuilder hook to get a shell if pbuilder fails so I can investigate or test more changes [21:24] doesn't pdebuild use it? never used pdebuild [21:24] heh, me neither... (as I once wrote my own pdebuild alike variant *g*) [21:25] one has to find the way which works for one the best (I prefer not to pollute my host system with -dev packages) [21:26] ok, it did build for me [21:26] so here's another thing that can be totally different [21:27] let's see if the package has a patch system. if so, we should add the fix as a patch, otherwise we can simply leave it as is [21:27] * sistpoty usually looks if there's a directory called debian/patches, but what-patch of ubuntu-dev-tools also should give you an answer [21:27] (is that the right command *g*) [21:28] in this case, it uses dpatch [21:28] so here's my tricky way to apply it [21:28] as I already added a changelog entry, I now have got 2 .dsc files lying around [21:28] and can simple debdiff between these two [21:29] this however means that my changelog entry (which I don't want in there) is also in the debdiff [21:29] but with filterdiff, it can easily get excluded: [21:29] debdiff libcommoncpp2_1.7.3-1.dsc libcommoncpp2_1.7.3-1ubuntu1.dsc | filterdiff -x "libcommoncpp2-1.7.3/debian/*" [21:30] this one gives me the patch, which I'm putting into debian/patches [21:30] hah, cheater. I never thought of that :) [21:31] now I've also need to add it to debian/patches/00list, so that it will get applied [21:31] and being a good citizen I should add a descriptive header to it [21:31] but the patch is now already applied to the sources. do you revert it manually ? [21:33] either that, or I unpack the old sources (depending on how much unwanted damage I did to the sources) [21:33] ok [21:34] however how you do it is pretty much your choice, you could also use dpatch-edit-patch [21:36] however after manually fiddling with patch systems, I usually do another test-build (one never knows *g*) [21:37] * c_korn usually sets up a git repository in the sources to get those patches :) [21:37] but what you should always do (after your final build) is to debdiff between the old and the new dsc file [21:38] that way you can make sure that only changes you really want are in the new version [21:38] should I write something about adding a debian/changelog entry, or is this clear for everyone [21:38] ? [21:39] clear to me (dholbach explained it in a session) [21:40] so what's left to do... [21:41] testing your fix is always a good idea [21:41] so you should install the resulting package and test it [21:41] in this case its a library, so testing it gets a little bit hard [21:42] but you should install it, and check the file contents [21:42] eventually a lintian run on the resulting binaries might also be a good idea... [21:42] however don't try to fix these bugs, but rather look out for really critical stuff [21:43] (happened to me recently, that after a no-change-rebuild the resulting package was empty, of course that a must to get this right then) [21:44] looks all good here :) [21:44] so now it's time to upload it to the archive, or (if you don't have powers to do so) to request sponsorship [21:45] but wait, we're not yet done... [21:45] what about update-maintainer ? [21:45] c_korn: of course, mea culpa [21:46] (I silently did that for my package when dpkg-buildpackage bailed out) [21:46] however ther's more to do [21:47] forwarding the bug and the fix [21:47] as the same version is also in debian, I'm forwarding it there. [21:48] geser: anything else that should get mentioned? [21:50] any questions from anyone? [21:50] nothing missed (or I missed it too because of the routine in doing it (like calling update-maintainer)) [21:50] heh [21:50] oh, I missed an important thing [21:51] as you've seen, fixing a FTBFS bug is not always trivial [21:51] so if you've come to a point, where you have no clue, it's a good idea to paste the build error (with a little bit of context) and the failing function in pastebin [21:51] and ask around (for example at #ubuntu-motu) [21:52] I'm quite sure, there'll always be someone in knowledge of the fix around, and most of the time will also give you an answer ;) [21:53] so concluding, I'd like to invite you all to #ubuntu-motu now, to get practice from the theoretical lesson right now [21:53] as a side note, until I'm too tired I'll be happy to sponsor fixes for FTBFS bugs ;)( [21:54] geser: oh, did you upload libcommoncpp2 already or did anyone else or should I do it? [21:54] sistpoty: good timing :) [21:54] * DasEi lacks c- / gcc knowledge for that [21:54] I just subscribed you to my libofa bug [21:55] sistpoty: no, I didn't upload it [21:55] DasEi: there are also other FTBFS, if you are more familiar with e.g. perl look at those [21:56] geser: ok, then I'll uload it in 5 minutes (/me needs a short break first *g*) [21:57] I didn't really manage to follow the bug-fixing on this one as I don't know any c++, but I learnt stuff anyway :) [21:58] well, I'm already writing code in c++ since 7 years or so, but I still don't understand it yet :P [21:58] :D [21:59] thank you sistpoty and geser for the great lesson. I am going for some bug hunting now :) [21:59] thanks everyone for coming! [21:59] excellent c_korn: [22:00] I'm going to wait for my first patch to be looked at before I try anything else, in case I got something wrong. Don't want to keep doing the wrong thing if I did :) [22:00] funkyHat: not everyone is that hard like that one, some are only applying a patch from Debian (like libofa) or adding a const for the same error like in the libcommoncpp2 case. you still can try and abort if it gets to hard [22:02] geser: right, and I just fixed the libofa one, but I want to check I got it right, rather than fix another one wrong as well :) [22:04] funkyHat: not that bad, missed one thing [22:04] forget to call update-maintainer [22:05] Ah [22:05] Now I should debuild -S again, then debdiff again? [22:09] eh, gambas2 FTBFS but the same version is already in karmic ? [22:09] funkyHat: yes [22:09] geser: I guessed and did it already :) [22:10] c_korn: the toolchain or one of the build-dependecies changed since it got build. [22:11] that's the reason behind the archive test rebuild: to know that it still builds (we already know that it build (or not build) in the past) [22:11] ah, ok [22:12] imagine a SRU (or security upload) you want to do later just to find out that you have to first fix a FTBFS too [22:12] right [22:13] there is a newer version in debian but the changelog does not mention changes regarding the FTBFS bug: http://packages.debian.org/changelogs/pool/main/g/gambas2/gambas2_2.15.2-1/changelog [22:15] c_korn: AFAIR this specific problem only appears since g++ 4.4 with eglibc 2.10 (Debian still has g++ 4.3 as default and eglibc 2.10 is only in experimental, so there was no test build with it in Debian yet) [22:16] hm, so what should I do now? file a sync request (which would require a FFe first) and patch it then ? or just patch it and it has to be merged later with debian in karmic+1 ? [22:27] c_korn: if there is no good reason for the new upstream version, patch it [22:27] ok [22:33] hm, another invalid conversion in line 5. and I cannot constify it. http://pastebin.com/d3445f605 [22:42] I better ask in #ubuntu-motu [22:43] yep, let's move to -motu }}} ---- CategoryPackaging