[squeak-dev] Monticello's Package Loading Strategy (Bug)

Bert Freudenberg bert at freudenbergs.de
Wed Jun 28 11:15:49 UTC 2017


On Wed, Jun 28, 2017 at 12:19 AM, Eliot Miranda <eliot.miranda at gmail.com>
wrote:

> Hi All, Hi Bert, Hi Colin,
>
>     today I was working with Ron Teitelbaum on porting the Terf client to
> Squeak 6.x.  The bootstrap builder creates some base MCConfigurations and
> then updates them from the various repositories, hence downloading the
> latest versions of certain packages into the package cache.
>
> Some of the downloaded packages are for Tweak, and Tweak classes include
> field definitions which require special Monticello support to manage.  The
> special handling is provided by the TweakMC package.  Here are some of the
> relevant definitions:
>
> MCStWriter>>visitTweakFieldDefinition: definition
> self writeTweakFieldDefinition: definition
>
>
> MCStWriter>>writeTweakFieldDefinition: definition
> self chunkContents: [:s | definition printDefinitionOn: stream]
>
> which produces output like the following in the snapshot.st file:
>
> CCostumeGrid defineFields: '<?xml  version="1.0" ?>
> <fields>
>         <field toGet="origin" toSet="origin:" changeEvent="originChanged">
> origin</field>
>         <field toGet="extent" toSet="extent:" changeEvent="extentChanged">
> extent</field>
>         <field toGet="enabled" toSet="enabled:"
> changeEvent="enabledChanged">enabled</field>
>         <field toGet="visible" toSet="visible:"
> changeEvent="visibleChanged">visible</field>
>         <field toGet="color" toSet="color:" changeEvent="colorChanged">
> color</field>
> </fields>'!
>
> Note that no special support is needed for producing the snapshot.bin
> because that can save arbitrary objects.
>
> So, (I think) because we were loading packages saved by Squeak V3 into a
> Squeak Spur image the binary isn't loaded and instead the source is
> compiled. (Am I right?)
>

​It's not an ImageSegmen​t, so that should not be a problem. Missing class
definitions, however, would be a problem. This might be the case with
MCFieldDefs (or whatever they are called, don't
​remember).



>  If the update happens *before* TweakMC has been loaded then the packages
> that get installed into package-cache are missing all the field definitions
> because the support to produce them is missing.
>
> This was exactly our situation; we're freshly bootstrapping a Terf image,
> which includes adding support for Tweak, and TweakMC is loaded by the
> bootstrap /after/ the configurations have been updated, and hence the Tweak
> packages in the package-cache are corrupted.
>
> We fixed our issue by modifying the bootstrap to load TweakMC up front.
> But IMO *this shouldn't be necessary*.
>
> Why, when Monticello downloads a package from a repository, does it
> rewrite that package to install it in the package-cache directory?
>

​Because MC has no concept of "files". Each repository can have a different
representation for how it stores a version (e.g. MCDictionaryRepository is
literally just a dictionary of MCVersions). That's why an MCVersion is
created by an MCReader when loading from a repo, and writing to a repo
invokes that repo's MCWriter.

Surely it has *no business* rewriting the contents of the package and could
> simply copy the file itself.  If it does the rewrite, as it is doing now,
> it opens up the transcription to all sorts of errors, the above being one.
> Not only is the approach slow, and error prone, it is unnecessary.  The
> file could be copied quite simply.
>

​Only if both repos are MCFileBasedRepositories. ​

Is this easy to fix?  I think it's a really bad design flaw.
>

​I don't think it's simple to fix. A version is not a file conceptually, so
anything we hack to make it look like it is would be just that, a hack. The
Right Way is to load the extension for MC itself first, and then use the
mechanics that are there.

That hack could be added to MCVersion>>addToCache along the lines of

addToCache
(thisContext findContextSuchThat: [:c | c receiver isKindOf: MCReader])
ifNotNil:
[:ctxt | | contents |
contents := (ctxt receiver instVarNamed: 'stream') contentsOfEntireFile.
self halt].
MCCacheRepository default storeVersion: self

... and then store 'contents' in the cache directly. The hack could be
cleaned up by using a notification to get at the stream by more official
means. However, I don't think that's a good idea, as it goes against the
design as explained above. Other opinions/ideas very welcome :)

- Bert -
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20170628/4d928f2b/attachment.html>


More information about the Squeak-dev mailing list