luci-go/common/dirwalk: Code for walking a directory tree efficiently

common/dirwalk/observer.go

Index: common/dirwalk/observer.go
diff --git a/common/dirwalk/observer.go b/common/dirwalk/observer.go
new file mode 100644
index 0000000000000000000000000000000000000000..46e7bc86792ab5bb39c0c1d10378fcf43bfa5961
--- /dev/null
+++ b/common/dirwalk/observer.go
@@ -0,0 +1,24 @@
+// Copyright 2016 The LUCI Authors. All rights reserved.
+// Use of this source code is governed under the Apache License, Version 2.0
+// that can be found in the LICENSE file.
+
+package dirwalk
+
+/**

[M-A Ruel, 2016/09/15 14:31:02]

Always use // comments for consistency. I do it the ol' ye cranky style: Ctrl-V,
3LI, "// ", Esc.


(I lied, I do use Arrow keys but shuuuut, don't tell anymore, in particular
stip!)

[mithro, 2016/09/20 12:41:43]

Done.

+Interface for receiving the results of walking a directory tree.
+
+For performance reasons, small files and large files are treated differently.
+
+SmallFile and LargeFile must be called in sorted order.
+*/
+type WalkObserver interface {
+	SmallFile(filename string, alldata []byte)
+	LargeFile(filename string)

[M-A Ruel, 2016/09/15 14:31:02]

why not just have OnFile(path string, content []byte) and pass nil on content
when the file is deemed "large enough".

In fact, I'd rather have Error() be merged in too.

[mithro, 2016/09/20 12:41:43]

I originally had a single call back which provided a bunch more information (the
file size and such) but ended up culling back to providing the bare minimum for
both types.

I think having separate callbacks makes it clearer what the expected information
for each type of file is.
I don't quite understand how the Error() would be merged? Just as another
argument? You then end up with a bunch of optional arguments? With that you end
up with;
 File(filename string, alldata []byte (or nil), err error (or nil))

The Start/Finish directory functions are likely to be needed in the future (as
they are useful indicators for better sharding then just every N files) which
means this interface is going to expand then too.

[M-A Ruel, 2016/09/20 16:37:26]

I disagree. The walk is finished when the function accepting this interface
returns. There's still no need for Finished().

I prefer File(filename string, alldata []byte, err error)
Then as a matter of fact, you don't even need an interface anymore, just a
function call back. It's much simpler.

[mithro, 2016/09/22 11:19:58]

You seemed to miss the part where I mentioned the StartDir/FinishDir functions
will probably be need
Without the Finished callback I can't figure out how to make the
ParallelHashWalker work in the walkdir tool without needing special casing for
that walker.

The ParallelHashWalker doesn't finish until a hash has been done on all paths,
that occur well after the last path has been found. Without the Finished call
the program exits when the last path has been found and thus doesn't processing
the remaining queued files.
Something else I forgot to mention is that the error relates to a given path -
you don't know if that path is a file, directory or something else. Calling
"File(<path>, nil, <error>)" when the error is for a directory is actually going
to cause confusion.

+
+	//StartDir(dirname string) error
+	//FinishDir(dirname string)
+
+	Error(pathname string, err error)
+
+	Finished()

[M-A Ruel, 2016/09/15 14:31:02]

It's finished when the function returns.


Overall, this whole interface can be replaced by a single callback.

[mithro, 2016/09/20 12:41:44]

See my comments below.

+}