dalehenrich / filetree

Monticello repository for directory-based Monticello packages enabling the use of git, svn, etc. for managing Smalltalk source code.
https://github.com/CampSmalltalk/Cypress
MIT License
133 stars 26 forks source link

Cannot load package with no version file inside monticello.meta directory #229

Closed tcj closed 4 years ago

tcj commented 4 years ago

Hi —
 A package with an monticello.meta directory but no version file inside will confuse FileTree, at least in the FileTree version loaded into my Squeak 5.3 image. One such repo is Grease [1].

The issue can be resolved by modifying MCFileTreeAbstractReader>>#hasMonticelloMetadata so the logic uses an #and: rather than an #or:. This fix allows the Grease repo to be browsed in Monticello in Squeak 5.3, but I have no idea if it would break other repos.

The issue seems to be:

As it currently operates, MCFileTreeAbstractReader>>#hasMonticelloMetadata returns true on e.g. the Grease repo[1], so FileTree expects a version file and tries reading from it. In Squeak 5.3, the act of trying to read the version file actually creates the version file with a zero length. The general code flow is like this:

  1. MCFileTreeAbstractReader>>#hasMonticelloMetadata sees the presence of the monticello.meta directory as a sign that the version file should be present;
  2. MCFileTreeAbstractReader tries to read the version file;
  3. MCFileTreeFileDirectoryUtils>>#readStreamFor:in:do: calls FileDirectory>>#fileNamed:do: which, in Squeak, creates a FileStream on a new file (I believe)

Happy to create a pull request to change #and: to #or: if this seems reasonable.

Please note that I am assuming that Monticello metadata (as relates to the monticello.metadata directory) and method metadata (as toggled in the .filetree file) are two different things. If my assumption is wrong, and Monticello metadata and method metadata refer to the same thing, then it may be better to modify MCFileTreeAbstractReader>>#hasMonticelloMetadata so it pays attention to #noMethodMetaData of #packageProperties.

Thanks!

[1] https://github.com/SeasideSt/Grease/tree/master/repository/BaselineOfGrease.package/monticello.meta

tcj commented 4 years ago

Argh. This appears to have been addressed in 2016 in the squeak4.3 branch with the following commits:

30628e0b646eede0823e05fab1c3511aa1c5e982 262b0288b113bca0b3aff1bee005d87916c0c1ff

Now I guess my issue is: why didn't my image load these fixes when loading FileTree in Squeak 5.3?

tcj commented 4 years ago

Perhaps a newer ConfigurationOfFileTree is in order to include these changes for Squeak. I see in #version1062: the following:

spec for: #'squeak' do: [
    spec 
        package: 'MonticelloFileTree-Core' with: 'MonticelloFileTree-Core.squeak43-dkh.169';
        package: 'MonticelloFileTree-FileDirectory-Utilities' with: 'MonticelloFileTree-FileDirectory-Utilities.squeak43-dkh.12'. ].
dalehenrich commented 4 years ago

I think this has been addressed?