Boeing / config-file-validator

Cross Platform tool to validate configuration files
https://boeing.github.io/config-file-validator/
Apache License 2.0
324 stars 55 forks source link

feat: add support for pkl files #164

Open olunusib opened 1 month ago

olunusib commented 1 month ago

Adds support for PKL files (https://pkl-lang.org/). Note that pkl-go requires Go 1.22.1. This might be considered a breaking change for users of this tool who are on earlier Go versions.

Closes #141.

olunusib commented 1 month ago

@kehoecj @ccoVeille The CI is failing due to this issue with pkl-go. Basically, pkl needs to be installed for the tests to pass.

We could address this by adding a step in the workflow to install pkl before running the tests and including a note in the documentation to inform users that pkl must be installed to use this tool with pkl files. Should note that pkl comes with a built-in evaluator. Let me know how you want to proceed.

kehoecj commented 1 month ago

@kehoecj @ccoVeille The CI is failing due to this issue with pkl-go. Basically, pkl needs to be installed for the tests to pass.

We could address this by adding a step in the workflow to install pkl before running the tests and including a note in the documentation to inform users that pkl must be installed to use this tool with pkl files. Should note that pkl comes with a built-in evaluator. Let me know how you want to proceed.

I'm not sure I want to introduce a binary dependency for the validator for a single config type - especially because our release system relies on downloading statically-linked binaries or running go install rather than through package managers like RPMs/Debs, Homebrew, Chocolately, etc. We have started down that path with an AUR for arch linux and I'd like to keep adding packages and publishing to package managers but until we do I don't think we can support binary dependencies without negatively impacting the current installation workflow for existing users.

ccoVeille commented 1 month ago

I agree. Is there another pkl validation only in Go?

ccoVeille commented 1 month ago

But wait, do you need the code generation?

As documented it's the only use case when the binary is required

https://pkl-lang.org/go/current/quickstart.html#1-install-dependencies

olunusib commented 1 month ago

@kehoecj @ccoVeille The CI is failing due to this issue with pkl-go. Basically, pkl needs to be installed for the tests to pass. We could address this by adding a step in the workflow to install pkl before running the tests and including a note in the documentation to inform users that pkl must be installed to use this tool with pkl files. Should note that pkl comes with a built-in evaluator. Let me know how you want to proceed.

I'm not sure I want to introduce a binary dependency for the validator for a single config type - especially because our release system relies on downloading statically-linked binaries or running go install rather than through package managers like RPMs/Debs, Homebrew, Chocolately, etc. We have started down that path with an AUR for arch linux and I'd like to keep adding packages and publishing to package managers but until we do I don't think we can support binary dependencies without negatively impacting the current installation workflow for existing users.

Agreed

olunusib commented 1 month ago

But wait, do you need the code generation?

As documented it's the only use case when the binary is required

https://pkl-lang.org/go/current/quickstart.html#1-install-dependencies

There are 2 binaries:

ccoVeille commented 1 month ago

Then pkl format is a pain

This project could help

https://github.com/Drafteame/pkl-linter

But for now, it only supports CLI. But we could test the tool and ask developer (or help him) to provide a library

kehoecj commented 1 month ago

@olunusib @ccoVeille Trying to think of a way to keep this work without negatively impacting the installation experience for users who have no need for PKL and don't have the binary installed. What if PKL was opt-in only (at least for now) where if go can't find the pkl executable on the path then it won't try to lint it? If it finds a .pkl file during validation but PKL is not installed, it will give a warning saying something like "the following PKL files were found but not linted because the PKL binary was not found on the path".

For the Github Action and AUR/Scoop/WinGet packages I can add PKL as a dependency so that shouldn't be a problem for that type of distribution.

What do you think?

ccoVeille commented 1 month ago

I like your suggestion

kehoecj commented 1 month ago

It likes your suggestion

@olunusib What do you think? Would you want to take a shot at making PKL optional as described above?

olunusib commented 1 month ago

Sounds like a good plan @kehoecj. I'll find some time this weekend to look into it

olunusib commented 3 weeks ago

@kehoecj @ccoVeille this should be ready for review. I'd like to see the workflow run at least.