Use Artifacts and JLL packages on Julia 1.3+ by staticfloat · Pull Request #293 · JuliaGraphics/Cairo.jl (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation27 Commits6 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

staticfloat

This is an example PR that showcases how, using the capabilities in Julia 1.3+, binary installation can be accomplished both more quickly and with less developer hassle. No more build.jl files, no need for a Pkg.build() step at all, in fact. Simply write using Glib_jll, and you will import bindings for libgobject, libglib, etc... Similarly, using Cairo_jll will import a binding for libcairo. To learn more about these JLL packages, read this WIP blog post. To learn more about the Pkg.Artifacts system that this is all built on top of, read the relevant docs.

@coveralls

Coverage Status

Coverage increased (+0.7%) to 92.052% when pulling db8401b on staticfloat:sf/jll_pkg into 15706f2 on JuliaGraphics:master.

@codecov

@staticfloat

@staticfloat

To help frame this in light of #292, this branch only works on Julia 1.3+, and it uses the new Artifacts installation code which has many improvements over Binary Provider for complicated use cases exactly like Cairo.jl. Examples of things that using the Artifacts system will get you:

For these reasons, I encourage a Julia 1.3+ release of Cairo, however due to the importance of this package a 1.2- branch will undoubtedly be relied upon for a while yet. Thus, this PR simply makes it easy to launch the 1.3+ branch when the developers are ready.

@tknopp

This is great @staticfloat!

So this is an alternative to #292 or can #292 first be merged and then after that this one?

Regarding releases, I would prefer that we make simultaneous releases of Cairo and Gtk, once the later has migrated to BB/BP. I am fine making both 1.3 exclusive. But then the release can only be made once 1.3 is out.

@staticfloat

#292 should be merged first, as that is compatible with the largest number of users right now. IMO, it's best to make a release of Cairo with these new binaries vendored through BinaryProvider, have that be compatible with Julia 1.0-1.2, then split that commit off into a compat-1.2 branch, and merge this into master. Then, by the time 1.3 is released, we will have Cairo, Gtk, Rsvg, etc... all ready and working on 1.3+.

I do not think we're going to create a 1.0-compatible release for Gtk, we're just going to skip to 1.3+ directly for those packages, because it's a fair amount of developer work to get it working, and we'd rather just make quick headway with the new tools. Cairo.jl was an exception because it's an extremely high-visibility package, and I personally get a lot of mail from people complaining that it's not working on 1.0. :)

@timholy

I'm fine with going forward with Julia 1.3+.

I suspect Gtk will become the source of your mail now. At least for me, all my reports are of the form "can't use ImageView" or "can't use ProfileView" and that ultimately traces back to Cairo. Now we've moved a big step up the chain. Hoping that the new infrastructure makes Gtk not too bad. Anything we mere mortals can do to help out usefully? This is not an area of strength for me, but I feel bad about begging for help without doing something useful to pitch in.

@tknopp

I agree with Tim, that most Cairo issues reported are likely indirect issues with Gtk. Cairo of course stands on its own, but if you want to draw not only to a file but to the screen, something like Gtk is necessary

@lobingera

giordano

davidanthoff

Choose a reason for hiding this comment

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

It would be awesome to get this over the finishing line! I made some inline comments, and then in addition I think we need to add bounds for the jll package to [compat], right?

@@ -0,0 +1,256 @@
# This file is machine-generated - editing it directly is not advised

Choose a reason for hiding this comment

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

Remove this whole file?

Choose a reason for hiding this comment

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

Do you not want to include a Manifest.toml? It can be helpful to track down why, for instance, improper upper bounds caused a test to fail in the future.

Choose a reason for hiding this comment

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

I'm a big believer in not including Manifest.tomls in packages :) In my mind they record things that shouldn't be recorded in a package.

Not sure I understand the upper bound scenario.

Choose a reason for hiding this comment

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

I'm a big believer in not including Manifest.tomls in packages :)

It's your package, so by all means, manage it how you like! I'll remove it.

Not sure I understand the upper bound scenario.

The Manifest.toml records a "known good point" within the entire space of packages that the resolver can choose. If you use Pkg.add("Cairo"), you do not necessarily get a working setup for Cairo.jl, since the resolver is free to choose among a potentially very wide number of packages. If the dependency upper bounds are chosen perfectly, Pkg.add() should always work. However, it's pretty common that somebody updates a package somewhere in your deps tree that breaks something higher up in the deps tree, eventually causing Cairo.jl to break. This doesn't even have to be Cairo.jl's fault; it could be a transitive dependency that causes the problem.

Bundling a Manifest.toml provides two tools to help out:

Manifest.toml files bundled with packages aren't used for resolution or loading in any way during normal usage; you have to explicitly start Julia with --project= pointing at Cairo.jl for them to be used, and they'll only install their versions if you explicitly instantiate.

Choose a reason for hiding this comment

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

It's your package, so by all means, manage it how you like! I'll remove it.

Actually, it is not :) Not sure who is in charge, probably @lobingera?

Thanks for the explanation of the benefits, interesting. I think one main reason why I don't like to have Manifest.tomls in my packages is that VS Code will auto-activate an env if there is a Manifest.toml in a package folder, and that is something I don't want, so it throws a wrench into my normal workflow. Also, it just seems that almost all packages don't have it in the root, and I'm a fan of just following common practices with this kind of choice :)

Choose a reason for hiding this comment

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

Ah, I had no idea VS Code did that. Interesting. I can see the reason it does it, but yes that would be frustrating. This is where the concept of an "application" versus a "package" start to conflict a little bit. :P

Choose a reason for hiding this comment

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

From the VS Code extension point of view, it would be great if a package project was called Package.toml, and an app project was called Project.toml, even if they have exactly the same syntax/content. Or some other way to distinguish a package from an app. It would just be really convenient for tools to be able to look at a folder and just say "ah, this is a package", or "ah, this is an app"...

Project.toml Outdated Show resolved Hide resolved

Project.toml Outdated Show resolved Hide resolved

Project.toml Outdated Show resolved Hide resolved

@staticfloat

This will allow for easier/faster binary installation, but only supports Julia 1.3+

@giordano @staticfloat

@staticfloat

@staticfloat

@staticfloat @davidanthoff

Co-Authored-By: David Anthoff anthoff@berkeley.edu

@staticfloat

Alright there was some confusion because I re-named a branch halfway through, but I think this should be good now.

@staticfloat

davidanthoff

Choose a reason for hiding this comment

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

Looks great!

Can we merge? Who can actually merge? @lobingera or @timholy?

I would also suggest that we tag this version as 1.0.0, so that we enter sane semver territory.

@tknopp

To move forward I merged this now. Will prepare the next release if there are no objections from @lobingera. Regarding version number. I don't care but I do also not see the pressing advantage of 1.0.0. Currently any bump of the 0.x is breaking. Afterwards any change of the first version number is breaking. I am fine with both.