[Issue 19852] Cached settings and moving images
Hi all, I was checking issue https://pharo.fogbugz.com/f/cases/19852 which is about the problems that arise when we move an image of location, and moreover when we move it between different platforms (windows to linux or mac for example). The problem are dangling file references pointing to invalid locations. We have found 4 of them: - SystemResolver localdirectory is cached in a class variable - GTPlaybook caches the cache and stash directories in class variables - OMSessionStore retrieves at some point during startup an invalid file reference from the (wrongly cached) local directory - IceLibgitRepository Sharedrepositorieslocation is also cached All these are actually caused by settings, whose behavior is not defined when an image is moved. There are several strange issues like, if we store settings, the local directory is stored, and then all images will (wrongly) use the same pharo-local directory of the first image. This is particularly annoying when using the launcher for example :). Now, we can leave this as it is right now and just move it to pharo8. A quick fix for Pharo7 would be removing those settings and avoiding caching, but that is probably very disruptive too... Opinions? Guille
Thanks guille for this analysis. I would move it to Pharo 8.0 Now I think that we should revive your analysis on state because I would love to have first class objects with a good and nice logic for such cases and others. The lifetime of a variable looks more complex than it looks.
On 8 Jan 2019, at 12:19, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi all,
I was checking issue https://pharo.fogbugz.com/f/cases/19852 <https://pharo.fogbugz.com/f/cases/19852> which is about the problems that arise when we move an image of location, and moreover when we move it between different platforms (windows to linux or mac for example).
The problem are dangling file references pointing to invalid locations.
We have found 4 of them:
SystemResolver localdirectory is cached in a class variable GTPlaybook caches the cache and stash directories in class variables OMSessionStore retrieves at some point during startup an invalid file reference from the (wrongly cached) local directory IceLibgitRepository Sharedrepositorieslocation is also cached
All these are actually caused by settings, whose behavior is not defined when an image is moved. There are several strange issues like, if we store settings, the local directory is stored, and then all images will (wrongly) use the same pharo-local directory of the first image. This is particularly annoying when using the launcher for example :).
Now, we can leave this as it is right now and just move it to pharo8. A quick fix for Pharo7 would be removing those settings and avoiding caching, but that is probably very disruptive too...
Opinions?
Guille
Hi Guille,
On Jan 8, 2019, at 3:19 AM, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi all,
I was checking issue https://pharo.fogbugz.com/f/cases/19852 which is about the problems that arise when we move an image of location, and moreover when we move it between different platforms (windows to linux or mac for example).
The problem are dangling file references pointing to invalid locations.
We have found 4 of them:
SystemResolver localdirectory is cached in a class variable GTPlaybook caches the cache and stash directories in class variables OMSessionStore retrieves at some point during startup an invalid file reference from the (wrongly cached) local directory IceLibgitRepository Sharedrepositorieslocation is also cached
All these are actually caused by settings, whose behavior is not defined when an image is moved. There are several strange issues like, if we store settings, the local directory is stored, and then all images will (wrongly) use the same pharo-local directory of the first image. This is particularly annoying when using the launcher for example :).
Now, we can leave this as it is right now and just move it to pharo8. A quick fix for Pharo7 would be removing those settings and avoiding caching, but that is probably very disruptive too...
Opinions?
While a âproperâ fix might involve adding symbolic names that can be cross-platform (including using environment variables, etc) and that would indeed mean waiting for Pharo 8, surely something simple can be fine in the mean time. Why not have those classes maintain a âcurrent platformâ class var and a current directory on start up flush the file names if either the current platform or the current directory has changed? One would of course have to test the platform before the directory. Or even simpler, if the image name and directory are saved as strings (eg in Smalltalk as PreviousImageAndDirectory := { ... } and thereâs an accessor such as imageNameAndOrDirectoryHasChanged) then the file names can easily be changed on start up. Getting the startup order right might be a little tricky but surely not that hard in this case. On a related note I think time boxed releases are a bad idea. A system is ready when it is ready, based on proper acceptance criteria, such as tests, user experience reports, etc. when it is clearly broken it is clearly broken. Releasing a system that is clearly broken helps nobody. (I say that as the US prepares to enter the 2020 election cycle...)
Guille
Hi Eliot, Thanks for looking into this, see inline. On Tue, Jan 8, 2019 at 4:46 PM Eliot Miranda <eliot.miranda@gmail.com> wrote:
Hi Guille,
On Jan 8, 2019, at 3:19 AM, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi all,
I was checking issue https://pharo.fogbugz.com/f/cases/19852 which is about the problems that arise when we move an image of location, and moreover when we move it between different platforms (windows to linux or mac for example).
The problem are dangling file references pointing to invalid locations.
We have found 4 of them:
- SystemResolver localdirectory is cached in a class variable - GTPlaybook caches the cache and stash directories in class variables - OMSessionStore retrieves at some point during startup an invalid file reference from the (wrongly cached) local directory - IceLibgitRepository Sharedrepositorieslocation is also cached
All these are actually caused by settings, whose behavior is not defined when an image is moved. There are several strange issues like, if we store settings, the local directory is stored, and then all images will (wrongly) use the same pharo-local directory of the first image. This is particularly annoying when using the launcher for example :).
Now, we can leave this as it is right now and just move it to pharo8. A quick fix for Pharo7 would be removing those settings and avoiding caching, but that is probably very disruptive too...
Opinions?
While a âproperâ fix might involve adding symbolic names that can be cross-platform (including using environment variables, etc)
Sure, we have that in FileSystem with file locators. The thing is that people may be customizing those paths using the preference/settings to a very-specific directory in their system.
and that would indeed mean waiting for Pharo 8, surely something simple can be fine in the mean time. Why not have those classes maintain a âcurrent platformâ class var and a current directory on start up flush the file names if either the current platform or the current directory has changed? One would of course have to test the platform before the directory. Or even simpler, if the image name and directory are saved as strings (eg in Smalltalk as PreviousImageAndDirectory := { ... }
and thereâs an accessor such as imageNameAndOrDirectoryHasChanged) then the
file names can easily be changed on start up. Getting the startup order right might be a little tricky but surely not that hard in this case.
Sure, I've lately done something similar to (re)calculate the offsets of FFI structures on startup only if the platform has changed, which we required to have specially windows 64 FFI up and running. currentCompilationPlatform := { OSPlatform current family . Smalltalk vm wordSize }. LastCompilationPlatform = currentCompilationPlatform ifTrue: [ ^ self ]. LastCompilationPlatform := currentCompilationPlatform. ... https://github.com/pharo-project/pharo/pull/1985 However for this particular issue it's not clear in my mind yet if comparing platform, image name and location is enough. Because this looks like a so common issue for many other settings (and global/class variables too), not only those containing file references!
On a related note I think time boxed releases are a bad idea. A system is ready when it is ready, based on proper acceptance criteria, such as tests, user experience reports, etc. when it is clearly broken it is clearly broken. Releasing a system that is clearly broken helps nobody. (I say that as the US prepares to enter the 2020 election cycle...)
Guille
-- Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - *http://www.cnrs.fr <http://www.cnrs.fr>* *Web:* *http://guillep.github.io* <http://guillep.github.io> *Phone: *+33 06 52 70 66 13
Le mar. 8 janv. 2019 à 17:36, Guillermo Polito <guillermopolito@gmail.com> a écrit :
Hi Eliot,
Thanks for looking into this, see inline.
On Tue, Jan 8, 2019 at 4:46 PM Eliot Miranda <eliot.miranda@gmail.com> wrote:
Hi Guille,
On Jan 8, 2019, at 3:19 AM, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi all,
I was checking issue https://pharo.fogbugz.com/f/cases/19852 which is about the problems that arise when we move an image of location, and moreover when we move it between different platforms (windows to linux or mac for example).
The problem are dangling file references pointing to invalid locations.
We have found 4 of them:
- SystemResolver localdirectory is cached in a class variable - GTPlaybook caches the cache and stash directories in class variables - OMSessionStore retrieves at some point during startup an invalid file reference from the (wrongly cached) local directory - IceLibgitRepository Sharedrepositorieslocation is also cached
All these are actually caused by settings, whose behavior is not defined when an image is moved. There are several strange issues like, if we store settings, the local directory is stored, and then all images will (wrongly) use the same pharo-local directory of the first image. This is particularly annoying when using the launcher for example :).
Now, we can leave this as it is right now and just move it to pharo8. A quick fix for Pharo7 would be removing those settings and avoiding caching, but that is probably very disruptive too...
Opinions?
While a âproperâ fix might involve adding symbolic names that can be cross-platform (including using environment variables, etc)
Sure, we have that in FileSystem with file locators. The thing is that people may be customizing those paths using the preference/settings to a very-specific directory in their system.
and that would indeed mean waiting for Pharo 8, surely something simple can be fine in the mean time. Why not have those classes maintain a âcurrent platformâ class var and a current directory on start up flush the file names if either the current platform or the current directory has changed? One would of course have to test the platform before the directory. Or even simpler, if the image name and directory are saved as strings (eg in Smalltalk as PreviousImageAndDirectory := { ... }
and thereâs an accessor such as imageNameAndOrDirectoryHasChanged) then
the file names can easily be changed on start up. Getting the startup order right might be a little tricky but surely not that hard in this case.
Sure, I've lately done something similar to (re)calculate the offsets of FFI structures on startup only if the platform has changed, which we required to have specially windows 64 FFI up and running.
currentCompilationPlatform := { OSPlatform current family . Smalltalk vm wordSize }. LastCompilationPlatform = currentCompilationPlatform ifTrue: [ ^ self ]. LastCompilationPlatform := currentCompilationPlatform. ...
https://github.com/pharo-project/pharo/pull/1985
#me too :) http://source.squeak.org/FFI/FFI-Kernel-nice.49.diff
However for this particular issue it's not clear in my mind yet if
comparing platform, image name and location is enough. Because this looks like a so common issue for many other settings (and global/class variables too), not only those containing file references!
On a related note I think time boxed releases are a bad idea. A system is ready when it is ready, based on proper acceptance criteria, such as tests, user experience reports, etc. when it is clearly broken it is clearly broken. Releasing a system that is clearly broken helps nobody. (I say that as the US prepares to enter the 2020 election cycle...)
Guille
--
Guille Polito
Research Engineer
Centre de Recherche en Informatique, Signal et Automatique de Lille
CRIStAL - UMR 9189
French National Center for Scientific Research - *http://www.cnrs.fr <http://www.cnrs.fr>*
*Web:* *http://guillep.github.io* <http://guillep.github.io>
*Phone: *+33 06 52 70 66 13
participants (4)
-
ducasse -
Eliot Miranda -
Guillermo Polito -
Nicolas Cellier