[Glass] Git based repositories (package-cache) problem when moving extent to another stone
Dale Henrichs via Glass
glass at lists.gemtalksystems.com
Tue Oct 27 17:46:09 PDT 2015
Okay ... I'll ship it! ... thx
On 10/27/15 4:00 PM, Mariano Martinez Peck wrote:
> Yes, it does work too.
>
> On Tue, Oct 27, 2015 at 6:48 PM, Dale Henrichs
> <dale.henrichs at gemtalksystems.com
> <mailto: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
>> <mailto: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
>>> <mailto: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/3f89a117/attachment-0001.html>
More information about the Glass
mailing list