Skip to content

[Artifact] Move the artifact evaluation to its own private library#3330

Closed
bobot wants to merge 1 commit intoocaml:masterfrom
bobot:artifact_evaluation_as_library
Closed

[Artifact] Move the artifact evaluation to its own private library#3330
bobot wants to merge 1 commit intoocaml:masterfrom
bobot:artifact_evaluation_as_library

Conversation

@bobot
Copy link
Copy Markdown
Collaborator

@bobot bobot commented Apr 3, 2020

  • Adds an abstract type when we want to delay the decoding

From #3104, the evaluation is reused for sites_locations.

  * Adds an abstract type when we want to delay the decoding

Signed-off-by: François Bobot <francois.bobot@cea.fr>
@bobot bobot force-pushed the artifact_evaluation_as_library branch from 99c2880 to 267a9b9 Compare April 3, 2020 22:10
@bobot bobot requested a review from a user April 4, 2020 14:57
(data_module build_info_data)
(api_version 1))))
(api_version 1)))
(libraries dune-private-libs.artifact-eval)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatting is off here. Should be fixed with $ make fmt.

@rgrinberg
Copy link
Copy Markdown
Member

Why does it need a separate library? Is it possible to add this as a private module to the build info library?

@ghost
Copy link
Copy Markdown

ghost commented Apr 7, 2020

BTW, we recently decided against public libraries of dune depending on private things. For instance, we plan to extract dune-configurator into a separte repo and make sure it doesn't depend on dune-private-libs.

This means that if we want to extract this code into a library that is shared between dune and a public library, this library itself needs to be stabilised and extract as a public component. This feels a little overkill given that the code is pretty small. In particular, here it seems fine to me to duplicate the code. In particular, we are not going to change the format of the holes.

@bobot
Copy link
Copy Markdown
Collaborator Author

bobot commented Apr 20, 2020

Ok. I don't want this library to be public. My point was more that it is easier to code in real module than inside strings. Can you point me to the discution about private things? So that I can understand the rational and extent. Thanks.

@bobot bobot closed this Apr 20, 2020
@ghost
Copy link
Copy Markdown

ghost commented Apr 20, 2020

Agreed about coding in a real module. We could do the same thing as what Rudi did for OCaml files in OCaml syntax (see rule for src/dune/assets.ml).

Can you point me to the discution about private things? So that I can understand the rational and extent. Thanks.

This was the result of many discussions. You can search for "let's put a simple model in place" on the dune-dev channel for some context. The model is:

  • if something lives inside the dune repo, its versioning is synchronised with dune
  • to desynchronise a component, allow more combinations and make things more flexible, we must extract it into a separate repository
  • when extracted into a separate repository:
    • the interface between dune and the component must be minimal, explicit and versioned
    • the previous point implies that the separate repo cannot use dune private libraries, as this would increase the interface between dune and the separate component, make it implicit and unversioned
    • if we really need a dune private functionality such as module of Stdune or some other private library, this functionality itself must be stabilised and extracted into a separate component

The rationale is to keep things simple. If every public component is an entity of its own that only relies on the public API of other components, then it is easy to understand the relationship between the various components and make things flexible.

For instance, with the current state of things it is not possible for users to:

  • use recent dune features
  • use dune-configurator
  • use an OCaml compiler older than 4.07

And we didn't realise this until the problem actually occurred. The reason is indeed obscure: using a recent dune requires using a recent dune-configurator (because we changed the interface between dune and dune-configurator recently). Using a recent dune-configurator requires using a recent version of the compiler. First because the policy of being compatible with OCaml >= 4.07 applies to the whole repository and it would be difficult and confusing to apply different policies to different parts of the repo. And secondly because dune-configurator shares the same dune-project file as dune and so depends on a version of dune that is at least the version specified in the (lang dune x.y).

Once we have properly and cleanly split things off, the model will be simple and we will be able to apply different policies for different components, and in particular be compatible with different sets of OCaml versions.

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.

2 participants