pantsbuild / scie-pants

Protects your Pants from the elements.
https://www.pantsbuild.org/docs/installation
Apache License 2.0
19 stars 19 forks source link

`exported` array setting in `.pants.boostrap` parsed differently by `pants` than by `./pants` #107

Closed danxmoran closed 1 year ago

danxmoran commented 1 year ago

In our .pants.bootstrap, we have:

export PANTS_DOCKER_TOOLS="+['aws-oidc', 'open']"

(It's a conditional export, otherwise we'd put it in pants.toml)

When running ./pants, this export works to append the 2 values to the end of the config specified in pants.toml. When running the pants binary, the setting instead seems to overwrite the existing value with the escaped string contents of the env var, resulting in the error:

pants.core.util_rules.system_binaries.BinaryNotFoundError: Cannot find `'+['\''aws-oidc'\'', '\''open'\'']'` on `['/System/Cryptexes/App/usr/bin', '/Users/dan.moran/.asdf/shims', '/Users/dan.moran/.cargo/bin', '/Users/dan.moran/.go/bin', '/Users/dan.moran/bat-extras/bin', '/bin', '/opt/homebrew/bin', '/sbin', '/usr/bin', '/usr/local/MacGPG2/bin', '/usr/local/bin', '/usr/local/go/bin', '/usr/local/opt/asdf/libexec/bin', '/usr/local/opt/python3/libexec/bin', '/usr/local/sbin', '/usr/sbin']`. Please ensure that it is installed so that Pants can use docker.
kaos commented 1 year ago

I'll add to this that this .pants.bootstrap content doesn't work the same either:

export COLUMNS=$(tput cols)
export LINES=$(tput lines)
jsirois commented 1 year ago

Both of these issues can be solved by re-execing through bash instead of sandboxing a bash source. I'm still working through a test detail, but this is solved.

kaos commented 1 year ago

I'll add to this that this .pants.bootstrap content doesn't work the same either:

export COLUMNS=$(tput cols)
export LINES=$(tput lines)

Ah, this is broken in a slightly different way from what I initially thought. It does export and propagate, only it is reporting the wrong numbers.. i.e. it doesn't solve the issue when used with scie-pants, where as when used with ./pants it does.

# cat .pants.bootstrap
export COLUMNS=$(tput cols)
export LINES=$(tput lines)

echo "COLS: $COLUMNS, LINES: $LINES"
$ ./pants 
COLS: 241, LINES: 69
[...]

$ pants 
COLS: 80, LINES: 24
[...]

But that I even have this in the bootstrap is just a work-around for the underlying issue of the term not reporting the dimensions properly in the Pants process in the first place. So it's hack through'n through.

jsirois commented 1 year ago

Yeah, exactly. The fix fixes this use case anyhow and there is an IT for it now.

jsirois commented 1 year ago

This fix is now live in the 0.4.2 release.

kaos commented 1 year ago

This fix is now live in the 0.4.2 release.

Confirmed working for my tput use-case 👍🏽