From 2b4ad47c16dc0fb114be0fd03526ffb871aa1875 Mon Sep 17 00:00:00 2001
From: Linus Heckemann <git@sphalerite.org>
Date: Thu, 1 Jun 2023 21:21:23 +0200
Subject: [PATCH] fix(tvix/eval): emit only warnings on shadowed outputs

Unfortunately, nixpkgs has at least one case[1] where the out environment
variable is shadowed -- though it doesn't cause a problem, since it's
shadowed with the correct value, odd as this may be!

[1]: https://github.com/NixOS/nixpkgs/blob/c7c298471676ac1c7789ab3c424fbcebecaa6791/pkgs/development/python-modules/pybind11/default.nix#L19

Change-Id: Ibf6790d2484dc9cce8e424feeb5886664d498dc3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8696
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
---
 tvix/cli/src/derivation.rs     | 6 +++---
 tvix/cli/src/errors.rs         | 2 --
 tvix/eval/src/vm/generators.rs | 2 +-
 tvix/eval/src/warnings.rs      | 7 +++++++
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/tvix/cli/src/derivation.rs b/tvix/cli/src/derivation.rs
index cf15ebbb0..fa246cc74 100644
--- a/tvix/cli/src/derivation.rs
+++ b/tvix/cli/src/derivation.rs
@@ -5,8 +5,8 @@ use std::cell::RefCell;
 use std::collections::{btree_map, BTreeSet};
 use std::rc::Rc;
 use tvix_eval::builtin_macros::builtins;
-use tvix_eval::generators::{self, GenCo};
-use tvix_eval::{AddContext, CoercionKind, ErrorKind, NixAttrs, NixList, Value};
+use tvix_eval::generators::{self, emit_warning_kind, GenCo};
+use tvix_eval::{AddContext, CoercionKind, ErrorKind, NixAttrs, NixList, Value, WarningKind};
 
 use crate::errors::Error;
 use crate::known_paths::{KnownPaths, PathKind, PathName};
@@ -327,7 +327,7 @@ mod derivation_builtins {
                 .insert(output.to_string(), String::new())
                 .is_some()
             {
-                return Err(Error::ShadowedOutput(output.to_string()).into());
+                emit_warning_kind(&co, WarningKind::ShadowedOutput(output.to_string())).await;
             }
         }
 
diff --git a/tvix/cli/src/errors.rs b/tvix/cli/src/errors.rs
index 6f328dc00..5cbddcbba 100644
--- a/tvix/cli/src/errors.rs
+++ b/tvix/cli/src/errors.rs
@@ -11,8 +11,6 @@ pub enum Error {
     ConflictingOutputTypes,
     #[error("the environment variable '{0}' has already been set in this derivation")]
     DuplicateEnvVar(String),
-    #[error("the environment variable '{0}' shadows the name of an output")]
-    ShadowedOutput(String),
     #[error("invalid derivation parameters: {0}")]
     InvalidDerivation(DerivationError),
     #[error("invalid output hash: {0}")]
diff --git a/tvix/eval/src/vm/generators.rs b/tvix/eval/src/vm/generators.rs
index f86683dff..138600be1 100644
--- a/tvix/eval/src/vm/generators.rs
+++ b/tvix/eval/src/vm/generators.rs
@@ -650,7 +650,7 @@ pub(crate) async fn emit_warning(co: &GenCo, warning: EvalWarning) {
 }
 
 /// Emit a runtime warning with the span of the current generator.
-pub(crate) async fn emit_warning_kind(co: &GenCo, kind: WarningKind) {
+pub async fn emit_warning_kind(co: &GenCo, kind: WarningKind) {
     match co.yield_(VMRequest::EmitWarningKind(kind)).await {
         VMResponse::Empty => {}
         msg => panic!(
diff --git a/tvix/eval/src/warnings.rs b/tvix/eval/src/warnings.rs
index aa567f302..e007d9c34 100644
--- a/tvix/eval/src/warnings.rs
+++ b/tvix/eval/src/warnings.rs
@@ -17,6 +17,7 @@ pub enum WarningKind {
     DeadCode,
     EmptyInherit,
     EmptyLet,
+    ShadowedOutput(String),
 
     /// Tvix internal warning for features triggered by users that are
     /// not actually implemented yet, but do not cause runtime failures.
@@ -100,6 +101,11 @@ impl EvalWarning {
 
             WarningKind::EmptyLet => "this `let`-expression contains no bindings".to_string(),
 
+            WarningKind::ShadowedOutput(ref out) => format!(
+                "this derivation's environment shadows the output name {}",
+                out
+            ),
+
             WarningKind::NotImplemented(what) => {
                 format!("feature not yet implemented in tvix: {}", what)
             }
@@ -120,6 +126,7 @@ impl EvalWarning {
             WarningKind::DeadCode => "W008",
             WarningKind::EmptyInherit => "W009",
             WarningKind::EmptyLet => "W010",
+            WarningKind::ShadowedOutput(_) => "W011",
 
             WarningKind::NotImplemented(_) => "W999",
         }