Re: [Pharo-dev] FileSystem file attributes and #isSymlink patch

> Hi All,
>
> I'm nearly ready to submit a patch that started with the goal of being
> able to retrieve the device id and fixing FileReference>>isSymlink and
> also follows Esteban's suggestion of splitting out file existence and
> other attributes (which provides a performace gain).  See the summary
> below for a description of the changes.
>
> The patch involves adding a new VM plugin, FileAttriubutesPlugin.  To
> minimise the chance of any problems along the way I'd like to submit the
> patch in three steps:
>
> 1. Add the VM plugin (FileAttributesPlugin)
> 2. Add the code the image that allows testing of the code and plugin,
>    but doesn't do any integration with existing functionality.
>    This will allow the plugin to be tested by a few volunteers
>    (hopefully).
> 3. Add the patches that make the switch over to the new implementation.
>

>
> I've been using this as my production environment for about 2 months now
> on a linux 32 bit VM.  Running the full test suite results in the same
> set of test failing before and after applying the patch.
>
> I've also ran file related tests on linux 64 bit (run the Test Runner,
> select all packages with "file" as part of the name and run all the
> available tests) and the full test suite on Windows 32 bit.
>
> The summary is:
>
> 1. #isSymlink now works properly on Linux (and it should also work on
>    MacOS and BSD).
> 2. The list of file attributes available from FileReference now is:
> #accessTime (new)
> #changeTime (new)
> #creationTime
> #deviceId (new)
> #exists
> #gid (new)
> #inode (new)
> #isBlock (new)
> #isCharacter (new)
> #isDirectory
> #isExecutable (new)
> #isFIFO (new)
> #isFile
> #isReadable
> #isRegular (new)
> #isSocket (new)
> #isSymlink (works)
> #isWritable
> #modificationTime
> #numberOfHardLinks (new)
> #permissions
> #size
> #targetFile (new)
> #uid (new)
> 3. FileReference>>exists is faster than before (well, at least on my
>    linux laptop).  This is useful as it is called quite often.
> 4. It is possible to retrieve symbolic link attributes, e.g. all the
>    attributes listed above plus the target path.
>
>
> Given how similar MacOS and BSD are to linux, I assume that this will
> all work without problem on those platforms (but it obviously needs to
> be tested).
>
> As implied above, the changes are all backward compatible (except
> the broken #isSymlink), although a couple deserve mention:
>
> 1. Obviously #isSymlink now answers correctly (previously it would only
>    answer correctly for a broken link).
> 2. Requesting any of the attributes listed above (except #isSymlink)
>    will return the value of the target path.  If the FileReference is to a
>    broken symbolic link, it will return the attributes of the symbolic
>    link (keeping existing behaviour).
> 3. The attributes of a symbolic link can be retrieved using
>    FileReference>>symlinkAttributes.
>
> Overall, performance is slightly better than before.  Code that
> needs to access multiple attributes and is written to take advantage of
> the new behaviour will see significant performance improvements.
>
>
> If you've got this far and forgotten the original question :-)
>
> Can someone point me to how to submit a new VM plugin?
>
>
> Thanks,
> Alistair

>  
> Hi Alistair,
>
> I am copying this to vm-dev for follow up on the plugin, see below.
>
>
> On Mon, Jul 24, 2017 at 08:09:10AM +0000, Alistair Grant wrote:
> > Hi All,
> >
> > I'm nearly ready to submit a patch that started with the goal of being
> > able to retrieve the device id and fixing FileReference>>isSymlink and
> > also follows Esteban's suggestion of splitting out file existence and
> > other attributes (which provides a performace gain).  See the summary
> > below for a description of the changes.
> >
> > The patch involves adding a new VM plugin, FileAttriubutesPlugin.  To
> > minimise the chance of any problems along the way I'd like to submit the
> > patch in three steps:
> >
> > 1. Add the VM plugin (FileAttributesPlugin)
> > 2. Add the code the image that allows testing of the code and plugin,
> >    but doesn't do any integration with existing functionality.
> >    This will allow the plugin to be tested by a few volunteers
> >    (hopefully).
> > 3. Add the patches that make the switch over to the new implementation.
> >
>
> I think you are handling this in exactly the right way, kudos.
>
> >
> > Can someone point me to how to submit a new VM plugin?  The code is
> > contained in a subclass of InterpreterPlugin.
>
> Following up on the vm-dev list: If your plugin is available in a Monticello
> repository, that would be great because VM builders can easily include it
> and try it out. Any repository would be fine for starters, or if you have
> an account on squeaksource.com I can add you as developer in the somewhat
> loosely-related DirectoryPlugin project if that is of any help. Whatever is
> convenient for you.

> Dave
>
>
> >
> > I've been using this as my production environment for about 2 months now
> > on a linux 32 bit VM.  Running the full test suite results in the same
> > set of test failing before and after applying the patch.
> >
> > I've also ran file related tests on linux 64 bit (run the Test Runner,
> > select all packages with "file" as part of the name and run all the
> > available tests) and the full test suite on Windows 32 bit.
> >
> > The summary is:
> >
> > 1. #isSymlink now works properly on Linux (and it should also work on
> >    MacOS and BSD).
> > 2. The list of file attributes available from FileReference now is:
> > #accessTime (new)
> > #changeTime (new)
> > #creationTime
> > #deviceId (new)
> > #exists
> > #gid (new)
> > #inode (new)
> > #isBlock (new)
> > #isCharacter (new)
> > #isDirectory
> > #isExecutable (new)
> > #isFIFO (new)
> > #isFile
> > #isReadable
> > #isRegular (new)
> > #isSocket (new)
> > #isSymlink (works)
> > #isWritable
> > #modificationTime
> > #numberOfHardLinks (new)
> > #permissions
> > #size
> > #targetFile (new)
> > #uid (new)
> > 3. FileReference>>exists is faster than before (well, at least on my
> >    linux laptop).  This is useful as it is called quite often.
> > 4. It is possible to retrieve symbolic link attributes, e.g. all the
> >    attributes listed above plus the target path.
> >
> >
> > Given how similar MacOS and BSD are to linux, I assume that this will
> > all work without problem on those platforms (but it obviously needs to
> > be tested).
> >
> > As implied above, the changes are all backward compatible (except
> > the broken #isSymlink), although a couple deserve mention:
> >
> > 1. Obviously #isSymlink now answers correctly (previously it would only
> >    answer correctly for a broken link).
> > 2. Requesting any of the attributes listed above (except #isSymlink)
> >    will return the value of the target path.  If the FileReference is to a
> >    broken symbolic link, it will return the attributes of the symbolic
> >    link (keeping existing behaviour).
> > 3. The attributes of a symbolic link can be retrieved using
> >    FileReference>>symlinkAttributes.
> >
> > Overall, performance is slightly better than before.  Code that
> > needs to access multiple attributes and is written to take advantage of
> > the new behaviour will see significant performance improvements.
> >
> >
> > If you've got this far and forgotten the original question :-)
> >
> > Can someone point me to how to submit a new VM plugin?
> >
> >
> > Thanks,
> > Alistair

>  
> Hi All,
>
> I've realised that I have been doing all my development using an old
> version of VMMaker: dtl.392.
>
> Before submitting my plugin I thought I should check that it builds
> correctly with the latest VMMaker.
>
> However taking a clean squeak image
> (Squeak5.1-16549-32bit-201608171728-Linux) and loading what I believe is
> the latest VMMaker: oscog-eem.2257 causes the plugin to fail to compile
> with numerous errors (on Ubuntu 16.04, gcc 4.8.5).
>
> Could someone please let me know which version of VMMaker I should be
> using?
>
>
> If you're interested in the sorts of problems I'm facing:
>
> The full code is available from:
>
> MCHttpRepository
> location: 'http://smalltalkhub.com/mc/Alistair/FileAttributesPlugin/main'
> user: ''
> password: ''
>
>
> An example of one of the failures is a method that begins:
>
> fileToAttributeArray: cPathName mask: attributeMask array: attributeArray
> "Answer a file attribute array from pathNameOop."
>
> | getStats useLstat getAccess statArray accessArray combinedArray status fileNameOop statBuf statBufPointer  |
> <returnTypeC: 'int'>
> <var: 'cPathName' type: 'char *'>
> <var: 'attributeArray' type: 'sqInt *'>
> <var: 'statBuf' type: 'struct stat'>
> <var: 'statBufPointer' declareC: 'struct stat *statBufPointer= &statBuf'>
>
>
> dtl.392 correctly creates C definitions for statBuf and
> statBufPointer, i.e.:
>
> static int fileToAttributeArraymaskarray(char *cPathName, sqInt attributeMask, sqInt *attributeArray) {
>     sqInt accessArray;
>     sqInt combinedArray;
>     sqInt fileNameOop;
>     sqInt getAccess;
>     sqInt getStats;
>     sqInt statArray;
>     struct stat statBuf;
>     struct stat *statBufPointer= &statBuf;
>     sqInt status;
>     sqInt useLstat;
>
>
> oscog-eem.2257 produces:
>
>
> static int
> fileToAttributeArraymaskarray(char *cPathName, sqInt attributeMask, sqInt *attributeArray)
> {
>     sqInt accessArray;
>     sqInt combinedArray;
>     sqInt fileNameOop;
>     int getAccess;
>     int getStats;
>     sqInt statArray;
>     struct stat *statBufPointer= &statBuf;
>     sqInt status;
>     int useLstat;
>
>
> As can be seen, the statBuf declaration is missing.
>
>
> Further along:
>
> isSymlink: cPathName boolean: resultOop
> "Set resultOop to a boolean indicating whether cPathName is a symbolic link.
> Answer status (0 = success)"
>
> | status statBuf |
> <var: 'cPathName' type: 'char *'>
> <var: 'resultOop' type: 'sqInt *'>
> <var: 'statBuf' type: 'struct stat'>
> self isDefinedTrueExpression: 'HAVE_LSTAT == 1'
> inSmalltalk: []
> comment: ''
> ifTrue: [
> status := self cCode: 'lstat(cPathName, &statBuf)'.
> (status ~= 0) ifTrue: [^self cantStatPath].
> ((self cCode: 'S_ISLNK(statBuf.st_mode)') = 0)
> ifFalse: [self cCode: '*resultOop = interpreterProxy->trueObject()']
> ifTrue: [self cCode: '*resultOop = interpreterProxy->falseObject()'].
> ]
> ifFalse: [].
> ^0
>
>
>
> dtl.392 produces:
>
> static sqInt isSymlinkboolean(char *cPathName, sqInt *resultOop) {
>     struct stat statBuf;
>     sqInt status;
>
>        
> # if (HAVE_LSTAT == 1)  //
>         status = lstat(cPathName, &statBuf);
>         if (status != 0) {
>                 return -3;
>         }
>         if ((S_ISLNK(statBuf.st_mode)) == 0) {
>                 *resultOop = interpreterProxy->falseObject();
>         } else {
>                 *resultOop = interpreterProxy->trueObject();
>         }
> # endif  // HAVE_LSTAT == 1
>        
>         return 0;
> }
>
>
>
> oscog-eem.2257 produces:
>
> static sqInt
> isSymlinkboolean(char *cPathName, sqInt *resultOop)
> {
>     struct stat statBuf;
>     sqInt status;
>
>         isDefinedTrueExpressioninSmalltalkcommentifTrueifFalse("HAVE_LSTAT == 1", null, "", ((status =
>  lstat(cPathName, &statBuf)),
>         (status != 0
>                         ? (/* begin cantStatPath */
>                                 return -3)
>                         : 0),
>         ((S_ISLNK(statBuf.st_mode)) == 0
>                         ? *resultOop = interpreterProxy->falseObject()
>                         : *resultOop = interpreterProxy->trueObject())), null);
>         return 0;
> }
>
>
> Any help appreciated.
>
> Thanks very much,
> Alistair
>
>
>
>
> On Tue, Jul 25, 2017 at 03:21:16PM +0000, Alistair Grant wrote:
> >
> > I'll post another reply once I've got the plugin code in
> > smalltalkhub.com.
> >
> > Thanks again,
> > Alistair

> VMMaker.oscog-eem.2257 is the most up to date version of the oscog branch
> of VMMaker that is the basis of the latest Cog and Spur VMs for Squeak and
> Pharo. It is intended to be compiled with the platform sources from the GitHub
> repository. In the the VMMaker repository, the "oscog" suffix distinguishes
> the Cog/Spur development (suitable for Pharo) from the traditional branch.
>
> Your plugin will probably work fine with either combination of VMMaker and
> platforms sources, but you cannot mix the two without problems such as what
> you are seeing now.
>
> I personally use the traditional interpreter VM platform for plugin development,
> then validate the resulting plugin on Cog/Spur (which usually just works
> without issues). But either combination of VMMaker and platforms sources
> should be fine as long as you do not mix the two.
>
> Given that your goal is a plugin to support Pharo images, you will want
> test with VMMaker.oscog-eem.2257 and the latest platforms sources from
> the GitHub repository.

> HTH,
> Dave
>
>
> On Wed, Jul 26, 2017 at 09:02:41PM +0000, Alistair Grant wrote:
> >  
> > Hi All,
> >
> > I've realised that I have been doing all my development using an old
> > version of VMMaker: dtl.392.
> >
> > Before submitting my plugin I thought I should check that it builds
> > correctly with the latest VMMaker.
> >
> > However taking a clean squeak image
> > (Squeak5.1-16549-32bit-201608171728-Linux) and loading what I believe is
> > the latest VMMaker: oscog-eem.2257 causes the plugin to fail to compile
> > with numerous errors (on Ubuntu 16.04, gcc 4.8.5).
> >
> > Could someone please let me know which version of VMMaker I should be
> > using?
> >
> >
> > If you're interested in the sorts of problems I'm facing:
> >
> > The full code is available from:
> >
> > MCHttpRepository
> > location: 'http://smalltalkhub.com/mc/Alistair/FileAttributesPlugin/main'
> > user: ''
> > password: ''
> >
> >
> > An example of one of the failures is a method that begins:
> >
> > fileToAttributeArray: cPathName mask: attributeMask array: attributeArray
> > "Answer a file attribute array from pathNameOop."
> >
> > | getStats useLstat getAccess statArray accessArray combinedArray status fileNameOop statBuf statBufPointer  |
> > <returnTypeC: 'int'>
> > <var: 'cPathName' type: 'char *'>
> > <var: 'attributeArray' type: 'sqInt *'>
> > <var: 'statBuf' type: 'struct stat'>
> > <var: 'statBufPointer' declareC: 'struct stat *statBufPointer= &statBuf'>
> >
> >
> > dtl.392 correctly creates C definitions for statBuf and
> > statBufPointer, i.e.:
> >
> > static int fileToAttributeArraymaskarray(char *cPathName, sqInt attributeMask, sqInt *attributeArray) {
> >     sqInt accessArray;
> >     sqInt combinedArray;
> >     sqInt fileNameOop;
> >     sqInt getAccess;
> >     sqInt getStats;
> >     sqInt statArray;
> >     struct stat statBuf;
> >     struct stat *statBufPointer= &statBuf;
> >     sqInt status;
> >     sqInt useLstat;
> >
> >
> > oscog-eem.2257 produces:
> >
> >
> > static int
> > fileToAttributeArraymaskarray(char *cPathName, sqInt attributeMask, sqInt *attributeArray)
> > {
> >     sqInt accessArray;
> >     sqInt combinedArray;
> >     sqInt fileNameOop;
> >     int getAccess;
> >     int getStats;
> >     sqInt statArray;
> >     struct stat *statBufPointer= &statBuf;
> >     sqInt status;
> >     int useLstat;
> >
> >
> > As can be seen, the statBuf declaration is missing.
> >
> >
> > Further along:
> >
> > isSymlink: cPathName boolean: resultOop
> > "Set resultOop to a boolean indicating whether cPathName is a symbolic link.
> > Answer status (0 = success)"
> >
> > | status statBuf |
> > <var: 'cPathName' type: 'char *'>
> > <var: 'resultOop' type: 'sqInt *'>
> > <var: 'statBuf' type: 'struct stat'>
> > self isDefinedTrueExpression: 'HAVE_LSTAT == 1'
> > inSmalltalk: []
> > comment: ''
> > ifTrue: [
> > status := self cCode: 'lstat(cPathName, &statBuf)'.
> > (status ~= 0) ifTrue: [^self cantStatPath].
> > ((self cCode: 'S_ISLNK(statBuf.st_mode)') = 0)
> > ifFalse: [self cCode: '*resultOop = interpreterProxy->trueObject()']
> > ifTrue: [self cCode: '*resultOop = interpreterProxy->falseObject()'].
> > ]
> > ifFalse: [].
> > ^0
> >
> >
> >
> > dtl.392 produces:
> >
> > static sqInt isSymlinkboolean(char *cPathName, sqInt *resultOop) {
> >     struct stat statBuf;
> >     sqInt status;
> >
> >        
> > # if (HAVE_LSTAT == 1)  //
> >         status = lstat(cPathName, &statBuf);
> >         if (status != 0) {
> >                 return -3;
> >         }
> >         if ((S_ISLNK(statBuf.st_mode)) == 0) {
> >                 *resultOop = interpreterProxy->falseObject();
> >         } else {
> >                 *resultOop = interpreterProxy->trueObject();
> >         }
> > # endif  // HAVE_LSTAT == 1
> >        
> >         return 0;
> > }
> >
> >
> >
> > oscog-eem.2257 produces:
> >
> > static sqInt
> > isSymlinkboolean(char *cPathName, sqInt *resultOop)
> > {
> >     struct stat statBuf;
> >     sqInt status;
> >
> >         isDefinedTrueExpressioninSmalltalkcommentifTrueifFalse("HAVE_LSTAT == 1", null, "", ((status =
> >  lstat(cPathName, &statBuf)),
> >         (status != 0
> >                         ? (/* begin cantStatPath */
> >                                 return -3)
> >                         : 0),
> >         ((S_ISLNK(statBuf.st_mode)) == 0
> >                         ? *resultOop = interpreterProxy->falseObject()
> >                         : *resultOop = interpreterProxy->trueObject())), null);
> >         return 0;
> > }
> >
> >
> > Any help appreciated.
> >
> > Thanks very much,
> > Alistair