kicad-developers team mailing list archive

[Cirilo Bernardo <cirilo.bernardo@xxxxxxxxx>]

On Wed, Mar 1, 2017 at 11:13 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
> On 2/28/2017 7:07 PM, Cirilo Bernardo wrote:
>> On Wed, Mar 1, 2017 at 10:33 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>> On 2/28/2017 4:08 PM, Cirilo Bernardo wrote:
>>>> On Wed, Mar 1, 2017 at 12:18 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>>>> On 2/27/2017 8:57 PM, Cirilo Bernardo wrote:
>>>>>> On Tue, Feb 28, 2017 at 1:07 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>>>>>> On 2/26/2017 4:04 PM, Cirilo Bernardo wrote:
>>>>>>>> There is one other way which I found after much digging
>>>>>>>> and it involves a GCC extension. Since we use GCC on
>>>>>>>> Windows this might be acceptable:
>>>>>>>>
>>>>>>>> a. create a derived class of std::ifstream/ofstream. On
>>>>>>>> Windows the derived class will be used while on other
>>>>>>>> OS it will simply be typedef to std::ifstream/ofstream
>>>>>>>
>>>>>>> This seems reasonable to me.
>>>>>>>
>>>>>>
>>>>>>  I had a look at the GCC STL implementation and unfortunately
>>>>>> is is impossible for me to implement (a) since I can't accomplish
>>>>>> what I want by deriving std::ifstream/ofstream due to the access
>>>>>> specifiers on the necessary member variables and the fact that
>>>>>> the open() function is not declared virtual.
>>>>>>
>>>>>>>>
>>>>>>>> b. overload open() to use the gcc extension like this:
>>>>>>>> __gnu_cxx::stdio_filebuf buf( _wopen ( utf8_filename, _O_RDONLY ) );
>>>>>>>> std::istream mystream ( &buf );
>>>>>>>
>>>>>>> If this is portable, than I'm file with this as well but on the surface
>>>>>>> it looks gcc specific.  If that is the case, then I would rather got
>>>>>>> with option a.
>>>>>>>
>>>>>>
>>>>>> Even solution (a), which I now know is not possible, would have been
>>>>>> a gcc-specific hack.
>>>>>>
>>>>>> The solution I'm working on at the moment requires the replacement of
>>>>>>
>>>>>> std::ifstream X;
>>>>>> X.open( filename, ... );
>>>>>> X.close();
>>>>>>
>>>>>> with
>>>>>>
>>>>>> OPEN_ISTREAM( X, filename );
>>>>>> CLOSE_STREAM( X );
>>>>>>
>>>>>> On builds which are not MinGW the helper macros generate exactly
>>>>>> the same code as before. On MinGW builds, the helper macros
>>>>>> create an extra class which creates an i/ostream and cleans up
>>>>>> where required on destruction. The only caveat in MinGW is that
>>>>>> rather than an explicit ifstream/ofstream the object which is actually
>>>>>> created is an istream/ostream, but this is not a difficult thing to handle.
>>>>>> The good thing about preserving the use of std::iostream is that I
>>>>>> can eliminate some of the locale switching code and simply use
>>>>>> imbue() on the open streams to avoid unintended effects on other
>>>>>> code.
>>>>>
>>>>> I'm not sure at this point why you wouldn't just use FILE_LINE_READER or
>>>>> wxFFileInputStream which we know both work with utf8 file names.  It
>>>>> seems a bit like reinventing the wheel.  I realize this doesn't solve
>>>>> the oce issue but for KiCad's file parsing usage, I think it makes more
>>>>> sense.  I'm not saying your solution isn't valid, it just seems like
>>>>> unnecessary work.
>>>>>
>>>>
>>>> The std::stream objects have modular localization support which we need
>>>> to force "C" locale for VRML and IDF output and of course the '<<' and '>>'
>>>> stream operators. The code which needs to be reworked already uses
>>>> streams, so this gcc-specific hack is the easiest way to fix the UTF8
>>>> issue within KiCad (but of course not for external libs like OCE). This
>>>> change is already a 1600+ lines patch + a few hundred lines for the
>>>> additional files. Changing the modules to use FILE_LINE_READER
>>>> means that we need to perform app-wide locale changes just to input/
>>>> output a file since wx does not implement stream locale settings, plus
>>>> I would need to change the many hundreds (possibly a few thousand)
>>>> lines with streaming ops.
>>>
>>> Please fix it this way.  I'm not sure I really like it but it sounds
>>> like you've programmed yourself into a hole.  I try to avoid the stream
>>> << and >> operators just because of these issues.
>>>
>>
>> Hi Wayne,
>>
>>   Do you mean use the wxStreams instead of std::stream?
>
> Now I'm confused.  If you can fix it without resorting to a gcc-specific
> hack, that would be my preference but I assumed that you were talking
> about std::stream.
>

At the moment the only not-gcc-hack means is to painfully convert everything
to use the wxStream.  The gcc hack has already been implemented and tested
and works fine.

- Cirilo

>>
>> - Cirilo
>>
>>>>
>>>>
>>>>>>
>>>>>> I still need to look into how the issue with OCE can be tackled then
>>>>>> see if the devs are willing to make changes. This UTF8 filename
>>>>>> problem has come up on the MinGW list many times and a number
>>>>>> of users had suggested various changes over the years but this
>>>>>> really seems to be a "won't fix" issue.
>>>>>
>>>>> I spent about an hour the other day looking through the opencascade
>>>>> documentation and the oce source code and I couldn't find the code for
>>>>> the file parsers.  Could you point me to the source file where the base
>>>>> file parser code lives so I can take a look at it.  We can open utf-8
>>>>> file names just fine in mingw.  I don't understand why oce cannot open
>>>>> them on mingw as well.
>>>>>
>>>>
>>>> The OpenCascade source is easily grepped for FILE, fstream and so on.
>>>> The vast bulk of OpenCascade modules make use of defined classes to
>>>> handle I/O in various 'FSD' files, to to find all relevant files:
>>>>
>>>> find . -name "*FSD*"
>>>>
>>>> You can see the use of "_wopen" in those classes to open filestreams;
>>>> this is a Microsoft extension which is available in MSVC but not in
>>>> MinGW since the STL is different.  Other directories of interest are:
>>>>
>>>> src/STEPControl
>>>> src/IGESControl
>>>> src/STEPCAFControl
>>>> src/IGESCAFControl
>>>
>>> Thanks for the info.  I'll take a look at it when I can.
>>>
>>>>
>>>> - Cirilo
>>>>
>>>>
>>>>>>
>>>>>> If we use anything other than gcc on Windows we can tackle the
>>>>>> other issues then.
>>>>>>
>>>>>> - Cirilo
>>>>>>
>>>>>>>>
>>>>>>>> The destructor must contain code to delete buf since
>>>>>>>> the istream will not delete it on destruction.
>>>>>>>>
>>>>>>>> If that would be acceptable I'll make some test
>>>>>>>> programs and work on a patch set. This solution
>>>>>>>> would also work on OCE and I can talk to the OCE
>>>>>>>> patch team to see what they think of it.
>>>>>>>>
>>>>>>>> - Cirilo
>>>>>>>>
>>>>>>>>
>>>>>>>> On Mon, Feb 27, 2017 at 4:54 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>>>>>>>> On 2/26/2017 1:50 AM, Cirilo Bernardo wrote:
>>>>>>>>>> Hi folks,
>>>>>>>>>>
>>>>>>>>>>  This whole thing with UTF-8 filenames in Windows is a disaster.
>>>>>>>>>> What I've found so far:
>>>>>>>>>>
>>>>>>>>>> 1. Regarding OCE: Since OCE 0.17 (OpenCascade 6.8) UTF8
>>>>>>>>>> filenames have been supported when built with MSVC but
>>>>>>>>>> obviously not with MinGW.
>>>>>>>>>>
>>>>>>>>>> 2. MinGW does not provide any means of transparently using
>>>>>>>>>> UTF-8 filenames. All filenames within the STL *must* be
>>>>>>>>>> char* and MinGW *will* simply pass these on to OpenFileA()
>>>>>>>>>> on Windows resulting in UTF-8 being interpreted as ASCII-8
>>>>>>>>>> (and who uses ASCII-8 filenames anyway).
>>>>>>>>>>
>>>>>>>>>> So everything hinges on (2). If OCE uses std::stream then
>>>>>>>>>> fixing all issues under Windows is a lost cause. If OCE
>>>>>>>>>> simply plays with FILE* then it can be patched to work
>>>>>>>>>> in MinGW by invoking _wfopen() rather than fopen().
>>>>>>>>>>
>>>>>>>>>> As for kicad itself, std::stream is used in:
>>>>>>>>>> (a) VRML export
>>>>>>>>>> (b) IDF static library
>>>>>>>>>> (c) Scenegraph dynamic library for 3D plugins
>>>>>>>>>>
>>>>>>>>>> 2 paths forward come to mind and both will involve some work:
>>>>>>>>>>
>>>>>>>>>> (1) Move to the MSVC build system on Windows: this makes
>>>>>>>>>> it possible for us to use Microsoft extensions to STL to deal
>>>>>>>>>> with non-ASCII filename issues. There is no need to dig into
>>>>>>>>>> the OCE code since we know it will work correctly when built
>>>>>>>>>> with MSVC.
>>>>>>>>>
>>>>>>>>> This is not an acceptable solution.  It's not portable and would limit
>>>>>>>>> windows builds to using msvc.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> (2) Rework kicad code to play with FILE* (or wxFileStream)
>>>>>>>>>> rather than std::ifstream/ofstream. Although this will fix the
>>>>>>>>>> issues which are confined to kicad's source, it does nothing
>>>>>>>>>> to address the OCE issue. Whether or not OCE in MinGW
>>>>>>>>>> is a lost cause remains to be seen.
>>>>>>>>>
>>>>>>>>> FILE* is how we pretty much do it everywhere else in KiCad with good
>>>>>>>>> results so I don't see any reason not to do it this way with the model
>>>>>>>>> parser code.  At least it's portable across all build platforms.
>>>>>>>>> Doesn't oce have a reader function that takes a FILE *?
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> One other possibility (but one which I hadn't looked into)
>>>>>>>>>> is to see if the STL implementation within MinGW uses the
>>>>>>>>>> MinGW-CRT. If it does then it may be possible to fix
>>>>>>>>>> everything by ensuring that the MinGW-CRT converts all
>>>>>>>>>> filenames to UTF16 and opens a file using FileOpenW().
>>>>>>>>>> In all cases this is not a pleasant task.
>>>>>>>>>>
>>>>>>>>>> Any comments/suggestions?
>>>>>>>>>>
>>>>>>>>>> - Cirilo
>>>>>>>>>>
>>>>>>>>>> _______________________________________________
>>>>>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>>>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>>>>> More help   : https://help.launchpad.net/ListHelp
>>>
>