2015-06-03 15:46 GMT+02:00 Thierry Goubier <[email protected]>:

>
>
> 2015-06-03 15:09 GMT+02:00 Nicolai Hess <[email protected]>:
>
>>
>>
>> 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
>

issue and fix
15698 <https://pharo.fogbugz.com/default.asp?15698>
do not load package from cache if version info differs

please test and review





>
>
>>
>>
>>
>>
>>>
>>> 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 <[email protected]> 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 <[email protected]>
>>>>>>> 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 <[email protected]>
>>>>>>> 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
>>>>>>> >>>>
>>>>>>> >>
>>>>>>> >
>>>>>>> >
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Reply via email to