Include macro for building sysimgs

[MikeInnes]

This PR adds an @include macro which behaves like include, but is more compatible with compiling a sysimg.

Separately, it also adds a hook to replace include statements in @require block with @include. This risks being a bit magical but shouldn't affect normal usage at all, only sysimg usage. The main edge case is recursive include during sysimg compilation (e.g. file included by Requires has another include in it). Since this is pretty uncommon, it should make most Requires usage just work with sysimg building.

[MikeInnes]

Forgot that var"..." won't work on 1.0, but we can fix that if we're happy to merge this.

[MikeInnes]

Second commit runs macros inside @require at load time, rather than when the required package is loaded. Otherwise @include was effectively the same as include. I don't think this should have adverse affects for other macros, and might even be useful in cases similar to this one.

Third commit fixes the 1.0 issue.

[timholy]

Mostly this needs a test, otherwise looks good.

[ViralBShah]

Bump. We should get this tagged and released.

[andreasnoack]

Mostly this needs a test

Won't this code get exercised by the existing tests because of the rewrite part? Any idea of how to tests the part that helps system image building?

[ViralBShah]

Maybe if it works, let's get it merged?

[andreasnoack]

@timholy Could you provide a hint to the kind of testing that would be useful here. If any. I can try to write such tests but I haven't done any work here before so I'd need some directions.

[timholy]

I'm not exactly sure what the use-case really is here, but looking again it seems the existing tests already cover this. Sorry for the confusion. This will break Revise, but it should be fixable.

[andreasnoack]

Currently, things like https://github.com/FluxML/Zygote.jl/blob/ed366f32c0f520567526040d9f8acaf0d83613c3/src/Zygote.jl#L42-L43 aren't compatible with creating system images since the included file might not be available when using the system image. This PR is an attempt to make such usage compatible with system image building.

[timholy]

I see. And one cannot just use Base.include_dependency?

[KristofferC]

Base.include_dependency will cause the module to recompile if the file is changed but I don't see how that helps. The content of the file is still not serialized to the .ji file (or the sysimage), right? When we create a sysimage and send it to another machine, the only code we have available is that which got serialized, if there is an external file that is included at runtime, it will not be found. This PR causes the source of that file to be included in the serialization and can be executed whenever.

[timholy]

Sounds good. I guess I was wondering if we could keep it separate from the source code proper. If the file is quite long then this is a big string. But I guess this isn't a huge deal, and if people are worried about that then they should just make it using GluePkg instead.

[andreasnoack]

@timholy I think you are the only one here that can merge the PR and make the release.

[timholy]

That said, if there's a way to do this conditionally on building a sysimg, Revise will thank you. (Currently, @require Pkg=uuid include("extrafile.jl") adds extrafile.jl to the list of files tracked by Revise, and you can edit it and get updates. This will break that. I just spent a day and a half improving Revise's performance in this situation.)

Is there a way to detect that you're buidling a sysimg and not just ordinary precompiling? For example, could Requires define const building_sysimg = Ref(false) and then PkgCompiler does using Requires; building_sysimg[] = true?

[timholy]

Oh, that's crazy. I just added @KristofferC and @DilumAluthge. Any others I should invite?

[ViralBShah]

I have the permissions to merge.

[timholy]

OK, but let's see if anyone is willing to tackle #85 (comment).

[KristofferC]

How about we fall back to the serialized string in case the file doesn't exist anymore but otherwise use the current mechanism?

[andreasnoack]

@KristofferC Would you be able to add a commit with that proposal?

[KristofferC]

Yeah, I'll try.

[c42f]

I went ahead and implemented the approach suggested by @KristofferC in #85 (comment).

See #92

I also added a test to cover the new behavior.

[timholy]

Superseded by #92

[c42f]

@timholy I made #92 against this branch, so you'll also need to merge this PR. Sorry for the confusion!

[timholy]

Hmm, this still seems to have broken Revise on Windows. Can others see https://ci.appveyor.com/project/timholy/revise-jl/history? Ignoring nightly (which was broken for other reasons), between "Document Revise's latency costs" and "revise should throw when throw=true" the only thing that seems like it could be related is the switch from Requires 1.0.3 to 1.1.0.

EDIT: not just Windows, it turns out I was still using Requires 1.0.3 locally. See https://travis-ci.org/github/timholy/Revise.jl/builds/732701350. I don't have time to dig into this right now myself, and won't for several days, so I'd appreciate help.

[timholy]

Filed as a separate bug report, see #94.