Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

pyvenv activate script failure with specific bash option #69538

Closed
s-wakaba mannequin opened this issue Oct 9, 2015 · 8 comments
Closed

pyvenv activate script failure with specific bash option #69538

s-wakaba mannequin opened this issue Oct 9, 2015 · 8 comments
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@s-wakaba
Copy link
Mannequin

s-wakaba mannequin commented Oct 9, 2015

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
    Copy link
    Mannequin Author

    s-wakaba mannequin commented Oct 9, 2015

    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 -&gt; ${SPAM:-}

    @s-wakaba s-wakaba mannequin added type-feature A feature request or enhancement stdlib Python modules in the Lib dir labels Oct 9, 2015
    @vsajip
    Copy link
    Member

    vsajip commented Feb 14, 2016

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

    @ssbarnea
    Copy link
    Mannequin

    ssbarnea mannequin commented Sep 28, 2017

    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.

    @ssbarnea ssbarnea mannequin added type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels Sep 28, 2017
    @bitdancer
    Copy link
    Member

    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
    Copy link
    Mannequin

    ssbarnea mannequin commented Sep 28, 2017

    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
    Copy link
    Member

    vsajip commented Sep 29, 2017

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

    @vstinner
    Copy link
    Member

    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
    Copy link
    Member

    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

    @vstinner vstinner added the 3.7 (EOL) end of life label Sep 29, 2017
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @TomiBelan TomiBelan mentioned this issue Nov 29, 2023
    Open
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vsajip @vstinner @bitdancer