[Glass] Git based repositories (package-cache) problem when moving extent to another stone
Mariano Martinez Peck via Glass
glass at lists.gemtalksystems.com
Tue Oct 27 16:00:57 PDT 2015
Yes, it does work too.
On Tue, Oct 27, 2015 at 6:48 PM, Dale Henrichs <
dale.henrichs at gemtalksystems.com> wrote:
>
>
> On 10/27/2015 01:35 PM, Mariano Martinez Peck wrote:
>
>
>
> On Tue, Oct 27, 2015 at 4:39 PM, Dale Henrichs <
> dale.henrichs at gemtalksystems.com> wrote:
>
>>
>>
>> On 10/27/2015 11:56 AM, Mariano Martinez Peck wrote:
>>
>>
>>
>> On Tue, Oct 27, 2015 at 3:22 PM, Dale Henrichs <
>> <dale.henrichs at gemtalksystems.com>dale.henrichs at gemtalksystems.com>
>> wrote:
>>
>>>
>>>
>>> On 10/27/2015 10:50 AM, Mariano Martinez Peck wrote:
>>>
>>> Dale,
>>>
>>> While that more robust #exists does help in the sense that the user gets
>>> a much better error message, I think it's not enough. So now I get this
>>> message:
>>>
>>> ...RETRY->BaselineOfSeaside3
>>> ...RETRY->BaselineOfSeaside3
>>> gofer repository error: 'GoferRepositoryError: UserDefinedError: Disk
>>> error: for path:
>>> /opt/gemstoneAdditions/GsDevKit_home/server/stones/gs_329/logs/github-cache/GsDevKit/Seaside31/v3.1.4.2-gs/GsDevKit-Seaside31-55f1bac/repository'...ignoring
>>> ...FAILED->BaselineOfSeaside3
>>>
>>> Right, but aren't we trying to figure out why the load works when you
>>> remove the offending directory
>>>
>>
>> Yes, my load works when we remove the offending directory.
>>
>>
>>> but does not work when you explicitly set the cache directory to nil:
>>>
>>> MCGitHubRepository cacheDirectory: nil
>>>
>>
>> Yes, this does not fix the problem. What I found out is that besides
>> that, I could do:
>>
>> MCRepositoryGroup default repositoriesDo: [:rep | rep flushCache ].
>>
>> That WOULD have fixed my problem. However #flushCache fails too because
>> of the same error. So we are not even able to #flushCahe with current code.
>>
>> Ah this is the piece that I was missing ... I had misunderstood that this
>> did not work either ... and I thought you were trying to track down why
>> this did not clear the cache ... but I understand now that when you run
>> this code, you get a permissions related error and so you "cannot flush the
>> caches"
>>
>>
> Yes. So there were 2 things. First, the ` MCGitHubRepository
> cacheDirectory: nil` as not enough as I also needed to #flushCache
> instances.
> Second, was that I was not even able to flush caches because of
> permissions related errors.
>
>
>>
>>>
>>> ... when an extent is copied to a different stone directory there are
>>> potentially a number of places that need to be fixed ... basically anywhere
>>> that is going to look for things on disk ... the Smalltalk code that gets
>>> executed by $GS_HOME/bin/newExtent currently cleans up the tODE directory
>>> dependencies and it should also reset the github-cache directory (whether
>>> or not errors are going to be thrown because of permission errors)...
>>>
>>> The $GS_HOME/sys/{default|local}/client/tode-scripts/rebuildSys script
>>> is intended to clean up these directory/stone dependencies for tODE and
>>> GLASS and you are expected to override with application-specific clean up
>>> code ....
>>>
>>> Soooo is the above error happening before or after you have cleared
>>> MCGitHubRepository class>>cacheDirectory?
>>>
>>>
>> After. It has no effect for me running such code.
>>
>>
>>> I am under the impression that:
>>>
>>> MCGitHubRepository cacheDirectory: nil
>>>
>>> isn't working for you ... am I missing something?
>>>
>>
>> You are not missing anything. That is not working for me and I don't
>> understand why it would. I am under the impression that I would also need:
>>
>> MCRepositoryGroup default repositoriesDo: [:rep | rep flushCache ].
>>
>> No?
>>
>>
>>>
>>>
>>> But my code still fail to load. There are a couple of places that should
>>> be changed. Search for the string "*MCFileTreeFileUtils current
>>> directoryExists:*"
>>> And you will find
>>>
>>> --------------------
>>> MCGitHubRepository class>>resetCacheDirectoryIfInvalid
>>> MCGitBasedNetworkRepository>>directory
>>> MCGitBasedNetworkRepository class>>resetCacheDirectoryIfInvalid
>>> MCFileTreeRepository>>flushCache
>>>
>>> I think those guys should handle the error and on error, the flush or
>>> reset anyway. To avoid this (adding error handling in all those places), I
>>> suggested adding:
>>>
>>> MCFileTreeFileDirectoryUtils class >> directoryExistsAndValid:
>>> aDirectory
>>> ^ aDirectory exists on: Error do: [ false ]
>>>
>>> And so, the above 4 methods should actually send #directoryExistsAndValid:
>>> rather than #directoryExists
>>>
>>> I'm not sure that I want to try to bullet-proof this chunk of code ... I
>>> am worried about what happens if you don't have write permission in the
>>> first place or the disk is full or ???? there are too many possible error
>>> conditions that will either continue to fail or may mask the original
>>> problem if we just handle the errors ...
>>>
>>> Besides it seems to me that the problem here is that
>>> MCGitBasedNetworkRepository class>>resetCacheDirectoryIfInvalid (which sets
>>> the cacheDirectory nil) won't work because if I understand things correctly
>>> `MCGitHubRepository cacheDirectory: nil` doesn't work ... or am I missing
>>> something here ....
>>>
>>
>> Problem is that even if I do `MCGitHubRepository cacheDirectory: nil`
>> the MCGitHubRepository instances still have the offending directory as
>> part of the instvar 'directory' and so the MC load fails. That's why I
>> think I need this code:
>>
>> MCRepositoryGroup default repositoriesDo: [:rep | rep flushCache ].
>>
>> Which effectively clears the 'directory' from
>> the MCGitHubRepository instances. But..guess what? I cannot flush, because:
>>
>> *MCGitBasedNetworkRepository >> flushCache*
>> "the directory acts like a cache since we download the directory from a
>> git-based repository (github, bitbucket, etc.)"
>>
>> * super flushCache.*
>> self class flushDownloadCache.
>> * directory := nil*
>>
>>
>> that method calls super and so this is executed:
>>
>> *MCFileTreeRepository >> flushCache*
>> "force properties to be reread ... if the directory exists, otherwise
>> let nature
>> take it's course"
>>
>> super flushCache.
>> directory
>> ifNotNil: [
>> * (MCFileTreeFileUtils current directoryExists: directory) ->>>
>> this will throw error now*
>> ifTrue: [
>> repositoryProperties := nil.
>> self repositoryProperties ] ]
>>
>>
>> So...to conclude, THIS piece of code does seem to workaround my problem:
>>
>> *MCGitHubRepository cacheDirectory: nil.*
>> *System commit. *
>> *MCGitBasedNetworkRepository allSubInstances do: [ :rep | *
>> * " cannot send #directory: becaue it really expects a directory
>> instance. *
>> * By puttin a nil in 'directory' we are able to run #flushCache.
>> See MCFileTreeRepository >> flushCache. for more details "*
>> * rep instVarNamed: 'directory' put: nil.*
>> * rep flushCache ].*
>> *System commit. *
>>
>>
>> I didn't realize the bit about "would work except for error" ...
>>
>> No I have a good picture and I do think that MCFileTreeRepository >>
>> flushCache can and should be fixed[1] ... we should be able to
>> unconditionally clear the cache in the fact of an error. Here's my proposed
>> fix to MCGitBasedNetworkRepository >> flushCache from the bug report[1]:
>>
> flushCache
>> "the directory acts like a cache since we download the directory from a
>> git-based repository (github, bitbucket, etc.)"
>>
>> [super flushCache] on: Error do: [:ignored | "perhaps dump something to
>> Transcript?" ].
>> self class flushDownloadCache.
>> directory := nil
>>
>>
> Yes, I would dump the error into the transcript.
>
> Okay here's a modified patch:
>
> flushCache
> "the directory acts like a cache since we download the directory from a
> git-based repository (github, bitbucket, etc.)"
>
> [ super flushCache ]
> on: Error
> do: [ :ex |
> Transcript
> cr;
> show:
> 'Error for: ' , self description printString , ' during
> flushCache: '
> , ex description printString ].
> self class flushDownloadCache.
> directory := nil
>
>
> Could you test this one as well? ... There is still some exposure to
> permission errors for the superclass (MCFileTreeRepository), but in this
> case we're not dealing with a cache but the repo itself ... which is more
> serious and not really repairable by an automatic method ... there is a
> `project rehome` command that let's you point a different fileTree repo and
> that should be the solution for this particular case ...
>
> I'll start the process of pushing this fix to Metacello ...
>
> Dale
>
--
Mariano
http://marianopeck.wordpress.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gemtalksystems.com/mailman/private/glass/attachments/20151027/c16e1818/attachment-0001.html>
More information about the Glass
mailing list