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

feat: deno compile #8539

Merged
merged 17 commits into from
Nov 30, 2020
Merged

feat: deno compile #8539

merged 17 commits into from
Nov 30, 2020

Conversation

lucacasonato
Copy link
Member

@lucacasonato lucacasonato commented Nov 29, 2020

Closes #986

This implements the deno compile subcommand. It works be generating a new standalone binary, by appending a bundle of the source file and magic trailer from the current binary. If we find a magic trailer at the end of the the binary, we try to load the bundle that is embedded in the binary, and run it.

~/p/g/d/deno ❯❯❯ ./target/debug/deno compile https://deno.land/std/http/file_server.ts file_server
Bundle https://deno.land/std/http/file_server.ts
Check https://deno.land/std/http/file_server.ts
Compile https://deno.land/std/http/file_server.ts
~/p/g/d/deno ❯❯❯ ./file_server 
HTTP server listening on http://0.0.0.0:4507/

compile.ts Outdated Show resolved Hide resolved
compile.ts Outdated Show resolved Hide resolved
@lucacasonato lucacasonato changed the title POC deno compile feat: deno compile Nov 29, 2020
cli/flags.rs Outdated Show resolved Hide resolved
cli/standalone.rs Outdated Show resolved Hide resolved
cli/standalone.rs Outdated Show resolved Hide resolved
cli/standalone.rs Outdated Show resolved Hide resolved
cli/standalone.rs Outdated Show resolved Hide resolved
cli/standalone.rs Outdated Show resolved Hide resolved
cli/standalone.rs Show resolved Hide resolved
cli/main.rs Outdated Show resolved Hide resolved
cli/flags.rs Outdated Show resolved Hide resolved
cli/main.rs Outdated Show resolved Hide resolved
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a great first pass, but suggest to get more eyes on it before landing

cli/standalone.rs Outdated Show resolved Hide resolved
@lucacasonato lucacasonato requested a review from ry November 29, 2020 18:49
cli/standalone.rs Outdated Show resolved Hide resolved
flags.unstable = true;
let main_module = ModuleSpecifier::resolve_url(SPECIFIER)?;
let program_state = ProgramState::new(flags.clone())?;
let permissions = Permissions::allow_all();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not for this PR, but I think it would also be nice to allow optional permission flags in the future, where we add an extra trailer for the bit vector of permissions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I have some ideas on how to do this in the future.

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Milestone achievement.

@lucacasonato lucacasonato added this to the 1.6.0 milestone Nov 30, 2020
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@lucacasonato lucacasonato merged commit 6aa692f into denoland:master Nov 30, 2020
@ry ry mentioned this pull request Nov 30, 2020
22 tasks
@jveres
Copy link
Contributor

jveres commented Dec 1, 2020

I tried this new feature and found an issue where deno::program_state::ProgramState::get_emit tries to load file://$deno$/bundle.js which is fake for compiled bundles and deno::disk_cache::DiskCache::get_cache_filename panics. It was trivial to fix by patching cli/program_state/get_emit(...) with this:

fn get_emit(&self, url: &Url) -> Option<(Vec<u8>, Option<Vec<u8>>)> {
    match url.scheme() {
      // we should only be looking for emits for schemes that denote external
      // modules, which the disk_cache supports
      "wasm" | "file" | "http" | "https" => {
        // ignore when called from a self-contained binary
        if url.to_string() == crate::standalone::SPECIFIER {
          return None;
        }
      },
      _ => {
        return None;
      }
    }
...

Not sure if this is the most appropriate way, though.

@bartlomieju
Copy link
Member

I tried this new feature and found an issue where deno::program_state::ProgramState::get_emit tries to load file://$deno$/bundle.js which is fake for compiled bundles and deno::disk_cache::DiskCache::get_cache_filename panics. It was trivial to fix by patching cli/program_state/get_emit(...) with this:

fn get_emit(&self, url: &Url) -> Option<(Vec<u8>, Option<Vec<u8>>)> {
    match url.scheme() {
      // we should only be looking for emits for schemes that denote external
      // modules, which the disk_cache supports
      "wasm" | "file" | "http" | "https" => {
        // ignore when called from a self-contained binary
        if url.to_string() == crate::standalone::SPECIFIER {
          return None;
        }
      },
      _ => {
        return None;
      }
    }
...

Not sure if this is the most appropriate way, though.

@jveres could you open a new issue and provide a reproduction?

@nayeemrmn
Copy link
Collaborator

found an issue where deno::program_state::ProgramState::get_emit tries to load file://$deno$/bundle.js

Yep, the fix is this:

diff --git a/cli/standalone.rs b/cli/standalone.rs
index 06c20bc2..7c9e9fcb 100644
--- a/cli/standalone.rs
+++ b/cli/standalone.rs
@@ -58,7 +58,7 @@ pub fn try_run_standalone_binary(args: Vec<String>) -> Result<(), AnyError> {
   }
 }
 
-const SPECIFIER: &str = "file://$deno$/bundle.js";
+const SPECIFIER: &str = "file:///$deno$bundle.js";
 
 struct EmbeddedModuleLoader(String);

I'm trying to come up with tests.

@bartlomieju
I was able to reproduce with:

import * as path from "./std/path/mod.ts";

Seems pretty random though.

@jveres
Copy link
Contributor

jveres commented Dec 1, 2020

@nayeemrmn quickly checked with file:///$deno$bundle.js but it gives error: Self-contained binaries don't support module loading so maybe it's a different issue.

@bartlomieju
Copy link
Member

CC @lucacasonato

@bartlomieju
Copy link
Member

bartlomieju commented Dec 1, 2020

@nayeemrmn quickly checked with file:///$deno$bundle.js but it gives error: Self-contained binaries don't support module loading so maybe it's a different issue.

@jveres do you have dynamic imports or workers in your code? If so they are not supported

@lucacasonato lucacasonato deleted the standalone branch December 1, 2020 16:49
@jveres
Copy link
Contributor

jveres commented Dec 1, 2020

@nayeemrmn quickly checked with file:///$deno$bundle.js but it gives error: Self-contained binaries don't support module loading so maybe it's a different issue.

@jveres do you have dynamic imports or workers in your code? If so they are not supported

No, I don't. Using my fix above it works.
I will create a test case for this later.

@jveres
Copy link
Contributor

jveres commented Dec 1, 2020

as simple as

const e = new Error();
console.log(e);
deno compile --unstable test.ts test
RUST_BACKTRACE=full ./test
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ()', cli/disk_cache.rs:72:39
stack backtrace:
   0:        0x10b8c56d3 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hd4b962ed89f71a03
   1:        0x10b543b50 - core::fmt::write::h94ae1e793baa7a00
   2:        0x10b8c4dbb - std::io::Write::write_fmt::h5c716758fdc3057f
   3:        0x10b8c4745 - std::panicking::default_hook::{{closure}}::hc6119c7d16548caf
   4:        0x10b8c3e85 - std::panicking::rust_panic_with_hook::hc36596b4257bea99
   5:        0x10b8e133c - std::panicking::begin_panic_handler::{{closure}}::h49e5ddc3f21ca2fb
   6:        0x10b8e1307 - std::sys_common::backtrace::__rust_end_short_backtrace::h9bd32c9ad3fad18f
   7:        0x10b8e12c0 - _rust_begin_unwind
   8:        0x10c60cc4f - core::panicking::panic_fmt::hcdc9362d34d55302
   9:        0x10c60d225 - core::option::expect_none_failed::hef0458611b13747b
  10:        0x10b39962e - core::result::Result<T,E>::unwrap::h5376c858e103cf24
  11:        0x10b48c14d - deno::disk_cache::DiskCache::get_cache_filename_with_extension::h733f5eb5af07bc3e
  12:        0x10b4c086c - deno::program_state::ProgramState::get_emit::h13846ddd08588463
  13:        0x10b4c1514 - deno::source_maps::get_orig_position::hd223e3d5eecacb10
  14:        0x10b44515b - deno_core::ops::json_op_sync::{{closure}}::h1c84ae06d203eb0e
  15:        0x10b3d76c5 - <alloc::boxed::Box<F> as core::ops::function::Fn<A>>::call::h1744985c3c7bbe57
  16:        0x10b4a4917 - deno::metrics::metrics_op::{{closure}}::hfbfe0e6b2fb6b282
  17:        0x10b3d76c5 - <alloc::boxed::Box<F> as core::ops::function::Fn<A>>::call::h1744985c3c7bbe57
  18:        0x10b557b7e - <extern "C" fn(A0) .> R as rusty_v8::support::CFnFrom<F>>::mapping::c_fn::hf0d234ae6b0410d0
  19:        0x10bb6c059 - __ZN2v88internal25FunctionCallbackArguments4CallENS0_15CallHandlerInfoE
  20:        0x10bb6b5c8 - __ZN2v88internal12_GLOBAL__N_119HandleApiCallHelperILb0EEENS0_11MaybeHandleINS0_6ObjectEEEPNS0_7IsolateENS0_6HandleINS0_10HeapObjectEEESA_NS8_INS0_20FunctionTemplateInfoEEENS8_IS4_EENS0_16BuiltinArgumentsE
  21:        0x10bb6ab80 - __ZN2v88internalL26Builtin_Impl_HandleApiCallENS0_16BuiltinArgumentsEPNS0_7IsolateE
fatal runtime error: failed to initiate panic, error 5
[1]    62205 abort      RUST_BACKTRACE=full ./test

@lucacasonato
Copy link
Member Author

@jveres See #8578

@zlianon
Copy link

zlianon commented Dec 13, 2020

output size of file_server is 48MB 😬

@lucacasonato
Copy link
Member Author

@zlianon As discussed in the release post, we are confident we can reduce size to around 20 MB on Linux / macOS. Probably even less on Windows. See https://deno.land/posts/v1.6#codedeno-compilecode-self-contained-standalone-binaries

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.

"deno compile" into executable
7 participants