Merge pull request #13 from jaypipes/issue12

allow override chroot with New()

README.md

@@ -28,16 +28,26 @@ struct contains a number of fields that may be queried for PCI information:
 the term "product ID" in `pcidb` because it more accurately reflects what the
 identifier is for: a specific product line produced by the vendor.
 
-**NOTE**: The default root mountpoint that `pcidb` uses when looking for
-information about the host system is `/`. So, for example, when looking up
-known PCI IDS DB files on Linux, `pcidb` will attempt to discover a pciids DB
-file at `/usr/share/misc/pci.ids`. If you are calling `pcidb` from a system
-that has an alternate root mountpoint, you can set the `PCIDB_CHROOT`
-environment variable to that alternate path. for example, if you are executing
-from within an application container that has bind-mounted the root host
-filesystem to the mount point `/host`, you would set `PCIDB_CHROOT` to `/host`
-so that pcidb can find files like `/usr/share/misc/pci.ids` at
-`/host/usr/share/misc/pci.ids`.
+### Overriding the root mountpoint `pcidb` uses
+
+The default root mountpoint that `pcidb` uses when looking for information
+about the host system is `/`. So, for example, when looking up known PCI IDS DB
+files on Linux, `pcidb` will attempt to discover a pciids DB file at
+`/usr/share/misc/pci.ids`. If you are calling `pcidb` from a system that has an
+alternate root mountpoint, you can either set the `PCIDB_CHROOT` environment
+variable to that alternate path, or call the `pcidb.New()` function with the
+`pcidb.WithChroot()` modifier.
+
+For example, if you are executing from within an application container that has
+bind-mounted the root host filesystem to the mount point `/host`, you would set
+`PCIDB_CHROOT` to `/host` so that pcidb can find files like
+`/usr/share/misc/pci.ids` at `/host/usr/share/misc/pci.ids`.
+
+Alternately, you can use the `pcidb.WithChroot()` function like so:
+
+```go
+pci := pcidb.New(pcidb.WithChroot("/host"))
+```
 
 ### PCI device classes
 

discover.go

@@ -15,7 +15,6 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
-	"strconv"
 
 	homedir "github.com/mitchellh/go-homedir"
 )
@@ -24,13 +23,13 @@ const (
 	PCIIDS_URI = "https://pci-ids.ucw.cz/v2.2/pci.ids.gz"
 )
 
-func (db *PCIDB) load() error {
+func (db *PCIDB) load(opts *options) error {
 	cachePath := cachePath()
 	// A set of filepaths we will first try to search for the pci-ids DB file
 	// on the local machine. If we fail to find one, we'll try pulling the
 	// latest pci-ids file from the network
 	paths := []string{cachePath}
-	addSearchPaths(&paths)
+	addSearchPaths(opts, &paths)
 	var foundPath string
 	for _, fp := range paths {
 		if _, err := os.Stat(fp); err == nil {
@@ -72,28 +71,12 @@ func cachePath() string {
 
 // Depending on the operating system, returns a set of local filepaths to
 // search for a pci.ids database file
-func addSearchPaths(paths *[]string) {
-	// The PCIDB_CACHE_ONLY environs variable is mostly just useful for
-	// testing. It essentially disables looking for any non ~/.cache/pci.ids
-	// filepaths (which is useful when we want to test the fetch-from-network
-	// code paths
-	if val, exists := os.LookupEnv("PCIDB_CACHE_ONLY"); exists {
-		if parsed, err := strconv.ParseBool(val); err != nil {
-			fmt.Fprintf(
-				os.Stderr,
-				"Failed parsing a bool from PCIDB_CACHE_LOCAL_ONLY "+
-					"environ value of %s",
-				val,
-			)
-		} else if parsed {
-			return
-		}
+func addSearchPaths(opts *options, paths *[]string) {
+	if opts.cacheOnly {
+		return
 	}
 
-	rootPath := "/"
-	if val, exists := os.LookupEnv("PCIDB_CHROOT"); exists {
-		rootPath = val
-	}
+	rootPath := opts.chroot
 
 	if runtime.GOOS != "windows" {
 		*paths = append(*paths, filepath.Join(rootPath, "usr", "share", "hwdata", "pci.ids"))

internal_test.go

@@ -0,0 +1,30 @@
+//
+// Use and distribution licensed under the Apache license version 2.
+//
+// See the COPYING file in the root project directory for full text.
+//
+
+package pcidb
+
+import (
+	"testing"
+)
+
+func TestMergeOptions(t *testing.T) {
+	// Verify the default values are set if no overrides are passed
+	opts := mergeOptions()
+	if opts.Chroot == nil {
+		t.Fatalf("Expected opts.Chroot to be non-nil.")
+	}
+	if opts.CacheOnly == nil {
+		t.Fatalf("Expected opts.CacheOnly to be non-nil.")
+	}
+
+	// Verify if we pass an override, that value is used not the default
+	opts = mergeOptions(WithChroot("/override"))
+	if opts.Chroot == nil {
+		t.Fatalf("Expected opts.Chroot to be non-nil.")
+	} else if *opts.Chroot != "/override" {
+		t.Fatalf("Expected opts.Chroot to be /override.")
+	}
+}

main.go

@@ -6,6 +6,16 @@
 
 package pcidb
 
+import (
+	"fmt"
+	"os"
+	"strconv"
+)
+
+var (
+	cacheOnlyTrue = true
+)
+
 type PCIProgrammingInterface struct {
 	// Id is DEPRECATED in 0.2 and will be removed in the 1.0 release. Please
 	// use the equivalent ID field.
@@ -65,8 +75,84 @@ type PCIDB struct {
 	Products map[string]*PCIProduct
 }
 
-func New() (*PCIDB, error) {
+// WithOption is used to represent optionally-configured settings
+type WithOption struct {
+	// Chroot is the directory that pcidb uses when attempting to discover
+	// pciids DB files
+	Chroot *string
+	// CacheOnly is mostly just useful for testing. It essentially disables
+	// looking for any non ~/.cache/pci.ids filepaths (which is useful when we
+	// want to test the fetch-from-network code paths
+	CacheOnly *bool
+}
+
+func WithChroot(dir string) *WithOption {
+	return &WithOption{Chroot: &dir}
+}
+
+func WithCacheOnly() *WithOption {
+	return &WithOption{CacheOnly: &cacheOnlyTrue}
+}
+
+// Concrete merged set of configuration switches that get passed to pcidb
+// internal functions
+type options struct {
+	chroot    string
+	cacheOnly bool
+}
+
+func mergeOptions(opts ...*WithOption) *WithOption {
+	// Grab options from the environs by default
+	defaultChroot := "/"
+	if val, exists := os.LookupEnv("PCIDB_CHROOT"); exists {
+		defaultChroot = val
+	}
+	defaultCacheOnly := false
+	if val, exists := os.LookupEnv("PCIDB_CACHE_ONLY"); exists {
+		if parsed, err := strconv.ParseBool(val); err != nil {
+			fmt.Fprintf(
+				os.Stderr,
+				"Failed parsing a bool from PCIDB_CACHE_ONLY "+
+					"environ value of %s",
+				val,
+			)
+		} else if parsed {
+			defaultCacheOnly = parsed
+		}
+	}
+	mergeOpts := &WithOption{}
+	for _, opt := range opts {
+		if opt.Chroot != nil {
+			mergeOpts.Chroot = opt.Chroot
+		}
+		if opt.CacheOnly != nil {
+			mergeOpts.CacheOnly = opt.CacheOnly
+		}
+	}
+	// Set the default value if missing from mergeOpts
+	if mergeOpts.Chroot == nil {
+		mergeOpts.Chroot = &defaultChroot
+	}
+	if mergeOpts.CacheOnly == nil {
+		mergeOpts.CacheOnly = &defaultCacheOnly
+	}
+	return mergeOpts
+}
+
+// New returns a pointer to a PCIDB struct which contains information you can
+// use to query PCI vendor, product and class information. It accepts zero or
+// more pointers to WithOption structs. If you want to modify the behaviour of
+// pcidb, use one of the option modifiers when calling New. For example, to
+// change the root directory that pcidb uses when discovering pciids DB files,
+// call New(WithChroot("/my/root/override"))
+func New(opts ...*WithOption) (*PCIDB, error) {
+	mergeOpts := mergeOptions(opts...)
+	useOpts := &options{
+		chroot:    *mergeOpts.Chroot,
+		cacheOnly: *mergeOpts.CacheOnly,
+	}
+
 	db := &PCIDB{}
-	err := db.load()
+	err := db.load(useOpts)
 	return db, err
 }