Re: TortoiseCVS RC4 patches

Arthur Barrett wrote:
> Torsten et al,
>
> Here are some notes and patches for TortoiseCVS 1.12 RC4 build from source - at least one of these (#8) probably needs fixing before release.
>
> 1.
> If you want to build 32 bit releases using VS.NET 2003 (VC7) you need a few missing items.
>   
Yes. VC7 is sort-of deprecated nowadays. Still, might be worth fixing.
> 2.
> Bison/Flex have been removed from tools/gnuwin32 but there is no clear replacement, however:
>
> 2a:
> The macros add_bison_files and add_flex_files in build/CMakeLists.txt refers specifically to the working directory /GnuWin32/bin
>
> The way I got it to build was by downloading flex and bison from:
> http://gnuwin32.sourceforge.net
>   
Yes, that is necessary. I must get around to updating the documentation.
> 2b:
> If you want developers to use cygwin bison (as suggested by your commit comment) are you going to commit the binaries to tools/cygwin or just make it a dependency?
>   
No, the intention is to use the GnuWin32 tools. The Cygwin ones easily 
create conflicts if the user has another version installed.
And I have not succeeded in getting everything to work if the Gnuwin32 
tools are in the build tree - they need to be installed.
> 2c:
> Any chance you can explain this change - specifically whether it is a functional requirement (ie: gnuwin32 bison will create unstable TortoiseCVS binaries) or if it's just standardising on a toolset or something else?  The commit comment was a bit terse ;)
>   
Which commit comment are you referring to?
> 3.
> You've committed / added wxw/include with -kkv but I think it probably should be -ko to preserve the keyword expansion in the original headers. 
True. $Id$ is a nuisance.
> 4.
> RuntimeInstaller/RuntimeInstaller.c should include 
> #include "CvsNt/cvsnt-version.h"
>
> Or fix cmake...
>
> 5.
> RuntimeInstaller/RuntimeInstaller.c refers to non-existent header:
> #include "overlays-version.h"
>
> But there is a file tortoisecvs/src/SharedDlls/TortoiseOverlaysVersion
>
> Is overlays-version.h supposed to be generated somehow?  Any hints appreciated...
>   
Yes: 
http://tortoisecvs.cvs.sourceforge.net/viewvc/tortoisecvs/TortoiseCVS/build/utils.sh?r1=1.13&r2=1.14

You must use the /build/autobuild script at least once.
> 6.
> src/TortoiseShell/ShellExt.cpp refers to <wx/msgcatalog.h> which is not a part of wxWidgets 2.8 and I cannot find any reference to wxMsgCatalog in the wxWidgets documentation but it is forward defined in the wxLocale class <wx/intl.h> for a return value of private functions -- the comments say 'This is a "low-level" class and is used only by wxLocale'.
>   
I had to hack wxWidgets in order to fix a bug where locale changes 
affected other applications - wxWidgets sets the global locale, which of 
course is a bad idea in a DLL that can be loaded by arbitrary applications.
> Is this intentional?  If so I think there should be a note in:
> http://www.tortoisecvs.org/development.shtml
>
>
> If not intentional then I think you are 'supposed' to replace:
>
> wxMsgCatalog*   g_msgCatalog = 0;
>               g_msgCatalog = new wxMsgCatalog;
>               if (!g_msgCatalog->Load(info->CanonicalName, wxT("TortoiseCVS"), 0, true))
>               const wxChar* result = g_msgCatalog->GetString(str);
>               if (g_msgCatalog)
>               delete g_msgCatalog;
>
> with something a bit like:
>
>              wxLocale locale;
>              locale.AddCatalogLookupPathPrefix(wxTextCStr(localedir + "\\Locale"));
>              const wxLanguageInfo* info = wxLocale::GetLanguageInfo(language);
>              locale.Init(info->Description, info-> CanonicalName, info->CanonicalName, true, true);
>              locale.AddCatalog(wxT("TortoiseCVS"), info ,0);
>              locale.GetString(str,NULL);
>
>
>   
It is true that this is the canonical way, but the wxLocale ctor sets 
the global locale.
> 7.
>
> In HasMenu.cpp you define the variable 'i' twice and it causes lots of warnings about non standard use - I suspect VC7 may be a little confused which 'i' you mean...:
>   
Yes, VC7 does not obey the C++ for-loop scoping rules. That is one 
reason it is deprecated.
> 8.
> I get lots of these errors:
> ------ Build started: Project: TortoiseAct, Configuration: Release Win32 ------
>
> Compiling...
> ModuleBasicsPage.cpp
> \cvsbin\release builder\tcvs-1-12-RC2\src\DialogsWxw\ModuleBasicsPage.cpp(1149) : error C2664: 'WideToMultibyte' : cannot convert parameter 1 from 'wxString' to 'const std::wstring &'
>         Reason: cannot convert from 'wxString' to 'const std::wstring'
>         No constructor could take the source type, or constructor overload resolution was ambiguous
>   
That means that your wxWidgets is misconfigured - I will have to check 
on my build machine which defines to set.
> I notice that none of the DialogsWxw/*.h files are a part of the project - it makes it very difficult to search for the definition of something... so I added them manually since I'm not really sure how to magically add them via TortoiseAct/CMakeLists.txt
>   
Just add them to the list of sources, the same as for .cpp files. CMake 
automatically does the right thing.
> I believe all these are errors - they should be changed from:
>     return myUseBugCheckBox->GetValue() ? wxAscii(myBugNumber->GetValue()) : "";
> to
>     return myUseBugCheckBox->GetValue() ? wxAscii(myBugNumber->GetValue().c_str()) : "";
>   
Again, as in #8, wxWidgets needs to be configured so that wxString is 
compatible with std::string.
> 9.
> Should wxwidgets still be compiled by settting lib/vc_lib/msw*/wx/setup.h wxUSE_STL to 1 as described here:
> http://www.tortoisecvs.org/development.shtml
>   
See #8

-Torsten

------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev