[draft] Tool management design#13457
Conversation
7f68d71 to
884541d
Compare
|
@shonfeder I've gone through my own design notes and prototype to update this document. As it stands it is still a WIP however there is much more material that can be fleshed out and discussed. I've split the requirements into functional requirements and a technical specification, although it isn't perfect. I included my own research on similar features from other package managers, but this could be expanded further. Hopefully this closes the gap between what we dissected and what I had in the prototype. The CRs should answer most questions you have reading the docs, but further changes and additions/deletions are welcome. The end-goal will be to address all the CRs so they are not a permanent feature. |
Sudha247
left a comment
There was a problem hiding this comment.
Thanks for writing this down, this is very useful. I read through parts of the document and some comments below. Will do another pass later.
|
|
||
| CR-soon Alizter: Edge cases to specify: | ||
|
|
||
| - Tool not locked: error with suggestion to run `dune tools add`? |
There was a problem hiding this comment.
Any downsides to automatically locking and installing when the tool is not found?
There was a problem hiding this comment.
The problem is that in that case the command that would launch the dev tool locks up for an extended time period with no good way to communicate to the caller that it's locking, downloading and building the dev tool.
It might still be worthwhile but its good to have this issue in mind when designing.
| - Specified version not locked: error with suggestion? | ||
| - Build fails: propagate build error? | ||
|
|
||
| #### 2.2. Programmatic use |
There was a problem hiding this comment.
Worth mentioning this should try to maintain as much backwards compatibility as possible to not break the workflows of other platform tools?
There was a problem hiding this comment.
Do you have any particular tools in mind and how we might break their workflows?
There was a problem hiding this comment.
For example, ocaml-lsp-server uses some dune tools .. commands, I believe.
| - How does `path` interact with `(tool)` stanzas vs CLI-added tools? Same | ||
| resolution as `run`? | ||
|
|
||
| CR-someday Alizter: Consider a single bin directory with symlinks to all tool |
There was a problem hiding this comment.
How would this work if there are multiple versions of the same tool locked?
There was a problem hiding this comment.
Is this something we need to support? We should extend the spec with the requirement if so, but I'm not sure it's something we can or should consider as a requirement.
There was a problem hiding this comment.
It needs to be discussed further I think. There are technical / usability benefits to doing so, but it also doesn't fit in well with our execution model otherwise.
| don't need it (e.g., formatters that only parse source text). | ||
|
|
||
| CR-soon Alizter: The utility of opting out seems debatable, but is part of the | ||
| package-tool continuum approach. |
There was a problem hiding this comment.
package-tool continuum approach.
This sounds interesting. Can we read about this somewhere? More generally, I think both opt-in and opt-out come with their own set of caveats. But more tools require matching compilers than not, so it seems like a reasonable place to start.
There was a problem hiding this comment.
It appears in it's own section below, I've linked it for you in another comment #13457 (comment)
| - Remove tools | ||
| - Discover paths to tool executables | ||
|
|
||
| CLI-added tools persist until `dune clean` or `dune tools remove`, but are not |
There was a problem hiding this comment.
Do we need to remove the tools during dune clean? One might want to clean the project build without removing the tools. Does this also mean if one does rm -rf _build the tools will be gone?
|
|
||
| #### 5.1. Format rules (`dune fmt`, `dune build @fmt`) | ||
|
|
||
| `dune fmt` must use tools managed by `dune tools`. This includes respecting |
There was a problem hiding this comment.
Does this imply that ocamlformat installed via opam won't be used?
There was a problem hiding this comment.
I think it should unless (pkg enabled) is set.
There was a problem hiding this comment.
If dune tools add ocamlformat has not been done, then it will use the one from the switch. This is detailed in the PATH fallback requirement.
| ocamlformat to work for formatting your code. The standard tool isolation | ||
| handles this fine. | ||
|
|
||
| - **Working on the tool itself** (e.g., developing ocamllsp): You likely want to |
There was a problem hiding this comment.
In-and-out is an issue, yes, but only for dogfooding in the same repo - for example, a developer working on ocamllsp would still be able to pin their local branch for other projects?
There was a problem hiding this comment.
Yes, that would be achieved with pins.
| #### Syntax | ||
|
|
||
| ```lisp | ||
| (tool |
There was a problem hiding this comment.
Should we include a way to add dependency constraints for the tool?
There was a problem hiding this comment.
Yes, those should be included.
| - How does `path` interact with `(tool)` stanzas vs CLI-added tools? Same | ||
| resolution as `run`? | ||
|
|
||
| CR-someday Alizter: Consider a single bin directory with symlinks to all tool |
There was a problem hiding this comment.
Is this something we need to support? We should extend the spec with the requirement if so, but I'm not sure it's something we can or should consider as a requirement.
|
|
||
| The following are explicitly out of scope: | ||
|
|
||
| #### 6.1. System-wide installation |
There was a problem hiding this comment.
I do not agree with this being a non-requirement. What is the motive for excluding this?
There was a problem hiding this comment.
Counter-question: What is the motive for including this? I'm open to well thought out proposals, this is simply recording the requirements of the design I worked on and I didn't see a need for this yet.
There was a problem hiding this comment.
We should also note that Dune even with package management is predominantly a build tool, not a package manager. Building dependencies makes sense because they are inevitable to get a project to be built but I don't think Dune has a reason to become a specialized package manager to install a subset of OCaml packages (those that build with Dune).
There's plenty of tools that do that already, including OPAM, which can also install non-Dune projects and would be better suited for this task.
There was a problem hiding this comment.
We should also note that Dune even with package management is predominantly a build tool, not a package manager
Not if we are working towards the stated ocaml platform roadmap, in which case dune should become the frontend of the ocaml ecosystem.
If it will not become this, because we insist it should stay "predominantly a build tool" then we need a deep revision to the strategy we are working towards.
There was a problem hiding this comment.
Note that this removes a requirement I had already specified in this doc, so putting the onus on my to justify why it should not be removed seems a bit backwards, IMO.
There was a problem hiding this comment.
The OCaml Platform does state that Dune is intended to be the front-end of the OCaml ecosystem. If we decide to move away from this, it has to be clearly documented so as not to confuse users.
| #### 6.3. Tools for tools | ||
|
|
||
| Using tools while developing the tools themselves (or their dependencies) has | ||
| nuances related to the "in and out" problem ([#8652]). |
There was a problem hiding this comment.
I disagree with this being a non-requirement as well. The fact that there is an implementation problem making this infeasible to implement currently is not a good reason to explicitly exclude it from the requirements, IMO.
Whether or not it is a requirement should be based on whether it is feasible in principle and whether it is addresses legitimate user needs.
Given that your later remark seems to indicate that the need for this is well motivated, then this should be replaced as a requirement.
Note that it is not expected that all requirements will be satisfied in one fell swoop of implementation. Some may take some time to satisfy due to difficulty of implementation. In this case, the in-and-out problem is such a complication.
But this is exactly why the requirement must be included in the spec: stating it as an achievable aim (difficult tho it may be), based out our reasoning from essential needs and basic principles, helps us identify the dependencies between our different planned streams of work.
There was a problem hiding this comment.
In addition to what Shon has said above, now that in-and-out is being actively worked on, this needs to be revisited.
| project packages (the "wrapper package hack"), causing solver conflicts and | ||
| complex regeneration logic. | ||
|
|
||
| ### The package-tool continuum |
There was a problem hiding this comment.
@Sudha247 Here is the section about the "package-tool continuum" idea.
Leonidas-from-XIV
left a comment
There was a problem hiding this comment.
Thanks for pushing this along. It's one of the most important things in package management (arguably even the most) and I appreciate that this is starting to move along.
| **Why a new system?** The legacy dev-tools system only supported a hardcoded | ||
| list of tools and required dune package management. The new system works with | ||
| any opam package, whether you use dune's package management, an opam switch, or | ||
| system OCaml. |
There was a problem hiding this comment.
Does that mean it does not require dune package management? How should it work in the absence of package management?
There was a problem hiding this comment.
You don't require a lock file or package management to be enabled in order to use it. It does use package management behind the scenes but this is an implementation detail not exposed to the user. The idea is that dune tool will give the same experience for both dune pkg and opam users (or even just system ocaml).
There was a problem hiding this comment.
I don't think this paragraph is needed here. It is covered in the issue that describes the motivation for the redesign: #12914
there we can just focus on optimal design itself, I think.
|
|
||
| - **Generality**: Any opam package providing executables can be a tool | ||
| - **Orthogonality**: Tools are solved independently from project dependencies | ||
| - **Clean source tree**: All tool artifacts live in `_build/`, not your repo |
There was a problem hiding this comment.
"work tree" is probably the better wording here.
|
|
||
| CR-soon Alizter: Edge cases to specify: | ||
|
|
||
| - Tool not locked: error with suggestion to run `dune tools add`? |
There was a problem hiding this comment.
The problem is that in that case the command that would launch the dev tool locks up for an extended time period with no good way to communicate to the caller that it's locking, downloading and building the dev tool.
It might still be worthwhile but its good to have this issue in mind when designing.
|
|
||
| The following are explicitly out of scope: | ||
|
|
||
| #### 6.1. System-wide installation |
There was a problem hiding this comment.
We should also note that Dune even with package management is predominantly a build tool, not a package manager. Building dependencies makes sense because they are inevitable to get a project to be built but I don't think Dune has a reason to become a specialized package manager to install a subset of OCaml packages (those that build with Dune).
There's plenty of tools that do that already, including OPAM, which can also install non-Dune projects and would be better suited for this task.
| **Why a new system?** The legacy dev-tools system only supported a hardcoded | ||
| list of tools and required dune package management. The new system works with | ||
| any opam package, whether you use dune's package management, an opam switch, or | ||
| system OCaml. |
There was a problem hiding this comment.
| **Why a new system?** The legacy dev-tools system only supported a hardcoded | |
| list of tools and required dune package management. The new system works with | |
| any opam package, whether you use dune's package management, an opam switch, or | |
| system OCaml. |
| - **Clean source tree**: All tool artifacts live in `_build/`, not your repo | ||
| - **Multi-version**: Different projects can use different tool versions |
There was a problem hiding this comment.
These do not look like principles to me, they look like functional requirements?
| Any opam package that provides executables can be used as a tool. Unlike the | ||
| legacy system, tools are not hardcoded into dune. Users can install arbitrary | ||
| packages without waiting for dune releases. |
There was a problem hiding this comment.
We don't need to refer to opam packages again, because we have already defined tools as executables defined by opam.
| Any opam package that provides executables can be used as a tool. Unlike the | |
| legacy system, tools are not hardcoded into dune. Users can install arbitrary | |
| packages without waiting for dune releases. | |
| All available tools must be installable, without requiring changes to the dune | |
| source code. |
There was a problem hiding this comment.
Best to focus the requirements on what is required. Mentioning things that don't work and aren't required in this kind of context just adds to complexity and verbiage, IMO.
| Tools are installed per-workspace, not globally. Each workspace has its own | ||
| isolated tool installations that don't affect other workspaces or the system. | ||
|
|
||
| See [Directory structure](#directory-structure) for storage locations. |
There was a problem hiding this comment.
This is an implementation detail, but the "how to read this doc" makes clear requirements aren't concerned with those.
|
|
||
| The following are explicitly out of scope: | ||
|
|
||
| #### 6.1. System-wide installation |
|
|
||
| The following are explicitly out of scope: | ||
|
|
||
| #### 6.1. System-wide installation |
There was a problem hiding this comment.
We should also note that Dune even with package management is predominantly a build tool, not a package manager
Not if we are working towards the stated ocaml platform roadmap, in which case dune should become the frontend of the ocaml ecosystem.
If it will not become this, because we insist it should stay "predominantly a build tool" then we need a deep revision to the strategy we are working towards.
Adds reference docs for `dune tools` as stated in ocaml#14184. The implementation of dev tools is subject to change, as being discussed in ocaml#13457. But parts of the user facing commands might stay and we might want to update the docs as the implementation is being fleshed out. Signed-off-by: Sudha Parimala <sudharg247@gmail.com>
Adds reference docs for `dune tools` as stated in ocaml#14184. The implementation of dev tools is subject to change, as being discussed in ocaml#13457. But parts of the user facing commands might stay and we might want to update the docs as the implementation is being fleshed out. Signed-off-by: Sudha Parimala <sudharg247@gmail.com>
Adds reference docs for `dune tools` as stated in ocaml#14184. The implementation of dev tools is subject to change, as being discussed in ocaml#13457. But parts of the user facing commands might stay and we might want to update the docs as the implementation is being fleshed out. Signed-off-by: Sudha Parimala <sudharg247@gmail.com>
Adds reference docs for `dune tools` as stated in ocaml#14184. The implementation of dev tools is subject to change, as being discussed in ocaml#13457. But parts of the user facing commands might stay and we might want to update the docs as the implementation is being fleshed out. Signed-off-by: Sudha Parimala <sudharg247@gmail.com>
|
|
||
| See [Directory structure](#directory-structure) for storage locations. | ||
|
|
||
| CR-soon Alizter: Once we have the relocatable compiler, tools being built will |
There was a problem hiding this comment.
Now that we have the relocatable compiler, should we update this?
| can have different `.ocamlformat` files specifying different versions, and the | ||
| formatting rules will use the correct version for each project. | ||
|
|
||
| CR-someday Alizter: Multi-version scaling. Supporting N versions means N |
There was a problem hiding this comment.
Thinking more about this, do we really have other concrete use cases of having first-class support for multiple versions, other than ocamlformat? I do wonder whether this is introducing more complexity than solving user pain points.
One other use case is if the user wants to test out multiple versions of the same tool or test during development. The UI of dune tools run tool.version seems a bit unintuitive to me.
There was a problem hiding this comment.
Which part is unintuitive exactly?
There was a problem hiding this comment.
Markdown broke what I wrote, but I meant to say having to explicitly call tool.version if you have multiple versions seems unintuitive from the perspective of I want to run the tool.
It could be that this is fine for what needs to be done to support multiple versions, but I don't yet see concrete use cases other than ocamlformat.
There was a problem hiding this comment.
Unless there are multiple versions of a tool around then there is no requirement to run dune tools run tool.version, just dune tools run tool will be enough. The case of ocamlformat is the main application as you've said, however how often is it that you want to run dune tools run ocamlformat by hand versus getting dune fmt to pick up the correct one?
There was a problem hiding this comment.
if we need this kind of functionality, then rather than tool.version we can take an argument dune tools run --with-version 1.1.1 tool or something. The principle here is that collapsing two pieces of data into one string is usually not a good idea :)
There was a problem hiding this comment.
Sure that works too. The reason I suggested this one is for users familiar with opam but its not a requirement.
|
|
||
| CR-soon Alizter: `dune tools path` behavior is underspecified: | ||
|
|
||
| - Should `path` trigger download and build if not yet built? Or only return path |
There was a problem hiding this comment.
Another option could be to provide a warning that the tools are not built while returning the path. However, I'd imagine consumers of this could be other platform tools. In that case wouldn't having a hard error break their workflows?
Sudha247
left a comment
There was a problem hiding this comment.
It's good to see this moving along! I'm of the belief that this is one of the most important, if not the most important, features for Dune package management. Thanks for pushing this along @Alizter and @shonfeder.
I'm generally happy with the direction this is moving along. However, I think we still have some open questions to address before we can merge this.
| CR-soon Alizter: Specify the RPC protocol for tool operations. What messages are | ||
| needed? How does the watch server respond to tool additions? | ||
|
|
||
| CR-soon Alizter: Document concurrent access behavior. What happens if two |
There was a problem hiding this comment.
It's useful to address this soon - in particular what should be the behaviour of Dune when this happens.
|
|
||
| #### PATH fallback | ||
|
|
||
| When a tool is not installed via the tool manager, fall back to system PATH. |
There was a problem hiding this comment.
This is presumably also to accommodate platform tools trying to use dune tools. In which case it would be useful to solicit opinions of platform tools developers on this.
|
|
||
| #### 6.2. Automatic updates | ||
|
|
||
| Tools do not auto-update; explicit user action is required. |
There was a problem hiding this comment.
I want to make a note here that this differs from the auto-locking model we have been discussing in #13011. It could be the case that the two of them have different needs and hence different models. However, it's worth documenting why that is. Both of them share several similar concerns:
- It was stated in the quoted issue that users find it tedious to maintain lockfiles. Even though there is no explicit lock file visible to the user, that concern still applies here.
- Why is the model for deps to update by default when we run
dune build, and not the case for tools? - This model also makes offline mode a possibility, which is still an open problem for dependencies.
|
|
||
| The following are explicitly out of scope: | ||
|
|
||
| #### 6.1. System-wide installation |
There was a problem hiding this comment.
The OCaml Platform does state that Dune is intended to be the front-end of the OCaml ecosystem. If we decide to move away from this, it has to be clearly documented so as not to confuse users.
| #### 6.3. Tools for tools | ||
|
|
||
| Using tools while developing the tools themselves (or their dependencies) has | ||
| nuances related to the "in and out" problem ([#8652]). |
There was a problem hiding this comment.
In addition to what Shon has said above, now that in-and-out is being actively worked on, this needs to be revisited.
| ;; Using a binary package repository | ||
| (tool | ||
| (package ocaml-lsp-server) | ||
| (repositories binary-packages)) |
There was a problem hiding this comment.
This example is perhaps not in line with 6.5, which states that tools are always built from source.
|
|
||
| ## Learnings from the Prototype | ||
|
|
||
| CR-soon Alizter: This entire section may be considered garbage. Extract |
There was a problem hiding this comment.
This needs a pass before merging.
Signed-off-by: Shon Feder <shon.feder@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Let's add this back when the document takes shape to clarify confusion. At the moment it is not adding anything and is potentially confusing. Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
- It shouldn't mention implementation details (switch etc.) - It is too constrained (excluding tools defined in a project build) - Given unnecessary context (mentioning current design) - Does not include wide enough examples of tools (only mentions tools we widely used and already supported with bespoke mechanisms) Signed-off-by: Shon Feder <shon.feder@gmail.com>
Signed-off-by: Shon Feder <shon.feder@gmail.com>
This is essential for clear communication, otherwise key terms get used inconsistently or understood ambiguously. As a result of recovering terminology we also detect an error in the "generality" principle: packages themselves are not tools, but they provide tools so "any opam package ... can be a tool" is a category mistake. Signed-off-by: Shon Feder <shon.feder@gmail.com>
This is not a useful section: things that aren't required just don't need to be listed as requirements. The canonical way of differentiating necessary from merely recommended requirements is MUST vs. SHOULD (or RECOMMENDED): https://www.ietf.org/rfc/rfc2119.txt Signed-off-by: Shon Feder <shon.feder@gmail.com>
If we are going to have a section of principles, let's make them principled principles. - "Clean source tree" is a requirement or an implementation detail - "Orthogonality" as stated was an implementation detail, generalizing it gives a principle. - "Multi-version" looks like a requirement, and it is entailed by a the principle of orthogonality Signed-off-by: Shon Feder <shon.feder@gmail.com>
- Drop implementation details from definition of "install" - Redefine "available" in terms of "install" as "installable" Signed-off-by: Shon Feder <shon.feder@gmail.com>
- Everything but summary after TOC - "how to read doc" sections should be first Signed-off-by: Shon Feder <shon.feder@gmail.com>
- May as well link the sections. - Explain what the terminology section is for. - Remove CR explanation: no place for those in the mergable doc. Signed-off-by: Shon Feder <shon.feder@gmail.com>
Signed-off-by: Shon Feder <shon.feder@gmail.com>
Signed-off-by: Shon Feder <shon.feder@gmail.com>
Make it clear that, while compatability is required, satisfaction is not. Signed-off-by: Shon Feder <shon.feder@gmail.com>
Reference to legacy system is not relevant for requirements (could be in implementation) Signed-off-by: Shon Feder <shon.feder@gmail.com>
Signed-off-by: Shon Feder <shon.feder@gmail.com>
These are design ideas for possible functionality, they are not relevant to functional specifications.
But make this a "SHOULD" rather than a "MUST" Signed-off-by: Shon Feder <shon.feder@gmail.com>
Since packages provide tools, rather than define them, we avoid equivocating between package names and tools: a package may provide a tool that shares the package name, but when tools are invoked, it is done by the name of tool executable, not by the name of the package. Therefore, binary selection is just the ability to filter a subset of available packages in case you don't want everything provided by a package (the default). Signed-off-by: Shon Feder <shon.feder@gmail.com>
Signed-off-by: Shon Feder <shon.feder@gmail.com>
No idea why this was merged alongside an unrelated requirement. Factor the versions pecification requirement into its own issue. Signed-off-by: Shon Feder <shon.feder@gmail.com>
Not sure why this overspecification and reduction in generality. We want the general usability. Signed-off-by: Shon Feder <shon.feder@gmail.com>
This proposes a set of high level, functional requirements for our tooling management functionality, and contributes to #12914
The requirements proposed here are informed by our experience with the current prototype, our use of other successful tools addressing this problem space, and by recent design discussions.