julia 1.0 support: Merge "build-time" and "run-time" modules BuildApp and App into ApplicationBuilder

[NHDaly]

Split out change_dir_if_bundle into a new package,
ApplicationBuilderRuntimeUtils.jl.

This way, there's not the weird inner package BuildApp, which has almost the
same name as ApplicationBuilder, and I think is sort of confusing. I think
doing it this way really clears up the dependency relationship between the two
modules, and still allows application-code to only import the runtime
dependencies!

[NHDaly]

@ranjanan: Hey, can I get your opinion on this if you have a moment? I've split this package up into two, moving the application building stuff out of the parallel BuildApp module and up into the main ApplicationBuilder module.

Then i've moved all the runtime stuff from ApplicationBuilder.App -- which I think was kind of a weird place for it -- and into a separate package: ApplicationBuilderRuntimeUtils.jl.

I think this makes a lot more sense. What do you think?
Also, the name for that new package is kind of terrible... Any better ideas?

Thanks!! :D

[coveralls]

Pull Request Test Coverage Report for Build 185

• 90 of 105 (85.71%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+28.7%) to 86.555%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/ApplicationBuilder.jl	90	105	85.71%

Totals
Change from base Build 175:	28.7%
Covered Lines:	103
Relevant Lines:	119

---

💛 - Coveralls

[non-Jedi]

@NHDaly I've been watching this package from afar. If it were me, I'd just rename the inner module to RuntimeUtils; I can't imagine it getting used by other packages, so it seems silly to maintain it separate from this. It also sounds like the name confusion is the main reason to split it out. If you wanted stricter separation, you could even house it in a directory under src.

[NHDaly]

😄 Haha awesome, thanks for dropping in!! :D

just rename the inner module to RuntimeUtils

Ah, so that's exactly the problem: that's not how it's currently structured even though it should be. Currently (at HEAD), it's the outer module that holds the "runtime utils" (ApplicationBuilder) and the "inner", or really "sibling" module that holds the actual build functionality (BuildApp).

The reasoning is this:

1. A user application file that's being compiled shouldn't be importing the build functionality, otherwise all that logic will get compiled into the program unnecessarily. (And not only is that slow, also right now the build stuff doesn't compile, so it will break things.)
2. In order to import an "inner module" (ie module Foo module Bar end end) you have to also import the outer one -- that is, there is no way to import Foo.Bar without also loading, and initializing, all the code in Foo.
3. Following from 1. and 2., RuntimeUtils cannot be an inner module of ApplicationBuilder, or an application wouldn't have access to RuntimeUtils.
4. BUT even crazier, there's no way to import an outer module without also evaluating and initializing all the code in the inner module! i.e., calling import Foo would still cause the module Bar to be included in the compiled binary.
5. Following from 4., then, the build-functionality (BuildApp) also cannot be a standard "inner module" either!

To rephrase 1-5, neither BuildFunctionality.RuntimeUtils nor RuntimeUtils.BuildFunctionality are acceptable structures.

SO what we've done so far is have them as "siblings". That is, the "outer module", ApplicationBuilder does two things: A) it contains all the runtime utilities, and B) it adds the sibling module BuildApp to the LOAD_PATH, here:

ApplicationBuilder.jl/src/ApplicationBuilder.jl

Line 14 in 2ddc651

push!(LOAD_PATH, @__DIR__) # Expose all modules in this package.

So then, you can now get the runtime utilities by first importing ApplicationBuilder, then importing BuildApp:

using ApplicationBuilder; using BuildApp

But i think that's still not great. First of all, it's non-standard, so here's an understandability drawback. Second, it's brittle: if you flip the order of those using statements, it fails. That's weird. Third, it's weird that the names are so similar. Really the outer module should be called ApplicationBuilderRuntimeUtils, but it's required that the outer module name match the name of the Package, and currently the package provides both the runtime utilities and the build functionality.

I can't imagine it getting used by other packages, so it seems silly to maintain it separate from this. It also sounds like the name confusion is the main reason to split it out.

Yeah, so you're right that confusion is my main motivation for splitting it out. It's all the weirdness that I described above, including the name part. Also, the reason I first started doing it was that I had a problem with docstrings not showing up, and someone on slack said this might fix it, but it didn't...

If you wanted stricter separation, you could even house it in a directory under src.

I'm interested in this. What do you mean by that? I'm open to other suggestions for how to proceed.

---

I do think splitting them up is a bit cleaner, but the things I don't like about it are α) that on julia v0.7 users will have to manually add both packages in order to use both (on v0.6 that's not a problem since ApplicationBuilder installs the other as a dependency), β) it'll be a bit weirder to maintain both separately, and ɣ) the ApplicationBuilderRuntimeUtils name is super long.

But I do like that everything becomes more standard, so I'm currently leaning towards this being a good idea. What do you think? :) Thanks for jumping in here with me!!

[ranjanan]

@NHDaly, are you not able to implement this way?

module ApplicationBuilder
   module BuildUtils
   ...
   end
   module RuntimeUtils
   ...
   end
end

[NHDaly]

Unfortunately no. That's what we had at first, and it causes both submodules to be loaded and compiled when the outer module is imported. I actually can't find any documentation for that behavior in the manual (https://docs.julialang.org/en/release-0.6/manual/modules/), but that seems to be the case based on my simple experimentation. Consider:

julia> module Outer
    println("compiling Outer")
    module A
      println("compiling A")
    end
    module B
      println("compiling B")
    end
end
compiling Outer
compiling A
compiling B
>> Outer

So the problem is that in order for an app to import ApplicationBuilder.RuntimeUtils, PackageCompiler would end up also attempting to compile ApplicationBuilder.BuildUtils, which eventually means it attempts to compile itself (PackageCompiler), which currently fails on v0.6. And also it would mean (i'm pretty sure) that the resultant binary would contain all of the compiled code for BuildUtils and PackageCompiler, which it just doesn't need.

[NHDaly]

Here's what I'm talking about in the actual code. I've made the change we're talking about here:
dee26b2

And this is the failure:

julia> using ApplicationBuilder.BuildApp
PARSING (&& COMPILING) ApplicationBuilder
PARSING (&& COMPILING) BuildApp

julia> build_app_bundle("/Users/Daly/.julia/v0.6/ApplicationBuilder/examples/hello.jl")
...
PARSING (&& COMPILING) ApplicationBuilder
PARSING (&& COMPILING) BuildApp
ERROR: LoadError: LoadError: LoadError: LoadError: LoadError: GCC wasn't found. Please make sure that gcc is on the path and run Pkg.build("PackageCompiler")
while loading /Users/daly/.julia/v0.6/PackageCompiler/src/static_julia.jl, in expression starting on line 11
while loading /Users/daly/.julia/v0.6/PackageCompiler/src/PackageCompiler.jl, in expression starting on line 28
while loading /Users/daly/.julia/v0.6/ApplicationBuilder/src/BuildApp.jl, in expression starting on line 5
while loading /Users/daly/.julia/v0.6/ApplicationBuilder/src/ApplicationBuilder.jl, in expression starting on line 16
while loading /Users/Daly/.julia/v0.6/ApplicationBuilder/examples/hello.jl, in expression starting on line 1
ERROR: failed process: Process(`/Applications/Julia-0.6.app/Contents/Resources/julia/bin/julia -Cx86-64 -J/Applications/Julia-0.6.app/Contents/Resources/julia
/lib/julia/sys.dylib --compile=yes --depwarn=yes --precompiled=no --compilecache=no --startup-file=no -O3 -g0 --output-o hello.o -e '
  empty!(Base.LOAD_CACHE_PATH) # reset / remove any builtin paths
  push!(Base.LOAD_CACHE_PATH, "cache_ji_v0.6.4") # enable usage of precompiled files
  Sys.__init__(); Base.early_init(); # JULIA_HOME is not defined, initializing manually
  include("/Users/Daly/.julia/v0.6/ApplicationBuilder/examples/hello.jl") # include Julia program file
  empty!(Base.LOAD_CACHE_PATH) # reset / remove build-system-relative paths'`, ProcessExited(1)) [1]

It makes me sooooo sadddddd! :(((
Haha thanks for all your help everyone, sorry it's such a weird, annoying problem..

[NHDaly]

Okay actually, it turns out that the current approach cannot work in Julia v0.7, because a package cannot modify the LOAD_PATH in Main.

So i think i'm going to pull the trigger and merge in this PR!!! :)

Reach out to me if you have concerns, but i'm gonna try to do this today before my talk tomorrow! 😂😅😂

[non-Jedi]

I don't think I'm quite following all of this (probably because I haven't had time to actually look at the source code itself), but for your reference what I meant by "in a directory under src" was that all of the runtime stuff would just get housed in a directory under src so you'd have something like src/runtime/Runtime.jl or something like that, and then you'd just use include to add that module from Runtime.jl to wherever it needed to be.

Again, I haven't really wrapped my head around things here, so feel free to utterly discard, but I figured I should at least clarify what I meant before.

[NHDaly]

Thanks a million, @non-Jedi. Yeah, thanks, that does make sense. I played around with that as well, but -- as far as I can tell -- it still doesn't make v0.7 (and/or v1.0) happy. :(

So i'm going to play with getting everything to pass as the two separate packages, and if it works out, great! And if in the future, if we figure out a better way to move forward, these can always be re-combined! :)

Thanks again!

[NHDaly]

(jk, I've just gone ahead and pushed up a julia0.7] for this package instead, which has this merged in, so that you can check it out if you need v0.7 support, but i don't have to worry about breaking things in master. :) )

[NHDaly]

OKAY! Good news:
@KristofferC has saved the day! :) Thanks Kristoffer!!

So I've restructured everything, and I think this is going to be the right solution. :) After merging this, we can let it sit for a few days, and then if it looks good, i'll go ahead and officially register it on METADATA! :)

---

SO, here's what we did:
Before this PR, the only "runtime-util" that actually exists, is change_dir_if_bundle. But honestly, there's no reason for any code to not have that be the default behavior, so we don't even need to expose this to users -- we can just always do it for them! So that's what this PR does, it always injects code into the program to cd into Resources before running the user code.

This means that we no longer have any "runtime utilities", so we no longer need two separate modules. Therefore, I've moved everything back into ApplicationBuilder, and deleted BuildApp and App.

---

Because this is a breaking change from before, I've also added a @warn statement that runs whenever you import ApplicationBuilder, telling you about the change. This warning will now show up at the two places where your code will break due to this change! :)

(15:08:38) julia0.6> using ApplicationBuilder; using BuildApp  # BuildApp no longer exists
Warning: ApplicationBuilder has changed a bit since julia 0.6:
  - `module BuildApp` has been removed. You should remove `using BuildApp`
      from your build scripts to build with julia v0.7.
  - You should also remove `using ApplicationBuilder` from the source-code of
      programs being built, since the `change_dir_if_bundle` behavior will
      now come default for all applications. `using ApplicationBuilder` in
      your program source may cause it to fail to build.
 This message will be removed in the next version of ApplicationBuilder.
WARNING: Your Julia system image is not compiled natively for this CPU architecture.
        Please run `PackageCompiler.force_native_image!()` for optimal Julia performance
ERROR: ArgumentError: Module BuildApp not found in current path.
Run `Pkg.add("BuildApp")` to install the BuildApp package.
Stacktrace:
 [1] _require(::Symbol) at ./loading.jl:435
 [2] require(::Symbol) at ./loading.jl:405

and

(15:11:38) julia0.6> mktemp() do f,_
                         write(f, """using ApplicationBuilder
                                     Base.@ccallable function julia_main(ARGS::Vector{String})::Cint
                                         ApplicationBuilder.App.change_dir_if_bundle()
                                         println("HELLO!")
                                         return 0
                                     end
                                  """)
                         build_app_bundle(f);
                     end
~~~~~~ Creating mac app in "/Users/daly/Documents/developer/code/games/FTLBSG/builddir/tmpvIhINE.app" ~~~~~~~
~~~~~~ Compiling a binary from '/var/folders/5j/c6kd481j4l71m802wh5qp6vc0000gn/T/tmpvIhINE'... ~~~~~~~
Julia program file:
  "/Users/daly/Documents/developer/code/games/FTLBSG/builddir/tmpvIhINE.app/Contents/MacOS/applicationbuilderutils.jl"
C program file:
  "/Users/daly/.julia/v0.6/ApplicationBuilder/src/program.c"
Build directory:
  "/Users/daly/Documents/developer/code/games/FTLBSG/builddir/tmpvIhINE.app/Contents/MacOS"
Warning: ApplicationBuilder has changed a bit since julia 0.6:
  - `module BuildApp` has been removed. You should remove `using BuildApp`
      from your build scripts to build with julia v0.7.
  - You should also remove `using ApplicationBuilder` from the source-code of
      programs being built, since the `change_dir_if_bundle` behavior will
      now come default for all applications. `using ApplicationBuilder` in
      your program source may cause it to fail to build.
 This message will be removed in the next version of ApplicationBuilder.
ERROR: LoadError: LoadError: LoadError: LoadError: LoadError: GCC wasn't found. Please make sure that gcc is on the path and run Pkg.build("PackageCompiler")
while loading /Users/daly/.julia/v0.6/PackageCompiler/src/static_julia.jl, in expression starting on line 11
while loading /Users/daly/.julia/v0.6/PackageCompiler/src/PackageCompiler.jl, in expression starting on line 28
...
ERROR: failed process: Process(`/Applications/Julia-0.6.app/Contents/Resources/julia/bin/julia -Cx86-64 -J/Applications/Julia-0.6.app/Contents/Resources/julia/lib/julia/sys.dylib --compile=yes --depwarn=yes --pr
...

---

So I'm going to merge this in now once CI passes! :) And then i'll cut a new version, and after a few days we can add this to METADATA and maybe remove the warnings then.

[NHDaly]

@ranjanan does that all look good to you too? :) It was fun to hang out over last week!!

[NHDaly]

Okay, all the tests pass locally, and on Travis they all pass except the one that is still waiting on FluxML/MacroTools.jl#85 to be merged.

I'm going to merge this in now, and bump the version number!