Skip to content

Commit

Permalink
context: detect and report conflicting options
Browse files Browse the repository at this point in the history
`WithChroot` and `WithSnapshot` conflicts to each other,
and the users can get surprising results if they try to use them
at the same time.

Now: chroot is expected to be much more popular than snapshots,
being the latter useful mostly for tests or offline debug/troubleshoot,
but still the code should detect and handle these conflicts.

This is made a bit awkward because the current `context` pkg API.
Once we are free to break compatibility, we can make the flow more
linear.

Signed-off-by: Francesco Romani <fromani@redhat.com>
  • Loading branch information
ffromani committed May 26, 2021
1 parent c420557 commit 7dbe4eb
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
13 changes: 13 additions & 0 deletions pkg/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
package context

import (
"fmt"

"github.com/jaypipes/ghw/pkg/option"
"github.com/jaypipes/ghw/pkg/snapshot"
)
Expand All @@ -22,6 +24,7 @@ type Context struct {
snapshotUnpackedPath string
alert option.Alerter
refCount int
err error
}

// New returns a Context struct pointer that has had various options set on it
Expand Down Expand Up @@ -49,6 +52,13 @@ func New(opts ...*option.Option) *Context {
ctx.EnableTools = *merged.EnableTools
}

// New is not allowed to return error - it would break the established API.
// so the only way out is to actually do the checks here and record the error,
// and return it later, at the earliest possible occasion, in Setup()
if ctx.SnapshotPath != "" && ctx.Chroot != option.DefaultChroot {
// The env/client code supplied a value, but we are will overwrite it when unpacking shapshots!
ctx.err = fmt.Errorf("Conflicting options: chroot %q and snapshot path %q", ctx.Chroot, ctx.SnapshotPath)
}
return ctx
}

Expand Down Expand Up @@ -85,6 +95,9 @@ func (ctx *Context) Do(fn func() error) error {
// You should call `Setup` just once. It is safe to call `Setup` if you don't make
// use of optional extra features - `Setup` will do nothing.
func (ctx *Context) Setup() error {
if ctx.err != nil {
return ctx.err
}
if ctx.refCount > 1 {
// someone else came before and fixed things already!
ctx.refCount++
Expand Down
7 changes: 5 additions & 2 deletions pkg/option/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ import (
)

const (
defaultChroot = "/"
DefaultChroot = "/"
)

const (
envKeyChroot = "GHW_CHROOT"
envKeyDisableWarnings = "GHW_DISABLE_WARNINGS"
envKeyDisableTools = "GHW_DISABLE_TOOLS"
Expand Down Expand Up @@ -57,7 +60,7 @@ func EnvOrDefaultChroot() string {
if val, exists := os.LookupEnv(envKeyChroot); exists {
return val
}
return defaultChroot
return DefaultChroot
}

// EnvOrDefaultSnapshotPath returns the value of the GHW_SNAPSHOT_PATH environs variable
Expand Down

0 comments on commit 7dbe4eb

Please sign in to comment.