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 clangml.3.9.1.2 #9810

Merged
merged 3 commits into from
Jul 27, 2017
Merged

Add clangml.3.9.1.2 #9810

merged 3 commits into from
Jul 27, 2017

Conversation

thierry-martinez
Copy link
Contributor

No description provided.

@gasche
Copy link
Member

gasche commented Jul 16, 2017

The build fails on CentOS and Alpine CI machines. CentOS does not seem to have a clang package at all (or llvm), and Alpine does have clang packages, but only for even minor versions (3.6, 3.8, 4.0) -- while it does package llvm 3.9.

Could you add & os != "alpine" & os != "centos" to the available predicate?

In the future it would be also nice to reduce the depexts and use the community-maintained conf-* packages instead -- see #5791 -- cc @UnixJunkie as the upstream opam maintainer, but this can be done separately (we can also improve older releases as well). conf-binutils should be used instead of binutils. Some depexts also ask for ncurses, we have conf-ncurses, but I don't understand whether it is a dependency of this package for all systems ( @UnixJunkie? ). We don't have a conf-boost, but we should create one, and we should populate and use conf-llvm.

thierry-martinez added a commit to Antique-team/clangml that referenced this pull request Jul 16, 2017
thierry-martinez added a commit to Antique-team/clangml that referenced this pull request Jul 16, 2017
@thierry-martinez thierry-martinez changed the title Add clangml.3.9.1.1 Add clangml.3.9.1.2 Jul 16, 2017
@thierry-martinez
Copy link
Contributor Author

Thanks @gasche for your comments. I fixed the opam file according to your suggestions.

thierry-martinez added a commit to thierry-martinez/opam-repository that referenced this pull request Jul 16, 2017
Suggested by Gabriel Scherer.
ocaml#9810 (comment)
@gasche
Copy link
Member

gasche commented Jul 16, 2017

Thanks! This looks very nice -- I'm waiting for the CI reports.

My plan is to merge the current PR if the CI passes, and then come back to iterate on the opam metadata in the public repository to use more conf-* goodness, for example your conf-boost package if it is merged by then. I think it is reasonable because the conf-* packages are not specific to this clangml release: we can improve the installability/portability of older packaged versions as well by rewriting their opam metadata. (In particular, you don't actually need to do new upstream releases when this packaging metadata changes.)

P.S.: In your release message for 3.9.2, you announce that you are "dropping Antique", I suppose you only planned to drop Alpine ;-)

I started looking this morning at the conf-llvm state of affairs -- thanks for doing conf-boost yourself in #9851. I'll open a separate issue for this.

@thierry-martinez
Copy link
Contributor Author

Having a conf-llvm (and maybe even conf-clang) would be great. However, we should take care of the fact that each version of clangml.x.y is bound to the corresponding clang.x.y. Should we perhaps maintain different versions of conf-llvm/conf-clang packages?

@gasche
Copy link
Member

gasche commented Jul 16, 2017

We do have conf-llvm.3.4 and conf-llvm.3.8 today; my issue will get into way more details than this :-)

@avsm
Copy link
Member

avsm commented Jul 24, 2017

One big query point in this PR is the wget on osx. I see it's been done in previous versions as well, but it will break with opam2 and fetching distfiles outside of the url file breaks caching as well. Not a blocker to merge this one, but would be good to fix.

I don't see anything blocking this merge -- the previous releases follow the same format, and the CI could be fixed in a followup PR. Will leave to @gache to merge if he agrees.

@gasche
Copy link
Member

gasche commented Jul 24, 2017

Yes indeed, sorry for being slow on this. I thought that Thierry may want to make use of the new conf-llvm stuff (and I was expecting this to be merged sooner), but we could merge here and change things later if we want to improve the depext situation.

@gasche gasche merged commit 3865286 into ocaml:master Jul 27, 2017
@gasche
Copy link
Member

gasche commented Jul 27, 2017

Merged. @thierry-martinez, feel free to send a conf-llvm-using PR if that makes sense as well.

@AltGr
Copy link
Member

AltGr commented Sep 14, 2017

Could you add & os != "alpine" & os != "centos" to the available predicate?

I'm afraid os will be linux in all of these cases, and we don't have a distribution variable (yet: see ocaml/opam#3040)

@AltGr
Copy link
Member

AltGr commented Sep 14, 2017

One big query point in this PR is the wget on osx. I see it's been done in previous versions as well, but it will break with opam2 and fetching distfiles outside of the url file breaks caching as well.

opam 2 has native support, though, see http://opam.ocaml.org/doc/2.0/Manual.html#opamsection-extra-sources

@AltGr
Copy link
Member

AltGr commented Sep 14, 2017

I'm afraid os will be linux in all of these cases, and we don't have a distribution variable (yet: see ocaml/opam#3040)

On that matter, please review ocaml/opam#3058 if you want better support in opam 2!

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.

4 participants