pyvenv activate script failure with specific bash option

[s-wakaba]

BPO	25351
Nosy	@vsajip, @vstinner, @bitdancer, @ssbarnea
PRs	bpo-25351: avoid activate failure on strict shells #3804 [3.6] bpo-25351: avoid activate failure on strict shells (GH-3804) #3820
Files	activate

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-09-29.12:35:44.758>
created_at = <Date 2015-10-09.07:23:57.046>
labels = ['3.7', 'type-bug', 'library']
title = 'pyvenv activate script failure with specific bash option'
updated_at = <Date 2017-09-29.12:35:44.755>
user = 'https://bugs.python.org/s-wakaba'

bugs.python.org fields:

activity = <Date 2017-09-29.12:35:44.755>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2017-09-29.12:35:44.758>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2015-10-09.07:23:57.046>
creator = 's-wakaba'
dependencies = []
files = ['41925']
hgrepos = []
issue_num = 25351
keywords = ['patch']
message_count = 8.0
messages = ['252593', '260284', '303255', '303287', '303289', '303325', '303327', '303328']
nosy_count = 5.0
nosy_names = ['vinay.sajip', 'vstinner', 'r.david.murray', 'ssbarnea', 's-wakaba']
pr_nums = ['3804', '3820']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue25351'
versions = ['Python 3.6', 'Python 3.7']

[s-wakaba]

When writing bash shellscripts, I always set options "set -ueC" for strict error checking. However, loading pyvenv's activate and deactivate scripts from bash with "-u" option, they are failure because they may refer undefined shell variables.

I hope activate script and deactivate function are enclosed like following block

_OLD_BASH_OPTIONS="$-"
set +u

### script body ##

set -$_OLD_BASH_OPTIONS
unset _OLD_BASH_OPTIONS

Another option, all shell-variables in the script change like

$SPAM -> ${SPAM:-}

[vsajip]

Can you try the attached script in place of the current activate script and see if it meets the requirements?

[ssbarnea]

This is a perfectly valid bug and I am surprised it was ignored for so long.

I just raised a PR for addressing it, please review it.

[bitdancer]

Note that in the equivalent issue raised against virtualenv, the concern was raised that some old sh-like shells might not support this syntax, but it was pointed out that it is part of the posix standard. I'm not arguing for or against here, just noting relevant information :)

The script comment specifically says "from bash", but presumably the script can be and is sourced from other sh-like shells.

[ssbarnea]

Based on my tests this worked with all shells I tested with, the syntax being a very old one and not some new/modern one.

Passed: bash, zsh, dash, ksh
Platforms: MacOS, RHEL Linux

Failed with: tcsh but with the note that even the original code would fail with tcsh anyway. tcsh/csh are not bourne/posix compatible so there is no regression introduced by the the use of "${VAR:-}" syntax.

A simple way to test the syntax with enable shell is:
SHELLCMD -c 'echo "[${AAA:-}]"'

[vsajip]

New changeset 90f1d98 by Vinay Sajip (Sorin Sbarnea) in branch 'master':
bpo-25351: avoid activate failure on strict shells (GH-3804)
90f1d98

[vstinner]

New changeset a5610e0 by Victor Stinner (Miss Islington (bot)) in branch '3.6':
[3.6] bpo-25351: avoid activate failure on strict shells (GH-3804) (bpo-3820)
a5610e0

[vstinner]

The bug has been fixed in Python 3.6 and 3.7 (master), thank you Sorin Sbarnea!

Python 3.4 and 3.5 don't accept bug fixes anymore, only security fixes:
https://devguide.python.org/#status-of-python-branches