@clean nodes stopped updating

[Joe Orr]

I'm going back and forth between Leo and Webstorm. Until today, when I made a change in a @clean file in Webstorm, Leo would pop up a message saying that the file has been changed and update its copy. That just stopped happening. Now Leo doesn't update the node unless I close and open the Leo file.

I'm not aware of any changes I made in my setup that could have caused this.

After it started happening I did the following:

1. Deleted my Leo install and reinstalled from git, version is now 5.5.

2. Verified that Webstorm is updating the file on the filesystem.

3. Tried deleting and recreating the @clean node.

Refresh from disk works, also, I do get a warning when I try to save changes in Leo version and target file has other changes.

Using Mac OS 10.

Anyone have an idea how to fix this?

Joe

[vitalije]

I am not certain but it could be caused with the fixing of other bug I have reported recently in checkTimeStamp method. It used to report non-existing files in some cases as changed and to fix that the above method was changed to return True by default which means "file not changed". Try to test it with the version before this fix.
The change was introduced in b7638799d6 commit.
HTH Vitalije

[Edward K. Ream]

On Sat, Apr 8, 2017 at 9:58 AM, vitalije <vita...@gmail.com> wrote:

The change was introduced in b7638799d6 commit.
HTH Vitalije

​If that doesn't work, please use git bisect to isolate the offending commits.  I have done considerable hacking recently in this area.

Edward

[Joe Orr]

I tried rolling back to the commit before b7638799d6, no luck.

In fact, now it is worse, when I make a change in Webstorm and then go to Leo, not only is there no warning, any changes I then make via Leo clobber the Webstorm file with no warning.

And it is still doing this after I reset to the current version.

I'm not sure what to do with git bisect, I don't have any version (on Mac, OS 10.12.4) for which this isn't happening. It started happening for no reason that I can tell while I was still on version 5.3.

I think this needs to get sorted out before I tell people about the HTML5 viewer because I want to publicize this feature in particular.

I don't know Python but I could maybe try to step through debug it if someone could point me to the code lines that are involved here. 

Anyone else using Mac and not having problems? If so, what version are you using?

I'm not having this problem on Windows (version 5.3b) but having other problems with Windows version, will send separate email after this is sorted out. (For one thing, unable to install latest on Windows).

Joe

[Edward K. Ream]

On Tue, Apr 11, 2017 at 4:59 AM, Joe Orr <joe...@gmail.com> wrote:

I tried rolling back to the commit before b7638799d6, no luck.

In fact, now it is worse, when I make a change in Webstorm and then go to Leo, not only is there no warning, any changes I then make via Leo clobber the Webstorm file with no warning.

​Please file a bug report here so I can track this issue.  I'll attempt a fix today.

Edward

[Edward K. Ream]

On Tuesday, April 11, 2017 at 5:08:58 AM UTC-5, Edward K. Ream wrote:

> Please file a bug report here

Never mind.  I just created #472. I'll attempt a fix later today.

Edward

[Edward K. Ream]

On Tuesday, April 11, 2017 at 10:36:30 AM UTC-5, Edward K. Ream wrote:

> I'll attempt a fix later today.

As I noted in #472, everything works for me on Windows. So this is the typical long-distance debugging mystery.  Here are my suggestions.

1. [Crucial]  Make sure you are running Leo from a console.  That way you can see messages that might not appear in Leo's log pane.

2. Pull rev b91bc76.  It contains improved traces for ExternalFilesController.on_idle.  This is the code that periodically checks for changed files.  Depending on how many files your .leo file contains, this may take awhile.

3. Enable tracing in efc.on_idle by setting trace = True.  You should see regular traces when you restart Leo.

4. Try running Leo with the --no-plugins command-line option.

5. If none of this works there is not much option but to use git bisect.  Go back as far as you need to until you find a version that works.

Edward

[Edward K. Ream]

On Tuesday, April 11, 2017 at 3:34:35 PM UTC-5, Edward K. Ream wrote:

On Tuesday, April 11, 2017 at 10:36:30 AM UTC-5, Edward K. Ream wrote:

> I'll attempt a fix later today.

As I noted in #472, everything works for me on Windows. So this is the typical long-distance debugging mystery.  Here are my suggestions.

One more thing.  Apparently the pop-up that says that says that a file has been changed may not always be visible.  I'll see what I can do to bring it to the front more reliably.

Edward

[Edward K. Ream]

On Tuesday, April 11, 2017 at 3:34:35 PM UTC-5, Edward K. Ream wrote:

As I noted in #472, everything works for me on Windows. So this is the typical long-distance debugging mystery.  Here are my suggestions.

And here are a few more:

1. In efc.on_idle, try changing the "if 1:" statement to "if 0:".  This will revert to older code.  It's quicker and simpler if you only have a few files to check.

2. Enable other traces, especially in efc.has_changed.  Make sure this method is being called periodically, and check that it returns the proper value.  You might also want to enable traces in various other idle_check_* methods.

HTH.  We'll get to the bottom of this.

Edward

[Edward K. Ream]

On Tuesday, April 11, 2017 at 3:40:38 PM UTC-5, Edward K. Ream wrote:

> HTH.  We'll get to the bottom of this.

And be sure to verify that your tool chain is actually updating the files you think it is.

The symptom that you must reload Leo to see changed files is quite strange.  It argues that your tool chain is updating files, but we have to check everything.

We have to run Leo in a console so we can see all exceptions, especially uncaught exceptions.

For sure something strange is going on. A silently hidden exception, an invisible popup, files not actually being saved, on_idle checks not happening at all...

Edward

[vitalije]

I have bisected and found that the culprit is change in default setting.
Here is relevant commit:

2a0eca1635457b379e6e1cbfab6595e55afbd635 is the first bad commit
commit 2a0eca1635457b379e6e1cbfab6595e55afbd635
Author: Edward K. Ream <edreamleo@gmail.com>
Date:   Fri Dec 16 15:06:40 2016 -0500

    Set @bool check_for_changed_external_files = False by default, and reference #262,
    @bool check_for_changed_external_files may hang on network files.
   
    Leo build: 20161216150641

:040000 040000 a946721dce8ae22363024fd5eedd9caea780e930 81c2141e6c4404f6dab63b3d93971e9d7f930551 M    leo

[vitalije]

Just add :

@bool check_for_changed_external_files = True

to myLeoSettings.leo in your configuration file.
Vitalije

[Edward K. Ream]

> I have bisected and found that the culprit is change in default setting.

...

> Just add @bool check_for_changed_external_files = True to myLeoSettings.leo


Excellent sleuthing. Thank you Vitalije.

Edward

[Edward K. Ream]

On Wednesday, April 12, 2017 at 4:17:12 AM UTC-5, Edward K. Ream wrote:

> I have bisected and found that the culprit is change in default setting.
...

> Just add @bool check_for_changed_external_files = True to myLeoSettings.leo


@Joe Orr: Please report any further problems you might have.

Rev 0fb4cec improves traces and comments to make clear to me and future maintainers what is happening.

Here is the checkin log:

QQQ
Improved comments and traces in efc.on_idle so as to avoid future confusion. All tests pass.
   
Refactored the is_enabled test out of the helper and into on_idle, to make clearer what is happening.
   
Changed and tested code in the disable "else" clause so that it works like the code in the "if 1:" clause.
QQQ

Edward

[Edward K. Ream]

On Wed, Apr 12, 2017 at 5:20 AM, Edward K. Ream <edre...@gmail.com> wrote:

Rev 0fb4cec improves traces and comments to make clear to me and future maintainers what is happening.

​I had a lovely morning walk today, watching the crew teams train on Lake Mendota.

At the end of the walk I realized that it was a pretty big mistake to change the default of @bool check_for_changed_external_files from True to False. Joe Orr's confusion stems directly from this mistake.  However, having made that mistake it seems we are stuck with it. It might be a worse mistake to change the default yet again.

Yes, checking network drives can be dangerous, as claimed in #262: @bool check_for_changed_external_files may hang on network files However, the @bool node already contains an adequate warning, and references #262.

In short, it's time to close #262.

Any comments?

Edward

[vitalije]

Any comments?

Edward

I think it would be useful to have some list of such changes in default settings with the commits they were introduced and short description of the reason why the change happened. When researching why some function of Leo got broken, it should be the first thing to check. That's at least what I have learned from dealing with this bug.
Well, as I write this I am realizing that it would be possible to just use git blame on leoSettings.leo to see when some settings were introduced/changed. Maybe we can formulate a procedure for hunting bugs where this check could be one of the first steps.

...

I have just tried to see the blame of leoSettings.leo in browser on github.com and it is too hard work for browser. I think that the @settings tree should be kept in some external file under git control. That way blame function of github.com would produce much smaller page.

I wasn't very familiar with git bisect functionality and it took me some time to find my way through. At the end I think it is not overly complicated. Should anyone else try to do it, here are some tips. First I have cloned my Leo folder to /tmp/leo-trunk  (* could be any temporary folder). Then I made script (leo2) for launching Leo from that temporary location. After that I ran ```git checkout c235ff4```. That was the second last release (5.4.1). Then I ran leo2 and created a Leo file in /tmp/bug.leo with one @clean node in it. Then I made script that changes the content of the external @clean file. And I have tested that commit c235ff4 doesn't contain bug. Then I have issued following command: ```git bisect start``` and ```git bisect good```. After that ```git checkout <latest commit>``` and then again ```leo2 /tmp/bug.leo```, followed by executing the script to change @clean file. Leo didn't notice that file was changed. That is why I issued ```git bisect bad```. After that git has computed expected number of steps to find the problematic commit and automatically checked out some commit in the middle. The following steps were quite easy to perform:
1. leo2 /tmp/bug.leo        #opens leo
2. run script to change @clean file
3. if Leo noticed that file was changed
    3.1 git bisect good
    if Leo didn't notice that file was changed
    3.2 git bisect bad
4. close Leo

I kept repeating this four steps until git announced that the first commit containing bug was ... I had 3 terminals open: one to start leo2, another to start script for changing @clean file, and the third terminal was for git commands.

English is not my primary language. Maybe someone can reformulate this in concise and simple receipt, so that anyone who maybe contemplating to help with git bisecting Leo's code-base in order to find source of some bug, has clear and simple explanation of the process.

HTH Vitalije

[Edward K. Ream]

On Wed, Apr 12, 2017 at 12:56 PM, vitalije <vita...@gmail.com> wrote:

I think it would be useful to have some list of such changes in default settings with the commits they were introduced and short description of the reason why the change happened.

​Such changes are rare.  In fact, the change to @bool check_for_changed_external_files is the only one I can think of.  There are obvious reasons not to change defaults.

I'm dithering about @bool check_for_changed_external_files.  On the one hand, it seems unreasonable not to enable it by default.  It's the expected behavior for most editors and IDE's.  Otoh, there are rare performance problems with network drives.​

Earlier, I said it was a mistake to change the default, and it might be a bigger mistake to change it back.  Now I'm not so sure.  It seems bad not to enable checks by default. For those with problems with network files, setting @bool check_for_changed_external_files to False should be good enough.

 

When researching why some function of Leo got broken, it should be the first thing to check. That's at least what I have learned from dealing with this bug.

​In my experience, documentation almost never helps, and is almost never up to date.
​

 

Well, as I write this I am realizing that it would be possible to just use git blame on leoSettings.leo to see when some settings were introduced/changed. Maybe we can formulate a procedure for hunting bugs where this check could be one of the first steps

​We never want to change the defaults of settings.  Ever.  Imo, this is the take-away message from this experience.

 I have just tried to see the blame of leoSettings.leo in browser on github.com and it is too hard work for browser. I think that the @settings tree should be kept in some external file under git control. That way blame function of github.com would produce much smaller page.

​An interesting idea. It's one example of the difficulty of diffing .leo (xml) files.​

 

I wasn't very familiar with git bisect functionality and it took me some time to find my way through. At the end I think it is not overly complicated. Should anyone else try to do it, here are some tips. First I have cloned my Leo folder to /tmp/leo-trunk  (* could be any temporary folder). Then I made script (leo2) for launching Leo from that temporary location. After that I ran ```git checkout c235ff4```. That was the second last release (5.4.1). Then I ran leo2 and created a Leo file in /tmp/bug.leo with one @clean node in it. Then I made script that changes the content of the external @clean file. And I have tested that commit c235ff4 doesn't contain bug. Then I have issued following command: ```git bisect start``` and ```git bisect good```. After that ```git checkout <latest commit>``` and then again ```leo2 /tmp/bug.leo```, followed by executing the script to change @clean file. Leo didn't notice that file was changed. That is why I issued ```git bisect bad```. After that git has computed expected number of steps to find the problematic commit and automatically checked out some commit in the middle. The following steps were quite easy to perform:
1. leo2 /tmp/bug.leo        #opens leo
2. run script to change @clean file
3. if Leo noticed that file was changed
    3.1 git bisect good
    if Leo didn't notice that file was changed
    3.2 git bisect bad
4. close Leo

​I would have done something similar.  But you don't have to use a temp folder​. You can do the git bisect "in place", in your normal leo-editor folder. When you are done, git will leave your folder just as it was.

Edward

[vitalije]

​I would have done something similar.  But you don't have to use a temp folder​. You can do the git bisect "in place", in your normal leo-editor folder. When you are done, git will leave your folder just as it was.

Edward

Yes, I knew it would, but in my Leo folder there are myLeoSettings.leo, and some plug-ins that can affect the behavior  of the Leo. If I have had  @bool check_for_changed_external_files set to True in my Leo settings, I wouldn't be able to reproduce the bug at all. Besides, I wanted to have normal operational Leo at hand, ready to create and edit script files used in experiment. Perhaps, for some (or even many) bugs, it doesn't really matter, but with some more dangerous bug it would.
Vitalije

[vitalije]

I'm dithering about @bool check_for_changed_external_files.  On the one hand, it seems unreasonable not to enable it by default.  It's the expected behavior for most editors and IDE's.  Otoh, there are rare performance problems with network drives.​

Is it possible for Leo to discover if some file path is in fact path to network drive? If possible then perhaps we can have the default settings set to True and refuse to check network paths and maybe warn user that those paths are not going to be checked.
Vitalije

[Edward K. Ream]

On Thu, Apr 13, 2017 at 2:13 AM, vitalije <vita...@gmail.com> wrote:

​I would have done something similar.  But you don't have to use a temp folder​. You can do the git bisect "in place", in your normal leo-editor folder. When you are done, git will leave your folder just as it was.

 

Yes, I knew it would, but in my Leo folder there are myLeoSettings.leo, and some plug-ins that can affect the behavior  of the Leo. If I have had  @bool check_for_changed_external_files set to True in my Leo settings, I wouldn't be able to reproduce the bug at all.

​Quite right.  I was mistaken.​

 

​Edward

[Edward K. Ream]

On Thu, Apr 13, 2017 at 2:18 AM, vitalije <vita...@gmail.com> wrote:

I'm dithering about

​​

@bool check_for_changed_external_files.  On the one hand, it seems unreasonable not to enable it by default.  It's the expected behavior for most editors and IDE's.  Otoh, there are rare performance problems with network drives.​

Is it possible for Leo to discover if some file path is in fact path to network drive?

​Lately, whenever I have any such question, I google it. In this case: "how to tell if a windows drive is a network drive" yields this page. The most relevant link on the page seems to be one describing the GetDriveType.

Leo's checking code might be able to call this low-level function. Imo, this is not necessary, for two reasons.  First,

​

@bool check_for_changed_external_files to False should suffice in most cases.  If not, a new directive, say, @no-file-check, possibly created in a plugin, would be the more Leonine solution.  This directive would allow the user to enable or disable checking for specific external files.

Edward

[Joe Orr]

Thanks for figuring this out!

That did indeed fix it on my Mac.

On Windows I'm using 5.3 (was not able to get anything else to work, will write separate post) and this doesn't seem to be a problem with that one.

I think not having this be the default is pretty dangerous. I clobbered some source files several times before getting this setting back.

Joe