Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parsing/generation of installed dune files #909

Closed
ghost opened this issue Jun 22, 2018 · 4 comments
Closed

Fix parsing/generation of installed dune files #909

ghost opened this issue Jun 22, 2018 · 4 comments
Assignees
Milestone

Comments

@ghost
Copy link

ghost commented Jun 22, 2018

The current behaviour in master regarding installed dune files is not right as they are parsed using the new lexer. That means that if a user upgrades from jbuilder to dune, dune will fail when trying to read an installed dune file installed by jbuilder.

Parsing these files

Installed dune files start with: (dune <version>, where <version> is currently 1. Let's do the following to load these files:

  • we manually fetch 3 tokens using Sexp.Lexer.token and check that they are Lparen, Atom "dune", Atom s. If s is 1 we parse the rest of the file using Lexer.jbuild_token and set Stanza.syntax to (0, 0).
  • if it is 2 we parse the rest of the file using Lexer.token and set Stanza.syntax to (1, 0)

Versions of subsystems

Currently these files look like:

$ cat `ocamlfind query ppx_inline_test`/*.dune
(dune
 1
 ((inline_tests.backend
   1.0
   ((runner_libraries (ppx_inline_test.runner.lib))
    (flags
     (inline-test-runner
      ${library-name}
      -source-tree-root
      ${ROOT}
      -diff-cmd
      -))
    (generate_runner (echo "let () = Ppx_inline_test_lib.Runtime.exit ();;"))))))

Here the 1.0 was supposed to be the version of the sub-system. However, this doesn't correspond to the right thing anymore as the version of the inline test and ppx subsystems are aligned with the version of the dune language.

I have a proposal in mind where installed dune files will become similar to dune-project files, but it will be more work, so for now we should simply ignore this 1.0.

Generating these files

The syntax into which these files are generated should match the version of the file where the library is defined. So if the library was defined in a jbuild file, we should generate the file in version 1 (so jbuild syntax) if if the library was defined in a dune file, we should generate the file in version 2 (so dune syntax).

@ghost ghost added this to the 1.0.0 milestone Jun 22, 2018
@ghost ghost assigned rgrinberg Jun 22, 2018
@rgrinberg
Copy link
Member

Just curious about this:

Here the 1.0 was supposed to be the version of the sub-system. However, this doesn't correspond to the right thing anymore as the version of the inline test and ppx subsystems are aligned with the version of the dune language.

Is it just for simplicity? I can see an argument being made to make subsystems be versioned independently

@ghost
Copy link
Author

ghost commented Jun 25, 2018

The issue is the following: the versionning of the inline_tests and ppx subsystems is now in line with the versionning of the main dune language. This mean that for files installed by jbuilder the version of these subsystems should be 0.0 and for files installed by dune the version should be 1.0. However, at the moment the version in files installed by jbuilder is 1.0. I guess we could ignore the version in files starting with (dune 1 and not in newer ones.

The other proposal I have in mind is the following: replace the variable <libdir>/<pkg>/.../<lib>.dune files by a single <libdir>/dune-package one containing something like:

(lang dune-package 1.0)
(using foo 1.42)

(library
 ((name blah)
  (archives ((bytes x.cma) (native x.cmxa)))
  ...
  (sub_systems
   (inline_tests ...)
   (ppx.driver ....))))

Here the library stanza would be a direct representation of Lib.Info.t and the versionning would work exactly in the same way as for dune-project files. We would still generate and read META files, but when an installed library as a dune-package, we would read only this file rather than the META file. In particular, this should make it easier for us to implement complex features that require storing structured metadata in installed libraries.

@rgrinberg
Copy link
Member

The other proposal I have in mind is the following: replace the variable //.../.dune files by a single /dune-package one containing something like:

Are you missing a <pkg> in the second path? Otherwise, all the libraries are defined in 1 file?

I guess we could ignore the version in files starting with (dune 1 and not in newer ones.

I think it still makes sense to version subsystems individually given that we do end up getting requests to tweak them various ways. The inline tests for example ended up getting some (backwards compatible) features added later on.

@ghost
Copy link
Author

ghost commented Jun 29, 2018

Are you missing a in the second path? Otherwise, all the libraries are defined in 1 file?

Yes, that the idea, we could define them all in one file, no need to generate one file per library.

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

No branches or pull requests

1 participant