Tool stanza

[Sudha247]

Description

There has been discussion around this at #16185. For anyone curious about the context, I'd recommend reading the issue.

This patch contains a proposal for introducing a tool stanza (rather tool_group stanza), for dune tools. The idea is to have persistent configuration for tooling we use. It only contains the code for the stanza itself. I wanted to potentially get opinions on this before going further with locking, building, etc.

Following @Alizter's insightful comment at #16185 (comment), I rethought the tool stanza a bit, and introduced a inherit + shared_packages field. The idea is that packages mentioned in shared_packages would be copied directly enabling sharing with built packages and reducing build time. Due to the reasons Ali mentioned in his comment, this quirk is currently needed. My hope is that we can somehow provide a better abstraction on top of this, in the future.

We debated a bit on whether we should inherit from lock_dir or from a context. Here are the reasons I think it makes sense to inherit from a context:

• Users already address contexts in the CLI, like for example dune build --context new is something users are familiar with. This is a direct extension of the concept, of whether you want a tool to work in and inherit from a particular context.
• If we associate tool groups to context, whether a tool is declared and exists in a particular context is clear.
• Context to lock directory mapping exists, and its a cheap lookup. Reverse doesn't and we will need to build the machinery for it.

All that said, there are some open questions:

• Do we have any arguments agains inheriting from a context?
• To start with, while inheriting I've disabled other fields pertaining to a lock directory. Next step will be to add override semantics.
• Filters such as :with-test are accepted by default. Do we need to reject those?

Related Issue and Motivation

Fixes #16185

Checklist

• Tests added, if applicable.
• Change log entry added for any user-facing changes.
• Documentation added for any user-facing changes.

[Sudha247]

A couple of updates:

• I've tagged this stanza as unreleased. We can keep it this way and not add a version guard until it's stabilized a bit.
• @shonfeder gave a good suggestion to make lock_dir and inherit fields be at the same level. This makes sense because we use only one of them, and not both. We can extend this later, depending on how people want to use them.

[Alizter]

I'm not particularly fond of adding a bunch of dead-code, especially since the difficult job of implementing the rules hasn't been carried out yet.

It's difficult to make any comment on the contents of the stanza because we don't know what will be easy or difficult to implement yet without attempting it.

If you wish to work on this implementation, a better course of action would be to have a separate branch where you can review / add / etc. chunks of work. I think it's fine for that to live in this repo rather than on somebodies fork.

As it stands this PR adds a bunch of configuration for something that doesn't exist, so I don't think anybody can suggest anything substantive at this point.

The reason I am against adding dead-code is that dune has some history of speculative additions that never get finished. It only makes sense to speculatively merge parts as part of a larger already reviewed branch in that case. Otherwise, this work will be abandoned and we will have a strangely empty, documented and changeloged non-feature.

[Sudha247]

Thanks @Alizter! I agree that this is adding configuration for features that don't exist yet. But the idea is for the features to be built on top of this. In this regard, I find it useful to gather feedback early.

As it stands this PR adds a bunch of configuration for something that doesn't exist, so I don't think anybody can suggest anything substantive at this point.

True that the work is in its early stages, but I did get some useful feedback from @shonfeder, which has already been incorporated. I'm not a fan of developing in a silo and dropping a 100-commit PR for folks to review. I think that would make both parts (reviewing and steering) difficult. I agree that implementing the rules will require substantive work, but I also see this as an interesting UX problem regarding how config is exposed to users.

The reason I am against adding dead-code is that dune has some history of speculative additions that never get finished.

That's a fair concern, and I'm fine with keeping it out of the documentation and changelogs.

The intention behind opening a PR now is to review small chunks and iterate on them. We will have some dead code for a while, but I don't see it as a big risk, as it doesn't affect users, probably costs a few more ms of tests, and can be removed at any point.

I am continuing to work on top of this, e.g. https://github.com/Sudha247/dune/tree/tool-stanza-lock. If the consensus is we don't merge now, I will push my branches here. Whether the branch lives on my fork or in this repo doesn't change much in practice; either way, the work needs to be reviewed, but if that's the convention you want to be followed, I'm okay going with it.