Skip to content

Commit 8a4d044

Browse files
weihangloehuss
weihanglo
authored and
ehuss
committed
fix: normalize relative git submodule urls with ssh://
Git only assumes a submodule URL is a relative path if it starts with `./` or `../` [^1]. To fetch the correct repo, we need to construct an aboslute submodule URL. At this moment it comes with some limitations: * GitHub doesn't accept non-normalized URLs wth relative paths. (`ssh://[email protected]/rust-lang/cargo.git/relative/..` is invalid) * `url` crate cannot parse SCP-like URLs. (`[email protected]:rust-lang/cargo.git` is not a valid WHATWG URL) To overcome these, this patch always tries `Url::parse` first to normalize the path. If it couldn't, append the relative path as the last resort and pray the remote git service supports non-normalized URLs. See also #12404 and #12295. [^1]: <https://git-scm.com/docs/git-submodule>
1 parent e999957 commit 8a4d044

File tree

2 files changed

+155
-17
lines changed

2 files changed

+155
-17
lines changed

‎src/cargo/sources/git/utils.rs‎

Lines changed: 152 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -440,20 +440,7 @@ impl<'a> GitCheckout<'a> {
440440
return Ok(());
441441
}
442442

443-
// Git only assumes a URL is a relative path if it starts with `./` or `../`.
444-
// See [`git submodule add`] documentation.
445-
//
446-
// [`git submodule add`]: https://git-scm.com/docs/git-submodule
447-
let child_remote_url = if ["./", "../"].iter().any(|p| child_url_str.starts_with(p)) {
448-
let mut new_remote_url = parent_remote_url.to_string();
449-
if !new_remote_url.ends_with('/') {
450-
new_remote_url.push('/');
451-
}
452-
new_remote_url.push_str(child_url_str);
453-
Cow::from(new_remote_url)
454-
} else {
455-
Cow::from(child_url_str)
456-
};
443+
let child_remote_url = absolute_submodule_url(parent_remote_url, child_url_str)?;
457444

458445
// A submodule which is listed in .gitmodules but not actually
459446
// checked out will not have a head id, so we should ignore it.
@@ -507,6 +494,58 @@ impl<'a> GitCheckout<'a> {
507494
}
508495
}
509496

497+
/// Constructs an absolute URL for a child submodule URL with its parent base URL.
498+
///
499+
/// Git only assumes a submodule URL is a relative path if it starts with `./`
500+
/// or `../` [^1]. To fetch the correct repo, we need to construct an absolute
501+
/// submodule URL.
502+
///
503+
/// At this moment it comes with some limitations:
504+
///
505+
/// * GitHub doesn't accept non-normalized URLs with relative paths.
506+
/// (`ssh://[email protected]/rust-lang/cargo.git/relative/..` is invalid)
507+
/// * `url` crate cannot parse SCP-like URLs.
508+
/// (`[email protected]:rust-lang/cargo.git` is not a valid WHATWG URL)
509+
///
510+
/// To overcome these, this patch always tries [`Url::parse`] first to normalize
511+
/// the path. If it couldn't, append the relative path as the last resort and
512+
/// pray the remote git service supports non-normalized URLs.
513+
///
514+
/// See also rust-lang/cargo#12404 and rust-lang/cargo#12295.
515+
///
516+
/// [^1]: <https://git-scm.com/docs/git-submodule>
517+
fn absolute_submodule_url<'s>(base_url: &str, submodule_url: &'s str) -> CargoResult<Cow<'s, str>> {
518+
let absolute_url = if ["./", "../"].iter().any(|p| submodule_url.starts_with(p)) {
519+
match Url::parse(base_url) {
520+
Ok(mut base_url) => {
521+
let path = base_url.path();
522+
if !path.ends_with('/') {
523+
base_url.set_path(&format!("{path}/"));
524+
}
525+
let absolute_url = base_url.join(submodule_url).with_context(|| {
526+
format!(
527+
"failed to parse relative child submodule url `{submodule_url}` \
528+
using parent base url `{base_url}`"
529+
)
530+
})?;
531+
Cow::from(absolute_url.to_string())
532+
}
533+
Err(_) => {
534+
let mut absolute_url = base_url.to_string();
535+
if !absolute_url.ends_with('/') {
536+
absolute_url.push('/');
537+
}
538+
absolute_url.push_str(submodule_url);
539+
Cow::from(absolute_url)
540+
}
541+
}
542+
} else {
543+
Cow::from(submodule_url)
544+
};
545+
546+
Ok(absolute_url)
547+
}
548+
510549
/// Prepare the authentication callbacks for cloning a git repository.
511550
///
512551
/// The main purpose of this function is to construct the "authentication
@@ -1486,3 +1525,102 @@ fn is_short_hash_of(rev: &str, oid: Oid) -> bool {
14861525
None => false,
14871526
}
14881527
}
1528+
1529+
#[cfg(test)]
1530+
mod tests {
1531+
use super::absolute_submodule_url;
1532+
1533+
#[test]
1534+
fn test_absolute_submodule_url() {
1535+
let cases = [
1536+
(
1537+
"ssh://[email protected]/rust-lang/cargo",
1538+
"[email protected]:rust-lang/cargo.git",
1539+
"[email protected]:rust-lang/cargo.git",
1540+
),
1541+
(
1542+
"ssh://[email protected]/rust-lang/cargo",
1543+
"./",
1544+
"ssh://[email protected]/rust-lang/cargo/",
1545+
),
1546+
(
1547+
"ssh://[email protected]/rust-lang/cargo",
1548+
"../",
1549+
"ssh://[email protected]/rust-lang/",
1550+
),
1551+
(
1552+
"ssh://[email protected]/rust-lang/cargo",
1553+
"./foo",
1554+
"ssh://[email protected]/rust-lang/cargo/foo",
1555+
),
1556+
(
1557+
"ssh://[email protected]/rust-lang/cargo/",
1558+
"./foo",
1559+
"ssh://[email protected]/rust-lang/cargo/foo",
1560+
),
1561+
(
1562+
"ssh://[email protected]/rust-lang/cargo/",
1563+
"../foo",
1564+
"ssh://[email protected]/rust-lang/foo",
1565+
),
1566+
(
1567+
"ssh://[email protected]/rust-lang/cargo",
1568+
"../foo",
1569+
"ssh://[email protected]/rust-lang/foo",
1570+
),
1571+
(
1572+
"ssh://[email protected]/rust-lang/cargo",
1573+
"../foo/bar/../baz",
1574+
"ssh://[email protected]/rust-lang/foo/baz",
1575+
),
1576+
(
1577+
"[email protected]:rust-lang/cargo.git",
1578+
"ssh://[email protected]/rust-lang/cargo",
1579+
"ssh://[email protected]/rust-lang/cargo",
1580+
),
1581+
(
1582+
"[email protected]:rust-lang/cargo.git",
1583+
"./",
1584+
"[email protected]:rust-lang/cargo.git/./",
1585+
),
1586+
(
1587+
"[email protected]:rust-lang/cargo.git",
1588+
"../",
1589+
"[email protected]:rust-lang/cargo.git/../",
1590+
),
1591+
(
1592+
"[email protected]:rust-lang/cargo.git",
1593+
"./foo",
1594+
"[email protected]:rust-lang/cargo.git/./foo",
1595+
),
1596+
(
1597+
"[email protected]:rust-lang/cargo.git/",
1598+
"./foo",
1599+
"[email protected]:rust-lang/cargo.git/./foo",
1600+
),
1601+
(
1602+
"[email protected]:rust-lang/cargo.git",
1603+
"../foo",
1604+
"[email protected]:rust-lang/cargo.git/../foo",
1605+
),
1606+
(
1607+
"[email protected]:rust-lang/cargo.git/",
1608+
"../foo",
1609+
"[email protected]:rust-lang/cargo.git/../foo",
1610+
),
1611+
(
1612+
"[email protected]:rust-lang/cargo.git",
1613+
"../foo/bar/../baz",
1614+
"[email protected]:rust-lang/cargo.git/../foo/bar/../baz",
1615+
),
1616+
];
1617+
1618+
for (base_url, submodule_url, expected) in cases {
1619+
let url = absolute_submodule_url(base_url, submodule_url).unwrap();
1620+
assert_eq!(
1621+
expected, url,
1622+
"base `{base_url}`; submodule `{submodule_url}`"
1623+
);
1624+
}
1625+
}
1626+
}

‎tests/testsuite/git.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3633,7 +3633,7 @@ fn different_user_relative_submodules() {
36333633
.file("Cargo.toml", &basic_lib_manifest("dep1"))
36343634
.file("src/lib.rs", "")
36353635
});
3636-
let _user2_git_project2 = git::new("user2/dep2", |project| {
3636+
let user2_git_project2 = git::new("user2/dep2", |project| {
36373637
project
36383638
.file("Cargo.toml", &basic_lib_manifest("dep1"))
36393639
.file("src/lib.rs", "")
@@ -3673,14 +3673,14 @@ fn different_user_relative_submodules() {
36733673
"\
36743674
[UPDATING] git repository `{}`
36753675
[UPDATING] git submodule `{}`
3676-
[UPDATING] git submodule `{}/../dep2`
3676+
[UPDATING] git submodule `{}`
36773677
[COMPILING] dep1 v0.5.0 ({}#[..])
36783678
[COMPILING] foo v0.5.0 ([CWD])
36793679
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
36803680
",
36813681
path2url(&user1_git_project.root()),
36823682
path2url(&user2_git_project.root()),
3683-
path2url(&user2_git_project.root()),
3683+
path2url(&user2_git_project2.root()),
36843684
path2url(&user1_git_project.root()),
36853685
))
36863686
.run();

0 commit comments

Comments
 (0)