builder
builder copied to clipboard
Potential Typo in builder
Should this be MANY_LINUX_VERSION instead of MANY_LINUX_VERSON?
https://github.com/pytorch/builder/blob/3e6551ca3b205d17fa9570e747c87fdf8df1449c/manywheel/build_docker.sh#L56
Noticed this last night, MANY_LINUX_VERSON appears only once in this entire repo, almost certainly a typo and they meant MANY_LINUX_VERSION.
Further, given the set -eou pipefail settings, it should fail, per the following snippet derived from the original file
$ cat ./variable_test.sh
#!/usr/bin/env bash
set -eou pipefail
MANY_LINUX_VERSION=${MANY_LINUX_VERSION:-}
if [[ -n ${MANY_LINUX_VERSION} ]]; then
DOCKERFILE_SUFFIX=_${MANY_LINUX_VERSON}
else
DOCKERFILE_SUFFIX=''
fi
echo "DOCKERFILE_SUFFIX: '${DOCKERFILE_SUFFIX}'"
$ ./variable_test.sh
DOCKERFILE_SUFFIX: ''
$ MANY_LINUX_VERSION=FOO ./variable_test.sh
./variable_test.sh: line 7: MANY_LINUX_VERSON: unbound variable
Given how any script calling such with MANY_LINUX_VERSION would surely fail, I looked into the wrapper script, and discovered a 2nd possible issue. Note how in the wrapper script it uses MANYLINUX_VERSION instead. So, the reason it is not failing is because MANY_LINUX_VERSION is never actually set due to a 2nd typo?
Investigating a fix in this fork / branch: https://github.com/ORNL/builder/tree/issue_969
Update 1: Fix is now in pull request #979 , my company's IP attorneys are awaiting a response from meta on the agreement phraseology before they can sign.
Update 2: Looks like one typo was fixed by another user and merged into main.
But, MANYLINUX_VERSION vs MANY_LINUX_VERSION still exists.