nimble config breaks choosenim

[samdmarshall]

I have a nimble config with the following:

nimbleDir="/home/demi/.nimble/" 

[PackageList]
name="local"
path="/home/demi/.config/nimble/packages.json"

this configuration seems to work with nimble, I can search the packages I've added to it and locate them without issue. however, now whenever I run choosenim, I get the following error:

choosenim.nim(201)       choosenim 
cliparams.nim(134)       newCliParams
config.nim(101)          parseConfig
Error: unhandled exception: Unable to parse config file: Unknown key: path [NimbleError]

this seems to trace back to where choosenim calls on nimble to parse the config file, but comes back as a failure even though the path property is a valid and expected property for the additional package list to have. this error happens regardless of command or flags passed to choosenim. if I completely comment out the additional packagelist section and properties from the nimble.ini then choosenim operates without issue.

additional info: nimble version: nimble v0.9.0 compiled at 2019-01-28 23:13:35 (no git-hash attached) choosenim version: choosenim v0.3.2 (2018-02-28 14:12:37) [linux/amd64]

additional additional info: after building choosenim from source, the locally built version had no issues with the config, but the binary installed via the install script did; I don't know the significance of this as they are the same version, but figured this might help debug what is going on.

[dom96]

I don't know the significance of this as they are the same version, but figured this might help debug what is going on.

The old version was compiled with an old Nimble config parser, hence the failure and why recompiling it fixes the issue. Not sure what the best way to fix this is. Maybe it would be better to just create a simple Nimble config parser that is custom to choosenim and make it ignore any unknown keys.

[samdmarshall]

after seeing your comment and coming to a better understanding of the code; i agree with you, the best course of action is probably to just make a custom method using parsecfg to gleam the necessary information out of the nimble config rather than relying on the more thorough code to parse the config for full nimble use (as the nimble application does).

i had originally assumed that since the locally built version of choosenim was also matching up against the same library versions that the downloaded version had. maybe the --version flag information should also include the version of nimble it was built against (and date), to make the distinction from what the version that is installed.

[samdmarshall]

i worked around this by installing a local (new) build of choosenim and don’t have the issue; i guess that the solution here is to just be more aggressive in making releases when any of the dependencies (direct or indirect (like this config file)) are changed and make the parsing code less hostile to unknown keys.

[genotrance]

I believe this should be working right now since choosenim.nimble now requires a specific tag of Nimble rather than a minimum version. This means this specific version will be downloaded if it is not found and an older version won't be used. This will avoid issues for any user who compiles choosenim from scratch. However, it could still fail in the future since a newer Nimble could create an updated config file and break an older copy of choosenim all the same.

I did some testing and we can remove the dependency on the nimble package by setting --path:"$nim/dist/nimble" in choosenim.nims. This will use the version of Nimble installed along with Nim rather than an older version in ~/.nimble/pkgs. It still won't avoid issues 100% if a user uses an old copy of choosenim with a newer Nim/Nimble especially since end users don't typically compile choosenim. But it will avoid issues when users compile their own copy. Further, newer releases we post will be linked with the latest Nimble that goes with a release version of Nim and we won't need to maintain an explicit Nimble version dependency in Choosenim.

Feedback appreciated.

[dom96]

@genotrance that's just updating whatever Nimble version choosenim is using and not really fixing this issue. We need to instead parse the config ourselves and not rely on Nimble for this, that way we can ignore settings in there that choosenim doesn't understand and doesn't care about.

Btw please don't change that Nimble dependency to --path, we should have reproducible builds hence the requirement on a specific commit hash of Nimble. You can feel free to update this commit hash to something newer.

[genotrance]

Sounds fair. I'll bump up the Nimble hash when you make a new tag.

[genotrance]

Oh, then can we close this issue?

[dom96]

Nope, we need to fix the config parser to not depend on nimble.

[genotrance]

It is a rare occurrence where nimble will add new fields. Is it really worth the effort?

[dom96]

it's common enough for this issue to exist, IMO it's worth it. Maybe not a super high priority though.

[dom96]

I think what we may need to do here is actually modify Nimble (since Choosenim uses its config parsing modules) to enable a "soft" mode which doesn't error on unknown fields. Ideally though we should output warnings in this case.

Anyway, low priority IMO. If this is open in another year I'll close it.