opam
opam copied to clipboard
[WIP] Move most of the external tools calls to a single module
This PR aims at improving the way of describing and looking for external programs calls. This should also ease portability checks for Windows (cc @dra27) and minimal distributions such as Alpine Linux for example by checking which packages to install, what options are not available on the distribution/OS you want to port opam on.
Related to #3197
This is WIP because of some assert false to fill up in src/format/opamFile.ml and calls to OpamExternalTools.custom for Git/Hg/Darcs/…
Also I would know if the change to the API is ok before continuing further.
Thanks. I would have liked to discuss this beforehand, but I think it's a good idea.
A few comments:
- please update doc/index.html with the new module
- the name
OpamExternalToolsfits the contents of the module well. However, it feels weird that e.g.OpamSystem.make_commandnow relies onOpamExternalTools.t(andOpamExternalTools.custom) when its principal use is to call the commands from package scripts (e.g. the contents of thebuild:andinstall:fields, seeOpamAction.make_command). I would also prefer thatOpamProcesswouldn't depend on the new module at all, since it doesn't rely on external commands (it'd be nice to avoid the dependency forOpamStd, too, but I don't see how to achieve that easily). - maybe there could be a distinction between tools that opam relies on (
cp,mv,tar,patch,git, etc.), and the ones that are just polled at some point (for example, while I think this is generally a great improvement in readability, in the specific case ofOpamSysPoll, where commands are just checked once, and are not always expected to exist, the benefit is not clear. It's no longer obvious thatgetpropis only ever useful to detect an Android device, for example)
These are details, but gathering the commands like this is a good thing and changing from a list to a cmd, args pair is also a good change (that I am sure you noticed had been done for the low-level command engine, but without reaching most calling points).
One last thing, if you find the combination of OpamSystem and OpamFilename awkward, I totally agree... this might be the last part I didn't get around to rewriting since 1.0, and the OpamFilename types are not particularly convenient, while adding a redundant layer of indirection. So I'd be happy to discuss their replacement if you're so inclined :) Ideally, but this would be a much bigger change, we should have one module with basic definitions (e.g. file names) and system operations, then OpamExternalTools, then one based exclusively on our file name types, and defining the operations like cp and mv just once.
Also in the medium term, it would be good to start vendoring in things like ocaml-tar or ocaml-git to reduce the dependency on external tools by default. That might affect the consideration of names for this module...