tim Rowledge uploaded a new version of Monticello to project The Trunk: http://source.squeak.org/trunk/Monticello-tpr.791.mcz
==================== Summary ====================
Name: Monticello-tpr.791 Author: tpr Time: 31 July 2023, 1:08:42.078331 pm UUID: 4c75461c-f5f3-4b22-8730-a255e7a8139f Ancestors: Monticello-eem.790
Since the filebased repository requiresd the mcz extension when trying to load versions, let's actually try to add it. This chage allows comparing versions within a repository inspector that is examining your local package-cache directory, for example. I can't find any way that it makes anything less-good so far.
=============== Diff against Monticello-eem.790 ===============
Item was changed: ----- Method: MCFileBasedRepository>>versionNamed: (in category 'versions') ----- versionNamed: aMCVersionName "For FileBased repositories, aMCVersionName must have the appropriate extension!! :-(" + | version fileName | - | version | version := self cache at: aMCVersionName ifAbsent: + [[fileName := (aMCVersionName endsWith: '.mcz' ) + ifFalse:[aMCVersionName, '.mcz'] + ifTrue:[aMCVersionName]. + self loadVersionFromFileNamed: fileName ] - [ [ self loadVersionFromFileNamed: aMCVersionName ] on: FileDoesNotExistException , NotFound do: [ : err | nil ] ]. self resizeCache: cache. (version notNil and: [ version isCacheable ]) ifTrue: [ cache at: aMCVersionName asMCVersionName put: version ]. ^ version!
On Mon, Jul 31, 2023 at 08:08:44PM +0000, commits@source.squeak.org wrote:
tim Rowledge uploaded a new version of Monticello to project The Trunk: http://source.squeak.org/trunk/Monticello-tpr.791.mcz
==================== Summary ====================
Name: Monticello-tpr.791 Author: tpr Time: 31 July 2023, 1:08:42.078331 pm UUID: 4c75461c-f5f3-4b22-8730-a255e7a8139f Ancestors: Monticello-eem.790
Since the filebased repository requiresd the mcz extension when trying to load versions, let's actually try to add it. This chage allows comparing versions within a repository inspector that is examining your local package-cache directory, for example. I can't find any way that it makes anything less-good so far.
I think this is breaking the update stream after Squeak6.1alpha-22687-64bit.zip
The issue is that .mcm configuration maps are versions that end with file extension mcm instead of mcz, and we need to locate the mcm update map version in the repository when the updater is running.
The Monticello-tpr.791 change will append '.mcz' to the MCVersionName so it ends up looking for update.dtl.558.mcm.mcz in the repository when it should be looking for update.dtl.558.mcm. The update map cannot be found and "Update Squeak" fails.
Dave
On 2023-08-02, at 8:06 PM, David T. Lewis lewis@mail.msen.com wrote:
I think this is breaking the update stream after Squeak6.1alpha-22687-64bit.zip
Well that sucks. I didn't spot that usage when trying to find potential problems.
The problem is/was that the package names didn't include the full filename and so trying to compare package versions in a file based repository crashed because it would try to find fooblePackage-woz.7678 and fail. Now we're trying to find a Wrong Thing a different way; how exciting!
Perhaps if we invert the test to see if there is no extension at all? That has a problem that we can't simply check for $. because we also use that in the version number part of the name. So maybe use the exception and retry with the '.mcz' added? Do we ever end up trying to handle '.mcm' or '.mcd' files where the package doesn't tell us which it is?
Should I simply delete that package version from trunk? I see that the latest update-dtl.558.mcm doesn't include it... but the updater goes and looks for any newer packages anyway.
Since it has broken the pre-built zip I guess it (the zip) has to be removed and the faulty mcz deleted. Any other corrections needed?
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Never forget: 2 + 2 = 5 for extremely large values of 2.
On 2023-08-03, at 10:04 AM, Tim Rowledge tim@rowledge.org wrote: So maybe use the exception and retry with the '.mcz' added? Do we ever end up trying to handle '.mcm' or '.mcd' files where the package doesn't tell us which it is?
Handling the exception looks like a decently clean approach
versionNamed: aMCVersionName "For FileBased repositories, aMCVersionName must have the appropriate extension! Try to handle that " | version | version := self cache at: aMCVersionName ifAbsent: [[self loadVersionFromFileNamed: aMCVersionName ] on: FileDoesNotExistException , NotFound do: [:err| "in some cases we try to load package versions where aMCVersionName does not include the filename extension. Try again with the mcz extension; we might need to be even cleverer?" err return: (self loadVersionFromFileNamed: aMCVersionName, '.mcz')] ]. self resizeCache: cache. (version notNil and: [ version isCacheable ]) ifTrue: [ cache at: aMCVersionName asMCVersionName put: version ]. ^ version
Note that we are caching based on the incoming package name whatever it is, rather than the actual name of the found package. It seems to work though. The above works ok for comparing mcz with mcd etc within a repository whether my local or trunk, so that seems good.
What it doesn't improve is comparing mcm versions, but that's because MCRepositoryInspector>>#versionListMenu: carefully works to strip the extension off the name.
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Oxymorons: Resident alien
On Thu, Aug 03, 2023 at 10:04:20AM -0700, Tim Rowledge wrote:
On 2023-08-02, at 8:06 PM, David T. Lewis lewis@mail.msen.com wrote:
I think this is breaking the update stream after Squeak6.1alpha-22687-64bit.zip
Well that sucks. I didn't spot that usage when trying to find potential problems.
The problem is/was that the package names didn't include the full filename and so trying to compare package versions in a file based repository crashed because it would try to find fooblePackage-woz.7678 and fail. Now we're trying to find a Wrong Thing a different way; how exciting!
Ah, I think I see it now, never noticed it before. Just to confirm my understanding: If I open a file based repository, then look at the "History" for any given version, then within the history browser I try to "view change to <some version>" or "view changes from <some version>" then I end up in a debugger. That's the error condition, right?
Perhaps if we invert the test to see if there is no extension at all? That has a problem that we can't simply check for $. because we also use that in the version number part of the name. So maybe use the exception and retry with the '.mcz' added? Do we ever end up trying to handle '.mcm' or '.mcd' files where the package doesn't tell us which it is?
I'm not sure but it sounds like you might be on the right track with that.
Should I simply delete that package version from trunk? I see that the latest update-dtl.558.mcm doesn't include it... but the updater goes and looks for any newer packages anyway.
Since it has broken the pre-built zip I guess it (the zip) has to be removed and the faulty mcz deleted. Any other corrections needed?
My suggestion - never delete a package from trunk unless it has only been there for a few minutes and you are very certain that nobody has noticed it yet. It is better to just revert the method and commit a new Monticello-tpr.792 to replace Monticello-tpr.791.
That is what git does, and it is fundamentally the Right Thing to Do. Never try to rewrite history, just move forward with a new commit.
Yes I know this is very inefficient in Monticello, but that's not your fault. If we want to fix it then we should fix Monticello to track history efficiently.
Dave
On 2023-08-03, at 2:10 PM, David T. Lewis lewis@mail.msen.com wrote:
My suggestion - never delete a package from trunk unless it has only been there for a few minutes and you are very certain that nobody has noticed it yet. It is better to just revert the method and commit a new Monticello-tpr.792 to replace Monticello-tpr.791.
That is what git does, and it is fundamentally the Right Thing to Do. Never try to rewrite history, just move forward with a new commit.
Forward into the tar-pit! :-)
OK, .792 seems to solve the problem. I downloaded a 22687 image and it updates to include the new version ok. BUT a 22692 image doesn't because it has the broken version that won't let it update. Manually update the Monticello package and all is ok.
I don't have any knowledge of the image build process these days. What do we need to do to fix that part?
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Useful random insult:- Puts a finger in his ear so the draft through his head isn't annoying.
What do we need to do to fix that part?
Nothing. Just ignore the 22692 build. We already have a 22964 one from last night :-)
http://files.squeak.org/trunk/
http://files.squeak.org/trunk/Squeak6.1alpha-22694-64bit/
Best, Marcel Am 04.08.2023 01:04:40 schrieb Tim Rowledge tim@rowledge.org:
On 2023-08-03, at 2:10 PM, David T. Lewis wrote:
My suggestion - never delete a package from trunk unless it has only been there for a few minutes and you are very certain that nobody has noticed it yet. It is better to just revert the method and commit a new Monticello-tpr.792 to replace Monticello-tpr.791.
That is what git does, and it is fundamentally the Right Thing to Do. Never try to rewrite history, just move forward with a new commit.
Forward into the tar-pit! :-)
OK, .792 seems to solve the problem. I downloaded a 22687 image and it updates to include the new version ok. BUT a 22692 image doesn't because it has the broken version that won't let it update. Manually update the Monticello package and all is ok.
I don't have any knowledge of the image build process these days. What do we need to do to fix that part?
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Useful random insult:- Puts a finger in his ear so the draft through his head isn't annoying.
On 2023-08-04, at 2:19 AM, Marcel Taeumel via Squeak-dev squeak-dev@lists.squeakfoundation.org wrote:
What do we need to do to fix that part?
Nothing. Just ignore the 22692 build. We already have a 22964 one from last night :-)
Excellent; I wasn't sure where each overnight build started from. 22964 seems fine,
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Oxymorons: Religious tolerance
Hi Tim and all,
I wanted to point out that MCVersionName is a first-class object with a primary responsibility to abstract away the "Name" from the filename. This is important for the support of multiple Repository types. First class Names are rare, so it's easy to forget it's there, and that there should not be any code that expresses concerns about filename semantics for MC Version names.
Regards, Chris
On Thu, Aug 3, 2023 at 12:04 PM Tim Rowledge tim@rowledge.org wrote:
On 2023-08-02, at 8:06 PM, David T. Lewis lewis@mail.msen.com wrote:
I think this is breaking the update stream after
Squeak6.1alpha-22687-64bit.zip
Well that sucks. I didn't spot that usage when trying to find potential problems.
The problem is/was that the package names didn't include the full filename and so trying to compare package versions in a file based repository crashed because it would try to find fooblePackage-woz.7678 and fail. Now we're trying to find a Wrong Thing a different way; how exciting!
Perhaps if we invert the test to see if there is no extension at all? That has a problem that we can't simply check for $. because we also use that in the version number part of the name. So maybe use the exception and retry with the '.mcz' added? Do we ever end up trying to handle '.mcm' or '.mcd' files where the package doesn't tell us which it is?
Should I simply delete that package version from trunk? I see that the latest update-dtl.558.mcm doesn't include it... but the updater goes and looks for any newer packages anyway.
Since it has broken the pre-built zip I guess it (the zip) has to be removed and the faulty mcz deleted. Any other corrections needed?
tim
tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Never forget: 2 + 2 = 5 for extremely large values of 2.
On 2023-08-07, at 8:38 PM, Chris Muller asqueaker@gmail.com wrote:
Hi Tim and all,
I wanted to point out that MCVersionName is a first-class object with a primary responsibility to abstract away the "Name" from the filename. This is important for the support of multiple Repository types.
This is certainly true, but it is also true that two of the most used repository types absolutely rely upon filenames. We thus have an unfortunate tension between clean design and dirty implementation details that needs to be handled sensibly.
I'm not sure what the best answer is here right now. The repository certainly knows that it relies upon actual filenames, so we certainly have an argument for accepting that and handling them properly. We kindasorta do that with the cache of package names and perhaps that could be expanded upon? Each package name object is derived from the filename in this case, so perhaps making more use of that would help.
For example, if you click on a .mcd filename in the repository list, that name/file connection seems to end up in the cache and if you later try to compare some other version to it the cache already has useful info. Contrariwise, trying to compare a cached package to a non-cached is what can trigger the error I am trying to fix. I'm sure we can find a good solution to this with some thought.
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Useful random insult:- Has a pulse, but that's about all.
On 2023-08-02, at 8:06 PM, David T. Lewis lewis@mail.msen.com wrote:
On Mon, Jul 31, 2023 at 08:08:44PM +0000, commits@source.squeak.org wrote:
tim Rowledge uploaded a new version of Monticello to project The Trunk: http://source.squeak.org/trunk/Monticello-tpr.791.mcz
I think this is breaking the update stream after Squeak6.1alpha-22687-64bit.zip
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Oxymorons: Temporary tax increase
Hi Tim,
I think there was another regression with Monticello-tpr.791.mcz/Monticello-tpr.792.mcz which now raises a NotFound exception for absent versions (previously it just answered nil). This behavior is documented in the superclass, and SIT depends on it. I uploaded Monticello-ct.793 to the inbox which attempts to fix that. Would you mind reviewing the fix? :-)
Best, Christoph
--- Sent from Squeak Inbox Talk
On 2023-07-31T20:08:44+00:00, commits@source.squeak.org wrote:
tim Rowledge uploaded a new version of Monticello to project The Trunk: http://source.squeak.org/trunk/Monticello-tpr.791.mcz
==================== Summary ====================
Name: Monticello-tpr.791 Author: tpr Time: 31 July 2023, 1:08:42.078331 pm UUID: 4c75461c-f5f3-4b22-8730-a255e7a8139f Ancestors: Monticello-eem.790
Since the filebased repository requiresd the mcz extension when trying to load versions, let's actually try to add it. This chage allows comparing versions within a repository inspector that is examining your local package-cache directory, for example. I can't find any way that it makes anything less-good so far.
=============== Diff against Monticello-eem.790 ===============
Item was changed: ----- Method: MCFileBasedRepository>>versionNamed: (in category 'versions') ----- versionNamed: aMCVersionName "For FileBased repositories, aMCVersionName must have the appropriate extension!! :-("
- | version fileName |
- | version | version := self cache at: aMCVersionName ifAbsent:
[[fileName := (aMCVersionName endsWith: '.mcz' )
ifFalse:[aMCVersionName, '.mcz']
ifTrue:[aMCVersionName].
self loadVersionFromFileNamed: fileName ]
self resizeCache: cache. (version notNil and: [ version isCacheable ]) ifTrue: [ cache at: aMCVersionName asMCVersionName put: version ]. ^ version![ [ self loadVersionFromFileNamed: aMCVersionName ] on: FileDoesNotExistException , NotFound do: [ : err | nil ] ].
OK, I see what you mean now. Your change is fine to solve that part of the issue but as I've said before we need to be cleverer here.
And don't forget you'll need to reintegrate it with your .794 version too!
On 2023-08-17, at 12:54 AM, christoph.thiede@student.hpi.uni-potsdam.de wrote:
Hi Tim,
I think there was another regression with Monticello-tpr.791.mcz/Monticello-tpr.792.mcz which now raises a NotFound exception for absent versions (previously it just answered nil). This behavior is documented in the superclass, and SIT depends on it. I uploaded Monticello-ct.793 to the inbox which attempts to fix that. Would you mind reviewing the fix? :-)
Best, Christoph
Sent from Squeak Inbox Talk
On 2023-07-31T20:08:44+00:00, commits@source.squeak.org wrote:
tim Rowledge uploaded a new version of Monticello to project The Trunk: http://source.squeak.org/trunk/Monticello-tpr.791.mcz
==================== Summary ====================
Name: Monticello-tpr.791 Author: tpr Time: 31 July 2023, 1:08:42.078331 pm UUID: 4c75461c-f5f3-4b22-8730-a255e7a8139f Ancestors: Monticello-eem.790
Since the filebased repository requiresd the mcz extension when trying to load versions, let's actually try to add it. This chage allows comparing versions within a repository inspector that is examining your local package-cache directory, for example. I can't find any way that it makes anything less-good so far.
=============== Diff against Monticello-eem.790 ===============
Item was changed: ----- Method: MCFileBasedRepository>>versionNamed: (in category 'versions') ----- versionNamed: aMCVersionName "For FileBased repositories, aMCVersionName must have the appropriate extension!! :-("
- | version fileName |
- | version | version := self cache at: aMCVersionName ifAbsent:
[[fileName := (aMCVersionName endsWith: '.mcz' )
ifFalse:[aMCVersionName, '.mcz']
ifTrue:[aMCVersionName].
self loadVersionFromFileNamed: fileName ]
self resizeCache: cache. (version notNil and: [ version isCacheable ]) ifTrue: [ cache at: aMCVersionName asMCVersionName put: version ]. ^ version![ [ self loadVersionFromFileNamed: aMCVersionName ] on: FileDoesNotExistException , NotFound do: [ : err | nil ] ].
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Oxymorons: Clearly misunderstood
squeak-dev@lists.squeakfoundation.org