Code::Blocks Forums

User forums => Nightly builds => Topic started by: killerbot on May 10, 2007, 10:32:58 pm

Title: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: killerbot on May 10, 2007, 10:32:58 pm
because once again the linux build is broken.


Could things please be corrected (I have done it once, but things got changed again ...) :

1) keybinder.cpp is using the following stuff :
Code
Manager::Get()->GetMessageManager()->DebugLog(wxString::Format(wxT("KeyBinder failed UpdateById on[%d][%s]"), nMenuItemID, pMenuItem->GetText().GetData()));

that means that the following includes are needed (don't just think fucking pch) :
Code
#include "manager.h"  	 
#include "messagemanager.h"
 

And : they belong here and not in the corresponding header file !!!

2) And let the error speak for itself :
Code
cbkeybinder.cpp: In member function 'void cbKeyBinder::OnAppStartShutdown(wxCommandEvent&)':
cbkeybinder.cpp:1086: error: 'Sleep' was not declared in this scope
make[4]: *** [cbkeybinder.lo] Error 1

Please understand my frustration.
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: Grom on May 10, 2007, 10:41:11 pm
write your own h file  and contribute it to wx :lol:
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: stahta01 on May 11, 2007, 12:24:47 am
FYI:

I created thread "Quick and Dirty way to compile CB using wxMSW without using PCH"
http://forums.codeblocks.org/index.php/topic,5881.msg45006.html#msg45006

I plan to double check my patches and upload them to BerliOS in a few hours.

Tim S
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: Pecan on May 11, 2007, 01:01:10 am
Well, I think pch is getting blamed more than it deserves.

The mistake was forgetting to copy the code back from Linux to Windows before commiting.

I was surprised to see the errs. I created and fixed those around 5 am this morning.


This is all confusing to me. On the one hand we have a core developer removing preprocessor flags so that pch always works, and on the other hand we have another developer yelling at us NOT to use pch at all.

geesh

I wouldn't mind relegating pch to the rubish pile anyway. It's bitten too many times.

Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: stahta01 on May 11, 2007, 01:29:05 am
Well, I think pch is getting blamed more than it deserves.

The mistake was forgetting to copy the code back from Linux to Windows before commiting.

I was surprised to see the errs. I created and fixed those around 5 am this morning.


This is all confusing to me. On the one hand we have a core developer removing preprocessor flags so that pch always works, and on the other hand we have another developer yelling at us NOT to use pch at all.

geesh

I wouldn't mind relegating pch to the rubish pile anyway. It's bitten too many times.



@Pecan:

[ Patch #1993 ] Enable Keybinder build using wxMSW without PCH
https://developer.berlios.de/patch/index.php?func=detailpatch&patch_id=1993&group_id=5358

Tim S
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: byo on May 11, 2007, 03:27:50 am
Hmm, I was thinking lately about writing some tool that would add such missing headers automatically.

It's really easy task. It would base on fact that certain identifiers used in sources require certain header files. So when there's for example GetMessageManager or DebugLog used anywhere, it would notify that messagemanager.h is needed. After generating list of headers it could scan for all #include directives and add missing ones. Of course it would be not an universal tool but could work perfectly in case of C::B stuff (for both missing sdk and wx headers).

I think no more than few hours to make core tool. Maybe some more time to create Ident -> header bindings., but that doesn't have to be complite from the very beginning. What do you think about that?

I could sacrifice one night so Killerbot would be able to sleep well all the time without having nightmares about missing headers 8)

Regards
   BYO
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: killerbot on May 11, 2007, 08:00:35 am
well I don't say don't use pch. They have some advantage, they can speed up compilation, certainly when one is using a 'stable' sdk. The sdk of CB is not stable yet so because of that sometimes pch make the compilation longer, because there's a very high coupling of unrelated things in that pch file. On the other hand it introduces broken builds way to many, why because one adds an extra statement with whatever objects or types, hit compile .... it compiles. Cool, yes, but it compiled because most of the developers are using the pch build system or they environment automatically does the pch way, but then when building on a non pch build environment, the build is broken because the correct headers were not added for those just added statements.

My personal feeling is that pch makes programmers a bit lazy and the programmers forget to think about how headers actually (should) work. How would I say it, it gives a sense of false safety.

The basic line is : use headers correctly --> then it builds for non pch, and if you also want to support pch well then you add some special things to your file.
I once have written an article on correct use, I will review it once more tjhis weekend and publish it, I think it will make a whole bunch of things more clear.

Synopsis : pch are not bad, they need to be used correctly in ADDITION to the normal way of header inclusion, so one gets correct and portable code. ;-)
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: MortenMacFly on May 11, 2007, 08:22:40 am
My personal feeling is that pch makes programmers a bit lazy and the programmers forget to think about how headers actually (should) work. How would I say it, it gives a sense of false safety.
I've another solution besides Byo's beautiful idea (which I think is great):
I'm about to submit an update to the envvars plugin where I have simply added another target "no_pch". In there I do *not* define the *PRECOMP stuff but simply define NOPCH. Thus if this target builds it's most likely that it will immediately work on other machines without PCH (Linux), too. I've detected quite some PCH issues using this method. Once I have submitted you can have a look at it... 8)

With regards, Morten.
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: byo on May 12, 2007, 08:21:06 pm
Ok, as promised, tool that can help fixing missing headers. Sources are in attachment (project file may still need some tuning since I've not updated it to use $(WX_SUFFIX) etc. stuff).

I've added most of identifiers from C::B so it should work in most critical situations.

The plugin itself is very "dumb". For example, when processing any file, it won't process included files and check what stuff they include. So generally, when it process cpp file and some headers are in corresponsing h file, all those headers will probably be added into cpp.

As a result, the tool adds missing includes at the beginning of file, before any previous content (that was easiest to implement ;) ) surrounding it with // *** ADDED BY HEADER FIXUP ***  and // *** END *** comments so they can be quickly found by Search In Files.

I have tested it on wxSmith and it looks like it's working, but of courseback-up your files before using it ;).

BTW. I don't provice cbplugin file because there's some binary incompatibility between my wx and the one used in nightlies.

BYO

[attachment deleted by admin]
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: MortenMacFly on May 14, 2007, 06:01:39 pm
Thanks Byo, as we have seen here: http://forums.codeblocks.org/index.php/topic,5903.msg45142.html#msg45142 (http://forums.codeblocks.org/index.php/topic,5903.msg45142.html#msg45142) my attempt was far from being perfect anyway... :cry:
With regards, Morten.

Edit: Wohooo! Looks great! Have tested in on some of the plugins and (in fact) discovered missing includes. As you said there are some header included twice then but it's easy to remove the duplicates afterwards. Much easier than searching for the header files missing. Great work! 8)
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: MortenMacFly on May 14, 2007, 10:16:03 pm
Right then... I had the crazy idea of starting to add support for wxWidgets to this plugin. So what I did was entering the config, creating a new set and started with "wxBeginBusyCursor". Unfortunately I received the error "Please enter valid C++ identifier". I looked into configuration.cpp and found the line:
Code
if ( wxString(_T("_ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstvwxyz")).Find(Identifier.GetChar(0)) == wxNOT_FOUND )
which is causing the error message. Well... Identifier.GetChar(0) is obviously "w" (I verified that)... Now I read the line a million times - either it's too late or I'm simply too dumb: Why is it failing??? I mean: it seems obviously to be a minor bug but I cant' find the reason for this .Find() method to fail. Any help?!
With regards, Morten.
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: byo on May 15, 2007, 12:05:34 am
Right then... I had the crazy idea of starting to add support for wxWidgets to this plugin. So what I did was entering the config, creating a new set and started with "wxBeginBusyCursor". Unfortunately I received the error "Please enter valid C++ identifier". I looked into configuration.cpp and found the line:
Code
if ( wxString(_T("_ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstvwxyz")).Find(Identifier.GetChar(0)) == wxNOT_FOUND )
which is causing the error message. Well... Identifier.GetChar(0) is obviously "w" (I verified that)... Now I read the line a million times - either it's too late or I'm simply too dumb: Why is it failing??? I mean: it seems obviously to be a minor bug but I cant' find the reason for this .Find() method to fail. Any help?!
With regards, Morten.

'u' is missing in strings with allowed chars  :oops:
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: gjsmo on May 15, 2007, 12:30:10 am
reason
'u' is missing in strings with allowed chars  :oops:
Huh. How did that happen? I wonder...

P.S. I am not really familiar with the CB code and wxWidgets. However, I can pretty much see what this is doing (checking for allowed characters by having a string with all allowed chars, then searching for the entered char in the string to see if it is valid) and I would think that this would be an obvious mistake.

Now I read the line a million times - either it's too late or I'm simply too dumb: Why is it failing???

I am not sure but I think maybe the latter reason explains it. :oops:
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: killerbot on May 15, 2007, 08:53:52 am
Thanks Byo, as we have seen here: http://forums.codeblocks.org/index.php/topic,5903.msg45142.html#msg45142 (http://forums.codeblocks.org/index.php/topic,5903.msg45142.html#msg45142) my attempt was far from being perfect anyway... :cry:
With regards, Morten.

Edit: Wohooo! Looks great! Have tested in on some of the plugins and (in fact) discovered missing includes. As you said there are some header included twice then but it's easy to remove the duplicates afterwards. Much easier than searching for the header files missing. Great work! 8)

sounds nice but ...... I had a look at the envvars.h, you added cbproject.h to the include list. Why ?
I don't see any reason why, or maybe it's still to early in the morning for me ;-)


In case it would be for FileTreeData : you should use the forward declaration :
class FileTreeData;
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: MortenMacFly on May 15, 2007, 09:28:32 am
In case it would be for FileTreeData : you should use the forward declaration :
class FileTreeData;
It was indeed for that. Anyway - forward decl's are not yet supported by the plugin... but yes - would have been better. :oops: I'd better had not done that.
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: byo on May 15, 2007, 10:07:06 am
sounds nice but ...... I had a look at the envvars.h, you added cbproject.h to the include list. Why ?
I don't see any reason why, or maybe it's still to early in the morning for me ;-)


In case it would be for FileTreeData : you should use the forward declaration :
class FileTreeData;

Yup, cbproject.h is included because of FileTreeData. Plugin adds it because this structure is used somewhere in code. Now the only function where it's used is:

Code
  void    BuildModuleMenu(const ModuleType type, wxMenu* menu,
                          const FileTreeData* data = 0)
  { return; }

Without expensive parsing there's no way to detect whether we access members of structure in source or not. And if we use members of structure, the structure has to be fully defined. cbproject.h is added "just in case".

Plugin works on very low level - it only does basic tokenization of sources and take list of used identifiers (no matter what context it is, whether it's class member or type name etc) so it can blindly add some headers, like the one mentioned before, "just in case". In some situations it's not advised (f.ex. when forward declaration is required to compile properly), so I don't advise using this plugin on C::B sdk project because it may even break the build. But it's output may be used as a hint when some plugins can not compile because of missing headers. One more sdk header in plugin's source won't break the build but may be usefull when sources need to be compiled quickly without investigating what headers are missing.

And one more thing - when C::B will have some really accurate c++ parser and refactoring tool, this tool would probably do this job much better. So I don't plan to upgrade my plugin and I would preffer not putting it into svn since it may cause more problems than benefits for unexperienced users. Just threat it as temporary solution ;)

BYO
Title: Re: The 10 May 2007 build will NOT be out. (developers please read this)
Post by: byo on May 15, 2007, 01:10:14 pm
More on Header-Fixup plugin in this (http://forums.codeblocks.org/index.php/topic,5911.msg45205.html#msg45205) thread.

BYO