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

Method definition warning that only shows up on Windows #18769

Closed
musm opened this issue Oct 2, 2016 · 19 comments · Fixed by #21540
Closed

Method definition warning that only shows up on Windows #18769

musm opened this issue Oct 2, 2016 · 19 comments · Fixed by #21540
Labels
system:windows Affects only Windows

Comments

@musm
Copy link
Contributor

musm commented Oct 2, 2016

On windows

> julia
               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: http://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?help" for help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.5.0 (2016-09-19 18:14 UTC)
 _/ |\__'_|_|_|\__'_|  |  Official http://julialang.org/ release
|__/                   |  x86_64-w64-mingw32

julia> *{T}(x::Int, ::Type{T}) = T(x)
ERROR: error in method definition: function Base.* must be explicitly imported to be extended

However on Linux I don't see this warning

@vtjnash vtjnash added the system:windows Affects only Windows label Oct 2, 2016
@yuyichao
Copy link
Contributor

yuyichao commented Oct 2, 2016

Somehow the REPL uses * only on windows?

@vtjnash
Copy link
Member

vtjnash commented Oct 2, 2016

This is caused by our default contrib/windows/juliarc.jl file (adding JULIA_HOME to ENV["PATH"])

@yuyichao
Copy link
Contributor

yuyichao commented Oct 2, 2016

So is there anything to fix here?

@StefanKarpinski
Copy link
Member

Don't make that sketchy method overload for *?

@yuyichao
Copy link
Contributor

yuyichao commented Oct 3, 2016

I.e. close as user error? (Julia doesn't override anything.)

@yuyichao yuyichao added the won't change Indicates that work won't continue on an issue or pull request label Oct 4, 2016
@yuyichao yuyichao closed this as completed Oct 4, 2016
@musm
Copy link
Contributor Author

musm commented Apr 24, 2017

@vtjnash why exactly does windows need https://github.com/JuliaLang/julia/blob/master/contrib/windows/juliarc.jl
also if we do something like "$(JULIA_HOME);$(ENV["PATH"])" will that not fix the issue?

@TotalVerb
Copy link
Contributor

TotalVerb commented Apr 24, 2017

That'll just move the issue to string instead of *.

Edit: my mistake, "$x$y" expands to Base.string(x, y) not string(x, y). You're right, that should fix the issue.

@musm
Copy link
Contributor Author

musm commented Apr 24, 2017

...sigh....I thought I found a loophole

@vtjnash
Copy link
Member

vtjnash commented Apr 24, 2017

I guess because we've always done it that way (79dd218), and nobody has questioned or tested it since?

@musm
Copy link
Contributor Author

musm commented Apr 24, 2017

I just deleted the juliarc.jl that shipped with the binaries in /etc/julia and ran julia ... no problems there
@tkelman do you see any problems with deleting the file?

@vtjnash
Copy link
Member

vtjnash commented Apr 25, 2017

Does WinRPM still work?

@musm
Copy link
Contributor Author

musm commented Apr 25, 2017

doesn't look like it will work

ERROR: could not spawn `7z x -y 'C:\Users\Mus\.julia\v0.6\WinRPM\cache\4\noarch%2Fmingw64-liblzma5-5.0.4-9.22.noarch.rpm' '-oC:\Users\Mus\.julia\v0.6\WinRPM\cache\4'`: no such file or directory (ENOENT)
Stacktrace:
 [1] _jl_spawn(::String, ::Array{String,1}, ::Ptr{Void}, ::Base.Process, ::Base.DevNullStream, ::Base.PipeEndpoint, ::Base.TTY) at .\process.jl:365
 [2] #373 at .\process.jl:517 [inlined]
 [3] setup_stdio(::Base.##373#374{Cmd}, ::Tuple{Base.DevNullStream,Pipe,Base.TTY}) at .\process.jl:503
 [4] #spawn#372(::Nullable{Base.ProcessChain}, ::Function, ::Cmd, ::Tuple{Base.DevNullStream,Pipe,Base.TTY}) at .\process.jl:516
 [5] spawn(::Cmd, ::Tuple{Base.DevNullStream,Pipe,Base.TTY}) at .\process.jl:511
 [6] open(::Cmd, ::String, ::Base.DevNullStream) at .\process.jl:583
 [7] do_install(::WinRPM.Package) at C:\Users\Mus\.julia\v0.6\WinRPM\src\WinRPM.jl:465
 [8] do_install(::WinRPM.Packages{Array{LibExpat.ETree,1}}) at C:\Users\Mus\.julia\v0.6\WinRPM\src\WinRPM.jl:443
 [9] #install#21(::Bool, ::Function, ::WinRPM.Package) at C:\Users\Mus\.julia\v0.6\WinRPM\src\WinRPM.jl:387
 [10] (::WinRPM.#kw##install)(::Array{Any,1}, ::WinRPM.#install, ::WinRPM.Package) at .\<missing>:0
 [11] install(::String) at C:\Users\Mus\.julia\v0.6\WinRPM\src\WinRPM.jl:359

@vtjnash
Copy link
Member

vtjnash commented Apr 25, 2017

OK – it looks like that'll need a PR first then

@musm
Copy link
Contributor Author

musm commented Apr 25, 2017

So it's only because WinRPM needs 7z, @tkelman would you have any objects to changing WinRPM to instead look for 7z using the JULIA_HOME variable instead ?

@tkelman
Copy link
Contributor

tkelman commented Apr 25, 2017

We shouldn't be messing with this kind of thing until after branching

@musm
Copy link
Contributor Author

musm commented Apr 28, 2017

how about we remove the won't fix bit since #21540 will fix this ;)

@StefanKarpinski
Copy link
Member

Sure, although there's not harm in fixing a "won't fix" issue either :)

@StefanKarpinski StefanKarpinski removed the won't change Indicates that work won't continue on an issue or pull request label Apr 28, 2017
@musm
Copy link
Contributor Author

musm commented Jun 17, 2017

could we reopen until #21540 is merged so we don't lose track (note the pr has been ready for sometime now)?

@musm
Copy link
Contributor Author

musm commented Jul 7, 2017

Anyone mind taking a look at #21540 ? it's been ready since May, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
system:windows Affects only Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants