replace osutils macros with `@static` #16219

vtjnash · 2016-05-05T21:03:51Z

implements #5892
closes #6674 and #4233
see discussion in those issues

StefanKarpinski · 2016-05-05T21:16:13Z

I like it. Question: what context are statically evaluated expressions evaluated in?

yuyichao · 2016-05-05T21:26:13Z

base/osutils.jl

    else
-        return esc(ex.args[2])
+        throw(ArgumentError("unknown operating system \"$os\""))


What's the exception garentee about @pure functions? If is_* can be @pure I believe a lot of the @static won't be necessary. (this also determines if we can make math functions like sin(::Float64) pure)

I don't see any reason pure functions can't throw exceptions (preferably as long as they always throw the exception). fwiw, I don't actually enforce that the call in @static must be pure, but left that as a future exercise.

In a few places, I used @static mostly as a way of saving a few bytes in the sysimg. There's only a few contexts in which it makes a significant difference right now (ccall needs a constant name that is defined on the current system)

stevengj · 2016-05-05T21:32:07Z

I dunno, still seems more convenient to be able to write @windows_only code... rather than @static if is_windows(); ...code...; end. Could we keep the OS convenience macros, even if they are implemented in terms of a more general @static construct?

vtjnash · 2016-05-05T21:33:03Z

Question: what context are statically evaluated expressions evaluated in?

Answer: current_module()

vtjnash · 2016-05-05T21:36:32Z

I dunno, still seems more convenient to be able to write @windows_only code... rather than @static if is_windows(); ...code...; end. Could we keep the OS convenience macros, even if they are implemented in terms of a more general @static construct?

how about @static is_windows() && code? I think we can easily add that syntax to the @static

otoh, I've found that the typical usage is to select between two (or more) implementations of something, in which case it can be replaced with a non-@static toplevel conditional

if is_windows()
   code
else
   more code
end

tkelman · 2016-05-05T21:37:37Z

The @unix_only etc macros aren't too terrible, we could maybe hold off on getting rid of those for a later release - it's mainly the fake ternary macros that we should get rid of asap.

tkelman · 2016-05-05T21:40:07Z

Also don't forget the NEWS.md mention.

tkelman · 2016-05-05T21:49:53Z

base/initdefs.jl

-    push!(LOAD_CACHE_PATH,abspath(JULIA_HOME,"..","usr","lib","julia")) #TODO: fixme
+    push!(LOAD_PATH, abspath(JULIA_HOME, "..", "local", "share", "julia", "site", vers))
+    push!(LOAD_PATH, abspath(JULIA_HOME, "..", "share", "julia", "site", vers))
+    push!(LOAD_CACHE_PATH, abspath(JULIA_HOME, "..", "lib", "julia")) #TODO: fixme?


apparently not with you around 😜

this is one way of responding to aacf7d3#commitcomment-15940506

JeffBezanson · 2016-05-06T19:07:00Z

OS_NAME was not changed AFAICT, so this doesn't fix #6674 yet.

tkelman · 2016-05-13T20:16:31Z

base/osutils.jl

-    OS_NAME
+let UNAME = ccall(:jl_get_UNAME, Any, ())
+    # change some `uname -s` values to "friendly" names
+    # for internal usage


Why not advertise these as the public facing constants? uname, Darwin, kernel specifics etc are pretty obscure for a lot of users.

because the kernel identifier is canonical and unambiguous. these are sort of just random substitutions. I'm more inclined to just drop this renaming and make Base.OSNAME === Sys.KERNEL. this does the renaming only because the exported names below use the "friendly" names.

What's canonical about WINNT? uname is a posix thing. The Julia function names and the Julia constant identifiers should be consistent, I don't see why shell scripting jargon matters.

I think the correct name would be NT. I'll change our make file to use that instead and flow the changes through to here. WindowsNT, Windows_NT, WinNT, winnt, and WINNT all seem to be common variants though, so I had gone with our Makefile normalization initially

No one is going to write code testing against "NT." Formal kernel names are internal implementation details that don't matter so much at the Julia level - the userland OS, environment, and common names that people will want to test against and know what it means even if they've never written a shell script that called uname, do matter.

what would you call it?

A userspace posix compatibility layer. It doesn't implement a filesystem or other things that it just delegates to win32.

midipix would allow us to bypass the restrictions of the win32 wrapper and access the nt kernel directly.

I doubt this is really possible, unless you're microsoft and building code into the kernel (like the WSL subsystem).

If the goal of midipix is run Linux applications on Windows, WSL does a better job. midipix has failed in that regard, in that they're not measurably getting closer to anything we can use. The Linux binaries of Julia do actually run in WSL though (not perfectly, but the basics work). It doesn't make the win32 build of Julia any less necessary because it doesn't interoperate and can't run on older than Windows 10, but this is beside the point. The value of this variable is not actually the kernel, so it shouldn't be called that.

It doesn't implement a filesystem or other things that it just delegates to win32.

Did I forget filesystem that in my list above? My bad. Cygwin also implements symlinks, filename mappings, and permissions using nt as the HAL. It also supports filesystems such as procfs and handles mounting in some cases such as folders.

No it wouldn't, midipix still goes through win32. Unless you're microsoft and building code into the kernel (like the WSL subsystem), there's no way to avoid that.

While the nt kernel interface is almost undocumented, sufficient information is available due to the need to write drivers against the nt side that it is possible to avoid it. Afaict from skimming the code in midipix, it skips the win32 layer and calls directly into nt for a number of syscalls. (I don't know whether they avoid linking to win32 entirely, or just in specific instances, although I suspect the former may be true).

because it doesn't interoperate

yes, this is exactly the fatal flaw in the whole WSL project. midipix may not have this shortcoming and thus is still more interesting (imo).

Only fatal if you would like win32 to cease to exist. Clearly Microsoft does not feel that way, and WSL will be more useful than midipix for some time to come. midipix is also GPL licensed at the moment which I consider to be a similarly fatal flaw for people who would like to create Windows applications using Julia.

What is the point of your argument? This appears to be a validation that midipix would is a viable counterpart to win32 as a api to the nt kernel, while wsl is an independent, disconnected kernel. License may be a concern for actually being able to use it, but it's not relevant to a discussion of what it is called.

It sounds like WSL may provide yet another cross-compile environment that we will be expected to support, without providing any benefit to end users. Since it lies about it's uname, this will likely be made more difficult, so forgive me if I'm not quite thrilled about this. cygwin is at least truthful about its uname and allows coding against both the win32 and posix APIs to create hybrid apps.

We're off in the weeds, but mainly I don't think Sys.KERNEL should be called that.

The benefit of WSL to end-users is being able to do things like apt-get install openmpi-bin and have it work (without a VM or installing a Linux partition, which wouldn't interoperate either) without having to teach every single build system in the world what to do with a new uname value. It's disconnected from the win32 subsystem, but uses the same NT kernel - just not in a way that's exposed to the user, who never needs to worry about system calls. Supporting WSL from a Julia standpoint will just be a matter of running our Linux binaries there, unmodified, and reporting to Microsoft when things don't work.

tkelman · 2016-05-15T19:16:09Z

The OS makefile variable isn't really the kernel - it's not accurate for WSL for example. Considering Julia's all in userland, I don't think we should be using that terminology. We could call it Sys.uname() but that has no meaning on Windows.

tkelman · 2016-05-16T17:06:44Z

accepting bsd in REQUIRE still needs documentation.

how about not giving your Sys.KERNEL a name in Julia, and just using the ccall the few places it's needed?

vtjnash · 2016-05-16T18:08:35Z

how about not giving your Sys.KERNEL a name in Julia, and just using the ccall the few places it's needed?

I've considered making it a function instead of a constant (or populating the constant at runtime), but anyhow, I've already removed the constant from Base entirely now.

tkelman · 2016-05-16T19:24:42Z

I've already removed the constant from Base entirely now.

Let's remove it from Sys entirely too. This doesn't need to be exposed and calling it KERNEL is incorrect.

vtjnash · 2016-05-16T19:41:10Z

Let's remove it from Sys entirely too. This doesn't need to be exposed and calling it KERNEL is incorrect.

How is it incorrect for a variable named KERNEL to contain the name of the kernel?

tkelman · 2016-05-16T19:45:29Z

That's not what it contains, it contains what uname returned at compile time, munged by our makefile in the Windows case. This is not the name of the kernel. Even posix documents uname as returning the "operating system name."

vtjnash · 2016-05-16T19:59:29Z

munged by our makefile in the Windows case.

Fixed.

Even posix documents uname as returning the "operating system name."

Posix usually treats the two as synonyms, although Linux doesn't (compare uname -s vs. uname -o). On some systems, these coincide so the point is moot. On Windows, the OS is Win32 while the Kernel is NT. On the Linux kernel, the OS is typically GNU/Linux.

tkelman · 2016-05-16T20:00:21Z

src/gf.c

@@ -747,7 +747,7 @@ struct ambiguous_matches_env {
    jl_array_t *shadowed;
    int after;
 };
-const int eager_ambiguity_printing = 0;
+const int eager_ambiguity_printing = 1;


tkelman · 2016-05-16T20:08:54Z

doc/stdlib/base.rst


-.. function:: @linux
+.. function:: windows_version()


is_windows needs a docstring

isn't the genstdlib.jl supposed to warn about that? I hadn't checked since I assumed that's what all the "missing doc for" messages were intended to catch.

tkelman · 2016-05-20T23:27:05Z

base/deprecated.jl

+deprecate(:OS_NAME) # use Sys.KERNEL now
+
+import .Sys.CPU_CORES
+export CPU_CORES # use the export from Sys (can't deprecate this because it instead deprecates Sys.CPU_CORES)


Why does @deprecate_binding work for WORD_SIZE but not CPU_CORES ? It's still dangerous and will break things to have its export here where it will be deleted.

tkelman · 2016-05-21T04:11:47Z

base/atomics.jl

@@ -3,6 +3,7 @@
 using Core.Intrinsics: llvmcall

 import Base: setindex!, getindex, unsafe_convert
+import Base.Sys: Sys, WORD_SIZE


importing Sys from itself looks odd, why not ARCH if that's what's needed?

tkelman · 2016-05-21T05:56:14Z

Sorry about yet another NEWS.md conflict, #16435 was needed to fix the nightlies. And #16308 means that tkelman@0e81c84 will be necessary to get through bootstrap.

implements #5892 closes #6674 and #4233 Sys.KERNEL now replaces OS_NAME and unambiguously returns the name of the kernel reported by uname for the build system configuration

joehuchette · 2016-05-22T01:38:04Z

base/file.jl

-    finally
-        systemerror(:fchdir, ccall(:fchdir,Int32,(Int32,),fd) != 0)
-        systemerror(:close, ccall(:close,Int32,(Int32,),fd) != 0)
+if is_windows()


is there a @static missing here?

no, statically evaluating a top level static expression is redundant

Backport @static macro (JuliaLang/julia#16219)

omus · 2016-05-25T13:17:54Z

Am I correct in that the removal of the @*_only macros means you can no longer mark a function as OS specific without the use of an if?

# old
@osx_only func() = 1

# new
if is_apple()
    func() = 1
end

stevengj · 2016-05-25T16:21:32Z

base/osutils.jl

+"""
+    @static
+
+Partially evaluates an expression at parse time.


This is pretty terse. Maybe:

Given a conditional expression like `@static cond ? yes : no` or `@static if cond; yes; else; no`, evaluates the condition `cond` at parse time (via `eval`, i.e. in global scope for the current module) and produces either the `yes` or `no` expression. For example, ...

stevengj · 2016-05-25T16:23:24Z

@omus, the analogue of @osx_only is @static if is_apple(); func() = ...; end

omus · 2016-05-25T16:29:26Z

@stevengj thanks, that's what I thought. It was nice being able to use the macros on long form functions:

@osx_only function func()
    ...
end

Oh well

* These were convenient and less noisy than the current {@}static if is_{os} counterpart introduced in #16219 * {@}osx_only is now {@}apple_only, to match the change in #16219 * added {@}bsd_only

Cf. 694413a, PR JuliaLang#16219.

vtjnash force-pushed the jn/static-if branch from 061fcf8 to f342c7d Compare May 5, 2016 21:09

yuyichao reviewed May 5, 2016
View reviewed changes

tkelman reviewed May 5, 2016
View reviewed changes

vtjnash force-pushed the jn/static-if branch from f342c7d to 5c0882e Compare May 13, 2016 20:04

tkelman reviewed May 13, 2016
View reviewed changes

vtjnash force-pushed the jn/static-if branch from 5c0882e to 1f8eecb Compare May 16, 2016 16:05

tkelman reviewed May 16, 2016
View reviewed changes

vtjnash force-pushed the jn/static-if branch from 8c2ae69 to 5581be9 Compare May 16, 2016 20:02

tkelman reviewed May 16, 2016
View reviewed changes

vtjnash force-pushed the jn/static-if branch from 5581be9 to 2899409 Compare May 16, 2016 20:24

tkelman reviewed May 20, 2016
View reviewed changes

tkelman added a commit to tkelman/julia that referenced this pull request May 21, 2016

contrib and test/perf cleanup for JuliaLang#16219

71a70db

vtjnash force-pushed the jn/static-if branch from 55b03f5 to 16088d1 Compare May 21, 2016 00:10

tkelman reviewed May 21, 2016
View reviewed changes

vtjnash and others added 2 commits May 21, 2016 13:03

implement @static macro for replacing osutils macros

694413a

implements #5892 closes #6674 and #4233 Sys.KERNEL now replaces OS_NAME and unambiguously returns the name of the kernel reported by uname for the build system configuration

contrib and test/perf cleanup for #16219

6154c85

vtjnash force-pushed the jn/static-if branch from 16088d1 to 6154c85 Compare May 21, 2016 17:10

Fix runtests.jl warnings, tweak docs and add NEWS.md links

8a419de

tkelman merged commit 7430e83 into master May 21, 2016

tkelman deleted the jn/static-if branch May 21, 2016 21:01

TotalVerb mentioned this pull request May 22, 2016

Add macro for conditionally choosing code based on Julia version JuliaLang/Compat.jl#131

Closed

joehuchette reviewed May 22, 2016
View reviewed changes

TotalVerb mentioned this pull request May 23, 2016

Backport @static macro (JuliaLang/julia#16219) JuliaLang/Compat.jl#204

Merged

tkelman added a commit to JuliaLang/Compat.jl that referenced this pull request May 23, 2016

Merge pull request #204 from TotalVerb/pull-request/7f4d7c50

090b82d

Backport @static macro (JuliaLang/julia#16219)

stevengj reviewed May 25, 2016
View reviewed changes

tkelman mentioned this pull request May 29, 2016

conditional expressions that eagerly evaluate the condition at top-level? #5892

Closed

kmsquire mentioned this pull request May 30, 2016

Undeprecate {os}_only macros #16658

Closed

rfourquet added a commit to rfourquet/julia that referenced this pull request Jun 11, 2016

intfuncs.jl: fix bug introduced when deprecating WORD_SIZE

6ba061d

Cf. 694413a, PR JuliaLang#16219.

rfourquet mentioned this pull request Jun 11, 2016

ndigits: fix (harmless) bug introduced when deprecating WORD_SIZE #16879

Merged

rfourquet added a commit to rfourquet/julia that referenced this pull request Jun 11, 2016

ndigits: fix (harmless) bug introduced when deprecating WORD_SIZE

69ffa31

Cf. 694413a, PR JuliaLang#16219.

ararslan mentioned this pull request Jun 4, 2017

Redefining how to identify operating systems #22214

Closed

replace osutils macros with @static #16219

replace osutils macros with @static #16219

Conversation

vtjnash commented May 5, 2016

StefanKarpinski commented May 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevengj commented May 5, 2016

vtjnash commented May 5, 2016

vtjnash commented May 5, 2016

tkelman commented May 5, 2016

tkelman commented May 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffBezanson commented May 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman May 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman May 16, 2016 • edited Loading

Choose a reason for hiding this comment

tkelman commented May 15, 2016

tkelman commented May 16, 2016 • edited Loading

vtjnash commented May 16, 2016

tkelman commented May 16, 2016

vtjnash commented May 16, 2016

tkelman commented May 16, 2016

vtjnash commented May 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman commented May 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omus commented May 25, 2016

Choose a reason for hiding this comment

stevengj commented May 25, 2016

omus commented May 25, 2016

replace osutils macros with `@static` #16219

replace osutils macros with `@static` #16219

tkelman May 16, 2016 •

edited

Loading

tkelman May 16, 2016 •

edited

Loading

tkelman commented May 16, 2016 •

edited

Loading