Hi Subbu,
On Tue, Jun 27, 2017 at 09:19:20PM +0530, K K Subbu wrote:
> On Monday 26 June 2017 01:42 PM, Alistair Grant wrote:
> >1. Path's are canonicalised during initial creation, but are not when
> >extending the path, i.e. sending #/
> >
> >E.g.
> >
> >'/dev/shm/d1/d2/../t1.txt' asFileReference " File @ /dev/shm/d1/t1.txt"
> >
> >'/dev/shm/d1/' asFileReference / 'd2/../t1.txt' " File @
> >/dev/shm/d1/d2/../t1.txt"
> >
> >Automatic canonicalisation is problematic as the code doesn't handle
> >symbolic links in the path name:
> >
> >('/dev/shm/d1/' asFileReference / 's3/../t2.txt') exists " true (correct
> >answer)"
> >
> >'/dev/shm/d1/s3/../t2.txt' asFileReference exists " false (wrong answer)"
>
> The meaning of 's3/..' is ambiguous when s3 is a symlink. Should 's3' be
> resolved or not before #parent op is applied? Disambiguation depends on
> context, so early canonicalization is a bug.
>
> My own preference is to not to canonicalize at all and 's3/..' should be
> left as {s3, ..}. See
>
> http://man7.org/linux/man-pages/man7/path_resolution.7.html
>
> >2. In an attempt to be completely general, the argument to #/ is assumed
> >to be a single directory / file. This is incorrect as the path
> >delimiter is not allowed to be part of the file name. The result is
> >that path comparisons and operations such as #parent give unexpected
> >results.
>
> Path class>>from:delimiter has a bug. It attempts to canonicalize elements
> parsed from the String regardless of the context. If the code is changed to:
>
> pathClass withAll: (aDelimiterCharacter split: aString)
>
> your test will pass.
This is basically what I do (it also removes empty segments).
> I wonder why Path was subclassed from Object and not from
> SequenceableCollection?
>
> >My PR modified FileSystem so that:
> >
> >1. Canonicalisation is separated out as an operation that has to be
> >manually requested:
> >
> >'/dev/shm/d1/d2/../t1.txt' asFileReference " File @
> >/dev/shm/d1/d2/../t1.txt"
> >
> >'/dev/shm/d1/d2/../t1.txt' asFileReference canonicalize " File @
> >/dev/shm/d1/t1.txt"
>
> I am ok with this.
>
> >2. The argument to #/ can be either a single file / directory name or a
> >path.
> >
> >('/dev/shm/d1/' asFileReference / 'd2/d3/t3.txt') parent " File @
> >/dev/shm/d1/d2/d3"
>
> ok with this too.
>
> >3. Adds unit tests to cover the changed behaviour.
> >...
> >Before I submit the patch, does anyone have any strong objections to the
> >changes as described above?
>
> None from my side.
>
> Regards .. Subbu
Thanks!
Alistair
