[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 ImageSegment, 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
|