Skip to content

Commit 974c559

Browse files
mxclmxcl
committed
Fix symlinked Sources directories
If a user should use a symlink to define the Sources directory we were with some layouts failing to detect and construct targets correctly. Fixes https://bugs.swift.org/browse/SR-29
1 parent 21c3e51 commit 974c559

File tree

13 files changed

+117
-35
lines changed

13 files changed

+117
-35
lines changed

‎Sources/dep/Git.swift

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -66,40 +66,29 @@ class Git {
6666
var branch: String! {
6767
return try? popen([Git.tool, "-C", root, "rev-parse", "--abbrev-ref", "HEAD"]).chomp()
6868
}
69+
70+
func fetch() throws {
71+
try systém(Git.tool, "-C", root, "fetch", "--tags", "origin")
72+
}
6973
}
7074

7175
class func clone(url: String, to dstdir: String) throws -> Repo {
72-
var out = ""
73-
7476
// canonicalize URL
7577
var url = url
7678
if URL.scheme(url) == nil {
7779
url = try realpath(url)
7880
}
7981

80-
let args = [Git.tool, "clone",
81-
"--recursive", // get submodules too so that developers can use these if they so choose
82-
"--depth", "10",
83-
url, dstdir]
84-
8582
do {
86-
if sys.verbosity == .Concise {
87-
print("Cloning", Path(dstdir).relative(to: "."))
88-
fflush(stdout) // should git ask for credentials ensure we displayed the above status message first
89-
try popen(args, redirectStandardError: true) { line in
90-
out += line
91-
}
92-
} else {
93-
try system(args)
94-
}
95-
96-
return Repo(root: dstdir)! //TODO no bangs
97-
83+
try systém(Git.tool, "clone",
84+
"--recursive", // get submodules too so that developers can use these if they so choose
85+
"--depth", "10",
86+
url, dstdir, preamble: "Cloning \(Path(dstdir).relative(to: "."))")
9887
} catch POSIX.Error.ExitStatus {
99-
print("$", prettyArguments(args), toStream: &stderr)
100-
print(out, toStream: &stderr)
10188
throw Error.GitCloneFailure(url, dstdir)
10289
}
90+
91+
return Repo(root: dstdir)! //TODO no bangs
10392
}
10493

10594
class var tool: String {
@@ -108,6 +97,29 @@ class Git {
10897
}
10998

11099

100+
private func systém(args: String..., preamble: String? = nil) throws {
101+
var out = ""
102+
do {
103+
if sys.verbosity == .Concise {
104+
if let preamble = preamble {
105+
print(preamble)
106+
fflush(stdout) // should git ask for credentials ensure we displayed the above status message first
107+
}
108+
try popen(args, redirectStandardError: true) { line in
109+
out += line
110+
}
111+
} else {
112+
try system(args)
113+
}
114+
115+
} catch POSIX.Error.ExitStatus(let foo) {
116+
print("$", prettyArguments(args), toStream: &stderr)
117+
print(out, toStream: &stderr)
118+
throw POSIX.Error.ExitStatus(foo)
119+
}
120+
}
121+
122+
111123

112124
extension Version {
113125
private static func vprefix(string: String.CharacterView) -> Version? {

‎Sources/dep/get.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,13 @@ extension Sandbox: Fetcher {
101101
let dstdir = Path.join(prefix, Package.name(forURL: url))
102102
if let repo = Git.Repo(root: dstdir) where repo.origin == url {
103103
//TODO need to canolicanize the URL need URL struct
104-
return try RawClone(path: dstdir)
104+
return RawClone(path: dstdir)
105105
}
106-
try Git.clone(url, to: dstdir)
107106

108107
// fetch as well, clone does not fetch all tags, only tags on the master branch
109-
try system("git", "-C", dstdir, "fetch", "--tags", "origin")
108+
try Git.clone(url, to: dstdir).fetch()
110109

111-
return try RawClone(path: dstdir)
110+
return RawClone(path: dstdir)
112111
}
113112

114113
func finalize(fetchable: Fetchable) throws -> Package {

‎Sources/sys/Path.swift

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,21 +196,25 @@ extension String {
196196

197197
/**
198198
- Returns: true if the string is a directory on the filesystem
199-
- Note: symlinks are NOT resolved
199+
- Note: if the entry is a symlink, but the symlink points to a
200+
directory, then this function returns true. Use `isSymlink`
201+
if the distinction is important.
200202
*/
201203
public var isDirectory: Bool {
202204
var mystat = stat()
203-
let rv = lstat(self, &mystat)
205+
let rv = stat(self, &mystat)
204206
return rv == 0 && (mystat.st_mode & S_IFMT) == S_IFDIR
205207
}
206208

207209
/**
208210
- Returns: true if the string is a file on the filesystem
209-
- Note: symlinks are NOT resolved
211+
- Note: if the entry is a symlink, but the symlink points to Array
212+
file, then this function returns true. Use `isSymlink` if the
213+
distinction is important.
210214
*/
211215
public var isFile: Bool {
212216
var mystat = stat()
213-
let rv = lstat(self, &mystat)
217+
let rv = stat(self, &mystat)
214218
return rv == 0 && (mystat.st_mode & S_IFMT) == S_IFREG
215219
}
216220

‎Sources/sys/misc.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public func rmtree(components: String...) throws {
2222
let path = Path.join(components)
2323
var dirs = [String]()
2424
for entry in walk(path, recursively: true) {
25-
if entry.isDirectory {
25+
if entry.isDirectory && !entry.isSymlink {
2626
dirs.append(entry)
2727
} else {
2828
try POSIX.unlink(entry)

‎Tests/dep/Fixtures/26_symlinked_sources_directory/Foo/Foo.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
class Foo {
2+
var bar: Int = 0
3+
}

‎Tests/dep/Fixtures/26_symlinked_sources_directory/Package.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import PackageDescription
2+
3+
let package = Package(name: "Foo")

‎Tests/dep/Fixtures/26_symlinked_sources_directory/Sources

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
/Users/mxcl/src/swiftpm/Tests/dep/Fixtures/26_symlinked_sources_directory/Foo

‎Tests/dep/Fixtures/27_symlinked_nested_sources_directory/Foo/Bar/Foo.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
class Foo {
2+
var bar: Int = 0
3+
}

‎Tests/dep/Fixtures/27_symlinked_nested_sources_directory/Package.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import PackageDescription
2+
3+
let package = Package(name: "foo")

‎Tests/dep/Fixtures/27_symlinked_nested_sources_directory/Sources

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
/Users/mxcl/src/swiftpm/Tests/dep/Fixtures/27_symlinked_nested_sources_directory/Foo

‎Tests/dep/FunctionalBuildTests.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ class FunctionalBuildTests: XCTestCase, XCTestCaseProvider {
5656
("testSingleTargetWithCustomName", testSingleTargetWithCustomName),
5757
("testCanBuildIfADependencyAlreadyCheckedOut", testCanBuildIfADependencyAlreadyCheckedOut),
5858
("testCanBuildIfADependencyClonedButThenAborted", testCanBuildIfADependencyClonedButThenAborted),
59+
("testFailsIfVersionTagHasNoPackageSwift", testFailsIfVersionTagHasNoPackageSwift),
60+
("testTipHasNoPackageSwift", testTipHasNoPackageSwift),
5961
]
6062
}
6163

@@ -466,4 +468,18 @@ class FunctionalBuildTests: XCTestCase, XCTestCaseProvider {
466468
XCTAssertNil(try? executeSwiftBuild("\(prefix)/app"))
467469
}
468470
}
471+
472+
func testSymlinkedSourceDirectoryWorks() {
473+
fixture(name: "26_symlinked_sources_directory") { prefix in
474+
XCTAssertNotNil(try? executeSwiftBuild(prefix))
475+
XCTAssertTrue(Path.join(prefix, ".build/debug/Foo.a").isFile)
476+
}
477+
}
478+
479+
func testSymlinkedNestedSourceDirectoryWorks() {
480+
fixture(name: "27_symlinked_nested_sources_directory") { prefix in
481+
XCTAssertNotNil(try? executeSwiftBuild(prefix))
482+
XCTAssertTrue(Path.join(prefix, ".build/debug/Bar.a").isFile)
483+
}
484+
}
469485
}

‎Tests/sys/MiscTests.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ class RmtreeTests: XCTestCase, XCTestCaseProvider {
4444
XCTAssertTrue("\(root)/bar/baz/goo".isDirectory)
4545
}
4646
} catch {
47-
XCTFail("\(error)")
47+
print(error)
48+
XCTFail()
4849
}
4950
}
5051
}

‎Tests/sys/PathTests.swift

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class PathTests: XCTestCase, XCTestCaseProvider {
2222
("testEmpties", testEmpties),
2323
("testNormalizePath", testNormalizePath),
2424
("testJoinWithAbsoluteReturnsLastAbsoluteComponent", testJoinWithAbsoluteReturnsLastAbsoluteComponent),
25-
("testParentDirectory", testParentDirectory)
25+
("testParentDirectory", testParentDirectory),
2626
]
2727
}
2828

@@ -97,6 +97,7 @@ class WalkTests: XCTestCase {
9797
("testNonRecursive", testNonRecursive),
9898
("testRecursive", testRecursive),
9999
("testSymlinksNotWalked", testSymlinksNotWalked),
100+
("testWalkingADirectorySymlinkResolvesOnce", testWalkingADirectorySymlinkResolvesOnce),
100101
]
101102
}
102103

@@ -132,11 +133,9 @@ class WalkTests: XCTestCase {
132133
func testSymlinksNotWalked() {
133134
do {
134135
try mkdtemp("foo") { root in
135-
let root = try realpath(root)
136+
let root = try realpath(root) //FIXME not good that we need this?
136137

137138
try mkdir(root, "foo")
138-
try mkdir(root, "bar")
139-
try mkdir(root, "bar/baz")
140139
try mkdir(root, "bar/baz/goo")
141140
try symlink(create: "\(root)/foo/symlink", pointingAt: "\(root)/bar", relativeTo: root)
142141

@@ -154,6 +153,26 @@ class WalkTests: XCTestCase {
154153
}
155154
}
156155

156+
func testWalkingADirectorySymlinkResolvesOnce() {
157+
try! mkdtemp("foo") { root in
158+
let root = try realpath(root) //FIXME not good that we need this?
159+
160+
try mkdir(root, "foo/bar")
161+
try mkdir(root, "abc/bar")
162+
try symlink(create: "\(root)/symlink", pointingAt: "\(root)/foo", relativeTo: root)
163+
try symlink(create: "\(root)/foo/baz", pointingAt: "\(root)/abc", relativeTo: root)
164+
165+
XCTAssertTrue(Path.join(root, "symlink").isSymlink)
166+
167+
let results = walk(root, "symlink").map{$0}
168+
169+
// we recurse a symlink to a directory, so this should work,
170+
// but `abc` should not show because `baz` is a symlink too
171+
// and that should *not* be followed
172+
173+
XCTAssertEqual(results, ["\(root)/symlink/bar", "\(root)/symlink/baz"])
174+
}
175+
}
157176
}
158177

159178
class StatTests: XCTestCase {
@@ -170,6 +189,23 @@ class StatTests: XCTestCase {
170189
func test_isdir() {
171190
XCTAssertTrue("/usr".isDirectory)
172191
XCTAssertTrue("/etc/passwd".isFile)
192+
193+
try! mkdtemp("foo") { root in
194+
try mkdir(root, "foo/bar")
195+
try symlink(create: "\(root)/symlink", pointingAt: "\(root)/foo", relativeTo: root)
196+
197+
XCTAssertTrue("\(root)/foo/bar".isDirectory)
198+
XCTAssertTrue("\(root)/symlink/bar".isDirectory)
199+
XCTAssertTrue("\(root)/symlink".isDirectory)
200+
XCTAssertTrue("\(root)/symlink".isSymlink)
201+
202+
try POSIX.rmdir("\(root)/foo/bar")
203+
try POSIX.rmdir("\(root)/foo")
204+
205+
XCTAssertTrue("\(root)/symlink".isSymlink)
206+
XCTAssertFalse("\(root)/symlink".isDirectory)
207+
XCTAssertFalse("\(root)/symlink".isFile)
208+
}
173209
}
174210

175211
func test_isfile() {

0 commit comments

Comments
 (0)