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

Add library linking proposal. #7

Merged
6 commits merged into from
Apr 7, 2020
Merged

Add library linking proposal. #7

6 commits merged into from
Apr 7, 2020

Conversation

dbuenzli
Copy link
Contributor

Some discussion already happened on this gist.

@gasche
Copy link
Member

gasche commented Nov 20, 2019

Thanks for sending it here, I was meaning to invite you to submit anyway.

A link to the rendered RFC (instead of its source) is
https://github.com/dbuenzli/RFCs/blob/master/rfcs/ocamlib.md

I wonder whether and how you changed the document following the feedback on your gist (which I'm sure comes after copious rounds of internal feedback on another document.)

Could you point out explicitly, in the "Compilation phase" description of ocaml{c,opt}, that type-checking does not need to read/access .cma files?

The idea of not-seeing the directories and compiled objects of transitive dependencies is for now the most controversial part of the RFC. Could you add a section somewhere that discusses this specifically, explaining the rationale/intention for this proposal, and listing the objections against it? (It could also be a place to list alternative approaches to this part of the proposal, if interesting ones have been proposed.)

@dbuenzli
Copy link
Contributor Author

dbuenzli commented Nov 20, 2019

I added an "unresolved issues" section.

aliases and module type aliases.
2. Add a `--lib-hidden LIB` option that tells the compiler to also
look into `LIB` for `cmis` but without directly exposing the
declarations to the source files that are being compiled.
Copy link
Member

Choose a reason for hiding this comment

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

The approach I had in mind in my comment, and I believe also in Leo's followup was rather to add a --hiden-cmi foo.cmi or -I-hidden foo/ option to the compiler (the later, more global approach allows us to decide to also exfiltrate certain information from .cmx files for example, or we may also need a --hiden-cmx foo.cmx option), and then have transitive dependencies of --lib arguments be turned into -I-hidden <transdeps> on the command-line instead of nothing at all. The intent to hide would be implicit in the use of the -lib flag (for its transitive dependencies that are not also direct dependencies), instead of having to be passed explicitly by the user. (It's not clear to me whether you intend the --lib-hidden flag to be passed by the user explicitly, or by the compiler implicitly from -lib arguments).

In the presence of this logic, indeed the compiler needs to know of transitive dependencies also during the compilation file, and indeed currently that means reading .cma file, which was a worry of Alain. I believe that the existence of this design choice (not yet resolved) might justify the idea of storing library dependencies in other intermediate files than .cma in addition, here in both .cmi and .cmx. Having the information in those files (is not very hard to implement and) extends our design/decision space for what we want to do on transitive dependencies, so it sounds like a safer design to me.

rfcs/ocamlib.md Outdated Show resolved Hide resolved
rfcs/ocamlib.md Show resolved Hide resolved
Copy link
Member

@gasche gasche left a comment

Choose a reason for hiding this comment

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

I believe that the RFC is now in a satisfying state and I'm not personally planning to request additional changes to it. At our last maintainer meeting, there was broad support for the proposal, but it has not been formally approved -- people where not informed enough of the proposal for that to make much sense. I think it has a broad consensus and we could consider starting to work along the lines proposed.

I still have the impression that we should use a slightly different design where dependencies are known at compile time (preferably not fully visible, but making it possible to expose them in some partial form), so I would personally prefer if .cmi files did record library dependencies in addition to .cma files. I think there will be further iterations on this point, in the proposal or when people get to implement that part.

As a first step, I hope to find time in the short future to do a proper implementation review of the OCAMLPATH PR ( ocaml/ocaml#8946 ) -- of course anyone else should feel free to do the same.

@ghost
Copy link

ghost commented Nov 26, 2019

Just reporting something that we discussed on slack:

The original proposal was to use / as a separator in library names, i.e. the library lwt.unix would become lwt/unix. From the viewpoint of the compiler, this proposal is new and using / makes sense. However from the viewpoint of users libraries have been supported for many years via ocamlfind. Migrating from . to / would cause a lot of churn with no benefit for end users. It could also cause confusion when namespaces are introduced in the future. Older projects would still use lwt.unix to refer to lwt.unix as a library while newer ones would use lwt.unix to refer to it as a namespace. In general, reusing an old name with a different semantic is not great since not everybody follows the evolution of the ecosystem at the same pace. In the end we agreed that it is simpler to stick to ..

@lpw25
Copy link
Contributor

lpw25 commented Nov 26, 2019

@diml Please make sure such adjustments get made to the document in the PR as well as mentioned in the discussion.

@dbuenzli
Copy link
Contributor Author

I'll make the appropriate amendments when I get a bit of time.

As a first step, I hope to find time in the short future to do a proper implementation review of the OCAMLPATH PR ( ocaml/ocaml#8946 ) -- of course anyone else should feel free to do the same.

Note that formally there's no deep need to have this and we could eschew it. It simply redefines the notion of +DIR as far as the compiler is concerned. Though it could allow distributions to shift their install structure from the current state where they install ocaml packages in ocamlc -where.

@rgrinberg
Copy link
Member

Does this proposal overlap in any way with the namespace proposal?

@dbuenzli
Copy link
Contributor Author

@rgrinberg sorry, I missed your question. It is well aligned on the namespace proposal and a good step towards it. It shares the same file system structure, naming convention and lookup mecanisms.

It slightly differs in that the lookups are simpler, library names are not reified in the language and there's no attempt to mangle compilation unit identifiers to avoid cross-library name clashes.

@rgrinberg
Copy link
Member

Thanks for the response.

Is it out of scope for this proposal to formalize the linking hack? It's quite old, well understood, and as far as I can tell, even the compiler uses it. Recently, it has been made accessible to a wide range of users in dune under the virtual libraries feature. Perhaps it's time to standardize it?

@dbuenzli
Copy link
Contributor Author

Is it out of scope for this proposal to formalize the linking hack?

Yes. We discussed it with members of the proposal and decided to keep it out. It's still a bit unclear whether the technique is useful at scale (e.g. once you tell people they won't benefit of cross-module optimizations they may actually prefer to have multiple install prefixes (opam switches) with different installs of the same library, rather than co-installed library implementations). In any case it's better to move in small steps.

In particular adds information about `-linkall` implementation and
Dynlink.
@ghost ghost mentioned this pull request Mar 19, 2020
@ghost
Copy link

ghost commented Apr 7, 2020

I'm merging this RFC as it has been accepted. Additionally, the implementation raised some issues that require changing the RFC, which we will do via subsequent PRs.

@ghost ghost merged commit 1eea97a into ocaml:master Apr 7, 2020
@lpw25
Copy link
Contributor

lpw25 commented Apr 15, 2020

I'm merging this RFC as it has been accepted.

There was indeed approval of the idea in principle at the last meeting, but in future I would prefer there to be specific agreement amongst caml-devel to accept the RFC before the corresponding PR is merged.

It was not clear from the last meeting that people necessarily agreed with the finer details in the RFC document, and so general agreement with the broad idea might not have meant that they thought the RFC was ready for accepting. In this case I think everyone is fine with it, but if people are a bit more cautious on pressing merge then we can avoid that possibility on future RFCs.

@dbuenzli
Copy link
Contributor Author

More practically: I wish it would not have been merged as it gives a centralized place to report progress on actual implementation and/or unforseen challenges. Maybe it would better to have an approved tag and merge when it's done ?

@gasche
Copy link
Member

gasche commented Apr 15, 2020

@diml merged to make it easier to propose changes to the RFC as further PRs. (Not to signify approval, if I understand correctly.) In the future if we want to do this without merging, we should push the RFC branch to the central repository and then create PRs against that branch.

@lpw25
Copy link
Contributor

lpw25 commented Apr 15, 2020

In the future if we want to do this without merging, we should push the RFC branch to the central repository and then create PRs against that branch.

That's a great idea. Probably worth doing for any long running RFC that multiple people are collaborating on.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants