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
<[email protected]> 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-tt4956517.html

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 <[email protected]>
> 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
>> <[email protected]> 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
>> > <[email protected]> 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
>> >> <[email protected]> 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
>> >>> <[email protected]> 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

Reply via email to