2009-09-18
Next Session: TBD |
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
Packaging/Training/Logs/2009-09-18 (last edited 2009-09-20 07:05:48 by cpe-68-173-227-236)