Address feedback and fix venv with symlinks

python · zooba · Jun 29, 2019 · Jun 26, 2019 · Jun 27, 2019 · Jun 27, 2019
commit d5fcb036992374899871c70d8b7317921b8cb85e
diff --git a/Include/cpython/initconfig.h b/Include/cpython/initconfig.h
@@ -374,7 +374,7 @@ typedef struct {
                                        to zero. */
 
     wchar_t *executable;        /* sys.executable */
-    wchar_t *base_executable;   /* sys.base_executable */
+    wchar_t *base_executable;   /* sys._base_executable */
     wchar_t *prefix;            /* sys.prefix */
     wchar_t *base_prefix;       /* sys.base_prefix */
     wchar_t *exec_prefix;       /* sys.exec_prefix */

diff --git a/Lib/site.py b/Lib/site.py
@@ -459,12 +459,6 @@ def venv(known_paths):
     env = os.environ
     if sys.platform == 'darwin' and '__PYVENV_LAUNCHER__' in env:
         executable = sys._base_executable = os.environ['__PYVENV_LAUNCHER__']
-    elif sys.platform == 'win32':
-        executable = sys.executable
-        if '__PYVENV_LAUNCHER__' in env:
-            # bpo-35873: Clear the environment variable to avoid it being
-            # inherited by child processes.
-            del os.environ['__PYVENV_LAUNCHER__']
     else:
         executable = sys.executable
     exe_dir, _ = os.path.split(os.path.abspath(executable))

diff --git a/Lib/test/test_venv.py b/Lib/test/test_venv.py
@@ -158,20 +158,16 @@ def test_prefixes(self):
         """
         Test that the prefix values are as expected.
         """
-        #check our prefixes
-        self.assertEqual(sys.base_prefix, sys.prefix)
-        self.assertEqual(sys.base_exec_prefix, sys.exec_prefix)
-
         # check a venv's prefixes
         rmtree(self.env_dir)
         self.run_with_capture(venv.create, self.env_dir)
         envpy = os.path.join(self.env_dir, self.bindir, self.exe)
         cmd = [envpy, '-c', None]
         for prefix, expected in (
             ('prefix', self.env_dir),
-            ('prefix', self.env_dir),
-            ('base_prefix', sys.prefix),
-            ('base_exec_prefix', sys.exec_prefix)):
+            ('exec_prefix', self.env_dir),
+            ('base_prefix', sys.base_prefix),
+            ('base_exec_prefix', sys.base_exec_prefix)):
             cmd[2] = 'import sys; print(sys.%s)' % prefix
             out, err = check_output(cmd)
             self.assertEqual(out.strip(), expected.encode())

diff --git a/Lib/venv/__init__.py b/Lib/venv/__init__.py
@@ -163,47 +163,66 @@ def create_configuration(self, context):
             if self.prompt is not None:
                 f.write(f'prompt = {self.prompt!r}\n')
 
-    def symlink_or_copy(self, src, dst, relative_symlinks_ok=False):
-        """
-        Try symlinking a file, and if that fails, fall back to copying.
-        """
-        force_copy = not self.symlinks
-        if not force_copy:
-            try:
-                if not os.path.islink(dst): # can't link to itself!
+    if os.name != 'nt':
+        def symlink_or_copy(self, src, dst, relative_symlinks_ok=False):
+            """
+            Try symlinking a file, and if that fails, fall back to copying.
+            """
+            force_copy = not self.symlinks
+            if not force_copy:
+                try:
+                    if not os.path.islink(dst): # can't link to itself!
+                        if relative_symlinks_ok:
+                            assert os.path.dirname(src) == os.path.dirname(dst)
+                            os.symlink(os.path.basename(src), dst)
+                        else:
+                            os.symlink(src, dst)
+                except Exception:   # may need to use a more specific exception
+                    logger.warning('Unable to symlink %r to %r', src, dst)
+                    force_copy = True
+            if force_copy:
+                shutil.copyfile(src, dst)
+    else:
+        def symlink_or_copy(self, src, dst, relative_symlinks_ok=False):
+            """
+            Try symlinking a file, and if that fails, fall back to copying.
+            """
+            bad_src = os.path.lexists(src) and not os.path.exists(src)
+            if self.symlinks and not bad_src and not os.path.islink(dst):
+                try:
                     if relative_symlinks_ok:
                         assert os.path.dirname(src) == os.path.dirname(dst)
                         os.symlink(os.path.basename(src), dst)
                     else:
                         os.symlink(src, dst)
-            except Exception:   # may need to use a more specific exception
-                logger.warning('Unable to symlink %r to %r', src, dst)
-                force_copy = True
-        if force_copy:
-            if os.name == 'nt':
-                # On Windows, we rewrite symlinks to our base python.exe into
-                # copies of venvlauncher.exe
-                basename, ext = os.path.splitext(os.path.basename(src))
-                srcfn = os.path.join(os.path.dirname(__file__),
-                                     "scripts",
-                                     "nt",
-                                     basename + ext)
-                # Builds or venv's from builds need to remap source file
-                # locations, as we do not put them into Lib/venv/scripts
-                if sysconfig.is_python_build(True) or not os.path.isfile(srcfn):
-                    if basename.endswith('_d'):
-                        ext = '_d' + ext
-                        basename = basename[:-2]
-                    if basename == 'python':
-                        basename = 'venvlauncher'
-                    elif basename == 'pythonw':
-                        basename = 'venvwlauncher'
-                    src = os.path.join(os.path.dirname(src), basename + ext)
-                else:
-                    src = srcfn
-                if not os.path.exists(src):
-                    logger.warning('Unable to copy %r', src)
                     return
+                except Exception:   # may need to use a more specific exception
+                    logger.warning('Unable to symlink %r to %r', src, dst)
+
+            # On Windows, we rewrite symlinks to our base python.exe into
+            # copies of venvlauncher.exe
+            basename, ext = os.path.splitext(os.path.basename(src))
+            srcfn = os.path.join(os.path.dirname(__file__),
+                                 "scripts",
+                                 "nt",
+                                 basename + ext)
+            # Builds or venv's from builds need to remap source file
+            # locations, as we do not put them into Lib/venv/scripts
+            if sysconfig.is_python_build(True) or not os.path.isfile(srcfn):
+                if basename.endswith('_d'):
+                    ext = '_d' + ext
+                    basename = basename[:-2]
+                if basename == 'python':
+                    basename = 'venvlauncher'
+                elif basename == 'pythonw':
+                    basename = 'venvwlauncher'
+                src = os.path.join(os.path.dirname(src), basename + ext)
+            else:
+                src = srcfn
+            if not os.path.exists(src):
+                if not bad_src:
+                    logger.warning('Unable to copy %r', src)
+                return
 
             shutil.copyfile(src, dst)
 

diff --git a/Misc/NEWS.d/next/Windows/2019-06-28-09-44-08.bpo-37369.1iVpxq.rst b/Misc/NEWS.d/next/Windows/2019-06-28-09-44-08.bpo-37369.1iVpxq.rst
@@ -0,0 +1 @@
+Fixes path for :data:`sys.executable` when running from the Microsoft Store.
diff --git a/PC/getpathp.c b/PC/getpathp.c
@@ -549,13 +549,16 @@ get_program_full_path(const PyConfig *config,
         /* If overridden, preserve the original full path */
         pathconfig->base_executable = PyMem_RawMalloc(
             sizeof(wchar_t) * (MAXPATHLEN + 1));
-
         PyStatus status = canonicalize(pathconfig->base_executable,
                                        program_full_path);
         if (_PyStatus_EXCEPTION(status)) {
             return status;
         }
+
         wcscpy_s(program_full_path, MAXPATHLEN+1, pyvenv_launcher);
+        /* bpo-35873: Clear the environment variable to avoid it being
+        * inherited by child processes. */
+        _wputenv_s(L"__PYVENV_LAUNCHER__", L"");
     }
 
     pathconfig->program_full_path = PyMem_RawMalloc(

diff --git a/PC/python_uwp.cpp b/PC/python_uwp.cpp
@@ -27,51 +27,51 @@ const wchar_t *PROGNAME = L"python.exe";
 #endif
 #endif
 
-static void
-set_user_base()
+static std::wstring
+get_user_base()
 {
-    wchar_t envBuffer[2048];
     try {
         const auto appData = winrt::Windows::Storage::ApplicationData::Current();
         if (appData) {
             const auto localCache = appData.LocalCacheFolder();
             if (localCache) {
                 auto path = localCache.Path();
-                if (!path.empty() &&
-                    !wcscpy_s(envBuffer, path.c_str()) &&
-                    !wcscat_s(envBuffer, L"\\local-packages")
-                ) {
-                    _wputenv_s(L"PYTHONUSERBASE", envBuffer);
+                if (!path.empty()) {
+                    return std::wstring(path) + L"\\local-packages";
                 }
             }
         }
     } catch (...) {
     }
+    return std::wstring();
 }
 
-static winrt::hstring
+static std::wstring
 get_package_family()
 {
     try {
         const auto package = winrt::Windows::ApplicationModel::Package::Current();
         if (package) {
             const auto id = package.Id();
-            return id ? id.FamilyName() : winrt::hstring();
+            if (id) {
+                return std::wstring(id.FamilyName());
+            }
         }
     }
     catch (...) {
     }
 
-    return winrt::hstring();
+    return std::wstring();
 }
 
-static int
+static PyStatus
 set_process_name(PyConfig *config)
 {
+    PyStatus status;
     std::wstring executable, home;
 
     const auto family = get_package_family();
-    
+
     if (!family.empty()) {
         PWSTR localAppData;
         if (SUCCEEDED(SHGetKnownFolderPath(FOLDERID_LocalAppData, 0,
@@ -101,6 +101,8 @@ set_process_name(PyConfig *config)
             size_t bslash = home.find_last_of(L"/\\");
             if (bslash != std::wstring::npos) {
                 home.erase(bslash);
+            } else {
+                home.clear();
             }
             break;
         }
@@ -111,22 +113,34 @@ set_process_name(PyConfig *config)
     }
 
     if (!home.empty()) {
-        PyConfig_SetString(config, &config->home, home.c_str());
-        // XXX: Why doesn't setting prefix and exec_prefix work?
-        //PyConfig_SetString(config, &config->prefix, home.c_str());
-        //PyConfig_SetString(config, &config->exec_prefix, home.c_str());
+        status = PyConfig_SetString(config, &config->home, home.c_str());
+        if (PyStatus_Exception(status)) {
+            return status;
+        }
     }
     if (!executable.empty()) {
-        PyConfig_SetString(config, &config->base_executable, executable.c_str());
+        status = PyConfig_SetString(config, &config->base_executable, executable.c_str());
+        if (PyStatus_Exception(status)) {
+            return status;
+        }
+
         const wchar_t *launcherPath = _wgetenv(L"__PYVENV_LAUNCHER__");
         if (launcherPath) {
-            PyConfig_SetString(config, &config->executable, launcherPath);
+            status = PyConfig_SetString(
+                config, &config->executable, launcherPath);
+            /* bpo-35873: Clear the environment variable to avoid it being
+            * inherited by child processes. */
+            _wputenv_s(L"__PYVENV_LAUNCHER__", L"");
         } else {
-            PyConfig_SetString(config, &config->executable, executable.c_str());
+            status = PyConfig_SetString(
+                config, &config->executable, executable.c_str());
+        }
+        if (PyStatus_Exception(status)) {
+            return status;
         }
     }
 
-    return 1;
+    return PyStatus_Ok();
 }
 
 int
@@ -153,11 +167,12 @@ wmain(int argc, wchar_t **argv)
         goto fail;
     }
 
-    if (!set_process_name(&config)) {
-        status = PyStatus_Exit(121);
+    status = set_process_name(&config);
+    if (PyStatus_Exception(status)) {
         goto fail;
     }
-    set_user_base();
+
+    _wputenv_s(L"PYTHONUSERBASE", get_user_base().c_str());
 
     const wchar_t *p = wcsrchr(argv[0], L'\\');
     if (!p) {
@@ -178,9 +193,18 @@ wmain(int argc, wchar_t **argv)
         }
 
         if (moduleName) {
-            PyConfig_SetString(&config, &config.run_module, moduleName);
-            PyConfig_SetString(&config, &config.run_filename, NULL);
-            PyConfig_SetString(&config, &config.run_command, NULL);
+            status = PyConfig_SetString(&config, &config.run_module, moduleName);
+            if (PyStatus_Exception(status)) {
+                goto fail;
+            }
+            status = PyConfig_SetString(&config, &config.run_filename, NULL);
+            if (PyStatus_Exception(status)) {
+                goto fail;
+            }
+            status = PyConfig_SetString(&config, &config.run_command, NULL);
+            if (PyStatus_Exception(status)) {
+                goto fail;
+            }
         }
     }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Fixes path for :data:`sys.executable` when running from the Microsoft Store.