Skip to content

Migrate to new constraints location#345

Merged
chennes merged 1 commit intoFreeCAD:devfrom
chennes:migrateToNewConstraintsLocation
Feb 10, 2026
Merged

Migrate to new constraints location#345
chennes merged 1 commit intoFreeCAD:devfrom
chennes:migrateToNewConstraintsLocation

Conversation

@chennes
Copy link
Member

@chennes chennes commented Feb 10, 2026

No description provided.

Copilot AI review requested due to automatic review settings February 10, 2026 03:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates Addon Manager’s pip constraints handling to match a new constraints file layout/location.

Changes:

  • Update pip constraints filename/path construction to use {major}.{minor}/constraints.txt.
  • Update default pip_constraints_path to a new GitHub raw URL under FreeCAD/Addons/.../Data/Python/.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
addonmanager_utilities.py Adjusts how the constraints file path is derived for pip installs.
addonmanager_preferences_defaults.json Points the default constraints base URL to the new repository/path.
Comments suppressed due to low confidence (2)

addonmanager_utilities.py:644

  • For the local-path case, os.path.join(constraints, expected_filename) is joining a base path with a string that already contains a forward slash. This can produce mixed separators (e.g. C:\base\3.11/constraints.txt) and is harder to reason about across platforms. Consider building the relative path with os.path.join(f"{major}.{minor}", "constraints.txt") (and separately use a POSIX join for the URL case) so each join uses the correct separator semantics.
            expected_filename = f"{major}.{minor}/constraints.txt"
            if parsed_url.scheme == "https":
                # The only supported remote scheme is https, and this is the default setup
                if not constraints.endswith("/"):
                    constraints += "/"
                constraints += expected_filename
            else:
                # If it wasn't https, treat it like it's a local path
                constraints = os.path.join(constraints, expected_filename)
            args.extend(["--constraint", constraints])

addonmanager_utilities.py:645

  • This change alters the constraints URL/path constructed by create_pip_call, but there’s currently no unit test coverage for this function in AddonManagerTest/app/test_utilities.py (which already tests other functions from addonmanager_utilities). Adding tests that mock fci.Preferences().get("pip_constraints_path") and sys.version_info to assert the resulting --constraint argument (both for https URLs and local paths) would help prevent regressions in dependency installation.
    if "install" in args:
        constraints = fci.Preferences().get("pip_constraints_path")
        if not constraints:
            fci.Console.PrintWarning(
                "pip constraints explicitly disabled by unsetting 'pip_constraints_path'\n"
            )
        else:
            parsed_url = urlparse(constraints)
            major = sys.version_info.major
            minor = sys.version_info.minor
            expected_filename = f"{major}.{minor}/constraints.txt"
            if parsed_url.scheme == "https":
                # The only supported remote scheme is https, and this is the default setup
                if not constraints.endswith("/"):
                    constraints += "/"
                constraints += expected_filename
            else:
                # If it wasn't https, treat it like it's a local path
                constraints = os.path.join(constraints, expected_filename)
            args.extend(["--constraint", constraints])


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 633 to 636
major = sys.version_info.major
minor = sys.version_info.minor
expected_filename = f"constraints-py{major}{minor}.txt"
expected_filename = f"{major}.{minor}/constraints.txt"
if parsed_url.scheme == "https":
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected_filename now contains a path segment (e.g. 3.11/constraints.txt), not just a filename. Consider renaming it (e.g. constraints_rel_path) to avoid confusion and make it clearer that a directory component is included.

Copilot uses AI. Check for mistakes.
@chennes chennes force-pushed the migrateToNewConstraintsLocation branch from 5dedb02 to ab12d44 Compare February 10, 2026 03:37
@chennes chennes merged commit 5a03e25 into FreeCAD:dev Feb 10, 2026
11 checks passed
@chennes chennes deleted the migrateToNewConstraintsLocation branch February 10, 2026 03:39
@chennes chennes added the release to main Trigger an Action to create a PR backporting to `main` label Feb 10, 2026
@github-actions
Copy link

Successfully created backport PR for main:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release to main Trigger an Action to create a PR backporting to `main`

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant