Skip to content

Commit da453fb

Browse files
authored
Make copying more predictable. (#73)
Don't make the copy operation dependent on whether the target directory exists or not. Making it dependent has the following drawbacks: 1. the directory could be created outside of our control. 2. It's annoying in programs to test first whether the target exists. 3. There isn't a good way to copy the content of the source to the target if it already exists.
1 parent 9a15c71 commit da453fb

File tree

2 files changed

+68
-36
lines changed

2 files changed

+68
-36
lines changed

src/file.toit

Lines changed: 53 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -336,16 +336,18 @@ Reads the destination of the link $name
336336
readlink name/string -> string:
337337
#primitive.file.readlink
338338

339-
/** Windows specific attribute for read-only files */
340-
WINDOWS-FILE-ATTRIBUTE-READONLY ::= 0x01
341-
/** Windows specific attribute for hidden files */
342-
WINDOWS-FILE-ATTRIBUTE-HIDDEN ::= 0x02
343-
/** Windows specific attribute for system files */
344-
WINDOWS-FILE-ATTRIBUTE-SYSTEM ::= 0x04
345-
/** Windows specific attribute for normal files */
346-
WINDOWS-FILE-ATTRIBUTE-NORMAL ::= 0x80
347-
/** Windows specific attribute for archive files */
348-
WINDOWS-FILE-ATTRIBUTE-ARCHIVE ::= 0x20
339+
/** Windows specific attribute for read-only files. */
340+
WINDOWS-FILE-ATTRIBUTE-READONLY ::= 0x01
341+
/** Windows specific attribute for hidden files. */
342+
WINDOWS-FILE-ATTRIBUTE-HIDDEN ::= 0x02
343+
/** Windows specific attribute for system files. */
344+
WINDOWS-FILE-ATTRIBUTE-SYSTEM ::= 0x04
345+
/** Windows specific attribute for sub directories. */
346+
WINDOWS-FILE-ATTRIBUTE-DIRECTORY ::= 0x10
347+
/** Windows specific attribute for archive files. */
348+
WINDOWS-FILE-ATTRIBUTE-ARCHIVE ::= 0x20
349+
/** Windows specific attribute for normal files. */
350+
WINDOWS-FILE-ATTRIBUTE-NORMAL ::= 0x80
349351

350352
/**
351353
Changes filesystem permissions for the file $name to $permissions.
@@ -356,10 +358,13 @@ chmod name/string permissions/int:
356358
/**
357359
Copies $source to $target.
358360
359-
If $target is a directory, then takes the basename of $source and appends it to
360-
$target.
361+
If $source is a file, then $target contains the copy and permissions of $source after
362+
the call.
363+
If $source is a directory and $recursive is true, then $target contains the copy of
364+
the content of $source after the call. That is, all files that exist in $source
365+
will exist in $target after the call. The $target directory may exist.
361366
362-
The location for $target must exist. That is, when copying to `foo/bar`, `foo`
367+
The location (dirname) of $target must exist. That is, when copying to `foo/bar`, `foo`
363368
must exist.
364369
365370
If $dereference is true, then symbolic links are followed.
@@ -370,10 +375,14 @@ If $recursive is true, then directories are copied recursively. If $recursive is
370375
copy --source/string --target/string --dereference/bool=false --recursive/bool=false -> none:
371376
// A queue for pending recursive copies.
372377
queue := Deque
373-
if is-directory target:
374-
target = "$target/$(basename_ source)"
375378

376-
copy_ --source=source --target=target --dereference=dereference --recursive=recursive --queue=queue
379+
copy_
380+
--source=source
381+
--target=target
382+
--dereference=dereference
383+
--recursive=recursive
384+
--queue=queue
385+
--allow-existing-target-directory
377386
while not queue.is-empty:
378387
next := queue.remove-first
379388
new-source := next[0]
@@ -405,33 +414,45 @@ copy --source/string --target/string --dereference/bool=false --recursive/bool=f
405414
--dereference=dereference
406415
--recursive=recursive
407416
--queue=queue
417+
--no-allow-existing-target-directory
408418
finally:
409419
directory-stream.close
410420
if target-permissions != -1:
411421
// Make the directory read-only again.
412422
chmod new-target target-permissions
413423

424+
SPECIAL-WINDOWS-PERMISSIONS_ ::= WINDOWS-FILE-ATTRIBUTE-READONLY | WINDOWS-FILE-ATTRIBUTE-HIDDEN
425+
414426
/**
415427
Copies $source to $target.
416428
417429
The given $queue is filled with pending recursive copies. Each entry in the $queue
418430
is a pair of source, target, where both are directories that exist.
419431
*/
420-
copy_ --source/string --target/string --dereference/bool --recursive/bool --queue/Deque -> none:
432+
copy_ -> none
433+
--source/string
434+
--target/string
435+
--dereference/bool
436+
--recursive/bool
437+
--queue/Deque
438+
--allow-existing-target-directory/bool:
439+
is-windows := system.platform == system.PLATFORM-WINDOWS
440+
421441
source-stat := stat source --follow-links=dereference
422442
if not source-stat:
423443
throw "File/directory $source not found"
444+
source-permissions := source-stat[ST-MODE]
445+
type := source-stat[ST-TYPE]
424446
target-stat := stat target
425-
if target-stat:
447+
if target-stat and (type != DIRECTORY or not allow-existing-target-directory):
426448
throw "'$target' already exists"
427449

428-
type := source-stat[ST-TYPE]
429450
if type == SYMBOLIC-LINK or type == DIRECTORY-SYMBOLIC-LINK:
430451
// When taking the stat of the source we already declared whether we
431452
// dereference the link or not. If we are here, then we do not dereference
432453
// and should thus copy the link.
433454
link-target := readlink source
434-
if system.platform == system.PLATFORM-WINDOWS:
455+
if is-windows:
435456
// Work around https://github.com/toitlang/toit/issues/2090, where reading
436457
// an absolute symlink starts with '\??\' which Toit can't deal with if
437458
// written as value of a link.
@@ -445,19 +466,29 @@ copy_ --source/string --target/string --dereference/bool --recursive/bool --queu
445466
if type == DIRECTORY:
446467
if not recursive:
447468
throw "Cannot copy directory '$source' without --recursive"
448-
mkdir target source-stat[ST-MODE]
469+
if not target-stat:
470+
mkdir target source-permissions
471+
if is-windows and source-permissions & SPECIAL-WINDOWS-PERMISSIONS_ != 0:
472+
// The Windows file attributes are not taken into account when creating a new directory.
473+
// Apply them now.
474+
chmod target source-permissions
475+
449476
queue.add [source, target]
450477
return
451478

452479
in-stream := Stream.for-read source
453-
out-stream := Stream.for-write target --permissions=source-stat[ST-MODE]
480+
out-stream := Stream.for-write target --permissions=source-permissions
454481
out-writer := Writer out-stream
455482
try:
456483
while data := in-stream.read:
457484
out-writer.write data
458485
finally:
459486
in-stream.close
460487
out-writer.close
488+
if is-windows and (source-permissions & SPECIAL-WINDOWS-PERMISSIONS_) != 0:
489+
// The Windows file attributes are not taken into account when creating a new file.
490+
// Apply them now.
491+
chmod target source-permissions
461492

462493
/**
463494
Returns the directory part of the given $path.

tests/copy_test.toit

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ with-tmp-dir [block]:
1616
directory.rmdir --force --recursive tmp-dir
1717

1818
main:
19-
// test-recursive
20-
// test-permissions
19+
test-recursive
20+
test-permissions
2121
test-symlinks
2222

2323
test-recursive:
@@ -34,16 +34,9 @@ test-recursive:
3434
file.copy --source=tmp-file --target=file2
3535
expect-equals content (file.read-content file2)
3636

37-
// Copy of absolute path to absolute directory path.
38-
subdir := "$tmp-dir/subdir"
39-
directory.mkdir subdir
40-
sub-file := "$subdir/file.txt"
41-
file.copy --source=tmp-file --target=subdir
42-
expect-equals content (file.read-content sub-file)
43-
4437
// Copy of relative file to relative directory path.
4538
directory.mkdir "subdir2"
46-
file.copy --source="file.txt" --target="subdir2"
39+
file.copy --source="file.txt" --target="subdir2/file.txt"
4740
expect-equals content (file.read-content "subdir2/file.txt")
4841
expect-equals content (file.read-content "$tmp-dir/subdir2/file.txt")
4942

@@ -54,16 +47,24 @@ test-recursive:
5447
expect-equals content (file.read-content "subdir3/file.txt")
5548
expect-equals other-content (file.read-content "subdir3/nested-subdir/other.txt")
5649

50+
// Copy recursive to existing directory.
51+
directory.mkdir "subdir4"
52+
file.copy --source="subdir3" --target="subdir4" --recursive
53+
expect-equals content (file.read-content "subdir4/file.txt")
54+
expect-equals other-content (file.read-content "subdir4/nested-subdir/other.txt")
55+
5756
test-permissions:
5857
file-permission0/int := ?
5958
file-permission1/int := ?
6059
dir-permission/int := ?
6160
read-only-dir-permission/int := ?
6261
if platform == PLATFORM-WINDOWS:
63-
file-permission0 = file.WINDOWS-FILE-ATTRIBUTE-HIDDEN | file.WINDOWS-FILE-ATTRIBUTE-NORMAL
64-
file-permission1 = file.WINDOWS-FILE-ATTRIBUTE-READONLY | file.WINDOWS-FILE-ATTRIBUTE-NORMAL
65-
dir-permission = 0
66-
read-only-dir-permission = file.WINDOWS-FILE-ATTRIBUTE-READONLY
62+
// We use the "archive" attribute, since we modify the files, and this is the default
63+
// attribute for modified files. The idea is that the backup software clears this bit.
64+
file-permission0 = file.WINDOWS-FILE-ATTRIBUTE-HIDDEN | file.WINDOWS-FILE-ATTRIBUTE-ARCHIVE
65+
file-permission1 = file.WINDOWS-FILE-ATTRIBUTE-READONLY | file.WINDOWS-FILE-ATTRIBUTE-ARCHIVE
66+
dir-permission = file.WINDOWS-FILE-ATTRIBUTE-DIRECTORY
67+
read-only-dir-permission = file.WINDOWS-FILE-ATTRIBUTE-READONLY | file.WINDOWS-FILE-ATTRIBUTE-DIRECTORY
6768
else:
6869
file-permission0 = 0b111_000_000
6970
file-permission1 = 0b100_000_000

0 commit comments

Comments
 (0)