-
Notifications
You must be signed in to change notification settings - Fork 57
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
Use Artifacts and JLL packages on Julia 1.3+ #293
Conversation
f2f0080
to
907200f
Compare
Codecov Report
@@ Coverage Diff @@
## master #293 +/- ##
==========================================
- Coverage 92.16% 92.12% -0.04%
==========================================
Files 1 1
Lines 434 432 -2
==========================================
- Hits 400 398 -2
Misses 34 34
Continue to review full report at Codecov.
|
907200f
to
469d381
Compare
469d381
to
8790a70
Compare
Huzzah! It's working! |
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. |
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. |
#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 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. |
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. |
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 |
@timholy If you don't work on the binaries but want to contribute, there is still something you can do for adapting interfaces on the julia level e.g. GiovineItalia/Gadfly.jl#1309 |
59b494d
to
d9f2184
Compare
b459885
to
d5ea6c8
Compare
There was a problem hiding this comment.
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?
Manifest.toml
Outdated
@@ -0,0 +1,256 @@ | |||
# This file is machine-generated - editing it directly is not advised |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this whole file?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.toml
s in packages :) In my mind they record things that shouldn't be recorded in a package.
Not sure I understand the upper bound scenario.
There was a problem hiding this comment.
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:
- If you are in the "just work, darnit" frame of mind, you can instantiate the
Manifest.toml
to get the versions that worked in the past (and thereby should always work, especially in light of JLL packages which extend instantiation's effect to binary packages). - If you are trying to debug what versions of a dependency should work, the
Manifest
at least gives you a known good point; you don't need to go and look at the Registry to see what versions would have been installed on your machine six months ago when you last modified this package; all that information is stored here.
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
.
There was a problem hiding this comment.
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.toml
s 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 :)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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"...
This will allow for easier/faster binary installation, but only supports Julia 1.3+
Co-Authored-By: David Anthoff <[email protected]>
d5ea6c8
to
77f9c12
Compare
Alright there was some confusion because I re-named a branch halfway through, but I think this should be good now. |
There was a problem hiding this comment.
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.
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. |
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 aPkg.build()
step at all, in fact. Simply writeusing Glib_jll
, and you will import bindings forlibgobject
,libglib
, etc... Similarly,using Cairo_jll
will import a binding forlibcairo
. To learn more about these JLL packages, read this WIP blog post. To learn more about thePkg.Artifacts
system that this is all built on top of, read the relevant docs.