Still no joy in Mudville. After seeing failures in a variety of different ways with each new check-in, the build is stuck on box3d.cpp:
box3d.cpp: In function ‘Inkscape::XML::Node* box3d_write(SPObject*, Inkscape::XML::DocumentTree*, Inkscape::XML::Node*, guint)’: box3d.cpp:281: error: invalid use of incomplete type ‘struct Inkscape::XML::Document’ ./xml/xml-forward.h:23: error: forward declaration of ‘struct Inkscape::XML::Document’ box3d.cpp: In function ‘Geom::Matrix box3d_set_transform(SPItem*, const Geom::Matrix&)’: box3d.cpp:342: error: cannot convert ‘Inkscape::XML::Document*’ to ‘Document*’ for argument ‘1’ to ‘Persp3D* persp3d_create_xml_element(Document*, Persp3D*)’ box3d.cpp: In function ‘SPGroup* box3d_convert_to_group(SPBox3D*)’: box3d.cpp:1382: error: invalid use of incomplete type ‘struct Inkscape::XML::Document’ ./xml/xml-forward.h:23: error: forward declaration of ‘struct Inkscape::XML::Document’ box3d.cpp:1395: error: invalid use of incomplete type ‘struct Inkscape::XML::Document’ ./xml/xml-forward.h:23: error: forward declaration of ‘struct Inkscape::XML::Document’ box3d.cpp:1421: error: invalid use of incomplete type ‘struct Inkscape::XML::Document’ ./xml/xml-forward.h:23: error: forward declaration of ‘struct Inkscape::XML::Document’
Tav
On Aug 5, 2009, at 11:29 PM, Tavmjong Bah wrote:
Still no joy in Mudville. After seeing failures in a variety of different ways with each new check-in, the build is stuck on box3d.cpp:
box3d.cpp: In function ‘Inkscape::XML::Node* box3d_write(SPObject*, Inkscape::XML::DocumentTree*, Inkscape::XML::Node*, guint)’: box3d.cpp:281: error: invalid use of incomplete type ‘struct Inkscape::XML::Document’ ./xml/xml-forward.h:23: error: forward declaration of ‘struct Inkscape::XML::Document’
Ah, seems quite simple. Here is the root problem:
r21994 | johnce | 2009-08-05 11:32:38 -0700 (Wed, 05 Aug 2009) | 1 line
xml/Document -> DocumentTree
It appears that someone renamed the struct in the .h but did not rename it in forward.h, and did not change the usages of it.
2009/8/6 Jon A. Cruz <jon@...18...>:
Ah, seems quite simple. Here is the root problem:
r21994 | johnce | 2009-08-05 11:32:38 -0700 (Wed, 05 Aug 2009) | 1 line
xml/Document -> DocumentTree
It appears that someone renamed the struct in the .h but did not rename it in forward.h, and did not change the usages of it.
I spent the better part of four hours trying to fix this (because each change broke things at other places) and finally ended up with the discovery that there are some #includes in file.(h|cpp) (namely, streams-webdav.h and friends) for which the corresponding files are missing. Thus I decided to revert the recent set of changes by johnce, also because I'm not sure it's a good idea to have such huge commits so close to the release. If I overlooked something and there is an easy fix please feel free to undo my revert.
BTW, during my fixing attempts I partially had to revert changes made by johnce because Document (formerly known as SPDocument) and Inkscape::XML::DocumentTree (formerly known as Inkscape::XML::Document) got mixed up in his latest commits. So it seems to me that there may be quite a few places where bugs might be introduced by these commits. I don't know what others think about it, but IMHO it would be better to hold them back until 0.47 is out. Don't get me wrong, I believe that the changes are valuable and the renaming can eliminate potential sources of confusion, but it's just too dangerous to apply them now. Opinions?
Max
@johnce: Please always make sure to only commit changes which don't break the build! I had planned to work on some other things tonight but essentially spent the whole evening trying to fix compiling so my pending work will have to wait until I can spare some more time (which may only be in a couple of days if I'm unlucky).
On 6/8/09 16:18, Maximilian Albert wrote:
I spent the better part of four hours trying to fix this (because each change broke things at other places) and finally ended up with the discovery that there are some #includes in file.(h|cpp) (namely, streams-webdav.h and friends) for which the corresponding files are missing.
bug #258418 “webdav/http/ftp uri patch” https://bugs.launchpad.net/inkscape/+bug/258418 ?
~suv
Maximilian,
thank you so much!
In my opinion to simply put namespaces into the codebases in this stage of the project is not adequate. This can wait until after 0.47.
Adib. ---- PS Maximilian: Ich freu' mich, wieder was von dir zu hören :-)
On 8/6/09, Maximilian Albert <maximilian.albert@...1439...> wrote:
2009/8/6 Jon A. Cruz <jon@...18...>:
Ah, seems quite simple. Here is the root problem:
r21994 | johnce | 2009-08-05 11:32:38 -0700 (Wed, 05 Aug 2009) | 1 line
xml/Document -> DocumentTree
It appears that someone renamed the struct in the .h but did not rename it in forward.h, and did not change the usages of it.
I spent the better part of four hours trying to fix this (because each change broke things at other places) and finally ended up with the discovery that there are some #includes in file.(h|cpp) (namely, streams-webdav.h and friends) for which the corresponding files are missing. Thus I decided to revert the recent set of changes by johnce, also because I'm not sure it's a good idea to have such huge commits so close to the release. If I overlooked something and there is an easy fix please feel free to undo my revert.
BTW, during my fixing attempts I partially had to revert changes made by johnce because Document (formerly known as SPDocument) and Inkscape::XML::DocumentTree (formerly known as Inkscape::XML::Document) got mixed up in his latest commits. So it seems to me that there may be quite a few places where bugs might be introduced by these commits. I don't know what others think about it, but IMHO it would be better to hold them back until 0.47 is out. Don't get me wrong, I believe that the changes are valuable and the renaming can eliminate potential sources of confusion, but it's just too dangerous to apply them now. Opinions?
Max
@johnce: Please always make sure to only commit changes which don't break the build! I had planned to work on some other things tonight but essentially spent the whole evening trying to fix compiling so my pending work will have to wait until I can spare some more time (which may only be in a couple of days if I'm unlucky).
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
2009/8/6 ~suv <suv-sf@...58...>:
On 6/8/09 16:18, Maximilian Albert wrote:
I spent the better part of four hours trying to fix this (because each change broke things at other places) and finally ended up with the discovery that there are some #includes in file.(h|cpp) (namely, streams-webdav.h and friends) for which the corresponding files are missing.
bug #258418 “webdav/http/ftp uri patch” https://bugs.launchpad.net/inkscape/+bug/258418 ?
Ah, okay. I hadn't noticed this patch, thanks for pointing me there. I won't apply it, though, since I don't have any expertise in this area and cannot review it.
Still, even if johnce's recent commits can be made to compile with this patch the question remains: should these changes be applied now or should they be helf off till after 0.47 was released? As I said before, I vote for the latter but of course I don't have the definitive say in this.
Max
On Thu, 2009-08-06 at 23:18 +0900, Maximilian Albert wrote:
I spent the better part of four hours trying to fix this (because each change broke things at other places) and finally ended up with the discovery that there are some #includes in file.(h|cpp) (namely, streams-webdav.h and friends) for which the corresponding files are missing. Thus I decided to revert the recent set of changes by johnce, also because I'm not sure it's a good idea to have such huge commits so close to the release. If I overlooked something and there is an easy fix please feel free to undo my revert.
Thanks for getting the build working again!!
Tav
On Fri, 2009-08-07 at 14:55 +0900, Maximilian Albert wrote:
Still, even if johnce's recent commits can be made to compile with this patch the question remains: should these changes be applied now or should they be helf off till after 0.47 was released? As I said before, I vote for the latter but of course I don't have the definitive say in this.
Luckily, I do have the definitive say for this dev cycle. ;)
Yes, it does need to be held off until after 0.47 is out the door. Quite simply, the changes are too deep and intrusive to introduce so late in the cycle.
Max, thank you for getting the cleanup taken care of.
Both Aaron and I have made attempts to get in contact with johnce informing him of the status of the dev-cycle. Once we hear back, I'm sure we'll get confirmation that we will see the change re-committed soon after trunk opens to general development again (and will have been tested to not break the build). :)
Cheers, Josh
Maximilian Albert-2 wrote:
Ah, okay. I hadn't noticed this patch, thanks for pointing me there. I won't apply it, though, since I don't have any expertise in this area and cannot review it.
I had a look at the patch, and I think we should NOT commit it even after the release:
1. It introduces an additional runtime dependency (libneon); moreover this additional dependency is completely redundant, because accessing FTP, HTTP and WebDAV can be done in a more high level way using GIO which is already in our deps (as part of Glib), and we do not need to care about a lot of boring things like asking for login credentials.
2. It tries to access remote locations using derived versions of standard C++ library stream buffers with a 4KB buffer size, I don't think this is a good idea :/ Moreover very few people know how to properly subclass an std::streambuf.
3. Uses non-portable code in streams-handles.cpp: this file includes unistd.h, sys/socket.h, sys/types.h and netinet/in.h; some of those are not available on Windows, or include different functions.
4. Not directly related to the patch, but if the author wants to rename Inkscape::XML::Document to Inkscape::XML::DocumentTree because he gets conflicts with the SPDocument he renamed to Document in the global namespace, then he has no idea what he's doing :( The change from Inkscape::XML::Document to Inkscape::XML::DocumentTree is counter-productive because that object represents an XML document, not some 'document tree'.
Regards, Krzysztof
On Aug 10, 2009, at 3:59 PM, Krzysztof Kosiński wrote:
Maximilian Albert-2 wrote:
Ah, okay. I hadn't noticed this patch, thanks for pointing me there. I won't apply it, though, since I don't have any expertise in this area and cannot review it.
I had a look at the patch, and I think we should NOT commit it even after the release:
- It introduces an additional runtime dependency
[SNIP]
Good assessment overall. Covers what I was starting to see there.
participants (8)
-
Alexandre Prokoudine -
Jon A. Cruz -
Joshua A. Andler -
Krzysztof Kosiński -
Maximilian Albert -
Tavmjong Bah -
the Adib -
~suv