Bug 105609 - Python script provider does not reload modified embedded scripts
Summary: Python script provider does not reload modified embedded scripts
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Stephan Bergmann
URL:
Whiteboard: target:7.5.0
Keywords:
Depends on:
Blocks: Macro-Python
  Show dependency treegraph
 
Reported: 2017-01-30 10:22 UTC by jeanmarczambon
Modified: 2023-08-12 13:30 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
odt document illustrating the defect (14.70 KB, application/vnd.oasis.opendocument.text)
2018-08-30 11:33 UTC, jeanmarczambon
Details
Second odt file illustrating the defect (19.07 KB, application/vnd.oasis.opendocument.text)
2018-09-02 09:24 UTC, jeanmarczambon
Details
Third odt file illustrating the defect (19.52 KB, application/vnd.oasis.opendocument.text)
2018-09-02 13:50 UTC, jeanmarczambon
Details
Fourth odt file illustrating the defect (19.13 KB, application/vnd.oasis.opendocument.text)
2018-09-04 21:30 UTC, jeanmarczambon
Details
Reproduce bug in specific condition. (20.40 KB, application/vnd.oasis.opendocument.text)
2023-08-10 13:28 UTC, jeanmarczambon
Details

Note You need to log in before you can comment on or make changes to this bug.
Description jeanmarczambon 2017-01-30 10:22:38 UTC
The python script provider (pythoncript.py) checks if a module needs to be reloaded by comparing file date-time. This is done using the method "getDateTimeModified" of service css.ucb.SimpleFileAccess.
But this works only for external scripts : when executed on embedded scripts, "getDateTimeModified" always returns a null DateTime struct (let's assume this is not a bug) and therefore these scripts are never reloaded when modified.

For example, with the following instruction :
    # sfa = instance of css.ucb.SimpleFileAccess
    sfa.copy("/user/pyscript.py", "vnd.sun.star.tdoc:/1/Scripts/python/pyscript.py")
the embedded pyscript.py is updated and correctly loaded with any *new* instance of the script provider. However, the provider instance attached to the document will keep his reference to the first loaded version of the py file until next reopening. In other words, when an embedded script is bound to some control or toolbar element and executed once, it will never be reloaded even if modified.

I'm not totally sure but it seems that, instead of using "getDateTimeModified" in this particular case, it would be at least possible to rely on the method "isModified" of the document storage to track changes.
If this is correct, a kind of fix could look like this :

[old code, starting at line 426 of pythonscript.py]
    def getModuleByUrl( self, url ):
        entry =  self.modules.get(url)
        load = True
        lastRead = self.sfa.getDateTimeModified( url )
        if entry:
            if hasChanged( entry.lastRead, lastRead ):
                log.debug( "file " + url + " has changed, reloading" )
            else:
                load = False
        if load:
            [...]

[new code]
    def getModuleByUrl( self, url ):
        entry =  self.modules.get(url)
        load = True
        if entry:
            if url.startswith( "file:" ):
                lastRead = self.sfa.getDateTimeModified( url )
                if hasChanged( entry.lastRead, lastRead ):
                    log.debug( "file " + url + " has changed, reloading" )
                else:
                    load = False
            else:
                storage = self.scriptContext.doc.getDocumentStorage()
                load = storage.isModified()
                storage.commit()
        if load:
            [...]

Thanks in advance.
Comment 1 jeanmarczambon 2017-01-30 21:24:05 UTC
Sorry, there is a typo in the code above.
Correct [new code] would be :

[new code]
    def getModuleByUrl( self, url ):
        entry =  self.modules.get(url)
        load = True
        lastRead = self.sfa.getDateTimeModified( url )
        if entry:
            if url.startswith( "file:" ):
                if hasChanged( entry.lastRead, lastRead ):
                    log.debug( "file " + url + " has changed, reloading" )
                else:
                    load = False
            else:
                storage = self.scriptContext.doc.getDocumentStorage()
                load = storage.isModified()
                storage.commit()
        if load:
            [...]
Comment 2 Buovjaga 2017-02-06 08:56:30 UTC
Could you submit a patch for it: https://wiki.documentfoundation.org/Development/gerrit
Comment 3 jeanmarczambon 2017-02-06 09:35:21 UTC
Hello,

I never did this, but will have a look at the link you provided.
Thank you for your feedback.

JMz
Comment 4 jeanmarczambon 2017-02-07 08:33:06 UTC
Patch has been submitted. See https://gerrit.libreoffice.org/33981.
Comment 5 Buovjaga 2017-02-07 09:21:01 UTC
(In reply to jeanmarczambon from comment #4)
> Patch has been submitted. See https://gerrit.libreoffice.org/33981.

Great :)

I forgot to mention that adding tdf#105609 to the commit message would be good for some automation etc., but it's not the end of the world.
Comment 6 Xisco Faulí 2018-01-30 10:41:23 UTC
Dear jeanmarczambon@gmail.com,
This bug has been in ASSIGNED status for more than 3 months without any
activity. Resetting it to NEW.
Please assigned it back to yourself if you're still working on this.
Comment 7 jeanmarczambon 2018-01-31 11:01:56 UTC
The patch is still waiting for review...
Comment 8 Xisco Faulí 2018-05-29 09:19:10 UTC
Patch Restored!
Comment 9 Xisco Faulí 2018-08-29 09:42:40 UTC
Ask reviewers to review it...
Comment 10 Julien Nabet 2018-08-29 21:09:27 UTC
Please prefer attaching a file in the bugtracker instead of trying on gerrit.

Retrieving your second file, here's what I got:
1) Open the file, I see message = "TWO"
2) Click Test button, I get "ONE" in a message box
3) Click Ok to close the message box
4) Click "Substitute Code", it doesn't change the content of the frame, I still got message = "TWO"
5) Clic Test button, I get again "ONE".

BTW, I noticed I can't edit the Python macro, only "Execute", "Close" and "Help" buttons are enabled.
"Create", "Edit", "Rename" and "Remove" buttons are disabled.
Comment 11 jeanmarczambon 2018-08-30 11:32:09 UTC
> 1) Open the file, I see message = "TWO" -
This is the new text, to be embedded with the "substitute" button

> 2) Click Test button, I get "ONE" in a message box
> 3) Click Ok to close the message box
> 4) Click "Substitute Code", it doesn't change the content of the frame, I
> still got message = "TWO"
I was unclear, sorry: this frame text is used to replace the existing one, that prints "ONE"

> 5) Clic Test button, I get again "ONE".
This is the problem: you should get "TWO". If you save and reload the document, pressing the "TEST" button will show "TWO", confirming that the module was actually edited.

> BTW, I noticed I can't edit the Python macro, only "Execute", "Close" and
> "Help" buttons are enabled.
> "Create", "Edit", "Rename" and "Remove" buttons are disabled.
For now the python script provider doesn't implement these functions.
Comment 12 jeanmarczambon 2018-08-30 11:33:07 UTC
Created attachment 144553 [details]
odt document illustrating the defect
Comment 13 Julien Nabet 2018-09-01 21:22:48 UTC
Let's start again with the attached example.


1) opened the doc and see in the definition: "message = "THREE""
2) clicked "Test" button
=> I got a message box with "THREE"
3) clicked "Ok"
=> message box closed
4) clicked "Substitute code" button
=> nothing seems to change
5) click "Test" button again
=> I got a message box with "THREE"
6) selected File reload
=> it reloads the file proposing to enable or disable macro (I click enable)
7) clicked Test button
big error message:
A Scripting Framework error occurred while running the Python script vnd.sun.start.script:message.py$msgbox?
language=Python&location=document.

Message: <class 'NameError'>: name 'ox' is not defined
File "/home/julien/lo/libreoffice/instdir/program/pythonscript.py", line 1021, in getScript mod = self.provCtx.getModuleByUrl( fileUri 
File "/home/julien/lo/libreoffice/instdir/program/pythonscript.py", line 458, in getModuleByUrl
exec(code, entry.module.__dic__)
File "vnd.sun.star.tdoc:/2/Scripts/python/message.py", line 15, in <module>
Comment 14 jeanmarczambon 2018-09-02 09:24:54 UTC
Created attachment 144605 [details]
Second odt file illustrating the defect

I feel ashamed, I picked up the first file in the wrong directory.
This time the file is the right one. Steps to reproduce are detailed inside.
Comment 15 Julien Nabet 2018-09-02 11:43:28 UTC
I could reproduce the pb with your second file.

I gave a try with https://gerrit.libreoffice.org/#/c/33981/ and so copied scripting/source/pyprov/pythonscript.py from the patch in instdir/program/ but I still reproduce this.

Did I miss something?
Comment 16 jeanmarczambon 2018-09-02 13:50:17 UTC
Created attachment 144612 [details]
Third odt file illustrating the defect

In my previous file the DocumentStorage was not set as modified. I correct this and it should work as expected with the patched pythonscript.py file.

BUT: I realize that, if the document is saved between the script update and the next call to it, the DocumentStorage is reset to "not modified", which means the script won 't be reloaded :( .

So we need to find a way to check if the script has changed compared to the one already loaded, regardless of the DocumentStorage state. But have still no idea how...
Comment 17 Julien Nabet 2018-09-04 19:58:23 UTC
With last file retrieved, I got "THREE" at the beginning and at the end before applying the patch.
Comment 18 jeanmarczambon 2018-09-04 21:30:13 UTC
Created attachment 144681 [details]
Fourth odt file illustrating the defect

I think that test file will drive me crazy...
I tested this last one about a dozen of time, hope it is the right one.
Comment 19 Julien Nabet 2018-09-05 19:19:38 UTC
Ok I could reproduce the problem with this file and master sources updated today on pc Debian x86-64.
Comment 20 Xisco Faulí 2018-12-05 09:49:30 UTC
Dear jeanmarczambon,
This bug has been in ASSIGNED status for more than 3 months without any
activity. Resetting it to NEW.
Please assigned it back to yourself if you're still working on this.
Comment 21 jeanmarczambon 2019-01-31 08:00:27 UTC
It's seems definitly not possible to use a change flag for identifying any modification in an embedded script.

So I proposed another (easy) way to workaround this: making use of a global variable in the pythonscript.py code, accessible from the client which is modifying the script and allowing it to force the reload.
That client could use it this way:

    def update_embedded_script(sfa, sourceurl, scripturi):
        '''embed script in document and inform 
        pythonscript.py about the changes'''
        # sfa is an instance of css.ucb.SimpleFileAccess
        sfa.copy(sourceurl, scripturi)
        import pythonscript
        pythonscript.g_forcedReloadUrls.add(scripturi)

See details here: https://gerrit.libreoffice.org/33981.
Comment 22 Xisco Faulí 2019-05-07 08:09:18 UTC
Patch restored in gerrit
Comment 23 Commit Notification 2022-10-18 10:27:25 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d4512d391bc7f96af9b66a8ee44f3f2be38dc5ec

tdf#105609: Support DateModified for updated tdoc stream contents

It will be available in 7.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 24 jeanmarczambon 2023-08-10 13:28:09 UTC
Created attachment 188910 [details]
Reproduce bug in specific condition.

Thank you very much for the fix.

I first thought all was fixed, but just a realized that it doesn't work if the python script is updated from an external file. The use case is illustrated in Apso, which allows to edit embedded file by copying it in a temp directory before embedding it back into the document.

I attach a new file that mimics this behaviour and reproduce the bug.

Thanks again.
Comment 25 Julien Nabet 2023-08-10 16:09:14 UTC
(In reply to jeanmarczambon from comment #24)
> Created attachment 188910 [details]
> Reproduce bug in specific condition.
> 
> Thank you very much for the fix.
> 
> I first thought all was fixed, but just a realized that it doesn't work if
> the python script is updated from an external file. The use case is
> illustrated in Apso, which allows to edit embedded file by copying it in a
> temp directory before embedding it back into the document.
> 
> I attach a new file that mimics this behaviour and reproduce the bug.
> 
> Thanks again.

Please open a new bugtracker about this new pb since the initial pb has been fixed.