Changed repo_dir variable to be uppercase

[radarhere]

In build_multilinux, I would expect repo_dir to be uppercase, referring to the environment variable, rather than lowercase, referring to a non-existent local variable.

multibuild/travis_linux_steps.sh

Lines 66 to 104 in 4eb8556

	function build_multilinux {
	# Runs passed build commands in manylinux container
	#
	# Depends on
	# MB_PYTHON_VERSION
	# MB_ML_VER
	# MB_ML_LIBC (optional)
	# UNICODE_WIDTH (optional)
	# BUILD_DEPENDS (optional)
	# DOCKER_IMAGE (optional)
	# MANYLINUX_URL (optional)
	# WHEEL_SDIR (optional)
	local plat=$1
	[ -z "$plat" ] && echo "plat not defined" && exit 1
	local build_cmds="$2"
	local libc=${MB_ML_LIBC:-manylinux}
	local docker_image=${DOCKER_IMAGE:-quay.io/pypa/${libc}${MB_ML_VER}_\$plat}
	docker_image=$(eval echo "$docker_image")
	retry docker pull $docker_image
	docker run --rm \
	-e BUILD_COMMANDS="$build_cmds" \
	-e PYTHON_VERSION="$MB_PYTHON_VERSION" \
	-e MB_PYTHON_VERSION="$MB_PYTHON_VERSION" \
	-e UNICODE_WIDTH="$UNICODE_WIDTH" \
	-e BUILD_COMMIT="$BUILD_COMMIT" \
	-e CONFIG_PATH="$CONFIG_PATH" \
	-e ENV_VARS_PATH="$ENV_VARS_PATH" \
	-e WHEEL_SDIR="$WHEEL_SDIR" \
	-e MANYLINUX_URL="$MANYLINUX_URL" \
	-e BUILD_DEPENDS="$BUILD_DEPENDS" \
	-e USE_CCACHE="$USE_CCACHE" \
	-e REPO_DIR="$repo_dir" \
	-e PLAT="$PLAT" \
	-e MB_ML_VER="$MB_ML_VER" \
	-e MB_ML_LIBC="$libc" \
	-v $PWD:/io \
	-v $HOME:/parent-home \
	$docker_image /io/$MULTIBUILD_DIR/docker_build_wrap.sh
	}

A local repo_dir variable was originally added in 09555a6, but was effectively removed in 3998141

[mattip]

It seems we could also remove it, since apparently things work without it.

[matthew-brett]

Looking quickly now, REPO_DIR (uppercase) appears in various places as the default - but I suppose we often supply the argument and therefore don't use the default.

It seems totally reasonable to rename for passing to the Linux container - the user should probably expect to set REPO_DIR instead of repo_dir, given that would be the active variable name for e.g. the Mac builds.

But - I wonder whether there is someone out there who is setting repo_dir? Worth a warning if so? On the lines of:

if [ -n "$repo_dir" ]; then
    echo 'Warning - please use variable name REPO_DIR rather than repo_dir'
    echo 'In future, repo_dir will have no effect, and we will only use REPO_DIR'
    echo 'For now, setting REPO_DIR to repo_dir'
    REPO_DIR=$repo_dir
fi

[radarhere]

Transitioning away from repo_dir as an argument doesn't seem so easy for functions where there are more arguments afterwards.

multibuild/common_utils.sh

Lines 282 to 284 in 4eb8556

	function clean_code {
	local repo_dir=${1:-$REPO_DIR}
	local build_commit=${2:-$BUILD_COMMIT}

multibuild/travis_linux_steps.sh

Lines 28 to 44 in 4eb8556

	function build_wheel {
	# Builds wheel, puts into $WHEEL_SDIR
	#
	# In fact wraps the actual work which happens in the container.
	#
	# Depends on
	# REPO_DIR (or via input argument)
	# PLAT (can be passed in as argument)
	# MB_PYTHON_VERSION
	# BUILD_COMMIT
	# UNICODE_WIDTH (optional)
	# BUILD_DEPENDS (optional)
	# MANYLINUX_URL (optional)
	# WHEEL_SDIR (optional)
	local repo_dir=${1:-$REPO_DIR}
	[ -z "$repo_dir" ] && echo "repo_dir not defined" && exit 1
	local plat=${2:-${PLAT:-x86_64}}

[radarhere]

It seems we could also remove it, since apparently things work without it.

In practice before now, yes, because the build_cmds supplied from build_wheel includes the $repo_dir as the second argument.

In theory though, you can specify any build_cmds to build_multilinux. If you specified "build_wheel" without the $repo_dir, then the REPO_DIR environment variable would be used.

[mattip]

Thanks @radarhere