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

next: add package subcommand #2551

Closed
wants to merge 8 commits into from
Closed

next: add package subcommand #2551

wants to merge 8 commits into from

Conversation

seliopou
Copy link
Contributor

This pull request adds the package subcommand to OPAM, which exposes a command-line interface to lifecycle events already described by OPAM files. Specifically, this pull requests includes three subcommands:

  • opam package build runs the build script of the OPAM file in the current project;
  • opam package build-test runs the build-test script of the OPAM file in the current project; and
  • opam package build-doc runs the build-doc script of the OPAM file in the current project.

These commands will search an OPAM file in the typical locations, relative the current directory, e.g., opam, opam/opam, etc. In addition, the commands will also search up the directory tree for an opam file and if found, use that to determine which commands to run.

The opam package subcommands can be used in any context, but were developed alongside directory sandbox support included in #2550. In conjuction with #2250, users can develop OPAM packages in local sandbox, using the switch associated with that sandbox while leaving the globally-configured switch unchanged, and out of the picture.

This pull request targets the next branch of OPAM development.

As mentioned to @AltGr via email, this pull request intentionally does not include command-line documentation.

locate_ancestor will find the ancestor of the provided directory for
which the predicate returns true.
Allow users to call build_package on a directory containing a package
that is ready to be built. It seems as if the the function should really
perform the functionality exposed through the `In_place variant,
delegating the work for extracting the package to the caller. It is left
like this to minimize the change necessary for the implementation of the
local sandbox feature.
in
(* XXX(seliopou): This whole dance is necessary because
* OpamAction.build_package reads global state, rather than explicitly taking
* the build_test and build_doc arguments *)
Copy link
Member

Choose a reason for hiding this comment

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

Indeed. It would be best to add them as arguments troughout. Meanwhile, it may be more consistent to remove the argument here and relying on the options being set in OpamMain.

Note that since we would like to be able to set these options for specific packages (#2506), this will need to be changed anyway at that point.

@AltGr
Copy link
Member

AltGr commented May 17, 2016

Thanks a lot! This is great and many are looking forward to this feature.

I think you could go further though: I would expect opam package build foo to resolve the dependencies of the package before building it, resulting to approximately the same results as:

$ opam pin foo . --no-act
$ opam install --deps-only foo
$ opam config exec -- <build commands for foo>
$ opam unpin foo --no-act

For that you would need to rely on OpamClient.install.

Some more design questions:

  • You restrict the use of opam package install to local-switches. While the two definitely fit together, it may be useful even for centralised switches, do you think we could relax that constraint ?
  • Have you considered running the install commands too ? There may be some difficulties to solve (upgrading/recompiling ?), but it would be nice to be able to test them too.

@AltGr
Copy link
Member

AltGr commented May 17, 2016

Ah, also, this still seems to target the next branch.

The package subcommand is meant to work in any directory that contains a
valid package, not just in local sandboxes. As far as I can recall, the
only thing that tied the package subcommand to a local sandbox was this
error message.
@samoht
Copy link
Member

samoht commented Aug 17, 2016

Tentative initial doc. Feedback welcome: https://gist.github.com/samoht/2188fbd01b1d40a3e2063f3a270bf8fd

@AltGr
Copy link
Member

AltGr commented May 2, 2017

Superseded by opam build and the newer opam install <dir> interface. Thanks!

@AltGr AltGr closed this May 2, 2017
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.

None yet

3 participants