Hi Eliot,

Thanks for taking a look.

TL;DR: I've made the changes you suggested.

On Sat, 16 Jun 2018 at 21:44, Eliot Miranda eliot.miranda@gmail.com wrote:

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:

I had a few problems with the code generation and changed these while trying to figure out what I'd done wrong. Fixed.

<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 *'>

Hmm. I thought we'd already changed these. I've gone through the entire plugin and converted them to symbols.

Thanks! Alistair

On Tue, Jun 12, 2018 at 8:45 AM, Alistair Grant akgrant0710@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%20...
• 64 bit: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20...

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