From feee0efca0f669c21478d4429ce8ffb4ffa571ef Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Mon, 25 Sep 2023 12:59:14 -0700 Subject: [PATCH 1/2] [swift-syntax-dev-utils] Throw an error in if an executable could not be found --- .../commands/VerifySourceCode.swift | 6 +- .../common/BuildCommand.swift | 10 +--- .../swift-syntax-dev-utils/common/Paths.swift | 57 ++++++++++++++++--- 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/commands/VerifySourceCode.swift b/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/commands/VerifySourceCode.swift index 03a1a465c13..eed941d8ba1 100644 --- a/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/commands/VerifySourceCode.swift +++ b/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/commands/VerifySourceCode.swift @@ -36,16 +36,12 @@ struct VerifySourceCode: ParsableCommand, SourceCodeGeneratorCommand { logSection("Verifing code generated files") - guard let diffExec = Paths.diffExec else { - throw ScriptExectutionError(message: "Didn't find a diff execution path") - } - for module in modules { let selfGeneratedDir = tempDir.appendingPathComponent(module).appendingPathComponent("generated") let userGeneratedDir = Paths.sourcesDir.appendingPathComponent(module).appendingPathComponent("generated") let process = ProcessRunner( - executableURL: diffExec, + executableURL: try Paths.diffExec, arguments: [ "--recursive", "--exclude", diff --git a/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/common/BuildCommand.swift b/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/common/BuildCommand.swift index 6a1d4f383f9..5376d194c01 100644 --- a/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/common/BuildCommand.swift +++ b/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/common/BuildCommand.swift @@ -86,16 +86,8 @@ extension BuildCommand { @discardableResult func invokeXcodeBuild(projectPath: URL, scheme: String) throws -> ProcessResult { return try withTemporaryDirectory { tempDir in - guard let xcodebuildExec = Paths.xcodebuildExec else { - throw ScriptExectutionError( - message: """ - Error: Could not find xcodebuild. - Looking at '\(Paths.xcodebuildExec?.path ?? "N/A")'. - """ - ) - } let processRunner = ProcessRunner( - executableURL: xcodebuildExec, + executableURL: try Paths.xcodebuildExec, arguments: [ "-project", projectPath.path, "-scheme", scheme, diff --git a/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/common/Paths.swift b/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/common/Paths.swift index 48a42e5e42f..57a77239e28 100644 --- a/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/common/Paths.swift +++ b/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/common/Paths.swift @@ -61,16 +61,40 @@ enum Paths { .appendingPathComponent("lit.py") } - static var python3Exec: URL? { - return lookupExecutable(for: "python3") + static var python3Exec: URL { + get throws { + return try lookupExecutable(for: "python3") + } } - static var diffExec: URL? { - return lookupExecutable(for: "diff") + static var diffExec: URL { + get throws { + return try lookupExecutable(for: "diff") + } } - static var xcodebuildExec: URL? { - return lookupExecutable(for: "xcodebuild") + static var gitExec: URL { + get throws { + try lookupExecutable(for: "git") + } + } + + static var swiftExec: URL { + get throws { + try lookupExecutable(for: "swift") + } + } + + /// The directory in which swift-format should be built. + static var swiftFormatBuildDir: URL { + packageDir + .appendingPathComponent(".swift-format-build") + } + + static var xcodebuildExec: URL { + get throws { + return try lookupExecutable(for: "xcodebuild") + } } private static var envSearchPaths: [URL] { @@ -98,13 +122,28 @@ enum Paths { return ProcessInfo.processInfo.environment[pathArg] } - private static func lookupExecutable(for filename: String) -> URL? { - return envSearchPaths.map { $0.appendingPathComponent(filename) } + enum ExecutableLookupError: Error, CustomStringConvertible { + case notFound(executableName: String) + + var description: String { + switch self { + case .notFound(executableName: let executableName): + return "Executable \(executableName) not found in PATH" + } + } + } + + private static func lookupExecutable(for filename: String) throws -> URL { + let executable = envSearchPaths.map { $0.appendingPathComponent(filename) } .first(where: { $0.isExecutableFile }) + guard let executable else { + throw ExecutableLookupError.notFound(executableName: filename) + } + return executable } } -fileprivate extension URL { +extension URL { var isExecutableFile: Bool { return (self.isFile(path) || self.isSymlink(path)) && FileManager.default.isExecutableFile(atPath: path) } From b80422c194dc7140d599b8ff0964f1dc4d8476ac Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Mon, 25 Sep 2023 13:46:12 -0700 Subject: [PATCH 2/2] Re-architect format.py in swift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make a couple of changes to `format.py` and use the opportunity to re-write it in Swift. 1. Always build a local copy of swift-format in `swift-syntax/.swift-format-build` in release mode. This eliminates issues that occur if you had `swift-format` installed on your system and `format.py` would pick it up 2. Since the local swift-format build is persistent (doesn’t live in `/tmp`), we can build it in release. This increases the first run but decreases the time for any subsequent runs from ~15s to ~1s, which should pay usually pay off for frequent contributors because of faster turnaround times. 3. Add a little tip to add `swift-format` as a git hook to Contributing.md And as an added bonus, the rewrite reduced the runtime of `format.(py|swift)` from 1.2s to 900ms. --- .gitignore | 3 + CONTRIBUTING.md | 21 +- .../SwiftSyntaxDevUtils.swift | 1 + .../commands/Format.swift | 256 ++++++++++++++++++ format.py | 137 ---------- 5 files changed, 278 insertions(+), 140 deletions(-) create mode 100644 SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/commands/Format.swift delete mode 100755 format.py diff --git a/.gitignore b/.gitignore index 3a0571a4597..93be96f2258 100644 --- a/.gitignore +++ b/.gitignore @@ -26,3 +26,6 @@ Package.resolved *.pyc Tests/PerformanceTest/baselines.json + +# The local build of swift-format to format swift-syntax +.swift-format-build diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e30f708fed6..fb91ef5b86b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,12 +21,27 @@ swift-syntax is a SwiftPM package, so you can build and test it using anything t swift-syntax is formatted using [swift-format](http://github.com/apple/swift-format) to ensure a consistent style. -To format your changes run `format.py` at the root of this repository. If you have a `swift-format` executable ready, you can pass it to `format.py`. If you do not, `format.py` will build its own copy of `swift-format` in `/tmp/swift-format`. - -If you are seeing surprising formatting results, you most likely have a `swift-format` installed on your system that’s not the most recent version built from the `main` branch. To fix this, clone [swift-format](http://github.com/apple/swift-format), build it using `swift build` and pass the freshly built executable to `format.py` as `--swift-format path/to/swift-format/.build/debug/swift-format`. Alternatively, you can uninstall `swift-format` on your system and `format.py` will build it from scratch. +To format your changes run the formatter using the following command +```bash +swift run --package-path SwiftSyntaxDevUtils/ swift-syntax-dev-utils format +``` +It will build a local copy of swift-format from the `main` branch and format the repository. Since it is building a release version of `swift-format`, the first run will take few minutes. Subsequent runs take less than 2 seconds. Generated source code is not formatted to make it easier to spot changes when re-running code generation. +> [!NOTE] +> You can add a git hook to ensure all commits to the swift-syntax repository are correctly formatted. +> 1. Save the following contents to `swift-syntax/.git/hooks/pre-commit` +> ```bash +> #!/usr/bin/env bash +> set -e +> SOURCE_DIR=$(realpath "$(dirname $0)/../..") +> swift run --package-path "$SOURCE_DIR/SwiftSyntaxDevUtils" swift-syntax-dev-utils format --lint +> ``` +> 2. Mark the file as executable: `chmod a+x swift-syntax/.git/hooks/pre-commit` +> 3. If you have global git hooks installed, be sure to call them at the end of the script with `path/to/global/hooks/pre-commit "$@"` + + ## Generating Source Code If you want to modify the generated files, open the [CodeGeneration](CodeGeneration) package and run the `generate-swift-syntax` executable. diff --git a/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/SwiftSyntaxDevUtils.swift b/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/SwiftSyntaxDevUtils.swift index c1988f67cde..bb53834da16 100644 --- a/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/SwiftSyntaxDevUtils.swift +++ b/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/SwiftSyntaxDevUtils.swift @@ -26,6 +26,7 @@ struct SwiftSyntaxDevUtils: ParsableCommand { """, subcommands: [ Build.self, + Format.self, GenerateSourceCode.self, Test.self, VerifySourceCode.self, diff --git a/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/commands/Format.swift b/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/commands/Format.swift new file mode 100644 index 00000000000..660f125fcb3 --- /dev/null +++ b/SwiftSyntaxDevUtils/Sources/swift-syntax-dev-utils/commands/Format.swift @@ -0,0 +1,256 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import ArgumentParser +import Foundation + +/// Directories that should not be formatted. +fileprivate let directoriesToExclude = [ + "lit_tests", + "generated", + "build", + "Inputs", + ".build", + ".swift-format-build", +] + +struct Format: ParsableCommand { + static var configuration: CommandConfiguration { + return CommandConfiguration( + abstract: "Format files in SwiftSyntax using swift-format.", + discussion: """ + This command automatically builds the '\(Self.swiftFormatBranch)' branch \ + of swift-format in the '\(Paths.swiftFormatBuildDir.lastPathComponent)' \ + directory of this repository and uses the build to format the swift-syntax \ + sources. + """ + ) + } + + @Flag(help: "Update the sources of swift-format and rebuild swift-format") + var update: Bool = false + + @Flag(help: "Instead of formatting in-place, verify that the files are correctly formatted. Exit with 1 if files are not correctly formatted.") + var lint: Bool = false + + @Option( + help: """ + Instead of building a local swift-format, use this swift-format executable. \ + Should primarily be used for CI, which has already built swift-format. + """ + ) + var swiftFormat: String? = nil + + @Flag(help: "Enable verbose logging.") + var verbose: Bool = false + + /// The configuration to build swift-format in. + private static let swiftFormatBuildConfiguration: String = "release" + + /// The branch of swift-format to build. + private static let swiftFormatBranch: String = "main" + + private enum Error: Swift.Error, CustomStringConvertible { + case swiftFormatNotFound + case lintFailed + + var description: String { + switch self { + case .swiftFormatNotFound: + return "The locally built swift-format could not be found" + case .lintFailed: + return """ + The swift-syntax repo is not formatted according to the style guides. + + Run the following command to format swift-syntax + + swift run --package-path SwiftSyntaxDevUtils/ swift-syntax-dev-utils format + + If the issue persists, try updating swift-format by running + + swift run --package-path SwiftSyntaxDevUtils/ swift-syntax-dev-utils format --update + + Should that still fail, fix any remaining issues manually and verify they match the swift-format style using + + swift run --package-path SwiftSyntaxDevUtils/ swift-syntax-dev-utils format --lint + """ + } + } + } + + /// Run `git` in the .swift-format-build directory with the provided arguments. + private func runGitCommand(_ arguments: String...) throws { + try ProcessRunner( + executableURL: Paths.gitExec, + arguments: ["-C", Paths.swiftFormatBuildDir.path] + arguments + ).run( + captureStdout: false, + captureStderr: false, + verbose: verbose + ) + } + + /// Run `swift` for the `.swift-format-build` package with the provided arguments. + private func runSwiftCommand(_ action: String, _ arguments: String...) throws { + try ProcessRunner( + executableURL: Paths.swiftExec, + arguments: [action, "--package-path", Paths.swiftFormatBuildDir.path] + arguments + ).run( + captureStdout: false, + captureStderr: false, + verbose: verbose + ) + } + + /// Ensure that we have an up-to-date checkout of swift-format in `.swift-format-build`. + private func cloneOrUpdateSwiftFormat() throws { + if FileManager.default.fileExists(atPath: Paths.swiftFormatBuildDir.appendingPathComponent(".git").path) { + try runGitCommand("checkout", Self.swiftFormatBranch) + try runGitCommand("pull") + } else { + try FileManager.default.createDirectory(atPath: Paths.swiftFormatBuildDir.path, withIntermediateDirectories: true) + try runGitCommand("clone", "https://github.com/apple/swift-format.git", ".") + try runGitCommand("checkout", Self.swiftFormatBranch) + } + try runSwiftCommand("package", "update") + } + + /// Build the swift-format executable. + private func buildSwiftFormat() throws { + try runSwiftCommand("build", "--product", "swift-format", "--configuration", Self.swiftFormatBuildConfiguration) + } + + /// Get the URL of the locally-built swift-format executable. + private func findSwiftFormatExecutable() throws -> URL { + if let swiftFormat = swiftFormat { + return URL(fileURLWithPath: swiftFormat) + } + + // We could run `swift build --show-bin-path` here but that takes 0.4s. + // Since the path seems really stable, let’s build the path ourselves. + let swiftFormatExec = URL(fileURLWithPath: Paths.swiftFormatBuildDir.path) + .appendingPathComponent(".build") + .appendingPathComponent(Self.swiftFormatBuildConfiguration) + .appendingPathComponent("swift-format") + if !swiftFormatExec.isExecutableFile { + throw Error.swiftFormatNotFound + } + return swiftFormatExec + } + + /// Get the list of files that should be formatted using swift-format. + /// + /// This excludes some files like generated files or test inputs. + private func filesToFormat() -> [URL] { + guard let enumerator = FileManager.default.enumerator(at: Paths.packageDir.resolvingSymlinksInPath(), includingPropertiesForKeys: [], options: []) else { + return [] + } + + var result: [URL] = [] + for case let url as URL in enumerator { + if directoriesToExclude.contains(url.lastPathComponent) { + enumerator.skipDescendants() + } + if url.pathExtension == "swift" { + result.append(url) + } + } + + return result + } + + /// Format all files in the repo using the locally-built swift-format. + private func formatFilesInRepo() throws { + let swiftFormatExecutable = try findSwiftFormatExecutable() + + let filesToFormat = self.filesToFormat() + + try ProcessRunner( + executableURL: swiftFormatExecutable, + arguments: [ + "format", + "--in-place", + "--parallel", + ] + filesToFormat.map { $0.path } + ) + .run( + captureStdout: false, + captureStderr: false, + verbose: verbose + ) + } + + /// Lint all files in the repo using the locally-built swift-format. + private func lintFilesInRepo() throws { + let swiftFormatExecutable = try findSwiftFormatExecutable() + + let filesToFormat = self.filesToFormat() + + do { + try ProcessRunner( + executableURL: swiftFormatExecutable, + arguments: [ + "lint", + "--strict", + "--parallel", + ] + filesToFormat.map { $0.path } + ) + .run( + captureStdout: false, + captureStderr: false, + verbose: verbose + ) + } catch is NonZeroExitCodeError { + throw Error.lintFailed + } + } + + func run() throws { + #if compiler(<5.10) + print("💡 You are building running the format script with Swift 5.9 or lower. Running it with SwiftPM 5.10 is about 10s faster.") + #endif + + try run(updateAndBuild: self.update) + } + + /// - Parameter updateAndBuild: Whether to update the locally checked out + /// swift-format sources and rebuild swift-format. + func run(updateAndBuild: Bool) throws { + if updateAndBuild { + try cloneOrUpdateSwiftFormat() + try buildSwiftFormat() + } + do { + if lint { + try lintFilesInRepo() + } else { + try formatFilesInRepo() + } + } catch Error.swiftFormatNotFound { + if !updateAndBuild { + print( + """ + No build of swift-format was found in '\(Paths.swiftFormatBuildDir.lastPathComponent)'. + Building swift-format now. This may take a couple of minutes. + Future invocations of this command will re-use the build and are much faster. + + """ + ) + + // If swift-format cannot be found, try again, updating (aka cloning + building) swift-format this time + try run(updateAndBuild: true) + } else { + throw Error.swiftFormatNotFound + } + } + } +} diff --git a/format.py b/format.py deleted file mode 100755 index 00fad55d14f..00000000000 --- a/format.py +++ /dev/null @@ -1,137 +0,0 @@ -#!/usr/bin/env python3 - -import argparse -import subprocess -from pathlib import Path -from textwrap import dedent -from typing import List - - -def parse_args() -> argparse.Namespace: - parser = argparse.ArgumentParser(description=''' - Format files in SwiftSyntax using swift-format. If swift-format is installed in your - PATH or passed using --swift-format that executable is used to format the files. - Otherwise swift-format is cloned and built by this script into /tmp - ''') - parser.add_argument( - '--swift-format', default='swift-format', - help=''' - The path to the swift-format executable. - Looks for swift-format in PATH by default. - ''') - parser.add_argument( - '--lint', action='store_true', - help=''' - Instead of formatting in-place verify that the files are correctly formatted. - Exit with 1 if files are not correctly formatted. - ''' - ) - - return parser.parse_args() - - -def clone_and_build_swiftformat() -> Path: - """ - Clone the swift-format repository into /tmp, build it and return the path to the - swift-format executable. This allows format.py to be fully standalone and run even - if swift-format is not installed on your system yet. - """ - print(dedent('''\ - swift-format was not found on your system. Cloning and building swift-format in /tmp/swift-format. - - To skip this step in the future build swift-format (https://github.com/apple/swift-format) yourself and - - put it in your PATH or - - pass the path to it to this script via --swift-format - ''')) # noqa: E501 - try: - swift_format_dir = Path('/tmp/swift-format') - if swift_format_dir.exists(): - subprocess.check_call(['git', 'pull'], cwd=swift_format_dir) - else: - subprocess.check_call([ - 'git', 'clone', - 'https://github.com/apple/swift-format.git' - ], cwd=swift_format_dir.parent) - subprocess.check_call([ - 'swift', 'build', - '--product', 'swift-format', '-c', 'release' - ], cwd=swift_format_dir) - bin_dir = subprocess.check_output([ - 'swift', 'build', '-c', 'release', - '--show-bin-path' - ], encoding='utf-8', cwd=swift_format_dir) - return Path(bin_dir.rstrip()) / 'swift-format' - except subprocess.CalledProcessError: - print(dedent(''' - format.py failed to build swift-format. - - Please build it yourself and pass the swift-format executable to format.py - using the --swift-format parameter. - ''')) - raise SystemExit(1) - - -def find_swiftformat(swift_format: str) -> Path: - """ - Return a fully resolved path to the swift-format executable named or located at - `swift_format`. - If swift-format couldn't be found, this clones and builds swift-format. - """ - try: - output = subprocess.check_output(['which', swift_format], encoding='utf-8') - return Path(output.rstrip()) - except subprocess.CalledProcessError: - return clone_and_build_swiftformat() - - -def get_files_to_format() -> List[Path]: - package_dir = Path(__file__).parent - files_to_format = package_dir.glob('**/*.swift') - - def should_exclude(path: Path) -> bool: - if 'lit_tests' in path.parts: - return True - elif 'generated' in path.parts: - return True - elif '/build' in path.parts: - return True - elif 'Inputs' in path.parts: - return True - elif '.build' in path.parts: - return True - return False - - files_to_format = [file for file in files_to_format if not should_exclude(file)] - - return files_to_format - - -def main() -> None: - args = parse_args() - swift_format = find_swiftformat(args.swift_format) - files_to_format = get_files_to_format() - - if args.lint: - try: - subprocess.check_call([swift_format, 'lint', '--parallel', '--strict'] + - files_to_format) - except subprocess.CalledProcessError: - print(dedent('''\ - The swift-syntax repo is not formatted according to the style guides. - Run the following command to format swift-syntax - - swift-syntax/format.py --swift-format /path/to/executable/swift-format - - If the issue persists, fix any remaining issues manually and verify they - match the swift-format style using - - swift-syntax/format.py --swift-format /path/to/executable/swift-format --lint - ''')) # noqa: E501 - raise SystemExit(1) - else: - subprocess.check_call([swift_format, 'format', '--in-place', '--parallel'] + - files_to_format) - - -if __name__ == '__main__': - main()