From 607e0e338aaa1686290252c3b8b028ebc03f61df Mon Sep 17 00:00:00 2001 From: Amos Wenger Date: Tue, 29 Oct 2024 14:02:01 +0100 Subject: [PATCH] Turn on 'strict undefined behavior' For now, this depends on a branch of minijinja, until lands --- Cargo.lock | 3 +-- Cargo.toml | 3 ++- cargo-dist/src/backend/templates.rs | 1 + cargo-dist/src/errors.rs | 18 ++++++++++++++++-- cargo-dist/templates/ci/github/release.yml.j2 | 2 +- cargo-dist/templates/installer/homebrew.rb.j2 | 10 +++++----- cargo-dist/templates/installer/installer.sh.j2 | 2 +- cargo-dist/templates/installer/npm/run.js.j2 | 2 ++ cargo-dist/tests/gallery/repo.rs | 1 + 9 files changed, 30 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b37bb53be..160c9dd39 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1577,8 +1577,7 @@ checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" [[package]] name = "minijinja" version = "2.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c9ca8daf4b0b4029777f1bc6e1aedd1aec7b74c276a43bc6f620a8e1a1c0a90e" +source = "git+https://github.com/mitsuhiko/minijinja?rev=c9fe2eb#c9fe2ebf50ed7d06668c717826e9b0d7ee84a3dc" dependencies = [ "aho-corasick", "memo-map", diff --git a/Cargo.toml b/Cargo.toml index e0087a0df..523eb090d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,7 +43,8 @@ semver = "1.0.23" newline-converter = "0.3.0" dialoguer = "0.11.0" sha2 = "0.10.6" -minijinja = { version = "2.4.0", features = ["debug", "loader", "builtins", "json", "custom_syntax"] } +# until lands +minijinja = { version = "2.4.0", features = ["debug", "loader", "builtins", "json", "custom_syntax"], git = "https://github.com/mitsuhiko/minijinja", rev = "c9fe2eb" } include_dir = "0.7.4" itertools = "0.13.0" cargo-wix = "0.3.8" diff --git a/cargo-dist/src/backend/templates.rs b/cargo-dist/src/backend/templates.rs index 0f4b9c0b9..095b875c7 100644 --- a/cargo-dist/src/backend/templates.rs +++ b/cargo-dist/src/backend/templates.rs @@ -131,6 +131,7 @@ impl Templates { } for env in envs.values_mut() { env.set_debug(true); + env.set_undefined_behavior(minijinja::UndefinedBehavior::Strict); fn jinja_error(details: String) -> std::result::Result { Err(minijinja::Error::new( diff --git a/cargo-dist/src/errors.rs b/cargo-dist/src/errors.rs index 9ae5925dc..605035eab 100644 --- a/cargo-dist/src/errors.rs +++ b/cargo-dist/src/errors.rs @@ -9,7 +9,7 @@ use axoproject::errors::AxoprojectError; use camino::Utf8PathBuf; use cargo_dist_schema::TargetTriple; -use miette::Diagnostic; +use miette::{Diagnostic, SourceOffset, SourceSpan}; use thiserror::Error; /// An alias for the common Result type for this crate @@ -587,7 +587,21 @@ pub struct AxoassetYamlError { impl From for DistError { fn from(details: minijinja::Error) -> Self { let source: String = details.template_source().unwrap_or_default().to_owned(); - let span = details.range().map(|r| r.into()); + let span = details.range().map(|r| r.into()).or_else(|| { + details.line().map(|line| { + // some minijinja errors only have a line, not a range, so let's just highlight the whole line + let start = SourceOffset::from_location(&source, line, 0); + let end = SourceOffset::from_location(&source, line + 1, 0); + let len = (end.offset() - start.offset()).wrapping_sub(1); + SourceSpan::from((start, len)) + }) + }); + + if std::env::var("CARGO_DIST_FORCE_BACKTRACE").as_deref() == Ok("1") { + let bt = std::backtrace::Backtrace::force_capture(); + eprintln!("Cooking up a miette error, backtrace:\n{bt}"); + } + DistError::Jinja { source, span, diff --git a/cargo-dist/templates/ci/github/release.yml.j2 b/cargo-dist/templates/ci/github/release.yml.j2 index 1fd9c0640..a39aa4caf 100644 --- a/cargo-dist/templates/ci/github/release.yml.j2 +++ b/cargo-dist/templates/ci/github/release.yml.j2 @@ -300,7 +300,7 @@ jobs: # Actually do builds and make zips and whatnot dist build ${{ needs.plan.outputs.tag-flag }} --print=linkage --output-format=json ${{ matrix.dist_args }} > dist-manifest.json echo "dist ran successfully" - {{%- if github_attestations %}} + {{%- if github_attestations is defined and github_attestations %}} - name: Attest uses: actions/attest-build-provenance@v1 with: diff --git a/cargo-dist/templates/installer/homebrew.rb.j2 b/cargo-dist/templates/installer/homebrew.rb.j2 index d07705e8b..d93bac558 100644 --- a/cargo-dist/templates/installer/homebrew.rb.j2 +++ b/cargo-dist/templates/installer/homebrew.rb.j2 @@ -36,9 +36,9 @@ class {{ formula_class }} < Formula end {%- endif %} {#- #} - {%- if arm64_linux.id or x86_64_linux.id %} + {%- if arm64_linux.id is defined or x86_64_linux.id is defined %} if OS.linux? - {%- if arm64_linux.id %} + {%- if arm64_linux.id is defined %} if Hardware::CPU.arm? url "{{ inner.base_url }}/{{ arm64_linux.id }}" {%- if arm64_linux %} @@ -46,7 +46,7 @@ class {{ formula_class }} < Formula {%- endif %} end {%- endif %} - {%- if x86_64_linux.id %} + {%- if x86_64_linux.id is defined %} if Hardware::CPU.intel? url "{{ inner.base_url }}/{{ x86_64_linux.id }}" {%- if x86_64_linux_sha256 %} @@ -110,9 +110,9 @@ class {{ formula_class }} < Formula {%- endif %} end {%- endif %} - {%- if arm64_linux.executables or arm64_linux.cdylibs %} + {%- if arm64_linux.executables is defined or arm64_linux.cdylibs is defined %} if OS.linux? && Hardware::CPU.arm? - {%- if arm64_linux.executables %} + {%- if arm64_linux.executables is defined %} bin.install {% for binary in arm64_linux.executables %}"{{ binary }}"{{ ", " if not loop.last else "" }}{% endfor %} {%- endif %} {%- if arm64_linux.cdylibs and "cdylib" in install_libraries %} diff --git a/cargo-dist/templates/installer/installer.sh.j2 b/cargo-dist/templates/installer/installer.sh.j2 index a4120c7fe..0bd7bb2f3 100644 --- a/cargo-dist/templates/installer/installer.sh.j2 +++ b/cargo-dist/templates/installer/installer.sh.j2 @@ -339,7 +339,7 @@ select_archive_for_arch() { "{{ target }}") {%- for option in platform_support.platforms[target] %} _archive="{{platform_support.archives[option.archive_idx].id}}" - {%- if option.runtime_conditions.min_glibc_version %} + {%- if option.runtime_conditions.min_glibc_version is defined %} if ! check_glibc "{{option.runtime_conditions.min_glibc_version.major}}" "{{option.runtime_conditions.min_glibc_version.series}}"; then _archive="" fi diff --git a/cargo-dist/templates/installer/npm/run.js.j2 b/cargo-dist/templates/installer/npm/run.js.j2 index 8d37593f2..a3be07fbc 100644 --- a/cargo-dist/templates/installer/npm/run.js.j2 +++ b/cargo-dist/templates/installer/npm/run.js.j2 @@ -1,4 +1,6 @@ #!/usr/bin/env node const { run } = require("./binary"); +{%- if bin is defined %} run({{ bin }}); +{%- endif %} diff --git a/cargo-dist/tests/gallery/repo.rs b/cargo-dist/tests/gallery/repo.rs index e230da61d..d9738dbe4 100644 --- a/cargo-dist/tests/gallery/repo.rs +++ b/cargo-dist/tests/gallery/repo.rs @@ -142,6 +142,7 @@ where /// Run a test on this repo pub fn run_test(&self, test: impl FnOnce(TestContext) -> Result<()>) -> Result<()> { std::env::set_var("CARGO_DIST_MOCK_NETWORKING", "1"); + std::env::set_var("CARGO_DIST_FORCE_BACKTRACE", "1"); let maybe_tools = self.tools.lock(); let maybe_repo = self.ctx.lock(); // Intentionally unwrapping here to poison the mutexes if we can't fetch