Skip to content

Fix submodule URL resolution for JihuLab esp-mirror (EIM-885)#913

Open
ceerqingtingml wants to merge 1 commit into
espressif:masterfrom
ceerqingtingml:fix/jihulab-mirror-submodule-urls
Open

Fix submodule URL resolution for JihuLab esp-mirror (EIM-885)#913
ceerqingtingml wants to merge 1 commit into
espressif:masterfrom
ceerqingtingml:fix/jihulab-mirror-submodule-urls

Conversation

@ceerqingtingml

Copy link
Copy Markdown

Description

Fix incorrect submodule URL resolution when using the JihuLab mirror (https://jihulab.com/esp-mirror).

Relative paths in .gitmodules (e.g. ../../espressif/esp32-bt-lib.git) were resolved against the mirror parent URL, which dropped the esp-mirror prefix and produced incorrect URLs like https://jihulab.com/espressif/esp32-bt-lib.git instead of https://jihulab.com/esp-mirror/espressif/esp32-bt-lib.git.

This PR resolves relative URLs against the logical GitHub parent URL first, then rewrites to the mirror prefix — equivalent to git config url.https://jihulab.com/esp-mirror/.insteadOf https://github.com/. The same behavior is applied in the gix submodule path and the git CLI fallback path. Unit tests are included.

Related

N/A (no existing issue filed)

Testing

  • cargo test mirror_url_tests --lib in src-tauri/
  • Manual install on Windows with idf_mirror = "https://jihulab.com/esp-mirror": esp32-bt-lib resolves to https://jihulab.com/esp-mirror/espressif/esp32-bt-lib.git and submodules fetch successfully

Checklist

  • This PR does not introduce breaking changes
  • All CI checks pass (pending)
  • Documentation updated (not required for this fix)
  • Tests added/updated

Relative submodule paths in .gitmodules are defined against the GitHub
layout. Resolving them directly against a mirror parent URL drops the
esp-mirror prefix (e.g. esp32-bt-lib becomes jihulab.com/espressif/...
instead of jihulab.com/esp-mirror/espressif/...).

Resolve relative URLs against the logical GitHub parent, then rewrite to
the mirror prefix (equivalent to git url.insteadOf). Apply the same
behavior in the git CLI fallback path and add unit tests.

Co-authored-by: Cursor <cursoragent@cursor.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for an optional GitHub mirror prefix when cloning repositories and updating submodules. It adds helper functions to rewrite URLs to and from the mirror, configures local git settings using insteadOf, and updates the submodule update logic to resolve relative URLs correctly. The review feedback suggests robustly handling trailing slashes when comparing mirror URLs to the default GitHub URL across several helper functions, and using prefix-based replacement instead of global string replacement in apply_github_mirror to prevent accidental replacements elsewhere in the URL.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +2070 to +2073
if mirror == "https://github.com" {
return url.to_string();
}
let mirror_prefix = format!("{}/", mirror.trim_end_matches('/'));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Normalizing the mirror URL by trimming trailing slashes before comparing it to "https://github.com" makes the check robust. If mirror is configured with a trailing slash (e.g., "https://github.com/"), the current check will evaluate to false, leading to redundant string formatting and replacement operations.

    let mirror_base = mirror.trim_end_matches('/');
    if mirror_base == "https://github.com" {
        return url.to_string();
    }
    let mirror_prefix = format!("{}/", mirror_base);

Comment on lines +2088 to +2094
if mirror == "https://github.com" {
return url.to_string();
}
let mirror_base = mirror.trim_end_matches('/');
let mirror_prefix = format!("{}/", mirror_base);
url.replace("https://github.com/", &mirror_prefix)
.replace("https://github.com", mirror_base)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using global replace on the URL can lead to unexpected bugs if the string "https://github.com/" or "https://github.com" appears elsewhere in the URL (for example, in a repository name, branch name, or path). It is much safer to only replace the prefix at the start of the URL using starts_with and replacen with a limit of 1, similar to how reverse_github_mirror is implemented. Additionally, normalizing the mirror URL by trimming trailing slashes first ensures that the GitHub check is robust.

    let mirror_base = mirror.trim_end_matches('/');
    if mirror_base == "https://github.com" {
        return url.to_string();
    }
    let mirror_prefix = format!("{}/", mirror_base);
    if url.starts_with("https://github.com/") {
        url.replacen("https://github.com/", &mirror_prefix, 1)
    } else if url == "https://github.com" {
        mirror_base.to_string()
    } else { 
        url.to_string()
    }

Comment on lines +2099 to +2102
if mirror == "https://github.com" {
return None;
}
let mirror_prefix = format!("{}/", mirror.trim_end_matches('/'));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Normalize the mirror URL by trimming trailing slashes before comparing it to "https://github.com" to ensure that a mirror configured as "https://github.com/" is correctly identified and skipped, avoiding the generation of redundant git config pairs.

    let mirror_base = mirror.trim_end_matches('/');
    if mirror_base == "https://github.com" {
        return None;
    }
    let mirror_prefix = format!("{}/", mirror_base);

@github-actions github-actions Bot changed the title Fix submodule URL resolution for JihuLab esp-mirror Fix submodule URL resolution for JihuLab esp-mirror (EIM-885) Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant