search

SeasideSt / Seaside

The framework for developing sophisticated web applications in Smalltalk.
MIT License
519 stars 71 forks source link

PackageManifest extensions should not be loaded in Squeak #1382

Closed theseion closed 1 year ago

theseion commented 1 year ago

Loading Seaside in Squeak trunk causes warnings due to extensions on PackageManifest classes that can't be loaded because the classes don't exist there.

timrowledge commented 1 year ago

The message provided by the installer was

"This package depends on the following classes: PackageManifest You must resolve these dependencies before you will be able to load these definitions: ManifestJavascriptCore ManifestJavascriptCore class>>ruleTempsReadBeforeWrittenRuleV1FalsePositive"

The package/version in the debugger at this point was Javascript-Core-cypress.1 I've been making good use of Seaside in a Squeak 5.3 image for the last several years and I'd like to update.

The potentially interesting side-news is that the overall system works reasonably well but there seems to be no 'style' - ie the displayed web pages look as if they're missing any css stuff. Maybe that's a consequence, maybe it's a separate thing.

Screen Shot 2023-10-28 at 11 34 36 AM

theseion commented 1 year ago

The missing styles are due to a missing method on Integer in Squeak. See #1383.

theseion commented 1 year ago

The issue here is that Pharo uses a subclass of PackageManifest to store linting information, such as rule bans. Banning a rule from the UI will cause a (possibly new) subclass to be compiled in the category Manifest of the respective package, named Manifest<PackageName>.

My suggestion: create a separate package for Pharo to hold the manifest classes. Lookup of linting information works reliably (tested). The only caveat is that manipulation of the linting information (e.g., banning a rule) will move the class back to the respective package.

@jbrichau, what do you think?

jbrichau commented 1 year ago

@theseion You could also fix it by adding PackageManifest class to a Squeak-Seaside-Compatibility package (or whatever name is more appropriate) and have it loaded first. This would not break how it works in our main development environment (Pharo). At some point, we also considered adding these classes to Grease but we did not. In the end, these classes will also disappear in Pharo as the general understanding is that this is not a good way to store this meta-information.

I think adding the superclass to a Squeak-only package to make sure it can get loaded is the cleanest solution. Thanks for picking this up!