Skip to content

[build](fix) delete submodule Triton-distributed-ascend - #1530

Merged
KanuaK merged 1 commit into
triton-lang:release/3.2.2from
LH-123L:release/3.2.2-td
Aug 13, 2026
Merged

[build](fix) delete submodule Triton-distributed-ascend#1530
KanuaK merged 1 commit into
triton-lang:release/3.2.2from
LH-123L:release/3.2.2-td

Conversation

@LH-123L

@LH-123L LH-123L commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Fixing the cyclic reference issue
Symptom:
The submodule of td contains triton-ascend, and the submodule of triton-ascend contains td. As a result, a cyclic reference occurs when td pulls code.

image

Solution:
The td submodule is removed from triton-ascend. The td code is pulled only when necessary using git clone, avoiding cyclic reference.
image

New contributor declaration

  • I am not making a trivial change, such as fixing a typo in a comment.

  • I have written a PR description following these
    rules.

  • I have run pre-commit run --from-ref origin/main --to-ref HEAD.

  • Select one of the following.

    • I have added tests.
      • /test for lit tests
      • /unittest for C++ tests
      • /python/test for end-to-end tests
    • This PR does not need a test because FILL THIS IN.
  • Select one of the following.

    • I have not added any lit tests.
    • The lit tests I have added follow these best practices,
      including the "tests should be minimal" section. (Usually running Python code
      and using the instructions it generates is not minimal.)

@github-actions github-actions Bot added restricted-files Changes include files outside the repository-construction allowlist. release python Changes to Python runtime or bindings ascend-backend Changes to the Ascend NPU backend labels Aug 13, 2026
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🔍 OpenCodeReview found 5 issue(s) in this PR.

  • ✅ Successfully posted inline: 0 comment(s)
  • 📝 In summary (no line info): 1 comment(s)
  • ❌ Failed to post inline: 4 comment(s)

bug · medium

📄 .gitmodules

⚠️ GitHub could not post this as an inline comment: No line information provided

The git checkout and git pull commands run unconditionally in distributed_dir when the main repo is a git repo, but distributed_dir may not be a git repository (e.g., if the directory was manually created, copied, or from an incomplete clone). This will cause subprocess.check_call to fail with a CalledProcessError. The code should first verify that distributed_dir is itself a git repository before running git commands inside it.

💡 Suggested Change

Before:

if is_git_repo():
        subprocess.check_call([
            "git",
            "checkout",
            "release/3.2.2",
        ], cwd=distributed_dir)
        subprocess.check_call([
            "git",
            "pull",
            "origin",
        ], cwd=distributed_dir)

After:

if is_git_repo():
    # Only run git commands if distributed_dir is itself a git repository
    if (distributed_dir / ".git").exists():
        subprocess.check_call([
            "git",
            "checkout",
            "release/3.2.2",
        ], cwd=distributed_dir)
        subprocess.check_call([
            "git",
            "pull",
            "origin",
        ], cwd=distributed_dir)

bug · medium

📄 .gitmodules (L5-L7)

⚠️ GitHub could not post this as an inline comment: Lines 5-7 could not be resolved (outside PR diff hunks)

The ensure_distributed_submodule() function will attempt git clone from the internet unconditionally when the distributed directory does not exist. However, the project has an is_offline_build() mechanism (line 144-157) to prevent network access in sandboxed/offline environments. The function should check is_offline_build() before cloning and skip or raise an appropriate error if the dependency cannot be satisfied locally.

💡 Suggested Change

Before:

[submodule "third_party/ascend/Triton-distributed-ascend"]
	path = third_party/ascend/Triton-distributed-ascend
	url = https://gitcode.com/Ascend/Triton-distributed-ascend.git

After:

if not distributed_dir.is_dir():
    if is_offline_build():
        raise RuntimeError(
            "Requested an offline build but Triton-distributed-ascend is not available. "
            "Clone it manually to third_party/ascend/Triton-distributed-ascend or disable TRITON_BUILD_DISTRIBUTED."
        )
    subprocess.check_call([
        "git",
        "clone",
        "https://gitcode.com/Ascend/Triton-distributed-ascend.git",
        "-b",
        "release/3.2.2",
    ], cwd=Path(triton_dir) / "third_party" / "ascend")

bug · high

📄 .gitmodules (L5-L7)

⚠️ GitHub could not post this as an inline comment: Lines 5-7 could not be resolved (outside PR diff hunks)

The git pull origin command after checkout introduces non-reproducible builds. After cloning with -b release/3.2.2, the git pull will fetch and merge the latest (potentially untested) commits pushed to that branch on every build, defeating the purpose of pinning to a specific release tag. The original submodule approach pinned commits via .gitmodules; this new approach should either remove git pull entirely or pin to a specific commit hash instead of a branch name for deterministic builds.

💡 Suggested Change

Before:

[submodule "third_party/ascend/Triton-distributed-ascend"]
	path = third_party/ascend/Triton-distributed-ascend
	url = https://gitcode.com/Ascend/Triton-distributed-ascend.git

After:

# Remove git pull to preserve version pinning, or pin to a specific commit hash
if is_git_repo():
    subprocess.check_call([
        "git",
        "checkout",
        "release/3.2.2",
    ], cwd=distributed_dir)

bug · medium

📄 .gitmodules (L5-L7)

⚠️ GitHub could not post this as an inline comment: Lines 5-7 could not be resolved (outside PR diff hunks)

git pull origin without specifying a branch will fail with a non-zero exit code if the cloned repository has no remote tracking branch configured (e.g., if the directory was manually created or cloned in an unusual way). It will also abort on merge conflicts if the local branch has diverged. This makes the build fragile in CI/CD or sandboxed environments.


bug · medium

📄 .gitmodules (L5-L7)

⚠️ GitHub could not post this as an inline comment: Lines 5-7 could not be resolved (outside PR diff hunks)

The condition if not distributed_dir.is_dir() only checks if the directory exists, not if it's a valid checkout. If a previous clone failed partially (leaving an incomplete directory), the code will skip cloning and attempt git operations on a broken repository. Using os.path.isdir with a .git subdirectory check would be more robust.

💡 Suggested Change

Before:

[submodule "third_party/ascend/Triton-distributed-ascend"]
	path = third_party/ascend/Triton-distributed-ascend
	url = https://gitcode.com/Ascend/Triton-distributed-ascend.git

After:

if not (distributed_dir / ".git").is_dir():
    # Clean up any partial directory before cloning
    if distributed_dir.exists():
        shutil.rmtree(distributed_dir)
    subprocess.check_call([
        "git",
        "clone",
        "https://gitcode.com/Ascend/Triton-distributed-ascend.git",
        "-b",
        "release/3.2.2",
    ], cwd=Path(triton_dir) / "third_party" / "ascend")

Comment thread python/setup.py
Comment thread python/setup.py
Comment thread python/setup.py
@LH-123L
LH-123L requested a review from WuTYSFG August 13, 2026 12:36
@KanuaK
KanuaK merged commit ca921c8 into triton-lang:release/3.2.2 Aug 13, 2026
12 checks passed
HinPeng pushed a commit that referenced this pull request Aug 13, 2026
…" (#1550)

This reverts commit ca921c8.

Co-authored-by: jimmy <jimmy@jimmydeMacBook-Pro.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ascend-backend Changes to the Ascend NPU backend python Changes to Python runtime or bindings release restricted-files Changes include files outside the repository-construction allowlist.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants