zyedidia / eget

Easily install prebuilt binaries from GitHub.
MIT License
866 stars 39 forks source link

Bad configuration files are silently ignored #84

Closed hhromic closed 10 months ago

hhromic commented 10 months ago

While configuring eget using a file, by mistake I used an array for the file config for one repository block which made eget silently ignore the whole file. I got puzzled for a long time on why eget started to simply forge/ignore all my previously working configuration until I realized the mistake (file must be a string, not an array of strings).

Looking at the source code, I see that the InitializeConfig() function probes different configuration files and locations until it finds a working file, falling back to default settings if none are found.

While the above is as designed and expected, I think eget should only move on to try the next file candidate if and only if the current candidate does not exist. Any other errors such as TOML parsing should be reported and the application stopped instead of silently being discarded and the probing moving on.

If you agree, I am happy to submit a PR with this change so users can easily spot unintended configuration errors.

EDIT: For completeness, here is an example valid TOML file with a slightly invalid eget configuration that is silently ignored and, if it is the only configuration file available, silently falls back to the defaults for everything:

[global]
target = "~/.local/bin"

["bad/config"]
file = ["some-binary"]
hhromic commented 10 months ago

I just made a simple fix for this in https://github.com/zyedidia/eget/compare/master...hhromic:eget:feat-cfg-errors

Using the above example bad config file, eget with the fix shows the below and exits:

$ ./eget whatever/whatever
error loading config: eget.toml: toml: line 5 (last key "\"bad/config\".file"): incompatible types: TOML value has type []interface {}; destination has type string

I think this is much better UX than silently ignoring errors and loading the defaults. Let me know if you want me to send the above as a PR.

zyedidia commented 10 months ago

Sounds good to fix. You can open a PR, I'll make a few comments and then merge when fixed.