diff --git a/CONSTRUCT.md b/CONSTRUCT.md index 17a71302f..a36239ab6 100644 --- a/CONSTRUCT.md +++ b/CONSTRUCT.md @@ -552,11 +552,11 @@ interactive installation. (Windows only). Only applies if `register_python` is true. -### `check_path_length` +### ~~`check_path_length`~~ -Check the length of the path where the distribution is installed to ensure nodejs -can be installed. Raise a message to request shorter paths (less than 46 character) -or enable long paths on windows > 10 (require admin right). Default is False. (Windows only). +_Deprecated_. Path length validation is now always performed using the computed +maximum relative path length from the package contents. This option will be +removed in a future version. (Windows only). ### `check_path_spaces` diff --git a/constructor/_schema.py b/constructor/_schema.py index 87945ad03..280fff613 100644 --- a/constructor/_schema.py +++ b/constructor/_schema.py @@ -721,11 +721,11 @@ class ConstructorConfiguration(BaseModel): Only applies if `register_python` is true. """ - check_path_length: bool = False + check_path_length: bool = Field(False, deprecated=True) """ - Check the length of the path where the distribution is installed to ensure nodejs - can be installed. Raise a message to request shorter paths (less than 46 character) - or enable long paths on windows > 10 (require admin right). Default is False. (Windows only). + _Deprecated_. Path length validation is now always performed using the computed + maximum relative path length from the package contents. This option will be + removed in a future version. (Windows only). """ check_path_spaces: bool = True """ diff --git a/constructor/data/construct.schema.json b/constructor/data/construct.schema.json index f0178d738..1c72a59f4 100644 --- a/constructor/data/construct.schema.json +++ b/constructor/data/construct.schema.json @@ -477,7 +477,8 @@ }, "check_path_length": { "default": false, - "description": "Check the length of the path where the distribution is installed to ensure nodejs can be installed. Raise a message to request shorter paths (less than 46 character) or enable long paths on windows > 10 (require admin right). Default is False. (Windows only).", + "deprecated": true, + "description": "_Deprecated_. Path length validation is now always performed using the computed maximum relative path length from the package contents. This option will be removed in a future version. (Windows only).", "title": "Check Path Length", "type": "boolean" }, diff --git a/constructor/fcp.py b/constructor/fcp.py index 4ad7f53ca..f92c36b3d 100644 --- a/constructor/fcp.py +++ b/constructor/fcp.py @@ -7,6 +7,8 @@ fcp (fetch conda packages) module """ +from __future__ import annotations + import logging import os import shutil @@ -38,6 +40,9 @@ ) if TYPE_CHECKING: + from collections.abc import Iterable + from typing import Literal + from .conda_interface import PackageCacheRecord logger = logging.getLogger(__name__) @@ -144,8 +149,36 @@ def _fetch(download_dir, precs): return list(dict.fromkeys(PrefixGraph(pc.iter_records()).graph)) -def check_duplicates_files(pc_recs, platform, duplicate_files="error"): +def check_duplicates_files( + pc_recs: Iterable[PackageCacheRecord], + platform: str, + duplicate_files: Literal["error", "warn", "skip"] = "error", + env_prefixes: dict[PackageCacheRecord, str] | None = None, +) -> tuple[int, int, int]: + """ + Check for duplicate files across packages and compute size/path metrics. + + Iterates through all files in the provided package cache records to: + 1. Detect duplicate files (same path in multiple packages) + 2. Compute approximate tarball and extracted sizes + 3. Track the longest relative file path (for MAX_PATH validation on Windows) + + Args: + pc_recs: Package cache records to check. + platform: Target platform string (e.g., "win-64", "linux-64"). + duplicate_files: How to handle duplicates - "error", "warn", or "skip". + env_prefixes: Optional dict mapping PackageCacheRecord -> path prefix string. + Used to account for extra_envs paths which are installed under + "envs//" rather than the base install directory. Records not + in this dict are assumed to be in the base environment (no prefix). + The prefix string should include the trailing slash (e.g., "envs/myenv/"). + + Returns: + Tuple of (approx_tarball_size, approx_extracted_size, max_relative_path_length) + """ assert duplicate_files in ("warn", "skip", "error") + if env_prefixes is None: + env_prefixes = {} map_members_scase = defaultdict(set) map_members_icase = defaultdict(lambda: {"files": set(), "fns": set()}) @@ -153,6 +186,7 @@ def check_duplicates_files(pc_recs, platform, duplicate_files="error"): # Keep a min, 50MB buffer size total_tarball_size = 52428800 total_extracted_pkgs_size = 52428800 + max_relative_path_length = 0 for pc_rec in pc_recs: fn = pc_rec.fn @@ -161,8 +195,12 @@ def check_duplicates_files(pc_recs, platform, duplicate_files="error"): total_tarball_size += int(pc_rec.get("size", 0)) paths_data = read_paths_json(extracted_package_dir).paths + env_prefix_len = len(env_prefixes.get(pc_rec, "")) 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) + ) try: size = path_data.size_in_bytes or getsize(join(extracted_package_dir, short_path)) except AttributeError: @@ -176,7 +214,7 @@ def check_duplicates_files(pc_recs, platform, duplicate_files="error"): map_members_icase[short_path_lower]["fns"].add(fn) if duplicate_files == "skip": - return total_tarball_size, total_extracted_pkgs_size + return total_tarball_size, total_extracted_pkgs_size, max_relative_path_length logger.info("Checking for duplicate files ...") for member in map_members_scase: @@ -201,7 +239,7 @@ def check_duplicates_files(pc_recs, platform, duplicate_files="error"): else: sys.exit(f"Error: {msg_str}") - return total_tarball_size, total_extracted_pkgs_size + return total_tarball_size, total_extracted_pkgs_size, max_relative_path_length def _precs_from_environment(environment, input_dir): @@ -443,18 +481,24 @@ def _main( input_dir=input_dir, ) if dry_run: - return None, None, None, None, None, None, None, None + return None, None, None, None, None, None, None, None, None pc_recs, _urls, dists, has_conda = _fetch_precs( precs, download_dir, transmute_file_type=transmute_file_type ) all_pc_recs = pc_recs.copy() extra_envs_data = {} + env_prefixes = {} # Maps pc_rec -> "envs//" prefix for max path calculation for env_name, env_precs in extra_envs_precs.items(): env_pc_recs, env_urls, env_dists, _ = _fetch_precs( env_precs, download_dir, transmute_file_type=transmute_file_type ) extra_envs_data[env_name] = {"_urls": env_urls, "_dists": env_dists, "_records": env_precs} + env_prefix = f"envs/{env_name}/" + for pc_rec in env_pc_recs: + existing_prefix = env_prefixes.get(pc_rec, "") + if len(env_prefix) > len(existing_prefix): + env_prefixes[pc_rec] = env_prefix all_pc_recs += env_pc_recs duplicate_files = "warn" if ignore_duplicate_files else "error" @@ -463,8 +507,12 @@ def _main( duplicate_files = "skip" all_pc_recs = list({rec: None for rec in all_pc_recs}) # deduplicate - approx_tarballs_size, approx_pkgs_size = check_duplicates_files( - pc_recs, platform, duplicate_files=duplicate_files + # Pass all_pc_recs (base + extra_envs) to check_duplicates_files: + # - When extra_envs exists, duplicate_files="skip" so only sizes and max path are computed + # - When no extra_envs, all_pc_recs == pc_recs + # - env_prefixes dict ensures max path accounts for "envs//" prefix in extra_envs + approx_tarballs_size, approx_pkgs_size, max_relative_path_length = check_duplicates_files( + all_pc_recs, platform, duplicate_files=duplicate_files, env_prefixes=env_prefixes ) return ( @@ -476,6 +524,7 @@ def _main( approx_pkgs_size, has_conda, extra_envs_data, + max_relative_path_length, ) @@ -529,6 +578,7 @@ def main(info, verbose=True, dry_run=False, conda_exe="conda.exe"): approx_pkgs_size, has_conda, extra_envs_info, + max_relative_path_length, ) = _main( name, version, @@ -561,3 +611,4 @@ def main(info, verbose=True, dry_run=False, conda_exe="conda.exe"): info["_has_conda"] = has_conda # contains {env_name: [_dists, _urls, _records]} for each extra environment info["_extra_envs_info"] = extra_envs_info + info["_max_relative_path_length"] = max_relative_path_length diff --git a/constructor/nsis/main.nsi.tmpl b/constructor/nsis/main.nsi.tmpl index 10bf6c9f4..bed31f6ef 100644 --- a/constructor/nsis/main.nsi.tmpl +++ b/constructor/nsis/main.nsi.tmpl @@ -130,6 +130,7 @@ ${Using:StrFunc} StrStr # are not written into install.log. STEP_LOG creates an intermittent file # that is output into these streams after the commands finish. !define STEP_LOG "$INSTDIR\.step.log" +!define MAX_RELATIVE_PATH_LENGTH {{ max_relative_path_length }} var /global INIT_CONDA var /global REG_PY @@ -156,7 +157,6 @@ var /global ARGV_RegisterPython var /global ARGV_NoRegistry var /global ARGV_NoScripts var /global ARGV_NoShortcuts -var /global ARGV_CheckPathLength var /global ARGV_QuietMode {%- if uninstall_with_conda_exe %} var /global ARGV_Uninst_RemoveConfigFiles @@ -165,7 +165,6 @@ var /global ARGV_Uninst_RemoveCaches {%- endif %} var /global IsDomainUser -var /global CheckPathLength var /global LongPathsEnabled var /global InstDirLen @@ -292,7 +291,6 @@ Function SkipPageIfUACInnerInstance FunctionEnd Function InitializeVariables - StrCpy $CheckPathLength "{{ 1 if check_path_length else 0 }}" StrCpy $NO_REGISTRY "0" # Package cache option @@ -365,7 +363,6 @@ FunctionEnd /NoRegistry=[0|1] [default: AllUsers: 0, JustMe: 0]$\n\ /NoScripts=[0|1] [default: 0]$\n\ /NoShortcuts=[0|1] [default: 0]$\n\ - /CheckPathLength=[0|1] [default: {{ 1 if check_path_length else 0 }}]$\n\ /? (show this help message)$\n\ /S (run in CLI/headless mode)$\n\ /Q (quiet mode, do not print output to console)$\n\ @@ -497,16 +494,6 @@ FunctionEnd StrCpy $Ana_CreateShortcuts_State ${BST_UNCHECKED} ${EndIf} - ClearErrors - ${GetOptions} $ARGV "/CheckPathLength=" $ARGV_CheckPathLength - ${IfNot} ${Errors} - ${If} $ARGV_CheckPathLength = "0" - StrCpy $CheckPathLength "0" - ${ElseIf} $ARGV_CheckPathLength = "1" - StrCpy $CheckPathLength "1" - ${EndIf} - ${EndIf} - ClearErrors ${GetOptions} $ARGV "/Q" $ARGV_QuietMode ${IfNot} ${Errors} @@ -1083,34 +1070,22 @@ Function OnDirectoryLeave ReadRegStr $LongPathsEnabled HKLM "SYSTEM\CurrentControlSet\Control\FileSystem" "LongPathsEnabled" StrLen $InstDirLen "$InstDir" - ${If} $CheckPathLength == "1" - ${AndIf} $LongPathsEnabled == "0" - ${AndIf} $InstDirLen > 46 - ; With windows 10, we can enable support for long path, for earlier - ; version, suggest user to use shorter installation path - ${If} ${AtLeastWin10} - ${AndIfNot} $NO_REGISTRY = "1" - ; If we have admin right, we enable long path on windows - ${If} ${UAC_IsAdmin} - WriteRegDWORD HKLM "SYSTEM\CurrentControlSet\Control\FileSystem" "LongPathsEnabled" 1 - ; If we don't have admin right, we suggest a shorter path or suggest to run with admin right + # MAX_PATH check - installation will fail if path exceeds 260 characters + ${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 + IntOp $1 260 - ${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." ${Else} - ${Print} "::error:: The installation path should be shorter than 46 characters or \ - the installation requires administrator rights to enable long \ - path on Windows." - MessageBox MB_OK|MB_ICONSTOP "The installation path should be shorter than 46 characters or \ - the installation requires administrator rights to enable long \ - path on Windows." \ - /SD IDOK - Abort + MessageBox MB_OK|MB_ICONSTOP \ + "The installation path is too long.$\n$\n\ + Your path is $InstDirLen characters, but must be at most $1 characters.$\n$\n\ + Please choose a shorter installation path." \ + /SD IDOK ${EndIf} - ; If we don't have admin right, we suggest a shorter path or suggest to run with admin right - ${Else} - ${Print} "::error:: The installation path should be shorter than 46 characters. \ - Please choose another location." - MessageBox MB_OK|MB_ICONSTOP "The installation path should be shorter than 46 characters. \ - Please choose another location." \ - /SD IDOK Abort ${EndIf} ${EndIf} diff --git a/constructor/winexe.py b/constructor/winexe.py index caf49f066..5cd2ec522 100644 --- a/constructor/winexe.py +++ b/constructor/winexe.py @@ -250,8 +250,8 @@ def make_nsi( variables.update(ns_platform(info["_platform"])) variables["initialize_conda"] = info.get("initialize_conda", "classic") variables["initialize_by_default"] = info.get("initialize_by_default", None) - variables["check_path_length"] = info.get("check_path_length", False) variables["check_path_spaces"] = info.get("check_path_spaces", True) + variables["max_relative_path_length"] = info.get("_max_relative_path_length", 0) variables["keep_pkgs"] = info.get("keep_pkgs") or False variables["pre_install_exists"] = bool(info.get("pre_install")) variables["post_install_exists"] = bool(info.get("post_install")) diff --git a/docs/source/construct-yaml.md b/docs/source/construct-yaml.md index 17a71302f..a36239ab6 100644 --- a/docs/source/construct-yaml.md +++ b/docs/source/construct-yaml.md @@ -552,11 +552,11 @@ interactive installation. (Windows only). Only applies if `register_python` is true. -### `check_path_length` +### ~~`check_path_length`~~ -Check the length of the path where the distribution is installed to ensure nodejs -can be installed. Raise a message to request shorter paths (less than 46 character) -or enable long paths on windows > 10 (require admin right). Default is False. (Windows only). +_Deprecated_. Path length validation is now always performed using the computed +maximum relative path length from the package contents. This option will be +removed in a future version. (Windows only). ### `check_path_spaces` diff --git a/tests/requirements.txt b/tests/requirements.txt index 7284b97cb..637b33cca 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,5 +1,6 @@ pytest pytest-cov +pytest-mock coverage jinja2 ruamel.yaml diff --git a/tests/test_examples.py b/tests/test_examples.py index 4ec5c9a64..71c944fcd 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -162,7 +162,7 @@ def _check_installer_log(install_dir): "Once you have installed it, set NSIS_USING_LOG_BUILD=1.\n" "Otherwise, this usually means that the destination folder could not be created.\n" "Possible causes: permissions, non-supported characters, long paths...\n" - "Consider setting 'check_path_spaces' and 'check_path_length' to 'False'." + "Consider setting 'check_path_spaces' to 'False'." ) if error_lines: raise AssertionError("\n".join(error_lines)) diff --git a/tests/test_fcp.py b/tests/test_fcp.py index a27411580..40d233410 100644 --- a/tests/test_fcp.py +++ b/tests/test_fcp.py @@ -4,7 +4,7 @@ import pytest -from constructor.fcp import check_duplicates, exclude_packages +from constructor.fcp import check_duplicates, check_duplicates_files, exclude_packages class GenericObject: @@ -64,3 +64,101 @@ def test_exclude_packages(values: tuple[..., GenericObject], expected_value): with context: packages = exclude_packages(*values) assert packages == expected_value + + +class MockPathData: + """Represents a file path entry.""" + + def __init__(self, path): + self.path = path + self.size_in_bytes = 1 # must be non-zero to avoid filesystem access + + +class MockPathsJson: + """Wraps a list of MockPathData entries.""" + + def __init__(self, paths): + self.paths = [MockPathData(p) for p in paths] + + +class MockPackageCacheRecord: + """Represents a package record.""" + + def __init__(self, fn, extracted_package_dir, paths): + self.fn = fn + self.extracted_package_dir = extracted_package_dir + self._paths = paths + + def get(self, key, default=None): + return default + + +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", + paths=["lib/short.py"], # 12 chars + ) + pc_rec2 = MockPackageCacheRecord( + fn="pkg2-1.0.tar.bz2", + extracted_package_dir="/cache/pkg2", + paths=["lib/python3.10/site-packages/longer.py"], # 38 chars + ) + + def read_paths_side_effect(extracted_dir): + if extracted_dir == "/cache/pkg1": + return MockPathsJson(pc_rec1._paths) + return MockPathsJson(pc_rec2._paths) + + mock_read_paths = mocker.patch("constructor.fcp.read_paths_json") + mock_read_paths.side_effect = read_paths_side_effect + + result = check_duplicates_files([pc_rec1, pc_rec2], "win-64", duplicate_files="skip") + + assert len(result) == 3 + _, _, max_path_len = result + assert max_path_len == 38 + + +def test_check_duplicates_files_empty_packages(mocker): + """Verify returns 0 when no packages provided.""" + mock_read_paths = mocker.patch("constructor.fcp.read_paths_json") + + result = check_duplicates_files([], "win-64", duplicate_files="skip") + + assert len(result) == 3 + assert result[2] == 0 + mock_read_paths.assert_not_called() + + +def test_check_duplicates_files_with_env_prefixes(mocker): + """Verify env_prefixes adds prefix length to max path calculation.""" + pc_rec_base = MockPackageCacheRecord( + fn="base-pkg-1.0.tar.bz2", + extracted_package_dir="/cache/base-pkg", + paths=["lib/short.py"], # 12 chars, no prefix -> 12 + ) + pc_rec_env = MockPackageCacheRecord( + fn="env-pkg-1.0.tar.bz2", + extracted_package_dir="/cache/env-pkg", + paths=["lib/short.py"], # 12 chars, with "envs/myenv/" prefix (11) -> 23 + ) + + def read_paths_side_effect(extracted_dir): + if extracted_dir == "/cache/base-pkg": + return MockPathsJson(pc_rec_base._paths) + return MockPathsJson(pc_rec_env._paths) + + mock_read_paths = mocker.patch("constructor.fcp.read_paths_json") + mock_read_paths.side_effect = read_paths_side_effect + + env_prefixes = {pc_rec_env: "envs/myenv/"} + result = check_duplicates_files( + [pc_rec_base, pc_rec_env], "win-64", duplicate_files="skip", env_prefixes=env_prefixes + ) + + assert len(result) == 3 + _, _, max_path_len = result + # "envs/myenv/" (11) + "lib/short.py" (12) = 23 + assert max_path_len == 23