Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

existingpath with default value fails even if a config file is provided with a valid path #454

Open
calebcase opened this issue Aug 27, 2024 · 1 comment

Comments

@calebcase
Copy link

Flags which are of type existingfile fail to work properly with a config file.

package main

import (
	"os"

	"github.com/alecthomas/kong"
)

type CLI struct {
	Path string `kong:"default='myfile',type='existingfile'"`
}

func Run() (err error) {
	cli := &CLI{}

	kctx := kong.Parse(
		cli,

		kong.Configuration(
			kong.JSON,
			"config.json",
		),
	)

	return kctx.Run()
}

func main() {
	err := Run()
	if err != nil {
		os.Exit(1)
	}
}
$ cat config.json 
{
  "path": "somefile"
}
$ touch somefile
$ ls somefile
somefile

My expectation was that this would succeed without error (somefile exists as set in config file). However it instead fails with an error:

$ go build && ./bug
bug: error: --path: stat /home/ccase/bug/myfile: no such file or directory

The error shows it isn't looking for somefile but for the default myfile (which does not exist).

Tracing in delve shows it fails in the call to Reset before the resolver is run:

> github.com/alecthomas/kong.(*Kong).Parse() /home/ccase/go/pkg/mod/github.com/alecthomas/[email protected]/kong.go:291 (PC: 0x5fa81f)
   286:		}
   287:		if err = k.applyHook(ctx, "BeforeReset"); err != nil {
   288:			return nil, &ParseError{error: err, Context: ctx}
   289:		}
   290:		if err = ctx.Reset(); err != nil {
=> 291:			return nil, &ParseError{error: err, Context: ctx}
   292:		}
   293:		if err = k.applyHook(ctx, "BeforeResolve"); err != nil {
   294:			return nil, &ParseError{error: err, Context: ctx}
   295:		}
   296:		if err = ctx.Resolve(); err != nil {
(dlv) p err
error(*fmt.wrapError) *{
	msg: "--path: stat /home/ccase/bug/myfile: no such file or directory",
	err: error(*io/fs.PathError) *{
		Op: "stat",
		Path: "/home/ccase/bug/myfile",
		Err: error(syscall.Errno) *(*error)(0xc00021c860),},}

kong/kong.go

Lines 290 to 298 in 6c216a3

if err = ctx.Reset(); err != nil {
return nil, &ParseError{error: err, Context: ctx}
}
if err = k.applyHook(ctx, "BeforeResolve"); err != nil {
return nil, &ParseError{error: err, Context: ctx}
}
if err = ctx.Resolve(); err != nil {
return nil, &ParseError{error: err, Context: ctx}
}

Removing the default value allows it to pass the call to reset, but is less than ideal since now it isn't possible to configure a default.

This may be the same underlying issue as in:

This may be a regression since I also see #297 and #319 tried to fix variants of this before.

Looking at the debugger again I see that Active has been set to true which seems to run counter to the idea presented in PR #319.

(dlv) c
> [Breakpoint 4] github.com/alecthomas/kong.(*Context).Reset.func1() /home/ccase/go/pkg/mod/github.com/alecthomas/[email protected]/context.go:328 (hits goroutine(1):4 total:4) (PC: 0x615b6a)
   323:	}
   324:	
   325:	// Reset recursively resets values to defaults (as specified in the grammar) or the zero value.
   326:	func (c *Context) Reset() error {
   327:		return Visit(c.Model.Node, func(node Visitable, next Next) error {
=> 328:			if value, ok := node.(*Value); ok {
   329:				return next(value.Reset())
   330:			}
   331:			return next(nil)
   332:		})
   333:	}
(dlv) p node
github.com/alecthomas/kong.Visitable(*github.com/alecthomas/kong.Flag) *{
	Value: *github.com/alecthomas/kong.Value {
		Flag: *(*"github.com/alecthomas/kong.Flag")(0xc00014ecb0),
		Name: "path",
		Help: "",
		OrigHelp: "",
		HasDefault: true,
		Default: "myfile",
		DefaultValue: (*reflect.Value)(0xc00012ec80),
		Enum: "",
		Mapper: github.com/alecthomas/kong.Mapper(github.com/alecthomas/kong.MapperFunc) *(*"github.com/alecthomas/kong.Mapper")(0xc00012eca8),
		Tag: *(*"github.com/alecthomas/kong.Tag")(0xc000152640),
		Target: (*reflect.Value)(0xc00012ecc0),
		Required: false,
		Set: false,
		Format: "",
		Position: 0,
		Passthrough: false,
		Active: true,},
	Group: *github.com/alecthomas/kong.Group nil,
	Xor: []string len: 0, cap: 0, nil,
	PlaceHolder: "",
	Envs: []string len: 0, cap: 0, nil,
	Aliases: []string len: 0, cap: 0, nil,
	Short: 0,
	Hidden: false,
	Negated: false,}
@alecthomas
Copy link
Owner

This is a bit of a chicken and egg problem. As you can see, configuration isn't loaded until the CLI is reset with defaults, at which point the default has already failed.

To be honest I am more and more of the opinion that default on existingfile should be disallowed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants