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

Remove the light mode #332

Merged
merged 3 commits into from
May 29, 2024
Merged

Remove the light mode #332

merged 3 commits into from
May 29, 2024

Conversation

hhugo
Copy link
Collaborator

@hhugo hhugo commented May 29, 2024

The light mode is not installed anyway.
The change simplify the code, remove the "duplicated" implementation of "implem" and remove the degraded mode.

In practice, the content of ocamlbuild_unix_plugin.ml was moved inside my_unix.ml.

@kit-ty-kate
Copy link
Member

Previously the interface for Ocamlbuild_executor and Ocamlbuild_unix_plugin would have been installed. Were they used or accessible from anywhere?

Here is the diff in the installed files:

--- /tmp/before	2024-05-29 13:01:07.088805668 +0100
+++ /tmp/after	2024-05-29 13:01:13.373818218 +0100
@@ -1,9 +1,6 @@
 META
 ocamlbuild.cmo
 ocamlbuild.cmx
-ocamlbuild_executor.cmi
-ocamlbuild_executor.cmx
-ocamlbuild_executor.o
 ocamlbuildlib.a
 ocamlbuildlib.cma
 ocamlbuildlib.cmxa
@@ -13,9 +10,6 @@
 ocamlbuild_plugin.cmi
 ocamlbuild_plugin.cmx
 ocamlbuild_plugin.o
-ocamlbuild_unix_plugin.cmi
-ocamlbuild_unix_plugin.cmx
-ocamlbuild_unix_plugin.o
 opam
 signatures.cmi
 signatures.cmti

@gasche
Copy link
Member

gasche commented May 29, 2024

The light mode can go away. I think it was meant to make it possible to build the compiler distribution with ocamlbuild (the original goal) and work well in semi-bootstrapped, Unix-less environments. I don't think that any user of ocamlbuild needs this anymore.

On the other hand, the use of the plugin to build your own ocamlbuild "shim" is something that some people might be doing still, I am not sure. (Maybe it is possible to analyze opam to find out. I remember an impressive ocamlbuild plugin for building C projects, was/is it using this mechanism?) So preserving the list of available modules in the plugin, as @kit-ty-kate, sounds like a safer approach.

@hhugo
Copy link
Collaborator Author

hhugo commented May 29, 2024

Previously the interface for Ocamlbuild_executor and Ocamlbuild_unix_plugin would have been installed. Were they used or accessible from anywhere?

Here is the diff in the installed files:

--- /tmp/before	2024-05-29 13:01:07.088805668 +0100
+++ /tmp/after	2024-05-29 13:01:13.373818218 +0100
@@ -1,9 +1,6 @@
 META
 ocamlbuild.cmo
 ocamlbuild.cmx
-ocamlbuild_executor.cmi
-ocamlbuild_executor.cmx
-ocamlbuild_executor.o
 ocamlbuildlib.a
 ocamlbuildlib.cma
 ocamlbuildlib.cmxa
@@ -13,9 +10,6 @@
 ocamlbuild_plugin.cmi
 ocamlbuild_plugin.cmx
 ocamlbuild_plugin.o
-ocamlbuild_unix_plugin.cmi
-ocamlbuild_unix_plugin.cmx
-ocamlbuild_unix_plugin.o
 opam
 signatures.cmi
 signatures.cmti
  • ocamlbuild_unix_plugin was simply removed. It only exported a val setup : unit -> unit used to build the ocamlbuild binary. I found one usage outside ocamlbuild in https://github.com/ocsigen/eliom/blob/master/build/bud.ml which create a custom ocamlbuild binary with custom rules. We can either export a dummy ocamlbuild_unix_plugin module that does nothing or fix the code in eliom.

  • ocamlbuild_executor is now included in Ocamlbuild_pack, I couldn't find any usage outside ocamlbuild.

@hhugo
Copy link
Collaborator Author

hhugo commented May 29, 2024

I've restored the ocamlbuild_unix_plugin module ! Do we really need to restore the ocamlbuild_executor one ?

@gasche
Copy link
Member

gasche commented May 29, 2024

My impression is that it is probably safe to assume that no one explicitly relies on accessing Ocamlbuild_executor directly as a plugin module. (It would be nice to have a pool of ocamlbuild-using projects to test this on, but I am too lazy to consider doing something like that. We will get opam-repository testing on the next release, but by then it will be a bit late.) I am in favor of moving to merge this, if @kit-ty-kate also agrees.

@kit-ty-kate
Copy link
Member

I used both https://sherlocode.com/ and opam grep and only found:

bsbnative matches your regexp.
ocamlbuild matches your regexp.
rmlbuild matches your regexp.

all of which are forks of ocamlbuild, so i think this is safe to remove Ocamlbuild_executor

@gasche gasche merged commit 660a6ae into ocaml:master May 29, 2024
20 checks passed
@gasche
Copy link
Member

gasche commented May 29, 2024

Thanks! Merged.

@hhugo hhugo deleted the no-light-more branch May 29, 2024 16:38
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