-
Notifications
You must be signed in to change notification settings - Fork 177
Improved error handling of long paths for EXE installers #1228
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
base: main
Are you sure you want to change the base?
Changes from all commits
5d68300
85c1bb5
dbd5521
ddcb0b0
c66ac79
28f6004
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,15 +149,44 @@ 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/<name>/" 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()}) | ||
|
|
||
| # 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, "")) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the |
||
| 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/<name>/" 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/<name>/" 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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The path-length handling logic before this section needs to be trimmed. We can get rid of all the |
||
| ${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} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| pytest | ||
| pytest-cov | ||
| pytest-mock | ||
| coverage | ||
| jinja2 | ||
| ruamel.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is deprecated, we should warn about it. In fact, I would remove that template parameter entirely form the NSIS template.