← Back to team overview

kicad-developers team mailing list archive

Re: comments and questions on SEARCH_STACK

 

On 9/13/2015 11:31 PM, Cirilo Bernardo wrote:
> Hi folks,
> 
>  I was just looking into how KiCad currently handles partial paths for
> files and I have found:
> 
> (a) 3D model paths are now resolved exclusively via the KISYS3DMOD
> environment variable
> 
> (b) kiface_i.cpp contains what appears to be some dead code:
> (lines 64 - 76):
> 
>         // Add PCB library file path to search path list.
>         if( aId == KIWAY::FACE_PCB || aId == KIWAY::FACE_CVPCB )
>         {
>             fn.AppendDir( wxT( "modules" ) );
>             aDst->AddPaths( fn.GetPath() );
> 
>             // Add 3D module library file path to search path list.
>             fn.AppendDir( wxT( "packages3d" ) );
>             aDst->AddPaths( fn.GetPath() );
> 
>             fn.RemoveLastDir();
>             fn.RemoveLastDir();     // "../../" up twice, remove
> modules/packages3d
>         }
> 
> While I haven't checked if "modules" is used (I presume it is since
> schematic
> symbols are there), I don't believe the "packages3d" directory is ever
> searched
> via the SEARCH_STACK object. I suggest we remove the lines 70..74:
>             // Add 3D module library file path to search path list.
>             fn.AppendDir( wxT( "packages3d" ) );
>             aDst->AddPaths( fn.GetPath() );
> 
>             fn.RemoveLastDir();

For the stable release there is no harm in leaving it in there but I
agree that non-existent paths should be removed from the list.  I
thought at one time, they were removed but maybe that code got dropped
during the kiway transition.

> 
> My reasons are (1) anyone who's already set up to use 3D models is
> no doubt using KISYS3DMOD and (2) the 3D code in my refactoring
> branch will have its own configuration file containing path roots for
> 3D models and will not use SEARCH_STACK anyway. (Then again
> I can always make a small change to use SEARCH_STACK.)

I would reject any patch that uses the SEARCH_STACK.  I would also
reject using platform specific paths unless the user specifically
requested them.  The 3D model file paths must be defined in a platform
agnostic manner.  This is why we have the fp-lib-table to map footprint
paths, urls, etc. to a nickname.  Defined properly, this hides the
platform path dependency issues.  All of the libraries provided by KiCad
should be defined in this manner.  If the user wants to use hard coded
paths, that is their choice and they can be responsible for the lack of
portability.  KiCad as a project should provide tools that enforce
portability out of the box as much as possible.

> 
> Another suggestion for SEARCH_STACK: there are obviously a
> few hard-coded assumptions about paths and paths are added
> without checking that (a) they exist and (b) they are directories.
> If we eliminate candidate paths which do not exist or are not
> directories then we may save some time otherwise wasted
> searching non-existent paths; this will also avoid the error which
> I frequently get complaining about items not being available
> in non-existent paths.
> 
> - 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
> 


Follow ups

References