kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxxxx>]

On 11/11/2011 8:43 AM, Øyvind Aabling wrote:
> On Fri, 11 Nov 2011, Øyvind Aabling wrote:
> 
>> On Thu, 10 Nov 2011, Wayne Stambaugh wrote:
>>
>>> On 11/10/2011 1:15 PM, Øyvind Aabling wrote:
>>>> There seems to be a problem with the eeschema
>>>> library editor in recent BZR versions of kicad:
>>>> Changes to edited parts aren't saved to disk, and the only
>>>> way to exit the library editor is to discard all changes :-(
>>>>
>>>> I've now checked the BZR versions that I've been running lately,
>>>> and it works in BZR 3001 and 3153, and is broken in 3212,
>>>> 3221 and 3226 (so it broke somewhere between 3153 and 3212).
>>>>
>>>> I've updated my Debian recently, so I pulled BZR 3001
>>>> again (had deleted it :-) and the latest (BZR 3226), and
>>>> compiled both, to see if the problem was (in some obscure way)
>>>> related to library files on my system that have been upgraded
>>>> since compiling the various versions of kicad (No, it isn't ...).
>>>>
>>>> To recreate:
>>>>
>>>> Start eeschema,
>>>>
>>>> start the Library Editor,
>>>>
>>>> "Select working library",
>>>> (any library that you have write access to, e.g. in your project dir)
>>>>
>>>> "Load component to edit from the current lib",
>>>> (any component will do)
>>>>
>>>> "Create a new component from the current one",
>>>> (or change the component in some way, or create a new
>>>> component, or ... - anything that's considered a change)
>>>>
>>>> "Update current component in current library",
>>>> (this step can be skipped - the next step will
>>>> then ask for inclusion of the changes, which
>>>> doesn't make any difference on the behaviour)
>>>>
>>>> "Save current library to disk"
>>>> Include last component changes? Yes
>>>> Modify library file "<name>.lib"? Yes
>>>>
>>>> Now, the <name>.lib and/or <name>.dcm file(s) should have been
>>>> updated, but no, they're not, and the "Save current library to disk"
>>>> icon (far left) doesn't inactivate (gray out) either.
>>>
>>> I do not have this problem on 64 bit Debian Testing using KiCad version BZR
>>> 3226 and wxWidgets 2.8.12 even with all of the fancy new Gnome 3 stuff. The
>>> library file changes are saved properly and the save current library tool
>>> bar icon and file menu entry are disabled as expected.
>>
>> Well, I kinda suspected that, as such a major
>> problem probably couldn't go unnoticed for long :-)
>>
>> I'm also using 64-bit Debian Testing, but with wxWidgets 2.8.10.1-3.1.
>>
>> With wxWidgets upgraded to 2.8.12.1-2, I've now recompiled a
>> clean BZR 3226 tree with $MAKE unset (normally set to "make -j5").
>>
>> The problem isn't in wxWidgets or the parallel
>> make, as the problem still persists :-(
>>
>> Since you don't see this problem on a similar system, it sounds like
>> my system contains a buggy version of some (other) library - one that
>> contains a feature not used in kicad until somewhere between 3153 and 3212.
>>
>> Even though I upgraded only a few weeks ago, apt-get now wants
>> to update almost 1k packages (out of 3k), so I think I'll do a
>> full (dist-)upgrade tomorrow, and see if that makes a difference.
> 
> I've now done an apt-get dist-upgrade (1k packages updated,
> 2k unchanged), and have recompiled various kicad versions.
> 
> The problem still persists after the upgrade, so I've also done
> a binary search between BZR revisions 3153 and 3212 (and also
> did 3227), to determine the exact revision containing the break:
> 
>   bzr update -r revno:XXXX ; rsync ./ ../kicad-test ;
>   cd ../kicad-test ; make build ; cd build ; cmake ;
>   make ; make install ; kicad (start eeschema, test it ...)
> 
> The result is:
>   BZR 3153: Works
>   BZR 3180: Works
>   BZR 3195: Works
>   BZR 3203: Works
>   BZR 3204: Broken
>   BZR 3205: Broken
>   BZR 3208: Broken
>   BZR 3212: Broken
>   BZR 3227: Broken
> 
> So, it's something in BZR 3204 that hits me.
> 
> I've looked at a "diff -ur" between the 3203 and 3204 source trees,
> and have determined that it's the changes to common/basicframe.cpp
> that breaks the Library editor Save function for me:
> 
> --- === ---
> 
> diff -ur kicad-3203/common/basicframe.cpp kicad-3204/common/basicframe.cpp
> --- kicad-3203/common/basicframe.cpp    2011-11-11 12:20:58.000000000 +0100
> +++ kicad-3204/common/basicframe.cpp    2011-11-11 12:21:40.000000000 +0100
> @@ -543,25 +543,21 @@
>  {
>      wxString msg;
> 
> -    wxCHECK_MSG( aFileName.IsOk(), false, wxT( "Invalid file name object.  Bad
> programmer!" ) );
> +    wxCHECK_MSG( aFileName.IsOk(), false,
> +                 wxT( "File name object is invalid.  Bad programmer!" ) );
> +    wxCHECK_MSG( !aFileName.GetPath().IsEmpty(), false,
> +                 wxT( "File name object path <" ) + aFileName.GetFullPath() +
> +                 wxT( "> is not set.  Bad programmer!" ) );
> 
>      if( aFileName.IsDir() && !aFileName.IsDirWritable() )
>      {
>          msg.Printf( _( "You do not have write permissions to folder <%s>." ),
>                      GetChars( aFileName.GetPath() ) );
>      }
> -    else if( !aFileName.FileExists() )
> +    else if( !aFileName.FileExists() && !aFileName.IsDirWritable() )
>      {
> -        // Extract filename path, and if void, uses the CWD
> -        // because IsDirWritable does not like void path
> -        wxString filedir = aFileName.GetPath();
> -        if( filedir.IsEmpty() )
> -            filedir = wxGetCwd();
> -        if( !aFileName.IsDirWritable(filedir) )
> -        {
> -            msg.Printf( _( "You do not have write permissions to save file
> <%s> to folder <%s>." ),
> -                        GetChars( aFileName.GetFullName() ), GetChars( filedir
> ) );
> -        }
> +        msg.Printf( _( "You do not have write permissions to save file <%s> to
> folder <%s>." ),
> +                    GetChars( aFileName.GetFullName() ), GetChars(
> aFileName.GetPath() ) );
>      }
>      else if( aFileName.FileExists() && !aFileName.IsFileWritable() )
>      {
> 
> --- === ---
> 
> AFAICS, all other changes between 3203
> and 3204 are irrelevant for this problem.
> 
> The wxCHECK_MSG's are probably harmless (unless they
> have sideeffects ?), and why these changes hits me and
> not you (_any_ of you ?), I can't imagine, but ...:

Are you running release or debug builds?  I can see a potential problem in
release builds if for some reason an invalid file or path is pass to
IsWritable() where it would fail silently.  What I don't understand is how that
is happening unless something in the GTK file dialog is returning a file name
or path that fails the wxFileName IsOk() or the GetPath().IsEmpty() tests.  Try
running a debug build and see if you get one of the assertions.  If you do,
then the GTK file dialog is returning something that wxFileName doesn't like.
I could change the assertions to always show a message box if the file name is
not valid but I'ld rather not.  I see these as programming errors rather than
checking for an abnormal system condition like a disk full or a write
privileges issue.  A valid file name or path should be passed to IsWritable().

Thanks for doing the leg work on this.  I'll take another look at the library
editor code that calls IsWritable() to see if something is going on there.

Wayne

> 
> By reverting this file (and only this file) back to 3203, the Library
> editor Save function works again for me in 3204, just as in <= 3203.
> 
> As common/basicframe.cpp is unchanged between 3204 and 3227, I've
> also recompiled 3227 with the 3203 version of common/basicframe.cpp,
> and then Libedit Save works in 3227 also :-)
> 
> 
> Apart from the (catastrophic) failure to save the .lib file,
> there's one more difference between working and broken versions,
> as the "Save current library to disk" button behaves different.
> 
> On working versions, it looks like this:
> 
> * open eeschema
> * open Library editor
> * select a writable working lib, e.g. in the project dir
> * select an existing part
> # The Save button is inactive
> * make a change to the current part
> # The Save button is now active
> * Press "Save"
> Popup: "Include last component changes?"
> # The Save button is still active
> * Select Yes
> Popup: "Component "xxx" already exists. Change it?"
> # The Save button is now _inactive_ !-)
> * Select Yes
> Popup: "Modify libray file "xxx.lib"?"
> # The Save button is again active
> * Select Yes
> # The Save button is now _inactive_, and the lib file(s) is/are saved :-)
> 
> On broken versions, the Save button stays active (not grayed out)
> during the whole dialog (same three popup windows), and it also
> stays active after (where the file should have been saved).
> 
> []
>>>> Btw., "bzr -r revno:3001 update" _does_ downgrade the source to
>>>> BZR 3001, and the resulting kicad _is_ the older version (old style
>>>> icons and layout, and w/o the lib editor bug), but it presents
>>>> itself in the kicad titlebar as the newest BZR release ?
>>>> This was a bit confusing at first ...
>>>
>>> Run "make rebuild_cache" before running "make" to update the version file
>>> every time you update your source branch.  Otherwise the version file will
>>> be out of synch with the repo version.
>>
>> This was just a minor confusion, but as I copy (rsync) a clean and
>> freshly updated (or in this case, downgraded) bzr source tree to another
>> location for cmake + make, I found this behaviour a bit strange.
>>
>> "make rebuild_cache" sounds like the right cure, although
>> I don't quite understand why it should be necessary.
>> ... unless the bzr tree contains a file with the newest known release
>> no. in it, which then gets used by kicad (w/o the "make rebuild_cache").
>>
>>> Wayne
>>
>> Øyvind.
> 
> A "make rebuild_cache" doesn't make any difference to
> the reported kicad version (or to build/version.h):
> 
> $ bzr update -r revno:3203
> Tree is up to date at revision 3203 of branch
> http://bazaar.launchpad.net/~kicad-testing-committers/kicad/testing
> $ bzr help revno
> Purpose: Show current revision number.
> $ bzr revno
> 3227
> # No, that's the _latest_ revno, not the _current_ revno :-(
> 
> $ make build ; cd build
> $ DST=/usr/local/kicad-3203
> $ cmake -DCMAKE_INSTALL_PREFIX=$DST -DKICAD_TESTING_VERSION=ON ../
> Build testing (unstable) version of Kicad
> -- Check for installed OpenGL -- found
> -- Check for installed wxWidgets -- found
> -- Bazaar version control system version  found.
> -- Kicad Bazaar build version: (2011-11-11 BZR 3227)
> # No, wrong ...
> -- Configuring done
> -- Generating done
> -- Build files have been written to: /home/kicad/kicad-3203/build
> 
> $ make rebuild_cache
> Running CMake to regenerate build system...
> Build testing (unstable) version of Kicad
> -- Check for installed OpenGL -- found
> -- Check for installed wxWidgets -- found
> -- Bazaar version control system version  found.
> -- Kicad Bazaar build version: (2011-11-11 BZR 3227)
> # No, still wrong ...
> -- Configuring done
> -- Generating done
> -- Build files have been written to: /home/kicad/kicad-3203/build
> 
> As make rebuild_cache aparantly just reruns cmake, this failure is not
> surprising, as cmake also reports the wrong (i.e. most recent) version.
> 
> The most recent version is in .bzr/branch/last-revision, but I can't find
> the current version anywhere, except maybe in .bzr/checkout/dirstate.
> That file is binary, so grep finding 3203
> there could easy be a false positive, though.
> 
> Since "bzr revno" reports the latest revno, even though "bzr help revno"
> says it should be the current revno, maybe this is a bizarre (bzr ;-) bug ?
> 
> Øyvind.
> 
> **************************************************************************
> * Øyvind Aabling     E-mail : Oyvind.Aabling@xxxxxxxx    /~\ The ASCII   *
> * UNI-C Lyngby       Phone  : +45 35 87 88 89            \ / Ribbon      *
> * DTU Building 304   Phone  : +45 35 87 89 51 (direct)    X  Campaign    *
> * DK-2800 Lyngby     Fax    : +45 35 87 89 90            / \ Against     *
> * Denmark            Web    : http://www.uni-c.dk/           HTML Email! *
> **************************************************************************
> Quidquid latine dictum sit, altum viditur.