2009-09-18

   1 [20:05] <sistpoty> so who's around for the FTBFS session?
   2 [20:05]  * Rail is
   3 [20:05] <sistpoty> thanks pleia2
   4 [20:05] <pleia2> sure thing :)
   5 [20:05]  * norax_ is
   6 [20:05]  * sebner waves 
   7 [20:05]  * funkyHat is ^·^
   8 [20:05]  * ikt dances
   9 [20:06] <ikt> i mean hi
  10 [20:06] <sistpoty> is geser around as well? :)
  11 [20:06]  * funkyHat dances on ikt 
  12 [20:06] <geser> sistpoty: yes
  13 [20:06] <sistpoty> excellent, then let's get started
  14 [20:06] <geser> I'm already looking for a good example
  15 [20:06] <sistpoty> :)
  16 [20:06] <sistpoty> first off, our recent archive rebuild showed a lot of packages, that fail to build from source (that's what FTBFS stands for)
  17 [20:07] <sistpoty> you can see the results at http://people.ubuntuwire.org/~wgrant/rebuild-ftbfs-test/test-rebuild-20090909.html
  18 [20:07] <geser> http://launchpadlibrarian.net/32020496/buildlog_ubuntu-karmic-i386.libofa_0.9.3-3_FAILEDTOBUILD.txt.gz looks like a good candidate
  19 [20:08] <sistpoty> yes, let's take this one
  20 [20:08] <sistpoty> first off, a number of packages have already been fixed on the list
  21 [20:09] <sistpoty> so please first check if the version in the archive is not already newer than the one in this list
  22 [20:10] <sistpoty> however there are more good sources to look at in this list...
  23 [20:10] <sistpoty> the "PTS" link goes straight to the debian package tracking system
  24 [20:10] <dhillon-v10> hi all how are you guys doing?
  25 [20:10] <sistpoty> maybe unstable already has a newer version
  26 [20:11] <sistpoty> the "BTS" link goes to the debian bug tracking system
  27 [20:11] <sistpoty> eventually there's already a bug and/or a patch there
  28 [20:11] <sistpoty> let's check
  29 [20:12] <c_korn> sorry, I am late. which bug are you on currently ?
  30 [20:12] <RoAkSoAx> c_korn, http://launchpadlibrarian.net/32020496/buildlog_ubuntu-karmic-i386.libofa_0.9.3-3_FAILEDTOBUILD.txt.gz
  31 [20:12] <sistpoty> c_korn: libofa from http://people.ubuntuwire.org/~wgrant/rebuild-ftbfs-test/test-rebuild-20090909.html
  32 [20:12] <c_korn> thanks
  33 [20:12] <dhillon-v10> quit
  34 [20:13] <sistpoty> now in this case, we're lucky, because at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=504902 there already is a patch :)
  35 [20:13] <stlsaint> lo all
  36 [20:13] <geser> one should also look at the bugs in LP for that package in case an other contributor fixed it already and waits on sponsoring
  37 [20:14] <sistpoty> https://launchpad.net/ubuntu/+source/libofa/+bugs
  38 [20:15] <sistpoty> maybe we'll pick another package, where it's up to us to do the work?
  39 [20:17]  * sistpoty looks for a good one
  40 [20:17] <geser> like http://launchpadlibrarian.net/31983196/buildlog_ubuntu-karmic-i386.libcommoncpp2_1.7.3-1_FAILEDTOBUILD.txt.gz?
  41 [20:18]  * funkyHat doesn't see where libofa has been fixed ?
  42 [20:18] <geser> funkyHat: there is a Debian bug with a patch which "just" need to be packaged and sponsored into Ubuntu
  43 [20:19] <sistpoty> geser: that one is excellent :)
  44 [20:19] <DasEi> or tor..
  45 [20:19] <sistpoty> funkyHat: here: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=504902
  46 [20:19] <geser> while in itself easy to do (and should be done) it's not an good example how to fix it yourself
  47 [20:20] <funkyHat> Ok. Perhaps I will have a go at fixing that one then, should be good practice
  48 [20:20] <sistpoty> ok, so at first we'll be looking again at BTS and PTS, and at lp bugs of libcommoncpp2
  49 [20:21] <sebner> funkyHat: ping me and I'll sponsor it then
  50 [20:22] <geser> sebner: to main?
  51 [20:22] <sebner> geser: ah, didn't check where it's based :\
  52 [20:22] <sistpoty> so no fixes in BTS, no newer version in unstable and no open bugs in launchpad
  53 [20:22] <sistpoty> for libcommoncpp2
  54 [20:22] <sistpoty> I guess I could offer sponsoring to main for FTBFSes ;)
  55 [20:23] <sistpoty> however there's another thing we could check for libcommoncpp2
  56 [20:23] <sistpoty> if you type apt-cache showsrc libcommoncpp2
  57 [20:23] <sistpoty> you'll see a field called Vcs-Browser: http://svn.debian.org/wsvn/pkg-voip/libcommoncpp2/?op=log
  58 [20:23] <funkyHat> (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)
  59 [20:23] <sistpoty> and Vcs-Svn: svn://svn.debian.org/pkg-voip/libcommoncpp2/trunk/
  60 [20:24] <geser> (or use the links on the PTS page for libcommoncpp2)
  61 [20:24] <sistpoty> funkyHat: along the lines "patch to fix FTBFS", please subscribe me if you've got a patch ;)
  62 [20:25] <sistpoty> at this link (or the VCS link from PTS) is where development of the debianization happens
  63 [20:25] <sistpoty> at a glance, there doesn't seem to be anything related to the FTBFS
  64 [20:25] <sistpoty> finally, let's grab the sourcepackage and get working
  65 [20:26] <sistpoty> while the source package downloads, let's try to see where the first error is that gcc reported
  66 [20:27] <sistpoty> to make it more clear, I've pastebinned it (as taken from the build log)
  67 [20:27] <sistpoty> http://paste.ubuntu.com/273724/
  68 [20:28] <sistpoty> so we'll need to look add ciddr.cpp, lines 205 and 335
  69 [20:28] <sistpoty> (they reside in the "src" subdirectory)
  70 [20:28] <sistpoty> everyone got that file open right now?
  71 [20:29] <funkyHat> yep
  72 [20:30] <sistpoty> I've pastebinned the relevant part again: http://paste.ubuntu.com/273729/
  73 [20:31] <sistpoty> now gcc tells us about an "invalid conversion from 'const char*' to 'char*'"
  74 [20:32] <sistpoty> a const pointer means, that you cannot change the object (memory) it points to
  75 [20:32] <sistpoty> in c++ (and in c as well) you can only get rid of that const by casting
  76 [20:32] <sistpoty> however casting often enough is not safe
  77 [20:33] <sistpoty> e.g. if the memory is read-only memory (e.g. if it's a string constant like const char *s = "hello world";)
  78 [20:33] <sistpoty> if you'd cast away the const for read only memory, the program would simply segfault when trying to write to it
  79 [20:33] <sistpoty> the posix c string functions are a little bit nasty... you pass them a const char *
  80 [20:34] <sistpoty> and you obtain a char * out of these, pointing to the memory that the parameter referred to
  81 [20:34] <sistpoty> erm, I'm referring to strrchr for example (strchr and a few other follow that scheme)
  82 [20:35] <sistpoty> this means, they implicitely get rid of the const (which might be dangerous)
  83 [20:35] <sistpoty> in c there is sadly no alternative to it
  84 [20:35] <sistpoty> however in c++, you can overload functions
  85 [20:35] <sistpoty> and that's one of the gcc-4.4 changes
  86 [20:35] <sistpoty> there exist two overloaded strrchr functions
  87 [20:35] <sistpoty> one which gets a char * as parameter and returns a char *
  88 [20:36] <sistpoty> and one which gets a const char * as parameter and returns a const char *
  89 [20:36] <sistpoty> gcc then selects the correct one based on the parameter passed into it (not by the return type)
  90 [20:37] <sistpoty> so with gcc-4.4 you can no longer accidentally get rid of the const by calling strrchr
  91 [20:37] <sistpoty> in this example (line 205) , cp is declared const, hence the return value (ep) must also be const
  92 [20:38] <sistpoty> however as ep is written to later, we cannot simply declare it const as well
  93 [20:39] <sistpoty> let's try to see what the entire IPV4Cidr::set method does
  94 [20:41] <sistpoty> the only place, where ep is written to, is straight afterwards, so let's take a closer look at this snippet
  95 [20:41] <sistpoty> http://paste.ubuntu.com/273732/
  96 [20:41] <sistpoty> still following me so far?
  97 [20:41] <c_korn> yes
  98 [20:41] <funkyHat> Just about!
  99 [20:41] <jureba> ya
 100 [20:42] <sistpoty> ok, good
 101 [20:42] <sistpoty> this snippet tries to find the last occurance of a '/' in cp
 102 [20:42] <geser> in case you missed this write access, gcc will inform you during your test build (-> FTBFS :)
 103 [20:42] <sistpoty> and then sets it to '\0'
 104 [20:43] <sistpoty> which means it simply truncates cp at (i.e. before) the last '/'.
 105 [20:43] <sistpoty> now comes the fun: this means that it writes to cp, which is passed as *const* into the method. tststs
 106 [20:44] <sistpoty> at this point, we can either choose the unelegant and simple way, or try to get it right (which might mean pain, pain, pain)
 107 [20:44] <geser> it looks like we found a bug
 108 [20:44] <sistpoty> the simple way would be to assume that it worked before, and obviously the const'ness of cp is a red herring
 109 [20:45] <sistpoty> so we can cast it away
 110 [20:45] <sistpoty> with: const_cast<char *>(whatwewanttocast)
 111 [20:46] <sistpoty> or in this case line 205: ep = strchr(const_cast<char *>(cp), '/');
 112 [20:46] <geser> sistpoty: from a look at this function, should line 211 (cp = cbuf) be moved before line 202?
 113 [20:47] <sistpoty> geser: that could be
 114 [20:47] <sistpoty> however the program could also require the side effect that cp is in fact changed
 115 [20:48] <sistpoty> (that's always hard to judge from a glimpse)
 116 [20:48] <geser> that's a problem :(
 117 [20:48] <sistpoty> yes
 118 [20:48] <geser> but anyway: this function doesn't seem to the right thing in case someone passes a string of e.g. "192.168.1/24"
 119 [20:49] <sbeattie> geser: indeed.
 120 [20:49] <sistpoty> yes, then it segfaults
 121 [20:49] <sistpoty> of course if you're unsure, there's always one very good option to choose:
 122 [20:49] <geser> sistpoty: I mean even if it's passed in a memory from e.g. strcpy
 123 [20:49] <sistpoty> ask upstream :)
 124 [20:51] <geser> 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
 125 [20:52] <geser> when now someone passes "192.168.1/24" it turns cp into "192.168.1" and makes cbuf contain "192.168.1/24.0"
 126 [20:52] <sistpoty> yes, that looks like it
 127 [20:52] <geser> don't know what inet_aton will make out of it
 128 [20:53] <sistpoty> (parse it and convert it to a non-string representation)
 129 [20:53] <geser> (and probably fail at the parsing)
 130 [20:54] <geser> 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
 131 [20:56] <c_korn> their homepage announces: GNU Common C++ 2.0 Beta Candidate
 132 [20:56] <c_korn> http://www.gnu.org/software/commoncpp/
 133 [20:59] <sistpoty> hm... has anyone found a download link for the 2.0 beta yet?
 134 [20:59]  * sistpoty only sees 1.7.3
 135 [20:59]  * geser too
 136 [21:00] <sistpoty> and cvs seems to be 404
 137 [21:00] <c_korn> seems so
 138 [21:01] <funkyHat> ftp://www.mirrorservice.org/sites/ftp.gnu.org/gnu/commoncpp/
 139 [21:01]  * BlackFate away
 140 [21:02] <sistpoty> ah, so it's called ucommon nowadays?
 141 [21:04] <sistpoty> indeed it is, and I've also found our nice method again in the ucommon source package
 142 [21:05] <sistpoty> it's in src/socket.cpp, line 788
 143 [21:06] <sistpoty> which a) has majored a bit, and b) seems to confirm geser's first idea how to fix it
 144 [21:07] <sistpoty> <geser> sistpoty: from a look at this function, should line 211 (cp = cbuf) be moved before line 202?
 145 [21:08] <sistpoty> so the string as is should be copied to cbuf, and only cbuf should get adjusted
 146 [21:09] <sistpoty> let's try to do this
 147 [21:10] <sistpoty> so we change line 205 to      ep = strchr(cbuf, '/');
 148 [21:11] <sistpoty> any objection?
 149 [21:12] <sistpoty> anyone still around? :)
 150 [21:12] <c_korn> here
 151 [21:12]  * geser is
 152 [21:12] <c_korn> just let's try if it builds :)
 153 [21:12] <funkyHat> here, but this is over my head, I'll try to follow what I understand :)
 154 [21:13] <sistpoty> well, for sure it won't because we didn't adjust the error in line 305 yet
 155 [21:13] <geser> as one has seen here fixing FTBFS trains ones detective skill :)
 156 [21:13] <sistpoty> funkyHat: just ask if anythings unclear
 157 [21:13] <sistpoty> let's take a look at line 305
 158 [21:13] <sistpoty> any suggestions how to fix this?
 159 [21:14] <geser> why does it need fixing? I seem to overlook something there
 160 [21:15] <c_korn> line 335 ?
 161 [21:15] <sistpoty> c_korn: yes, in the same file
 162 [21:15] <sistpoty> cidr.cpp:335: error: invalid conversion from 'const char*' to 'char*'
 163 [21:15] <sistpoty> (from the original build log)
 164 [21:15] <geser> line 335 makes more sense
 165 [21:16] <sistpoty> erm, sorry :)
 166 [21:16] <c_korn> hm, ep gets written again
 167 [21:16] <sistpoty> looks like a copy&paste bug to me :)
 168 [21:17] <sistpoty> so anyone with a suggestion?
 169 [21:17] <geser> and when you compare the function name and what it does with the one we just fixed, then the fix should be pretty obvious
 170 [21:18] <nicolasvw> copy and paste fix ? ;)
 171 [21:18] <sistpoty> nicolasvw: righto, let's use the buffer again
 172 [21:19] <sistpoty> and finally now it's time to do a test-build
 173 [21:19] <sistpoty> let's add a changelog entry and build it in pbuilder
 174 [21:20] <sistpoty> hint: if you're working on a package, which might need a number of patches
 175 [21:20] <sistpoty> it might be easier to install the build-dependencies on your system
 176 [21:20] <sistpoty> and do a fakeroot make -f debian/rules binary
 177 [21:20] <sistpoty> to test-build
 178 [21:20] <sistpoty> as this can then (ideally) reuse the already built files and will only built your new changes (and dependencies) again
 179 [21:21] <sistpoty> -- i.e. if the upstream build system supports it
 180 [21:21] <sistpoty> of course as last action, you should then always test-build it in a clean (=pbuilder) environment
 181 [21:22] <sistpoty> so anyone with a buildresult yet?
 182 [21:23] <geser> 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
 183 [21:23] <nicolasvw> (or use the --bindmounts option to pbuilder)
 184 [21:23] <nicolasvw> ?
 185 [21:23] <geser> what I currently do is using a pbuilder hook to get a shell if pbuilder fails so I can investigate or test more changes
 186 [21:24] <geser> doesn't pdebuild use it? never used pdebuild
 187 [21:24] <sistpoty> heh, me neither... (as I once wrote my own pdebuild alike variant *g*)
 188 [21:25] <geser> one has to find the way which works for one the best (I prefer not to pollute my host system with -dev packages)
 189 [21:26] <sistpoty> ok, it did build for me
 190 [21:26] <sistpoty> so here's another thing that can be totally different
 191 [21:27] <sistpoty> 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
 192 [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
 193 [21:27] <sistpoty> (is that the right command *g*)
 194 [21:28] <sistpoty> in this case, it uses dpatch
 195 [21:28] <sistpoty> so here's my tricky way to apply it
 196 [21:28] <sistpoty> as I already added a changelog entry, I now have got 2 .dsc files lying around
 197 [21:28] <sistpoty> and can simple debdiff between these two
 198 [21:29] <sistpoty> this however means that my changelog entry (which I don't want in there) is also in the debdiff
 199 [21:29] <sistpoty> but with filterdiff, it can easily get excluded:
 200 [21:29] <sistpoty> debdiff libcommoncpp2_1.7.3-1.dsc libcommoncpp2_1.7.3-1ubuntu1.dsc | filterdiff -x "libcommoncpp2-1.7.3/debian/*"
 201 [21:30] <sistpoty> this one gives me the patch, which I'm putting into debian/patches
 202 [21:30] <c_korn> hah, cheater. I never thought of that :)
 203 [21:31] <sistpoty> now I've also need to add it to debian/patches/00list, so that it will get applied
 204 [21:31] <sistpoty> and being a good citizen I should add a descriptive header to it
 205 [21:31] <c_korn> but the patch is now already applied to the sources. do you revert it manually ?
 206 [21:33] <sistpoty> either that, or I unpack the old sources (depending on how much unwanted damage I did to the sources)
 207 [21:33] <c_korn> ok
 208 [21:34] <sistpoty> however how you do it is pretty much your choice, you could also use dpatch-edit-patch
 209 [21:36] <sistpoty> however after manually fiddling with patch systems, I usually do another test-build (one never knows *g*)
 210 [21:37]  * c_korn usually sets up a git repository in the sources to get those patches :)
 211 [21:37] <sistpoty> but what you should always do (after your final build) is to debdiff between the old and the new dsc file
 212 [21:38] <sistpoty> that way you can make sure that only changes you really want are in the new version
 213 [21:38] <sistpoty> should I write something about adding a debian/changelog entry, or is this clear for everyone
 214 [21:38] <sistpoty> ?
 215 [21:39] <c_korn> clear to me (dholbach explained it in a session)
 216 [21:40] <sistpoty> so what's left to do...
 217 [21:41] <sistpoty> testing your fix is always a good idea
 218 [21:41] <sistpoty> so you should install the resulting package and test it
 219 [21:41] <sistpoty> in this case its a library, so testing it gets a little bit hard
 220 [21:42] <sistpoty> but you should install it, and check the file contents
 221 [21:42] <sistpoty> eventually a lintian run on the resulting binaries might also be a good idea...
 222 [21:42] <sistpoty> however don't try to fix these bugs, but rather look out for really critical stuff
 223 [21:43] <sistpoty> (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)
 224 [21:44] <sistpoty> looks all good here :)
 225 [21:44] <sistpoty> so now it's time to upload it to the archive, or (if you don't have powers to do so) to request sponsorship
 226 [21:45] <sistpoty> but wait, we're not yet done...
 227 [21:45] <c_korn> what about update-maintainer ?
 228 [21:45] <sistpoty> c_korn: of course, mea culpa
 229 [21:46] <sistpoty> (I silently did that for my package when dpkg-buildpackage bailed out)
 230 [21:46] <sistpoty> however ther's more to do
 231 [21:47] <sistpoty> forwarding the bug and the fix
 232 [21:47] <sistpoty> as the same version is also in debian, I'm forwarding it there.
 233 [21:48] <sistpoty> geser: anything else that should get mentioned?
 234 [21:50] <sistpoty> any questions from anyone?
 235 [21:50] <geser> nothing missed (or I missed it too because of the routine in doing it (like calling update-maintainer))
 236 [21:50] <sistpoty> heh
 237 [21:50] <sistpoty> oh, I missed an important thing
 238 [21:51] <sistpoty> as you've seen, fixing a FTBFS bug is not always trivial
 239 [21:51] <sistpoty> 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
 240 [21:51] <sistpoty> and ask around (for example at #ubuntu-motu)
 241 [21:52] <sistpoty> 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 ;)
 242 [21:53] <sistpoty> so concluding, I'd like to invite you all to #ubuntu-motu now, to get practice from the theoretical lesson right now
 243 [21:53] <sistpoty> as a side note, until I'm too tired I'll be happy to sponsor fixes for FTBFS bugs ;)(
 244 [21:54] <sistpoty> geser: oh, did you upload libcommoncpp2 already or did anyone else or should I do it?
 245 [21:54] <funkyHat> sistpoty: good timing :)
 246 [21:54]  * DasEi lacks c- / gcc knowledge for that
 247 [21:54] <funkyHat> I just subscribed you to my libofa bug
 248 [21:55] <geser> sistpoty: no, I didn't upload it
 249 [21:55] <geser> DasEi: there are also other FTBFS, if you are more familiar with e.g. perl look at those
 250 [21:56] <sistpoty> geser: ok, then I'll uload it in 5 minutes (/me needs a short break first *g*)
 251 [21:57] <funkyHat> 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 :)
 252 [21:58] <sistpoty> well, I'm already writing code in c++ since 7 years or so, but I still don't understand it yet :P
 253 [21:58] <funkyHat> :D
 254 [21:59] <c_korn> thank you sistpoty and geser for the great lesson. I am going for some bug hunting now :)
 255 [21:59] <sistpoty> thanks everyone for coming!
 256 [21:59] <sistpoty> excellent c_korn:
 257 [22:00] <funkyHat> 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 :)
 258 [22:00] <geser> 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
 259 [22:02] <funkyHat> 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 :)
 260 [22:04] <geser> funkyHat: not that bad, missed one thing
 261 [22:04] <geser> forget to call update-maintainer
 262 [22:05] <funkyHat> Ah
 263 [22:05] <funkyHat> Now I should debuild -S again, then debdiff again?
 264 [22:09] <c_korn> eh, gambas2 FTBFS but the same version is already in karmic ?
 265 [22:09] <geser> funkyHat: yes
 266 [22:09] <funkyHat> geser: I guessed and did it already :)
 267 [22:10] <geser> c_korn: the toolchain or one of the build-dependecies changed since it got build.
 268 [22:11] <geser> 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)
 269 [22:11] <c_korn> ah, ok
 270 [22:12] <geser> imagine a SRU (or security upload) you want to do later just to find out that you have to first fix a FTBFS too
 271 [22:12] <c_korn> right
 272 [22:13] <c_korn> 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
 273 [22:15] <geser> 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)
 274 [22:16] <c_korn> 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 ?
 275 [22:27] <geser> c_korn: if there is no good reason for the new upstream version, patch it
 276 [22:27] <c_korn> ok
 277 [22:33] <c_korn> hm, another invalid conversion in line 5. and I cannot constify it. http://pastebin.com/d3445f605
 278 [22:42] <c_korn> I better ask in #ubuntu-motu
 279 [22:43] <sistpoty> yep, let's move to -motu
 280 


CategoryPackaging

Packaging/Training/Logs/2009-09-18 (last edited 2009-09-20 07:05:48 by andrewsomething)