Pharo's FileSystem behaviour is currently somewhat inconsistent in its
treatment of path strings.

To be able to demonstrate the limitations and fixes, I'm assuming that
the following files and directories have been created (/dev/shm is a
memory based file system in Ubuntu):

cd /dev/shm
mkdir -p d1/d2/d3/d4
touch d1/t1.txt d1/d2/t2.txt d1/d2/d3/t3.txt d1/d2/d3/d4/t4.txt
pushd d1 && ln -s d2/d3 ./s3 && popd



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)"


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.

E.g. 

('/dev/shm/d1/' asFileReference / 'd2/d3/t3.txt') exists  " true"

('/dev/shm/d1/' asFileReference / 'd2/d3/t3.txt') parent  " File @ /dev/shm/d1"


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"


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"



3. Adds unit tests to cover the changed behaviour.


While I believe that these changes improve Pharo overall, making it more
intuitive and consistent, modifying the path creation to remove
canonicalisation is not backward compatible and requires a change to
PathTest>>testRelativeFromStringNormalization and
PathTest>>testRelativeFromStringNormalizationParent to manually
canonicalise the paths.

Before I submit the patch, does anyone have any strong objections to the
changes as described above?

Thanks,
Alistair

Reply via email to