Skip to content

Commit 15e77b0

Browse files
authored
Do not use canonical location for checkouts (#7001)
In #6780, we switched to using the canonical location of a package for the checkouts paths, but that can lead to mixing up checkouts storage when there has been a legitimate change to locations that isn't reflected in the canonical location (most prominently switching from SSH to HTTPS or vice versa). Because of that, we should undo that change. rdar://116534183
1 parent d0ecef5 commit 15e77b0

File tree

4 files changed

+70
-35
lines changed

4 files changed

+70
-35
lines changed

Sources/PackageModel/PackageIdentity.swift

Lines changed: 52 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -419,50 +419,71 @@ public struct CanonicalPackageLocation: Equatable, CustomStringConvertible {
419419

420420
/// Instantiates an instance of the conforming type from a string representation.
421421
public init(_ string: String) {
422-
var description = string.precomposedStringWithCanonicalMapping.lowercased()
422+
self.description = computeCanonicalLocation(string).description
423+
}
424+
}
423425

424-
// Remove the scheme component, if present.
425-
let detectedScheme = description.dropSchemeComponentPrefixIfPresent()
426+
/// Similar to `CanonicalPackageLocation` but differentiates based on the scheme.
427+
public struct CanonicalPackageURL: CustomStringConvertible {
428+
public let description: String
429+
public let scheme: String?
426430

427-
// Remove the userinfo subcomponent (user / password), if present.
428-
if case (let user, _)? = description.dropUserinfoSubcomponentPrefixIfPresent() {
429-
// If a user was provided, perform tilde expansion, if applicable.
430-
description.replaceFirstOccurenceIfPresent(of: "/~/", with: "/~\(user)/")
431-
}
431+
public init(_ string: String) {
432+
let location = computeCanonicalLocation(string)
433+
self.description = location.description
434+
self.scheme = location.scheme
435+
}
436+
}
432437

433-
// Remove the port subcomponent, if present.
434-
description.removePortComponentIfPresent()
438+
private func computeCanonicalLocation(_ string: String) -> (description: String, scheme: String?) {
439+
var description = string.precomposedStringWithCanonicalMapping.lowercased()
435440

436-
// Remove the fragment component, if present.
437-
description.removeFragmentComponentIfPresent()
441+
// Remove the scheme component, if present.
442+
let detectedScheme = description.dropSchemeComponentPrefixIfPresent()
443+
var scheme = detectedScheme
438444

439-
// Remove the query component, if present.
440-
description.removeQueryComponentIfPresent()
445+
// Remove the userinfo subcomponent (user / password), if present.
446+
if case (let user, _)? = description.dropUserinfoSubcomponentPrefixIfPresent() {
447+
// If a user was provided, perform tilde expansion, if applicable.
448+
description.replaceFirstOccurenceIfPresent(of: "/~/", with: "/~\(user)/")
441449

442-
// Accomodate "`scp`-style" SSH URLs
443-
if detectedScheme == nil || detectedScheme == "ssh" {
444-
description.replaceFirstOccurenceIfPresent(of: ":", before: description.firstIndex(of: "/"), with: "/")
450+
if user == "git", scheme == nil {
451+
scheme = "ssh"
445452
}
453+
}
446454

447-
// Split the remaining string into path components,
448-
// filtering out empty path components and removing valid percent encodings.
449-
var components = description.split(omittingEmptySubsequences: true, whereSeparator: isSeparator)
450-
.compactMap { $0.removingPercentEncoding ?? String($0) }
455+
// Remove the port subcomponent, if present.
456+
description.removePortComponentIfPresent()
451457

452-
// Remove the `.git` suffix from the last path component.
453-
var lastPathComponent = components.popLast() ?? ""
454-
lastPathComponent.removeSuffixIfPresent(".git")
455-
components.append(lastPathComponent)
458+
// Remove the fragment component, if present.
459+
description.removeFragmentComponentIfPresent()
456460

457-
description = components.joined(separator: "/")
461+
// Remove the query component, if present.
462+
description.removeQueryComponentIfPresent()
458463

459-
// Prepend a leading slash for file URLs and paths
460-
if detectedScheme == "file" || string.first.flatMap(isSeparator) ?? false {
461-
description.insert("/", at: description.startIndex)
462-
}
464+
// Accomodate "`scp`-style" SSH URLs
465+
if detectedScheme == nil || detectedScheme == "ssh" {
466+
description.replaceFirstOccurenceIfPresent(of: ":", before: description.firstIndex(of: "/"), with: "/")
467+
}
468+
469+
// Split the remaining string into path components,
470+
// filtering out empty path components and removing valid percent encodings.
471+
var components = description.split(omittingEmptySubsequences: true, whereSeparator: isSeparator)
472+
.compactMap { $0.removingPercentEncoding ?? String($0) }
463473

464-
self.description = description
474+
// Remove the `.git` suffix from the last path component.
475+
var lastPathComponent = components.popLast() ?? ""
476+
lastPathComponent.removeSuffixIfPresent(".git")
477+
components.append(lastPathComponent)
478+
479+
description = components.joined(separator: "/")
480+
481+
// Prepend a leading slash for file URLs and paths
482+
if detectedScheme == "file" || string.first.flatMap(isSeparator) ?? false {
483+
description.insert("/", at: description.startIndex)
465484
}
485+
486+
return (description, scheme)
466487
}
467488

468489
#if os(Windows)

Sources/SourceControl/RepositoryManager.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -587,8 +587,9 @@ extension RepositorySpecifier {
587587
}
588588

589589
extension RepositorySpecifier {
590-
fileprivate var canonicalLocation: CanonicalPackageLocation {
591-
.init(self.location.description)
590+
fileprivate var canonicalLocation: String {
591+
let canonicalPackageLocation: CanonicalPackageURL = .init(self.location.description)
592+
return "\(canonicalPackageLocation.description)_\(canonicalPackageLocation.scheme ?? "")"
592593
}
593594
}
594595

Tests/PackageModelTests/CanonicalPackageLocationTests.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,4 +324,19 @@ final class CanonicalPackageLocationTests: XCTestCase {
324324
"example.com/mona/linked:list"
325325
)
326326
}
327+
328+
func testScheme() {
329+
XCTAssertEqual(CanonicalPackageURL("https://example.com/mona/LinkedList").scheme, "https")
330+
XCTAssertEqual(CanonicalPackageURL("[email protected]/mona/LinkedList").scheme, "ssh")
331+
XCTAssertEqual(CanonicalPackageURL("[email protected]:mona/LinkedList.git ").scheme, "ssh")
332+
XCTAssertEqual(CanonicalPackageURL("ssh://[email protected]/~/LinkedList.git").scheme, "ssh")
333+
XCTAssertEqual(CanonicalPackageURL("file:///Users/mona/LinkedList").scheme, "file")
334+
XCTAssertEqual(CanonicalPackageURL("example.com:443/mona/LinkedList").scheme, nil)
335+
XCTAssertEqual(CanonicalPackageURL("example.com/mona/%F0%9F%94%97List").scheme, nil)
336+
XCTAssertEqual(CanonicalPackageURL("example.com/mona/LinkedList.git").scheme, nil)
337+
XCTAssertEqual(CanonicalPackageURL("example.com/mona/LinkedList/").scheme, nil)
338+
XCTAssertEqual(CanonicalPackageURL("example.com/mona/LinkedList#installation").scheme, nil)
339+
XCTAssertEqual(CanonicalPackageURL("example.com/mona/LinkedList?utm_source=forums.swift.org").scheme, nil)
340+
XCTAssertEqual(CanonicalPackageURL("user:[email protected]:/mona/Linked:List.git").scheme, nil)
341+
}
327342
}

Tests/SourceControlTests/RepositoryManagerTests.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,6 @@ class RepositoryManagerTests: XCTestCase {
328328
let variants: [RepositorySpecifier] = [
329329
.init(url: "https://scm.com/org/foo"),
330330
.init(url: "https://scm.com/org/foo.git"),
331-
.init(url: "http://scm.com/org/foo"),
332-
.init(url: "http://scm.com/org/foo.git")
333331
]
334332

335333
for variant in variants {

0 commit comments

Comments
 (0)