ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.62k stars 401 forks source link

Kind of annoying to have both `dune.lock` and `dev-tools.locks` as top-level folders #10955

Open tjdevries opened 1 week ago

tjdevries commented 1 week ago

Desired Behavior

Instead of:

dev-tools.locks/
dune.lock/

Maybe it could be:

dune.lock/dev-tools.locks/

I don't know what the right name would be, but it's kind of annoying to have two additional top-level folders for the same build tool.

moyodiallo commented 1 week ago

This is a temporary solution because the way dune pkg lock is written at the moment that we are reusing the same lock. Ideally we would like to hide dev-tools.locks from the user by putting it inside the build directory but some changes need to be done on dune pkg lock.

We are trying to avoid mixing it with dune.lock as possible.

tjdevries commented 1 week ago

Yeah, sorry - I don't think I wrote my request very clearly (the downside of writing issues at midnight).

I was hoping that the dev tools lock could just be moved to be nested underneath the dune.lock but I would want it to be a separate set of locks. Personally, I wouldn't like having it in the _build/ directory because then we can't check in the locks for dev tools (which might be something you would want to do - for example, force the same ocamlformat version).

Leonidas-from-XIV commented 1 week ago

Let's preface it with my assumptions: IMHO the dev-tools lock directory should never be versioned as the exact versions of the dependencies of tools should not matter, while dune.lock should probably (?) be included in the repo.

So we have a number of options:

  1. Put multiple .lock directories in the project. I agree with @tjdevries that this leads to a bit of clutter on the toplevel, especially if you have multiple lock folders for different configurations. However it is fairly easy to .gitignore the dev-tools lock dir and git add dune.lock does the right thing. We could also extend dune init to ignore it by default (#9588 is attempting to tackle this).
  2. Put dev-tools.locks into dune.lock. This removes the clutter, however semantically it is weird because dune.lock is not "the lock" it is just the default location for the lock and you can have multiple lock dirs for different configurations (e.g. for GNU/Linux, macOS, *BSD). If you have multiple lock dirs, where should dev-tools.locks live? Furthermore this makes git add dune.lock do the wrong thing and git add dune.lock && git rm dune.lock/dev-tools.locks is fairly obscure so a lot of people will commit their dev-tool lock files by accident.
  3. Put all locks into a locks directory. Maybe an ok compromise, but still fairly easy to git add locks and add the files by accident. On one hand this is an improvement, but on the other hand this makes the common case of one lock directory turn into a mess of subdirectories (locks/default/foo.pkg vs dune.lock/foo.pkg). For context: OPAM went with this, where now .opam files can also be in an opam subfolder and Dune recently gained support for generating them there.
  4. Putting them in _build has the advantage that it doesn't clutter the users project with information that should not be committed.

As such I am not sure whether changing the status quo makes sense, as I generally would hope that dev-tools.locks will disappear over time. Input welcome.

I don't think that the argument for checking in the tool lock directories is great, because what you most likely care about is the version of the dev-tool, not its dependencies. The better solution for this very sensible usecase is a place to specify the version to use. Generally this should most likely be a dev-depends section in dune-project (which then can compile out to depends with :with-dev-setup in OPAM files to retain compatibility with opam). In the specific case of ocamlformat I believe we do read .ocamlformat and use that version if it is specified there (@moyodiallo correct me if I'm wrong).

rgrinberg commented 1 week ago

The simplest thing does sound like just removing this lock directory from the source directory. Why don't we put it. There's no need to allow the user to interact with this build plan at all.

moyodiallo commented 1 week ago

The simplest thing does sound like just removing this lock directory from the source directory. Why don't we put it. There's no need to allow the user to interact with this build plan at all.

If I understood well, it would be then following steps.

At the end no dev-tools.locks and ocamlformat is available as before. I wonder what constraints we could have doing this ?

rgrinberg commented 1 week ago

Why _build though? The user often deletes this directory and it would surprising if this caused dev tools to be re-solved or re-installed. Some private directory would make more sense to me.

moyodiallo commented 1 week ago

Why _build though? The user often deletes this directory and it would surprising if this caused dev tools to be re-solved or re-installed. Some private directory would make more sense to me.

This is one of the concern we were discussing about IIRC. If I understood you, it's after the build of ocamlformat we relocate it out of _build and that would solve the annoying re-installation after any dune clean.

rgrinberg commented 1 week ago

Where do you plan to relocate it out of _build?

Leonidas-from-XIV commented 1 week ago

Where do you plan to relocate it out of _build?

Where would you suggest to put it? It still has to be somewhere in the project folder as we don't want to leak dev-tools into other projects. In that case it would probably be sensible to install the packages in that location as well there as well, as we don't want dune clean to delete the already-built dependencies either.

rgrinberg commented 1 week ago

~/.cache/dune comes to mind

Leonidas-from-XIV commented 1 week ago

That creates the issue that removing a project leaves behind unused dev-tools (and dependencies if we chose to put them there).

I feel like this would be a good topic to discuss at the next dev-meeting (2nd of October).

rgrinberg commented 1 week ago

Dependencies? Those should always be built inside the workspace. I'm only talking about the lock directory itself. If we rely on the cache, we'd already need a trimmer. Might as well teach the trimmer to remove whatever else might be lingering.

rgrinberg commented 1 week ago

Btw, no strong opinions where you plan to hide this file. I do think you'll have a slightly worse user experience if you use _build though. More importantly, you do need to make sure that wherever your lock directory lives, its path is represented via Path.External.t so that you can watch it.