pharo-vcs / iceberg

Iceberg is the main toolset for handling VCS in Pharo.
MIT License
133 stars 83 forks source link

Allow to set tonel writer version to use for export #1798

Closed jecisc closed 5 months ago

jecisc commented 5 months ago

This change allows one to set the tonel writer version to use for a project in the .properties file of a project.

For project available on P11 and P12, to avoid to rewrite all the file each time we commit on a different version of Pharo we can provide those properties:

{
    #format : #tonel,
    #version: #'1.0'
}

And the v1 format will be used on both versions of Pharo.

If it does not find the version provided, it will fallback to the default version of the image.

jecisc commented 5 months ago

Failing tests are not related and seems to come from the current state of Iceberg

tinchodias commented 5 months ago

This feature would be useful for Bloc but I think for all projects that work in Pharo 11 and 12. We manually change the default version in TonelWriter class but from time to time we forget in a new image and change unintentionally the tonel version in the changed package.

jecisc commented 5 months ago

Yes! The goal here is to define it once in the properties and then nobody has to care anymore :)

dalehenrich commented 5 months ago

Cyril, just want to mention that there are other users of the tonel format and while it is fun to add new features "as needed" you should consider the impact your changes will have on shared projects ...

There used to be an actual tonel spec that was used for this purpose and the process of maintaining and conforming to the project should allow for moving forward in an orderly fashion ...

I actually am in favor of including version information in "a properties file" but I think the devil is in the details ... for example because of the way that Pharo used to treat symbols and strings as interchangeable (and thanks for your effort in this area!), I was forced change the Rowan tonel implementation to read/write the symbols back out the way they came in an to attempt to preserve the integrity of the original repository ...

So far Pharo and GemStone/Rowan has been able to steer clear of each other in terms of the exact details of the disk representation details for tonel ... but once GemStone/Rowan is released, tonel will be in active use by both GemStone and Pharo in a number of SHARED github repositories and it is imperative that we are tolerant of each others foibles ....

I know that there is a principle in the original tonel spec that all readers/writers preserve class/package/method/repository attributes so that platforms can actually share the code, without sharing feature sets ...

I actually have several RowanSample* repositories where I have created branches with particular disk-based content that I can use in my unit tests ... I created this particular branch a couple of years ago so that I could verify that I could "faithfully preserve" Pharo's use of symbols as keys and I regularly run unit tests using that branch to "maintain conformance to non-standard tonel formats" ... while I haven't looked closely at the Rowan code, it does look like I was able to do that without any special flags on disk ...

I "taught" Rowan to detect and preserve the cases when Symbols were used where Strings were supposed to be used ...

And it seems to me that from a practical perspective, it makes sense to create a shared git repository with a series of branches each containing examples of the unique features of each of the platforms so that unit tests can be written by each platform to confirm that they can faithfully read and preserve on write the disk formats used by the other platforms ....

Proposals for changes would be added as examples and when all of the platforms can successfully read/write the new format, it can be added the mix ... We should be able to create github actions to automatically run the unit tests, so the validation can be automated ... and changes that fail the unit tests would then be the only time it is necessary to talk about changes that would need to require changes to the implementations ...

jecisc commented 5 months ago

Hi Dale,

From what I know this property file I updated is not related to Tonel but to Iceberg that is used by Pharo to store some properties. Maybe I'm wrong but I think this file is ignored in other smalltalk flavors?

So I'm not sure this change anything for Gemstone. But maybe I'm wrong?

dalehenrich commented 5 months ago

@jecisc, well that file looks an awful lot like this properties.st file ... we may already be colliding ... with my addition of a #convention field :) ... I know that there are number of these little files laying around on disk and if we have a repo with the current/active layouts on display, we can make informed decisions about what needs to be preserved and what field names we are using ...

As I think about this file, I think it is currently readonly, so it will be safe to add fields manually until someone decides that they want to write it without preserving fields, then we would be in trouble .....

jecisc commented 5 months ago

Oh I see that you also have it.

In Pharo we are preserving everything that we read in it and we ignore every field that does not interest us.

The problem might be that symbol/strings export but my change does not change anything about this. I think pharo exports symbol/symbol for now which is not good for gemstone :/ This would need to be updated

dalehenrich commented 5 months ago

... and to be more friendly, the unrecognized fields need to be retained IFF the file is ever written ... presumably some process creates/writes that file and that process should be prepared to read/write if updates are needed ...

I really think we need to have a shared repository Sample where we can place a Pharo repository on one branch and a Gemstone repository on another branch and we should ensure that we can properly read/update each sample ... and if there are multiple formats of data, then we need a branch per version ...

We should probably do this for filetree format repositories as well, since they are still used in the wild ...

Having a set of standard format examples is really the only way for us to know that we won't stomp on each other when we read/write ...