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.

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

Windows installer: Append Python to PATH instead of prepending it #89097

Closed
bneuburg mannequin opened this issue Aug 17, 2021 · 20 comments
Closed

Windows installer: Append Python to PATH instead of prepending it #89097

bneuburg mannequin opened this issue Aug 17, 2021 · 20 comments
Labels
3.11 only security fixes OS-windows type-feature A feature request or enhancement

Comments

@bneuburg
Copy link
Mannequin

bneuburg mannequin commented Aug 17, 2021

BPO 44934
Nosy @pfmoore, @tjguk, @zware, @zooba, @bneuburg, @Saxomania
PRs
  • bpo-44934: Add optional feature AppendPath to Windows msi installer #27889
  • Files
  • buildout.txt
  • orca.png: Orca output for appendpath.msi environment table
  • 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 2022-01-18.00:21:05.140>
    created_at = <Date 2021-08-17.10:35:52.085>
    labels = ['type-feature', 'OS-windows', '3.11']
    title = 'Windows installer: Append Python to PATH instead of prepending it'
    updated_at = <Date 2022-01-18.00:21:05.139>
    user = 'https://github.com/bneuburg'

    bugs.python.org fields:

    activity = <Date 2022-01-18.00:21:05.139>
    actor = 'steve.dower'
    assignee = 'none'
    closed = True
    closed_date = <Date 2022-01-18.00:21:05.140>
    closer = 'steve.dower'
    components = ['Windows']
    creation = <Date 2021-08-17.10:35:52.085>
    creator = 'bn_append'
    dependencies = []
    files = ['50231', '50233']
    hgrepos = []
    issue_num = 44934
    keywords = ['patch']
    message_count = 20.0
    messages = ['399737', '399759', '399935', '399936', '399944', '399994', '399999', '400000', '400001', '400003', '400077', '400148', '400172', '400177', '400178', '400179', '400187', '400230', '410836', '410837']
    nosy_count = 6.0
    nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'steve.dower', 'bn_append', 'Saxomania']
    pr_nums = ['27889']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue44934'
    versions = ['Python 3.11']

    @bneuburg
    Copy link
    Mannequin Author

    bneuburg mannequin commented Aug 17, 2021

    Hi there,

    in our organization Python 3.9 is installed on Windows with the PrependPath option; as expected the Install and Scripts directories are prepended to path.

    However if there are Python scripts with the same name as a system command (e.g. a script named ping.py vs. the included ping.exe), the Python script gets preferred if I run ping in cmd.exe or Powershell, which makes sense, since the Python script path is considered before e.g. C:\Windows\System32\.

    Is it possible to either change the option to append the Install and scripts directories to the systems PATH instead of prepending it or add an AppendPath option?

    I searched for a discussion why prepending was chosen instead of appending but I didn't find anything.

    @bneuburg bneuburg mannequin added 3.9 only security fixes type-bug An unexpected behavior, bug, or error OS-windows labels Aug 17, 2021
    @zooba
    Copy link
    Member

    zooba commented Aug 17, 2021

    Prepending is used because it makes the most recently installed version of Python the "active" one. It also ensures that a deliberate install is going to override any applications that may have put their own copy on PATH (deliberately or otherwise). There's no real mechanism for managing multiple Python entries on PATH, so it just works out better to prepend and let the latest install win.

    However, we also disable this option by default because it can lead to confusion like what you've experienced. The best option here is to leave it disabled and update PATH yourself (or use "py.exe" which is always on PATH), since you've got the best awareness of your environment.

    @Saxomania
    Copy link
    Mannequin

    Saxomania mannequin commented Aug 19, 2021

    hi,

    if you know that it will lead to confusion why is this confusion not written here:

    https://docs.python.org/3/using/windows.html

    If you do software deployment in a company you will only have one version so having more than one version is not the point. And if you make append path optional available like prepend everybody can choose. Anyway.

    What i have seen now is if you uninstall Python it is not cleaning up the path variable. Is there any parameter i missed?

    @zooba
    Copy link
    Member

    zooba commented Aug 19, 2021

    why is this confusion not written here

    From https://docs.python.org/3/using/windows.html#configuring-python

    While the installer provides an option to configure the PATH and PATHEXT variables for you, this is only reliable for a single, system-wide installation. If you regularly use multiple versions of Python, consider using the Python Launcher for Windows.

    Now, that isn't *exactly* the issue in the original post, but the warning is present.

    If you do software deployment in a company you will only have one version so having more than one version is not the point.

    Unfortunately not true. It's been a while since I had actual data on this, but last time I did the *majority* had 2 versions installed, and 3-5 versions were all more likely than only a single one. So handling multiple versions matters.

    What i have seen now is if you uninstall Python it is not cleaning up the path variable.

    We do the best we can by using the native Windows support for installing environment variables. It sometimes doesn't work if you (or another installer) has modified the variable since you installed Python, or if you've updated it yourself. That's out of our control - we can't do a better job than the OS.

    if you make append path optional available like prepend everybody can choose

    Contributions are welcome. We aren't paid to implement features - we volunteer to manage contributions, so if you'd really like this to be in the installer, feel free to go for it. (Last time the installer was modified we had most trouble making the UI not break, but it's probably not too hard if this was a command-line only.)

    @zooba zooba added 3.11 only security fixes type-feature A feature request or enhancement and removed 3.9 only security fixes type-bug An unexpected behavior, bug, or error labels Aug 19, 2021
    @bneuburg
    Copy link
    Mannequin Author

    bneuburg mannequin commented Aug 20, 2021

    Hi Steve,

    Of course there are various reasons for having multiple Python versions on a developer workstation but I would assume that the majority of Windows installer users simple want to run Python projects, at least this holds true for the majority of Python on Windows users in my org.

    if you'd really like this to be in the installer, feel free to go for it.

    could you point me to the relevant files and lines for the PrependPath CLI option and implementation logic on Github? Not sure if I got it right, but I assume the following file is in charge of modifying the env:path:

    https://github.com/python/cpython/blob/main/Tools/msi/path/path.wxs

    Since I have never worked within the MSI framework, I could not figure out how the "call flow" from enabling the definition of the PrependPath in
    https://github.com/python/cpython/blob/main/Tools/msi/bundle/bundle.wxs
    to actually modifying the path

    Regarging path.wxs: Would you prefer that I add a pathappend/pathappend.wxs or should I simply add a new Component ID like so:

    <Component Id="AppendPath_LM" Directory="InstallDirectory" Guid="*">
    <Condition>ALLUSERS=1</Condition>
    ...

    Or would I introduce a new condition ALLUSERS_APPEND or something?

    @zooba
    Copy link
    Member

    zooba commented Aug 20, 2021

    The logic is probably simplest if you clone the Tools/msi/path project entirely as pathappend. That will technically allow people to both prepend and append if they set both flags, but I think it's okay to say "don't do that", especially if AppendPath is only a command line option.

    Per-component or optional features are complicated and best avoided if we can. For a simple on-off like this, a whole MSI is actually the simplest way to manage it.

    @bneuburg
    Copy link
    Mannequin Author

    bneuburg mannequin commented Aug 20, 2021

    Thanks for the rundown, after digging a little bit into the abyss that is the WIX toolset if you've never dealt with it before I already started going down that route.

    I pushed it to my fork of cpython:
    https://github.com/bneuburg/cpython/commits/issue44934

    Since I'm pretty sure that my commits are not correct yet (following the instructions in Tools/msi/README.txt regarding fetching dependencies building the MSI resulted in a failed build before I even touched a single line of code) I abstained from handing in a pull request.

    Feedback would be very much appreciated. Of course if you'd want to use that stuff and refine it I would be totally fine with it ;)

    In case I messed up any formal requirements (i.e. wording of commit messages etc.): I will get myself up to speed and fix that, but since I'll be offline for a week starting Monday I wanted to get my initial draft out there, so in case you have any feedback until Monday I can incorporate it before my brain pushes that stuff out...

    @bneuburg
    Copy link
    Mannequin Author

    bneuburg mannequin commented Aug 20, 2021

    @bneuburg
    Copy link
    Mannequin Author

    bneuburg mannequin commented Aug 20, 2021

    Previous URL got "fixed" by roundup, hopefully this one works:

    16b9be4...bneuburg:bpo-44934

    @zooba
    Copy link
    Member

    zooba commented Aug 20, 2021

    That branch looks like you're on the right track. The build scripts in
    Tools/msi should Just Work, so I'd be interested to see the error you get.

    You can also submit it as a PR and the MSI will be built automatically.

    One thing I noticed is updating the name of the "Add Python to PATH"
    option, which I'd rather keep as "Add" since we won't have both options
    in the UI. The word "Prepend" is quite obscure jargon, and for the range
    of users we have it isn't fair to expect them to know it (or to break
    all of the guidance out there explaining that checkbox already).

    @bneuburg
    Copy link
    Mannequin Author

    bneuburg mannequin commented Aug 22, 2021

    Steve, I signed the CLA and created the PR, I don't know how long it takes until the info that the CLA is signed is propagated to bpo or github.

    Regarding failed builds: I will try to build again tomorrow when back at the office and can copy the errors, how would you like to receive them?

    For building I spun up a MS Windows 10 Edge Test VM, installed Visual Studio 2019 and then tried to run get_externals.bat; that already failed since the nuget config was empty and thus had no upstreams configured. Maybe this problem stems from the free test VM. After editing the nuget config, that script worked out fine and then I run build.bat, which seemed to be quite busy for at least 10 minutes, then I left my office. Upon return I saw plenty of errors.

    @zooba
    Copy link
    Member

    zooba commented Aug 23, 2021

    I kicked off the build on the PR so we can see what errors it finds. Nothing jumps out as obviously wrong in the changes, but it only takes one missing reference to a file to break it all.

    If you want to share the errors from your test machine, copy-pasting all the output into a text file and attaching it here is best.

    @bneuburg
    Copy link
    Mannequin Author

    bneuburg mannequin commented Aug 23, 2021

    Attached is the full output of:

    • msbuild.exe -version
    • get_externals.bat
    • build.bat

    For the first few build tasks there are git related errors, which make sense since I ran the build from a downloaded zip release of the source instead of a git clone. I assume the first "real" error occurs when building the wix targets, just grep for MSB4062, this is when things go downhill.

    @zooba
    Copy link
    Member

    zooba commented Aug 23, 2021

    Oh I remember this one. The Wix toolset still requires .NET 3.5, which you have to install manually these days. Because it's being launched as a console app, the UI never gets triggered.

    I thought we had this in the devguide already... but maybe not (or not prominently enough). Might be worth trying to add some detection for it - hopefully a new version of Wix will move to newer .NET too.

    See https://docs.microsoft.com/en-us/dotnet/framework/install/dotnet-35-windows-10 for install instructions. It should work after that. The CI build succeeded, but it'll need manual testing.

    @bneuburg
    Copy link
    Mannequin Author

    bneuburg mannequin commented Aug 23, 2021

    Ah ok. Do you know if VS 2019 will do the job at all? I encountered a few (unrelated) projects on github that state they will only build with VS 2017.

    I will install .NET 3.5 via the VS 2019 installer (should do the trick, right?) and see how it turns out.

    Could you attach the msi installer build artifact from the build you kicked off so I can test if the appendpath stuff works as expected? Or did you already have a look at it?

    P.S.: bpo now recognizes that I signed the CLA, github not yet.

    @zooba
    Copy link
    Member

    zooba commented Aug 23, 2021

    I don't think we upload the artifact from the MSI builds, sorry.

    If Visual Studio has an option for the older version of .NET, then yeah, that'll do it. But because it was an OS component, you may have to do it through the OS dialog in the link I posted.

    GitHub will get the CLA update soon enough. It's always a little slower - bpo is the canonical source.

    @bneuburg
    Copy link
    Mannequin Author

    bneuburg mannequin commented Aug 24, 2021

    Update: .NET 3.5 installation worked out fine via VS 2019 installer, I also added the VS2017 build tools as a precaution since the VM builds take some time.

    Afterwards build.bat still failed during wix steps, complaining about missing python.exe in PCBuild/win32, not sure if the 32bit interpreter should be fetched/built automagically. Then I tried build.bat -x64, which ran succesfully and provided attached installer.

    To test it (in administrative shell):

    1. git clone https://github.com/bneuburg/python_msi_appendpath
    2. cd python_appendpath_installer
    3. .\python-3.11.0.7539-amd64.exe /quiet InstallAllUsers=1 AppendPath=1 Include_test=0

    Running this in an administrative shell appended InstallDir and Installdir/Scripts to the sytem PATH and created the PATHEXT entries for PY and PYC as well. So far so good!

    I only noticed the following problem:
    The first uninstall also succesfully removed all environment variables, however after reinstalling and re-uninstalling, only the PATHEXT were removed, the Python entries in PATH were not removed. This was also independent of the order of software removal (Python Launcher first or second).

    Thus I had a look at it with orca.exe [1]:

    1. Install with appendpath=1
    2. Get list of installed msis with: wmic product get /format:csv > software.csv
    3. Search for appendpath in software.csv which will return the location of the msi somewhere in c:\windows\installer
    4. Right click that file and select 'Edit with Orca'

    In the environment table it has entries in the following form for a Local Machine install (c.f. for attached screenshot):

    Environment | Name | Value | Component
    PATH_LM | =-*PATH | [~]:[InstallDirectory] | AppendPath_LM

    which according to Microsoft documentation [2] seems to be as intended (add on install, remove on uninstall). But it just doesn't, also see Sascha's comment above. When looking at Orca I also noticed that I forgot to change the position of InstallDirectory in PATH for the CU install from first to last, I updated the PR accordingly.

    Since you have a Microsoft e-mail address: Any chances you could ask the MSI pros there what is going on?
    I'll be busy with offline stuff for a few weeks, but if you have any ideas how to troubleshoot this, let me know. I found a discussion where the order of the prefixes for names was slightly changed [3], i.e. *=- instead of =-* but I don't know if this matters at all or how to test it.

    Sorry for the text book, things like that really keep me digging.

    [1] https://docs.microsoft.com/de-de/windows/win32/msi/orca-exe
    [2] https://docs.microsoft.com/en-us/windows/win32/msi/environment-table
    [3] https://www.itninja.com/question/add-to-path-envronment-variable

    @zooba
    Copy link
    Member

    zooba commented Aug 24, 2021

    Removing environment variables is "known" to be unreliable - another reason I don't like modifying them from an installer. It's meant to remove the item from the list based on value, so if anything has changed then it's not a huge surprise it will fail.

    I'm more inclined to think that the "[~]:" prefix might be up to something. I forget exactly what it means, but if it's not all resolving to an empty string, it might concatenate fine but then mess up the removal?

    @zooba
    Copy link
    Member

    zooba commented Jan 18, 2022

    New changeset c47c9e6 by bneuburg in branch 'main':
    bpo-44934: Add optional feature AppendPath to Windows MSI installer (GH-27889)
    c47c9e6

    @zooba
    Copy link
    Member

    zooba commented Jan 18, 2022

    Thanks for the PR! This is a good contribution, that wasn't trivial to do. It should be in Python 3.11 alpha 5, so please test it out and make sure it's behaving as you expect.

    @zooba zooba closed this as completed Jan 18, 2022
    @zooba zooba closed this as completed Jan 18, 2022
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes OS-windows type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant