Skip to content

Comments

Include build-directory lock directories in lock scanning#13649

Draft
Sudha247 wants to merge 1 commit intoocaml:mainfrom
Sudha247:dev-tools-lockdir-scan
Draft

Include build-directory lock directories in lock scanning#13649
Sudha247 wants to merge 1 commit intoocaml:mainfrom
Sudha247:dev-tools-lockdir-scan

Conversation

@Sudha247
Copy link
Collaborator

While testing #13595 with OxCaml projects, I discovered that configuring lock directories within _build isn't working --

$ dune tools install ocamllsp
Internal error, please report upstream including the contents of _build/log.
Description:
  ("as_outside_build_dir_exn",
   { path =
       In_build_dir
         ".dev-tools.locks/ocaml-lsp-server/ocaml-compiler-libs.files"
   })
Raised at Stdune__Code_error.raise in file
  "otherlibs/stdune/src/code_error.ml", line 10, characters 30-62
Called from Dune_rules__Lock_rules.scan_lock_directory.scan in file
  "src/dune_rules/lock_rules.ml", line 430, characters 25-60
Called from Fiber__Core.apply2 in file "src/fiber/src/core.ml", line 92,
  characters 6-11
-> required by ("gen-rules", In_build_dir "_private/default/.lock")
-> required by ("load-dir", In_build_dir "_private/default/.lock")
-> required by
   ("build-file", In_build_dir "_private/default/.lock/dune.lock")
-> required by ("<unnamed>", ())
-> required by ("toplevel", ())

I must not crash.  Uncertainty is the mind-killer. Exceptions are the
little-death that brings total obliteration.  I will fully express my cases. 
Execution will pass over me and through me.  And when it has gone past, I
will unwind the stack along its path.  Where the cases are handled there will
be nothing.  Only I will remain.

This patch adds support for configuring lock directories within _build. But the question also remains, is this the desirable way to configure dev tools or do we ask users to create a lock directory outside of _build.

Signed-off-by: Sudha Parimala <sudharg247@gmail.com>
@rgrinberg
Copy link
Member

I think your fix doesn't really work. It doesn't declare dependencies correctly on what is being read.

@Sudha247
Copy link
Collaborator Author

I think your fix doesn't really work. It doesn't declare dependencies correctly on what is being read.

You're right! I was discussing this with @Leonidas-from-XIV today, and we realised the same as well. This actually doesn't solve the problem because the dependencies become untracked. However, we are also unclear about the way forward. Do you have any ideas?

So, this part of the code is assuming the lock directory is outside the build directory, but the lock directory of the dev tools is within the build directory. Hence, adding config to the dev tools lock directory is triggering this bug. One idea @Leonidas-from-XIV suggested is we could perhaps copy the contents of the dev tools lock directory to somewhere outside the build temporarily and read from there, but we're not sure that's the best solution.

We also discovered this case is not covered in the test suite. I'll add it in a separate PR.

@Leonidas-from-XIV
Copy link
Collaborator

To clarify, my suggestion was to list all files of the lock directory in the lock directory in a known location (lock.dune or a known file) at creation time and keep it in _build, thus there would be no need to scan in _build, thus no need for Path.Untracked.readdir.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants