diff --git a/android/config.go b/android/config.go
index b481cac..04c9129 100644
--- a/android/config.go
+++ b/android/config.go
@@ -14,6 +14,9 @@
 
 package android
 
+// This is the primary location to write and read all configuration values and
+// product variables necessary for soong_build's operation.
+
 import (
 	"encoding/json"
 	"fmt"
@@ -32,20 +35,31 @@
 	"android/soong/android/soongconfig"
 )
 
+// Bool re-exports proptools.Bool for the android package.
 var Bool = proptools.Bool
+
+// String re-exports proptools.String for the android package.
 var String = proptools.String
+
+// StringDefault re-exports proptools.StringDefault for the android package.
 var StringDefault = proptools.StringDefault
 
+// FutureApiLevelInt is a placeholder constant for unreleased API levels.
 const FutureApiLevelInt = 10000
 
+// FutureApiLevel represents unreleased API levels.
 var FutureApiLevel = ApiLevel{
 	value:     "current",
 	number:    FutureApiLevelInt,
 	isPreview: true,
 }
 
-// The configuration file name
+// configFileName is the name the file containing FileConfigurableOptions from
+// soong_ui for the soong_build primary builder.
 const configFileName = "soong.config"
+
+// productVariablesFileName contain the product configuration variables from soong_ui for the
+// soong_build primary builder and Kati.
 const productVariablesFileName = "soong.variables"
 
 // A FileConfigurableOptions contains options which can be configured by the
@@ -56,6 +70,8 @@
 	Host_bionic_arm64 *bool `json:",omitempty"`
 }
 
+// SetDefaultConfig resets the receiving FileConfigurableOptions to default
+// values.
 func (f *FileConfigurableOptions) SetDefaultConfig() {
 	*f = FileConfigurableOptions{}
 }
@@ -65,29 +81,38 @@
 	*config
 }
 
+// BuildDir returns the build output directory for the configuration.
 func (c Config) BuildDir() string {
 	return c.buildDir
 }
 
-// A DeviceConfig object represents the configuration for a particular device being built.  For
-// now there will only be one of these, but in the future there may be multiple devices being
-// built
+// A DeviceConfig object represents the configuration for a particular device
+// being built. For now there will only be one of these, but in the future there
+// may be multiple devices being built.
 type DeviceConfig struct {
 	*deviceConfig
 }
 
+// VendorConfig represents the configuration for vendor-specific behavior.
 type VendorConfig soongconfig.SoongConfig
 
+// Definition of general build configuration for soong_build. Some of these
+// configuration values are generated from soong_ui for soong_build,
+// communicated over JSON files like soong.config or soong.variables.
 type config struct {
+	// Options configurable with soong.confg
 	FileConfigurableOptions
+
+	// Options configurable with soong.variables
 	productVariables productVariables
 
 	// Only available on configs created by TestConfig
 	TestProductVariables *productVariables
 
+	// A specialized context object for Bazel/Soong mixed builds and migration
+	// purposes.
 	BazelContext BazelContext
 
-	PrimaryBuilder           string
 	ConfigFileName           string
 	ProductVariablesFileName string
 
@@ -97,8 +122,8 @@
 	AndroidCommonTarget      Target // the Target for common modules for the Android device
 	AndroidFirstDeviceTarget Target // the first Target for modules for the Android device
 
-	// multilibConflicts for an ArchType is true if there is earlier configured device architecture with the same
-	// multilib value.
+	// multilibConflicts for an ArchType is true if there is earlier configured
+	// device architecture with the same multilib value.
 	multilibConflicts map[ArchType]bool
 
 	deviceConfig *deviceConfig
@@ -128,6 +153,8 @@
 	// in tests when a path doesn't exist.
 	testAllowNonExistentPaths bool
 
+	// The list of files that when changed, must invalidate soong_build to
+	// regenerate build.ninja.
 	ninjaFileDepsSet sync.Map
 
 	OncePer
@@ -151,7 +178,8 @@
 	return loadFromConfigFile(&config.productVariables, absolutePath(config.ProductVariablesFileName))
 }
 
-// loads configuration options from a JSON file in the cwd.
+// loadFromConfigFile loads and decodes configuration options from a JSON file
+// in the current working directory.
 func loadFromConfigFile(configurable jsonConfigurable, filename string) error {
 	// Try to open the file
 	configFileReader, err := os.Open(filename)
@@ -191,7 +219,7 @@
 
 	f, err := ioutil.TempFile(filepath.Dir(filename), "config")
 	if err != nil {
-		return fmt.Errorf("cannot create empty config file %s: %s\n", filename, err.Error())
+		return fmt.Errorf("cannot create empty config file %s: %s", filename, err.Error())
 	}
 	defer os.Remove(f.Name())
 	defer f.Close()
@@ -223,7 +251,7 @@
 	}
 }
 
-// TestConfig returns a Config object suitable for using for tests
+// TestConfig returns a Config object for testing.
 func TestConfig(buildDir string, env map[string]string, bp string, fs map[string][]byte) Config {
 	envCopy := make(map[string]string)
 	for k, v := range env {
@@ -269,6 +297,9 @@
 	return Config{config}
 }
 
+// TestArchConfigNativeBridge returns a Config object suitable for using
+// for tests that need to run the arch mutator for native bridge supported
+// archs.
 func TestArchConfigNativeBridge(buildDir string, env map[string]string, bp string, fs map[string][]byte) Config {
 	testConfig := TestArchConfig(buildDir, env, bp, fs)
 	config := testConfig.config
@@ -283,6 +314,8 @@
 	return testConfig
 }
 
+// TestArchConfigFuchsia returns a Config object suitable for using for
+// tests that need to run the arch mutator for the Fuchsia arch.
 func TestArchConfigFuchsia(buildDir string, env map[string]string, bp string, fs map[string][]byte) Config {
 	testConfig := TestConfig(buildDir, env, bp, fs)
 	config := testConfig.config
@@ -299,7 +332,8 @@
 	return testConfig
 }
 
-// TestConfig returns a Config object suitable for using for tests that need to run the arch mutator
+// TestArchConfig returns a Config object suitable for using for tests that
+// need to run the arch mutator.
 func TestArchConfig(buildDir string, env map[string]string, bp string, fs map[string][]byte) Config {
 	testConfig := TestConfig(buildDir, env, bp, fs)
 	config := testConfig.config
@@ -331,10 +365,10 @@
 	return testConfig
 }
 
-// Returns a config object which is "reset" for another bootstrap run.
-// Only per-run data is reset. Data which needs to persist across multiple
-// runs in the same program execution is carried over (such as Bazel context
-// or environment deps).
+// ConfigForAdditionalRun is a config object which is "reset" for another
+// bootstrap run. Only per-run data is reset. Data which needs to persist across
+// multiple runs in the same program execution is carried over (such as Bazel
+// context or environment deps).
 func ConfigForAdditionalRun(c Config) (Config, error) {
 	newConfig, err := NewConfig(c.srcDir, c.buildDir, c.moduleListFile)
 	if err != nil {
@@ -345,10 +379,10 @@
 	return newConfig, nil
 }
 
-// New creates a new Config object.  The srcDir argument specifies the path to
-// the root source directory. It also loads the config file, if found.
+// NewConfig creates a new Config object. The srcDir argument specifies the path
+// to the root source directory. It also loads the config file, if found.
 func NewConfig(srcDir, buildDir string, moduleListFile string) (Config, error) {
-	// Make a config with default options
+	// Make a config with default options.
 	config := &config{
 		ConfigFileName:           filepath.Join(buildDir, configFileName),
 		ProductVariablesFileName: filepath.Join(buildDir, productVariablesFileName),
@@ -394,6 +428,8 @@
 		config.katiEnabled = true
 	}
 
+	// Sets up the map of target OSes to the finer grained compilation targets
+	// that are configured from the product variables.
 	targets, err := decodeTargetProductVariables(config)
 	if err != nil {
 		return Config{}, err
@@ -427,9 +463,14 @@
 		multilib[target.Arch.ArchType.Multilib] = true
 	}
 
+	// Map of OS to compilation targets.
 	config.Targets = targets
+
+	// Compilation targets for host tools.
 	config.BuildOSTarget = config.Targets[BuildOs][0]
 	config.BuildOSCommonTarget = getCommonTargets(config.Targets[BuildOs])[0]
+
+	// Compilation targets for Android.
 	if len(config.Targets[Android]) > 0 {
 		config.AndroidCommonTarget = getCommonTargets(config.Targets[Android])[0]
 		config.AndroidFirstDeviceTarget = firstTarget(config.Targets[Android], "lib64", "lib32")[0]
@@ -444,13 +485,9 @@
 			Bool(config.productVariables.ClangCoverage))
 
 	config.BazelContext, err = NewBazelContext(config)
-	if err != nil {
-		return Config{}, err
-	}
-	return Config{config}, nil
-}
 
-var TestConfigOsFs = map[string][]byte{}
+	return Config{config}, err
+}
 
 // mockFileSystem replaces all reads with accesses to the provided map of
 // filenames to contents stored as a byte slice.
@@ -486,12 +523,15 @@
 	return c.stopBefore
 }
 
+// SetStopBefore configures soong_build to exit earlier at a specific point.
 func (c *config) SetStopBefore(stopBefore bootstrap.StopBefore) {
 	c.stopBefore = stopBefore
 }
 
 var _ bootstrap.ConfigStopBefore = (*config)(nil)
 
+// BlueprintToolLocation returns the directory containing build system tools
+// from Blueprint, like soong_zip and merge_zips.
 func (c *config) BlueprintToolLocation() string {
 	return filepath.Join(c.buildDir, "host", c.PrebuiltOS(), "bin")
 }
@@ -534,7 +574,7 @@
 			"for the full list of allowed host tools on your system.", name))
 }
 
-// PrebuiltOS returns the name of the host OS used in prebuilts directories
+// PrebuiltOS returns the name of the host OS used in prebuilts directories.
 func (c *config) PrebuiltOS() string {
 	switch runtime.GOOS {
 	case "linux":
@@ -551,10 +591,14 @@
 	return fmt.Sprintf("%s/prebuilts/go/%s", c.srcDir, c.PrebuiltOS())
 }
 
+// PrebuiltBuildTool returns the path to a tool in the prebuilts directory containing
+// checked-in tools, like Kati, Ninja or Toybox, for the current host OS.
 func (c *config) PrebuiltBuildTool(ctx PathContext, tool string) Path {
 	return PathForSource(ctx, "prebuilts/build-tools", c.PrebuiltOS(), "bin", tool)
 }
 
+// CpPreserveSymlinksFlags returns the host-specific flag for the cp(1) command
+// to preserve symlinks.
 func (c *config) CpPreserveSymlinksFlags() string {
 	switch runtime.GOOS {
 	case "darwin":
@@ -602,6 +646,8 @@
 	return value == "0" || value == "n" || value == "no" || value == "off" || value == "false"
 }
 
+// EnvDeps returns the environment variables this build depends on. The first
+// call to this function blocks future reads from the environment.
 func (c *config) EnvDeps() map[string]string {
 	c.envLock.Lock()
 	defer c.envLock.Unlock()
@@ -617,11 +663,18 @@
 	return String(c.productVariables.BuildId)
 }
 
+// BuildNumberFile returns the path to a text file containing metadata
+// representing the current build's number.
+//
+// Rules that want to reference the build number should read from this file
+// without depending on it. They will run whenever their other dependencies
+// require them to run and get the current build number. This ensures they don't
+// rebuild on every incremental build when the build number changes.
 func (c *config) BuildNumberFile(ctx PathContext) Path {
 	return PathForOutput(ctx, String(c.productVariables.BuildNumberFile))
 }
 
-// DeviceName returns the name of the current device target
+// DeviceName returns the name of the current device target.
 // TODO: take an AndroidModuleContext to select the device name for multi-device builds
 func (c *config) DeviceName() string {
 	return *c.productVariables.DeviceName
@@ -693,19 +746,20 @@
 	return append(levels, c.PreviewApiLevels()...)
 }
 
+// DefaultAppTargetSdk returns the API level that platform apps are targeting.
+// This converts a codename to the exact ApiLevel it represents.
 func (c *config) DefaultAppTargetSdk(ctx EarlyModuleContext) ApiLevel {
 	if Bool(c.productVariables.Platform_sdk_final) {
 		return c.PlatformSdkVersion()
-	} else {
-		codename := c.PlatformSdkCodename()
-		if codename == "" {
-			return NoneApiLevel
-		}
-		if codename == "REL" {
-			panic("Platform_sdk_codename should not be REL when Platform_sdk_final is true")
-		}
-		return ApiLevelOrPanic(ctx, codename)
 	}
+	codename := c.PlatformSdkCodename()
+	if codename == "" {
+		return NoneApiLevel
+	}
+	if codename == "REL" {
+		panic("Platform_sdk_codename should not be REL when Platform_sdk_final is true")
+	}
+	return ApiLevelOrPanic(ctx, codename)
 }
 
 func (c *config) AppsDefaultVersionName() string {
@@ -737,19 +791,17 @@
 	defaultCert := String(c.productVariables.DefaultAppCertificate)
 	if defaultCert != "" {
 		return PathForSource(ctx, filepath.Dir(defaultCert))
-	} else {
-		return PathForSource(ctx, "build/make/target/product/security")
 	}
+	return PathForSource(ctx, "build/make/target/product/security")
 }
 
 func (c *config) DefaultAppCertificate(ctx PathContext) (pem, key SourcePath) {
 	defaultCert := String(c.productVariables.DefaultAppCertificate)
 	if defaultCert != "" {
 		return PathForSource(ctx, defaultCert+".x509.pem"), PathForSource(ctx, defaultCert+".pk8")
-	} else {
-		defaultDir := c.DefaultAppCertificateDir(ctx)
-		return defaultDir.Join(ctx, "testkey.x509.pem"), defaultDir.Join(ctx, "testkey.pk8")
 	}
+	defaultDir := c.DefaultAppCertificateDir(ctx)
+	return defaultDir.Join(ctx, "testkey.x509.pem"), defaultDir.Join(ctx, "testkey.pk8")
 }
 
 func (c *config) ApexKeyDir(ctx ModuleContext) SourcePath {
@@ -759,12 +811,14 @@
 		// When defaultCert is unset or is set to the testkeys path, use the APEX keys
 		// that is under the module dir
 		return pathForModuleSrc(ctx)
-	} else {
-		// If not, APEX keys are under the specified directory
-		return PathForSource(ctx, filepath.Dir(defaultCert))
 	}
+	// If not, APEX keys are under the specified directory
+	return PathForSource(ctx, filepath.Dir(defaultCert))
 }
 
+// AllowMissingDependencies configures Blueprint/Soong to not fail when modules
+// are configured to depend on non-existent modules. Note that this does not
+// affect missing input dependencies at the Ninja level.
 func (c *config) AllowMissingDependencies() bool {
 	return Bool(c.productVariables.Allow_missing_dependencies)
 }
@@ -834,9 +888,8 @@
 func (c *config) EnableCFI() bool {
 	if c.productVariables.EnableCFI == nil {
 		return true
-	} else {
-		return *c.productVariables.EnableCFI
 	}
+	return *c.productVariables.EnableCFI
 }
 
 func (c *config) DisableScudo() bool {
@@ -881,11 +934,13 @@
 	return c.IsEnvTrue("RUN_ERROR_PRONE")
 }
 
+// XrefCorpusName returns the Kythe cross-reference corpus name.
 func (c *config) XrefCorpusName() string {
 	return c.Getenv("XREF_CORPUS")
 }
 
-// Returns Compilation Unit encoding to use. Can be 'json' (default), 'proto' or 'all'.
+// XrefCuEncoding returns the compilation unit encoding to use for Kythe code
+// xrefs. Can be 'json' (default), 'proto' or 'all'.
 func (c *config) XrefCuEncoding() string {
 	if enc := c.Getenv("KYTHE_KZIP_ENCODING"); enc != "" {
 		return enc
@@ -920,6 +975,10 @@
 	return Bool(c.productVariables.ArtUseReadBarrier)
 }
 
+// Enforce Runtime Resource Overlays for a module. RROs supersede static RROs,
+// but some modules still depend on it.
+//
+// More info: https://source.android.com/devices/architecture/rros
 func (c *config) EnforceRROForModule(name string) bool {
 	enforceList := c.productVariables.EnforceRROTargets
 	// TODO(b/150820813) Some modules depend on static overlay, remove this after eliminating the dependency.
@@ -966,6 +1025,9 @@
 	return c.productVariables.ModulesLoadedByPrivilegedModules
 }
 
+// DexpreoptGlobalConfigPath returns the path to the dexpreopt.config file in
+// the output directory, if it was created during the product configuration
+// phase by Kati.
 func (c *config) DexpreoptGlobalConfigPath(ctx PathContext) OptionalPath {
 	if c.productVariables.DexpreoptGlobalConfig == nil {
 		return OptionalPathForPath(nil)
@@ -974,6 +1036,12 @@
 		pathForBuildToolDep(ctx, *c.productVariables.DexpreoptGlobalConfig))
 }
 
+// DexpreoptGlobalConfig returns the raw byte contents of the dexpreopt global
+// configuration. Since the configuration file was created by Kati during
+// product configuration (externally of soong_build), it's not tracked, so we
+// also manually add a Ninja file dependency on the configuration file to the
+// rule that creates the main build.ninja file. This ensures that build.ninja is
+// regenerated correctly if dexpreopt.config changes.
 func (c *config) DexpreoptGlobalConfig(ctx PathContext) ([]byte, error) {
 	path := c.DexpreoptGlobalConfigPath(ctx)
 	if !path.Valid() {
@@ -1332,26 +1400,31 @@
 //   - "system_ext:foo"
 //
 type ConfiguredJarList struct {
-	apexes []string // A list of apex components.
-	jars   []string // A list of jar components.
+	// A list of apex components, which can be an apex name,
+	// or special names like "platform" or "system_ext".
+	apexes []string
+
+	// A list of jar module name components.
+	jars []string
 }
 
-// The length of the list.
+// Len returns the length of the list of jars.
 func (l *ConfiguredJarList) Len() int {
 	return len(l.jars)
 }
 
-// Jar component of idx-th pair on the list.
+// Jar returns the idx-th jar component of (apex, jar) pairs.
 func (l *ConfiguredJarList) Jar(idx int) string {
 	return l.jars[idx]
 }
 
-// Apex component of idx-th pair on the list.
+// Apex returns the idx-th apex component of (apex, jar) pairs.
 func (l *ConfiguredJarList) Apex(idx int) string {
 	return l.apexes[idx]
 }
 
-// If the list contains a pair with the given jar.
+// ContainsJar returns true if the (apex, jar) pairs contains a pair with the
+// given jar module name.
 func (l *ConfiguredJarList) ContainsJar(jar string) bool {
 	return InList(jar, l.jars)
 }
@@ -1366,7 +1439,8 @@
 	return false
 }
 
-// Index of the first pair with the given jar on the list, or -1 if none.
+// IndexOfJar returns the first pair with the given jar name on the list, or -1
+// if not found.
 func (l *ConfiguredJarList) IndexOfJar(jar string) int {
 	return IndexList(jar, l.jars)
 }
@@ -1394,7 +1468,7 @@
 	return ConfiguredJarList{apexes, jars}
 }
 
-// Filter out sublist.
+// RemoveList filters out a list of (apex, jar) pairs from the receiving list of pairs.
 func (l *ConfiguredJarList) RemoveList(list ConfiguredJarList) ConfiguredJarList {
 	apexes := make([]string, 0, l.Len())
 	jars := make([]string, 0, l.Len())
@@ -1410,12 +1484,14 @@
 	return ConfiguredJarList{apexes, jars}
 }
 
-// A copy of the list of strings containing jar components.
+// CopyOfJars returns a copy of the list of strings containing jar module name
+// components.
 func (l *ConfiguredJarList) CopyOfJars() []string {
 	return CopyOf(l.jars)
 }
 
-// A copy of the list of strings with colon-separated (apex, jar) pairs.
+// CopyOfApexJarPairs returns a copy of the list of strings with colon-separated
+// (apex, jar) pairs.
 func (l *ConfiguredJarList) CopyOfApexJarPairs() []string {
 	pairs := make([]string, 0, l.Len())
 
@@ -1427,7 +1503,7 @@
 	return pairs
 }
 
-// A list of build paths based on the given directory prefix.
+// BuildPaths returns a list of build paths based on the given directory prefix.
 func (l *ConfiguredJarList) BuildPaths(ctx PathContext, dir OutputPath) WritablePaths {
 	paths := make(WritablePaths, l.Len())
 	for i, jar := range l.jars {
@@ -1436,7 +1512,8 @@
 	return paths
 }
 
-// Called when loading configuration from JSON into a configuration structure.
+// UnmarshalJSON converts JSON configuration from raw bytes into a
+// ConfiguredJarList structure.
 func (l *ConfiguredJarList) UnmarshalJSON(b []byte) error {
 	// Try and unmarshal into a []string each item of which contains a pair
 	// <apex>:<jar>.
@@ -1456,16 +1533,19 @@
 	return nil
 }
 
+// ModuleStem hardcodes the stem of framework-minus-apex to return "framework".
+//
+// TODO(b/139391334): hard coded until we find a good way to query the stem of a
+// module before any other mutators are run.
 func ModuleStem(module string) string {
-	// b/139391334: the stem of framework-minus-apex is framework. This is hard coded here until we
-	// find a good way to query the stem of a module before any other mutators are run.
 	if module == "framework-minus-apex" {
 		return "framework"
 	}
 	return module
 }
 
-// A list of on-device paths.
+// DevicePaths computes the on-device paths for the list of (apex, jar) pairs,
+// based on the operating system.
 func (l *ConfiguredJarList) DevicePaths(cfg Config, ostype OsType) []string {
 	paths := make([]string, l.Len())
 	for i, jar := range l.jars {
@@ -1526,6 +1606,8 @@
 	}
 }
 
+// CreateTestConfiguredJarList is a function to create ConfiguredJarList for
+// tests.
 func CreateTestConfiguredJarList(list []string) ConfiguredJarList {
 	apexes, jars, err := splitListOfPairsIntoPairOfLists(list)
 	if err != nil {
@@ -1535,6 +1617,7 @@
 	return ConfiguredJarList{apexes, jars}
 }
 
+// EmptyConfiguredJarList returns an empty jar list.
 func EmptyConfiguredJarList() ConfiguredJarList {
 	return ConfiguredJarList{}
 }
@@ -1544,8 +1627,7 @@
 func (c *config) BootJars() []string {
 	return c.Once(earlyBootJarsKey, func() interface{} {
 		list := c.productVariables.BootJars.CopyOfJars()
-		list = append(list, c.productVariables.UpdatableBootJars.CopyOfJars()...)
-		return list
+		return append(list, c.productVariables.UpdatableBootJars.CopyOfJars()...)
 	}).([]string)
 }
 
diff --git a/cc/binary.go b/cc/binary.go
index da29412..fbd293e 100644
--- a/cc/binary.go
+++ b/cc/binary.go
@@ -42,6 +42,7 @@
 	// extension (if any) appended
 	Symlinks []string `android:"arch_variant"`
 
+	// override the dynamic linker
 	DynamicLinker string `blueprint:"mutated"`
 
 	// Names of modules to be overridden. Listed modules can only be other binaries
@@ -80,6 +81,7 @@
 // Executables
 //
 
+// binaryDecorator is a decorator containing information for C++ binary modules.
 type binaryDecorator struct {
 	*baseLinker
 	*baseInstaller
@@ -105,11 +107,15 @@
 	// Location of the files that should be copied to dist dir when requested
 	distFiles android.TaggedDistFiles
 
+	// Action command lines to run directly after the binary is installed. For example,
+	// may be used to symlink runtime dependencies (such as bionic) alongside installation.
 	post_install_cmds []string
 }
 
 var _ linker = (*binaryDecorator)(nil)
 
+// linkerProps returns the list of individual properties objects relevant
+// for this binary.
 func (binary *binaryDecorator) linkerProps() []interface{} {
 	return append(binary.baseLinker.linkerProps(),
 		&binary.Properties,
@@ -117,6 +123,10 @@
 
 }
 
+// getStemWithoutSuffix returns the main section of the name to use for the symlink of
+// the main output file of this binary module. This may be derived from the module name
+// or other property overrides.
+// For the full symlink name, the `Suffix` property of a binary module must be appended.
 func (binary *binaryDecorator) getStemWithoutSuffix(ctx BaseModuleContext) string {
 	stem := ctx.baseModuleName()
 	if String(binary.Properties.Stem) != "" {
@@ -126,10 +136,14 @@
 	return stem
 }
 
+// getStem returns the full name to use for the symlink of the main output file of this binary
+// module. This may be derived from the module name and/or other property overrides.
 func (binary *binaryDecorator) getStem(ctx BaseModuleContext) string {
 	return binary.getStemWithoutSuffix(ctx) + String(binary.Properties.Suffix)
 }
 
+// linkerDeps augments and returns the given `deps` to contain dependencies on
+// modules common to most binaries, such as bionic libraries.
 func (binary *binaryDecorator) linkerDeps(ctx DepsContext, deps Deps) Deps {
 	deps = binary.baseLinker.linkerDeps(ctx, deps)
 	if ctx.toolchain().Bionic() {
@@ -155,6 +169,13 @@
 			deps.LateStaticLibs = append(groupLibs, deps.LateStaticLibs...)
 		}
 
+		// Embed the linker into host bionic binaries. This is needed to support host bionic,
+		// as the linux kernel requires that the ELF interpreter referenced by PT_INTERP be
+		// either an absolute path, or relative from CWD. To work around this, we extract
+		// the load sections from the runtime linker ELF binary and embed them into each host
+		// bionic binary, omitting the PT_INTERP declaration. The kernel will treat it as a static
+		// binary, and then we use a special entry point to fix up the arguments passed by
+		// the kernel before jumping to the embedded linker.
 		if ctx.Os() == android.LinuxBionic && !binary.static() {
 			deps.DynamicLinker = "linker"
 			deps.LinkerFlagsFile = "host_bionic_linker_flags"
@@ -170,9 +191,13 @@
 }
 
 func (binary *binaryDecorator) isDependencyRoot() bool {
+	// Binaries are always the dependency root.
 	return true
 }
 
+// NewBinary builds and returns a new Module corresponding to a C++ binary.
+// Individual module implementations which comprise a C++ binary should call this function,
+// set some fields on the result, and then call the Init function.
 func NewBinary(hod android.HostOrDeviceSupported) (*Module, *binaryDecorator) {
 	module := newModule(hod, android.MultilibFirst)
 	binary := &binaryDecorator{
@@ -190,11 +215,15 @@
 	return module, binary
 }
 
+// linkerInit initializes dynamic properties of the linker (such as runpath) based
+// on properties of this binary.
 func (binary *binaryDecorator) linkerInit(ctx BaseModuleContext) {
 	binary.baseLinker.linkerInit(ctx)
 
 	if !ctx.toolchain().Bionic() {
 		if ctx.Os() == android.Linux {
+			// Unless explicitly specified otherwise, host static binaries are built with -static
+			// if HostStaticBinaries is true for the product configuration.
 			if binary.Properties.Static_executable == nil && ctx.Config().HostStaticBinaries() {
 				binary.Properties.Static_executable = BoolPtr(true)
 			}
@@ -217,9 +246,13 @@
 	return true
 }
 
+// linkerFlags returns a Flags object containing linker flags that are defined
+// by this binary, or that are implied by attributes of this binary. These flags are
+// combined with the given flags.
 func (binary *binaryDecorator) linkerFlags(ctx ModuleContext, flags Flags) Flags {
 	flags = binary.baseLinker.linkerFlags(ctx, flags)
 
+	// Passing -pie to clang for Windows binaries causes a warning that -pie is unused.
 	if ctx.Host() && !ctx.Windows() && !binary.static() {
 		if !ctx.Config().IsEnvTrue("DISABLE_HOST_PIE") {
 			flags.Global.LdFlags = append(flags.Global.LdFlags, "-pie")
@@ -248,7 +281,7 @@
 				"-Bstatic",
 				"-Wl,--gc-sections",
 			)
-		} else {
+		} else { // not static
 			if flags.DynamicLinker == "" {
 				if binary.Properties.DynamicLinker != "" {
 					flags.DynamicLinker = binary.Properties.DynamicLinker
@@ -288,7 +321,7 @@
 				"-Wl,-z,nocopyreloc",
 			)
 		}
-	} else {
+	} else { // not bionic
 		if binary.static() {
 			flags.Global.LdFlags = append(flags.Global.LdFlags, "-static")
 		}
@@ -300,6 +333,9 @@
 	return flags
 }
 
+// link registers actions to link this binary, and sets various fields
+// on this binary to reflect information that should be exported up the build
+// tree (for example, exported flags and include paths).
 func (binary *binaryDecorator) link(ctx ModuleContext,
 	flags Flags, deps PathDeps, objs Objects) android.Path {
 
@@ -309,6 +345,7 @@
 
 	var linkerDeps android.Paths
 
+	// Add flags from linker flags file.
 	if deps.LinkerFlagsFile.Valid() {
 		flags.Local.LdFlags = append(flags.Local.LdFlags, "$$(cat "+deps.LinkerFlagsFile.String()+")")
 		linkerDeps = append(linkerDeps, deps.LinkerFlagsFile.Path())
@@ -342,12 +379,15 @@
 
 	outputFile = maybeInjectBoringSSLHash(ctx, outputFile, binary.Properties.Inject_bssl_hash, fileName)
 
+	// If use_version_lib is true, make an android::build::GetBuildNumber() function available.
 	if Bool(binary.baseLinker.Properties.Use_version_lib) {
 		if ctx.Host() {
 			versionedOutputFile := outputFile
 			outputFile = android.PathForModuleOut(ctx, "unversioned", fileName)
 			binary.injectVersionSymbol(ctx, outputFile, versionedOutputFile)
 		} else {
+			// When dist'ing a library or binary that has use_version_lib set, always
+			// distribute the stamped version, even for the device.
 			versionedOutputFile := android.PathForModuleOut(ctx, "versioned", fileName)
 			binary.distFiles = android.MakeDefaultDistFiles(versionedOutputFile)
 
@@ -361,6 +401,7 @@
 		}
 	}
 
+	// Handle host bionic linker symbols.
 	if ctx.Os() == android.LinuxBionic && !binary.static() {
 		injectedOutputFile := outputFile
 		outputFile = android.PathForModuleOut(ctx, "prelinker", fileName)
@@ -386,6 +427,7 @@
 	linkerDeps = append(linkerDeps, objs.tidyFiles...)
 	linkerDeps = append(linkerDeps, flags.LdFlagsDeps...)
 
+	// Register link action.
 	TransformObjToDynamicBinary(ctx, objs.objFiles, sharedLibs, deps.StaticLibs,
 		deps.LateStaticLibs, deps.WholeStaticLibs, linkerDeps, deps.CrtBegin, deps.CrtEnd, true,
 		builderFlags, outputFile, nil)
@@ -406,6 +448,7 @@
 			ctx.PropertyErrorf("symlink_preferred_arch", "must also specify suffix")
 		}
 		if ctx.TargetPrimary() {
+			// Install a symlink to the preferred architecture
 			symlinkName := binary.getStemWithoutSuffix(ctx)
 			binary.symlinks = append(binary.symlinks, symlinkName)
 			binary.preferredArchSymlink = symlinkName
diff --git a/cc/cc.go b/cc/cc.go
index 3a9349b..6deb1b4 100644
--- a/cc/cc.go
+++ b/cc/cc.go
@@ -1857,6 +1857,11 @@
 		return
 	}
 
+	// sysprop_library has to support both C++ and Java. So sysprop_library internally creates one
+	// C++ implementation library and one Java implementation library. When a module links against
+	// sysprop_library, the C++ implementation library has to be linked. syspropImplLibraries is a
+	// map from sysprop_library to implementation library; it will be used in whole_static_libs,
+	// static_libs, and shared_libs.
 	syspropImplLibraries := syspropImplLibraries(actx.Config())
 	vendorSnapshotStaticLibs := vendorSnapshotStaticLibs(actx.Config())
 
diff --git a/cc/library.go b/cc/library.go
index 2127c08..7ae75f2 100644
--- a/cc/library.go
+++ b/cc/library.go
@@ -1202,15 +1202,21 @@
 		}
 	}
 
+	// If the library is sysprop_library, expose either public or internal header selectively.
 	if library.baseCompiler.hasSrcExt(".sysprop") {
 		dir := android.PathForModuleGen(ctx, "sysprop", "include")
 		if library.Properties.Sysprop.Platform != nil {
-			isProduct := ctx.ProductSpecific() && !ctx.useVndk()
-			isVendor := ctx.useVndk()
+			isClientProduct := ctx.ProductSpecific() && !ctx.useVndk()
+			isClientVendor := ctx.useVndk()
 			isOwnerPlatform := Bool(library.Properties.Sysprop.Platform)
 
+			// If the owner is different from the user, expose public header. That is,
+			// 1) if the user is product (as owner can only be platform / vendor)
+			// 2) if one is platform and the other is vendor
+			// Exceptions are ramdisk and recovery. They are not enforced at all. So
+			// they always use internal header.
 			if !ctx.inRamdisk() && !ctx.inVendorRamdisk() && !ctx.inRecovery() &&
-				(isProduct || (isOwnerPlatform == isVendor)) {
+				(isClientProduct || (isOwnerPlatform == isClientVendor)) {
 				dir = android.PathForModuleGen(ctx, "sysprop/public", "include")
 			}
 		}
diff --git a/cc/linker.go b/cc/linker.go
index cbf8898..9d4a643 100644
--- a/cc/linker.go
+++ b/cc/linker.go
@@ -211,6 +211,7 @@
 	linker.Properties.Ldflags = append(linker.Properties.Ldflags, flags...)
 }
 
+// linkerInit initializes dynamic properties of the linker (such as runpath).
 func (linker *baseLinker) linkerInit(ctx BaseModuleContext) {
 	if ctx.toolchain().Is64Bit() {
 		linker.dynamicProperties.RunPaths = append(linker.dynamicProperties.RunPaths, "../lib64", "lib64")
diff --git a/cc/sysprop.go b/cc/sysprop.go
index 6cac7fb..f578b50 100644
--- a/cc/sysprop.go
+++ b/cc/sysprop.go
@@ -14,6 +14,25 @@
 
 package cc
 
+// This file contains a map to redirect dependencies towards sysprop_library.
+// As sysprop_library has to support both Java and C++, sysprop_library internally
+// generates cc_library and java_library. For example, the following sysprop_library
+//
+//     sysprop_library {
+//         name: "foo",
+//     }
+//
+// will internally generate with prefix "lib"
+//
+//     cc_library {
+//         name: "libfoo",
+//     }
+//
+// When a cc module links against "foo", build system will redirect the
+// dependency to "libfoo". To do that, SyspropMutator gathers all sysprop_library,
+// records their cc implementation library names to a map. The map will be used in
+// cc.Module.DepsMutator.
+
 import (
 	"sync"
 
@@ -22,7 +41,7 @@
 
 type syspropLibraryInterface interface {
 	BaseModuleName() string
-	CcModuleName() string
+	CcImplementationModuleName() string
 }
 
 var (
@@ -43,6 +62,8 @@
 		syspropImplLibrariesLock.Lock()
 		defer syspropImplLibrariesLock.Unlock()
 
-		syspropImplLibraries[m.BaseModuleName()] = m.CcModuleName()
+		// BaseModuleName is the name of sysprop_library
+		// CcImplementationModuleName is the name of cc_library generated by sysprop_library
+		syspropImplLibraries[m.BaseModuleName()] = m.CcImplementationModuleName()
 	}
 }
diff --git a/java/sysprop.go b/java/sysprop.go
index 1a70499..e41aef6 100644
--- a/java/sysprop.go
+++ b/java/sysprop.go
@@ -14,6 +14,10 @@
 
 package java
 
+// This file contains a map to redirect dependencies towards sysprop_library. If a sysprop_library
+// is owned by Platform, and the client module links against system API, the public stub of the
+// sysprop_library should be used. The map will contain public stub names of sysprop_libraries.
+
 import (
 	"sync"
 
diff --git a/sysprop/sysprop_library.go b/sysprop/sysprop_library.go
index 1740ba8..828d1cf 100644
--- a/sysprop/sysprop_library.go
+++ b/sysprop/sysprop_library.go
@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// sysprop package defines a module named sysprop_library that can implement sysprop as API
+// See https://source.android.com/devices/architecture/sysprops-apis for details
 package sysprop
 
 import (
@@ -71,6 +73,8 @@
 	})
 }
 
+// syspropJavaGenRule module generates srcjar containing generated java APIs.
+// It also depends on check api rule, so api check has to pass to use sysprop_library.
 func (g *syspropJavaGenRule) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	var checkApiFileTimeStamp android.WritablePath
 
@@ -170,6 +174,7 @@
 	syspropLibrariesLock sync.Mutex
 )
 
+// List of sysprop_library used by property_contexts to perform type check.
 func syspropLibraries(config android.Config) *[]string {
 	return config.Once(syspropLibrariesKey, func() interface{} {
 		return &[]string{}
@@ -192,7 +197,7 @@
 	return m.properties.Property_owner
 }
 
-func (m *syspropLibrary) CcModuleName() string {
+func (m *syspropLibrary) CcImplementationModuleName() string {
 	return "lib" + m.BaseModuleName()
 }
 
@@ -223,6 +228,8 @@
 	return m.currentApiFile
 }
 
+// GenerateAndroidBuildActions of sysprop_library handles API dump and API check.
+// generated java_library will depend on these API files.
 func (m *syspropLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	baseModuleName := m.BaseModuleName()
 
@@ -251,7 +258,8 @@
 	// check API rule
 	rule = android.NewRuleBuilder()
 
-	// 1. current.txt <-> api_dump.txt
+	// 1. compares current.txt to api-dump.txt
+	// current.txt should be identical to api-dump.txt.
 	msg := fmt.Sprintf(`\n******************************\n`+
 		`API of sysprop_library %s doesn't match with current.txt\n`+
 		`Please update current.txt by:\n`+
@@ -267,7 +275,8 @@
 		Flag(`"` + msg + `"`).
 		Text("; exit 38) )")
 
-	// 2. current.txt <-> latest.txt
+	// 2. compares current.txt to latest.txt (frozen API)
+	// current.txt should be compatible with latest.txt
 	msg = fmt.Sprintf(`\n******************************\n`+
 		`API of sysprop_library %s doesn't match with latest version\n`+
 		`Please fix the breakage and rebuild.\n`+
@@ -410,14 +419,16 @@
 	installedInVendorOrOdm := ctx.SocSpecific() || ctx.DeviceSpecific()
 	installedInProduct := ctx.ProductSpecific()
 	isOwnerPlatform := false
-	var stub string
+	var javaSyspropStub string
 
+	// javaSyspropStub contains stub libraries used by generated APIs, instead of framework stub.
+	// This is to make sysprop_library link against core_current.
 	if installedInVendorOrOdm {
-		stub = "sysprop-library-stub-vendor"
+		javaSyspropStub = "sysprop-library-stub-vendor"
 	} else if installedInProduct {
-		stub = "sysprop-library-stub-product"
+		javaSyspropStub = "sysprop-library-stub-product"
 	} else {
-		stub = "sysprop-library-stub-platform"
+		javaSyspropStub = "sysprop-library-stub-platform"
 	}
 
 	switch m.Owner() {
@@ -441,8 +452,10 @@
 			"Unknown value %s: must be one of Platform, Vendor or Odm", m.Owner())
 	}
 
+	// Generate a C++ implementation library.
+	// cc_library can receive *.sysprop files as their srcs, generating sources itself.
 	ccProps := ccLibraryProperties{}
-	ccProps.Name = proptools.StringPtr(m.CcModuleName())
+	ccProps.Name = proptools.StringPtr(m.CcImplementationModuleName())
 	ccProps.Srcs = m.properties.Srcs
 	ccProps.Soc_specific = proptools.BoolPtr(ctx.SocSpecific())
 	ccProps.Device_specific = proptools.BoolPtr(ctx.DeviceSpecific())
@@ -463,15 +476,17 @@
 
 	// We need to only use public version, if the partition where sysprop_library will be installed
 	// is different from owner.
-
 	if ctx.ProductSpecific() {
-		// Currently product partition can't own any sysprop_library.
+		// Currently product partition can't own any sysprop_library. So product always uses public.
 		scope = "public"
 	} else if isOwnerPlatform && installedInVendorOrOdm {
 		// Vendor or Odm should use public version of Platform's sysprop_library.
 		scope = "public"
 	}
 
+	// Generate a Java implementation library.
+	// Contrast to C++, syspropJavaGenRule module will generate srcjar and the srcjar will be fed
+	// to Java implementation library.
 	ctx.CreateModule(syspropJavaGenFactory, &syspropGenProperties{
 		Srcs:  m.properties.Srcs,
 		Scope: scope,
@@ -486,7 +501,7 @@
 		Product_specific: proptools.BoolPtr(ctx.ProductSpecific()),
 		Installable:      m.properties.Installable,
 		Sdk_version:      proptools.StringPtr("core_current"),
-		Libs:             []string{stub},
+		Libs:             []string{javaSyspropStub},
 	})
 
 	// if platform sysprop_library is installed in /system or /system-ext, we regard it as an API
@@ -505,11 +520,13 @@
 			Srcs:        []string{":" + m.javaGenPublicStubName()},
 			Installable: proptools.BoolPtr(false),
 			Sdk_version: proptools.StringPtr("core_current"),
-			Libs:        []string{stub},
+			Libs:        []string{javaSyspropStub},
 			Stem:        proptools.StringPtr(m.BaseModuleName()),
 		})
 	}
 
+	// syspropLibraries will be used by property_contexts to check types.
+	// Record absolute paths of sysprop_library to prevent soong_namespace problem.
 	if m.ExportedToMake() {
 		syspropLibrariesLock.Lock()
 		defer syspropLibrariesLock.Unlock()
@@ -519,6 +536,8 @@
 	}
 }
 
+// syspropDepsMutator adds dependencies from java implementation library to sysprop library.
+// java implementation library then depends on check API rule of sysprop library.
 func syspropDepsMutator(ctx android.BottomUpMutatorContext) {
 	if m, ok := ctx.Module().(*syspropLibrary); ok {
 		ctx.AddReverseDependency(m, nil, m.javaGenModuleName())
diff --git a/ui/build/build.go b/ui/build/build.go
index e9196a9..cfd0b83 100644
--- a/ui/build/build.go
+++ b/ui/build/build.go
@@ -183,7 +183,7 @@
 		help(ctx, config, what)
 		return
 	} else if inList("clean", config.Arguments()) || inList("clobber", config.Arguments()) {
-		clean(ctx, config, what)
+		clean(ctx, config)
 		return
 	}
 
@@ -218,12 +218,12 @@
 
 	if inList("installclean", config.Arguments()) ||
 		inList("install-clean", config.Arguments()) {
-		installClean(ctx, config, what)
+		installClean(ctx, config)
 		ctx.Println("Deleted images and staging directories.")
 		return
 	} else if inList("dataclean", config.Arguments()) ||
 		inList("data-clean", config.Arguments()) {
-		dataClean(ctx, config, what)
+		dataClean(ctx, config)
 		ctx.Println("Deleted data files.")
 		return
 	}
diff --git a/ui/build/cleanbuild.go b/ui/build/cleanbuild.go
index 03e884a..06f6c63 100644
--- a/ui/build/cleanbuild.go
+++ b/ui/build/cleanbuild.go
@@ -26,8 +26,11 @@
 	"android/soong/ui/metrics"
 )
 
+// Given a series of glob patterns, remove matching files and directories from the filesystem.
+// For example, "malware*" would remove all files and directories in the current directory that begin with "malware".
 func removeGlobs(ctx Context, globs ...string) {
 	for _, glob := range globs {
+		// Find files and directories that match this glob pattern.
 		files, err := filepath.Glob(glob)
 		if err != nil {
 			// Only possible error is ErrBadPattern
@@ -45,13 +48,15 @@
 
 // Remove everything under the out directory. Don't remove the out directory
 // itself in case it's a symlink.
-func clean(ctx Context, config Config, what int) {
+func clean(ctx Context, config Config) {
 	removeGlobs(ctx, filepath.Join(config.OutDir(), "*"))
 	ctx.Println("Entire build directory removed.")
 }
 
-func dataClean(ctx Context, config Config, what int) {
+// Remove everything in the data directory.
+func dataClean(ctx Context, config Config) {
 	removeGlobs(ctx, filepath.Join(config.ProductOut(), "data", "*"))
+	ctx.Println("Entire data directory removed.")
 }
 
 // installClean deletes all of the installed files -- the intent is to remove
@@ -61,8 +66,8 @@
 //
 // This is faster than a full clean, since we're not deleting the
 // intermediates.  Instead of recompiling, we can just copy the results.
-func installClean(ctx Context, config Config, what int) {
-	dataClean(ctx, config, what)
+func installClean(ctx Context, config Config) {
+	dataClean(ctx, config)
 
 	if hostCrossOutPath := config.hostCrossOut(); hostCrossOutPath != "" {
 		hostCrossOut := func(path string) string {
@@ -145,85 +150,95 @@
 // Since products and build variants (unfortunately) shared the same
 // PRODUCT_OUT staging directory, things can get out of sync if different
 // build configurations are built in the same tree. This function will
-// notice when the configuration has changed and call installclean to
+// notice when the configuration has changed and call installClean to
 // remove the files necessary to keep things consistent.
 func installCleanIfNecessary(ctx Context, config Config) {
 	configFile := config.DevicePreviousProductConfig()
 	prefix := "PREVIOUS_BUILD_CONFIG := "
 	suffix := "\n"
-	currentProduct := prefix + config.TargetProduct() + "-" + config.TargetBuildVariant() + suffix
+	currentConfig := prefix + config.TargetProduct() + "-" + config.TargetBuildVariant() + suffix
 
 	ensureDirectoriesExist(ctx, filepath.Dir(configFile))
 
 	writeConfig := func() {
-		err := ioutil.WriteFile(configFile, []byte(currentProduct), 0666)
+		err := ioutil.WriteFile(configFile, []byte(currentConfig), 0666) // a+rw
 		if err != nil {
 			ctx.Fatalln("Failed to write product config:", err)
 		}
 	}
 
-	prev, err := ioutil.ReadFile(configFile)
+	previousConfigBytes, err := ioutil.ReadFile(configFile)
 	if err != nil {
 		if os.IsNotExist(err) {
+			// Just write the new config file, no old config file to worry about.
 			writeConfig()
 			return
 		} else {
 			ctx.Fatalln("Failed to read previous product config:", err)
 		}
-	} else if string(prev) == currentProduct {
+	}
+
+	previousConfig := string(previousConfigBytes)
+	if previousConfig == currentConfig {
+		// Same config as before - nothing to clean.
 		return
 	}
 
-	if disable, _ := config.Environment().Get("DISABLE_AUTO_INSTALLCLEAN"); disable == "true" {
-		ctx.Println("DISABLE_AUTO_INSTALLCLEAN is set; skipping auto-clean. Your tree may be in an inconsistent state.")
+	if config.Environment().IsEnvTrue("DISABLE_AUTO_INSTALLCLEAN") {
+		ctx.Println("DISABLE_AUTO_INSTALLCLEAN is set and true; skipping auto-clean. Your tree may be in an inconsistent state.")
 		return
 	}
 
 	ctx.BeginTrace(metrics.PrimaryNinja, "installclean")
 	defer ctx.EndTrace()
 
-	prevConfig := strings.TrimPrefix(strings.TrimSuffix(string(prev), suffix), prefix)
-	currentConfig := strings.TrimPrefix(strings.TrimSuffix(currentProduct, suffix), prefix)
+	previousProductAndVariant := strings.TrimPrefix(strings.TrimSuffix(previousConfig, suffix), prefix)
+	currentProductAndVariant := strings.TrimPrefix(strings.TrimSuffix(currentConfig, suffix), prefix)
 
-	ctx.Printf("Build configuration changed: %q -> %q, forcing installclean\n", prevConfig, currentConfig)
+	ctx.Printf("Build configuration changed: %q -> %q, forcing installclean\n", previousProductAndVariant, currentProductAndVariant)
 
-	installClean(ctx, config, 0)
+	installClean(ctx, config)
 
 	writeConfig()
 }
 
 // cleanOldFiles takes an input file (with all paths relative to basePath), and removes files from
 // the filesystem if they were removed from the input file since the last execution.
-func cleanOldFiles(ctx Context, basePath, file string) {
-	file = filepath.Join(basePath, file)
-	oldFile := file + ".previous"
+func cleanOldFiles(ctx Context, basePath, newFile string) {
+	newFile = filepath.Join(basePath, newFile)
+	oldFile := newFile + ".previous"
 
-	if _, err := os.Stat(file); err != nil {
-		ctx.Fatalf("Expected %q to be readable", file)
+	if _, err := os.Stat(newFile); err != nil {
+		ctx.Fatalf("Expected %q to be readable", newFile)
 	}
 
 	if _, err := os.Stat(oldFile); os.IsNotExist(err) {
-		if err := os.Rename(file, oldFile); err != nil {
-			ctx.Fatalf("Failed to rename file list (%q->%q): %v", file, oldFile, err)
+		if err := os.Rename(newFile, oldFile); err != nil {
+			ctx.Fatalf("Failed to rename file list (%q->%q): %v", newFile, oldFile, err)
 		}
 		return
 	}
 
-	var newPaths, oldPaths []string
-	if newData, err := ioutil.ReadFile(file); err == nil {
-		if oldData, err := ioutil.ReadFile(oldFile); err == nil {
-			// Common case: nothing has changed
-			if bytes.Equal(newData, oldData) {
-				return
-			}
-			newPaths = strings.Fields(string(newData))
-			oldPaths = strings.Fields(string(oldData))
-		} else {
-			ctx.Fatalf("Failed to read list of installable files (%q): %v", oldFile, err)
-		}
+	var newData, oldData []byte
+	if data, err := ioutil.ReadFile(newFile); err == nil {
+		newData = data
 	} else {
-		ctx.Fatalf("Failed to read list of installable files (%q): %v", file, err)
+		ctx.Fatalf("Failed to read list of installable files (%q): %v", newFile, err)
 	}
+	if data, err := ioutil.ReadFile(oldFile); err == nil {
+		oldData = data
+	} else {
+		ctx.Fatalf("Failed to read list of installable files (%q): %v", oldFile, err)
+	}
+
+	// Common case: nothing has changed
+	if bytes.Equal(newData, oldData) {
+		return
+	}
+
+	var newPaths, oldPaths []string
+	newPaths = strings.Fields(string(newData))
+	oldPaths = strings.Fields(string(oldData))
 
 	// These should be mostly sorted by make already, but better make sure Go concurs
 	sort.Strings(newPaths)
@@ -242,42 +257,55 @@
 				continue
 			}
 		}
+
 		// File only exists in the old list; remove if it exists
-		old := filepath.Join(basePath, oldPaths[0])
+		oldPath := filepath.Join(basePath, oldPaths[0])
 		oldPaths = oldPaths[1:]
-		if fi, err := os.Stat(old); err == nil {
-			if fi.IsDir() {
-				if err := os.Remove(old); err == nil {
-					ctx.Println("Removed directory that is no longer installed: ", old)
-					cleanEmptyDirs(ctx, filepath.Dir(old))
+
+		if oldFile, err := os.Stat(oldPath); err == nil {
+			if oldFile.IsDir() {
+				if err := os.Remove(oldPath); err == nil {
+					ctx.Println("Removed directory that is no longer installed: ", oldPath)
+					cleanEmptyDirs(ctx, filepath.Dir(oldPath))
 				} else {
-					ctx.Println("Failed to remove directory that is no longer installed (%q): %v", old, err)
+					ctx.Println("Failed to remove directory that is no longer installed (%q): %v", oldPath, err)
 					ctx.Println("It's recommended to run `m installclean`")
 				}
 			} else {
-				if err := os.Remove(old); err == nil {
-					ctx.Println("Removed file that is no longer installed: ", old)
-					cleanEmptyDirs(ctx, filepath.Dir(old))
+				// Removing a file, not a directory.
+				if err := os.Remove(oldPath); err == nil {
+					ctx.Println("Removed file that is no longer installed: ", oldPath)
+					cleanEmptyDirs(ctx, filepath.Dir(oldPath))
 				} else if !os.IsNotExist(err) {
-					ctx.Fatalf("Failed to remove file that is no longer installed (%q): %v", old, err)
+					ctx.Fatalf("Failed to remove file that is no longer installed (%q): %v", oldPath, err)
 				}
 			}
 		}
 	}
 
 	// Use the new list as the base for the next build
-	os.Rename(file, oldFile)
+	os.Rename(newFile, oldFile)
 }
 
+// cleanEmptyDirs will delete a directory if it contains no files.
+// If a deletion occurs, then it also recurses upwards to try and delete empty parent directories.
 func cleanEmptyDirs(ctx Context, dir string) {
 	files, err := ioutil.ReadDir(dir)
-	if err != nil || len(files) > 0 {
+	if err != nil {
+		ctx.Println("Could not read directory while trying to clean empty dirs: ", dir)
 		return
 	}
-	if err := os.Remove(dir); err == nil {
-		ctx.Println("Removed directory that is no longer installed: ", dir)
-	} else {
-		ctx.Fatalf("Failed to remove directory that is no longer installed (%q): %v", dir, err)
+	if len(files) > 0 {
+		// Directory is not empty.
+		return
 	}
+
+	if err := os.Remove(dir); err == nil {
+		ctx.Println("Removed empty directory (may no longer be installed?): ", dir)
+	} else {
+		ctx.Fatalf("Failed to remove empty directory (which may no longer be installed?) %q: (%v)", dir, err)
+	}
+
+	// Try and delete empty parent directories too.
 	cleanEmptyDirs(ctx, filepath.Dir(dir))
 }
diff --git a/ui/build/environment.go b/ui/build/environment.go
index 9bca7c0..6d8a28f 100644
--- a/ui/build/environment.go
+++ b/ui/build/environment.go
@@ -37,8 +37,8 @@
 // It's equivalent to the os.LookupEnv function, but with this copy of the
 // Environment.
 func (e *Environment) Get(key string) (string, bool) {
-	for _, env := range *e {
-		if k, v, ok := decodeKeyValue(env); ok && k == key {
+	for _, envVar := range *e {
+		if k, v, ok := decodeKeyValue(envVar); ok && k == key {
 			return v, true
 		}
 	}
@@ -65,37 +65,40 @@
 
 // Unset removes the specified keys from the Environment.
 func (e *Environment) Unset(keys ...string) {
-	out := (*e)[:0]
-	for _, env := range *e {
-		if key, _, ok := decodeKeyValue(env); ok && inList(key, keys) {
+	newEnv := (*e)[:0]
+	for _, envVar := range *e {
+		if key, _, ok := decodeKeyValue(envVar); ok && inList(key, keys) {
+			// Delete this key.
 			continue
 		}
-		out = append(out, env)
+		newEnv = append(newEnv, envVar)
 	}
-	*e = out
+	*e = newEnv
 }
 
 // UnsetWithPrefix removes all keys that start with prefix.
 func (e *Environment) UnsetWithPrefix(prefix string) {
-	out := (*e)[:0]
-	for _, env := range *e {
-		if key, _, ok := decodeKeyValue(env); ok && strings.HasPrefix(key, prefix) {
+	newEnv := (*e)[:0]
+	for _, envVar := range *e {
+		if key, _, ok := decodeKeyValue(envVar); ok && strings.HasPrefix(key, prefix) {
+			// Delete this key.
 			continue
 		}
-		out = append(out, env)
+		newEnv = append(newEnv, envVar)
 	}
-	*e = out
+	*e = newEnv
 }
 
 // Allow removes all keys that are not present in the input list
 func (e *Environment) Allow(keys ...string) {
-	out := (*e)[:0]
-	for _, env := range *e {
-		if key, _, ok := decodeKeyValue(env); ok && inList(key, keys) {
-			out = append(out, env)
+	newEnv := (*e)[:0]
+	for _, envVar := range *e {
+		if key, _, ok := decodeKeyValue(envVar); ok && inList(key, keys) {
+			// Keep this key.
+			newEnv = append(newEnv, envVar)
 		}
 	}
-	*e = out
+	*e = newEnv
 }
 
 // Environ returns the []string required for exec.Cmd.Env
@@ -105,11 +108,11 @@
 
 // Copy returns a copy of the Environment so that independent changes may be made.
 func (e *Environment) Copy() *Environment {
-	ret := Environment(make([]string, len(*e)))
-	for i, v := range *e {
-		ret[i] = v
+	envCopy := Environment(make([]string, len(*e)))
+	for i, envVar := range *e {
+		envCopy[i] = envVar
 	}
-	return &ret
+	return &envCopy
 }
 
 // IsTrue returns whether an environment variable is set to a positive value (1,y,yes,on,true)
@@ -140,15 +143,20 @@
 	return e.appendFromKati(file)
 }
 
+// Helper function for AppendFromKati. Accepts an io.Reader to make testing easier.
 func (e *Environment) appendFromKati(reader io.Reader) error {
 	scanner := bufio.NewScanner(reader)
 	for scanner.Scan() {
 		text := strings.TrimSpace(scanner.Text())
 
 		if len(text) == 0 || text[0] == '#' {
+			// Skip blank lines and comments.
 			continue
 		}
 
+		// We expect two space-delimited strings, like:
+		// unset 'HOME'
+		// export 'BEST_PIZZA_CITY'='NYC'
 		cmd := strings.SplitN(text, " ", 2)
 		if len(cmd) != 2 {
 			return fmt.Errorf("Unknown kati environment line: %q", text)
@@ -159,6 +167,8 @@
 			if !ok {
 				return fmt.Errorf("Failed to unquote kati line: %q", text)
 			}
+
+			// Actually unset it.
 			e.Unset(str)
 		} else if cmd[0] == "export" {
 			key, value, ok := decodeKeyValue(cmd[1])
@@ -175,6 +185,7 @@
 				return fmt.Errorf("Failed to unquote kati line: %q", text)
 			}
 
+			// Actually set it.
 			e.Set(key, value)
 		} else {
 			return fmt.Errorf("Unknown kati environment command: %q", text)