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

common/dirwalk/tests/tools/gendir/main.go

+// 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 main
+
+// Quick tool for generating directories to walk.

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

Move this to a real package docstring.

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

This docstring is only for the tool though? (The whole package is "dirwalk"
right?)

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

Yes, but tools still show up in godoc.

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

The part I didn't understand was that you wanted this as a string for the
"package main" in this directory. (The idea that there are multiple "package
main" things I still find very confusing to me.)

I have done that.

+
+import (
+        "encoding/json"
+        "flag"
+        "fmt"
+        "io/ioutil"
+        "log"
+        "math/rand"
+        "os"
+)
+
+type TestSettings struct {
+        Name    string
+        Comment string
+        Seed    int64
+        Tree    TreeSettings
+}
+
+var config = flag.String("config", "", "JSON config file for generating test file.")

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

I'm a big fan of putting these inside the main function. I'm also a big fan of
the 
func mainImpl() error { ... }
pattern so you can return an error and it is nicely printed out, instead of
log.Fatal(), which is only useful for toys IMHO.

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

Do you have an example? I'm not sure I understand...

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

Here's the simplest example I could find:
https://github.com/maruel/dlibox/blob/master/go/pio/cmd/ir/main.go

the other tools in luci-go generally have subcommands so their design is
different.

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

Done for both tools.

+var outdir = flag.String("outdir", "", "Where to write the output.")
+var remove = flag.Bool("remove", false, "Remove the directory if it exists.")
+var seed = flag.Int64("seed", 4, "Seed for random.")
+
+func main() {
+        flag.Parse()
+
+        var settings TestSettings
+        settings.Seed = *seed
+
+        configdata, err := ioutil.ReadFile(*config)

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

I don't think we need that much configurability. A few named configurations is
likely enough IMHO.

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

We have 6 configs so far, it is likely we'll end up getting quite a few more.

If the configs are all inside this tool, then we need support for things like
"listing the configs" (where as we can just list a directory).

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

Why?
- layout tests
- compile step
- small test

I don't see how this could scale more than 10 or so.
Fine for now.

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

These tests are designed to provide a set of different configurations which can
uncover different performance problems. They aren't designed to exactly emulate
any of the real world structures.

They should cover;

 * Depth of the directory structure (shallow or deep with different branching
factors)
 * File distribution (large number in one directory, large number over many
directories)
 * The mixture of files and directories (just directories, just files, mixture)
 * Different file content types (small files, large files, mixture) etc.

These tests helped me understand how the walking was performing and where the
bottlenecks were coming from.

We probably *also* want to have tests which emulate the real world structures
you pointed out.

+        if err != nil {
+                log.Fatal(err)
+        }
+
+        if err := json.Unmarshal(configdata, &settings); err != nil {
+                log.Fatal(err)
+        }
+
+        fmt.Printf("%s: %s\n", settings.Name, settings.Comment)
+        fmt.Println("===============================================")
+        fmt.Println()
+
+        if len(*outdir) == 0 {
+                log.Fatal("No output directory supplied.")
+        }
+
+        if _, err := os.Stat(*outdir); !os.IsNotExist(err) {
+                if !*remove {
+                        log.Fatal("directory exists!")
+                } else {
+                        if err := os.RemoveAll(*outdir); err != nil {

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

Remove the remove support. Caller can handle that.

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

Done.

+                                log.Fatal(err)
+                        }
+                }
+        }
+
+        if *seed != 4 && settings.Seed != *seed {

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

I don't see why we put settings.Seed. There's no need for this knob.

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

A config which wants to produce identical output needs to set the seed.

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

Yes but why make it configurable and why not make it default to 0?

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

Seed of 0 is special cased as "no-seed" in most systems, thus a default of zero
isn't a great idea.

+                log.Fatal("Seed supplied by test config.")
+        }
+
+        r := rand.New(rand.NewSource(settings.Seed))
+
+        // Create the root directory
+        if err := os.MkdirAll(*outdir, 0755); err != nil {

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

0700

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

Why 0700 rather than 0755?

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

Because there's no point in other users to read this.

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

Done.

+                log.Fatal(err)
+        }
+
+        GenerateTree(r, *outdir, &settings.Tree)

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

something like:
return settings.GenerateTree(*outdir, *seed)

+}