kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

On 9/18/2015 7:59 AM, Mark Roszko wrote:
> Speaking of paths, there's invalid default paths(%PROGRAM
> FILES%/KiCad) being used for Save and Open dialogs thoroughout KiCad
> which need to be changed to the user's document folder. Wayne said he
> was working on it awhile ago but its something to fix.

I'm still am working on it.  I hope to commit it tomorrow.  As soon as
Brian gets the help search path stuff fixed I'll roll out rc2 unless
some other major bug pops up.

@Brian, attached are patches for search_stack.cpp and
searchhelpfilefullpath.cpp which should solve the help problem.
Actually, some of the changes should be moved into the system search
stack which should solve all of the search path issues not just the help
path.  Feel free to cherry pick these changes or ignore them.  One thing
that is broken in both cases is that the KICAD env var is not always the
first entry in the search stack when it's defined and it should be.
User defined paths should *always* take precedence.

> 
> On Fri, Sep 18, 2015 at 6:03 AM, Brian Sidebotham
> <brian.sidebotham@xxxxxxxxx> wrote:
>> GetExecutablePath() has a fallback which is poor but not too
>> unreasonable: https://github.com/wxWidgets/wxWidgets/blob/master/src/common/stdpbase.cpp#L67
>>
>> For Linux it grabs /proc/self/exe which seems reasonable(ish):
>> https://github.com/wxWidgets/wxWidgets/blob/master/src/unix/stdpaths.cpp#L120
>>
>> For Windows it uses the Module path which is fine:
>> https://github.com/wxWidgets/wxWidgets/blob/master/src/msw/stdpaths.cpp#L343
>> https://github.com/wxWidgets/wxWidgets/blob/master/include/wx/msw/private.h#L894
>>
>> For OSX the code looks ok, though I can't test it at all:
>> https://github.com/wxWidgets/wxWidgets/blob/master/src/osx/core/stdpaths_cf.cpp#L142
>>
>> The only problem I see if the BSD range of OS, which is basically
>> where /proc/self/exe is not available because that will fall back to
>> the base implementation. Although the base implementation is not too
>> bad either. We could do a pull request for wxWidgets to fix up
>> GetExecutablePath() for those by testing for /proc/curproc/exe and
>> /proc/curproc/file and using if one exists - that should cover most of
>> the BSD range better.
>>
>> I think GetExecutablePath() is accurate enough for us to use without
>> problem. It's so common to tie installed files relative the executable
>> path it's virtually standard for me. This means we can run sandboxed
>> versions, i.e. without having to have a global config file which is
>> awkward when you're trying to test different things at once. Otherwise
>> a /etc/kicad.conf would be perfect, but has no equivalent on Windows.
>>
>> Best Regards,
>>
>> Brian.
>>
>> On 18 September 2015 at 00:40, Cirilo Bernardo
>> <cirilo.bernardo@xxxxxxxxx> wrote:
>>>
>>>
>>> On Fri, Sep 18, 2015 at 8:20 AM, Brian Sidebotham
>>> <brian.sidebotham@xxxxxxxxx> wrote:
>>>>
>>>> As mentioned in the bug report here
>>>> https://bugs.launchpad.net/kicad/4.0/+bug/1496049 there's a problem
>>>> with paths on the Windows 4.0.0-RC1.
>>>>
>>>> So far the paths that don't work:
>>>>
>>>>     (1) Searching for help files fails
>>>>     (2) Default paths in the "Path configuration" dialog all point to
>>>> places in c:\msys64\mingw64, etc.
>>>>     (3) Python footprint wizards don't load
>>>>
>>>> and probably more. Needless to say, the KiCad experience out of the
>>>> box because of this is very poor.
>>>>
>>>> We need to fix up the path search which started with the help in the
>>>> bug report. The current consensus is to have (yet another) env var
>>>> called something like KIHELP which can default on Windows to the
>>>> literal string "${KICAD}\share\doc\kicad\help" which alleviates the
>>>> previous problem of trying to guess the install path at compile time
>>>> which is clearly flawed.
>>>>
>>>
>>> There are problems for sure but I don't believe in adding yet another
>>> env var if it can be helped, but this may be a complex enough issue that
>>> an env var would be the best option for the release. On MSWin documents
>>> and such should go into an appropriate one of MSWin's Known Folders -
>>> the question then is can we access these folder names via some library
>>> in MSys/MinGW?
>>>
>>>
>>>> My personal preference would have just to have checked
>>>> "BINDIR\..\share\doc\kicad\help" where BINDIR is gathered from
>>>> ::wxStandardPaths::Get().GetExecutablePath();
>>>>
>>>
>>> I have my doubts here; there are too many ways that GetExecutablePath()
>>> can give you the wrong result - certainly on *NIX there is no guarantee
>>> what result you will get (but on Linux there are tricks to get it right -
>>> assuming wxWidgets has a good implementation), but I'm not so sure of
>>> the behavior on MSWin. Short story: inspect the wx implementation
>>> before believing the results.
>>>
>>>>
>>>> However, the decision appears to be to add another configurable path.
>>>>
>>>> While adding the new KIHELP env var solves part of the problem, the
>>>> Windows installer should be updated to set the KICAD env var to the
>>>> chosen install path while installing. et. voila, paths fixed.
>>>>
>>>> Best Regards,
>>>>
>>>> Brian.
>>>>
>>>
>>>  I think a well-known path is preferable to yet another configurable path;
>>> this is true for all platforms. In my 3D refactoring branch there is some
>>> code which attempts to find a plugins directory using a number of
>>> known paths. I think for all items specifically associated with the
>>> kicad install, known paths should be used. For other items like
>>> 3D models some user configuration should be supported (though
>>> any 3D models installed by a kicad installer should be put into a
>>> known path).
>>>
>>> - 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
> 
> 
> 

=== modified file 'common/search_stack.cpp'
--- common/search_stack.cpp	2015-02-20 19:04:32 +0000
+++ common/search_stack.cpp	2015-09-18 14:25:40 +0000
@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2014 CERN
- * Copyright (C) 2014 KiCad Developers, see CHANGELOG.TXT for contributors.
+ * Copyright (C) 2014-2015 KiCad Developers, see CHANGELOG.TXT for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -198,12 +198,13 @@
 
 
 #if defined(DEBUG)
-void SEARCH_STACK::Show( const char* aPrefix ) const
+void SEARCH_STACK::Show( const wxString& aPrefix ) const
 {
-    printf( "%s SEARCH_STACK:\n", aPrefix );
+    wxLogDebug( wxT( "%s SEARCH_STACK:" ), GetChars( aPrefix ) );
+
     for( unsigned i=0;  i<GetCount();  ++i )
     {
-        printf( "  [%2u]:%s\n", i, TO_UTF8( (*this)[i] ) );
+        wxLogDebug( wxT( "  [%2u]:%s" ), i, TO_UTF8( (*this)[i] ) );
     }
 }
 #endif

=== modified file 'common/searchhelpfilefullpath.cpp'
--- common/searchhelpfilefullpath.cpp	2015-05-15 12:49:11 +0000
+++ common/searchhelpfilefullpath.cpp	2015-09-18 14:26:01 +0000
@@ -26,6 +26,7 @@
 #include <pgm_base.h>
 #include <common.h>
 #include <config.h>     // to define DEFAULT_INSTALL_PATH
+#include <macros.h>
 
 
 /**
@@ -49,6 +50,8 @@
                 fn.AppendDir( (*aSubdirs)[j] );
         }
 
+        wxLogDebug( wxT( "    %s" ), GetChars( fn.GetFullPath() ) );
+
         if( fn.DirExists() )
         {
             paths.Add( fn.GetPath() );
@@ -68,18 +71,36 @@
 
     // It might already be in aSStack, but why depend on other code
     // far away when it's so easy to add it again (to our copy) as the first place to look.
-    // This is CMAKE_INSTALL_PREFIX:
+
+    // This is CMAKE_INSTALL_PREFIX unless DEFAULT_INSTALL_PATH was defined during
+    // build configuration:
     ss.AddPaths( wxT( DEFAULT_INSTALL_PATH ), 0 );
 
-    // If there's a KICAD environment variable set, use that guy's path also
-    ss.AddPaths( Pgm().GetKicadEnvVariable(), 0 );
-
 #if defined(__WXMAC__)
     ss.AddPaths( GetOSXKicadMachineDataDir() );
     ss.AddPaths( Pgm().GetExecutablePath(), 0 );
+
+    // OS X packages can have the help files in
+    // /Library/Application\ Support/kicad/help,
+    // and in Contents/SharedSupport/help inside the
+    // bundle.
+    // Below we account for an international subdirectory.
+    subdirs.Add( wxT( "help" ) );
+    altsubdirs.Add( wxT( "Contents" ) );
+    altsubdirs.Add( wxT( "SharedSupport" ) );
+    altsubdirs.Add( wxT( "help" ) );
 #endif
 
 #if ! defined(__WXMAC__) // && defined(__linux__)
+    // This is the executable path minus the trailing bin directory used on Windows and Linux.
+    wxFileName tmp( Pgm().GetExecutablePath(), wxEmptyString );
+    wxArrayString binDirs = tmp.GetDirs();
+
+    if( !binDirs.IsEmpty() && binDirs[ binDirs.GetCount() - 1 ].CmpNoCase( wxT( "bin" ) ) == 0 )
+        tmp.RemoveLastDir();
+
+    ss.AddPaths( tmp.GetPath(), 0 );
+
     // Based on kicad-doc.bzr/CMakeLists.txt, line 20, the help files are
     // installed into "<CMAKE_INSTALL_PREFIX>/share/doc/kicad/help" for linux.
     // This is ${KICAD_HELP} var in that CMakeLists.txt file.
@@ -88,9 +109,7 @@
     subdirs.Add( wxT( "doc" ) );
     subdirs.Add( wxT( "kicad" ) );
     subdirs.Add( wxT( "help" ) );
-#endif
 
-#if ! defined(__WXMAC__) // && defined(__WINDOWS__)
     // Based on kicad-doc.bzr/CMakeLists.txt, line 35, the help files are
     // installed into "<CMAKE_INSTALL_PREFIX>/doc/help" for Windows.
     // This is ${KICAD_HELP} var in that CMakeLists.txt file.
@@ -99,17 +118,9 @@
     altsubdirs.Add( wxT( "help" ) );
 #endif
 
-#if defined (__WXMAC__)
-    // OS X packages can have the help files in
-    // /Library/Application\ Support/kicad/help,
-    // and in Contents/SharedSupport/help inside the
-    // bundle.
-    // Below we account for an international subdirectory.
-    subdirs.Add( wxT( "help" ) );
-    altsubdirs.Add( wxT( "Contents" ) );
-    altsubdirs.Add( wxT( "SharedSupport" ) );
-    altsubdirs.Add( wxT( "help" ) );
-#endif
+    // If there's a KICAD environment variable set, always use that guy's path first.
+    if( !Pgm().GetKicadEnvVariable().IsEmpty() )
+        ss.AddPaths( Pgm().GetKicadEnvVariable(), 0 );
 
     /* Search for a help file.
      *  we *must* find a help file.
@@ -126,15 +137,18 @@
     // If fails, try to find help file in help/en
     wxArrayString locale_name_dirs;
     locale_name_dirs.Add( i18n->GetCanonicalName() );           // canonical name like fr_FR
+
     // wxLocale::GetName() does not return always the short name
     locale_name_dirs.Add( i18n->GetName().BeforeLast( '_' ) );  // short canonical name like fr
     locale_name_dirs.Add( wxT( "en" ) );                        // default (en)
 
-#if defined(DEBUG) && 0
-    ss.Show( __func__ );
-    printf( "%s: m_help_file:'%s'\n", __func__, TO_UTF8( aBaseName ) );
+#if defined(DEBUG) && 1
+    ss.Show( wxString( __func__ ) );
+    wxLogDebug( wxT( "%s: m_help_file:'%s'" ), __func__, GetChars( aBaseName ) );
 #endif
 
+    wxLogDebug( wxT( "Checking SEARCH_STACK for file %s" ), GetChars( aBaseName ) );
+
     // Help files can be html (.html ext) or pdf (.pdf ext) files.
     // Therefore, <BaseName>.html file is searched and if not found,
     // <BaseName>.pdf file is searched in the same paths