Smalltalk › Pharo › Pharo Smalltalk Developers

#renameTo: and #moveTo:

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

7 messages Options

alistairgrant

#renameTo: and #moveTo:

Hi All,

While recently running the test suite I've been getting failures in
FileLocatorTest>>testMoveTo if the working directory when the test
is run is on a different unix file system to the user's home directory.

The failing primitive is 'primitiveFileRename' in module 'FilePlugin',
which calls rename(), which is presumably part of libc - I didn't chase
it back through the C code.

libc rename() requires the source and destination filenames to be on the
same file system.

This has been discussed multiple times before:

https://pharo.fogbugz.com/f/cases/13957/Add-exception-for-cross-volume-folder-renames
https://pharo.fogbugz.com/f/cases/12992/Cannot-move-files-to-another-volume-partition-under-linux
https://pharo.fogbugz.com/f/cases/12965/Cannot-moveTo-FileLocator

Issue 12965 is even supposed to include a fix, although when I searched
through the slice I couldn't find anything that actually looked like a
fix.

As Nicolai suggested in 12965, the obvious solution is to implement move
as copy+delete. It looks like there isn't an easy way to check whether
the two files are on the same unix file system, so presumably it would
be implemented as try to rename, and if that fails, try to copy and
delete.

Is there a reason not to propose this as a patch?

Thanks,
Alistair

alistairgrant

Re: #renameTo: and #moveTo:

On Wed, Apr 19, 2017 at 09:57:41PM +0200, Nicolas Cellier wrote:

> 2017-04-19 21:41 GMT+02:00 Alistair Grant <[hidden email]>:
>
> Hi All,
>
> While recently running the test suite I've been getting failures in
> FileLocatorTest>>testMoveTo if the working directory when the test
> is run is on a different unix file system to the user's home directory.
>
> The failing primitive is 'primitiveFileRename' in module 'FilePlugin',
> which calls rename(), which is presumably part of libc - I didn't chase
> it back through the C code.
>
> libc rename() requires the source and destination filenames to be on the
> same file system.
>
> This has been discussed multiple times before:
>
> https://pharo.fogbugz.com/f/cases/13957/Add-exception-for-
> cross-volume-folder-renames
> https://pharo.fogbugz.com/f/cases/12992/Cannot-move-files-
> to-another-volume-partition-under-linux
> https://pharo.fogbugz.com/f/cases/12965/Cannot-moveTo-FileLocator
>
> Issue 12965 is even supposed to include a fix, although when I searched
> through the slice I couldn't find anything that actually looked like a
> fix.
>
> As Nicolai suggested in 12965, the obvious solution is to implement move
> as copy+delete. It looks like there isn't an easy way to check whether
> the two files are on the same unix file system, so presumably it would
> be implemented as try to rename, and if that fails, try to copy and
> delete.
>
>
> What? Google this: man stat
> The first field of stat structure is a device ID, so deciding if two files are
> on same file system or not is doable by checking this field for the source file
> and destination file (if it exists) or directory - modulo the fact that you'd
> have to care of symbolic links with lstat.

I meant from within the image. Also, while this is fine for Linux,
and probably for MacOS, what about Windows?

It would be great if this exists in the image. Maybe it's time for me
to learn FFI.

> Is there a reason not to propose this as a patch?

Cheers,
Alistair

Stephane Ducasse-3

Re: #renameTo: and #moveTo:

In reply to this post by alistairgrant

Thanks for looking at this. We should really improve this part.

On Wed, Apr 19, 2017 at 9:41 PM, Alistair Grant <[hidden email]> wrote:

Hi All,

While recently running the test suite I've been getting failures in
FileLocatorTest>>testMoveTo if the working directory when the test
is run is on a different unix file system to the user's home directory.

The failing primitive is 'primitiveFileRename' in module 'FilePlugin',
which calls rename(), which is presumably part of libc - I didn't chase
it back through the C code.

libc rename() requires the source and destination filenames to be on the
same file system.

This has been discussed multiple times before:

https://pharo.fogbugz.com/f/cases/13957/Add-exception-for-cross-volume-folder-renames
https://pharo.fogbugz.com/f/cases/12992/Cannot-move-files-to-another-volume-partition-under-linux
https://pharo.fogbugz.com/f/cases/12965/Cannot-moveTo-FileLocator

Issue 12965 is even supposed to include a fix, although when I searched
through the slice I couldn't find anything that actually looked like a
fix.

As Nicolai suggested in 12965, the obvious solution is to implement move
as copy+delete. It looks like there isn't an easy way to check whether
the two files are on the same unix file system, so presumably it would
be implemented as try to rename, and if that fails, try to copy and
delete.

Is there a reason not to propose this as a patch?

Thanks,
Alistair

alistairgrant

Re: #renameTo: and #moveTo:

Hi Stef,

On Thu, Apr 20, 2017 at 08:11:31PM +0200, Stephane Ducasse wrote:
> Thanks for looking at this. We should really improve this part.

No problem. I've just finished coding and testing.

The one part I'm not completely happy with is what I mentioned earlier:
I'm not aware of any way within Pharo to determine whether two files are
on the same disk filesystem, so currently the code tries a rename, and if
that fails it falls back to copy and delete. If I get a chance to
learn FFI soon I may try and improve it.

The original FileLocatorTest>>testMoveTo test now passes, and running
the entire test suite produces the same results (there are a few other
tests that fail).

There are some additional tests, so it currently checks:

- Moving files within the disk filesystem works.
- Moving files within the memory filesystem works.
- Moving files from the memory filesystem to the disk filesystem works.

Since this is a core part of the system, I'm going to wait until Pharo 7
development begins before posting the patch. This will also provide
some ad-hoc testing as I've got this in my daily work image.

Cheers,
Alistair

> On Wed, Apr 19, 2017 at 9:41 PM, Alistair Grant <[hidden email]> wrote:
>
> Hi All,
>
> While recently running the test suite I've been getting failures in
> FileLocatorTest>>testMoveTo if the working directory when the test
> is run is on a different unix file system to the user's home directory.
>
> The failing primitive is 'primitiveFileRename' in module 'FilePlugin',
> which calls rename(), which is presumably part of libc - I didn't chase
> it back through the C code.
>
> libc rename() requires the source and destination filenames to be on the
> same file system.
>
> This has been discussed multiple times before:
>
> https://pharo.fogbugz.com/f/cases/13957/Add-exception-for-
> cross-volume-folder-renames
> https://pharo.fogbugz.com/f/cases/12992/Cannot-move-files-
> to-another-volume-partition-under-linux
> https://pharo.fogbugz.com/f/cases/12965/Cannot-moveTo-FileLocator
>
> Issue 12965 is even supposed to include a fix, although when I searched
> through the slice I couldn't find anything that actually looked like a
> fix.
>
> As Nicolai suggested in 12965, the obvious solution is to implement move
> as copy+delete. It looks like there isn't an easy way to check whether
> the two files are on the same unix file system, so presumably it would
> be implemented as try to rename, and if that fails, try to copy and
> delete.
>
> Is there a reason not to propose this as a patch?
>
> Thanks,
> Alistair

Nicolas Cellier

Re: #renameTo: and #moveTo:

In reply to this post by alistairgrant

2017-04-20 9:53 GMT+02:00 Alistair Grant <[hidden email]>:

On Wed, Apr 19, 2017 at 09:57:41PM +0200, Nicolas Cellier wrote:
> 2017-04-19 21:41 GMT+02:00 Alistair Grant <[hidden email]>:
>
> Hi All,
>
> While recently running the test suite I've been getting failures in
> FileLocatorTest>>testMoveTo if the working directory when the test
> is run is on a different unix file system to the user's home directory.
>
> The failing primitive is 'primitiveFileRename' in module 'FilePlugin',
> which calls rename(), which is presumably part of libc - I didn't chase
> it back through the C code.
>
> libc rename() requires the source and destination filenames to be on the
> same file system.
>
> This has been discussed multiple times before:
>
> https://pharo.fogbugz.com/f/cases/13957/Add-exception-for-
> cross-volume-folder-renames
> https://pharo.fogbugz.com/f/cases/12992/Cannot-move-files-
> to-another-volume-partition-under-linux
> https://pharo.fogbugz.com/f/cases/12965/Cannot-moveTo-FileLocator
>
> Issue 12965 is even supposed to include a fix, although when I searched
> through the slice I couldn't find anything that actually looked like a
> fix.
>
> As Nicolai suggested in 12965, the obvious solution is to implement move
> as copy+delete. It looks like there isn't an easy way to check whether
> the two files are on the same unix file system, so presumably it would
> be implemented as try to rename, and if that fails, try to copy and
> delete.
>
>
> What? Google this: man stat
> The first field of stat structure is a device ID, so deciding if two files are
> on same file system or not is doable by checking this field for the source file
> and destination file (if it exists) or directory - modulo the fact that you'd
> have to care of symbolic links with lstat.

I meant from within the image. Also, while this is fine for Linux,
and probably for MacOS, what about Windows?

Ah, so it's more a Squeak/Pharo VM problem than a unix problem ;)

You are right, there is a call to stat() in unix file plugin

https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/unix/plugins/FilePlugin/sqUnixFile.c

but the device info is not passed back to the image...

Too bad!

As for windows, it's not a problem, there is a rename function that would move files across devices (but not directories!)

See rename() on msdn https://msdn.microsoft.com/en-us/library/zw5t957f.aspx

It would be great if this exists in the image. Maybe it's time for me
to learn FFI.

Yes, that would probably work.

However, what I don't like with FFI is that we have to pass a pointer on the stat structure to the foreign function.

But what is the exact layout of this structure? Is it identical on all suported OS? Is the order of fields stable across implementations?
Is it going to gain new fields in future versions?

I'm not so sure that a standard does impose those implementation details...

So someone must check image implementation on each supported OS (and major OS version).

Practically, the layout is not going to change everyday, but we have no guaranty.

If it's C source code, then those details are not exposed, they are in implementation provided headers, and we can compile identical source code at application side (the file plugin).

We already had this conversation many times. Eliot said that image side implementation-defined features like constants or structure size and layout could be generated, but this work is yet to be done.

> Is there a reason not to propose this as a patch?

Cheers,
Alistair

alistairgrant

Re: #renameTo: and #moveTo:

On Thu, Apr 20, 2017 at 09:21:01PM +0200, Nicolas Cellier wrote:

> 2017-04-20 9:53 GMT+02:00 Alistair Grant <[hidden email]>:
>
> > What? Google this: man stat The first field of stat structure is
> > a device ID, so deciding if two files are on same file system or
> > not is doable by checking this field for the source file and
> > destination file (if it exists) or directory - modulo the fact
> > that you'd have to care of symbolic links with lstat.
>
> I meant from within the image. Also, while this is fine for
> Linux, and probably for MacOS, what about Windows?
>
> Ah, so it's more a Squeak/Pharo VM problem than a unix problem ;) You
> are right, there is a call to stat() in unix file plugin
>
> https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/unix/plugins/FilePlugin/sqUnixFile.c
>
> but the device info is not passed back to the image... Too bad!

But it should be a straightforward extension. I think I'll ask on
vm-dev if anyone objects to me adding this extension.

Thanks for pointing this out!

Cheers,
Alistair

> As for windows, it's not a problem, there is a rename function that
> would move files across devices (but not directories!) See rename() on
> msdn https://msdn.microsoft.com/en-us/library/zw5t957f.aspx

Stephane Ducasse-3

Re: #renameTo: and #moveTo:

Yes we should improve the primitive information return back to the image.

On Fri, Apr 21, 2017 at 10:08 AM, Alistair Grant <[hidden email]> wrote:

On Thu, Apr 20, 2017 at 09:21:01PM +0200, Nicolas Cellier wrote:
> 2017-04-20 9:53 GMT+02:00 Alistair Grant <[hidden email]>:
>
> > What? Google this: man stat The first field of stat structure is
> > a device ID, so deciding if two files are on same file system or
> > not is doable by checking this field for the source file and
> > destination file (if it exists) or directory - modulo the fact
> > that you'd have to care of symbolic links with lstat.
>
> I meant from within the image. Also, while this is fine for
> Linux, and probably for MacOS, what about Windows?
>
> Ah, so it's more a Squeak/Pharo VM problem than a unix problem ;) You
> are right, there is a call to stat() in unix file plugin
>
> https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/unix/plugins/FilePlugin/sqUnixFile.c
>
> but the device info is not passed back to the image... Too bad!

But it should be a straightforward extension. I think I'll ask on
vm-dev if anyone objects to me adding this extension.

Thanks for pointing this out!

Cheers,
Alistair

> As for windows, it's not a problem, there is a rename function that
> would move files across devices (but not directories!) See rename() on
> msdn https://msdn.microsoft.com/en-us/library/zw5t957f.aspx