Author Topic: Code::Blocks, Scintilla and Fedora (Read 41514 times)

Folco · « **on:** October 22, 2010, 06:03:16 pm »

yop,

A "bug" has been reported to Fedora : https://bugzilla.redhat.com/show_bug.cgi?id=644183
If you don't care about the distros which provide Code::Blocks packages, this is not important at all.

But Fedora will remove Code::Blocks from its repositories if they can't use their system Scintilla (not the bundled one).
This is the politic of Fedora : no soft may bundle its libraries, it MUST use shared libs.

So I don't know if there is a lot of changes in your version of (wx)Scintilla. Is it still important to keep a bundle of that libs ?

But perhaps this it not important at all for you... ?

oBFusCATed · « **Reply #1 on:** October 22, 2010, 06:41:15 pm »

Scintilla probably could be unbundled, but wxScintilla can't.
I think they would agree to leave it bundled.

Jenna · « **Reply #2 on:** October 24, 2010, 03:06:01 pm »

We use a patched Scintilla to fit the needs of Code::Blocks, so a plain vanilla scintilla will not work.

Biplab · « **Reply #3 on:** October 24, 2010, 03:18:49 pm »

Quote from: Folco on October 22, 2010, 06:03:16 pm

But Fedora will remove Code::Blocks from its repositories if they can't use their system Scintilla (not the bundled one).
This is the politic of Fedora : no soft may bundle its libraries, it MUST use shared libs.

Another possible solution is we build wxscintilla as a static lib and link other *.so against it. This would increase the binary size. However we won't be violating Fedora rules. However I'm not sure of any immediate side effects.

Other devs- what do you think about it?

Jenna · « **Reply #4 on:** October 24, 2010, 03:28:18 pm »

Quote from: Biplab on October 24, 2010, 03:18:49 pm

Other devs- what do you think about it?

This should work, but I'm not sure if it would be enough for fedora.
If they just don't want shared libraries twice in the system, it will most likely be okay, but it can also be a problem with our own scintilla sources (harder to maintain in case of security problems or other issues).

MortenMacFly · « **Reply #5 on:** October 24, 2010, 08:32:38 pm »

Quote from: jens on October 24, 2010, 03:28:18 pm

but it can also be a problem with our own scintilla sources (harder to maintain in case of security problems or other issues).

What do you mean by that? I didn't get it... we are already maintaining this ourselves, so what would be the difference?!

And yes: I agree with Biplab. But only if we believe Fedora is that important. I am not so much into Linux as you know so I cannot make such a decision. As long as Ubuntu is not based on Fedora it would be no problem for me (and only me).

Jenna · « **Reply #6 on:** October 24, 2010, 08:49:30 pm »

I don't mean it's a problem for us, but probably for the fedora maintainers, because having two different sources (one for the official shared library and one inside C::B's sources) can be a problem.

The package maintainers normally have to react if possible security risks (or other system-relevant issues) are discovered, and they normally do not wait until upstream fixes the problem, they have to fix this immediately and that can be a problem for the person who maintains the C::B package for fedora.

But maybe they say, unless it's restricted to C::B, because of using a statically linked lib, it's no problem.

oBFusCATed · « **Reply #7 on:** October 24, 2010, 10:22:02 pm »

Jen, does Scintilla (not wxScintilla) has changes in svn?

I think that the lib will be marked bundled no matter if it is static or dynamic.

Some info why packagers hate bundled libs (gentoo related):

http://blog.flameeyes.eu/2009/01/02/bundling-libraries-for-despair-and-insecurity
http://blog.flameeyes.eu/2010/04/17/a-visible-case-against-bundled-libraries

Jenna · « **Reply #8 on:** October 24, 2010, 10:43:13 pm »

Quote from: oBFusCATed on October 24, 2010, 10:22:02 pm

Jen, does Scintilla (not wxScintilla) has changes in svn?

We use patched scintilla sources for several reasons.
If you search the wxscintilla folder in our svn-sources for C::B begin, you will find many changes, not only in wxScintilla and I think it's not possible to move all these changes into wxscintilla.
Many of the changes do not make sense in scintilla upstream, or will not be accepted (as the changebar-code).
So we will either be forced to use "our" scintilla or lose a lot of (basically) functionality, if we would switch to plain scintilla.

MortenMacFly · « **Reply #9 on:** October 25, 2010, 07:23:14 am »

Quote from: jens on October 24, 2010, 10:43:13 pm

So we will either be forced to use "our" scintilla or lose a lot of (basically) functionality, if we would switch to plain scintilla.

I think we CAN go with a bundled version of scintilla. Basically we have forked both: wxScintilla and scintilla for our purposes. It makes no sense to me if due to Fedora "laws" forks would become basically forbidden. Especially in the case of wxScintilla which is no longer supported for ages meanwhile and where we wouldn't even have another option. To me the only solution is either a statically binding or forbid forks of software components which is the end of development. Period.

SharkCZ · « **Reply #10 on:** October 25, 2010, 09:03:47 am »

Fedora won't remove C::B if it uses a bundled copy of Scintilla/wxScintilla, but there are some rules that packages must follow - https://fedoraproject.org/wiki/Packaging/Guidelines#Duplication_of_system_libraries and https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

If I understand correctly then C::B uses a modified versions of both, effectively using a fork. That's naturally not forbidden (and a reality in many other situations), but can be a bit unfortunate for the whole open-source world. So my question is - can the changes that you made be merged with the original libraries? And also - what is the canonical source for the libraries? Is it the wxWidgets project for wxScintilla (containing a copy of Scintilla)?

MortenMacFly · « **Reply #11 on:** October 25, 2010, 10:00:59 am »

Quote from: SharkCZ on October 25, 2010, 09:03:47 am

So my question is - can the changes that you made be merged with the original libraries? And also - what is the canonical source for the libraries? Is it the wxWidgets project for wxScintilla (containing a copy of Scintilla)?

Well - for wxScintilla - this project is dead as the maintainer does not develop it actively anymore. So naturally there are a lot forks meanwhile to my knowledge, one in wxWidgets itself, one through wxCode and another one in C::B, even CodeLite has one.

For scintilla: Some of our modifications are e.g. based on a patch that was rejected by scintilla maintainers. So we need to stay with the fork. However, we maintain up-stream compatibility by using a quiet up-to-date version on scintilla, usually and marking every change done by C::B with a special tag in the source code. I'm afraid that's all we can do ATM.

Biplab · « **Reply #12 on:** October 25, 2010, 04:21:20 pm »

I didn't get time to go through the issue. So I can't comment why they don't want to allow us to bundle patched source. But I'd like to mention few points-

1) Fedora is one of the popular Linux Distros. We should try our best to keep C::B listed in Fedora.
2) I read the bug report. Mozilla chardet is also mentioned there. AFAIK, it's not patched heavily.
3) Another possible solution (also mentioned in bug report) we switch to wxScintilla bundled with wx.

We can do item 3 only when we switch to wx-2.9 (pls correct me if I'm wrong) which seems a long way to go.

Jenna · « **Reply #13 on:** October 25, 2010, 04:39:21 pm »

Quote from: Biplab on October 25, 2010, 04:21:20 pm

I didn't get time to go through the issue. So I can't comment why they don't want to allow us to bundle patched source. But I'd like to mention few points-

1) Fedora is one of the popular Linux Distros. We should try our best to keep C::B listed in Fedora.
2) I read the bug report. Mozilla chardet is also mentioned there. AFAIK, it's not patched heavily.
3) Another possible solution (also mentioned in bug report) we switch to wxScintilla bundled with wx.

We can do item 3 only when we switch to wx-2.9 (pls correct me if I'm wrong) which seems a long way to go.

Item 3 will break many of our features, because we use patched scintilla and wxscintilla.
Mozilla chardet is not available as library as far as I know and can not be a problem therefore.

Biplab · « **Reply #14 on:** October 25, 2010, 05:22:58 pm »

Quote from: jens on October 25, 2010, 04:39:21 pm

Mozilla chardet is not available as library as far as I know and can not be a problem therefore.

You are right.

SharkCZ · « **Reply #15 on:** October 26, 2010, 12:02:45 pm »

Quote from: jens on October 25, 2010, 04:39:21 pm

Quote from: Biplab on October 25, 2010, 04:21:20 pm
I didn't get time to go through the issue. So I can't comment why they don't want to allow us to bundle patched source. But I'd like to mention few points-

1) Fedora is one of the popular Linux Distros. We should try our best to keep C::B listed in Fedora.
2) I read the bug report. Mozilla chardet is also mentioned there. AFAIK, it's not patched heavily.
3) Another possible solution (also mentioned in bug report) we switch to wxScintilla bundled with wx.

We can do item 3 only when we switch to wx-2.9 (pls correct me if I'm wrong) which seems a long way to go.
Item 3 will break many of our features, because we use patched scintilla and wxscintilla.

I've checked the diff between upstream scintilla and the one used in C::B and there are changes that could go upstream in my opinion (still depends on upstream to accept them) and changes that are C::B specific (like some wxSmith support). It means that C::B uses effectively a fork of scintilla and we must live with that.

The situation with wxScintilla can change when an active developer will appear, but it won't help much here.

Quote

Mozilla chardet is not available as library as far as I know and can not be a problem therefore.

yes, that's no problem

MortenMacFly · « **Reply #16 on:** October 26, 2010, 09:26:24 pm »

Quote from: SharkCZ on October 26, 2010, 12:02:45 pm

The situation with wxScintilla can change when an active developer will appear, but it won't help much here.

I don't think this will happen. There is the STC component in wxWidgets base, basically a replacement for wxScintilla, hence no good for us. The only think I can imagine is taking this as code base and making out patches against that component. But that won't happen in the near future.

Loaden · « **Reply #17 on:** November 08, 2010, 10:27:06 am »

Quote from: Biplab on October 24, 2010, 03:18:49 pm

Another possible solution is we build wxscintilla as a static lib and link other *.so against it.

Agreed. I think that's the best way.
But it seems have some issue when build wxScintilla as a static lib.
Error log here:

Code

||=== Code::Blocks, sdk ===|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|760|undefined reference to `_imp___ZN11wxScintilla7SetZoomEi'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|765|undefined reference to `_imp___ZN11wxScintilla13SetMarginMaskEii'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|766|undefined reference to `_imp___ZN11wxScintilla13SetMarginMaskEii'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|767|undefined reference to `_imp___ZN11wxScintilla13SetMarginMaskEii'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|768|undefined reference to `_imp___ZN11wxScintilla13SetMarginMaskEii'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|828|undefined reference to `_imp___ZNK11wxScintilla9GetModifyEv'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|838|undefined reference to `_imp___ZN11wxScintilla12SetSavePointEv'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|844|undefined reference to `_imp___ZNK11wxScintilla11GetReadOnlyEv'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|875|undefined reference to `_imp___ZN11wxScintilla7GotoPosEi'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|876|undefined reference to `_imp___ZN11wxScintilla12ScrollToLineEi'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|877|undefined reference to `_imp___ZN11wxScintilla14ScrollToColumnEi'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|910|undefined reference to `_imp___ZNK11wxScintilla13GetCurrentPosEv'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|911|undefined reference to `_imp___ZNK11wxScintilla19GetFirstVisibleLineEv'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|918|undefined reference to `_imp___ZN11wxScintilla12MarkerDefineEiiRK8wxColourS2_'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|919|undefined reference to `_imp___ZN11wxScintilla19MarkerSetForegroundEiRK8wxColour'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|920|undefined reference to `_imp___ZN11wxScintilla19MarkerSetBackgroundEiRK8wxColour'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|924|undefined reference to `_imp___ZN11wxScintilla12MarkerDefineEiiRK8wxColourS2_'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|925|undefined reference to `_imp___ZN11wxScintilla19MarkerSetForegroundEiRK8wxColour'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|926|undefined reference to `_imp___ZN11wxScintilla19MarkerSetBackgroundEiRK8wxColour'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|932|undefined reference to `_imp___ZN11wxScintilla12SetFoldFlagsEi'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|934|undefined reference to `_imp___ZN11wxScintilla12SetFoldFlagsEi'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|947|undefined reference to `_imp___ZN11wxScintilla8UsePopUpEb'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|955|undefined reference to `_imp__wxEVT_SCI_MARGINCLICK'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|958|undefined reference to `_imp__wxEVT_SCI_UPDATEUI'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|961|undefined reference to `_imp__wxEVT_SCI_CHANGE'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|964|undefined reference to `_imp__wxEVT_SCI_CHARADDED'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|967|undefined reference to `_imp__wxEVT_SCI_DWELLSTART'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|970|undefined reference to `_imp__wxEVT_SCI_DWELLEND'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|973|undefined reference to `_imp__wxEVT_SCI_USERLISTSELECTION'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|976|undefined reference to `_imp__wxEVT_SCI_MODIFIED'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_STYLENEEDED'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_SAVEPOINTREACHED'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_SAVEPOINTLEFT'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_ROMODIFYATTEMPT'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_KEY'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_DOUBLECLICK'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_MACRORECORD'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_NEEDSHOWN'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_PAINTED'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_URIDROPPED'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_START_DRAG'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_DRAG_OVER'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_DO_DROP'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_ZOOM'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_HOTSPOT_CLICK'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_HOTSPOT_DCLICK'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_CALLTIP_CLICK'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_AUTOCOMP_SELECTION'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_TAB'|
.objs\sdk\cbeditor.o:D:\DengYC\CodeBlocks-C\src\sdk\cbeditor.cpp|1019|undefined reference to `_imp__wxEVT_SCI_ESC'|
||More errors follow but not being shown.|
||Edit the max errors limit in compiler options...|
||=== Build finished: 50 errors, 0 warnings (0 minutes, 19 seconds) ===|

Any comments are welcome!

Jenna · « **Reply #18 on:** November 08, 2010, 10:45:17 am »

@Loaden:
Did you remove the devel and output folder before building or did you make rebuild (clean and build) ?
If that's not enough remove the pch's manually.

You should also remove all occurrences of scintilla in the link library list, where a target is linked against libcodeblocks, to avoid double linking and possible crashes (we had this in wxSmith with libtxml).
I attach a patch for C::B core that works on linux.

Loaden · « **Reply #19 on:** November 08, 2010, 10:55:23 am »

Quote from: jens on November 08, 2010, 10:45:17 am

@Loaden:
Did you remove the devel and output folder before building or did you make rebuild (clean and build) ?
If that's not enough remove the pch's manually.

You should also remove all occurrences of scintilla in the link library list, where a target is linked against libcodeblocks, to avoid double linking and possible crashes (we had this in wxSmith with libtxml).
I attach a patch for C::B core that works on linux.

Why not need remove "WXMAKINGDLL_SCI" macro?

Loaden · « **Reply #20 on:** November 08, 2010, 11:01:11 am »

I still can't build success if keep the "WXMAKINGDLL_SCI" macro defined.

Loaden · « **Reply #21 on:** November 08, 2010, 11:08:08 am »

This is my second trying, the error is as follows.

Code

MORE-WARNING-...
----
D:\DengYC\CodeBlocks-C\src\sdk\wxscintilla\src\wxscintilla.cpp|4990|warning: 'bool wxScintillaEvent::GetControl() const' redeclared without dllimport attribute: previous dllimport ignored|
D:\DengYC\CodeBlocks-C\src\sdk\wxscintilla\src\wxscintilla.cpp|4991|warning: 'bool wxScintillaEvent::GetAlt() const' redeclared without dllimport attribute: previous dllimport ignored|
D:\DengYC\CodeBlocks-C\src\sdk\wxscintilla\src\wxscintilla.cpp|4994|warning: 'wxScintillaEvent::wxScintillaEvent(const wxScintillaEvent&)' redeclared without dllimport attribute after being referenced with dll linkage|
D:\DengYC\CodeBlocks-C\src\sdk\wxscintilla\src\wxscintilla.cpp|122|error: definition of static data member 'wxScintilla::sm_eventTable' of dllimport'd class|
D:\DengYC\CodeBlocks-C\src\sdk\wxscintilla\src\wxscintilla.cpp|122|error: definition of static data member 'wxScintilla::sm_eventHashTable' of dllimport'd class|
D:\DengYC\CodeBlocks-C\src\sdk\wxscintilla\src\wxscintilla.cpp|150|error: definition of static data member 'wxScintilla::ms_classInfo' of dllimport'd class|
D:\DengYC\CodeBlocks-C\src\sdk\wxscintilla\src\wxscintilla.cpp|151|error: definition of static data member 'wxScintillaEvent::ms_classInfo' of dllimport'd class|
||=== Build finished: 4 errors, 685 warnings (0 minutes, 22 seconds) ===|

MortenMacFly · « **Reply #22 on:** November 08, 2010, 11:30:21 am »

Quote from: Loaden on November 08, 2010, 11:01:11 am

I still can't build success if keep the "WXMAKINGDLL_SCI" macro defined.

Probably I am missing something here, but why would you do that if you don't weant to build the DLL?

@Jens,Loaden: please keep in mind not to touch the wxscintilla component too much - I've have quite a lot changes pending from my side, including an update of scintilla. So please don't cause too much conflicts (project files are OK of course).

Loaden · « **Reply #23 on:** November 08, 2010, 11:44:10 am »

Quote from: MortenMacFly on November 08, 2010, 11:30:21 am

Quote from: Loaden on November 08, 2010, 11:01:11 am
I still can't build success if keep the "WXMAKINGDLL_SCI" macro defined.
Probably I am missing something here, but why would you do that if you don't weant to build the DLL?

@Jens,Loaden: please keep in mind not to touch the wxscintilla component too much - I've have quite a lot changes pending from my side, including an update of scintilla. So please don't cause too much conflicts (project files are OK of course).

I have the same question, see #19.

Because the patch from Jens that keep this macro, and build success on Linux, see #18.

Loaden · « **Reply #24 on:** November 08, 2010, 11:50:40 am »

Quote from: MortenMacFly on November 08, 2010, 11:30:21 am

@Jens,Loaden: please keep in mind not to touch the wxscintilla component too much - I've have quite a lot changes pending from my side, including an update of scintilla. So please don't cause too much conflicts (project files are OK of course).

Copy that.
Have another question, why we need hack this code from:

Code

SCNotification scn = {0};

to:

Code

/* C::B begin */
	SCNotification scn; memset((void*)&scn, 0, sizeof(scn));
/* C::B end */

:?

Jenna · « **Reply #25 on:** November 08, 2010, 12:21:17 pm »

Quote from: Loaden on November 08, 2010, 11:50:40 am

Quote from: MortenMacFly on November 08, 2010, 11:30:21 am
@Jens,Loaden: please keep in mind not to touch the wxscintilla component too much - I've have quite a lot changes pending from my side, including an update of scintilla. So please don't cause too much conflicts (project files are OK of course).
Copy that.
Have another question, why we need hack this code from:
Code
SCNotification scn = {0};
to:
Code
/* C::B begin */
	SCNotification scn; memset((void*)&scn, 0, sizeof(scn));
/* C::B end */
:?

To avoid build-warnings.

Can you test my new patch, works for me on XP SP3, wx2.8.10 and TDM gcc 4.5.0 (just for the core project-file at the moment).

MortenMacFly · « **Reply #26 on:** November 08, 2010, 02:11:17 pm »

Quote from: Loaden on November 08, 2010, 11:50:40 am

Have another question, why we need hack this code from:
Code
SCNotification scn = {0};
to:
Code
/* C::B begin */
	SCNotification scn; memset((void*)&scn, 0, sizeof(scn));
/* C::B end */
:?

Because it is technically more correct. This is a structure and if you only set the first value to zero, the other values may still be invalid on initialisation (platform / compiler dependent). Why are you asking? Is this causing trouble?

oBFusCATed · « **Reply #27 on:** November 08, 2010, 02:57:13 pm »

Quote from: MortenMacFly on November 08, 2010, 02:11:17 pm

...This is a structure and if you only set the first value to zero, the other values may still be invalid on initialisation (platform / compiler dependent).

Hm, C++ standard says that the unspecified members are initialized with zero (If I remember correctly).
Which compiler fails to compile this code (={0}) correctly?

Also memset should be used on POD only structs, probably =0 will error if used with non-POD struct.

Loaden · « **Reply #28 on:** November 08, 2010, 03:37:55 pm »

Quote from: oBFusCATed on November 08, 2010, 02:57:13 pm

Quote from: MortenMacFly on November 08, 2010, 02:11:17 pm
...This is a structure and if you only set the first value to zero, the other values may still be invalid on initialisation (platform / compiler dependent).
Hm, C++ standard says that the unspecified members are initialized with zero (If I remember correctly).

I can sure it, so, I think it maybe not needed to hack.

Jenna · « **Reply #29 on:** November 08, 2010, 04:34:13 pm »

Quote from: Loaden on November 08, 2010, 03:37:55 pm

Quote from: oBFusCATed on November 08, 2010, 02:57:13 pm
Quote from: MortenMacFly on November 08, 2010, 02:11:17 pm
...This is a structure and if you only set the first value to zero, the other values may still be invalid on initialisation (platform / compiler dependent).
Hm, C++ standard says that the unspecified members are initialized with zero (If I remember correctly).
I can sure it, so, I think it maybe not needed to hack.

But it should be

Code

SCNotification scn = {{0}};

Loaden · « **Reply #30 on:** November 08, 2010, 04:42:20 pm »

Quote from: jens on November 08, 2010, 12:21:17 pm

Can you test my new patch, works for me on XP SP3, wx2.8.10 and TDM gcc 4.5.0 (just for the core project-file at the moment).

Well done! testing passed.
And have a issue, I think we need change the lib name from "libwxscintilla.a" to "libcbscintilla.a" or "libwxscintilla_cb.a".
Because in wx2.9.x, the wxscintilla library named "libwxscintilla.a" too.

Loaden · « **Reply #31 on:** November 08, 2010, 05:01:57 pm »

Quote from: jens on November 08, 2010, 04:34:13 pm

Quote from: Loaden on November 08, 2010, 03:37:55 pm
Quote from: oBFusCATed on November 08, 2010, 02:57:13 pm
Quote from: MortenMacFly on November 08, 2010, 02:11:17 pm
...This is a structure and if you only set the first value to zero, the other values may still be invalid on initialisation (platform / compiler dependent).
Hm, C++ standard says that the unspecified members are initialized with zero (If I remember correctly).
I can sure it, so, I think it maybe not needed to hack.
But it should be
Code
SCNotification scn = {{0}};

No, you can use just {0}.
See:

Code

#include <iostream>

using namespace std;

struct A
{
    int i;
    float j;
    char c;
    int* p;
};

int main()
{
    A a = {0};
    cout << a.i << "," << a.j << "," << a.c << "," << a.p << endl;
    return 0;
}

Jenna · « **Reply #32 on:** November 08, 2010, 05:29:17 pm »

It's not correct in our case, because the first element of the struct is a struct, that needs to be initialised with {0} itself.
All other elements are set to zero (or their default values ?) automagically.

Code

#include <iostream>

using namespace std;

struct B
{
    int j;
};

struct A
{
    struct B b;
    int i;
    float j;
    char c;
    int* p;
};

int main()
{
    A a = {0};
    cout << a.b.j << "," << a.i << "," << a.j << "," << a.c << "," << a.p << endl;
    return 0;
}

leads to warning:

Code

main.cpp:21:13: warning: missing braces around initializer for ‘B’

even if it works.

MortenMacFly · « **Reply #33 on:** November 08, 2010, 08:13:01 pm »

Quote from: jens on November 08, 2010, 05:29:17 pm

It's not correct in our case, because the first element of the struct is a struct, that needs to be initialised with {0} itself.
All other elements are set to zero (or their default values ?) automagically.

That's what I meant: For me it is not really clear what is fully correct. Even if it may work due to the standard (what does the standard say about initialising structs within structs btw?!) as it is now - it'll definitely work reliable.

So again my question: What's the issue with it?

Loaden · « **Reply #34 on:** November 09, 2010, 12:53:18 am »

Quote from: jens on November 08, 2010, 05:29:17 pm

It's not correct in our case, because the first element of the struct is a struct, that needs to be initialised with {0} itself.
All other elements are set to zero (or their default values ?) automagically.

leads to warning:
Code
main.cpp:21:13: warning: missing braces around initializer for ‘B’
even if it works.

In VC++2010, their have no warning when use {0} to initialize a struct member variable.
So, We can avoid this warning in project settings.
This is C++ standard behavior. So, don't worry, all the member will initialize to zero.
Include struct member value.

Loaden · « **Reply #35 on:** November 09, 2010, 12:55:02 am »

Quote from: MortenMacFly on November 08, 2010, 08:13:01 pm

So again my question: What's the issue with it?

This will increase the difficulty of maintenance.
I think we should change the scintilla/wxScintilla as little as possible.

Loaden · « **Reply #36 on:** November 09, 2010, 01:24:05 am »

Quote from: Loaden on November 08, 2010, 04:42:20 pm

Quote from: jens on November 08, 2010, 12:21:17 pm
Can you test my new patch, works for me on XP SP3, wx2.8.10 and TDM gcc 4.5.0 (just for the core project-file at the moment).
Well done! testing passed.
And have a issue, I think we need change the lib name from "libwxscintilla.a" to "libcbscintilla.a" or "libwxscintilla_cb.a".
Because in wx2.9.x, the wxscintilla library named "libwxscintilla.a" too.

Any comments about the library name?

BTW, I am testting in XPSP3, if build wxScintilla as static library, the core (not include contrib) result is : 16.5MB.
And, If build wxScintilla as shared library, the core reulst is 16.5 MB too.
In fact, only codeblocks.dll size have changed.

Code

DLL Name		static build		shared build
codeblocks.dll		4.76MB			3.62MB

Loaden · « **Reply #37 on:** November 09, 2010, 02:33:33 am »

Here is a patch for wx2.9.2, build wxScintilla as static library too. :lol:

Biplab · « **Reply #38 on:** November 09, 2010, 03:37:26 am »

If a struct is inside struct and we want to initialize it to zero we need to enclose it with { } Thus gcc was throwing warning on the original code.

So correct form should be -

Code

struct foo f = { {0}, 0};

M$VC not being standard compliant can ignore such code.

MortenMacFly · « **Reply #39 on:** November 09, 2010, 09:30:27 am »

Quote from: Biplab on November 09, 2010, 03:37:26 am

Code

struct foo f = { {0}, 0};

That sounds reasonable to me. If that removes the error and is standard compliant we can go for it. However, we would still have these portions in the code patched than... ;-)

oBFusCATed · « **Reply #40 on:** November 09, 2010, 09:51:44 am »

Quote from: Biplab on November 09, 2010, 03:37:26 am

M$VC not being standard compliant can ignore such code.

You're wrong here

This warning is specific to GCC, here is an explanation: http://www.mail-archive.com/gcc@gcc.gnu.org/msg47795.html

Biplab · « **Reply #41 on:** November 09, 2010, 10:33:34 am »

Quote from: MortenMacFly on November 09, 2010, 09:30:27 am

Quote from: Biplab on November 09, 2010, 03:37:26 am
Code
struct foo f = { {0}, 0};
That sounds reasonable to me. If that removes the error and is standard compliant we can go for it. However, we would still have these portions in the code patched than... ;-)

Agreed.

Quote from: oBFusCATed on November 09, 2010, 09:51:44 am

Quote from: Biplab on November 09, 2010, 03:37:26 am
M$VC not being standard compliant can ignore such code.
You're wrong here

This warning is specific to GCC, here is an explanation: http://www.mail-archive.com/gcc@gcc.gnu.org/msg47795.html

I can't go through the link at the moment. But if I understand clause 8.5.1.8 of N1905-05-0165 correctly draft standard shows how it should be initialized.

I agree that I am not be correct in saying M$VC compiler is not standard compliant when the standard is a draft one.

News:

Author Topic: Code::Blocks, Scintilla and Fedora (Read 41514 times)