instantiations / tonel-vast Goto Github PK

When writing a pool dictionary the order of the keys is not the same as the order in which the declaration method might have defined it.

While trying to import https://github.com/ba-st/HighchartsSt/releases/tag/v10.0.0 into VAST 9, a General Protection Fault is stopping the process.

This happens only with the larger packages like HighchartsSt-V8, the prerrequisites are imported correctly.

| reader |
reader := TonelReader new readPackagesFrom:  ((CfsPath named: CfsDirectoryDescriptor getcwd) append: 'inTonelFormat').
 (TonelLoader on:  reader) loadApplicationsNamed: #('HighchartsStV8').

We have a few custom VAST attributes such as vaParent, vaPrerequisites, vaCategories etc. at all different types: methods, classes and packages. When these attributes are read in other Smalltalk dialects, they are ignored, and when written back to disk they are lost. Therefore, when you import back in VAST, you lost them.

One possible way to deal with this is to have all our custom metadata stored in separate files/directories so that other Smalltalks would just ignore it but we don't loose it. Could be:

1. All our metadata in another file but rather than .st a .vast or vst where the file is together with the original. For example, TFDevice.class.st and TFDevice.class.vast.

2. We could still use .st (or .vast or vst) but store these files in a separate directory of the root. Just like in .project we specify the source we could have another attribute that specifies the VAST metadata directory. Could be something like /vast.

I think Rowan does this as well so that all his metadata is not lost between Pharo and GemStone

And let the more complex / portable as an option.

In the TonelLoaderBaseEditionStrategy the default behavior is to list base editions in reverse chronological order, this might cause that the latest edition of an app is not versioned and then when loading from Tonel that edition is taken as base to load code into.

When looking up base editions there should be an option to lookup only for versioned editions, and this option should be by default.

Currently the loader only creates a new edition for classes and subapplications if there are changes affecting it.

So far, we are still depending on MonticelloImporter because we reuse a Pool Dictionary defined by it. The idea is to have our own pool and remove the dependency with it.
Also, remember to update map and remove required map.
In addition, we should be able to cleanly remove the prereq EtToolsVendorExtensionsApp from TonelTools app.

Currently there is confict when a class in a Tonel package has the same same name as the application of the package (e.g. package BaselineOfSomeApp that contains the BaselineOfSomeApp class).

This is related with #34, where we might define a suffix for such cases, or classes that are going to be ignored when imported.

Currently we only provide contextual menus to load applications from the Application Manager, we should also add the option to Configurations Maps browsers (for config maps) and the Visual Organizer (for apps).

The current loading of Applications will create unneeded editions when the loaded application is the same as the one in the EM Library

Right now we only support apps as the Tonel unit to write and read but on the future, we would also need to be able to do this at Configuration Map level. For this, we have discussed a lot with Dale Henrichs and see if we can use the same idea of Rowan about "configs".

I am seeing the following behavior... with normal classes, all class side methods seem to be exported and imported correctly. However, with the class side methods of the first class Apps, there is an issue.
The writer does correctly write all class side methods, for example, this. However, when loading that very same version, all those class side methods are deleted and in addition a new #_TONEL_packageName is created.

I am not sure yet what happens with instance side methods.

Class comments are not being imported, and only a this is a test is set by default.

When using the Tonel application import option if you select several applications to import that depends on themselves you cannot specify it on the pre-requisites step.
For example:
I have two packages Buoy-Assertions and Buoy-Conditions, and Buoy-Assertions depends on Buoy-Conditions. When I try to import all this packages at the same time since the importer offers to select the pre-requisites of Buoy-Assertions first and Buoy-Conditions is still not imported I cannot select it as pre-requisite.

Maybe an option will be to include all the future applications I will import in the pre-requisites dialog, so I can already configure the pre-requisites right without having to create a new version of the application, once all the packages are imported.

If you try to import an existing application and the logged user is not the same as the manager of the application you get a Walkback. (Even when MonticelloLoader appSuperGroupSetting: #world ) .
See attached stack trace:
Manager-stack.txt

Steps to reproduce:

1. Create and version an application with the same of some app to import (by default it will be managed by LibrarySupervisor) and change the privileges so superGroup = #world.
2. Create a new user and login with it
3. Configure MonticelloLoader appSuperGroupSetting: #world
4. Try importing a package that will end up in that application, and you will get something like: 'Base edition for BuoySUnitModel is managed by Supervisor. You are currently gcotelli and cannot create new editions of the application.'

It seems that the importer doesn't take into account the privileges, just check that the current user is the same as the application manager, even if the current user will have the required permissions to create and release editions in the app.

This is the opposite of #1, when writing a _PRAGMA_SomePool output, it should be written as a subclass of SharedPool instead of a VAST declaration method.

When loading a SharedPool subclass whose class side initialize method is factored to call other methods, the initialization of the shared pool fails because these methods are not loaded or not done in the proper order.

Take as example this shared pool

If a method source has a LF as separator between the selector and method body, the TonelWriter will write an extra CRLF after that, causing that when the method is loaded back, it is treated as a difference.

E.g.

someSelector<LF>
    ^self

Is written as

someSelector<LF>
<CRLF>
    ^self

So when it is read back, there is an extra line after the selector, causing the loader to create a new edition for the method (and hence the class/extension).

As a reference, when using the fileout from VAST, it replaces the original <LF> by a single <CRLF>

But we can leave this for the time the project gets into the base product

The configuration map "Test Tonel" version 1.0 references config map Tonel version 1.48, but latest version is 1.49

Test Tonel 1.0
==============
Applications:
	TonelLoaderTests 1.5
	TonelReaderTests 1.30
	TonelWriterTests 1.31
Required Maps:
	Tonel Missing - (26.03.2020 17:52:41)
	z.ST: SUnit V 9.2.1  [457]

When importing a Package, the method #_TONEL_packageName is created again, even if such method already exists with the same content.

As with #39 when loading, we need a strategy to "flatten" an Application Subapps into a single Tonel Package, where each class defined in a Subapp is assigned to a single package and moved into a category.

E.g. MyApplication contains a Core SubApplication, then the classes defined in Core will exported as part of the MyApplication package, with the category MyApplication-Core.

Both the application and subapplication should know how to map to a specific tag if defined (so CoreSubApp could be converted to whatever CoreSubApp class>>_tonelTagName returns (in case there is an App/SubApp prefix defined in #34).

Currently if you import a package named MyPackage-Core it will be converted to the application MyPackageCore because the class name can't have dashes in it, but if you export this back to tonel, then the Package will be named MyPackageCore.

We should keep the original name somewhere, e.g. in an application class side method called tonelPackageName, this way we can lookup for the package name, and if it's not defined it will fall back to the original application name.

When loading a package that has a class extension that is not in the image it currently raises an error because the class was not found.

Determine what to do in these cases. E.g. the class might exist as part of an application in the library, or the extension could be omitted (and properly logged).

I found this while trying to load BuoyConditions from issue #12.

| reader |
reader := TonelReader new readPackagesFrom:
      ((CfsPath named: CfsDirectoryDescriptor getcwd)  
         append: '..\TonelRepositories\Buoy').
(TonelLoader on:  reader) loadApplicationNamed: 'BuoyConditions'

Import configuration map terminates with error message below

Some classes such as TonelLoaderSubApplication have lots of methods, most of them are categorized, but some are not.

Categorize methods Not categorized and determine which methods must be Public and which Private

Right now, what complicates me the most to use the Tonel reader is the popups and user needs. I propose to be able to plug a headless behavior that does:

1. Allow specifying a version (same) to be used when versioning all apps, classes, etc. If it is a git directory then assume the git hash etc (exactly as what the popup offers right now).
2. For base version, just assume the imported version is a root (no parent). Makes sense?
3. Assume the prereqs are fine (don't popup to add/remove)
4. add an explicit boolean to control whether to version or not (rather than using nil as version to define we want open editions).

Of course, it would be nice if all these things could be controlled/specified/set based on each user needs.

We need a setting to define a way to define global settings that are going to be used by default when using the context menu or by all instances of TonelLoader/TonelWriter.

In these settings will be the preferred strategy classes, app suffix, user, etc.

When loading a Tonel package in Pharo, it will organize the classes into tags by looking at the category of the class. E.g. MyPackage-Core, MyPackage-Something.

Implement a loader strategy to create Core and Something as SubApplications of MyPackage, and another strategy to load the class as part of MyPackage application (as it currently is).

We need to preserve the current behavior because for VAST "native" subapplications exported to Tonel we need to honor where the class was defined, regardless of its category.

The TonelWriter has a lineDelimiter variable that is not honored when exporting an Application to a Tonel file.

Right now we only have a small piece of code that looks for the hash and timestamp of the git commit to use it as default... that is nice. But in the future we would need to move this to a strategy that we can pass into the loader or something similar...because we don't want to couple FORMAT with git or tooling..

If one tries to load a pool dictionary exported in the converted shared pool format in an image where there currently is the same pool dictionary defined using the conventional %%PRAGMA DECLARE declaration method, the loader complains about being unable to declare the keys (Errors 402 and 404)

Things can be quite different if we are targeting VA to VA only or VA - Pharo (or any other dialect). Keys that are not necessary to be written or extra effort that would need to be done and typical cases for this.

The idea is that we could have an object "TonelConvention" or something with concrete instances / subclasses like VAOnly or CrossPlatform. This convention object will be used when writing and reading and affecting the process. In addition, the convention used could be stored as a key in the package.st. For example:

   #convention: 'VA-ENVY'

Below is a list with SOME of the things I have already identified:

• Method key #category is not necessary for VA only as we have #vaCategories
• Extension method key #category : '*TonelExampleForPharoApp' is NOT necessary for VA.. I mean, the $* preffix neither the package as a category. On VA we can use the real category for it.
• Class key #category is not necessary for VA only.
• Class types "class types: bytes, words, pointers, variable... " are not uniform...we need to deal with this if we want cross platform.
• Shared Pools are different in other dialects (subclass of SharedPool) than in VA (_PRAGMA* method). We need to deal with this if we want cross platform.

For this, we can also check Rowan as it also includes this concept of Convention.

Currently Pool Dictionaries not coming from VAST output (in the form of _PRAGMA_xxx methods) are not imported (although there is partial support in the code for it).

The current behavior for Shared Pool definitions is to export them as subclasses of SharedPool and when importing to define the pool in the context of the Application being created and and initialize method in that Application class too, creating a _PRAGMA_SharedPoolName_initialize method as described by the shared pool documentation.

But when exporting pools defined by methods such as _PRAGMA_IS_ that are defined in the context of a particular class, we don't have any way to know to which class it should be moved to, since Tonel (and other dialects in particular) don't have the concept of associating the definition of a shared pool to a particular class.

One possible option is to add a metadata to the SharedPool Tonel class definition, something like #vaDefinitionClass:.

In order to ease the use of VAST only use of Tonel, we can disable the convertion of declaration methods into SharedPool subclasses, then when loading no amending of definitions must be performed on the load, just a regular method load.

This will work as a workaround to #36

The method #tonelSingleEdgeTopologicallySortedCollection relies on topologicalSubordinateOf: implementors that come from another package.

This is what I see when I edit the package.st file in vim on linux:

I've been looking at the vast package properties for several days now and only just noticed that the
#vaSubApplications field is an array of dictionaries. The big blue ^M blinded me to the fact that it was simple STON format:)

This was discussed here.
This would help bringing more complex projects as well as projects you didn't write and hence, you don't know the dependencies and order of load.

We could include this as a feature in the Pharo2VA project.

Please cleanup the Tonel.dat library, config map Tonel version 1.52 is duplicated with different versions of application TonelWriterModel.

Tonel 1.52
==========
Applications:
	AbtShellProgramStarterApp 1.0
	TonelBaseApp 1.1
	TonelFileSystem 1.23
	TonelLoaderModel 1.58
	TonelReaderModel 1.49
	TonelTools 1.11
	TonelWriterModel 1.54
Required Maps:
	z.ST: STON Support V 9.2.1  [457]

Tonel 1.52
==========
Applications:
	AbtShellProgramStarterApp 1.0
	TonelBaseApp 1.1
	TonelFileSystem 1.23
	TonelLoaderModel 1.58
	TonelReaderModel 1.49
	TonelTools 1.11
	TonelWriterModel 1.53
Required Maps:
	z.ST: STON Support V 9.2.1  [457]

For more details see thread "Problema con Tonel y Carrillon".

Currently the Tonel writer creates the classes using the following category RootApplicationName-SubApplicationName.

So if you have TonelExampleApplication with a subapp TonelExampleSubapp, the classes defined in the later will have the category TonelExampleApplication-TonelExampleSubapp.

This is fine, but once you load them into Pharo it will create two packages: TonelExampleApplication and TonelExampleSubapp, but the classes categorized as TonelExampleApplication-TonelExampleSubapp will be moved to the TonelExampleApplication package.

So we need to output the class category with the containing App/Subapp as the root category.

I was trying to use the new tonel importer to import some packages I have in Pharo and I'm getting the following error:

Debugger Stack Trace Report:
Error String: 'UndefinedObject does not understand methodDictionary'
Resumable: true

Find attached the stack trace:
stack.txt

It seems that the extensions and classes are not created in the application, so the method loading step is failing.

Steps to reproduce:

• Download VAST ecap-2
• Clone https://github.com/ba-st/Buoy and checkout v5.1.0 tag
• Use the import options and try to import all the packages except the Baseline and Development Tools ones.
• Put v5.1.0 as version when requested
• Let Kernel as the first application pre-requisites selection

VAST version info:
VA Smalltalk V9.2ECAP2 (64-bit); Image: 9.2 [433]
on Linux Mint

When parsing a class that has no method definition, the class is ignored completely.

Currently, #loadWithRequiredMaps is only looking by version name:

 (EmConfigurationMap editionsFor: self name)
		detect: [:one | one versionName = self versionName]
		ifNone: [

However, its quite common and likely for versions to be renamed. Therefore, the lookup should be:

Look for versions with exact timestamp...if only one, pick that one, regardless of version name. If more than one, try to detect if there is one with same version name as specified.

Now that we are discussing the integration of the Rowan disk structure into tonel-vast support. I would like to discuss the pros and cons of the use of the package properties:

• vaParent
• vaPrerequisites
• vaSubApplications

In Rowan V1.2, we have package-specific properties, but instead of embedding the properties in the package.st file, I chose to put the package-specific properties into a configuration file located in a rowan/configs directory.

For Rowan V2.0, I have continued to follow the same model with package-specific properties located in a rowan/v2/components directory. The default values for the package-specific properties are located in a load specification. The defaults are used when the package-specific property is not explicitly declared.

I don't have the option of changing disk format for V1.2, but for V2.0 I do have the option of changing the disk format and I have been considering moving the package-specific properties into the packages.st file, but there are some cons to doing so and these cons are worth sharing with you, since I think they have the same impact on VAST.

The primary con to adding package specific properties is the impact on ones ability to seamlessly add Rowan support to existing git-based projects like Seaside. If the on-disk format of a package is written by Rowan with additional package properties, existing users of those packages (Pharo, Squeak, and GLASS) will not preserve the additional package properties and it would be impossible to use Rowan with any existing project until Pharo, Squeak, and GLASS have adopted the Rowan package format. Seaside uses filetree format, but by isolating the package-specific properties in the rowan directory structure it becomes practical for Rowan to support both filetree and tonel based projects --- seamlessly.

If VAST moved their package-specific properties into the components files, it would become possible for VAST to support the reading and writing of filetree format and make it possible for VAST users to use the Seaside project directly without waiting for Seaside to convert to tonel. If VAST leaves the package-specific properties in the package.st file, then Rowan could/should move it's package-specific properties into the package.st file as well ...

This argument works the other way as well. If VAST chooses to use the Rowan component structure, it is less necessary to have to change the existing package disk format, since there are alternate locations to store the VAST package meta data.

With that said I am not wedded to the existing component file format and I am very open to different component structure suggestions.

Sooo, I'm raising of the issue here for discusion ... I don't want to make a unilateral decision one way or the other and I think that a joint discussion is warranted (@kurtkilpela and @martinmcclure please chime in).

Add a new TonelLoaderGitParentBaseEditionStrategy (or whatever name) that rather than use "Latest" it looks into the ENVY manager if there is a version matching the parent of the current commit hash. For this we would need to investigate how to read the parent of the current commit with the file system.

Currently a package named Package-Tag will be mapped to an application (or subapplication) named PackageTag, this is fine to import code from Pharo, but if you export the content back to Pharo it will be PackageTag losing the original name.

We could keep the original name during the load and assign it to the app/subapp as a class side tonelPackageName that we could use afterwards to write the package name when exporting.

For Instantiations, backward compatibility with customers is important. Therefore, we should store (maybe as part of the /source/.properties) a vaFormatVersion: "1.1" kind of attribute were we write down out VAST format version in which that project was written. This would allow us write migration tools or things like that if the format changes over time.

If the Tonel files are located in a git repo, along the reading of the commit hash (currently implemented) read the timestamp of the current commit.