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 <[hidden email]> wrote: Hi Esteban, Guille, Marcus, Cyril and Everyone, _,,,^..^,,,_ best, Eliot |
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 <[hidden email]> 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 <[hidden email]> 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 |
Free forum by Nabble | Edit this page |