- Pharo-dev - lists.pharo.org

Re: [Pharo-dev] Monticello file out bug during bootstrap

by Alistair Grant

Hi All, I've submitted: https://pharo.fogbugz.com/f/cases/21431/ https://github.com/pharo-project/pharo/pull/976 The issue has additional analysis of the problem, namely that #classSide is a convenience method for retrieving the class from either an instance or the class, so isn't appropriate for chunk file out. Cheers, Alistair On 26 February 2018 at 17:43, Alistair Grant <akgrant0710(a)gmail.com> wrote: > Hi Everyone, > > The following demonstrates the problem (and will allow me to provide an > automated test when I submit a patch): > > > | stream definition writer diskStore nonExistent | > > definition := MCMethodDefinition > className: #DiskStore > classIsMeta: true > selector: #activeClass > category: 'current' > timeStamp: nil > source: 'activeClass > self allSubclassesDo: [:ea | > ea isActiveClass ifTrue: [^ ea]]. > ^ self'. > stream := String new writeStream. > writer := MCStWriter new > stream: stream; > yourself. > > writer writeMethodPreamble: definition. > diskStore := stream contents. > > definition := MCMethodDefinition > className: #NotALoadedClass > classIsMeta: true > selector: #reset > category: 'imagine' > timeStamp: nil > source: 'reset > > ^self doReset'. > stream := String new writeStream. > writer := MCStWriter new > stream: stream; > yourself. > > writer writeMethodPreamble: definition. > nonExistent := stream contents. > > { diskStore. nonExistent. } > > " #(' > !DiskStore class methodsFor: ''current'' stamp: ''nil''! > ' ' > !NotALoadedClass classSide methodsFor: ''imagine'' stamp: ''nil''! > ')" > > > As can be seen, the DiskStore preamble is as expected, while the > NotALoadedClass preamble uses "classSide" instead of "class". > > > Modifying one method: > > > MCStWriter>> > writeMethodPreamble: definition > self chunkContents: [:str | > stream bang. > str nextPutAll: definition className asString. > definition classIsMeta ifTrue: > [ str nextPutAll: ' class' ]. > str > nextPutAll: ' methodsFor: '; > nextPutAll: definition category asString printString; > nextPutAll: ' stamp: '; > nextPutAll: definition timeStamp asString printString > ] > > > produces the correct result, i.e. running the test code above produces: > > > " #(' > !DiskStore class methodsFor: ''current'' stamp: ''nil''! > ' ' > !NotALoadedClass class methodsFor: ''imagine'' stamp: ''nil''! > ')" > > > The NotALoadedClass has the "class" keyword as expected. > > > Cheers, > Alistair > > On 26 February 2018 at 11:14, Alistair Grant <akgrant0710(a)gmail.com> wrote: >> Hi Guille and Stef, >> >> Stef, thanks also for your reply. All the information I was going to >> put there I've included below, so please treat is a combined reply. >> >> >> On 26 February 2018 at 09:51, Guillermo Polito >> <guillermopolito(a)gmail.com> wrote: >>> Hi Alistair, >>> >>> Can you provide some more context on what are you trying to do and how? >> >> I'm working on integrating the changes I've made to FileSystem to >> properly support symbolic links and all the other attributes returned >> by libc stat(). >> >> There's a slightly dated but more detailed description of the changes >> in http://forum.world.st/FileSystem-file-attributes-and-isSymlink-patch-tt4956… >> >> The patch introduces a number of new classes to the FileSystem package: >> >> - FileAttributesPluginPrims with Unx and Windows subclasses >> - Subclasses of FileSystemDirectoryEntry for symbolic links >> - FileAttributeNotSupported >> >> >>> Generally you should not change anything in the bootstrap, just modify the >>> classes you want to modify and commit them into the repository. The >>> bootstrap will take care of it. >> >> That's true, but as mentioned this introduces new classes, which also >> should be handled automatically, but there is a bug in the Monticello >> file out code that is causing it to fail. >> >> I haven't gone through the bootstrap process in great detail, but my >> understanding is that it reads in the Pharo 7 code from the repository >> and writes out key classes to .st files to file in as part of the >> build, i.e.: >> >> ${VM70} "${COMPILER_IMAGE_NAME}.image" st ${ST_CACHE}/Multilingual.st >> ${ST_CACHE}/DeprecatedFileStream.st ${ST_CACHE}/FileSystem.st >> --no-source --quit --save >> >> The bug is only triggered because the new classes, e.g. >> FileAttributesPluginPrim, are not part of the running image. You can >> see the code here: >> >> >> MCMethodDefinition>>fullClassName >> "Using #class selector for classes for backwards compatibility" >> >> ^ self classIsMeta >> ifFalse: [self className] >> ifTrue: [ >> (self actualClass isNil or: [ self actualClass isTrait ]) >> ifFalse: [self className, ' class'] >> ifTrue: [self className, ' classSide']] >> >> >> Because FileAttributesPluginPrims isn't in the image, #actualClass >> returns nil, and the method answers "classSide" resulting in a chunk >> that looks like: >> >> >> !FileAttributesPluginPrims classSide methodsFor: 'initialize' stamp: 'nil'! >> reset >> "Reload the masks" >> >> Default := nil.! ! >> >> >> This then quietly fails to load, resulting in a DNU when the method is >> called (later on in the bootstrap process). >> >> A quick test of filing out a trait suggests this code isn't called as >> it uses "classTrait" in the chunk: >> >> >> !TestTrait classTrait methodsFor: 'testing' stamp: 'AlistairGrant >> 2/25/2018 15:37'! >> testTrait >> >> ^'testing'! ! >> >> >> I'm not familiar with the monticello code base, but #fullClassName is >> used in a number of different places, so I'm wondering whether it is >> OK to change MCStWriter>>writeMethodPreamble to always use "class". >> >> The would solve this bootstrap issue (there's one more below). >> >> >>> The only cases when you need to change bootstrap code are: >>> - your code modifies the API of the class builder, the binary >>> representation of compiled methods, requires changing some class >>> initialization or if it adds/removes some VM special object (as the ones in >>> the special objects array) >>> - you want to add a new package to pharo. In this case you need to add your >>> package to the repository and update the corresponding baseline so it is >>> loaded in the image (otherwise it will be in disk but ignored...) >> >> Or the new code is dependent on functionality introduced in the Pharo 7 VM. >> >> The Pharo 7 image is currently built using the Pharo 6 VM. Adding >> FileAttributesPlugin to the VM will require the bootstrap to use the >> Pharo 7 VM in build.sh. >> >> >> Thanks! >> Alistair >> >> >> >> >>> Guille >>> >>> On Sun, Feb 25, 2018 at 9:47 PM, Stephane Ducasse <stepharo.self(a)gmail.com> >>> wrote: >>>> >>>> Now I do not understand what is this classSide here. >>>> If we have a trait apparently we do not file out a class method as >>>> class but as classSide.... Super strange. >>>> >>>> Stef >>>> >>>> On Sun, Feb 25, 2018 at 9:46 PM, Stephane Ducasse >>>> <stepharo.self(a)gmail.com> wrote: >>>> > Apparently this method was like that in Pharo 60 so this is not my edit. >>>> > >>>> > fullClassName >>>> > "Using #class selector for classes for backwards compatibility" >>>> > >>>> > ^ self classIsMeta >>>> > ifFalse: [self className] >>>> > ifTrue: [ >>>> > (self actualClass isNil or: [ self actualClass isTrait ]) >>>> > ifFalse: [self className, ' class'] >>>> > ifTrue: [self className, ' classSide']] >>>> > >>>> > Now it does not mean that this is not a bug I introduced. >>>> > >>>> > >>>> > >>>> > >>>> > On Sun, Feb 25, 2018 at 9:42 PM, Stephane Ducasse >>>> > <stepharo.self(a)gmail.com> wrote: >>>> >> the problem is here of course but I need my mind back (In the train to >>>> >> home). ... >>>> >> >>>> >> >>>> >> MCMethodDefinition>>fullClassName >>>> >> "Using #class selector for classes for backwards compatibility" >>>> >> >>>> >> ^ self classIsMeta >>>> >> ifFalse: [self className] >>>> >> ifTrue: [ >>>> >> (self actualClass isNil or: [ self actualClass isTrait ]) >>>> >> ifFalse: [self className, ' class'] >>>> >> ifTrue: [self className, ' classSide']] >>>> >> >>>> >> >>>> >> >>>> >> >>>> >> >>>> >> On Sun, Feb 25, 2018 at 9:41 PM, Stephane Ducasse >>>> >> <stepharo.self(a)gmail.com> wrote: >>>> >>> Hi Alistair >>>> >>> >>>> >>> This is my mistake! I introduced this bug when I transformed >>>> >>> theMetaClass to classSide >>>> >>> Now I was paying super attention and I did not touch theMetaClass even >>>> >>> when class would have been far enough. >>>> >>> So do you have a scenario? So that I can try to have a look. >>>> >>> Unfortunately I was away from home and lab since a week and I may have >>>> >>> problem to allocate concentrated time. >>>> >>> But I will try. >>>> >>> Tx for reporting this! >>>> >>> >>>> >>> Stef >>>> >>> >>>> >>> >>>> >>> On Sun, Feb 25, 2018 at 3:48 PM, Alistair Grant >>>> >>> <akgrant0710(a)gmail.com> wrote: >>>> >>>> Hi Everyone, >>>> >>>> >>>> >>>> I'm working on integrating the file attribute primitives in to the >>>> >>>> main >>>> >>>> code during bootstrap. >>>> >>>> >>>> >>>> Class methods for FileAttributesPluginPrims are written out as: >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> !FileAttributesPluginPrims classSide methodsFor: 'initialize' stamp: >>>> >>>> 'nil'! >>>> >>>> reset >>>> >>>> "Reload the masks" >>>> >>>> >>>> >>>> Default := nil.! ! >>>> >>>> >>>> >>>> >>>> >>>> As you can see, the class name is written with "classSide" instead of >>>> >>>> "class", resulting in the method quietly being ignored during file >>>> >>>> in. >>>> >>>> >>>> >>>> This comes about in: >>>> >>>> >>>> >>>> >>>> >>>> MCStWriter>>writeMethodPreamble: definition >>>> >>>> self chunkContents: [:str | >>>> >>>> stream bang. >>>> >>>> str nextPutAll: definition fullClassName; >>>> >>>> nextPutAll: ' methodsFor: '; >>>> >>>> nextPutAll: definition category asString printString; >>>> >>>> nextPutAll: ' stamp: '; >>>> >>>> nextPutAll: definition timeStamp asString printString >>>> >>>> ] >>>> >>>> >>>> >>>> >>>> >>>> which calls: >>>> >>>> >>>> >>>> >>>> >>>> MCMethodDefinition>>fullClassName >>>> >>>> "Using #class selector for classes for backwards compatibility" >>>> >>>> >>>> >>>> ^ self classIsMeta >>>> >>>> ifFalse: [self className] >>>> >>>> ifTrue: [ >>>> >>>> (self actualClass isNil or: [ self actualClass isTrait ]) >>>> >>>> ifFalse: [self className, ' class'] >>>> >>>> ifTrue: [self className, ' classSide']] >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> Because FileAttributesPluginPrims doesn't exist in the image (the >>>> >>>> code >>>> >>>> is read in from the git repository and being written out to a .st >>>> >>>> file), >>>> >>>> #actualClass returns nil, resulting in "classSide" being used instead >>>> >>>> of >>>> >>>> "class". >>>> >>>> >>>> >>>> I've never seen "classSide" used in chunk format. Is there any time >>>> >>>> "classSide" is valid, and any suggestions on how this should be >>>> >>>> fixed? >>>> >>>> (My first thought is to just change MCStWriter>>writeMethodPreamble: >>>> >>>> to >>>> >>>> only write "class"). >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> Thanks, >>>> >>>> Alistair >>>> >>>> >>>> >>> >>> >>> >>> -- >>> >>> >>> >>> Guille Polito >>> >>> Research Engineer >>> >>> Centre de Recherche en Informatique, Signal et Automatique de Lille >>> >>> CRIStAL - UMR 9189 >>> >>> French National Center for Scientific Research - http://www.cnrs.fr >>> >>> >>> Web: http://guillep.github.io >>> >>> Phone: +33 06 52 70 66 13

Feb. 27, 2018

Re: [Pharo-dev] [Pharo 7.0-dev] Build #618: 21430-some-small-cleanup-of-for-reflectivity-tests

by Marcus Denker

This contains in addition 21419-Integrate-Calypso #966 https://github.com/pharo-project/pharo/pull/966 > On 27 Feb 2018, at 17:22, ci-pharo-ci-jenkins2(a)inria.fr wrote: > > There is a new Pharo build available! > > The status of the build #618 was: SUCCESS. > > The Pull Request #973 was integrated: "21430-some-small-cleanup-of-for-reflectivity-tests" > Pull request url: https://github.com/pharo-project/pharo/pull/973 > > Issue Url: https://pharo.fogbugz.com/f/cases/21430 > Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Feb. 27, 2018

[Pharo 7.0-dev] Build #619: 21419-Integrate-Calypso

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #619 was: SUCCESS. The Pull Request #966 was integrated: "21419-Integrate-Calypso" Pull request url: https://github.com/pharo-project/pharo/pull/966 Issue Url: https://pharo.fogbugz.com/f/cases/21419 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Feb. 27, 2018

[Pharo 7.0-dev] Build #618: 21430-some-small-cleanup-of-for-reflectivity-tests

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #618 was: SUCCESS. The Pull Request #973 was integrated: "21430-some-small-cleanup-of-for-reflectivity-tests" Pull request url: https://github.com/pharo-project/pharo/pull/973 Issue Url: https://pharo.fogbugz.com/f/cases/21430 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Feb. 27, 2018

[Pharo 7.0-dev] Build #617: 21425-remove-some-dead-code-referencing-SmalltalkEditor

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #617 was: SUCCESS. The Pull Request #969 was integrated: "21425-remove-some-dead-code-referencing-SmalltalkEditor" Pull request url: https://github.com/pharo-project/pharo/pull/969 Issue Url: https://pharo.fogbugz.com/f/cases/21425 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Feb. 27, 2018

[Pharo 7.0-dev] Build #616: 21269 Provide tags in package Announcements-Core for better structure

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #616 was: SUCCESS. The Pull Request #823 was integrated: "21269 Provide tags in package Announcements-Core for better structure" Pull request url: https://github.com/pharo-project/pharo/pull/823 Issue Url: https://pharo.fogbugz.com/f/cases/21269 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Feb. 27, 2018

[Pharo 7.0-dev] Build #615: 21423-Improve-even-more-the-Test-reports

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #615 was: SUCCESS. The Pull Request #968 was integrated: "21423-Improve-even-more-the-Test-reports" Pull request url: https://github.com/pharo-project/pharo/pull/968 Issue Url: https://pharo.fogbugz.com/f/cases/21423 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Feb. 27, 2018

Re: [Pharo-dev] Simplified AST "Suggestions" menu definitions

by Torsten Bergmann

Added to: http://wiki.astares.com/pharo/607 > Gesendet: Montag, 26. Februar 2018 um 08:17 Uhr > Von: "Marcus Denker" <marcus.denker(a)inria.fr> > An: "Pharo Development List" <pharo-dev(a)lists.pharo.org> > Betreff: Re: [Pharo-dev] Simplified AST "Suggestions" menu definitions > > > > > On 25 Feb 2018, at 21:30, Stephane Ducasse <stepharo.self(a)gmail.com> wrote: > > > > Marcus I thought that the context was about to represent what the > > tools selected like the class currently selected. > > You have this information in the AST? > > > The AST as coming from the Parser: no. The AST after name Analysis: yes. > Which means you actually need the class to do name analysis (âSemantic Analysisâ, > implemented by OCASTSemanticAnalyzer). > > I think it will be easy: > > -> there is RubSmalltalkCodeMode / RubSmalltalkScriptingMode which models if the editor. > is editing Doits or Methods (important for parsing!). > -> RubSmalltalkCodeMode does have an ivar classOrMetaclass already. It is not set by most tools yet, though. > -> we can easily add there (or in RubSmalltalkEditor) an âast â ivar and re-parse at every keystroke. > > So as soon as we make sure that all users of RubSmalltalkCodeMode correctly set the class, this will be all > very simple. > > Marcus >

Feb. 27, 2018

[Pharo 7.0-dev] Build #614: 21415 Move primitive failure methods from Object to ProtoObject

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #614 was: SUCCESS. The Pull Request #964 was integrated: "21415 Move primitive failure methods from Object to ProtoObject" Pull request url: https://github.com/pharo-project/pharo/pull/964 Issue Url: https://pharo.fogbugz.com/f/cases/21415 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Feb. 26, 2018

Re: [Pharo-dev] [Pharo 7.0-dev] Build #593: 21390-Do-not-store-any-non-literals-in-the-literal-array

by Norbert Hartl

> Am 25.02.2018 um 22:02 schrieb Stephane Ducasse <stepharo.self(a)gmail.com>: > > I really like this idea of read only literals :) > Tx for pushing it. +1 > >> On Thu, Feb 22, 2018 at 11:52 AM, Marcus Denker <marcus.denker(a)inria.fr> wrote: >> This fixes the Reflectivity compiler to not store normal objects in the literalArrayâ¦ so instead of âpushLiteral:â is now uses >> âpushLiteralVariable:â. >> >> This is needed as we do now have a compiler option to compile methods with read-only literals. >> >> Marcus >> >>> On 22 Feb 2018, at 11:45, ci-pharo-ci-jenkins2(a)inria.fr wrote: >>> >>> There is a new Pharo build available! >>> >>> The status of the build #593 was: SUCCESS. >>> >>> The Pull Request #932 was integrated: "21390-Do-not-store-any-non-literals-in-the-literal-array" >>> Pull request url: https://github.com/pharo-project/pharo/pull/932 >>> >>> Issue Url: https://pharo.fogbugz.com/f/cases/21390 >>> Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2… >> >>

Feb. 26, 2018