Step export not working when special characters in filename or path

[Bob]

If special characters (german umlauts i.e.) are present in path or filename, step export does not work.
The following error message is printed out:

Warning: 09:59:45: * error reading file: 'C:\Users\XXXXXX\Documents\Küvettenheizung\Küvettenheizung_v2\Küvettenheizung_v2.1.kicad_pcb'
Warning: * Error occurred attempting to read in file
Warning:

Version info:

Application: pcbnew
Version: (5.0.0), release build
Libraries:
    wxWidgets 3.0.3
    libcurl/7.54.1 OpenSSL/1.0.2l zlib/1.2.11 libssh2/1.8.0 nghttp2/1.23.1 librtmp/2.3
Platform: Windows 7 (build 7601, Service Pack 1), 64-bit edition, 64 bit, Little endian, wxMSW
Build Info:
    wxWidgets: 3.0.3 (wchar_t,wx containers,compatible with 2.8)
    Boost: 1.60.0
    OpenCASCADE Community Edition: 6.8.0
    Curl: 7.54.1
    Compiler: GCC 7.1.0 with C++ ABI 1011

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_ACTION_MENU=OFF
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=ON

[Wayne Stambaugh (stambaughw)]

I confirmed that any character greater than 255 will cause the source file name to fail. Fixing this will likely not be trivial since kicad2step is run as an external process and the file name is passed via the command line and kicad2step parses the command line with wxCmdLineParser which hides the command line character parsing in wxAppConsole. Maybe it's time to move all of this inside kicad proper and get rid of kicad2step as an external utility.

[Wayne Stambaugh (stambaughw)]

I found that this issue[1] has been fixed in wx 3.1.1 for mingw so I'm guessing that this issue does not exist on Linux and MacOS. We would have to patch[2] wx 3.0.4 in order to provide a fix until the minimum wx version is set to 3.1.1 which probably wont be any time soon.

[1]: http://trac.wxwidgets.org/ticket/14580
[2]: http://trac.wxwidgets.org/ticket/14580#comment:11

[Jeff Young (jeyjey)]

@Wayne, not to beat a dead horse, but...

We've found it difficult to get platform packagers to apply wxWidgets patches. Fair enough; patching is an ugly business.

But what about simply cloning a kicad/wxwidgets git repo and building it? Would they be more amenable to that?

[Wayne Stambaugh (stambaughw)]

@Jeff, I am painfully aware of the wxWidgets packaging issues. To be fair to our packagers, this bug fix should have been applied to the 3.0 branch so it would be in 3.0.5 when it becomes available instead of having to use a development version (3.1) of wxwidgets. I'm not 100% sure how window builds are created but I'm guessing that we are using msys2/mingw(32/64) dependency packages as supplied (that's what use to build kicad on windows) although I could be wrong. So cloning our kicad/wxwidgets repo would require a significant change in how the windows builders are set up. The msys2 project already patches wxwidgets 3.0.4 so I guess I could submit this patch (assuming it applies cleanly) to the msys2 project to fix this issue. I will take a look at that solution over the weekend since I will be hunkered down as hurricane Florence passes by closer than I would like.

[Jeff Young (jeyjey)]

My dad's in Wilmington NC, so we're keeping our fingers crossed.

[Wayne Stambaugh (stambaughw)]

I would not be hanging around in Wilmington if the track projections are
correct. I hope you dad makes it out safe and sound.

On 09/12/2018 03:37 PM, Jeff Young wrote:
> My dad's in Wilmington NC, so we're keeping our fingers crossed.
>

[Wayne Stambaugh (stambaughw)]

I just spent several hours of time that I wish I could get back as neither wxWidgets 3.1.1 or the current head (75225cc1a6) of the master branch fixes this issue so patching 3.0.4 is not going to buy us anything unless we create our own patch. It looks like the original patch provided to fix this issue was ignored and another solution was implemented by one of the wxWidgets devs which apparently wasn't actually tested. I will ping the wxWidgets bug tracker and let them know that the issue is not resolved. If it doesn't get resolved, we may have to fall back to integrating step export without using and external process.

[Nick Østergaard (nickoe)]

I think cirilo submitred a patch to fox this tp apply to oce, I don't think it is applied in the current windows builds. Maybe I should try to apply that again and retest?

[Wayne Stambaugh (stambaughw)]

@Nick, I think the problem is fairly trivial (famous last words) to fix in that wmain() should be used instead on main() for windows compilers that support wmain(). Unfortunately, the wxWidgets devs have decided that only msvc supports wmain() which is no longer true (I confirmed by using gcc to compile a simple program with wmain() defined for both 32 and 64 bit versions of gcc). Even worse, they are relying on compiler definitions to determine which compilers support wmain(). This is a flawed design. The existence of wmain() should be checked by compiling a small source file with wmain() during configuration and setting a definition to tell the source code to use wmain() instead of main(). This would work properly for any windows compiler as well as cross compiling where wmain() is declared. Given the what I've seen in wxWidgets, I doubt I would waste my time submitting a patch to fix this properly.

[Seth Hillbrand (sethh)]

Given the error message reported, it looks to me like the issue is not parsing the command line (that would have failed when checking whether the file existed) but rather the .c_str() conversion when parsing in sexpr_parser.

On Linux, there are a whole separate set of issues[1] that prevent the files from being loaded.

[1] https://trac.wxwidgets.org/ticket/9652

[Wayne Stambaugh (stambaughw)]

@Seth, the failure is when checking if the board file name exists. I created a test app for the wxWidgets devs to use for testing. It's basically a stripped down version of the kicad2step wxApp object that just parses the string passed on the command line.

[Nick Østergaard (nickoe)]

I was thinking about this patch

https://lists.launchpad.net/kicad-developers/msg28461.html

I am not sure of this is the same issue. Maybe I misunderstood something.

[Seth Hillbrand (sethh)]

@Wayne- I don't follow the logic on that. Bob's error message is "Error occurred attempting to read in file", that is the exception thrown by sexp_parser when it can't read data. This is called by kicadpcb.cpp at line 146, which would indicate that he has successfully tested that the file exists at line 128.

My linux build is failing on UTF8 characters at the exists line in kicad2step.cpp:243, and it sounds like that is the same place yours is failing.

[Wayne Stambaugh (stambaughw)]

@Seth, your correct. I didn't even notice that but it appears there is more than one issue with unicode characters in kicad2step. I just spent at least 6 hours over the last two days chasing this down and I have made no progress (see https://trac.wxwidgets.org/ticket/14580#comment:22). I'm going to throw in the towel and kick this one down the road until we can integrate kicad2step and get rid kicad2step as an external utility.

I wonder if Bob would be willing to send us his board file because I cannot even get past the check for a valid file in kicad2step.cpp to even trigger the bug he reported. Have you been able to trigger this?

[Bob (b-ansay)]

@Wayne, is the .kicad_pcb enough or do you need any other files as well?

Cheers,
Bob

[Wayne Stambaugh (stambaughw)]

@Bob, thanks for the board file. That's all I need. Things really get interesting using your file. Your board file name gets past the check for the existence of the file which failed for everything I tried so there are two distinct problems here although they appear to be related. There is a check for the existence of the board file in KICAD2MCAD::OnRun() which passes for your file but fails for all of my attempts which doesn't make any sense. What makes even less sense is there is a second file existence test in KICADPCB::ReadFile() using the exact same string which is the failure you are seeing. This one might take a while.

[Bob (b-ansay)]

You're welcome. Glad that I can contribute a little bit :)

[KiCad Janitor (kicad-janitor)]

Fixed in revision 7372acdaa52e3a8722f1791335ffd7ea760301a7
https://git.launchpad.net/kicad/patch/?id=7372acdaa52e3a8722f1791335ffd7ea760301a7

[jean-pierre charras (jp-charras)]

I leave this as triaged, because when a full filename is not a full ASCII7 string, there are still some issues, especially with opencascade.

Perhaps this is noticeable only on Windows.

[Wayne Stambaugh (stambaughw)]

@JP, thanks for "fixing" this even though OCE still has it's own issues. Please apply this change to the 5.0 branch when you get a chance but please use the old error message string so it doesn't break the existing translation. I'm fine with the error string changes in 5.1 since the translations will need to be updated anyway. Did you by chance test this on Linux? I was testing some changes that I made to fix this on Linux and discovered that this is completely broken on Linux as well.

[jean-pierre charras (jp-charras)]

In fact kicad2step itself has no I18n strings, so no existing translation I am aware of.

In fact, I found different issues.
Inside kicad2step code, non ASCII7 strings (read: filenames) are incorrectly managed.
(For instance, I found at least one filename converted in UTF8, and the resulting string converted to UTF8 a second time, giving some strange filename).

Currently I am trying to clean the way filenames are managed inside kicad2step code.
I am guessing there are also issues for 3D step models filenames.

A second issue is the fact OCE uses (AFAIK) a c string in filename, not wide string (but I am not sure: OCE is a big library...).
(But it can be solved easily, by using a temporary filename)

I will test it on Linux.

[Wayne Stambaugh (stambaughw)]

@JP, your commit fixes the issue with Bob's file but it doesn't resolve the issue for all non ascii characters. I've attached a file that causes the file existence check on line 243 of kicad2step.cpp to fail so it never gets to the file read code that you fixed. If you get a chance please take a look at it and see if you can figure out.

[jean-pierre charras (jp-charras)]

@Wayne

I can commit a fix for names like hexapodu_Ā.kicad_pcb.

But I had a look at this issue on Linux, and I find really strange issues:

Inside kicad2step application, wxLogDebug, used a lot of time
in code, just does not work fine.
For instance, on my Kubuntu 14.04 install:
wxLogMessage( "123à" ); generates a segfault.

and wxLogMessage( "file %s", filename);
does not even print a message, if filename contains a "ascii code" between 0x80 and 0xFF

So it could take a bit of time before this issue is fixed.

[KiCad Janitor (kicad-janitor)]

KiCad bug tracker has moved to Gitlab. This report is now available here: https://gitlab.com/kicad/code/kicad/-/issues/1782