Skip to content

[coq] Deprecate public_name field in favor of package.#2087

Merged
rgrinberg merged 1 commit intoocaml:masterfrom
ejgallego:coq+deprecate_public_name
Feb 9, 2020
Merged

[coq] Deprecate public_name field in favor of package.#2087
rgrinberg merged 1 commit intoocaml:masterfrom
ejgallego:coq+deprecate_public_name

Conversation

@ejgallego
Copy link
Copy Markdown
Collaborator

In the current model, Coq theory names can be of the form A.b.C,
which does mean that modules do live under that module prefix.

This poses a problem for handling public_name in a sensible way if
we want to allow globally-installed theories to satisfy local
dependencies.

Imagine the following theory declaration:

(coq.theory
 (name mow)
 (theories foo.bar))

How should we satisfy foo.bar? Indeed, there is an ambiguity, does
it mean, "theory bar in package foo" or "theory bar.foo" in the
current package.

In particular, we could have a local stanza of the following form:

(coq.theory
 (name bar)
 (public_name foo.bar))

but then it would not be stable under vendoring, as this library would
be installed in user-contrib/bar. And moreover, it is ambiguous
with:

(coq.theory
 (name foo.bar))

One solution is to forbid the use of public_name and prefer
package instead. This way, libraries live under a global namespace
and the problem is solved.

However, this is not very satisfying, and the problem in future
versions of the language would still remain. Thus I am not very
convinced by this solution; suggestions are welcome.

This is a pre-requisite for #2053

@ejgallego ejgallego added the coq label Apr 25, 2019
@ejgallego ejgallego force-pushed the coq+deprecate_public_name branch 2 times, most recently from 50a9ea4 to 660b9b2 Compare April 25, 2019 16:46
@ejgallego
Copy link
Copy Markdown
Collaborator Author

cc: @Zimmi48 ; for future version of the coq language we could indeed drop compatibility with Coq makefile, then installing the corresponding dune-package files would allow us to properly recreate a package database.

Even so, the question on whether we should allow libraries names of the form foo.bar.mow where foo is a package is still unclear, maybe we should use for Coq a form foo+bar.mow or even foo.lib+bar.mow, or maybe separate package name from the point of insertion in the module hierarchy?

I dunno, the form of namespacing as used in the OCaml world should maybe something that Coq itself should be aware of.

@Zimmi48
Copy link
Copy Markdown
Contributor

Zimmi48 commented Apr 26, 2019

I think it is OK to be stricter on what we accept. A Dune package should be packed under a single namespace, and this is already what is conventionally done in Coq packages.
I don't understand what you mean by "drop compatibility with Coq makefile". It is important to be able to depend on libraries / plugins that were built and installed with coq_makefile obviously, and it seems also that it should be possible to make a Coq package installable in the same way with Dune and coq_makefile (assuming it follows some restrictions).

@Zimmi48 Zimmi48 mentioned this pull request Apr 26, 2019
4 tasks
@ejgallego
Copy link
Copy Markdown
Collaborator Author

ejgallego commented Apr 26, 2019

I don't understand what you mean by "drop compatibility with Coq makefile".

In the 2.0 version of the language we could allow fancier rules; the current rules for Coq makefile install everything under a global directory [which can be several if you use COQPATH, but that is still one for Coq].

This is a bit inconvenient, as for example there are no proper ways to compartmentalize nested libs.

Something that bothers me is that we identifiy the module namespace with the library name. IMHO these are two conceptually different things, however I am not sure under this "1.0" scheme we can do better; after all, all that Dune can query is the directory tree on lib/coq/user-contrib

So yeah, even in the version 1.0 of the language it is not clear what a library identifier should be.

For example, using this convention we cannot install two libraries that provide the module namespace Data. This is IMO very limiting.

I think that a good start for a "2.0" version of the Coq language would be to adopt a 2-level similar naming scheme as in OCaml.

Thus, a library that is declared pkg.theory would be installed under lib/pkg/theory.

In the end I dunno, it is hard to know how things are going to work out until we don't try more; we don't have a lot of experience in this Coq context I'm afraid.

@ejgallego ejgallego force-pushed the coq+deprecate_public_name branch from 660b9b2 to 2e1f0eb Compare April 26, 2019 18:17
@Zimmi48
Copy link
Copy Markdown
Contributor

Zimmi48 commented Apr 26, 2019

How can you, in OCaml, depend on two packages that provide modules under the same namespace? I personally feel like this is a very good practice to use namespaces that are reminiscent of the package name for less ambiguity. If you want a Data namespace, it can be nested, and with the right open or Import you can talk about it as Data.

@ejgallego
Copy link
Copy Markdown
Collaborator Author

How can you, in OCaml, depend on two packages that provide modules under the same namespace? I personally feel like this is a very good practice to use namespaces that are reminiscent of the package name for less ambiguity. If you want a Data namespace, it can be nested, and with the right open or Import you can talk about it as Data.

Oh, then we can go with this proposal to identify package name with its namespace for now.

In OCaml you cannot depend on two packages that provide the same namespace, but you can have indeed have virtual modules so for example, for namespace Foo you can have libraries foo-unix and foo-windows for example that will provide Foo.

That works because they are properly installed, however in the current Coq setup we cannot do that with the global namespace that is forced. The moment we break this discipline dune-produced packages cannot be use by coq-makefile users easily.

Copy link
Copy Markdown
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

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

The implementation looks fine to me. I have nothing to recommend, but it certainly seems like namespaces and library names are separate concepts. Regarding the installation and keeping namespaces "open", perhaps the OCaml namespace proposal for some inspiration? https://github.com/lpw25/namespaces

@Zimmi48
Copy link
Copy Markdown
Contributor

Zimmi48 commented Apr 27, 2019

In OCaml you cannot depend on two packages that provide the same namespace, but you can have indeed have virtual modules so for example, for namespace Foo you can have libraries foo-unix and foo-windows for example that will provide Foo.

That works because they are properly installed, however in the current Coq setup we cannot do that with the global namespace that is forced. The moment we break this discipline dune-produced packages cannot be use by coq-makefile users easily.

Good point. I now understand what you mean by breaking compatibility and how we could wish to follow this route in the future. Virtual libraries are a nice concept that I would hope to support in the future (but we are far from it as we don't have interface files yet).

@ejgallego ejgallego force-pushed the coq+deprecate_public_name branch from 2e1f0eb to 767b349 Compare April 27, 2019 15:37
@ejgallego
Copy link
Copy Markdown
Collaborator Author

ejgallego commented Apr 27, 2019

Virtual libraries are a nice concept that I would hope to support in the future (but we are far from it as we don't have interface files yet).

Yeah maybe virtual libraries are not the best name, but indeed with the current compat constraints we cannot do better. It is very unfortunate we cannot allow a better install for now.

I am not very comfortable going this way, but as long as we want to support the legacy install method I guess we'll have to leave with this. So let's make the 1.0 version use module names as namespaces and see what kind of workflow does people have.

In fact, as soon as Coq itself is moved to dune we could have the 1.1 version of the language with slightly more sensible rules.

@ejgallego ejgallego force-pushed the coq+deprecate_public_name branch from 767b349 to 30e2400 Compare April 29, 2019 01:24
@ejgallego ejgallego force-pushed the coq+deprecate_public_name branch 3 times, most recently from 1bb903e to 2beacc5 Compare July 3, 2019 01:41
@ejgallego ejgallego force-pushed the coq+deprecate_public_name branch from 2beacc5 to 37a658f Compare July 23, 2019 10:05
@ejgallego ejgallego force-pushed the coq+deprecate_public_name branch from 37a658f to 5f48078 Compare February 6, 2020 22:29
@ejgallego ejgallego requested a review from nojb as a code owner February 6, 2020 22:29
@ejgallego ejgallego force-pushed the coq+deprecate_public_name branch 3 times, most recently from bf32712 to bbad79d Compare February 9, 2020 17:59
Copy link
Copy Markdown
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

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

Reviewed again and it looks fine.

@ejgallego
Copy link
Copy Markdown
Collaborator Author

ejgallego commented Feb 9, 2020

Ready to merge IMO, thanks @rgrinberg

In the current model, Coq theory names can be of the form `A.b.C`,
which does mean that modules do live under that module prefix.

This poses a problem for handling `public_name` in a sensible way if
we want to allow globally-installed theories to satisfy local
dependencies.

Imagine the following theory declaration:

```lisp
(coq.theory
 (name mow)
 (theories foo.bar))
```

How should we satisfy `foo.bar`? Indeed, there is an ambiguity, does
it mean, "theory `bar` in package `foo`" or "theory `bar.foo`" in the
current package.

In particular, we could have a local stanza of the following form:

```
(coq.theory
 (name bar)
 (public_name foo.bar))
```

but then it would not be stable under vendoring, as this library would
be installed in `user-contrib/bar`. And moreover, it is ambiguous
with:
```
(coq.theory
 (name foo.bar))
```

One solution is to forbid the use of `public_name` and prefer
`package` instead. This way, libraries live under a global namespace
and the problem is solved.

However, this is not very satisfying, and the problem in future
versions of the language would still remain. Thus I am not very
convinced by this solution; suggestions are welcome.

This is a pre-requisite for ocaml#2053

Signed-off-by: Emilio Jesus Gallego Arias <e+git@x80.org>
@ejgallego ejgallego force-pushed the coq+deprecate_public_name branch from bbad79d to e27089e Compare February 9, 2020 20:24
@rgrinberg rgrinberg merged commit 7d6f282 into ocaml:master Feb 9, 2020
@ejgallego ejgallego deleted the coq+deprecate_public_name branch February 9, 2020 21:11
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Feb 15, 2020
…lugin, dune-private-libs and dune-glob (2.3.0)

CHANGES:

- Improve validation and error handling of arguments to `dune init` (ocaml/dune#3103, fixes
  ocaml/dune#3046, @shonfeder)

- `dune init exec NAME` now uses the `NAME` argument for private modules (ocaml/dune#3103,
  fixes ocaml/dune#3088, @shonfeder)

- Avoid linear walk to detect children, this should greatly improve
  performance when a target has a large number of dependencies (ocaml/dune#2959,
  @ejgallego, @aalekseyev, @Armael)

- [coq] Add `(boot)` option to `(coq.theories)` to enable bootstrap of
  Coq's stdlib (ocaml/dune#3096, @ejgallego)

- [coq] Deprecate `public_name` field in favour of `package` (ocaml/dune#2087, @ejgallego)

- Better error reporting for "data only" and "vendored" dirs. Using these with
  anything else than a strict subdirectory or `*` will raise an error. The
  previous behavior was to just do nothing  (ocaml/dune#3056, fixes ocaml/dune#3019, @voodoos)

- Fix bootstrap on bytecode only switches on windows or where `-j1` is set.
  (ocaml/dune#3112, @xclerc, @rgrinberg)

- Allow `enabled_if` fields in `executable(s)` stanzas (ocaml/dune#3137, fixes ocaml/dune#1690
  @voodoos)

- Do not fail if `ocamldep`, `ocamlmklib`, or `ocaml` are absent. Wait for them
  to be used to fail (ocaml/dune#3138, @rgrinberg)

- Introduce a `strict_package_deps` mode that verifies that dependencies between
  packages in the workspace are specified correctly. (@rgrinberg, ocaml/dune#3117)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Feb 15, 2020
…lugin, dune-private-libs and dune-glob (2.3.0)

CHANGES:

- Improve validation and error handling of arguments to `dune init` (ocaml/dune#3103, fixes
  ocaml/dune#3046, @shonfeder)

- `dune init exec NAME` now uses the `NAME` argument for private modules (ocaml/dune#3103,
  fixes ocaml/dune#3088, @shonfeder)

- Avoid linear walk to detect children, this should greatly improve
  performance when a target has a large number of dependencies (ocaml/dune#2959,
  @ejgallego, @aalekseyev, @Armael)

- [coq] Add `(boot)` option to `(coq.theories)` to enable bootstrap of
  Coq's stdlib (ocaml/dune#3096, @ejgallego)

- [coq] Deprecate `public_name` field in favour of `package` (ocaml/dune#2087, @ejgallego)

- Better error reporting for "data only" and "vendored" dirs. Using these with
  anything else than a strict subdirectory or `*` will raise an error. The
  previous behavior was to just do nothing  (ocaml/dune#3056, fixes ocaml/dune#3019, @voodoos)

- Fix bootstrap on bytecode only switches on windows or where `-j1` is set.
  (ocaml/dune#3112, @xclerc, @rgrinberg)

- Allow `enabled_if` fields in `executable(s)` stanzas (ocaml/dune#3137, fixes ocaml/dune#1690
  @voodoos)

- Do not fail if `ocamldep`, `ocamlmklib`, or `ocaml` are absent. Wait for them
  to be used to fail (ocaml/dune#3138, @rgrinberg)

- Introduce a `strict_package_deps` mode that verifies that dependencies between
  packages in the workspace are specified correctly. (@rgrinberg, ocaml/dune#3117)

- Make sure the `@all` alias is defined when no `dune` file is present
  in a directory (ocaml/dune#2946, fix ocaml/dune#2927, @diml)
mgree pushed a commit to mgree/opam-repository that referenced this pull request Feb 19, 2020
…lugin, dune-private-libs and dune-glob (2.3.0)

CHANGES:

- Improve validation and error handling of arguments to `dune init` (ocaml/dune#3103, fixes
  ocaml/dune#3046, @shonfeder)

- `dune init exec NAME` now uses the `NAME` argument for private modules (ocaml/dune#3103,
  fixes ocaml/dune#3088, @shonfeder)

- Avoid linear walk to detect children, this should greatly improve
  performance when a target has a large number of dependencies (ocaml/dune#2959,
  @ejgallego, @aalekseyev, @Armael)

- [coq] Add `(boot)` option to `(coq.theories)` to enable bootstrap of
  Coq's stdlib (ocaml/dune#3096, @ejgallego)

- [coq] Deprecate `public_name` field in favour of `package` (ocaml/dune#2087, @ejgallego)

- Better error reporting for "data only" and "vendored" dirs. Using these with
  anything else than a strict subdirectory or `*` will raise an error. The
  previous behavior was to just do nothing  (ocaml/dune#3056, fixes ocaml/dune#3019, @voodoos)

- Fix bootstrap on bytecode only switches on windows or where `-j1` is set.
  (ocaml/dune#3112, @xclerc, @rgrinberg)

- Allow `enabled_if` fields in `executable(s)` stanzas (ocaml/dune#3137, fixes ocaml/dune#1690
  @voodoos)

- Do not fail if `ocamldep`, `ocamlmklib`, or `ocaml` are absent. Wait for them
  to be used to fail (ocaml/dune#3138, @rgrinberg)

- Introduce a `strict_package_deps` mode that verifies that dependencies between
  packages in the workspace are specified correctly. (@rgrinberg, ocaml/dune#3117)

- Make sure the `@all` alias is defined when no `dune` file is present
  in a directory (ocaml/dune#2946, fix ocaml/dune#2927, @diml)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants