The Inbox: System-ct.1523.mcz

Christoph Thiede uploaded a new version of System to project The Inbox:
http://source.squeak.org/inbox/System-ct.1523.mcz

==================== Summary ====================

Name: System-ct.1523
Author: ct
Time: 5 July 2026, 1:57:39.933363 am
UUID: 79117f52-9291-4c0d-8d62-9bfc99b78e68
Ancestors: System-ct.1522

Fixes multi-environment issues for file-ins (that is, when using Environment>>beCurrentDuring: around a file-in operation). With this patch, changesets and MCZ/SAR archives can be correctly installed into a different environment.

Previously, all chunks in a file-in were evaluated in the default environment aka Smalltalk, since the Compiler determines the environment from the class of the receiver, which is nil in this case. However, class definitions/ClassBuilder already honored different current environments before. As a consequence, subsequent chunks would not find previously defined classes or other globals from preambles.
To fix this, this patch explicitly passes the current environment to the compiler.

However, arguably this is a breaking change, considering the following hypothetical example:
	(Environment withName: 'test') beCurrentDuring:
		['Installer merge: ''aMonticello-version.123'' "i.e., triggers Monticello load"' readStream fileIn].
Even though this would load the Monticello version into a separate environment (assuming the loaded version does not depend on other bindings such as Object), such code might have relied on the bindings used in the file-in script (here: Installer), to be resolved in the default environment Smalltalk, not in the current environment. With this patch, this example would no longer work but would have to be rewritten:
	(Environment withName: 'test') beCurrentDuring:
		['Installer merge: ''aMonticello-version.123'' "i.e., triggers Monticello load"' readStream fileInIntoEnvironment: Smalltalk].
The SARInstaller constitutes a real instance of this situation, as preambles of SAR files refer to the Monticello classes MCVersionLoader and MCMczReader. This patch updates the SARInstaller to correctly specify the environment in which Monticello is installed.

Without making this breaking change, it seems impossible to me to support automatic multi-environment support for file-ins by supporting the existing scheme (CurrentEnvironment). An alternative could be to have #fileInFor:announcing: default to Environment default (Smalltalk) unless users opt-in to a different target environment. However, this would be incompatible with other code loaders such as Monticello which adhere to CurrentEnvironment and cement that incompatibility.
(As a side note, it might also be worth discussing whether the compiler should in some way support the Environment scheme rather than falling back to nil environment (Smalltalk) for do-its that do not target a receiver.)

(Reviewer note: The effective change in fileInFor:announcing: is that (Compiler evaluate: ch for: client logged: true) gets replaced with (Compiler new evaluate: chunk in: nil to: client environment: anEnvironment notifying: nil ifFail: [] logged: true).)

=============== Diff against System-ct.1522 ===============

Item was added:
+ ----- Method: PositionableStream>>fileInAnnouncing:environment: (in category '*System-Changes-fileIn/Out') -----
+ fileInAnnouncing: announcement environment: anEnvironment
+ 
+ 	^ self fileInFor: nil announcing: announcement environment: anEnvironment!

Item was changed:
  ----- Method: PositionableStream>>fileInFor:announcing: (in category '*System-Changes-fileIn/Out') -----
  fileInFor: client announcing: announcement
- 	"This is special for reading expressions from text that has been formatted 
- 	with exclamation delimitors. The expressions are read and passed to the 
- 	Compiler. Answer the result of compilation.  Put up a progress report with
-      the given announcement as the title."
  
+ 	^ self
+ 		fileInFor: client
+ 		announcing: announcement
+ 		environment: Environment current!
- 	| val |
- 	announcement 
- 		displayProgressFrom: 0
- 		to: self size
- 		during: 
- 			[:bar | 
- 			[ [self atEnd] whileFalse: 
- 					[bar value: self position.
- 					self skipSeparators.
- 					
- 					[ | chunk |
- 					val := (self peekFor: $!!) 
- 								ifTrue: [ | ch |
- 									ch := self nextChunk.
- 									(self shouldIgnore: ch)
- 										ifTrue: [Transcript showln: 'Ignoring chunk: ', ch]
- 										ifFalse: [(Compiler evaluate: ch for: client logged: true) scanFrom: self]]
- 								ifFalse: 
- 									[chunk := self nextChunk.
- 									self checkForPreamble: chunk.
- 									Compiler evaluate: chunk for: client logged: true]] 
- 							on: InMidstOfFileinNotification
- 							do: [:ex | ex resume: true].
- 					self skipStyleChunk]
- 			] on: InvalidUTF8 do: [:ex |
- 				self notify: ex messageText, '\\Proceed to try the legacy MacRoman encoding.' translated withCRs.
- 				self reset; setConverterForOldCode.
- 				^ self fileInFor: client announcing: announcement].
- 			self close].
- 	"Note:  The main purpose of this banner is to flush the changes file."
- 	Smalltalk logChange: '----End fileIn of ' , self name , '----'.
- 	self flag: #ThisMethodShouldNotBeThere.	"sd"
- 	^val!

Item was added:
+ ----- Method: PositionableStream>>fileInFor:announcing:environment: (in category '*System-Changes-fileIn/Out') -----
+ fileInFor: client announcing: announcement environment: anEnvironment
+ 	"This is special for reading expressions from text that has been formatted with exclamation delimiters. The expressions are read and passed to the Compiler. Answer the result of compilation.  Put up a progress report with the given announcement as the title.
+ 	File-ins are used for installing change sets and SAR/MCZ archives.
+ 	anEnvironment specifies the environment in which the expressions should be evaluated (or, into which environment the new code should be loaded). This is orthogonal to Environment current, which is passed along to any do-its or class builders in evaluated expressions."
+ 
+ 	| val |
+ 	announcement 
+ 		displayProgressFrom: 0
+ 		to: self size
+ 		during: 
+ 			[:bar | 
+ 			[ [self atEnd] whileFalse: 
+ 					[bar value: self position.
+ 					self skipSeparators.
+ 					
+ 					[ | chunk |
+ 					val := (self peekFor: $!!) 
+ 								ifTrue: [ | ch |
+ 									ch := self nextChunk.
+ 									(self shouldIgnore: ch)
+ 										ifTrue: [Transcript showln: 'Ignoring chunk: ', ch]
+ 										ifFalse: [(Compiler new evaluate: ch in: nil to: client environment: anEnvironment notifying: nil ifFail: [] logged: true) scanFrom: self]]
+ 								ifFalse: 
+ 									[chunk := self nextChunk.
+ 									self checkForPreamble: chunk.
+ 									Compiler new evaluate: chunk in: nil to: client environment: anEnvironment notifying: nil ifFail: [] logged: true]] 
+ 							on: InMidstOfFileinNotification
+ 							do: [:ex | ex resume: true].
+ 					self skipStyleChunk]
+ 			] on: InvalidUTF8 do: [:ex |
+ 				self notify: ex messageText, '\\Proceed to try the legacy MacRoman encoding.' translated withCRs.
+ 				self reset; setConverterForOldCode.
+ 				^ self fileInFor: client announcing: announcement environment: anEnvironment].
+ 			self close].
+ 	"Note:  The main purpose of this banner is to flush the changes file."
+ 	Smalltalk logChange: '----End fileIn of ' , self name , '----'.
+ 	self flag: #ThisMethodShouldNotBeThere.	"sd"
+ 	^val!

Item was added:
+ ----- Method: PositionableStream>>fileInIntoEnvironment: (in category '*System-Changes-fileIn/Out') -----
+ fileInIntoEnvironment: anEnvironment
+ 
+ 	^ self fileInAnnouncing: 'Reading ' , self name environment: anEnvironment!

Item was changed:
  ----- Method: SARInstaller>>fileInFrom: (in category 'fileIn') -----
  fileInFrom: stream
  	"The zip has been saved already by the download.
  	Read the zip into my instvar, then file in the correct members"
  
  	
  
  	[ | postscript preamble |
  		stream position: 0.
  		zip := ZipArchive new readFrom: stream.
  
  		preamble := zip memberNamed: 'install/preamble'.
  		preamble ifNotNil: [
+ 			preamble contentStream text setConverterForCode fileInFor: self announcing: 'Preamble' environment: self environment.
- 			preamble contentStream text setConverterForCode fileInFor: self announcing: 'Preamble'.
  			self class currentChangeSet preambleString: preamble contents.
  		].
  
  		postscript := zip memberNamed: 'install/postscript'.
  		postscript ifNotNil: [
+ 			postscript contentStream text setConverterForCode fileInFor: self announcing: 'Postscript' environment: self environment.
- 			postscript contentStream text setConverterForCode fileInFor: self announcing: 'Postscript'.
  			self class currentChangeSet postscriptString: postscript contents.
  		].
  
  		preamble isNil & postscript isNil ifTrue: [
  			(self confirm: 'No install/preamble or install/postscript member were found.
  	Install all the members automatically?') ifTrue: [ self installAllMembers ]
  		].
  
  	] ensure: [ stream close ].
  
  !

Squeak-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]