client crash on copying non empty directory

[evgmik]

trafficstars

Steps to reproduce: start fresh and do

$ ali touch /test/dir1/t1
$ ali touch /test/dir1/t2
$ ali touch /test/dir1/t3
$ ali cp /test/dir1 /test/copydir1
08.03.2021/23:47:49 ⚡ cmd/parser.go:598:  panic rollback: runtime error: slice bounds out of range [14:13]; stack: goroutine 219 [running]:
runtime/debug.Stack(0xc0001767b0, 0xc0003f0140, 0xc000a780a0)
        /usr/lib/go-1.14/src/runtime/debug/stack.go:24 +0x9d

I just show first line of the crash report. Depending on what inside of the parent dir (I think it subdirs), the repo database could be left in a broken state.

While we at it, it would be nice to be able to copy to non-existing path. For example, in above to do ali cp /test/dir1 /to/new/non/existing/dir

[sahib]

Will check next.

[...] the repo database could be left in a broken state.

It should not, there is some defence against crashes in the linker. If the state got broken that is a separate bug.

While we at it, it would be nice to be able to copy to non-existing path. For example, in above to do ali cp /test/dir1 /to/new/non/existing/dir

I guess we can do a mkdir in the client to make this work.

[evgmik]

While we at it, it would be nice to be able to copy to non-existing path. For example, in above to do ali cp /test/dir1 /to/new/non/existing/dir

I guess we can do a mkdir in the client to make this work.

Note that staging to non existing dir works, i.e.

$ ali tree
•

1 directory, 0 files
$ ali touch /to/new/non/existing/dir/testfile
$ ali tree
• ✔
└── to/ ✔
   └── new/ ✔
      └── non/ ✔
         └── existing/ ✔
            └── dir/ ✔
               └── testfile ✔

6 directories, 1 file

Which deviates from standard shell touch. So the question, shall we preserve shell cp style and the current brig, where the destination dir must exist, or shall we allow such copy to succeed?

[evgmik]

Still digging the top most bug description with recursive copy, apparently we have a problem when a directory copied inside of another directory. Looks like Walk get into recursion of some type or maybe adding a child to a parent triggers consequent Walk over itself.

Quick questions: how content of the directory is calculated? Is it based on names of the children or their content?

Also, am I right that Tree hash is calculated based on the full path to the node? If I am right, than what is the point? We can just use full path for the DB key.

Also, order field in the directory node, seems to be unneeded. It can be calculated on a fly, from the children names.

[sahib]

Looks like Walk get into recursion of some type or maybe adding a child to a parent triggers consequent Walk over itself.

IIRC the logic for NotifyMove was not made for the Copy() case. It gets confused about the paths. I had a quick look and decided that it's probably wise to first do the staging-related re-design in linker.

Quick questions: how content of the directory is calculated? Is it based on names of the children or their content?

It's only based on the directory path, no content involved.

Also, am I right that Tree hash is calculated based on the full path to the node? If I am right, than what is the point? We can just use full path for the DB key.

That would only serve as key for one version of that file.

Also, order field in the directory node, seems to be unneeded. It can be calculated on a fly, from the children names.

Partly yes, It trades storage against CPU. On Add() for example we can do a log(n) binary search for the insert, while on the fly we have to do n * log(n). Do you suspect a problem with the storage space?