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 opam files to allow pinning #2207

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
9 participants
@lpw25
Copy link
Contributor

commented Dec 19, 2018

This PR adds two opam files to the distribution. These enable you to pin the compiler with opam 2. So you can do:

$ opam switch create . --empty
$ opam install .

to get a local opam switch with the current compiler installed from the source directory. This is useful for quickly testing packages with a modified compiler.

I cobbled the files together based on the ones in the main opam-repository. Possibly they could be simpler since we don't need to support things like system compilers, but these ones do at least work. Maybe @AltGr, @dra27 or @avsm have some thoughts on how they might be simplified.

I also added support for passing -j through to make, so opam install -j 16 . should run make with -j 16 to speed things up.

The files shouldn't need much maintenance -- they just need their version numbers updated when we change the number in the VERSION file.

@dra27

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2018

This is a very good idea!

Two changes for this GPR:

  • The ocaml-variants.opam file needs a typo.prune attribute in .gitattributes.
  • tools/release-checklist wants updating - when the VERSION file is amended, the opam file should be updated in the same commit. So after a new branch (e.g. 4.08), trunk needs the opam version bumping (but not the new version branch) and after a maintenance-release (e.g. 4.07.2), then version on the branch needs bumping and a new ocaml package needs to go to opam-repository.

I don't think it's necessary to have ocaml.opam here - I think it's better to add these versions to opam-repository. That means a new version of ocaml gets pushed to opam-repository on every branching and on every release (so right now this ocaml.4.08.0 should go to opam-repository; when 4.08 is actually branched, ocaml.4.09.0 should go straight to opam-repository and when 4.08.0 is actually released, ocaml.4.08.1 should go straight to opam-repository in the same pull request as the release itself). Does that sound correct, @AltGr?

@lpw25

This comment has been minimized.

Copy link
Contributor Author

commented Dec 19, 2018

The ocaml-variants.opam file needs a typo.prune attribute in .gitattributes

Could you explain why? All the other entries in .gitattributes have a comment explaining why they are needed, so I need to know in order to make the change.

tools/release-checklist wants updating

I'd like some input from @damiendoligez about whether he is happy for me to add more things to the checklist before I do that. The alternative is to treat this more like the dune files -- i.e. it would be my responsibility to maintain these files, so that I would make a PR to update them after the release was done.

@dra27

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2018

The ocaml-variants.opam file needs a typo.prune attribute in .gitattributes

Could you explain why? All the other entries in .gitattributes have a comment explaining why they are needed, so I need to know in order to make the change.

There others which don't (manual, emacs/COPYING, etc.). It can have typo.missing-header and typo.utf8 if you want, but it seemed quickest just to ignore it! More on the basis of "do you want Travis ever to complain about this file again" 🙂

tools/release-checklist wants updating

I'd like some input from @damiendoligez about whether he is happy for me to add more things to the checklist before I do that. The alternative is to treat this more like the dune files -- i.e. it would be my responsibility to maintain these files, so that I would make a PR to update them after the release was done.

IANDD obviously, but being able to pin specific commits of the compiler (it's not just your local directory - you'd be able to give a git URL for any commit after it's done which is potentially useful) so I certainly think we should "own" this one officially.

@gasche

This comment has been minimized.

Copy link
Member

commented Dec 20, 2018

I think it's a good idea to include this in the release list.

(I would rather apply "the dune logic" only for things that we fear may be maintenance burden, when they would be applied to people who did not volunteer for them and are not enthusiastic. I don't think editing this file would be a source of trouble.)

It would be nice if this approach could replace my opam-compiler-conf script, which sadly still does not work properly with opam2. What I miss is the ability to install directly by copying build artifacts from the build directory, instead of having a re-build happen in .opam.

@dra27

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2018

@gasche - there's the --assume-built switch which might help here?

@dra27

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2018

Although I'm not sure if that works with upgrades to pins...

@avsm

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

I don't think it's necessary to have ocaml.opam here - I think it's better to add these versions to opam-repository. That means a new version of ocaml gets pushed to opam-repository on every branching and on every release (so right now this ocaml.4.08.0 should go to opam-repository; when 4.08 is actually branched, ocaml.4.09.0 should go straight to opam-repository and when 4.08.0 is actually released, ocaml.4.08.1 should go straight to opam-repository in the same pull request as the release itself).

This sounds correct to me. Having just ocaml.4.08.0 in the opam-repository is fine, since it won't get selected unless there is an ocaml-variants or ocaml-system version that also matches. That will only exist via a pin to this or an external remote.

@Drup

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2018

I have used a similar file in the past, so I'm all for its inclusion in the distrib. I also agree that we should only have ocaml-variants.opam here.

@gasche One idea would be to tweak configure so that it doesn't force complete recompilation if the arguments are the same as before. Then, there is a bunch of options for opam install to manipulate build states. I think --inplace-build is the one we want here, as it will keep building directly in the source tree and install from there. --assume-build will just skip the building altogether.

@gasche

This comment has been minimized.

Copy link
Member

commented Dec 22, 2018

I don't have the time to deal with this for now, anyway. Anyone else (hopefully more familiar with the latest and greatest opam2 options) is welcome to lend a hand.

I would be happy to move forward to merge this PR, but I think that the excellent feedback from David in #2207 (comment) should be turned into changes.

@damiendoligez

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

@lpw25: No problem with a few more lines in release-checklist. The alternative seems wasteful of everyone's time.

@hhugo

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

This would have been helpful to test the upcoming 4.08

gretay-js added a commit to gretay-js/ocaml that referenced this pull request Mar 29, 2019

Address review comments
ocaml#2207 (comment)

Remove ocaml.opam.
Update release notes to change ocaml-variants.opam when VERSION changes.
Add .gitattribute prune.typo for ocaml-variants.opam.

gretay-js added a commit to gretay-js/ocaml that referenced this pull request Mar 29, 2019

Address review comments
ocaml#2207 (comment)

Remove ocaml.opam.
Update release notes to change ocaml-variants.opam when VERSION changes.
Add .gitattribute prune.typo for ocaml-variants.opam.
@TyOverby

This comment has been minimized.

Copy link

commented Apr 10, 2019

I've been using a slightly modified version of these files for doing tests with Js_of_ocaml, and it's been incredibly helpful.

Please let me know if there's anything I can do to help land this.

@gasche

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

The github cross-links seem to suggest that @gretay-js took another stab at this, so maybe we'll see an updated PR emerge. (I understand that Leo is on well-deserved vacations right now.) If someone else was willing to send an updated PR, that could also work.

@TyOverby you mention that you did slight modifications, should they also be included upstream? Would you like to share what those modifications are?

@gretay-js

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2019

Yes, github magically pick up the patch in which I addressed the comments in this PR.
I have been using it for a while. I can submit it as a new PR if we don't want to wait until Leo is back next week. This should be a patch against 4.08 branch, and a different version for 4.09 onto trunk.
Additionally, we need to add the corresponding versions of ocaml.opam files to the opam repository.

@gasche

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

I don't have an opinion for whether we should do it now or wait until Leo comes back -- I trust both of you to do a good job. There is user demand, so I guess moving faster would be appreciated; it's also a matter of whether you want to pick up the work.

Additionally, we need to add the corresponding versions of ocaml.opam files to the opam repository.

I don't understand, can you elaborate what you have in mind? I thought that having those development files locally for pinning did not require any change to the opam-repository. Or if the idea is that we cannot have ocaml-variants packages for 4.08 and 4.09 without also ocaml packages at those versions, isn't the solution to use https://github.com/ocaml/ocaml-beta-repository/ ?

To conclude, I think it would be nice (for whoever is working on this patch) to document how to use these files in our root HACKING.adoc file. (If we need to use the ocaml-beta-repository first, the commands to enable it would also be welcome.)

@gretay-js

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2019

@gasche yes, we need ocaml.opam for 4.08 in order to use ocaml-variants for 4.08, and this was discussed here: #2207 (comment)

@gasche

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

I see; this seems extremely related to the discussion we had with @AltGr and @avsm in ocaml/opam-repository#13751.

In any case we can merge the files within the compiler-distribution without waiting for myself to fully understand where is the best repository to have those files.

(@gretay-js : do you consider a HACKING.adoc addition to be within the scope of your new PR?)

gretay-js added a commit to gretay-js/ocaml that referenced this pull request Apr 10, 2019

gretay-js added a commit to gretay-js/ocaml that referenced this pull request Apr 10, 2019

gretay-js added a commit to gretay-js/ocaml that referenced this pull request Apr 10, 2019

gretay-js added a commit to gretay-js/ocaml that referenced this pull request Apr 10, 2019

@TyOverby

This comment has been minimized.

Copy link

commented Apr 11, 2019

@gasche: My minor changes were required for back-porting this file to 4.07, and they wouldn't be useful otherwise.

@gasche

This comment has been minimized.

Copy link
Member

commented Apr 13, 2019

Closed by further updates in #8604. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.