Abort commit if files are modified

[nicholascross]

It was mentioned here that it might be worth while considering if aborting a commit on file modification during a pre-commit hook could be incorporated directly into Komondor.

I had a look into the code to think about how this might be integrated and at once realised there was many different ways to tackle the problem.

• Should this applicable to all or specific hook types?
• Presumably this should be opt in, if so how is it configured?
  • There could be another hookOptions type dictionary, this could allow for additional options to be specified
  • we could check config for additional hook configuration config["\(hook)-option"]

The following demonstrates a possible way to add staged file modification for pre-commit hook only with out any opt in/out.

import Foundation
import PackageConfig
import ShellOut

// To emulate running the command as the script woudl do:
//
// swift run komondor run [hook-name]
//
//
public func runner(logger _: Logger, args: [String]) throws {
    let pkgConfig = getPackageConfig()

    guard let hook = args.first else {
        logger.logError("[Komondor] The runner was called without a hook")
        exit(1)
    }

    guard let config = pkgConfig["komondor"] else {
        logger.logError("[Komondor] Could not find komondor settings inside the Package.swift")
        exit(1)
    }

    guard let hookOptions = config[hook] else {
        logger.logError("[Komondor] Could not find a key for '\(hook)' under the komondor settings'")
        exit(1)
    }

    var commands = [] as [String]
    if let stringOption = hookOptions as? String {
        commands = [stringOption]
    } else if let arrayOptions = hookOptions as? [String] {
        commands = arrayOptions
    }

    logger.debug("Running commands for komondor \(commands.joined())")

    do {
        let hash = try stagedChangesHash()

        try commands.forEach { command in
            print("> \(command)")
            // Simple is fine for now
            print(try shellOut(to: command))
            // Ideal:
            //   Store STDOUT and STDERR, and only show it if it fails
            //   Show a stepper like system of all commands
        }

        if hook == "pre-commit", try stagedChangesHash() != hash {
            print("Staged files modified during \(hook) hook")
            exit(1)
        }
    } catch let error as ShellOutError {
        print(error.message)
        print(error.output)
        exit(error.terminationStatus)
    } catch {
        print(error)
        exit(1)
    }
}

private func stagedChangesHash() throws -> String {
    return try shellOut(to: "git diff --cached --name-only | xargs git diff | md5")
}

I played around a bit with the different options mentioned above but I always felt a little dirty like I was adding something too specific to what is currently a very generic.

Do you have any thoughts on how this might be implemented?

[orta]

Perhaps starting at the DSL we want and then extrapolating back might be useful here?

#if canImport(PackageConfig)
    import PackageConfig

    let config = PackageConfig([
        "komondor": [
            "pre-push": "swift test",
            "pre-commit": [
                .expectUnchanged([
                   "swift run swiftformat .",
                   "swift run swiftlint autocorrect --path Injectable/",
                 ]),
                "git add .",
            ],
        ],
    ])
#endif

This is all Swift - so I think this could work? A string could always represent a cmd to execute but enums could wrap with a context (like chdir )

Then it's a matter of looping through, seeing if it's a string or a special case?

[nicholascross]

Much more extensible and flexible 👍 Seems like the way to go!

[nicholascross]

Okay hit the first roadblock investigating this path.

It seems that you cannot reference the package code from the Package.swift

Which I guess makes sense as the Package.swift defines that package so it is a bit weird to reference it internally.

You can try this to reproduce

// swift-tools-version:4.2

import PackageDescription

let package = Package(
    name: "Komondor",
    products: [
        .executable(name: "komondor", targets: ["Komondor"]),
    ],
    dependencies: [
        // User deps
        .package(url: "https://github.com/orta/PackageConfig.git", from: "0.0.1"),
        .package(url: "https://github.com/JohnSundell/ShellOut.git", from: "2.1.0"),
        // Dev deps
        .package(url: "https://github.com/nicklockwood/SwiftFormat.git", from: "0.35.8"), // dev
        .package(url: "https://github.com/Realm/SwiftLint.git", from: "0.28.1"), // dev
        .package(url: "https://github.com/f-meloni/Rocket", from: "0.1.0"), // dev
    ],
    targets: [
        .target(
            name: "Komondor",
            dependencies: ["PackageConfig", "ShellOut"]
        ),
        .testTarget(
            name: "KomondorTests",
            dependencies: ["Komondor"]
        ),
    ]
)

#if canImport(PackageConfig) && canImport(Komondor)
    import PackageConfig
    import Komondor

    //This is a public function in Komondor
    renderScriptHeader("test")

    let config = PackageConfig([
        "komondor": [
            "pre-push": "swift test",
            "pre-commit": [
                "swift test",
                "swift run swiftFormat .",
                "swift run swiftlint autocorrect --path Sources/",
                "git add .",
            ],
        ],
        "rocket": [
            "after": [
                "push",
            ],
        ],
    ])
#endif

This will produce the following error.

<unknown>:0: error: fatal error encountered during compilation; please file a bug report with your project and the crash log
<unknown>:0: note: Program used external function '_$S8Komondor18renderScriptHeaderyS2SF' which could not be resolved!
0  swift                    0x000000011013e59a PrintStackTraceSignalHandler(void*) + 42
1  swift                    0x000000011013dd4e SignalHandler(int) + 302
2  libsystem_platform.dylib 0x00007fff71eb2b3d _sigtramp + 29
3  libsystem_platform.dylib 0x000000011bcabbc6 _sigtramp + 2850001062
4  libsystem_c.dylib        0x00007fff71d701c9 abort + 127
5  swift                    0x000000010c3c3a67 swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*)::$_1::__invoke(void*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, bool) + 519
6  swift                    0x00000001100f21c8 llvm::report_fatal_error(llvm::Twine const&, bool) + 280
7  swift                    0x000000010d17fe92 llvm::RuntimeDyld::resolveRelocations() + 3346
8  swift                    0x000000010d16a350 llvm::MCJIT::finalizeObject() + 176
9  swift                    0x000000010c3d0e2b performCompile(swift::CompilerInstance&, swift::CompilerInvocation&, llvm::ArrayRef<char const*>, int&, swift::FrontendObserver*, swift::UnifiedStatsReporter*) + 52219
10 swift                    0x000000010c3c0d35 swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) + 7717
11 swift                    0x000000010c366965 main + 1349
12 libdyld.dylib            0x00007fff71cc7ed9 start + 1
Stack dump:
0.	Program arguments: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift -frontend -interpret Package.swift -enable-objc-interop -sdk /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -I /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/pm/4_2 -I .build/debug -suppress-warnings -color-diagnostics -module-name Package -lPackageDescription -lPackageConfig 

Maybe I am just doing something wrong, but assuming not should we have yet another package that defines KomondorCommands?

[orta]

Oh yeah, that's a really good point!

I had thought about this for packageconfg but not tried implementation anything shibapm/PackageConfig#1

I think that might be really tricky (how would we be sure it was already built and in a linkable state?) to do.

Maybe then it'd just have to be special strings :-/

[nicholascross]

IMO after seeing what could be possible mixing in "special strings" doesn't seem like a very nice solution, but it might be the only reasonable way.

I did some hacking to see how it might look with an enum to denote command types. I like it.

I also had a crack at what a more strictly typed config might look like here

Still it is not coming together for me, due to linking issues. I thought it could be solved by using a seperate package but unfortunately it is still failing to resolve stuff.

<unknown>:0: error: fatal error encountered during compilation; please file a bug report with your project and the crash log
<unknown>:0: note: Program used external function '_$S16KomondorCommands16CommandProvidingMp' which could not be resolved!

It was fun but I have gone past the point of diminishing returns on this one, so I am going to stick with the original approach for now.

Thanks for your time!