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

Create and publish app #599

Closed
wants to merge 11 commits into from

Conversation

Octogonapus
Copy link

@Octogonapus Octogonapus commented May 26, 2022

This PR uses PackageCompiler to create a relocatable app. On my system, the time to run the app is short; using it to format this codebase takes 3 seconds. This isn't blazing fast, but it's substantially better than running the REPL.

There are some remaining questions/tasks before this PR should be merged:

  • Fix Windows build
  • Try to fix 32-bit builds
  • @domluna How do you want releases to work? I usually build and release artifacts from tags.

Closes #580. Closes #483.

@domluna
Copy link
Owner

domluna commented May 26, 2022

building releases from tags sounds fine to me, any reason not to do it that way?

@Octogonapus
Copy link
Author

building releases from tags sounds fine to me, any reason not to do it that way?

Not really. I always do it that way.

@domluna
Copy link
Owner

domluna commented May 26, 2022

So how does this work, would someone be able to download it like a binary and run it as a command line tool?

@Octogonapus
Copy link
Author

The easiest way will be to release the entire app's folder, which can be downloaded and run like:

/path/to/JuliaFormatter-0.22.10-x86_64-Linux/bin/JuliaFormatter [PATHS...]

We'll need to release an artifact for each platform-arch combination.

@domluna
Copy link
Owner

domluna commented May 26, 2022

another question, how would this interact with VSCode using JuliaFormatter. VSCode has it's own build release process would these be included in that or is it entirely separate?

@Octogonapus
Copy link
Author

If the vscode extension team wants to include this in their releases, they are welcome to, but I am just focused on getting artifacts published to the releases on this repository.

@domluna
Copy link
Owner

domluna commented May 26, 2022

@davidanthoff would this change anything for you folks?

@Octogonapus
Copy link
Author

I will mention that this would make the extension's formatting faster for the first few runs, but slower overall as this involves invoking a separate binary, doing argument parsing, etc. It would be more optimal to compile the entire vscode extension.

@Roger-luo
Copy link

Roger-luo commented May 26, 2022

This PR is already quite nice, but I guess you didn't know Comonicon, it actually takes care of a CLI application, e.g build system images, generating shell autocompletion, handling the CLI's Julia environment, packing tarballs for the application. I made a simpler subcmd for Ion as ion format.

For windows, I'm wondering how you pack the entire build? I'm not sure it's legit to just ask the user to unzip a set of binaries on Windows? This is why I set up a build script for Ion in this separate package and compile the system image in installation.

For *nix systems, I think at least a tarball makes more sense which is also missing from this PR

@Octogonapus
Copy link
Author

Ok, Comonicon could be a better option here. Though it needs at least one new feature; this cpu target needs to be customizable https://github.com/comonicon/Comonicon.jl/blob/master/src/builder/install.jl#L60.

I'm not sure it's legit to just ask the user to unzip a set of binaries on Windows? This is why I set up a build script for Ion in this separate package and compile the system image in installation.

What is wrong with it? Building a proper installer is a huge pain and asking the user to build it themselves is also problematic because it requires they have the time to run the compiler (which takes about 15 minutes). This means you couldn't use this CLI in CI jobs.

@Roger-luo
Copy link

Roger-luo commented May 26, 2022

this cpu target needs to be customizable https://github.com/comonicon/Comonicon.jl/blob/master/src/builder/install.jl#L60.

That's not the build pipeline for applications, because of the packing issue, there are two modes, one builds the system image locally, the other builds the tarball
https://github.com/comonicon/Comonicon.jl/blob/master/src/builder/app.jl#L20

the builder CLI has the corresponding help msg for this, basically

julia --project deps/build.jl app tarball

and you can of course configure the build target for applications in Comonicon.toml

[application]
cpu_target="whatever"

but this is not necessary, as I already handled it for you https://github.com/comonicon/Comonicon.jl/blob/master/src/configs.jl#L90

What is wrong with it? Building a proper installer is a huge pain.

I think for *nix, a tarball or single executable is fine, I'm not a Windows expert, but I hardly see any windows applications installed directly by copying a folder. But I can be wrong. So if the intention is to build the application as an official release of this package on Windows I'd say it's better to do the recommended way otherwise you can just build and host it on your own (I think depends on @domluna )

But at least do a tarball and checksum for *nix build in this PR. This at least helps checking if the download is complete with the original files.

and asking the user to build it themselves is also problematic because it requires they have the time to run the compiler (which takes about 15 minutes). This means you couldn't use this CLI in CI jobs.

Yes, I understand this is annoying, but Julia doesn't have a mature toolchain for releasing binaries. Comonicon has a feature to download the system image from GitHub release CDN, but that was painful since it's not compatible with things like JLLs. On the other hand, the built application sometimes has issues (compile result is incorrect) after the tag on CI, which is why I decided not to release built apps for IonCLI until it gets more mature: https://github.com/Roger-luo/IonCLI.jl/releases.

If we can get stable binaries packed and released on all platforms, no one would want to do so.

@Roger-luo
Copy link

and FYI: JuliaLang/Pkg.jl#1962

@pfitzseb
Copy link
Contributor

another question, how would this interact with VSCode using JuliaFormatter. VSCode has it's own build release process would these be included in that or is it entirely separate?

This shouldn't impact the extension at all since we're just loading JuliaFormatter as a package. It would be nice not to have to deal with any additional dependencies, but using Comonicon seems like it would solve that issue.

@Roger-luo
Copy link

to use Comonicon for Windows, I just need someone to tell me what to do with the installer, or that's not a thing comonicon/Comonicon.jl#175 if that's not a thing, we can just pack a zip file instead of erroring

@davidanthoff
Copy link
Contributor

I think in general it would be nicer if julia_main wasn't part of the JuliaFormatter package, but instead there could be a new folder app which as its own Project.toml and Manifest.toml for the actual application, and a main.jl with the definition of julia_main in it. That would keep any additional deps for the application out of the JuliaFormatter package (like ArgParse right now). Any additional dep like ArgParse causes some pain on the VS Code side :)

@Octogonapus
Copy link
Author

@Roger-luo So if I understand these docs correctly, the best way to approach this project is to create a JuliaFormatterCLI.jl project separate from this one. That project can release tarballs containing the CLI binaries. Users could also add that project and run the installer themselves for other use cases.

@domluna Does this approach work for you?

I also believe this approach addresses the concerns of @pfitzseb and @davidanthoff as it moves all the changes out of this project.

@domluna
Copy link
Owner

domluna commented May 26, 2022

@Roger-luo So if I understand these docs correctly, the best way to approach this project is to create a JuliaFormatterCLI.jl project separate from this one. That project can release tarballs containing the CLI binaries. Users could also add that project and run the installer themselves for other use cases.

@domluna Does this approach work for you?

I also believe this approach addresses the concerns of @pfitzseb and @davidanthoff as it moves all the changes out of this project.

sounds like a very good compromise to me

@Roger-luo
Copy link

Yes, the best way to build a CLI application is to have it inside a project module. It can be inside either JuliaFormatter or a new package JuliaFormatterCLI. But I guess to address the concern on VSCode side, we can do it in JuliaFormaterCLI.

@Octogonapus
Copy link
Author

I recently had time to revisit the rewrite of this project. It currently lives in this repo. The proof of concept is implemented. I'll be refining it over the next weeks.

@MilesCranmer
Copy link
Contributor

Status of this? Would be great to have a julia_format command that wraps a CLI around JuliaFormatter.format with next-to-zero startup time. I think Comonicon.jl is a good choice and is pretty simple to set up.

@IanButterworth
Copy link

I believe the native code caching in 1.9 will make this work out of the box. Load time will be a little slower than a sysimage though. Worth measuring

@Octogonapus
Copy link
Author

I got a build going based on Comonicon. It works, but the problem is the sysimage size. It's so big that downloading it in a CI job (my main use case) is slower then just eating the time-to-first-format.

@MilesCranmer
Copy link
Contributor

Do the settings incremental=true, filter_stdlibs=true help at all?

@Octogonapus
Copy link
Author

Using non-incremental mode would increase the binary size.
Filtering stdlibs reduced the size from 388 MB to 385 MB.

@domluna
Copy link
Owner

domluna commented May 7, 2023

I believe the native code caching in 1.9 will make this work out of the box. Load time will be a little slower than a sysimage though. Worth measuring

how does that work?

@Octogonapus
Copy link
Author

I don't think Julia is in a state to make this PR go anywhere useful. Maybe in the future the binaries can be small enough.

@Octogonapus Octogonapus closed this Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create and publish app Using JuliaFormatter as an executable (for usage with ALE)
7 participants