<div dir="ltr">Hi Alistair,<div><br></div><div> it looks good to me. I have only a few quibbles. I see a few</div><div><br></div><div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span><var: 'winAttrs' declareC: 'WIN32_FILE_ATTRIBUTE_DATA *winAttrs'></div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span><var: 'accessDate' declareC: 'sqLong *accessDate'></div></div><div><br></div><div>that are better written as var:type:</div><div><br></div><div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span><var: 'winAttrs' type: 'WIN32_FILE_ATTRIBUTE_DATA *'></div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span><var: 'accessDate' type: 'sqLong *'></div></div><div><br></div><div>and I prefer to use symbols for types, because they're usually not unique, so</div><div><br></div><div><div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span><var: 'winAttrs' type: #'WIN32_FILE_ATTRIBUTE_DATA *'></div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span><var: 'accessDate' type: #'sqLong *'></div></div></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 12, 2018 at 8:45 AM, Alistair Grant <span dir="ltr"><<a href="mailto:akgrant0710@gmail.com" target="_blank">akgrant0710@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Esteban, Guille, Marcus, Cyril and Everyone,<br>
<br>
I'm (finally!) almost ready to merge in the FileAttributesPlugin<br>
changes...<br>
<br>
I have one bug to fix in the VM related to file creation, modification<br>
and access times on Windows.<br>
<br>
While I sort that out (and probably lose what remaining hair I have) I<br>
was hoping that a few people could have a look at the PR on linux and /<br>
or Mac - since it is an admittedly big one.<br>
<br>
Note that you can't merge the changes in to an existing image as<br>
Iceberg will attempt to use the file system in the middle of the merge<br>
and fail. (I haven't tested this after the latest refactor, but I expect<br>
that hasn't changed). You'll need to use the image that has been<br>
bootstrapped from the PR (link below).<br>
<br>
<br>
The source code for the plugin is at: <a href="http://smalltalkhub.com/#!/~Alistair/FileAttributesPlugin" rel="noreferrer" target="_blank">http://smalltalkhub.com/#!/~<wbr>Alistair/FileAttributesPlugin</a><br>
<br>
PR: <a href="https://github.com/pharo-project/pharo/pull/1529" rel="noreferrer" target="_blank">https://github.com/pharo-<wbr>project/pharo/pull/1529</a><br>
<br>
Fogbugz:<br>
- <a href="https://pharo.fogbugz.com/f/cases/21368/Integrate-FileAttributesPlugin" rel="noreferrer" target="_blank">https://pharo.fogbugz.com/f/<wbr>cases/21368/Integrate-<wbr>FileAttributesPlugin</a><br>
- <a href="https://pharo.fogbugz.com/f/cases/18279/" rel="noreferrer" target="_blank">https://pharo.fogbugz.com/f/<wbr>cases/18279/</a><br>
<br>
Images:<br>
<br>
- 32 bit: <a href="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" rel="noreferrer" target="_blank">https://ci.inria.fr/pharo-ci-<wbr>jenkins2/job/Test%20pending%<wbr>20pull%20request%20and%<wbr>20branch%20Pipeline/job/PR-<wbr>1529/1/artifact/bootstrap-<wbr>cache/Pharo7.0-32bit-af43d95.<wbr>zip</a><br>
- 64 bit: <a href="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" rel="noreferrer" target="_blank">https://ci.inria.fr/pharo-ci-<wbr>jenkins2/job/Test%20pending%<wbr>20pull%20request%20and%<wbr>20branch%20Pipeline/job/PR-<wbr>1529/1/artifact/bootstrap-<wbr>cache/Pharo7.0-64bit-af43d95.<wbr>zip</a><br>
<br>
It requires a recent VM, from May 9 or later (the current stable VM).<br>
<br>
<br>
<br>
What are the changes?<br>
=====================<br>
<br>
- Fixes FileReference>>isSymlink<br>
Currently it only returns true for a broken symbolic link.<br>
- Adds the ability to retrieve symbolic link attributes.<br>
- Extends the set of file attributes that can be retrieved to all<br>
attributes returned by the libc stat() and lstat() calls.<br>
- Refactors the code to be more efficient. In particular<br>
FileReference>>exists.<br>
<br>
<br>
1. #isSymlink now works properly on Linux (and it should also work on <br>
MacOS and BSD).<br>
2. The list of file attributes available from DiskDirectoryEntry now is:<br>
#accessTime (new)<br>
#changeTime (new)<br>
#deviceId (new)<br>
#gid (new)<br>
#inode (new)<br>
#isBlock (new)<br>
#isCharacter (new)<br>
#isExecutable (new)<br>
#isFIFO (new)<br>
#isRegular (new)<br>
#isSocket (new)<br>
#isSymlink (works)<br>
#numberOfHardLinks (new)<br>
#targetFile (new)<br>
#uid (new)<br>
<br>
#creationTime<br>
#exists<br>
#isDirectory<br>
#isFile<br>
#isReadable<br>
#isWritable<br>
#modificationTime<br>
#permissions<br>
#size<br>
3. FileReference>>exists is faster than before (well, at least on my <br>
linux laptop). This is useful as it is called often.<br>
4. It is possible to retrieve symbolic link attributes, i.e. all the <br>
attributes listed above plus the target path.<br>
<br>
<br>
The public interface hasn't changed. There are a number of internal<br>
changes to FileSystem.<br>
<br>
As implied above, the changes are all backward compatible (except <br>
the broken #isSymlink), although a couple deserve mention:<br>
<br>
1. Obviously #isSymlink now answers correctly (previously it would only <br>
answer correctly for a broken link).<br>
2. Requesting any of the attributes listed above (except #isSymlink) <br>
will return the value of the target path. If the FileReference is to a <br>
broken symbolic link, it will return the attributes of the symbolic <br>
link (keeping existing behaviour).<br>
3. The attributes of a symbolic link can be retrieved using <br>
FileReference>>symlinkEntry.<br>
<br>
Overall, performance is slightly better than before. Code that accesses a<br>
single attribute through FileReference will be faster than before (it used <br>
to retrieve all supported elements and throw the unused ones away). Code<br>
that needs to access multiple attributes and uses FileSystemDirectoryEntry<br>
can access the new attributes without any additional performance cost.<br>
<br>
<br>
<br>
Rough development timeline<br>
==========================<br>
<br>
<br>
April 2017: Started development<br>
<br>
May 2017: First working version. I've been using this in my personal<br>
environment since May last year.<br>
<br>
August 2017: Plugin review by Nicolas Cellier (thanks, Nicolas!)<br>
<br>
September 2017 - January 2018 : Plugin review by Eliot Miranda (thanks<br>
Eliot!)<br>
This was very detailed and tidied up parameter checking, error<br>
return codes, making the code simulator friendly, etc.<br>
<br>
November 2017: FileAttributesPlugin added to the VM.<br>
<br>
February 2018: Tidy up Windows specific code, fix error handling in<br>
#creationTime.<br>
<br>
June 2018: Refactored to place primitives in File instead of<br>
FileAttributesPluginPrims (which matched FilePluginPrims).<br>
<br>
<br>
<br>
Testing<br>
=======<br>
<br>
- As mentioned above I've been using this on a daily basis since May 2017.<br>
Some of the packages I have loaded include: Roassal2, Glorp,<br>
OSSubprocess, NeoCSV, DataFrame.<br>
- Automated tests have been added for the new functionality.<br>
- Additional automated tests have been added for existing functionality.<br>
- All existing tests pass<br>
<br>
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888">Alistair<br>
<br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><span style="font-size:small;border-collapse:separate"><div>_,,,^..^,,,_<br></div><div>best, Eliot</div></span></div></div></div>
</div>