[Vm-dev] [Pharo-dev] FileAttributesPlugin: request for comment and testing

[Eliot Miranda]

Hi Alistair,

    it looks good to me.  I have only a few quibbles.  I see a few

<var: 'winAttrs' declareC: 'WIN32_FILE_ATTRIBUTE_DATA *winAttrs'>
<var: 'accessDate' declareC: 'sqLong *accessDate'>

that are better written as var:type:

<var: 'winAttrs' type: 'WIN32_FILE_ATTRIBUTE_DATA *'>
<var: 'accessDate' type: 'sqLong *'>

and I prefer to use symbols for types, because they're usually not unique,
so

<var: 'winAttrs' type: #'WIN32_FILE_ATTRIBUTE_DATA *'>
<var: 'accessDate' type: #'sqLong *'>


On Tue, Jun 12, 2018 at 8:45 AM, Alistair Grant <akgrant0710 at gmail.com>
wrote:

> Hi Esteban, Guille, Marcus, Cyril and Everyone,
>
> I'm (finally!) almost ready to merge in the FileAttributesPlugin
> changes...
>
> I have one bug to fix in the VM related to file creation, modification
> and access times on Windows.
>
> While I sort that out (and probably lose what remaining hair I have) I
> was hoping that a few people could have a look at the PR on linux and /
> or Mac - since it is an admittedly big one.
>
> Note that you can't merge the changes in to an existing image as
> Iceberg will attempt to use the file system in the middle of the merge
> and fail. (I haven't tested this after the latest refactor, but I expect
> that hasn't changed).  You'll need to use the image that has been
> bootstrapped from the PR (link below).
>
>
> The source code for the plugin is at: http://smalltalkhub.com/#!/~
> Alistair/FileAttributesPlugin
>
> PR: https://github.com/pharo-project/pharo/pull/1529
>
> Fogbugz:
> - https://pharo.fogbugz.com/f/cases/21368/Integrate-FileAttributesPlugin
> - https://pharo.fogbugz.com/f/cases/18279/
>
> Images:
>
> - 32 bit: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%
> 20pull%20request%20and%20branch%20Pipeline/job/PR-
> 1529/1/artifact/bootstrap-cache/Pharo7.0-32bit-af43d95.zip
> - 64 bit: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%
> 20pull%20request%20and%20branch%20Pipeline/job/PR-
> 1529/1/artifact/bootstrap-cache/Pharo7.0-64bit-af43d95.zip
>
> It requires a recent VM, from May 9 or later (the current stable VM).
>
>
>
> What are the changes?
> =====================
>
> - Fixes FileReference>>isSymlink
>   Currently it only returns true for a broken symbolic link.
> - Adds the ability to retrieve symbolic link attributes.
> - Extends the set of file attributes that can be retrieved to all
>   attributes returned by the libc stat() and lstat() calls.
> - Refactors the code to be more efficient.  In particular
>   FileReference>>exists.
>
>
> 1. #isSymlink now works properly on Linux (and it should also work on
>    MacOS and BSD).
> 2. The list of file attributes available from DiskDirectoryEntry now is:
>     #accessTime (new)
>     #changeTime (new)
>     #deviceId (new)
>     #gid (new)
>     #inode (new)
>     #isBlock (new)
>     #isCharacter (new)
>     #isExecutable (new)
>     #isFIFO (new)
>     #isRegular (new)
>     #isSocket (new)
>     #isSymlink (works)
>     #numberOfHardLinks (new)
>     #targetFile (new)
>     #uid (new)
>
>     #creationTime
>     #exists
>     #isDirectory
>     #isFile
>     #isReadable
>     #isWritable
>     #modificationTime
>     #permissions
>     #size
> 3. FileReference>>exists is faster than before (well, at least on my
>    linux laptop).  This is useful as it is called often.
> 4. It is possible to retrieve symbolic link attributes, i.e. all the
>    attributes listed above plus the target path.
>
>
> The public interface hasn't changed.  There are a number of internal
> changes to FileSystem.
>
> 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>>symlinkEntry.
>
> Overall, performance is slightly better than before.  Code that accesses a
> single attribute through FileReference will be faster than before (it used
> to retrieve all supported elements and throw the unused ones away).  Code
> that needs to access multiple attributes and uses FileSystemDirectoryEntry
> can access the new attributes without any additional performance cost.
>
>
>
> Rough development timeline
> ==========================
>
>
> April 2017: Started development
>
> May 2017: First working version.  I've been using this in my personal
> environment since May last year.
>
> August 2017: Plugin review by Nicolas Cellier (thanks, Nicolas!)
>
> September 2017 - January 2018 : Plugin review by Eliot Miranda (thanks
> Eliot!)
> This was very detailed and tidied up parameter checking, error
> return codes, making the code simulator friendly, etc.
>
> November 2017: FileAttributesPlugin added to the VM.
>
> February 2018: Tidy up Windows specific code, fix error handling in
> #creationTime.
>
> June 2018: Refactored to place primitives in File instead of
> FileAttributesPluginPrims (which matched FilePluginPrims).
>
>
>
> Testing
> =======
>
> - As mentioned above I've been using this on a daily basis since May 2017.
>   Some of the packages I have loaded include: Roassal2, Glorp,
>   OSSubprocess, NeoCSV, DataFrame.
> - Automated tests have been added for the new functionality.
> - Additional automated tests have been added for existing functionality.
> - All existing tests pass
>
>
> Thanks!
> Alistair
>
>


-- 
_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20180616/0ca9cdde/attachment.html>