search

Metacello / metacello

Metacello is a package management system for Smalltalk
MIT License
87 stars 43 forks source link

Loading Core group of project will load full dependant project if only group of it already in image #472

Closed dionisiydk closed 6 years ago

dionisiydk commented 6 years ago

Now in Pharo we have official version of Metacello. So I checked old issue:

https://pharo.fogbugz.com/f/cases/19599/Loading-Core-group-of-project-will-load-full-dependant-project-if-only-group-of-it-already-in-image.

And it is still the issue.

krono commented 6 years ago

Can you please copy the content here?

dionisiydk commented 6 years ago

Example to see problem: First to see normal behaviour load ObjectTravel project Core group in clean image:

Metacello new
        smalltalkhubUser: 'Pharo' project: 'ObjectTravel';
        configuration: 'ObjectTravel';
        version: #stable;
        load: 'Core'.

Then in Monticello browser you will not see dependant project StateSpecs because it is part of Tests group. Now in clean image try first to load StateSpecs:Core:

Metacello new
        smalltalkhubUser: 'dionisiy' project: 'StateSpecs';
        configuration: 'StateSpecs';
        version: #stable;
        load: 'Core'.

In monticello you will not see any tests for this project because they are in Tests group. And now the problem. In image with StateSpecs:Core try load ObjectTravel with first script. You will see that Tests group of StateSpecs will be loaded too.

It leads to problem with updating projects when tests could be loaded after update. For example load twice times PharmIDE project Server group:

Metacello new
        smalltalkhubUser: 'Pharo' project: 'PharmIDE';
        configuration: 'PharmIDE';
        version: #stable;
        load: 'Server'.

First time everything will be correct. No tests will be loaded. But second time tests for StateSpecs will be loaded which should not happen

dalehenrich commented 6 years ago

I don't normally work with Pharo (you don't say which version of Pharo this is happening in) so it will be simpler for me if you include the transcript output for each of the runs ... so I can see when the Tests groups is loaded and get an initial idea as to what might be causing the problem

dionisiydk commented 6 years ago

It is latest Pharo 7.

Here is the log. Sadly I can not retrieve information about Metacello version in image.

I put two logs: First is loading StateSpecs and then ObjectTravel. And another is loading only ObjectTravel without StateSpecs. In all cases only Core groups are requested. So tests should never be loaded.

First case:

Loading of StateSpecs Core group:

Fetched -> ConfigurationOfStateSpecs-DenisKudryashov.29 --- http://smalltalkhub.com/mc/dionisiy/StateSpecs/main/ --- http://smalltalkhub.com/mc/dionisiy/StateSpecs/main/ Loaded -> ConfigurationOfStateSpecs-DenisKudryashov.29 --- http://smalltalkhub.com/mc/dionisiy/StateSpecs/main/ --- http://smalltalkhub.com/mc/dionisiy/StateSpecs/main/ Loading master of ConfigurationOfStateSpecs... Fetched -> BaselineOfStateSpecs-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/StateSpecs (Libgit) Loaded -> BaselineOfStateSpecs-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/StateSpecs (Libgit) Project: StateSpecs baseline Fetched -> StateSpecs-Specs-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/StateSpecs (Libgit) Fetched -> StateSpecs-DSL-ShouldExpressions-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/StateSpecs (Libgit) Fetched -> StateSpecs-DSL-ClassWords-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/StateSpecs (Libgit) Loaded -> StateSpecs-Specs-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- cache Loaded -> StateSpecs-DSL-ShouldExpressions-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- cache Loaded -> StateSpecs-DSL-ClassWords-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- cache ...finished master

And following loading of ObjectTravel Core group. You can see that StateSpecs Tests are loaded in that case.

Fetched -> ConfigurationOfObjectTravel-DenisKudryashov.33 --- http://smalltalkhub.com/mc/Pharo/ObjectTravel/main/ --- http://smalltalkhub.com/mc/Pharo/ObjectTravel/main/ Loaded -> ConfigurationOfObjectTravel-DenisKudryashov.33 --- http://smalltalkhub.com/mc/Pharo/ObjectTravel/main/ --- http://smalltalkhub.com/mc/Pharo/ObjectTravel/main/ Loading master of ConfigurationOfObjectTravel... Fetched -> BaselineOfObjectTravel-GitHub.1500299086 --- git@github.com:dionisiydk/ObjectTravel.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/ObjectTravel (Libgit) Loaded -> BaselineOfObjectTravel-GitHub.1500299086 --- git@github.com:dionisiydk/ObjectTravel.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/ObjectTravel (Libgit) Project: ObjectTravel baseline Project: StateSpecs baseline Fetched -> StateSpecs-Help-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/StateSpecs (Libgit) Fetched -> StateSpecs-Specs-Tests-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/StateSpecs (Libgit) Fetched -> StateSpecs-DSL-ShouldExpressions-Tests-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/StateSpecs (Libgit) Fetched -> StateSpecs-DSL-ClassWords-Tests-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/StateSpecs (Libgit) Fetched -> ObjectTravel-GitHub.1500299086 --- git@github.com:dionisiydk/ObjectTravel.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/ObjectTravel (Libgit) Loaded -> StateSpecs-Help-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- cache Loaded -> StateSpecs-Specs-Tests-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- cache Loaded -> StateSpecs-DSL-ShouldExpressions-Tests-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- cache Loaded -> StateSpecs-DSL-ClassWords-Tests-GitHub.1500157621 --- git@github.com:dionisiydk/StateSpecs.git[master] --- cache Loaded -> ObjectTravel-GitHub.1500299086 --- git@github.com:dionisiydk/ObjectTravel.git[master] --- cache ...finished master

Second case:

Loading of ObjectTravel Core group in fresh image without StateSpecs. You can see that Tests are not loaded.

Fetched -> ConfigurationOfObjectTravel-DenisKudryashov.33 --- http://smalltalkhub.com/mc/Pharo/ObjectTravel/main/ --- http://smalltalkhub.com/mc/Pharo/ObjectTravel/main/ Loaded -> ConfigurationOfObjectTravel-DenisKudryashov.33 --- http://smalltalkhub.com/mc/Pharo/ObjectTravel/main/ --- http://smalltalkhub.com/mc/Pharo/ObjectTravel/main/ Loading master of ConfigurationOfObjectTravel... Fetched -> BaselineOfObjectTravel-GitHub.1500299086 --- git@github.com:dionisiydk/ObjectTravel.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/ObjectTravel (Libgit) Loaded -> BaselineOfObjectTravel-GitHub.1500299086 --- git@github.com:dionisiydk/ObjectTravel.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/ObjectTravel (Libgit) Project: ObjectTravel baseline Fetched -> ObjectTravel-GitHub.1500299086 --- git@github.com:dionisiydk/ObjectTravel.git[master] --- /Users/dkudrias/Pharo/pharo-local/iceberg/dionisiydk/ObjectTravel (Libgit) Loaded -> ObjectTravel-GitHub.1500299086 --- git@github.com:dionisiydk/ObjectTravel.git[master] --- cache ...finished master

dalehenrich commented 6 years ago

Okay here is the skinny on what's going on. To start with the default group for StateSpecs is defined like this:

spec
  group: 'default' with: #('Core' 'Tests');

... and ObjectTravel references the StateSpecs project like this:

spec for: #'common' do: [
    spec baseline: 'StateSpecs' with: [
      spec repository: 'github://dionisiydk/StateSpecs:v2.4.x' ].

... and this references ends up loading the default group of StateSpecs -- which brings in Core and Tests. If you want ObjectTravel to depend upon only the Core of StateSpecs then you should define the reference like this:

spec for: #'common' do: [
    spec baseline: 'StateSpecs' with: [
      spec 
        loads: #( 'Core');
        repository: 'github://dionisiydk/StateSpecs:v2.4.x' ].

... or change the default group to be only Core

dionisiydk commented 6 years ago

But do you noticed that StateSpecs is referenced only in Tests group of ObjectTravel? In both cases I loaded only Core group of ObjectTravel which not depends on StateSpecs.

And what you can see from log is that image state affects somehow the parts which are loaded together with ObjectTravel:Core.

dalehenrich commented 6 years ago

No ... I was under the impression from what you had said before that StateSpecs was expected and that the tests were unexpected ...

dalehenrich commented 6 years ago

Could you post the baseline of the configuration that you are loading (I can't look at source on smalltalkhub) ... both ObjectTravel and StateSpecs have the Test group in their default group and that is suspicious to me... presumably the problem is being caused by something in the configuration you are loading ...

dionisiydk commented 6 years ago

Ok.

StateSpecs baseline:

baseline: spec
    <baseline>

    spec for: #common do: [
        spec 
            package: 'StateSpecs-Specs';
            package: 'StateSpecs-DSL-ShouldExpressions' with: [ spec requires: #('StateSpecs-Specs') ];
            package: 'StateSpecs-DSL-ClassWords' with: [ spec requires: #('StateSpecs-Specs') ];
            package: 'StateSpecs-Help' with: [
                spec requires: #('StateSpecs-DSL-ShouldExpressions' 'StateSpecs-DSL-ClassWords')];

            package: 'StateSpecs-Specs-Tests' with: [ spec requires: 'StateSpecs-Specs' ];
            package: 'StateSpecs-DSL-ShouldExpressions-Tests' with: [ spec requires: 'StateSpecs-DSL-ShouldExpressions' ];
            package: 'StateSpecs-DSL-ClassWords-Tests' with: [ spec requires: 'StateSpecs-DSL-ClassWords' ].

        spec
            group: 'default' with: #('Core' 'Tests');
            group: 'Core' with: #('StateSpecs-Specs' 'StateSpecs-DSL-ShouldExpressions' 'StateSpecs-DSL-ClassWords');
            group: 'Tests' with: #('StateSpecs-Specs-Tests' 'StateSpecs-DSL-ShouldExpressions-Tests' 'StateSpecs-DSL-ClassWords-Tests' 'StateSpecs-Help').
    ]

ObjectTravel baseline:

baseline: spec
    <baseline>
    spec for: #'common' do: [
        spec baseline: 'StateSpecs' with: [
            spec repository: 'github://dionisiydk/StateSpecs:v2.4.x' ].
        spec 
            package: 'ObjectTravel';
            package: 'ObjectTravel-Tests' with: [spec requires: #(StateSpecs ObjectTravel)].
        spec 
            group: 'default' with: #('Core' 'Tests');
            group: 'Core' with: #('ObjectTravel');
            group: 'Tests' with: #('ObjectTravel-Tests') ].

I think even if you will find mistake in baselines it is wrong behaviour that different order of loading projects leads to different loaded code. So in my case I always load Core group of each project and depending on order in which I perform scripts I got extra dependency in image.

dalehenrich commented 6 years ago

Sorry for the delay in getting back to this, but right now I am trying to understand why things are going wrong for you this is not expected behavior and at the moment I do not know why you are getting things loaded incorrectly... for some reason this is not a common problem (AFAIK you are the only person hitting this problem) and there are several thousand Metacello tests that are passing ... I would like to be able to construct a test case that illustrates the problem adn once it is characterized I can go forward with attempting to fix it ...

dalehenrich commented 6 years ago

Since you are using Pharo 7.0 and I see that there appear to be alot of bugs related to the use of Metacello and Iceberg together ... I am wondering if your bug is related to Iceberg ... are you explicitly using Iceberg? there is a enableMetacello flag -- or something --- that apparently needs to be set in Iceberg for some reason --- I am not familiar with the details of the Iceberg related issues and am surprised that Iceberg can have an effect on Metacello, but if it is something to do with Iceberg, then I cannot fix it by changing Metacello --- at least noone from the Iceberg has asked me to change Metacello ...

dionisiydk commented 6 years ago

This problem appears without Iceberg. I noticed it in Pharo 6. But Pharo 6 uses some old Metacello version. Tomorrow I will try reproduce it in latest Squeak

dalehenrich commented 6 years ago

Okay ... I'm still taking shots in the dark:) .... nothing jumps out at me from the first reading of the baselines ...

dalehenrich commented 6 years ago

I am actually curious about the baselines in the configurations (ConfigurationOfObjectTravel-DenisKudryashov.3 and ConfigurationOfStateSpecs-DenisKudryashov.29) ... presumably the ConfigurationOf is where the bad things are happening (I think I mentioned that before) ...

dalehenrich commented 6 years ago

I'll go ahead and download the configurations ... and look at the baselines ...

dalehenrich commented 6 years ago

You mention that iceberg is not involved, but I see iceberg in the repository path from that transcript logs ... It's not that I don't believe what's happening, but if I am told one thing and then see something different in the log, everything I know is wrong ... I also need to see the expressions you use to do the load each of the log files ...If I'm going to go to the trouble of downloading a Pharo vm and image and attempt to reproduce the problem I would really like to know the exact steps to reproduce the problem and the exact results I expect to see ...

dalehenrich commented 6 years ago

... good news to a certain extent, but I am able to reproduce the loading error in GemStone ... so I don't have to fiddle with getting Pharo downloaded to characterize this puppy ...

dalehenrich commented 6 years ago

Okay ... this is a nasty one ... it looks like the issue is rooted in the old Monticello problem where it is/was "illegal" to name a package StateSpecs and StateSpecs-Tests ... at some point in the process, Metacello tries to determine which packages are loaded for StateSpecs project and during that process it looks like all of the test packages get included because the Monticello algorithms treat StateSpecs-Tests as a loaded package because it "matches" StateSpecs (i.e., it begins with 'StateSpecs' ... I haven't yet isolated the exact place in the code where this mistake is made, but I'm pretty sure this is the root cause ... You mention that you are going to test on Squeak so if you expect these projects to be loaded on any other platform than Pharo, then you will have to change the names of your package, because you are using a non-portable package naming conventions --- this is something that has worried ever since I heard that Pharo was unilaterally changing the Monticello naming convention without assessing the potential impact on Metacello and cross platform projects ...

So ... if you intend this project to be portable to all platforms that support the Monticello naming convention, then you must change the names of your packages to conform with the Monticello naming convention ... if you don't intend this to be a portable project, then we'll need to get @estebanlm involved, because Metacello has not been taught the new package naming rules ...

Before going too far, I suggest that you try renaming your packages to conform to the Monticello package naming rules (tack a -Core onto the "core" packages, so that there are no packages with names that are a subset of any other package name ... that should fix your problem on the sort term, until we can get around to teaching Metacello about the Pharo package naming rules ...

I've run out of time for now and will have to focus on some looming deadlines over the next week or so...

pavel-krivanek commented 6 years ago

I have the same problem and it does not seem to be related to the naming convention. In the baseline (of Moose) I have some packages like 'WebBrowser-Core' or GT that are by default loaded in the image. The baseline then specifies a group that contains only one different package that has no dependency on other packages. But anyway, Metacello reloads all packages that are specified in the baseline and are present in the image even if I try to load only the group that does not contain them.

pavel-krivanek commented 6 years ago

For this baseline:

    spec for: #'common' do: [
        spec 

            package: #'WebBrowser-Core';
            package: #'CustomPackage'.
        spec 
            group: 'Core' with: #(#'CustomPackage').]

and this loading command: 

Metacello new
  baseline: 'MetacelloIssue472';
  repository: 'github://pavel-krivanek/playground';
  load: 'Core'

it loads:

linear load : 
    linear load : baseline [BaselineOfMetacelloIssue472]
        load : WebBrowser-Core-PavelKrivanek.1519989037
        load : CustomPackage-PavelKrivanek.1519989037

The package WebBrowser-Core is some package that is present by default in the image in time of loading of the baseline.

dalehenrich commented 6 years ago

@pavel-krivanek I'm inclined to have a separate issue for this, because as you say it does not sound like a package naming issue .... and including a different conversation will just make things confusing ...

dionisiydk commented 6 years ago

But why you close this issue? This package convention is kind of standard in Pharo. Many Pharo packages are adopted to this rule. So Metacello should be fixed to support it correctly. Do you not agree with it?

ThierryGoubier commented 6 years ago

@dionisiydk @dalehenrich just said he wanted to fix that under a separate issue, since this does not seems to be a package naming issue.

dionisiydk commented 6 years ago

Then ok. I just thought that it is another issue in addition to naming problem.

dalehenrich commented 6 years ago

@dionisiydk the naming problem is not a Metacello issue ... Pharo has broken cross platform Monticello packages by not conforming to the Monticello naming conventions that have been in place for a decade ... Metacello is only involved because it uses Monticello for loading packages ...