.env loading silently fails under common scenarios

[xlevus]

With the following test:

## example_test.py

import os

def test_potato():
  assert os.getenv("POTATO") == "potato"

def test_cabbage():
  assert os.getenv("CABBAGE") == "cabbage"

## BUILD
python_tests(extra_env_vars=["POTATO"])

This .env file works when running pants test ::

POTATO=potato
CABBAGE=cabbage

So does:

POTATO=potato
CABBAGE=cabbage
PYTHONPATH="/foo/bar:$PYTHONPATH"

But the following does not, and does not warn you there was a problem loading the env file.

CABBAGE=cabbage
PYTHONPATH="/foo/bar:$PYTHONPATH"
POTATO=potato

This is particularly troublesome, as adding PYTHONPATH="...:$PYTHONPATH" to your .env file is recommended to get vscode integration working in the documentation: https://www.pantsbuild.org/docs/setting-up-an-ide#first-party-sources

This seems to be the case for both Pants 2.16 and 2.17 .

[huonw]

Thanks for filing an issue. This is subtle and annoying. To solve the immediate problem, it looks like a working syntax is without the quotes:

CABBAGE=cabbage
PYTHONPATH=/foo/bar:$PYTHONPATH
POTATO=potato

The Pants docs should be fixed to not generate the " while it doesn't work.

The root cause is "scie jump"'s use of dotenvy in a way that silently eats errors (a-scie/jump#163). Despite it being an upstream dep causing the problem, let's leave this open because we'll still need to change scie-pants even once upstream is fixed: to pull in the new version.

---

Investigation:

1. The loading of .env is controlled "Scie jump": scie-pants opts into it:

   scie-pants/package/scie-pants.toml

   Line 4 in 875e8a8

   load_dotenv = true

2. "scie jump" calls out to dotenvy: https://github.com/a-scie/jump/blob/f44ca18f9c6e8a02f8fab84f6e22c8887b5a3267/jump/src/lib.rs#L181-L186 (PROBLEM 1: scie-jump ignores all errors)
3. why does the ordering matter? dotenvy aborts on first error https://github.com/allan2/dotenvy/blob/08e35eea693ca12c4038a226bae2e3144445ba69/dotenv/src/iter.rs#L33, so the "working" version sets both POTATO and CABBAGE before bailing out due to the error, while the "broken" version sets CABBAGE and then bails out, never setting POTATO. (PROBLEM 2: env vars are partially set)

Test program:

[package]
name = "scie-pants-307"
version = "0.1.0"
edition = "2021"

[dependencies]
dotenvy = "0.15.7"

fn main() {
    let file: &[u8] = b"\
CABBAGE=cabbage
PYTHONPATH=\"/foo/bar:$PYTHONPATH\"
POTATO=potato";

    dbg!(
        dotenvy::from_read_iter(file).collect::<Vec<_>>()
    );

    // now actually apply those to this process
    let _ = dbg!(dotenvy::from_read(file));

    let _ = dbg!(std::env::var("CABBAGE"));
    let _ = dbg!(std::env::var("PYTHONPATH"));
    let _ = dbg!(std::env::var("POTATO"));
}

Output of cargo run:

[src/main.rs:7] dotenvy::from_read_iter(file).collect::<Vec<_>>() = [
    Ok(
        (
            "CABBAGE",
            "cabbage",
        ),
    ),
    Err(
        LineParse(
            "\"/foo/bar:$PYTHONPATH\"",
            21,
        ),
    ),
    Ok(
        (
            "POTATO",
            "potato",
        ),
    ),
]
[src/main.rs:12] dotenvy::from_read(file) = Err(
    LineParse(
        "\"/foo/bar:$PYTHONPATH\"",
        21,
    ),
)
[src/main.rs:14] std::env::var("CABBAGE") = Ok(
    "cabbage",
)
[src/main.rs:15] std::env::var("PYTHONPATH") = Err(
    NotPresent,
)
[src/main.rs:16] std::env::var("POTATO") = Err(
    NotPresent,
)

[huonw]

(Also filed pantsbuild/pants#20127 about the doc issue, thank you.)

[huonw]

Update: #319 theoretically gives noisier behaviour with a hard error on problems, but I'm concerned about it being a breaking change.

For instance, someone may have a repo that has an .env that isn't understood by pants and publishing that will cause their builds to break (especially if they have CI that installs the latest scie-pants, unpinned). This could happen "legitimately", such as if the .env file is meant for being read by another tool, that does understand X="Y", and the env vars within it don't affect pants' behaviour.

Thus, I'm thinking it'd be better to wait for at least either:

• Consider supporting VAR="with space and $substitution" in .env a-scie/jump#166
• some way to make it a warning instead of a hard error

[jsirois]

Closed by #339