Improved error handling of long paths for EXE installers by lrandersson · Pull Request #1228 · conda/constructor

lrandersson · 2026-04-28T14:05:47Z

Description

This pr covers #1170 which adds a warning for Windows EXE installers.

When users select an installation path that, combined with the longest file path in the packages, may exceed Windows' 260-character limit, the installer now:

Shows a warning dialog (Yes/No to continue) in interactive mode
Logs ::warning:: in silent mode
Appends a hint to extraction errors if path length may be the cause

The warning only appears when Long Paths is disabled in Windows. Computed at build time by tracking the longest relative path across all packages in check_duplicates_files().

Checklist - did you ...

Add a file to the news directory (using the template) for the next release's release notes?
Add / update necessary tests?
Add / update outdated documentation?

lrandersson · 2026-04-28T14:06:47Z

+    approx_tarballs_size, approx_pkgs_size, max_relative_path_length = check_duplicates_files(
+        all_pc_recs, platform, duplicate_files=duplicate_files


This change is done to account for extra_envs otherwise the error check is only done for the base environment. I've also verified this won't cause unintended duplicate file warnings/errors:

When extra_envs exists, duplicate_files is explicitly set to "skip" (line 465), so check_duplicates_files() skips all duplicate checking logic entirely and only computes sizes (and now also max path length).

When there are no extra_envs, all_pc_recs == pc_recs (they're the same list), so behavior is unchanged.

This change is done to account for extra_envs otherwise the error check is only done for the base environment. I've also verified this won't cause unintended duplicate file warnings/errors:

1. When `extra_envs` exists, `duplicate_files` is explicitly set to "skip" (line 465), so `check_duplicates_files()` skips all duplicate checking logic entirely and only computes sizes (and now also max path length). 2. When there are no `extra_envs`, `all_pc_recs == pc_recs` (they're the same list), so behavior is unchanged.

This should be a comment in the source code.

Does the check take into account that files inside extra_envs will have a longer base path though? For the base environment, the base path is $INSTDIR, for extra environments, it's $INSTDIR\envs\<environment name>.

lrandersson · 2026-04-28T16:29:09Z

    File {{ dist }}
 {%- endfor %}

+    # Set hint for extraction error if path may be too long


This is not needed for the actual underlying issue but I thought it would be more helpful than "Failed to extract packages", let me know what you think, I'm open to remove it completely.

This is not needed for the actual underlying issue but I thought it would be more helpful than "Failed to extract packages", let me know what you think, I'm open to remove it completely.

If we let the user continue, maybe. I would not use a global variable for this though, but put the push instruction into the if-else construction.

lrandersson · 2026-04-28T16:38:01Z

 from __future__ import annotations

 from contextlib import nullcontext
+from unittest.mock import patch


This is only using unittest to use the mock module which is part of the standard library. It's also possible to do this via pyest-mock but requires adding a new separate plugin. Let me know what you think.

The problem this solution is avoiding is that we need to know the exact paths.json format that conda expects, and create objects that satisfy conda's PackageCacheRecord interface. This couples the test to conda's internal implementation details which I don't like.

This is only using unittest to use the mock module which is part of the standard library. It's also possible to do this via pyest-mock but requires adding a new separate plugin. Let me know what you think.

The problem this solution is avoiding is that we need to know the exact paths.json format that conda expects, and create objects that satisfy conda's PackageCacheRecord interface. This couples the test to conda's internal implementation details which I don't like.

pytest-mock is a common enough plug-in. I would rather stick to one framework.

marcoesters · 2026-04-28T20:25:34Z

        ${EndIf}
    ${EndIf}

+    # MAX_PATH check - runs independently of check_path_length


I think this should make check_path_length obsolete. Now that the check is reliable, we should always check because we know the installation will fail.

@marcoesters I agree, I propose that I deprecate this option but potentially in a follow-up PR, what do you think?

I think we should do it here - a simple comment in the schema and a warning when parsing the option should suffice.

Added here 675b9bf

marcoesters · 2026-04-28T20:26:29Z

+        IntOp $0 $InstDirLen + ${MAX_RELATIVE_PATH_LENGTH}
+        ${If} $0 > 260
+            ${If} ${Silent}
+                ${Print} "::warning:: The installation path may be too long ($0 chars, limit 260). Some files could fail to extract."


Since they will (not could) fail to extract, the installation should not continue.

I also think that $0 is not helpful for the user - $InstDirLen (how long their path is) and 260 - ${MAX_RELATIVE_PATH_LENGTH} (how long the installation directory is allowed to be) are though.

I think that's a fair assessment, would this be considered a breaking change though? I agree that an error here makes a lot of sense but I am just worried that making this to an error to begin with would be too intrusive (I dont know if people run these installers with too long paths (hopefully not)).
Perhaps we could make it into a warning and mention this will become an error in a future release?

The installation will fail anyway because Windows will refuse to extract files that will exceed the maximum path length. The error they see is not very helpful either (see, for example, #1022). With this change, the installation fails earlier with a useful error message that gives the user feedback on how to fix the installation.

Unless you see a circumstance where this error would prevent a successful installation from running, I don't see where this is a breaking change.

Hmm, I agree with what you're saying.
I have changed the warning into an error in this commit: 675b9bf.
The hint has also been removed now since it doesnt serve any purpose with the warning now being an error. Note also the error message has been updated since the last review.

marcoesters · 2026-04-28T20:31:54Z

    File {{ dist }}
 {%- endfor %}

+    # Set hint for extraction error if path may be too long


This is not needed for the actual underlying issue but I thought it would be more helpful than "Failed to extract packages", let me know what you think, I'm open to remove it completely.

If we let the user continue, maybe. I would not use a global variable for this though, but put the push instruction into the if-else construction.

marcoesters · 2026-04-28T20:32:41Z

 {%- endfor %}

+    # Set hint for extraction error if path may be too long
+    IntOp $0 $InstDirLen + ${MAX_RELATIVE_PATH_LENGTH}


Does this take the (lack of?) trailing backslash for $InstDir into account?

marcoesters · 2026-04-28T20:37:48Z

+    approx_tarballs_size, approx_pkgs_size, max_relative_path_length = check_duplicates_files(
+        all_pc_recs, platform, duplicate_files=duplicate_files


This change is done to account for extra_envs otherwise the error check is only done for the base environment. I've also verified this won't cause unintended duplicate file warnings/errors:

1. When `extra_envs` exists, `duplicate_files` is explicitly set to "skip" (line 465), so `check_duplicates_files()` skips all duplicate checking logic entirely and only computes sizes (and now also max path length). 2. When there are no `extra_envs`, `all_pc_recs == pc_recs` (they're the same list), so behavior is unchanged.

This should be a comment in the source code.

Does the check take into account that files inside extra_envs will have a longer base path though? For the base environment, the base path is $INSTDIR, for extra environments, it's $INSTDIR\envs\<environment name>.

marcoesters · 2026-04-28T20:38:31Z

 from __future__ import annotations

 from contextlib import nullcontext
+from unittest.mock import patch


This is only using unittest to use the mock module which is part of the standard library. It's also possible to do this via pyest-mock but requires adding a new separate plugin. Let me know what you think.

The problem this solution is avoiding is that we need to know the exact paths.json format that conda expects, and create objects that satisfy conda's PackageCacheRecord interface. This couples the test to conda's internal implementation details which I don't like.

pytest-mock is a common enough plug-in. I would rather stick to one framework.

lrandersson · 2026-04-29T14:53:39Z

@marcoesters
I've had made some changes in commit 1552b5d to address some of the review comments:

check_duplicates_files() - Added env_prefixes parameter

You're right we missed accounting for the additional prefix for extra_envs. To fix this we needed to create a mapping because the list of files that were iterating over are not tracking the respective environment they are part of. The function now accepts an optional env_prefixes dict that maps package records to their environment prefix (e.g., "envs/myenv/"). When computing max path length, the prefix length is added to each file's path.

_main() - Build env_prefixes mapping (needed for above)

While iterating extra_envs, we now build the env_prefixes dict mapping each package record to its "envs/<name>/" prefix. If a package appears in multiple extra_envs, we keep the longest prefix. This mapping is passed to check_duplicates_files().

NSIS template - Path separator fix

Added + 1 to both MAX_PATH calculations to account for the backslash between install directory and relative path (e.g., C:\App + \ + lib\foo.py).

Tests - Migrated to pytest-mock

Switched from unittest.mock.patch decorator to pytest-mock mocker fixture. Added new test test_check_duplicates_files_with_env_prefixes to verify the prefix calculation works correctly.

lrandersson · 2026-05-01T09:48:42Z

@marcoesters I rebased the branch with the latest changes from main so this PR needs a new review

marcoesters · 2026-05-01T14:43:10Z

        ${EndIf}
    ${EndIf}

+    # MAX_PATH check - installation will fail if path exceeds 260 characters


The path-length handling logic before this section needs to be trimmed. We can get rid of all the $CheckPathLength logic.

marcoesters · 2026-05-01T14:47:23Z

    Only applies if `register_python` is true.
    """
-    check_path_length: bool = False
+    check_path_length: bool = Field(False, deprecated=True)


If this is deprecated, we should warn about it. In fact, I would remove that template parameter entirely form the NSIS template.

marcoesters · 2026-05-01T14:48:21Z


-def check_duplicates_files(pc_recs, platform, duplicate_files="error"):
+def check_duplicates_files(
+    pc_recs: Iterable["PackageCacheRecord"],


If you add from __future__ import annotations, you don't need the strings representation. You can then place Literal and Iterable into the type checking block.

marcoesters · 2026-05-01T14:52:41Z

        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, ""))


Does the env_prefix contain the trailing (back)slash? Might be worth documenting as a comment.

lrandersson · 2026-05-01T15:55:58Z

@marcoesters see commit 28f6004 for the latest changes

"The path-length handling logic before this section needs to be trimmed. We can get rid of all the $CheckPathLength logic."

Done. Removed all $CheckPathLength and related code.

"If this is deprecated, we should warn about it. In fact, I would remove that template parameter entirely from the NSIS template."

Done. If I understand this correctly, the deprecation warning is already handled via the JSON schema validator (deprecated_validator in construct.py). Removed the check_path_length template variable from winexe.py so it's no longer passed to the NSIS template.

"If you add from future import annotations, you don't need the strings representation. You can then place Literal and Iterable into the type checking block."

Done. Added from __future__ import annotations, moved Iterable and Literal to the TYPE_CHECKING section, and removed the string quotes from type annotations.

"Does the env_prefix contain the trailing (back)slash? Might be worth documenting as a comment."

Done. Added to the docstring: "The prefix string should include the trailing slash (e.g., "envs/myenv/")."

lrandersson self-assigned this Apr 28, 2026

conda-bot added this to 🔎 Review Apr 28, 2026

github-project-automation Bot moved this to 🆕 New in 🔎 Review Apr 28, 2026

conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Apr 28, 2026

lrandersson commented Apr 28, 2026

View reviewed changes

lrandersson marked this pull request as ready for review April 28, 2026 16:38

lrandersson requested a review from a team as a code owner April 28, 2026 16:38

marcoesters reviewed Apr 28, 2026

View reviewed changes

lrandersson requested a review from marcoesters April 29, 2026 16:12

lrandersson commented Apr 29, 2026

View reviewed changes

Comment thread examples/miniforge/construct.yaml

lrandersson force-pushed the dev-ra-875 branch 2 times, most recently from 0fcb67b to 88742bf Compare April 30, 2026 15:11

lrandersson added 5 commits May 1, 2026 05:47

Improve error handling of long paths for EXE installers

5d68300

Fix format

85c1bb5

Improve format

dbd5521

Fix some review comments

ddcb0b0

Review fixes, change warning into error

c66ac79

lrandersson force-pushed the dev-ra-875 branch from 88742bf to c66ac79 Compare May 1, 2026 09:47

marcoesters reviewed May 1, 2026

View reviewed changes

Review fixes

28f6004

		approx_tarballs_size, approx_pkgs_size, max_relative_path_length = check_duplicates_files(
		all_pc_recs, platform, duplicate_files=duplicate_files

Conversation

lrandersson commented Apr 28, 2026

Description

Checklist - did you ...

Uh oh!

lrandersson Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrandersson commented Apr 29, 2026

Uh oh!

Uh oh!

lrandersson commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrandersson commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lrandersson Apr 28, 2026 •

edited

Loading

lrandersson commented May 1, 2026 •

edited

Loading

lrandersson commented May 1, 2026 •

edited

Loading