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

Having this package installed on 0.3 causes the Base-tests to fail #1

Closed
mauro3 opened this issue Feb 19, 2015 · 33 comments
Closed

Having this package installed on 0.3 causes the Base-tests to fail #1

mauro3 opened this issue Feb 19, 2015 · 33 comments

Comments

@mauro3
Copy link

mauro3 commented Feb 19, 2015

Having this package installed leads to the following warning when starting Julia 0.3: Warning: using Base.Graphics in module Main conflicts with an existing identifier. which then causes the test in test/spawn.jl on line 154 to fail with:

exception on 1: ERROR: test failed: readall(@cmd "\$exename -f -e 'println(STDERR,\"Hello World\")'" .> @cmd "cat") == "Hello World\n"

@stevengj
Copy link

Presumably, this package should be deprecated.

@timholy
Copy link
Member

timholy commented Feb 19, 2015

Vice versa: julia 0.4 has had Base.Graphics removed from it, and this package is the future.

@ViralBShah
Copy link
Contributor

Cc: @jakebolewski

@jiahao jiahao closed this as completed in 6a47e33 Feb 20, 2015
@timholy
Copy link
Member

timholy commented Feb 20, 2015

Wouldn't it be better to just make it re-export Base.Graphics?

@jakebolewski
Copy link
Contributor

I agree that would be better, I'll make the change.

@jakebolewski
Copy link
Contributor

Actually, what @jiahao did looks like the best solution due to the naming conflict with this package and the Graphics module in 0.3.

@tkelman
Copy link

tkelman commented Feb 20, 2015

Won't 6a47e33 result in warnings for all the packages that now require this one on 0.3?

Or never upgrading? Should the require be adjusted for the existing tag in metadata? https://github.com/JuliaLang/METADATA.jl/tree/metadata-v2/Graphics/versions/0.1.0

@mauro3
Copy link
Author

mauro3 commented Feb 20, 2015

Updating Graphics to master now indeed results in a WARNING: julia is fixed at 0.3.6 conflicting with requirement for Graphics: [0.4.0-,∞) whilst the old warning on julia startup is still present. Am I doing something wrong or how is this now fixed?

@tkelman
Copy link

tkelman commented Feb 20, 2015

I don't think you're doing anything wrong. We just haven't gone through the process of migrating code out of base enough times recently. I would say the package should be a stub but still loadable for 0.3, but is there a way to do that without the conflicting module name warning? Do we have to rename this package to GraphicsBase or something?

@mauro3
Copy link
Author

mauro3 commented Feb 20, 2015

Ok, sorry for the noise. I thought that closing this meant it's fixed ;-)

@jiahao
Copy link
Contributor

jiahao commented Feb 26, 2015

The current state of affairs, quite frankly, leaves much to be desired. I don't see any way to make this package backward compatible without changing its name.

Loading this package on 0.3.6 via Color.jl breaks Gadfly.jl in a very odd way. (See GiovineItalia/Gadfly.jl#553 JuliaAttic/Color.jl#85.)

@tkelman
Copy link

tkelman commented Feb 26, 2015

Let's change the name then. Painful but would appear to be necessary for things to work correctly.

@stevengj
Copy link

It will be annoying if we have to change the name every time we migrate a module out of base...

@tkelman
Copy link

tkelman commented Feb 26, 2015

Or duplicate the exports list as imports for 0.3? https://github.com/JuliaLang/Graphics.jl/tree/jcb/exportbase

@jakebolewski
Copy link
Contributor

The problem with that "solution" is this behavior. It changes the scoping semantics.

julia> module Foo
       if false
       else
       function test(foo)
           xmin, xmax = NaN, NaN
           (xmin, xmax)
       end
       xmin(x, y) = min(x,y)
       end
       end
Warning: replacing module Foo

julia> Foo.test(10)
ERROR: invalid redefinition of constant xmin
 in test at none:5

julia> module Foo
       function test(foo)
           xmin, xmax = NaN, NaN
           (xmin, xmax)
       end
       xmin(x, y) = min(x,y)
       end
Warning: replacing module Foo

julia> Foo.test(10)
(NaN,NaN)

@jakebolewski
Copy link
Contributor

@mauro3 does running Pkg.checkout("Graphics", "jcb/exportbase") and re-running your test fix the problem for you?

@mauro3
Copy link
Author

mauro3 commented Feb 26, 2015

I still get the same warning Warning: using Base.Graphics in module Main conflicts with an existing identifier. when starting Julia, which is what makes the Julia test-suite fail.

@jakebolewski
Copy link
Contributor

Are you precompiling the Graphics package? There is no reason that it would be loaded on start up on v0.3.

@mauro3
Copy link
Author

mauro3 commented Feb 27, 2015

Good catch, thanks! I was precompiling Cairo.jl now with that removed from userimg.jl starting up julia does not warn anymore and the tests pass. However, I still get this with your branch

julia> using Graphics
Warning: imported binding for Graphics overwritten in module Main

And strangely, using Cairo does not produce a warning, but did when precompiled?

@timholy
Copy link
Member

timholy commented Mar 4, 2015

A recent tagging of Tk ended up breaking more stuff:
timholy/ProfileView.jl#30
JuliaImages/ImageView.jl#64

We either have use "load only on julia 0.4" logic everywhere, or nowhere. I just pushed changes to Cairo, Images, ImageView, and ProfileView.

I'm not a fan of any more removals of code from base during the 0.4 cycle.

@tknopp
Copy link
Contributor

tknopp commented Mar 5, 2015

@timholy: IMHO the critical point here is that 0.4 changes break 0.3 packages. So I seriously question why it is more important to make packages 0.4 compatible than to keep 0.3 stable.

I don't know if the issue persists but yesterday I tried to get Winston working on 3.6 (the current stable version) and it did not work and I had to checkout the Graphics.jl branch that fixed this.

I think it would be a lot safer to have 0.4 changes opt-in instead of breaking 0.3 packages. More radical would be to ignore that packages don't work on a dev branch. IMHO this would be sane. If we go from dev to pre period, the packages could be transitioned.

@timholy
Copy link
Member

timholy commented Mar 5, 2015

Actually, my complaints stem from breakages on 0.3 (see JuliaImages/ImageView.jl#64 and timholy/ProfileView.jl#30), both of which were triggered by tagging the 0.3.0 release of Tk and both of which were reported by users running 0.3.

I tagged new versions of Cairo, Winston, Images, ImageView, and ProfileView yesterday. I haven't checked Gtk, if you're using that with Winston there might still be issues.

@tkelman
Copy link

tkelman commented Mar 5, 2015

More radical would be to ignore that packages don't work on a dev branch. IMHO this would be sane.

This is quite true, and very reasonable. But you're going to be in a minority there, since so much of the work of maintaining Julia packages is happening by people who are also doing development work on base Julia right now. This may change over time but it'll take a while.

It is also beneficial when people try testing packages on the in-development branch of Julia if it helps identify bugs in the latter.

I'm not a fan of any more removals of code from base during the 0.4 cycle.

I'm with you there. Could the not-exporting-things idea help make this go a little smoother next time we want to try it? Should probably wait until early in 0.5-dev though. And do the work fleshing out the package replacement before merging code removal next time.

@tknopp
Copy link
Contributor

tknopp commented Mar 6, 2015

Tony, I don't think that I am in a minority there. When I oversimplify I could read that as "The package maintainers don't care about the stable (0.3) branch because they are on the dev branch". Your work on maintaining 0.3.x tells me that the opposite is true.

We should discuss this on a larger audience.
@timholy, @stevengj, @stevengj, @StefanKarpinski, @ViralBShah, @JeffBezanson, @vtjnash, @Keno: Would that be for you an option to stick packages on a stable Julia branch? I am really concerned that the current model pulls unstable stuff into packages as has happened during the Graphics removal. In the end its of course up to the maintainers of a package. But I think guidelines would be good here.

@tknopp
Copy link
Contributor

tknopp commented Mar 6, 2015

and of course ping @jiahao.

@tkelman
Copy link

tkelman commented Mar 6, 2015

When I oversimplify I could read that as "The package maintainers don't care about the stable (0.3) branch because they are on the dev branch". Your work on maintaining 0.3.x tells me that the opposite is true.

Not what I meant at all. Keeping 0.3.x healthy with backported bugfixes and regular point releases has been a bit of work, but surprisingly not that hard if you get a handful of people to worry about it consistently. Obviously the majority of users are and should be using the release branch.

What I meant was more the following - if packages start setting upper bounds or being in a failing state for the latest dev branch for a long period of time, that would likely interfere with work that needs to get done by the same people who are also doing development work on base. That's all. So PR's to get rid of deprecation warnings, work on the Compat.jl package to make that easier, and other fixes to make things work on 0.4-dev are inevitable.

Churn that causes breakage like what happened here however is a major issue and we should work harder to avoid repeating it in the future.

@tknopp
Copy link
Contributor

tknopp commented Mar 6, 2015

Tony, please note that I don't want to complaint about the breakage but think that the current model is a little dangerous and therefore want to start this discussion.

Actually I don't say that an upper bound should be put on packages. But it would make more sense to have a Compat.jl package for the dev branch instead of the other way around. There is a difference between letting a package work in 0.4-dev and in using new features of the dev branch.

In the current mode packages are as early ported as possible to new dev-branch features. To keep 0.3 alive the missing features are put into Compat.jl which means that development code is put into the stable package world before it is actually released. Thats at least how I understand it.

I am of course a little biased as I use 0.3 in my research group (six people using julia!). From that viewport stability is very important. While I love julia's package system and even promote moving more base functionality into default packages (JuliaLang/julia#5155) it can still be an issue to keep everything compatible across different versions.

@tkelman
Copy link

tkelman commented Mar 6, 2015

think that the current model is a little dangerous and therefore want to start this discussion.

I agree. It's a good discussion to have, but may be better to have on the mailing list.

But it would make more sense to have a Compat.jl package for the dev branch instead of the other way around. There is a difference between letting a package work in 0.4-dev and in using new features of the dev branch.

For the most part, Compat.jl has been about syntax deprecations, not so much feature additions - with the exception of Nullable which only recently went in. It's far more future-proof to start using the dev syntax with a compatibility layer. Most of these kind of simple changes should get deprecations so the default behavior is to keep the old syntax working but with a warning, so updates to use the new-syntax compatibility layer are primarily for convenience in getting rid of warnings.

In the current mode packages are as early ported as possible to new dev-branch features.

No, not really. Major breaking changes to the internals or standard library structure/content are much harder to paper over with a compatibility layer than syntax deprecations.

@simonster
Copy link

I think the main source of package issues on Julia 0.3 is that package maintainers mainly use Julia 0.4. I don't think there's approach that can entirely eliminate the problems that arise as a result except freezing packages when a new major Julia version is released, and if this is what you want, you can just pin packages on old versions. Having separate stable Julia and dev Julia branches for each package is no silver bullet, since people may still backport changes from dev to stable without realizing that they actually break stable. (This is hard to forecast, e.g., even if your changes seem trivial and don't use any dev-only syntax, they may depend on features of other packages that are only available in their dev branches, and it's hard to keep track of the entire package/Julia version dependency graph in your head.)

I'm not sure what we can do besides 1) test on both stable and dev Julia (which most packages are already doing, and packages that aren't doing should do) and 2) make code that runs on dev Julia more likely to run on stable Julia (which is what Compat is for).

For the most part, I think this cycle has been smoother than the previous cycle, where many packages ended up with separate stable and dev branches. Cases where we move code around always seem to be the most problematic (e.g., the somewhat disastrous Stats -> StatsBase rename), and perhaps we should recognize that and treat them with additional care.

@IainNZ
Copy link
Contributor

IainNZ commented Mar 6, 2015

For the most part, I think this cycle has been smoother than the previous cycle, where many packages ended up with separate stable and dev branches.

Indeed, its been great how much people have been able to do this.

On the whole, people haven't been breaking 0.3 much at all, its been quite impressive. In fact, as a percentage of packages passing is pretty much constant (with the exception of recent breakage caused by the new testing setup). Perhaps its been getting a little more choppy of late.

Test 0.3 percentages

@tknopp
Copy link
Contributor

tknopp commented Mar 6, 2015

Ok, I am silent then. Maybe I have looked to isolated on JuliaGraphics/Winston.jl@4122eda
which could have been avoided if 0.4-dev package stability would be less important.

I absolutely agree that 0.3 release cycle is pretty stable. One cannot compare it to the 0.3-pre time where we had effectively a rolling release and packages following master.

We could introduce a rule that packages should only be tagged if the tests have been run on a stable release. This would have fixed this particular issue.

@vtjnash
Copy link

vtjnash commented Jun 4, 2015

is it worth putting at the top of the file:

if VERSION < v"0.4-"
  const Graphics = Base.Graphics
else
module Graphics
...
end
end

so that it just works for users either way? or has this been solved by all packages by now?

@timholy
Copy link
Member

timholy commented Feb 24, 2017

I think this isn't really relevant anymore.

@timholy timholy closed this as completed Feb 24, 2017
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

No branches or pull requests