I need reviewer of https://pharo.fogbugz.com/default.asp?15646
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
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@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
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@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@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
I think the problem is that, some of the packages like Kernel-StephaneDucasse.2042 DebuggerActions-StephaneDucasse.75 are in both repositories (main/inbox) but with different UUIDs. 2015-06-01 15:04 GMT+02:00 Ben Coman <btc@openinworld.com>:
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@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@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
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@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@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
Indeed⦠this has to do something with caching: when looking a .mcz the system just takes the one it finds, which might be one with the same name but different. 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@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@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@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
2015-06-03 8:22 GMT+02:00 Marcus Denker <marcus.denker@inria.fr>:
Indeed... this has to do something with caching: when looking a .mcz the system just takes the one it finds, which might be one with the same name but different.
Unless the mcz searched for is a dependency, in which case it checks the uuid as well (not only the name).
Maybe we could for real put (part of) the UID in the filename?
Would that be backward compatible with existing repositories?
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.
Thierry
Marcus
On 02 Jun 2015, at 22:37, stepharo <stepharo@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@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@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
On 03 Jun 2015, at 09:52, Thierry Goubier <thierry.goubier@gmail.com> wrote:
2015-06-03 8:22 GMT+02:00 Marcus Denker <marcus.denker@inria.fr <mailto:marcus.denker@inria.fr>>: Indeed⦠this has to do something with caching: when looking a .mcz the system just takes the one it finds, which might be one with the same name but different.
Unless the mcz searched for is a dependency, in which case it checks the uuid as well (not only the name).
Maybe we could for real put (part of) the UID in the filename?
Would that be backward compatible with existing repositories?
I would just put it instead of the Name of the submitter⦠Marcus
2015-06-03 9:56 GMT+02:00 Marcus Denker <marcus.denker@inria.fr>:
On 03 Jun 2015, at 09:52, Thierry Goubier <thierry.goubier@gmail.com> wrote:
2015-06-03 8:22 GMT+02:00 Marcus Denker <marcus.denker@inria.fr>:
Indeed... this has to do something with caching: when looking a .mcz the system just takes the one it finds, which might be one with the same name but different.
Unless the mcz searched for is a dependency, in which case it checks the uuid as well (not only the name).
Maybe we could for real put (part of) the UID in the filename?
Would that be backward compatible with existing repositories?
I would just put it instead of the Name of the submitter...
I'm not that worried about that problem, but the fact that in MC, when you query a repo, you build a kind of a match between your target version info and a "possible" file name... If you start creating names with UUIDs in them, how will the system distinguish between version infos where you should use the UUID and version infos (for example, in the history) where you shouldn't. Give a higher priority to matching with an UUID-based name ? Go through Gofer twice, first with a UUID version of the name, then with a non-UUID version of the name if the first one fails? I believe there are places (MC itself, Gofer?) where the true version info is decoupled from the search and only the package name is used (i.e. name + author + version number). Thierry
Marcus
2015-06-03 8:22 GMT+02:00 Marcus Denker <marcus.denker@inria.fr>:
Indeed⦠this has to do something with caching: when looking a .mcz the system just takes the one it finds, which might be one with the same name but different.
here is a minimal change to the way it uses the cache, or omitting the cache after the correct version wasn't find in the cache. What do you think, may this work? 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@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@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@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
2015-06-03 11:41 GMT+02:00 Nicolai Hess <nicolaihess@web.de>:
2015-06-03 8:22 GMT+02:00 Marcus Denker <marcus.denker@inria.fr>:
Indeed... this has to do something with caching: when looking a .mcz the system just takes the one it finds, which might be one with the same name but different.
here is a minimal change to the way it uses the cache, or omitting the cache after the correct version wasn't find in the cache.
What do you think, may this work?
I'm not sure the problem is there. Often, in configurations, you don't have the full version info, just the mcz name :( And I would expect that a given repo has a single mcz with a given name (i.e. the repo cache should be working properly)... Nice find that each file based repository has a cache in addition to the main package cache.... 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@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@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@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
2015-06-03 13:22 GMT+02:00 Thierry Goubier <thierry.goubier@gmail.com>:
2015-06-03 11:41 GMT+02:00 Nicolai Hess <nicolaihess@web.de>:
2015-06-03 8:22 GMT+02:00 Marcus Denker <marcus.denker@inria.fr>:
Indeed⦠this has to do something with caching: when looking a .mcz the system just takes the one it finds, which might be one with the same name but different.
here is a minimal change to the way it uses the cache, or omitting the cache after the correct version wasn't find in the cache.
What do you think, may this work?
I'm not sure the problem is there. Often, in configurations, you don't have the full version info, just the mcz name :( And I would expect that a given repo has a single mcz with a given name (i.e. the repo cache should be working properly)...
Nice find that each file based repository has a cache in addition to the main package cache....
The problem is that the package "loading" again searches the cache, without the version id: search for package with name+id -> search cache for package with name+id -> "not found -> search other repos -> other repo: -> search again the cache for package with name+id -> "not found" -> load version with name -> search cache for package with *name only* -> "Found" "other" repo thinks it has loaded its package with that name -> compare name+id -> "not found" <- nothing found, even if it is (online) in other repo.
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@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@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@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
2015-06-03 14:19 GMT+02:00 Nicolai Hess <nicolaihess@web.de>:
2015-06-03 13:22 GMT+02:00 Thierry Goubier <thierry.goubier@gmail.com>:
2015-06-03 11:41 GMT+02:00 Nicolai Hess <nicolaihess@web.de>:
2015-06-03 8:22 GMT+02:00 Marcus Denker <marcus.denker@inria.fr>:
Indeed... this has to do something with caching: when looking a .mcz the system just takes the one it finds, which might be one with the same name but different.
here is a minimal change to the way it uses the cache, or omitting the cache after the correct version wasn't find in the cache.
What do you think, may this work?
I'm not sure the problem is there. Often, in configurations, you don't have the full version info, just the mcz name :( And I would expect that a given repo has a single mcz with a given name (i.e. the repo cache should be working properly)...
Nice find that each file based repository has a cache in addition to the main package cache....
The problem is that the package "loading" again searches the cache, without the version id:
search for package with name+id -> search cache for package with name+id -> "not found -> search other repos -> other repo: -> search again the cache for package with name+id -> "not found" -> load version with name -> search cache for package with *name only* -> "Found" "other" repo thinks it has loaded its package with that name -> compare name+id -> "not found" <- nothing found, even if it is (online) in other repo.
Are you sure? Because the code you describe does not correct/affect that. 'self versionFromFileNamed: fileName' looks only into that special repository cache, not the global one. 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 :( 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@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@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@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
2015-06-03 14:47 GMT+02:00 Thierry Goubier <thierry.goubier@gmail.com>:
2015-06-03 14:19 GMT+02:00 Nicolai Hess <nicolaihess@web.de>:
2015-06-03 13:22 GMT+02:00 Thierry Goubier <thierry.goubier@gmail.com>:
2015-06-03 11:41 GMT+02:00 Nicolai Hess <nicolaihess@web.de>:
2015-06-03 8:22 GMT+02:00 Marcus Denker <marcus.denker@inria.fr>:
Indeed⦠this has to do something with caching: when looking a .mcz the system just takes the one it finds, which might be one with the same name but different.
here is a minimal change to the way it uses the cache, or omitting the cache after the correct version wasn't find in the cache.
What do you think, may this work?
I'm not sure the problem is there. Often, in configurations, you don't have the full version info, just the mcz name :( And I would expect that a given repo has a single mcz with a given name (i.e. the repo cache should be working properly)...
Nice find that each file based repository has a cache in addition to the main package cache....
The problem is that the package "loading" again searches the cache, without the version id:
search for package with name+id -> search cache for package with name+id -> "not found -> search other repos -> other repo: -> search again the cache for package with name+id -> "not found" -> load version with name -> search cache for package with *name only* -> "Found" "other" repo thinks it has loaded its package with that name -> compare name+id -> "not found" <- nothing found, even if it is (online) in other repo.
Are you sure? Because the code you describe does not correct/affect that.
'self versionFromFileNamed: fileName'
looks only into that special repository cache, not the global one.
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.
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 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.
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@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@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@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
2015-06-03 15:09 GMT+02:00 Nicolai Hess <nicolaihess@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@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@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@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
2015-06-03 15:46 GMT+02:00 Thierry Goubier <thierry.goubier@gmail.com>:
2015-06-03 15:09 GMT+02:00 Nicolai Hess <nicolaihess@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
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 <stepharo@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@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@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
You guys are moving so fast. I went to review it but it is already integrated :P Anyway I tried it and it worked well, load Stef's 15646 slice no problem. Well done Nicolai. cheers -ben On Wed, Jun 3, 2015 at 5:41 PM, Nicolai Hess <nicolaihess@web.de> wrote:
2015-06-03 8:22 GMT+02:00 Marcus Denker <marcus.denker@inria.fr>:
Indeed⦠this has to do something with caching: when looking a .mcz the system just takes the one it finds, which might be one with the same name but different.
here is a minimal change to the way it uses the cache, or omitting the cache after the correct version wasn't find in the cache.
What do you think, may this work?
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@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@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@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
participants (5)
-
Ben Coman -
Marcus Denker -
Nicolai Hess -
stepharo -
Thierry Goubier