Skip to content

Improve handling of "extra" files #1771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Sources/Commands/SwiftPackageTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -373,9 +373,13 @@ public class SwiftPackageTool: SwiftTool<PackageToolOptions> {
generateXcodeParser.add(
option: "--watch", kind: Bool.self,
usage: "Watch for changes to the Package manifest to regenerate the Xcode project"),
generateXcodeParser.add(
option: "--skip-extra-files", kind: Bool.self,
usage: "Do not add file references for extra files to the generated Xcode project"),
to: {
$0.xcodeprojOptions.useLegacySchemeGenerator = $1 ?? false
$0.xcodeprojOptions.enableAutogeneration = $2 ?? false
$0.xcodeprojOptions.addExtraFiles = !($3 ?? false)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may look weird this way, but I thought it would be cleaner to have the option be positive ("add") vs. negative ("skip").

})

let completionToolParser = parser.add(
Expand Down
28 changes: 20 additions & 8 deletions Sources/Xcodeproj/generate().swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ public struct XcodeprojOptions {
/// Run watchman to auto-generate the project file on changes.
public var enableAutogeneration: Bool

/// Whether to add extra files to the generated project.
public var addExtraFiles: Bool

/// Reference to manifest loader, if present.
public var manifestLoader: ManifestLoader?

Expand All @@ -42,13 +45,15 @@ public struct XcodeprojOptions {
xcconfigOverrides: AbsolutePath? = nil,
isCodeCoverageEnabled: Bool? = nil,
useLegacySchemeGenerator: Bool? = nil,
enableAutogeneration: Bool? = nil
enableAutogeneration: Bool? = nil,
addExtraFiles: Bool? = nil
) {
self.flags = flags
self.xcconfigOverrides = xcconfigOverrides
self.isCodeCoverageEnabled = isCodeCoverageEnabled ?? false
self.useLegacySchemeGenerator = useLegacySchemeGenerator ?? false
self.enableAutogeneration = enableAutogeneration ?? false
self.addExtraFiles = addExtraFiles ?? true
}
}

Expand Down Expand Up @@ -83,14 +88,20 @@ public func generate(
try makeDirectories(xcodeprojPath)
try makeDirectories(schemesDir)

// Find the paths of any extra directories that should be added as folder
// references in the project.
let extraDirs = try findDirectoryReferences(path: srcroot)

let extraDirs: [AbsolutePath]
var extraFiles = [AbsolutePath]()
if try repositoryProvider.checkoutExists(at: srcroot) {
let workingCheckout = try repositoryProvider.openCheckout(at: srcroot)
extraFiles = try getExtraFilesFor(package: graph.rootPackages[0], in: workingCheckout)

if options.addExtraFiles {
// Find the paths of any extra directories that should be added as folder
// references in the project.
extraDirs = try findDirectoryReferences(path: srcroot)

if try repositoryProvider.checkoutExists(at: srcroot) {
let workingCheckout = try repositoryProvider.openCheckout(at: srcroot)
extraFiles = try getExtraFilesFor(package: graph.rootPackages[0], in: workingCheckout)
}
} else {
extraDirs = []
}

/// Generate the contents of project.xcodeproj (inside the .xcodeproj).
Expand Down Expand Up @@ -265,6 +276,7 @@ func findNonSourceFiles(path: AbsolutePath, recursively: Bool = false) throws ->
return filesFromPath.filter({
if !isFile($0) { return false }
if $0.basename.hasPrefix(".") { return false }
if $0.basename == "Package.resolved" { return false }
if let `extension` = $0.extension, SupportedLanguageExtension.validExtensions.contains(`extension`) {
return false
}
Expand Down
22 changes: 11 additions & 11 deletions Sources/Xcodeproj/pbxproj().swift
Original file line number Diff line number Diff line change
Expand Up @@ -319,17 +319,6 @@ func xcodeProject(
// Create a `Tests` group for the source targets in the root package.
createSourceGroup(named: "Tests", for: testModules, in: project.mainGroup)

// Add "blue folders" for any other directories at the top level (note that
// they are not guaranteed to be direct children of the root directory).
for extraDir in extraDirs {
project.mainGroup.addFileReference(path: extraDir.relative(to: sourceRootDir).asString, pathBase: .projectDir)
}

for extraFile in extraFiles {
let groupOfFile = srcPathsToGroups[extraFile.parentDirectory]
groupOfFile?.addFileReference(path: extraFile.basename)
}

// Determine the set of targets to generate in the project by excluding
// any system targets.
let targets = graph.reachableTargets.filter({ $0.type != .systemModule })
Expand Down Expand Up @@ -372,6 +361,17 @@ func xcodeProject(
// the various targets; these references will be added to the link phases.
let productsGroup = project.mainGroup.addGroup(path: "", pathBase: .buildDir, name: "Products")

// Add "blue folders" for any other directories at the top level (note that
// they are not guaranteed to be direct children of the root directory).
for extraDir in extraDirs {
project.mainGroup.addFileReference(path: extraDir.relative(to: sourceRootDir).asString, pathBase: .projectDir)
}

for extraFile in extraFiles {
let groupOfFile = srcPathsToGroups[extraFile.parentDirectory]
groupOfFile?.addFileReference(path: extraFile.basename)
}

// Set the newly created `Products` group as the official products group of
// the project.
project.productGroup = productsGroup
Expand Down
3 changes: 3 additions & 0 deletions Tests/XcodeprojTests/GenerateXcodeprojTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ class GenerateXcodeprojTests: XCTestCase {
let project = try Xcodeproj.generate(projectName: projectName, xcodeprojPath: outpath, graph: graph, options: XcodeprojOptions(), diagnostics: diagnostics)

XCTAssertTrue(project.mainGroup.subitems.contains { $0.path == "a.txt" })

let projectWithoutExtraFiles = try Xcodeproj.generate(projectName: projectName, xcodeprojPath: outpath, graph: graph, options: XcodeprojOptions(addExtraFiles: false), diagnostics: diagnostics)
XCTAssertFalse(projectWithoutExtraFiles.mainGroup.subitems.contains { $0.path == "a.txt" })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should create an extra test case for this? Seems easier to share the setup code, though.

}
}

Expand Down