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

Generalisable package? #1

Open
ranjanan opened this issue Apr 11, 2018 · 15 comments
Open

Generalisable package? #1

ranjanan opened this issue Apr 11, 2018 · 15 comments
Labels
tracking Issues that are used to document ongoing work on an issue/topic

Comments

@ranjanan
Copy link
Contributor

I was wondering if this can be turned into a more generalizable package. Perhaps we can specify a dictionary of paths to non jl files including binaries, which we can use to preprocess the source files. I noticed that in your Paddle Battle, you included a code snippet to change the paths and an environment variable to control whether the paths get changed.

@NHDaly
Copy link
Owner

NHDaly commented Apr 11, 2018

Yeah that would be really awesome. So, I already have included a mechanism for listing the files you need: I meant to have the -R and -L serve this purpose. (I just noticed they weren't working exactly correctly though, so i've fixed them here: 49c90a4. Thanks for helping me realize that!)

This is what it currently uses to know which files to copy. I had been intending -L to mean "files which will contain code that will need to be loaded. These are the ones that will probably have to have their location definition overridden in the Modules that use them.

Here is an example of the files I think you need in order to build an app which uses Blink.jl:

$ julia ~/src/build-jl-app-bundle/build_app.jl -v -R $HOME/.julia/v0.6/Blink/src/AtomShell/main.js -R $HOME/.julia/v0.6/Blink/deps/Julia.app  --certificate "Developer ID Application: nhdalyMadeThis, LLC"  --app_version=0.1 "main.jl" "MyAppThatUsesBlink"

(I haven't finished testing it, but I think those are the only two files you need for Blink.)


But you're right, currently the code you write has to include something like I did in Paddle Battle in order to use the provided libraries:
https://github.com/NHDaly/PaddleBattleJL/blob/c98cde1164e8da2d667837826e1b01246ac944da/main.jl#L9-L18

For Blink, I think it would be:

using Blink
if get(ENV, "COMPILING_APPLE_BUNDLE", "false") == "true"
    eval(Blink, :(_electron = "Julia.app/Contents/MacOS/Julia"))
    eval(Blink, :(mainjs = "main.js"))
end

So are you asking to have the tool automatically generate the above code and insert it into the project before running juliac? That would be cool, but also seems like it could be complicated to cover everyone's usecase.

It's particularly difficult, because I think you need to add this section after the using Blink, otherwise your definition will be overwritten by the module. :/

@NHDaly
Copy link
Owner

NHDaly commented Apr 11, 2018

But yes, i am definitely hoping to have this be generalizable!!

My goal is that you can simply run this tool against your julia code (maybe having to make a few changes like the one above), and produce an app you can distribute!

And I'd obviously love your/everyone's help on that!!

@NHDaly
Copy link
Owner

NHDaly commented Apr 11, 2018

And actually another thing that would be good to do would be to automatically update the dependency paths on any .dylibs you copy over, so you don't have to do that manually as well.

@ranjanan
Copy link
Contributor Author

Hi @NHDaly, thank you so much for your help and detailed comments. I actually managed to bundle Blink as per your instructions and with this package too.

However, there's one particular thing that I think you missed out on. I needed this for my app to actually work. We also have to define and run this function so that when we open the app, the PWD is appropriately changed. Also, just to complete the list, Blink needs a few other files, besides also having a dependency in HttpParser which needs a binary too. These different dependencies can only be found if you actually look at the source code for Blink and HttpParser. So I agree with you in that it's quite difficult to automate that.

As for generalisability, I think it's fine for now that we should manually change the internal paths. Perhaps it is too difficult to automate that in every case. I would also like my app to work on Windows and Linux and I shall provide updates on those two as well. I'll try to get those two platforms to work as well and maybe we can come up with an umbrella package for all three platforms.

@NHDaly
Copy link
Owner

NHDaly commented Apr 14, 2018

!! That's amazing!! I'm glad to hear you got the bundle to work!! :D if you want anyone to beta test it, feel free to send it to me! :) I've actually been trying to bundle a Blink program myself as well, so I would really appreciate if you could send the list of binary dependencies you had to copy?

Perhaps you could also post the command you used to execute this script? :D

We also have to define and run this function so that when we open the app, the PWD is appropriately changed.

Ah, yes, whoops good call. I forgot about that one too! Perhaps we could at least provide that as a utility function so that user code could simply say using AppBundler; AppBundler.chdir_if_bundle() or whatever? :)

There was some talk of migrating this code into PackageCompiler as well. Wherever it goes, providing some reusable functions seems like a good idea!

@NHDaly
Copy link
Owner

NHDaly commented Apr 14, 2018

I would also like my app to work on Windows and Linux and I shall provide updates on those two as well. I'll try to get those two platforms to work as well and maybe we can come up with an umbrella package for all three platforms.

Yay! Yeah, same. I think the linux one will probably be easiest, but I'm not sure. I would love it if you'd push up any of your contributions so we can work on them together!! :) Thanks, this is very exciting! :)

@ranjanan
Copy link
Contributor Author

ranjanan commented Apr 16, 2018

you could send the list of binary dependencies you had to copy?

if get(ENV, "COMPILING_APPLE_BUNDLE", "false") == "true"
     println("GOING TO CHANGE THEM PATHS!")
     eval(Blink.AtomShell, :(_electron = "Julia.app/Contents/MacOS/Julia"))
     eval(Blink.AtomShell, :(mainjs = "main.js"))
     eval(Blink, :(buzz = "main.html"))
     eval(Blink, :(resources = Dict("spinner.css" => "res/spinner.css",
                                    "blink.js" => "res/blink.js",
                                    "blink.css" => "res/blink.css",
                                    "reset.css" => "res/reset.css")))
     eval(HttpParser, :(lib = "libhttp_parser.dylib"))
     println("CHANGED ALL PATHS!")
 end

so that user code could simply say using AppBundler; AppBundler.chdir_if_bundle() or whatever? :)

Sure, this should be a utility provided by a package. I would say a Bundler.jl which can create Windows, Mac and Linux distributables.

There was some talk of migrating this code into PackageCompiler as well. Wherever it goes, providing some reusable functions seems like a good idea!

I think this should remain a separate package by itself, because it addresses package and shipping whereas PackageCompiler addresses compilation.

I would love it if you'd push up any of your contributions so we can work on them together!!

For sure. I shall keep you posted on any progress I make on the other platforms.

@NHDaly
Copy link
Owner

NHDaly commented Apr 17, 2018

We also have to define and run this function so that when we open the app, the PWD is appropriately changed.

Hmm, are you also adding this JULIA_HOME fix?:
JuliaLang/PackageCompiler.jl#47 (comment)

It seems I need that in order to get Blink.jl to compile. But when I add those lines (Sys.__init__(); Base.early_init()), it seems that the trick I was using in the function you linked to stops working! That is, the compiled binary reports /Applications/Julia-0.6.app/Contents/Resources/julia/bin/julia as the first item in Base.julia_cmd() (even if I move Julia-06.app so its unaccessible!)...

How did you get around that? Are you using a different trick to detect the location?

@NHDaly
Copy link
Owner

NHDaly commented Apr 18, 2018

(Also, i've made this a package, and called it ApplicationBuilder.jl. I copied change_dir_if_bundle() in there, but I'm still not sure it will work as-is, given the above problem I mentioned.)

@ranjanan
Copy link
Contributor Author

dding this JULIA_HOME fix?

Yes, I've had to hard code that path in the BinDeps package for it to work.

I was using in the function you linked to stops working

Does it? I didn't notice that. Gosh. I ran into a different problem now in Blink that I didn't notice before. Blink.resources doesn't seem to change. I'm going to try a bunch of hacks so that it does.

i've made this a package

Fantastic! I was going to suggest that you do. That way I can contribute to this directly.

I copied change_dir_if_bundle()

I just realised. The problem with doing import ApplicationBundler: change_dir_if_bundle() and then using it in code is that PackageCompiler would try to statically compile ApplicationBundler too, which in turn depends on PackageCompiler. Not sure how well that work.....

@NHDaly
Copy link
Owner

NHDaly commented Apr 18, 2018

I just realised. The problem with doing import ApplicationBundler: change_dir_if_bundle() and then using it in code is that PackageCompiler would try to statically compile ApplicationBundler too, which in turn depends on PackageCompiler. Not sure how well that work.....

haha oh man, that's true. ... well, it should hopefully be fine because (at least for now), the ApplicationBundler module doesn't actually use PackageCompiler, only the build_app.jl script in the root directory does. I've just tested it, and at least the simple hello.jl app worked no problem. But yeah, maybe we'll have to rethink the organization a bit.

@NHDaly
Copy link
Owner

NHDaly commented Apr 23, 2018

Okay, I fixed the change_dir_if_bundle() problems here: #4

You should be able to simply use ApplicationBundler.change_dir_if_bundle() to get the correct behavior! :)

@NHDaly NHDaly added the tracking Issues that are used to document ongoing work on an issue/topic label Apr 24, 2018
@ranjanan
Copy link
Contributor Author

@NHDaly Just an update: I've been working on some shipping issues relating to UI components. Usually, packages that want to serve assets put it into their Pkg.dir()/assets/ folder, and this is more or less hard coded. It wasn't possible to serve up assets from arbitrary file locations (which we need to do while shipping). We just recently came up with a solution involving an Asset Registry where you can register assets at arbitrary paths. There are still a couple of implementation issues, but once those are ironed out, we should be able to ship Blink+Application+UIs

@NHDaly
Copy link
Owner

NHDaly commented Jun 9, 2018

Hi, sorry for the late response. We've just finished our move to Rhode Island, but we're still setting up the apartment. It will be nice to be settled!

Huh, the AssetRegistry.jl seems neat! I'd considered something similar, which was that it would be nice if packages could declare what assets they're using so that when building an Application, we could programmatically find them, copy them, and replace their paths in those packages. Is that the kind of functionality you're targeting, or are you imagining solving the asset problem in a different way that I'm not understanding? :)


Usually, packages that want to serve assets put it into their Pkg.dir()/assets/ folder, and this is more or less hard coded. It wasn't possible to serve up assets from arbitrary file locations (which we need to do while shipping).

What do you mean by which we need to do while shipping? Do you mean that, currently (without AssetRegistry), in order to ship an Application that relies on Blink, we need to override Blink's asset locations within the package via those eval(Blink, assetX=...) statements?

If so, how will AssetRegistry help with that? :) I'm excited to find out!

@ranjanan
Copy link
Contributor Author

We've just finished our move to Rhode Island, but we're still setting up the apartment.

No problem! Take care, and I hope all goes well. :-)

With regards to AssetRegistry, what happens is that earlier, UI packages were made to do to place assets (such as CSS files) in the following directory: Pkg.dir("PackageName")/assets. Internally, the packages refer to their paths like this /pkg/PackageName/ (here's an example). The trouble with this is that when we ship our app, we'll want to change that path. So AssetRegistry allows you to register packages at arbitrary file paths. So, that particular example gets modified to:

const path = `path/to/asset`
function __init__()
    tachyons_css[] = Scope(imports = path)
end

This would let us modify the path variable at build time to be relative path that we wish for (namely res/tachyons.min.css) and the application should just work. This is the kind of solution that AssetRegistry enables. Forgive me, I guess I didn't explain it very well the last time.

What do you mean by which we need to do while shipping?

I simply meant that we need to change file paths in our build script through eval statements.

Blink, we need to override Blink's asset locations within the package via those eval(Blink, assetX=...) statements?

So the thing with Blink is that it seems to be implemented in a way such that the assets are in a different folder (Pkg.dir("Blink")/res/) and Blink picks these resources at runtime. The only modification Blink needs at this point to seamlessly enable compilation without hacks, is for these paths to be picked at compile time (that is, as part of module code) because they'd be overwritten at runtime. The AssetRegistry solution works with UI packages that use the /assets/ folder, which Blink doesn't do exactly.

I wonder if I have explained this very well. BTW, I'd be more than happy to hop on a Hangout to discuss these issues if you think that will help.

@NHDaly NHDaly closed this as completed Jun 14, 2018
@NHDaly NHDaly reopened this Jun 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracking Issues that are used to document ongoing work on an issue/topic
Projects
None yet
Development

No branches or pull requests

2 participants