diff --git a/constructor/fcp.py b/constructor/fcp.py index c32d2459d..c5de48ba3 100644 --- a/constructor/fcp.py +++ b/constructor/fcp.py @@ -16,7 +16,7 @@ import tempfile from collections import defaultdict from itertools import groupby -from os.path import abspath, expanduser, isdir, join +from os.path import abspath, basename, expanduser, isdir, join from subprocess import check_call from typing import TYPE_CHECKING @@ -199,10 +199,17 @@ def check_duplicates_files( paths_data = read_paths_json(extracted_package_dir).paths env_prefix_len = len(env_prefixes.get(pc_rec, "")) + # Before linking, conda extracts each package into the package cache at + # $INSTDIR/pkgs//. This intermediate path is + # longer than the final linked path (env_prefix + short_path) and is what + # actually overflows MAX_PATH, so it must drive the length check. + pkgs_prefix_len = len("pkgs/") + len(basename(extracted_package_dir)) + len("/") for path_data in paths_data: short_path = path_data.path max_relative_path_length = max( - max_relative_path_length, env_prefix_len + len(short_path) + max_relative_path_length, + env_prefix_len + len(short_path), + pkgs_prefix_len + len(short_path), ) try: size = path_data.size_in_bytes or getsize(join(extracted_package_dir, short_path)) diff --git a/constructor/nsis/main.nsi.tmpl b/constructor/nsis/main.nsi.tmpl index b04469c90..3b720c8c2 100644 --- a/constructor/nsis/main.nsi.tmpl +++ b/constructor/nsis/main.nsi.tmpl @@ -1076,14 +1076,16 @@ Function OnDirectoryLeave ${If} $LongPathsEnabled == "0" IntOp $0 $InstDirLen + ${MAX_RELATIVE_PATH_LENGTH} IntOp $0 $0 + 1 # Account for path separator between install dir and relative path - ${If} $0 > 260 + # MAX_PATH is 260 including the null terminator, so the usable path length is 259. + # A full path of exactly 260 characters already fails to be created. + ${If} $0 > 259 ${If} ${AtLeastWin10} ${AndIfNot} $NO_REGISTRY = "1" ${AndIf} ${UAC_IsAdmin} # Enable long path support via registry WriteRegDWORD HKLM "SYSTEM\CurrentControlSet\Control\FileSystem" "LongPathsEnabled" 1 ${Else} - IntOp $1 260 - ${MAX_RELATIVE_PATH_LENGTH} + IntOp $1 259 - ${MAX_RELATIVE_PATH_LENGTH} IntOp $1 $1 - 1 # Account for path separator ${If} ${Silent} ${Print} "::error:: The installation path is too long. Your path is $InstDirLen characters, but must be at most $1 characters." diff --git a/news/1282-fix-exe-pkgs-cache-path-length b/news/1282-fix-exe-pkgs-cache-path-length new file mode 100644 index 000000000..125a498e6 --- /dev/null +++ b/news/1282-fix-exe-pkgs-cache-path-length @@ -0,0 +1,19 @@ +### Enhancements + +* + +### Bug fixes + +* Improve the EXE installer path length check added in #1228 to also account for the package cache extraction path (`pkgs//`), preventing installations that were allowed to proceed from failing to extract packages when the install path was long. (#1282) + +### Deprecations + +* + +### Docs + +* + +### Other + +* diff --git a/tests/test_fcp.py b/tests/test_fcp.py index a6947fb65..a82f559d9 100644 --- a/tests/test_fcp.py +++ b/tests/test_fcp.py @@ -97,12 +97,12 @@ def test_check_duplicates_files_returns_max_path_length(mocker): """Verify function returns max path length as third tuple element.""" pc_rec1 = MockPackageCacheRecord( fn="pkg1-1.0.tar.bz2", - extracted_package_dir="/cache/pkg1", + extracted_package_dir="/cache/pkg1", # basename "pkg1" (4) paths=["lib/short.py"], # 12 chars ) pc_rec2 = MockPackageCacheRecord( fn="pkg2-1.0.tar.bz2", - extracted_package_dir="/cache/pkg2", + extracted_package_dir="/cache/pkg2", # basename "pkg2" (4) paths=["lib/python3.10/site-packages/longer.py"], # 38 chars ) @@ -118,7 +118,59 @@ def read_paths_side_effect(extracted_dir): assert len(result) == 3 _, _, max_path_len = result - assert max_path_len == 38 + # Longest is the pkgs-cache path for pkg2: + # "pkgs/" (5) + "pkg2" (4) + "/" (1) + 38 = 48 + assert max_path_len == 48 + + +def test_check_duplicates_files_accounts_for_pkgs_cache_path(mocker): + """Max path length must account for the pkgs\\\\ extraction prefix. + + Conda extracts each package into $INSTDIR\\pkgs\\\\ before + linking files to their final location. That intermediate path is longer than + the final linked path and is what actually overflows MAX_PATH. + """ + pc_rec = MockPackageCacheRecord( + fn="madeuppkg-1.2.3.tar.bz2", + # basename is the folder name used inside $INSTDIR\pkgs\ + extracted_package_dir="/cache/madeuppkg-1.2.3-h4567890_0", # basename: 24 chars + paths=["lib/x.py"], # 8 chars, final linked path in base env + ) + + mock_read_paths = mocker.patch("constructor.fcp.read_paths_json") + mock_read_paths.return_value = MockPathsJson(pc_rec._paths) + + result = check_duplicates_files([pc_rec], "win-64", duplicate_files="skip") + + # "pkgs/" (5) + "madeuppkg-1.2.3-h4567890_0" (26) + "/" (1) + "lib/x.py" (8) = 40 + assert result[2] == 40 + + +def test_check_duplicates_files_long_env_name_beats_pkgs_path(mocker): + """A long extra_envs name can make the linked path exceed the pkgs-cache path. + + The extraction path (pkgs//) and the final linked path + (envs//) are independent; whichever is longer for a given file must + win. Here the env name is long enough that the linked path dominates. + """ + pc_rec = MockPackageCacheRecord( + fn="p-1.tar.bz2", + extracted_package_dir="/cache/p-1-0", # short basename "p-1-0" (5) + paths=["lib/x.py"], # 8 chars + ) + + mock_read_paths = mocker.patch("constructor.fcp.read_paths_json") + mock_read_paths.return_value = MockPathsJson(pc_rec._paths) + + env_prefixes = {pc_rec: "envs/a-very-long-environment-name/"} # 34 chars + result = check_duplicates_files( + [pc_rec], "win-64", duplicate_files="skip", env_prefixes=env_prefixes + ) + + # linked: "envs/a-very-long-environment-name/" (34) + "lib/x.py" (8) = 42 + # pkgs: "pkgs/" (5) + "p-1-0" (5) + "/" (1) + "lib/x.py" (8) = 19 + # max is the linked path + assert result[2] == 42 def test_check_duplicates_files_empty_packages(mocker): @@ -160,8 +212,11 @@ def read_paths_side_effect(extracted_dir): assert len(result) == 3 _, _, max_path_len = result - # "envs/myenv/" (11) + "lib/short.py" (12) = 23 - assert max_path_len == 23 + # The pkgs-cache extraction path dominates the final linked path: + # base-pkg: "pkgs/" (5) + "base-pkg" (8) + "/" (1) + "lib/short.py" (12) = 26 + # env-pkg: "pkgs/" (5) + "env-pkg" (7) + "/" (1) + "lib/short.py" (12) = 25 + # (final linked path "envs/myenv/" (11) + 12 = 23 is shorter) + assert max_path_len == 26 def test_check_duplicates_files_env_prefix_normalizes_trailing_slash(mocker):