[Vm-dev] Fwd: [Pharo-dev] I need reviewer of https://pharo.fogbugz.com/default.asp?15646

Wed Jun 3 17:27:37 UTC 2015

Ah, that reminds me that our MC configuration map store the UUID, so Squeak
should rock...
Alas, we do not use this UUID to enforce correctness at load time !!!
If it's a feature, it's rather a bad one ;)

2015-06-03 15:46 GMT+02:00 Thierry Goubier <thierry.goubier at gmail.com>:

>
>
> 2015-06-03 15:09 GMT+02:00 Nicolai Hess <nicolaihess at web.de>:
>
>>
>>
>> self versionFromFileNamed: fileName
>> is called after it isn't found in the MCCacheRepository
>> and if it is not found in its own special repository cache , it is loaded
>> with
>> self loadVersionFromFileNamed:
>> and this again looks up in the MCCacheRepostory:
>>
>> loadVersionFromFileNamed: aString
>> (MCCacheRepository uniqueInstance includesFileNamed: aString)
>>         ifTrue: [ ^ MCCacheRepository uniqueInstance
>> loadVersionFromFileNamed: aString].
>>
>>     ^ self versionReaderForFileNamed: aString do: [:r | r version]
>>
>> but this time it searches the package in the MCCacheRepitory by its name
>> only and load
>> the version info from that file.
>>
>
> Thanks for finding that. Interesting, that semantic missmatch ... which
> amounts to loosing the version info in places during the loading process.
>
> Notes for documentation :
>
> - FileTree changes versionFromFileNamed: to disable the internal
> repository cache (but cache is still an instance variable because of
> inheritance, and it goes through PackageCache).
>
> - GitFileTree avoids the PackageCache altogether and is the only type of
> repo not to have that bug in Pharo4 (Yes! But I had no idea why :)).
>
>
>>
>>> We need to build test repositories to have correct coverage of those
>>> issues, because at the moment I'm not sure I understand the case you are
>>> describing :(
>>>
>>
>>
>> Try to load
>>
>> SLICE-Issue-15646-Cleaning-method-category-api-should-be-protocol-part-1-StephaneDucasse.1
>> from pharo 5.0 inbox repository.
>> This slice has dependencies to packages
>> DebuggerActions-StephaneDucasse.75, Kernel-StephaneDucasse.2042
>> which are both in pharo 5.0 main and inbox repository but with different
>> uuids
>>
>
> Yuck :(
>
> So, if I understood well, what is happening is:
>
> - Reading from Pharo5Main, DebuggerActions-StephaneDucasse.75 is loaded in
> PackageCache (and in the cache of Pharo5Main).
> - Comparing UUIDs say that this is not the right one.
> - Reading from Pharo5Inbox, DebuggerActions-StephaneDucasse.75 is loaded
> in the cache of Pharo5Inbox
> - First read inside package cache say that the file exist but has the
> wrong UUID
> - Second read has matching file name inside Pharo5Inbox
> - So it loads the version from Pharo5Inbox
>   - which test PackageCache for DebuggerActions-StephaneDucasse.75 ...
> which is true
>   - But uuids don't match
>   - so it fails....
>
> I hate package-cache.
>
>
>>
>>
>> If pharo 5.0 main repository is searched first, it will only find
>> packages with the wrong version info,
>> even if the packages in the 5.0 inbox have the correct version info.
>>
>
> You're right.
>
> So your fix should work. I would not hesitate however:
>
> - to move the version reading line inside versionWithInfo:ifAbsent:
> - to factor the cache update into an #updateCacheWith: v (so that it can
> also be called from versionFromFileNamed:) called from
> versionWithInfo:ifAbsent:
>
> Thierry
>
>
>>
>>
>>
>>
>>>
>>> Thierry
>>>
>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>
>>>>> Package loads with version info happens in two cases:
>>>>> - dependency loading (i.e. slices)
>>>>> - history loading (i.e. browsing history and merges).
>>>>>
>>>>> Anything else is loading by name, since this is what is recorded in
>>>>> Configurations and Gofer scripts.
>>>>>
>>>>> Thierry
>>>>>
>>>>>
>>>>>>
>>>>>> nicolai
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Maybe we could for real put (part of) the UID in the filename?
>>>>>>>
>>>>>>> In a disconnected system the case that the filename is the same for
>>>>>>> different files will always happen if the name is contructed
>>>>>>> as it is now.
>>>>>>>
>>>>>>>         Marcus
>>>>>>>
>>>>>>> > On 02 Jun 2015, at 22:37, stepharo <stepharo at free.fr> wrote:
>>>>>>> >
>>>>>>> > I hate so much this bug....
>>>>>>> >
>>>>>>> >
>>>>>>> > Le 1/6/15 15:04, Ben Coman a écrit :
>>>>>>> >> Stef, I can now see all the dependent packages for the new slice,
>>>>>>> but
>>>>>>> >> I still have a strange error.  However I'm not sure if its a bug
>>>>>>> or
>>>>>>> >> something unique at my end.
>>>>>>> >>
>>>>>>> >> Can someone try merging
>>>>>>> >>
>>>>>>> SLICE-Issue-15646-Cleaning-method-category-api-should-be-protocol-part-1-StephaneDucasse.1
>>>>>>> >>
>>>>>>> >> What I see at top of stack is two calls to
>>>>>>> MCVersionMerger>>addVersion:
>>>>>>> >>
>>>>>>> >>     MCVersionMerger>>addVersion: aVersion
>>>>>>> >>         records add: (MCMergeRecord version: aVersion).
>>>>>>> >>         aVersion dependencies
>>>>>>> >>             do: [:ea | | dep satisfied |
>>>>>>> >>                 dep := ea resolve.
>>>>>>> >>                 satisfied := (records anySatisfy: [:r | r version
>>>>>>> = dep]).
>>>>>>> >>                 satisfied ifFalse: [self addVersion: dep]]  "<<<
>>>>>>> race? "
>>>>>>> >>             displayingProgress: [ :ea| 'Searching dependency: ',
>>>>>>> ea
>>>>>>> >> package name]
>>>>>>> >>     "15646Note: variable /satisfied/ added for
>>>>>>> reporting/debugging"
>>>>>>> >>
>>>>>>> >> One level down from where the error occurs the debugger shows...
>>>>>>> >>
>>>>>>> >>     /aVersion/ --> a
>>>>>>> >>
>>>>>>> MCVersion(SLICE-Issue-15646-Cleaning-method-category-api-should-be-protocol-part-1-StephaneDucasse.1)
>>>>>>> >>
>>>>>>> >>     /ea/ --> a MCVersionInfo(DebuggerActions-StephaneDucasse.75)
>>>>>>> >>
>>>>>>> >>     /dep/ --> nil
>>>>>>> >>
>>>>>>> >>     /satisfied/  --> false
>>>>>>> >>
>>>>>>> >> and the following which contradicts the value in /satisfied/
>>>>>>> >>
>>>>>>> >>     (records anySatisfy: [:r | r version = dep]) --> true.
>>>>>>> >>
>>>>>>> >> so there seems to be a race such that the ifFalse block is
>>>>>>> improperly
>>>>>>> >> executed, such that the recursive call on top of stack has...
>>>>>>> >>
>>>>>>> >>     /aVersion/-->nil
>>>>>>> >>
>>>>>>> >> hence MNU receiver of "dependencies" is nil.
>>>>>>> >>
>>>>>>> >> cheers -ben
>>>>>>> >>
>>>>>>> >>
>>>>>>> >> On Mon, Jun 1, 2015 at 10:36 AM, Ben Coman <btc at openinworld.com>
>>>>>>> wrote:
>>>>>>> >>> I tried, but it seems some packages are missing from the inbox.
>>>>>>> >>> cheers -ben
>>>>>>> >>>
>>>>>>> >>> On Sun, May 31, 2015 at 2:19 PM, stepharo <stepharo at free.fr>
>>>>>>> wrote:
>>>>>>> >>>> Hi
>>>>>>> >>>>
>>>>>>> >>>> I continued to clean that classes have categories and method
>>>>>>> protocols
>>>>>>> >>>> because it was not finished.
>>>>>>> >>>> This entry is just adding protocol in the classes that were
>>>>>>> missing it,
>>>>>>> >>>> adding comments, and fixing some local senders
>>>>>>> >>>> It does not remove the category API but puts it in a
>>>>>>> accessing-backward
>>>>>>> >>>> protocol and in a second step I will fix all the senders I can
>>>>>>> (ie not
>>>>>>> >>>> Metacello for example).
>>>>>>> >>>> Category is really overloaded and we get lost when trying to
>>>>>>> understand
>>>>>>> >>>> code.
>>>>>>> >>>> I want to rename RBRule 'category' into 'kind' for this reason.
>>>>>>> >>>>
>>>>>>> >>>> Stef
>>>>>>> >>>>
>>>>>>> >>
>>>>>>> >
>>>>>>> >
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20150603/b9a5f344/attachment.htm