2015-06-03 15:09 GMT+02:00 Nicolai Hess <nicolaih...@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 <steph...@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 <b...@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 <steph...@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
>>>>>> >>>>
>>>>>> >>
>>>>>> >
>>>>>> >
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Reply via email to