Copying a directory without subdirectories should work fine. Are you seeing something going wrong where there are no subdirectories?

Copying a directory with subdirectories also 'works' (the first time). Copying whole directories 'works' (the first time). When a build happens and then a file is added to such directories the new file will not be copied in subsequent builds (there is no dependency on it).

For the current copy_command implementation see https://cs.chromium.org/chromium/src/build/toolchain/toolchain.gni?type=cs&q=copy_command&sq=package:chromium&g=0&rcl=c77b371695be9335ffe86608871d9dc2fd537d8a&l=108 

IIRC there's a dependency on the directory itself, and the directory's mtime gets updated when a file is added. (Only the direct parent, not all parents, which is why I think this might work as long as you don't have subdirs.)

True, in the case of adding files the immediate directory mtime is updated (at least with normal mount flags). Modifying a file in that directory does not affect the directory mtime. However, most editors hardly ever simply modify a file (they delete the old one and replace it with the new one) so most modifications look like adding files. However, on Windows editing a file with Notepad updates the 'W' time of the file, but not its containing directory (editing in Visual Studio does update the 'W' time of the containing directory). I'm not advocating the use of Notepad, but I have been known to use it locally for a super simple edit or two.

The current 'copy' seems to be explicitly recursive (on Windows it appears to be backed by shutil.copytree) which was of more concern. I think the only way to check if anything relies on this would be to at least locally change the copy_command to something which checks and fails if there are any subdirectories.

However, allowing copying directories generally makes me feel uneasy as the build no longer explicitly states its dependencies, it seems like a mostly invisible glob. Explicit dependencies seems to be a goal which gn and ninja have been striving (even if it is a rocky road getting there).

(To be clear, I personally agree with what you say; just wanted to point out that it happens to work in practice in many cases.)

It seems like this is probably a bug, or at least an unintentional feature. From looking at the docs and the code, I see no evidence that we intentionally wanted to support copying directories. I wonder how hard it would be at this point to detect and reject such uses.

@brettw - do you know if this was intentionally supported?

bump?

FWIW I made a CL to make copy() not support directories and try jobs look red at least on cronet: https://chromium-review.googlesource.com/c/chromium/src/+/1186031 :

[66/6116] COPY ../../components/cronet/test/data cronet/test/assets/test
FAILED: cronet/test/assets/test 
ln -f ../../components/cronet/test/data cronet/test/assets/test 2>/dev/null || (rm -rf cronet/test/assets/test && cp -pf ../../components/cronet/test/data cronet/test/assets/test)
cp: omitting directory '../../components/cronet/test/data'

from here: https://cs.chromium.org/chromium/src/components/cronet/android/BUILD.gn?type=cs&q=cronet/test/data+file:%5C.gn&sq=package:chromium&g=0&l=1476

We don't want to copy directories because this is an implicit glob. If somebody adds a file to the directory, we won't know the target is dirty and can't do incremental builds properly. If it worked it was just a side effect of how the tool command was implemented.

If someone adds a file, then the directory mtime is updated and since gn writes a dep to the directory node we do know how to rerun (as long as there aren't nested dirs).

(I'm not arguing that this is a good feature, but functionality-wise it does work better than people believe.)

If you update the file (as opposed to adding or removing it), I think the mtime is generally *not* modified, right? So you'd probably need an additional depfile anyway in order to correctly update copies? 

$ cd /tmp
$ mkdir foo
$ ls -l | grep foo
drwxr-xr-x  2 thakis  wheel   68 Aug 27 14:57 foo
# ...patiently wait 60 seconds...
$ touch foo/asdf
$ ls -l | grep foo
drwxr-xr-x  3 thakis  wheel  102 Aug 27 14:58 foo

Tha's adding a file, right? Not modifying an existing file.

On both Mac and Linux, in a regular chromium checkout, if I do:

$ date
Mon Aug 27 12:05:54 PDT 2018
$ ls -l | grep build/
drwxr-xr-x  100 dpranke  staff    3200 Aug 27 11:55 build/
$ touch build/gypi_to_gn.py
$ ls -l | grep build/
drwxr-xr-x  100 dpranke  staff    3200 Aug 27 11:55 build/
$

Ah, you're right.