cps-org / cps-config

A drop in replacement for pkg-config/pkgconf using cps files
MIT License
15 stars 7 forks source link

Improve help features #71

Closed bretbrownjr closed 1 month ago

bretbrownjr commented 2 months ago

Ideally the documentation for the environment variables would go after the documentation for the CLI arguments, but cxxopts doesn't seem to have a mechanism for that arrangement.

bretbrownjr commented 2 months ago

Moving this to a draft until I get the CI passing.

bretbrownjr commented 2 months ago

This is currently blocked on my ability to understand how tl-expected, Ubuntu 22.04, and Meson support for tl-expected all interact. I'm getting different behavior on Ubuntu WSL 22.04 and the Ubuntu 22.04 as experienced in the dockerfile under the cps-config tests directory.

That's why I'm assigning this to @dcbaker, including sending a little more detail offline via the #ecosystem_evolution channel in https://cpplang.slack.com. I'm considering this blocked for now.

dcbaker commented 1 month ago

Well, this is fun. cxxopts 3.1 changed how exceptions are handled in a non-backwards compatible way.

bretbrownjr commented 1 month ago

OK. So rockylinux and ubuntu have different versions of cxxopts and cxxopts made a breaking change. Lovely.

@dcbaker I can add the ifdefs (gross, but could be worse) or we could force fallbacks on rocky linux. Any preferences?

Again, either way, let's call it a temporary workaround. I'll want to use a different library for CLI parsing in the near future anyway.

bretbrownjr commented 1 month ago

After two days of yak shaving, we no longer terminate with an uncaught exception if a user passes --not-a-flag to cps-config.

As noted in the comments in the Rocky Linux dockerfile, I made a tracking issue to clean up the workaround I added there... to force use of the Meson wraps on Rocky Linux for now.

At any rate, I'm taking this out of Draft state. It's ready for another look, @dcbaker.

dcbaker commented 1 month ago

I have a couple of thoughts. One is that we need to change the dep_cxxopts in Meson to have the version field do something like dependency('cxxopts', version : ['>= 3', '< 3.1']). The other question I have, is with that constraint do we still want to force all of the dependencies to fall back to wraps on rockylinux, or is it sufficient to allow just cxxopts to fallback?

Alternatively, does it make more sense to pin the version at >= 3.1, and not work with the versions shipped with old Ubuntu? When generating a dist Meson can vendor into that dist tarball wrap sources, which may be useful for Windows and MacOS anyway. Since cxxopts is header only, it wont link to anything, that may be fine for distros, or they can carry the necessary patch themselves. The solution we have right now will force basically all non-LTS distros to either patch the source to use the new API or fall back to wrap, and my preference would be to let the LTS distros deal with the fact that they have really old software then put that burden on more bleeding edge distros, who are more likely to pick up packaging cps-config early

bretbrownjr commented 1 month ago

For all that fiddling, I'd rather switch to cli11. All this is downstream of a library breaking its API. CLI11 seems to be available on the interesting platforms.

I don't want any dependency management features in our build systems over time. Discovery? Sure. Version selection? No. Among other reasons, managing dependencies with build systems makes it difficult to support another build system, and we'll need to support at least CMake.

Let me budget a small block of time to see how much work it is to divest from cxxopts.

For the record, I'm not comfortable degrading support for LTS distros or for more progressive distros. This was just an attempt at a workaround to land the PR. I was going to circle back, hence the bug issue.

dcbaker commented 1 month ago

I'm fine with switch, I don't have a strong preference for parsing libraries.

We need to change the version requirement in Meson with your series, since if I pull your branch and run meson setup it will happily find the system provided cxxopts 3.2, and the fail to build. If we dry the version to either == 3.0.0 or ['>= 3', '< 3.1'] Meson will correctly reject the non-compatible version and use the wrap.

If you prefer to keep == 3.0 working and the rest of us use wraps thats fine, if you prefer to keep '>= 3.1' working and use wraps on Ubuntu that's fine, but we need to ensure things keep working out of the box.

bretbrownjr commented 1 month ago

Please see #77, which is a change in direction from this PR. Better support for a user providing bad flags is included in the scope of that PR.

But better documentation for environment variables is not included in that PR. I'll probably make a small follow-up PR for that given I would probably add new flags like --cps-path that are tied to corresponding environment variables like CPS_PATH when I implement that.

bretbrownjr commented 1 month ago

Obsoleted by #77