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

Have the "Install Python" command from the walkthrough open and fill in the suggested command #19448

Closed
brettcannon opened this issue Jul 11, 2022 · 10 comments · Fixed by #19487 or #19582
Assignees
Labels
area-environments Features relating to handling interpreter environments feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@brettcannon
Copy link
Member

For instance, on macOS we currently suggest folks install via Homebrew using a command. We could update the walkthrough to have a permanent link to download Python that's in the text and then have the "Install Python" command open the terminal and send the command. We would probably want to have custom text for the button like "Install Python via Homebrew".

@brettcannon brettcannon added the feature-request Request for new features or functionality label Jul 11, 2022
@github-actions github-actions bot added the triage-needed Needs assignment to the proper sub-team label Jul 11, 2022
@karthiknadig karthiknadig added needs PR area-environments Features relating to handling interpreter environments labels Jul 12, 2022
@karrtikr karrtikr added needs spike Label for issues that need investigation before they can be worked on. and removed needs PR triage-needed Needs assignment to the proper sub-team labels Jul 12, 2022
@karrtikr karrtikr added this to the July 2022 milestone Jul 12, 2022
@karrtikr
Copy link

karrtikr commented Jul 13, 2022

How to decide choose between what commands to run for linux:

image

Approach 1: Identify whether it's a Fedora or Debian distribution

  1. /etc/os-release seems to be the standard for all newer distributions: http://linuxmafia.com/faq/Admin/release-files.html
  2. lsb-release command is not available everywhere, not in default Fedora: https://unix.stackexchange.com/a/37065/533483
  3. The following bash scripts seems comprehensive enough to identify the distribution:

Approach 2: Check if dnf is available directly

  1. which dnf will probably work, however:
  2. Cannot find a npm package of which command equivalent
  3. Look into each entry in PATH for a dnf file

cc/ @karthiknadig

Approach 1.3 seems like a viable option.

@brettcannon
Copy link
Member Author

https://www.npmjs.com/package/which should take care of approach 2.2. I personally prefer approach 2 as it's feature detection instead of distro detection.

It's also simple enough to implement ourselves.

import * as fs from "node:fs";
import * as path from "node:path";
import * as process from "node:process";

// Static since it won't change for the life of this process.
const path = process.env.PATH?.split(":");

function binExists(name: string): boolean {
    return path.some(dir => fs.existsSync(path.join(dir, name)));
}

@karrtikr
Copy link

karrtikr commented Jul 25, 2022

Verification steps:

Have no Python installed. Run the Python: Select command and choose "Python is not installed" item to open the install tile:
image

  • For Fedora based linux distribution, verify the following is sent when "Install Python via Terminal" button clicked:
sudo dnf install python3
  • For Debian based linux distribution, verify the following is sent when "Install Python via Terminal" button clicked:
sudo apt-get update
sudo apt-get install python3 python3-venv python3-pip

@karrtikr karrtikr added the verification-needed Verification of issue is requested label Jul 25, 2022
@rzhao271
Copy link

This leaves out CentOS 7, which uses yum, and also any other distribution that uses neither dnf nor apt. I think I'd rather have the editor admit that it doesn't know which command to use rather than for it to suggest I use apt.

Ref https://github.com/microsoft/vscode-python/pull/19487/files#diff-4964e93c7c78a768a22c36637cf2103a709769bac23d7e424c0633036294ec99R74

@karrtikr
Copy link

karrtikr commented Jul 27, 2022

For distributions that uses neither dnf nor apt, it's probably simplest to point to the python.org install page in the walkthrough description. For purposes of this issue, I see a couple options:

  • I would have the button send no commands to terminal in those cases.
  • It opens the python.org download page and we rename the button text from "Install Python via Terminal" to just "Install Python".

Any suggestions?

@rzhao271
Copy link

I would assume that Python is available in the package manager. Downloading from a website seems tedious in comparison.
But, even in the first case, if the install button isn't actually doing anything, that's also confusing.
Maybe the install button can print a message telling the user they need to manually install the package using their package manager once it realizes neither dnf nor apt exist.

@karrtikr
Copy link

I would assume that Python is available in the package manager. Downloading from a website seems tedious in comparison.

It is, but not sure if users would know the command to install python, pip etc. from it.

Maybe the install button can print a message telling the user they need to manually install the package using their package manager once it realizes neither dnf nor apt exist.

+1. Here I could also point out python.org if they aren't aware of their package manager.

Regardless, most latest distros today come with some kind of Python installed, so we should not hit this case much.

@karrtikr
Copy link

Fix should be out in the pre-release version of the extension in a few minutes, use the following to try it out:

image

@rzhao271 rzhao271 added the verified Verification succeeded label Jul 28, 2022
@rzhao271
Copy link

I'm unable to get my CentOS 7 and Debian 9 VMs to the state where there's absolutely no Python installed.
Verified by code review

@rzhao271
Copy link

Moved the executables out of the way and that worked.
Verified on CentOS 7

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-environments Features relating to handling interpreter environments feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
4 participants