[bug] unbound variable caused by conanbuildenv-debug-x86_64.sh

[Ignition]

Describe the bug

I'm using antlr4 as a tool_requires

self.tool_requires("antlr4/4.13.1")

It's package_info sets CLASSPATH

def package_info(self):
    ...
    self.runenv_info.prepend_path("CLASSPATH", jar)

https://github.com/conan-io/conan-center-index/blob/0ae09f76f8efec51f00c6d8b0f019d64403d0f4f/recipes/antlr4/all/conanfile.py#L75

conan makes conanbuildenv-debug-x86_64.sh where it unconditionally joins paths

export CLASSPATH="/home/gareth/.conan2/p/antlr15795d040a28f/p/res/antlr-complete.jar:$CLASSPATH"

Which leads to error when sourced in hardened script environments

.../build/generators/conanbuildenv-debug-x86_64.sh: line 15: CLASSPATH: unbound variable

Example hardened script environment

$ bash -c 'set -eu; echo $UNKNOWN_VAR'
bash: line 1: UNKNOWN_VAR: unbound variable
$ echo $?
1

${CLASSPATH:+:$CLASSPATH} should work

How to reproduce it

No response

[memsharded]

Hi @Ignition

Thanks for the feedback.

I think the problem with ${CLASSPATH:+:$CLASSPATH} is that it was problematic for being a bash thing, and not all shells are bash, in fact many systems will default to sh or other shells, if I recall correctly.

I have checked, and we had that syntax in the legacy environment generators in Conan 1, and it was removed for the new environment generators, to make the generated .sh files try to be as generic as possible, and doing that could very likely break many users.

[Ignition]

@memsharded would this be portable enough?

export CLASSPATH="${CLASSPATH:-}"
export CLASSPATH="/home/gareth/.conan2/p/antlr15795d040a28f/p/res/antlr-complete.jar:$CLASSPATH"

It appears to be fine for sh, bash, and zsh

Although I tested sh with ${CLASSPATH:+:$CLASSPATH} and appeared fine.
Maybe its a portability issue with what sh actually is on any given platform.
For me sh on ubuntu is actually dash

$ readlink -f $(which sh)
/usr/bin/dash

$ dash -c 'set -eu; CLASSPATH="test${CLASSPATH:+:$CLASSPATH}"; echo "$CLASSPATH"'
test
$ bash -c 'set -eu; CLASSPATH="test${CLASSPATH:+:$CLASSPATH}"; echo "$CLASSPATH"' 
test
$ zsh -c 'set -eu; CLASSPATH="test${CLASSPATH:+:$CLASSPATH}"; echo "$CLASSPATH"' 
test
$ busybox sh -c 'set -eu; CLASSPATH="test${CLASSPATH:+:$CLASSPATH}"; echo "$CLASSPATH"' 
test
$ ksh -c 'set -eu; CLASSPATH="test${CLASSPATH:+:$CLASSPATH}"; echo "$CLASSPATH"' 
test

$ CLASSPATH=extra dash -c 'set -eu; CLASSPATH="test${CLASSPATH:+:$CLASSPATH}"; echo "$CLASSPATH"' 
test:extra
$ CLASSPATH=extra bash -c 'set -eu; CLASSPATH="test${CLASSPATH:+:$CLASSPATH}"; echo "$CLASSPATH"' 
test:extra
$ CLASSPATH=extra zsh -c 'set -eu; CLASSPATH="test${CLASSPATH:+:$CLASSPATH}"; echo "$CLASSPATH"' 
test:extra
$ CLASSPATH=extra busybox sh -c 'set -eu; CLASSPATH="test${CLASSPATH:+:$CLASSPATH}"; echo "$CLASSPATH"' 
test:extra
$ CLASSPATH=extra ksh -c 'set -eu; CLASSPATH="test${CLASSPATH:+:$CLASSPATH}"; echo "$CLASSPATH"' 
test:extra

[Ignition]

Trying to work around this I maybe found another bug.

If I do CLASSPATH= in my profile's [buildenv] then .../build/generators/conanbuildenv-debug-x86_64.sh will unset CLASSPATH completely ignoring the environment from antlr.

Empty does not mean unset, and it still should be combined with environment that come from tool requirements.

# should be fine
sh -c 'set -eu; X=; echo $X;'

[memsharded]

If I do CLASSPATH= in my profile's [buildenv] then .../build/generators/conanbuildenv-debug-x86_64.sh will unset CLASSPATH completely ignoring the environment from antlr.

Thanks for the report, that is indeed a bit unexpected, I am a bit concerned that changing things here might break other users, but this could be a bug, I am trying a fix in #19328

[memsharded]

Hi @Ignition

#19328 has been merged as a bug fix for next Conan 2.24

[memsharded]

Regarding

@memsharded would this be portable enough?

export CLASSPATH="${CLASSPATH:-}"
export CLASSPATH="/home/gareth/.conan2/p/antlr15795d040a28f/p/res/antlr-complete.jar:$CLASSPATH"
It appears to be fine for sh, bash, and zsh

It is very difficult to be sure, but this kind of change still concerns me, because of the past experiences. I recall not only the shell itself, but users running in systems that had older versions of sh, bash, or whatever, that had different behavior/support for this kind of expressions.

Let's see if the bugfix above in #19328 is good enough to solve it for your use case.

[Ignition]

That still does not fix it. The existence of any value in [buildenv] is having the effect that it prevents prepend_path from ANTLR recipe being applied. It just says export CLASSPATH="".

I think there is logic in the way multiple environment variables are prepended/appended into one export line that could be fixed.

Maybe have a multilayer system:

• Environment that conan was called in, it is what it is
• [buildenv] gets its own export line, which allows me to ensure CLASSPATH is set as an empty variable
• recipes have their own modifier export

Or, the simpler ${CLASSPATH:-} if its known to be a portable approach

[memsharded]

Hi @Ignition

I am checking this again and having a look with the team.

I would like to know, the hardening of the scripts is something that typically is done at the script level. Also, I'd expect a bunch of scripts from the environment, like possible scripts that build systems in open source third party libraries, that wouldn't be robust either to run as hardened, so those would fail too. So, how are you activating the hardening? are you doing it at the global level so all scripts in your system will use it?

[memsharded]

Doing #19591 as a proof of concept