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

Package name in Monticello Browser is wrong in Pharo 6 #201

Closed JurajKubelka closed 8 years ago

JurajKubelka commented 8 years ago

If I use fresh Pharo 6 image with my project. At some point the package names are wrong. At the beginning everything is fine, but after some time (several commits) it is wrong.

I use following package names:

GT-Collaboration
GT-Collaboration-UI-Brick
GT-Collaboration-OtherName

And you can see that at some point, part of the package names are written as part of author name.

screen shot 2016-07-29 at 15 55 53

Since then I cannot commit, because the changes are inappropriate.

ThierryGoubier commented 8 years ago

Hi @JurajKubelka,

are the package directory names changed in the git repository itself? I.e. do they become GT-Collaborate instead of GT-Collaboration-UI-Brick? Do you also see the package changing in Nautilus?

JurajKubelka commented 8 years ago

It happens because for some reason a MCWorkingCopy(GT-Collaborate-FogBugz) object has as ancestry (one ancestors) object a MCGitFileTreeVersionInfo(GT-Collaborate-FogBugz-UI-Brick-JurajKubelka.7). Because of that MCWorkingCopy>>description returns '* GT-Collaborate-FogBugz (UI-Brick-JurajKubelka.7)'.

ThierryGoubier commented 8 years ago

Does this means you have both a GT-Collaborate-FogBugz and a GT-Collaborate-FogBugz-UI-Brick packages in your project?

JurajKubelka commented 8 years ago

Hi @ThierryGoubier.

I have just commented a bit of the investigation. If I commit, then GT-Collaborate-FogBugz-UI-Brick.package includes classes from GT-Collaborate-FogBugz.package.

Nautilus does not change. Neither packages inside Nautilus (image).

JurajKubelka commented 8 years ago

Yes, I have both packages in the project. I have following packages:

BaselineOfGTCollaborate.package/
GT-Collaborate-Commands-Tests.package/
GT-Collaborate-Commands.package/
GT-Collaborate-FogBugz-Trantor.package/
GT-Collaborate-FogBugz-UI-Brick.package/
GT-Collaborate-FogBugz.package/
GT-Collaborate-Tests.package/
GT-Collaborate-Trantor.package/
GT-Collaborate-UI-Brick-Tests.package/
GT-Collaborate-UI-Brick.package/
GT-Collaborate.package/
ThierryGoubier commented 8 years ago

I wonder if there are bugs into the handling of packages with the same prefix (GT-Collaborate and GT-Collaborate-UI-Brick)

ThierryGoubier commented 8 years ago

I'll try to reproduce that.

JurajKubelka commented 8 years ago

Using Smalltalkhub I had no such issues. Since I have switched to GIT, I face this issue.

I think it happens right after the first commit. I was able to commit just one package. I have made commit of GT-Collaborate-Trantor.package and since then it is broken.

(I have to leave now, but I will be happy to help investigate it later.)

JurajKubelka commented 8 years ago

You can load my project https://github.com/JurajKubelka/GT-Collaborate and do a commit. But likely a more simple case is possible.

ThierryGoubier commented 8 years ago

Perfect, thanks.

ThierryGoubier commented 8 years ago

I think I have an idea about what it is...

JurajKubelka commented 8 years ago

Hi @ThierryGoubier, so what's the problem is about? Can we write a test case for it? It would be great having one. Do you have minimal steps to reproduce?

ThierryGoubier commented 8 years ago

Hi @JurajKubelka, sorry I haven't been able to work on it. I think I know how to write a test case exhibiting the symptom:

Create a package AAA-BBB, create a package AAA, save the package AAA-BBB, save the package AAA and note that it becomes AAA (BBB-...) in Monticello.

I had a short look and it's not so easy.

JurajKubelka commented 8 years ago

Great! Today I have no change to look at it. I will do best tomorrow.

dalehenrich commented 8 years ago

This relatively new feature of Pharo --- allowing independent packages called AAA and AAA-BBB to be created without collapsing all definitions into AAA and leaving AAA-BBB empty --- is not portable across platforms ... not surprising it is causing trouble from within Pharo itself, but it will just not work correctly in Squeak and GemStone ... Monticello might be broken in this respect, but at least the broken-ness was consistent ... this feature will basically cripple cross-platform projects ... it would have been one thing to let it be generally know that this was planned, but frankly this is the first I've even heard of this type of thing being contemplated and I don't look forward to the coming scramble when it becomes necessary to untangle the coming messes ...

JurajKubelka commented 8 years ago

So, do you prefer not to support it? I had the impression that it is common to use AAA and AAA-Tests packages. But maybe it happens just in Pharo.

dalehenrich commented 8 years ago

@JurajKubelka ... well yes I would prefer to not support it, but it is a current feature of Pharo and therefore it needs to be supported ... I am just lamenting the fact that something that is a fundamental contract for sharing source code across Smalltalk platforms has been "unilaterally broken" without any discussion ... and it is not easily "fixed" in other platforms ... IIRC, it has taken several years for Pharo to get their new RPackage implementation to work without errors (and it may still be buggy :) ...

ThierryGoubier commented 8 years ago

Ok, found the bug. It's present in most types of Monticello repositories, except FileTree (but I reintroduced it in GitFileTree :-1: by copy pasting Monticello bad code...), in #versionInfoFromVersionNamed:.

Should be solved by 062bb79

JurajKubelka commented 8 years ago

@ThierryGoubier Thanks! If it is present in most types of Monticello repositories, is it worth to open a case on FogBugz?

The code change should be the same?

- self versionInfoFromVersionNamed: (each info name copyUpToLast: $.)
+ self versionInfoFromVersionNamed: each package name
ThierryGoubier commented 8 years ago

@JurajKubelka : I don't know. Let me explain the issue:

in http / smalltalkhub / disk repositories, whatever you give as version name to #versionInfoFromVersionNamed:, the repository will match anything using the version name as a prefix, ordered by version number (if you give 'AAA', it will match 'AAA-Core-Tests-ThierryGoubier.25.mcz' for example, even if 'AAA-ThierryGoubier.23.mcz' exists).

In FileTree and GitFileTree as corrected, AAA will match the last version of the AAA package. It behaves as if it was in fact #versionInfoFromPackageNamed: . But trying with AAA-ThierryGoubier.23 will fail.

So you have two different semantics for the same API, the FileTree one and what I'd call the mcz one. Code calling that API (except GitFileTree) uses the mcz semantic ( @dalehenrich : this is the case of your Metacello code, for example). But any improperly written code for that semantic (passing a package name, for example) may silently select a wrong package... The FileTree semantic is a lot safer, but is not the correct one :(