Deno—an expression in need of love

Hi :slight_smile:

I’m looking for feedback on how I can improve this working nix expression for Deno.

There is an open issue on GitHub(#78461) for packaging Deno, and below is an expression that successfully builds it from source (warning: takes 49 minutes on my machine to compile!):

{ pkgs, ... } @ args: let
  #
  # @TODO: this hack shoves the llvm-* binaries alongside the clang/clang++
  #        binaries so we can reference them in the CLANG_BASE_PATH env
  #        var for the [rusty_v8 package][rv8].
  #
  # [rv8]: https://github.com/denoland/rusty_v8
  #
  llvm-with-clang = pkgs.stdenv.mkDerivation {
    name = "llvm-with-clang";
    srcs = [ pkgs.clang_8 pkgs.llvmPackages.bintools ];
    unpackPhase = ''
      runHook preUnpack

      for _src in $srcs; do
        cp -r "$_src" $(stripHash "$_src")
      done

      runHook postUnpack
    '';
    installPhase = ''
      mkdir -p $out/bin

      for _src in $srcs; do
        cp -r $(stripHash "$_src")/bin/* $out/bin
      done
    '';
  };
in pkgs.rustPlatform.buildRustPackage rec {
  name = "deno";

  verbose = true;

  buildInputs = [
    pkgs.pkgconfig
    pkgs.python2Full
    pkgs.glib
    pkgs.gn
    pkgs.ninja
    pkgs.zlib
    llvm-with-clang
  ];

  #
  # @TODO: skipping tests for now ~ the deno tests panic in some cases:
  #
  #   ```
  #   test result: FAILED. 211 passed; 10 failed; 0 ignored; 0 measured; 0 filtered out
  #   ```
  #
  # e.g.
  #
  #   ```
  #   ---- compilers::ts::tests::test_bundle stdout ----
  #   thread 'compilers::ts::tests::test_bundle' panicked at
  #   'called `Result::unwrap()` on an `Err` value:
  #   ErrBox(Os { code: 13, kind: PermissionDenied, message: "Permission denied" })',
  #   src/libcore/result.rs:1188:5
  #   ```
  #
  checkPhase = ''
    echo "skipping checkPhase"
  '';

  src = pkgs.fetchgit {
    url = "https://github.com/denoland/deno.git";
    # v0.42.0
    # @TODO: update `postUnpack` if you update the version!
    rev = "f92bb9cf4de6ed4b6a84a14775e0dbe4ed45291a";
    sha256 = "156w1x0kphb2r9xxnwyk2cqxpyf43w1dy9jd544xpydl09p96pql";
    fetchSubmodules = true;
  };

  cargoSha256 = "082wnld3vmyhla666cpf521yrl6bxa3q5mi6xkh62im7xh40mbhn";

  #
  # @TODO: This is a hack to recreate the necessary dotfiles to build
  #        the rusty_v8 dependency, namely, `.gn`
  #
  # When `cargo vendor` is called, it ignores dot-files that exist in
  # the rusty_v8 package / source code.
  #
  # I have a hunch that it's executing this line in cargo:
  #
  #  --> https://github.com/rust-lang/cargo/blob/bf1a26d355f2b12230eeef37b1e8617d90d2db99/src/cargo/sources/path.rs#L167
  #
  # which explicitally ignores dot-files and directories.
  #
  # More information here:
  #
  # --> https://github.com/rust-lang/cargo/issues/7183
  #
  postUnpack = ''
    cargoDepsCopy=$(stripHash $(basename $cargoDeps))

    cat > $cargoDepsCopy/rusty_v8/.clang-format <<EOL
    BasedOnStyle: google
    EOL

    cat > $cargoDepsCopy/rusty_v8/.gn <<EOL
    # This file is used by the GN meta build system to find the root of the source
    # tree and to set startup options. For documentation on the values set in this
    # file, run "gn help dotfile" at the command line.

    # The location of the build configuration file.
    buildconfig = "//build/config/BUILDCONFIG.gn"

    # These are the targets to check headers for by default. The files in targets
    # matching these patterns (see "gn help label_pattern" for format) will have
    # their includes checked for proper dependencies when you run either
    # "gn check" or "gn gen --check".
    check_targets = []

    # The secondary source root is a parallel directory tree where
    # GN build files are placed when they can not be placed directly
    # in the source tree, e.g. for third party source trees.
    secondary_source = "//v8/"

    default_args = {
      linux_use_bundled_binutils = false
      use_sysroot = false

      use_dummy_lastchange = true
      treat_warnings_as_errors = true

      # TODO(ry) remove
      v8_imminent_deprecation_warnings = false

      # https://cs.chromium.org/chromium/src/docs/ccache_mac.md
      clang_use_chrome_plugins = false
      v8_enable_i18n_support = false
      v8_monolithic = false
      v8_use_external_startup_data = false
      v8_use_snapshot = true
      is_component_build = false
      symbol_level = 1
    }
    EOL

    unset cargoDepsCopy
  '';

  # ==================================================
  # Environment Variables
  #
  V8_FROM_SOURCE = "1";
  DENO_GN_PATH = "${pkgs.gn}/bin";
  DENO_NINJA_PATH = "${pkgs.ninja}/bin";
  NINJA = "${pkgs.ninja}/bin/ninja";
  CLANG_BASE_PATH = "${llvm-with-clang}";
  GN = "${pkgs.gn}/bin/gn";
  GN_ARGS = "no_inline_line_tables=false";
  #
  # ==================================================

  meta = with pkgs.stdenv.lib; {
    description = "A secure runtime for JavaScript and TypeScript.";
    longDescription = ''
      Deno aims to provide a productive and secure scripting environment
      for the modern programmer. It is built on top of V8, Rust, and
      TypeScript.
    '';
    homepage = "https://deno.land";
    changelog = "https://github.com/denoland/deno/blob/master/Releases.md";
    license = licenses.mit;
    maintainers = [ ];
    platforms = platforms.linux;
  };
}

There are multiple hacks going on here:

1. combining llvm-* and clang binaries

The cargo package rusty_v8 let’s users set a CLANG_BASE_PATH environment variable to looks for both clang/clang++ and llvm-ar binaries. The llvm-with-clang derivation is a workaround for this lookup.

Also note that, the version of clang & llvm with this hack are different lol:

[/nix/store/*-llvm-with-clang]$ ./bin/clang --version
clang version 8.0.1 (tags/RELEASE_801/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /nix/store/bv17ssmbij00i2xdq4s1xkld1ix2g3b3-clang-8.0.1/bin

[/nix/store/*-llvm-with-clang]$ ./bin/llvm-ar --version
LLVM (http://llvm.org/):
  LLVM version 7.1.0
  Optimized build.
  Default target: x86_64-unknown-linux-gnu
  Host CPU: skylake

but it works so :woman_shrugging:

2. skipping test cases (checkPhase)

Unfortunately, 10 of the tests fail to pass with cargo test:

test result: FAILED. 211 passed; 10 failed; 0 ignored; 0 measured; 0 filtered out

What’s the common approach here when dealing with packages who’s test cases are, well, not friendly?

3. re-creating “dotfiles” that cargo vendor drops

I don’t think this is an issue with Nix at all — see comments above postUnpack.

This is more of a maintenance nightmare — whenever anyone updates rev in deno’s fetchgit, they have to make sure that these re-created files are in-sync with the current deno codebase.

Finishing Thoughts

I’d love to bring Deno to Nix, but I can’t realistically open a PR. This is my first Nix package for a 3rd party project, and, I don’t use GitHub.

The goal is that someone can help champion this into the nixpkgs repository on my behalf (after the hacks are cleaned up a bit), and, I learn a heck-of-a-lot from interacting with folks on this forum ^_^.

Help a Nix novice today!

TTFN~ :heart:

3 Likes

Have you tried clang-wrapped?

I think that’s not safe but I’m no expert…

Usually we like to have as much tests as possible. Could you check what tests fail and disable them individually? I’m not sure how to do that with cargo but I think it’s possible.

That cargo issue you mentioned - was fixed quiet some time ago but not released yet? I think it’s legitimate to put such a hack considering it’s an upstream thing but only if it’s not fixed / released.

I was just about to suggest you to open a PR and so we’ll be able to help you with the code at hand. Just a while ago, there was:

LOL :upside_down_face:.

2 Likes

hey-yo.

definitely tried clang-wrapper, though, not sure if that’s the same thing.

:sweat_smile:

It looks like cargo test kind of supports this (with the --test <NAME> flag), but not sure if you can shove in multiple --test flags and have it pick that up. I will have to spend more time looking into this.

I’ve compiled an exhaustive list of the working test names, along with the failing ones in deno 0.42. (211 passed; 10 failed)

working tests:

compilers::ts::tests::test_source_code_version_hash
diagnostics::tests::from_emit_result
compilers::ts::tests::test_compiler_config_load
diagnostics::tests::from_emit_result_none
diagnostics::tests::test_format_none_frame
diagnostics::tests::from_json
disk_cache::tests::test_create_cache_if_dir_exits
flags::tests::upgrade
flags::tests::types
flags::tests::tsconfig
flags::tests::test_with_allow_net
flags::tests::test_filter
flags::tests::script_args
flags::tests::run_v8_flags
flags::tests::run_with_cafile
flags::tests::run_seed_with_v8_flags
flags::tests::run_seed
flags::tests::run_reload_allow_write
flags::tests::run_reload
flags::tests::run_importmap
flags::tests::repl_with_inspect
flags::tests::repl_with_cafile
flags::tests::repl
flags::tests::quiet
flags::tests::no_remote
flags::tests::log_level
flags::tests::lock_write
flags::tests::install_with_args_and_dir_and_force
flags::tests::install_with_cafile
flags::tests::install_with_args
flags::tests::install
flags::tests::fmt
flags::tests::inspect_default_host
flags::tests::info_with_cafile
flags::tests::info
flags::tests::eval_with_v8_flags
flags::tests::eval_with_inspect
flags::tests::eval_with_cafile
file_fetcher::tests::test_get_source_code_3
flags::tests::eval_typescript
flags::tests::eval
flags::tests::double_hyphen
flags::tests::doc
flags::tests::default_to_run_with_permissions
flags::tests::default_to_run_importmap
flags::tests::default_to_run
flags::tests::completions
flags::tests::cached_only
flags::tests::cache_with_cafile
flags::tests::cache_multiple
flags::tests::cache_importmap
flags::tests::cache
flags::tests::bundle_with_output
flags::tests::bundle_with_cafile
flags::tests::bundle
flags::tests::arg_hacks_test
flags::tests::allow_write_whitelist
flags::tests::allow_read_whitelist
flags::tests::allow_read
flags::tests::allow_net_whitelist_with_ports
flags::tests::allow_net_whitelist
flags::tests::allow_hrtime
flags::tests::allow_all
file_fetcher::tests::test_resolve_module_3
disk_cache::tests::test_create_cache_if_dir_not_exits
disk_cache::tests::test_get_cache_filename_with_extension
disk_cache::tests::test_get_cache_filename
compilers::ts::tests::test_compile_js
diagnostics::tests::test_format_some_frame
diagnostics::tests::diagnostic_to_string1
diagnostics::tests::diagnostic_to_string2
doc::tests::export_const
doc::tests::export_interface2
doc::tests::export_fn2
doc::tests::export_fn
doc::tests::export_type_alias
doc::tests::export_interface
doc::tests::export_enum
doc::tests::declare_namespace
file_fetcher::tests::test_fetch_local_file_no_panic
doc::tests::export_class
doc::tests::optional_return_type
file_fetcher::tests::test_cache_blacklist
file_fetcher::tests::test_fetch_source_file
doc::tests::reexports
doc::tests::export_namespace
file_fetcher::tests::test_fetch_source_file_1
file_fetcher::tests::test_filter_shebang
file_fetcher::tests::test_fetch_source_file_2
file_fetcher::tests::test_fetch_source_0
file_fetcher::tests::test_fetch_with_etag
file_fetcher::tests::test_fetch_with_types_header
file_fetcher::tests::test_fetch_source_2
file_fetcher::tests::test_fetch_with_types_reference
file_fetcher::tests::test_get_source_code_1
file_fetcher::tests::test_get_source_cached_only
file_fetcher::tests::test_get_source_code_2
file_fetcher::tests::test_get_types_url_1
file_fetcher::tests::test_get_types_url_2
file_fetcher::tests::test_get_types_url_3
file_fetcher::tests::test_get_types_url_4
file_fetcher::tests::test_get_types_url_5
file_fetcher::tests::test_get_types_url_6
file_fetcher::tests::test_map_content_type_extension_only
file_fetcher::tests::test_map_content_type_media_type_with_no_extension
file_fetcher::tests::test_map_file_extension
file_fetcher::tests::test_map_file_extension_media_type_with_extension
test flags::tests::version
fmt::test_is_supported
fmt_errors::tests::test_format_none_source_line
fmt_errors::tests::test_format_some_source_line
fs::tests::resolve_from_cwd_absolute
fs::tests::resolve_from_cwd_child
fs::tests::resolve_from_cwd_dot
fs::tests::resolve_from_cwd_parent
fs::tests::test_normalize_path
global_state::import_map_given_for_repl
http_cache::tests::test_create_cache
http_cache::tests::test_get_set
http_cache::tests::test_url_to_filename
file_fetcher::tests::test_get_source_code_4
file_fetcher::tests::test_get_source_code_5
file_fetcher::tests::test_get_source_code_6
fmt::check_tests_dir
file_fetcher::tests::test_get_source_code_7
file_fetcher::tests::test_get_source_code_multiple_downloads_of_same_file
file_fetcher::tests::test_get_source_no_remote
http_util::tests::test_fetch_brotli
http_util::tests::test_fetch_gzip
http_util::tests::test_resolve_url_from_location_full_1
http_util::tests::test_resolve_url_from_location_full_2
http_util::tests::test_resolve_url_from_location_relative_1
http_util::tests::test_resolve_url_from_location_relative_2
http_util::tests::test_resolve_url_from_location_relative_3
import_map::tests::cant_resolve_to_built_in
import_map::tests::from_json_1
import_map::tests::from_json_2
import_map::tests::load_nonexistent
import_map::tests::parse_addresses_absolute_with_fetch_schemes
import_map::tests::parse_addresses_absolute_with_fetch_schemes_arrays
import_map::tests::parse_addresses_mismatched_trailing_slashes
import_map::tests::parse_addresses_mismatched_trailing_slashes_array
import_map::tests::parse_addresses_mismatched_trailing_slashes_with_nonmismatched_array
import_map::tests::parse_addresses_other_invalid
import_map::tests::parse_addresses_relative_url_like
import_map::tests::parse_addresses_unparseable
import_map::tests::parse_addresses_unparseable_arrays
import_map::tests::parse_scope_keys_absolute
import_map::tests::parse_scope_keys_relative
import_map::tests::parse_specifier_keys_absolute
import_map::tests::parse_specifier_keys_relative
import_map::tests::resolve_builtins_remap
import_map::tests::resolve_imports_mapped
import_map::tests::resolve_imports_overlapping_entities_with_trailing_slashes
import_map::tests::resolve_imports_package_like_modules
import_map::tests::resolve_imports_tricky_specifiers
import_map::tests::resolve_imports_url_like_specifier
import_map::tests::resolve_scopes_exact_vs_prefix_matching
import_map::tests::resolve_scopes_inheritance
import_map::tests::resolve_scopes_map_to_empty_array
import_map::tests::resolve_scopes_only_exact_in_map
import_map::tests::resolve_scopes_only_prefix_in_map
import_map::tests::resolve_scopes_package_like
import_map::tests::resolve_scopes_relative_url_keys
import_map::tests::resolve_unmapped_absolute_specifiers
import_map::tests::resolve_unmapped_bad_specifiers
import_map::tests::resolve_unmapped_relative_specifiers
http_util::tests::test_fetch_once_with_redirect
installer::tests::install_custom_dir_env_var
installer::tests::install_basic
installer::tests::install_custom_dir_option
installer::tests::install_force
installer::tests::install_local_module
installer::tests::test_is_remote_url
installer::tests::install_with_flags
http_util::tests::test_fetch_string
op_error::tests::test_bad_resource
op_error::tests::test_bad_resource_id
op_error::tests::test_import_map_error
op_error::tests::test_io_error
op_error::tests::test_permission_denied
op_error::tests::test_simple_error
op_error::tests::test_url_error
ops::dispatch_minimal::test_error_record
ops::dispatch_minimal::test_parse_min_record
permissions::tests::check_paths
permissions::tests::test_check_net
permissions::tests::test_permission_request_net
permissions::tests::test_permissions_request_env
permissions::tests::test_permissions_request_hrtime
permissions::tests::test_permissions_request_plugin
permissions::tests::test_permissions_request_read
permissions::tests::test_permissions_request_run
permissions::tests::test_permissions_request_write
resolve_addr::tests::resolve_addr1
resolve_addr::tests::resolve_addr2
resolve_addr::tests::resolve_addr3
resolve_addr::tests::resolve_addr_ipv6
source_maps::tests::apply_source_map_line
test_runner::tests::supports_dirs
test_runner::tests::test_is_supported
test_runner::tests::test_prepare_test_modules_urls
upgrade::test_compose_url_to_exec
upgrade::test_find_version
js::cli_snapshot
http_util::tests::test_fetch_with_cafile_brotli
http_util::tests::test_fetch_with_cafile_gzip
http_util::tests::test_fetch_with_cafile_string
js::compiler_snapshot
http_util::tests::test_fetch_with_cafile_with_etag
http_util::tests::test_fetch_with_etag

and the failing ones:

test compilers::ts::tests::test_bundle ... FAILED
test compilers::ts::tests::test_compile ... FAILED
test global_state::thread_safe ... FAILED
test web_worker::tests::removed_from_resource_table_on_close ... FAILED
test web_worker::tests::test_worker_messages ... FAILED
test worker::tests::execute_006_url_imports ... FAILED
test worker::tests::execute_mod_002_hello ... FAILED
test worker::tests::execute_mod_circular ... FAILED
test worker::tests::execute_mod_esm_imports_a ... FAILED
test worker::tests::execute_mod_resolve_error ... FAILED

ah cool.

it is released, but, only addresses projects that use [include] in Cargo.toml. Deno makes use of the [exclude] field instead, and includes things with “!” — I think this is a bug in Cargo itself, but, can’t quite say for sure.

:sob:

Maybe it’ll be better to use something like this:

This way you can be rather sure that mixed versions are not used together.

I’d try as hard as possible to avoid putting such a list in an expression. Now that I’ve looked for it, it seems that it should be possible to disable certain tests but that might require support from upstream. Refs:

https://www.reddit.com/r/rust/comments/3i1nki/how_to_skip_expensive_tests_with_cargo_test/

I’d also try to investigate why do these tests fail - perhaps consult upstream.

I see. Then perhaps it’ll be worth opening an issue in cargo for this as well? Even if you open an issue and get no response but at least you insert a link to the expression with a small summary, it makes such hacks more acceptable.

1 Like

hello everyone.

as an update to this, someone has packaged it (without compiling v8 from source) here:

https://github.com/NixOS/nixpkgs/pull/87653

this is where I bow out. @doronbehar thanks so much for your help on this.

1 Like

My dearest @britneyspears , see my comment: Deno: init at 1.0.0 by 06kellyjac · Pull Request #87653 · NixOS/nixpkgs · GitHub .

If I had more time, I’d proxy your packaging ideas and even put self effort in packaging deno in the best possible manner, based on your initial effort, but I’m afraid I won’t have time to dive deep into this until July (when my semester will be over).

Just in case one wants to quickly start using Deno with Nix, here’s my bare-bones shell.nix (archive):

# An alternative to the primitive approach below would
# be using `niv`; see https://nixos.org/guides/towards-reproducibility-pinning-nixpkgs.html

# To  get a  specific  version  (older, newer,  later,
# etc.)  of  Deno,   either  update  the  `commitHash`
# variable (see  comment after  "let") or  provide the
# Nixpkgs  github archive  link pinned  to a  specific
# commit on the command  line when calling `nix-shell`
# (see comment after "in").

let
  # git SHA1 commit hash in the NixOS/nixpkgs github repo
  # ( This will get Deno 1.8.3; to get the hash  of the
  #   most recent Nix expression,
  #
  #     1. either go to https://github.com/NixOS/nixpkgs/blob/413e9561f817ae4cbeb870f777ec5a66ab34e76c/pkgs/development/web/deno/default.nix
  #        and search for  the  text "Latest commit" (which
  #        will be followed by the abbreviated commit hash)
  #
  #     2. or  issue  the  command  below  in  your  cloned
  #        Nixpkgs repo  (make  sure the latest changes are
  #        fetched):
  #
  #        git log --oneline --follow -- pkgs/development/web/deno/default.nix | head -1
  #
  # and update the `commitHash` variable below.
  commitHash = "f4593ab";
  pinnedNixpkgsGithubURL = "https://github.com/NixOS/nixpkgs/archive/${commitHash}.tar.gz";

  # The downloaded archive will be (temporarily?) housed in the Nix store
  # e.g., "/nix/store/gk9x7syd0ic6hjrf0fs6y4bsd16zgscg-source"
  fetchedTarball = builtins.fetchTarball pinnedNixpkgsGithubURL;
in
  # If  `nix-shell`  is  simply  called  with  this  Nix
  # expression,  then  the  used Nixpkgs  link  will  be
  # pinned to the `commitHash` above.
  #
  # These are equivalent:
  #
  #     $ nix-shell deno-shell.nix
  #
  #     $ nix-shell -E 'import (builtins.fetchurl "https://raw.githubusercontent.com/toraritte/shell.nixes/main/deno-shell.nix")'
  #
  # A pinned link can also be used directly by
  #
  #     $ nix-shell --arg pkgs 'import (fetchTarball "https://github.com/NixOS/nixpkgs/archive/f4593ab.tar.gz") {}' deno-shell.nix
  #
  # which will override everything above.
  { pkgs ? import fetchedTarball {} }:

  pkgs.mkShell {
    buildInputs = [ pkgs.deno ];
  }