danger / swift

⚠️ Stop saying "you forgot to …" in code review
https://danger.systems/swift/
MIT License
1.05k stars 141 forks source link

Getting dataCorrupted from SwiftLint #339

Open skywalkerdude opened 4 years ago

skywalkerdude commented 4 years ago

Here is my danger.yml:

on:
  pull_request:
    branches: [ master ]

jobs:
  build:
    runs-on: ubuntu-latest
    name: "Run Danger"
    steps:
      - uses: actions/checkout@v1
      - name: Danger
        uses: danger/swift@3.0.0
        with:
            args: --failOnErrors --no-publish-check
        env:
            GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Here is my Dangerfile.swift:

import Danger

let danger = Danger()

SwiftLint.lint(swiftlintPath: "./Pods/SwiftLint/swiftlint")

// Ensure no copyright header
let editedFiles = danger.git.modifiedFiles + danger.git.createdFiles
let swiftFilesWithCopyright = editedFiles.filter {
    $0.contains("Copyright") && ($0.fileType == .swift  || $0.fileType == .m)
}
for file in swiftFilesWithCopyright {
    fail(message: "Please remove this copyright header", file: file, line: 0)
}

// Encourage smaller PRs
var bigPRThreshold = 600;
if (danger.github.pullRequest.additions! + danger.github.pullRequest.deletions! > bigPRThreshold) {
    warn("> Pull Request size seems relatively large. If this Pull Request contains multiple changes, please split each into separate PR will helps faster, easier review.");
}

I'm not sure why this is happening or how to fix it. I tried the SwiftLint action here (https://github.com/norio-nomura/action-swiftlint) and everything worked fine. Just the Danger SwiftLint plugin seems to barf every time :(

Here is a related issue: https://github.com/danger/swift/issues/266 I tried to open that issue but I didn't see a button to open it.

skywalkerdude commented 4 years ago

Here's what the raw log looks like:

2020-04-13T00:52:56.3660046Z /bin/sh: 1: ./Pods/SwiftLint/swiftlint: Exec format error
2020-04-13T00:52:56.3660233Z 
2020-04-13T00:52:56.4448804Z 
2020-04-13T00:52:56.4449711Z 
2020-04-13T00:52:56.4612122Z Failing the build, there is 1 fail.

And the issue posted on my PR is just

Error deserializing SwiftLint JSON response (): dataCorrupted(Swift.DecodingError.Context(codingPath: [], debugDescription: "The given data was not valid JSON.", underlyingError: Optional(Error Domain=NSCocoaErrorDomain Code=3840 "The data is not in the correct format.")))
f-meloni commented 4 years ago

I saw on your PR

// This works though, for some reason SwiftLint.lint(.modifiedAndCreatedFiles(directory:"HymnsTests"), configFile: ".swiftlint.yml", swiftlintPath: "Pods/SwiftLint/swiftlint")

I wonder if one of the modified file paths has a strange format of something like that, and that is breaking it. because .modifiedAndCreatedFiles(directory:"HymnsTests") is just filtering out all the items that are not in that folder

skywalkerdude commented 4 years ago

Yeah, that's what I thought about too. I was going to try the .files LintStyle to try to isolate the file(s) that are causing the error. But I couldn't find documentation on how to instantiate the File object that it needed. (I'm pretty new to swift, so I wasn't sure what exactly to make of https://github.com/danger/swift/blob/master/Sources/Danger/File.swift :confounded: )

f-meloni commented 4 years ago

File is just a typealias of String

/// A simple typealias for strings representing files
public typealias File = String

Then you can use the array of modified/created files and filter it, then pass the result to .files

skywalkerdude commented 4 years ago

@f-meloni Can you take a look at this PR? https://github.com/HymnalDreamTeam/Hymns-iOS/pull/51

I only ran it on a specific file. Doesn't look like there's anything wrong with that filename? If the path is wrong let me know, but I think it's the correct path.

alexfu commented 4 years ago

I ran into a similar error. I looked at the GitHub workflow log and saw:

Pods/SwiftLint/swiftlint: 1: Pods/SwiftLint/swiftlint: Syntax error: word unexpected (expecting ")")

Not sure what this means... it looks like swiftlint is linting swiftlint ?

f-meloni commented 4 years ago

@alexfu I don't really know what is happening, but Swiftlint seems to have some problems with Linux. I've released a Docker image that has an embedded version of Swiftlint (builded on Linux), made from this repo https://github.com/f-meloni/danger-swift-swiftlint-action

Try to use using: docker://frmeloni/danger-swift-with-swiftlint:1.1.0

It should also speed up your action of 2-3 minutes

ffried commented 4 years ago

We encountered the same problem in this PR in CocoaLumberjack after we've switched from Travis + Danger.ruby to GitHub Actions + Danger.swift.

@f-meloni thank you for providing the pre-built docker image. However, I'd like to avoid having to use it given the fact that it won't be kept up to date? (which I do not expect btw!) But given the additional speedup, would it make sense to include these pre-built docker images in Danger swift's release process? As a bonus, it would probably also solve this issue?

Here's the Raw Logs from our failing run:

2020-07-16T10:26:16.6026135Z ##[group]Run danger/swift@3.3.2
2020-07-16T10:26:16.6026285Z with:
2020-07-16T10:26:16.6026493Z   args: --pr https://api.github.com/repos/CocoaLumberjack/CocoaLumberjack/pulls/1146
2020-07-16T10:26:16.6026667Z env:
2020-07-16T10:26:16.6026778Z   LC_CTYPE: en_US.UTF-8
2020-07-16T10:26:16.6026905Z   LANG: en_US.UTF-8
2020-07-16T10:26:16.6027802Z   GITHUB_TOKEN: ***
2020-07-16T10:26:16.6027935Z ##[endgroup]
2020-07-16T10:26:16.6052318Z ##[command]/usr/bin/docker run --name c20136143a5a42d6474f89bad97df0f51daa_e059c4 --label 87c201 --workdir /github/workspace --rm -e LC_CTYPE -e LANG -e GITHUB_TOKEN -e INPUT_ARGS -e HOME -e GITHUB_JOB -e GITHUB_REF -e GITHUB_SHA -e GITHUB_REPOSITORY -e GITHUB_REPOSITORY_OWNER -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_ACTOR -e GITHUB_WORKFLOW -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -e GITHUB_EVENT_NAME -e GITHUB_SERVER_URL -e GITHUB_API_URL -e GITHUB_GRAPHQL_URL -e GITHUB_WORKSPACE -e GITHUB_ACTION -e GITHUB_EVENT_PATH -e RUNNER_OS -e RUNNER_TOOL_CACHE -e RUNNER_TEMP -e RUNNER_WORKSPACE -e ACTIONS_RUNTIME_URL -e ACTIONS_RUNTIME_TOKEN -e ACTIONS_CACHE_URL -e GITHUB_ACTIONS=true -e CI=true -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/runner/work/_temp/_github_home":"/github/home" -v "/home/runner/work/_temp/_github_workflow":"/github/workflow" -v "/home/runner/work/CocoaLumberjack/CocoaLumberjack":"/github/workspace" 87c201:36143a5a42d6474f89bad97df0f51daa --pr https://api.github.com/repos/CocoaLumberjack/CocoaLumberjack/pulls/1146
2020-07-16T10:26:30.8835685Z /bin/sh: 1: swiftlint: not found
2020-07-16T10:26:30.8835916Z 
2020-07-16T10:26:31.0004263Z 
2020-07-16T10:26:31.0004635Z 
2020-07-16T10:26:31.0232133Z Failing the build, there is 1 fail.
2020-07-16T10:26:32.3546162Z Feedback: https://github.com/CocoaLumberjack/CocoaLumberjack/pull/1146#issuecomment-659322158
f-meloni commented 4 years ago

@ffried Using a danger official docker image is been discussed on https://github.com/danger/swift/issues/292, and unfortunately that doesn't look a viable option. I created https://github.com/danger/swift/blob/master/.github/workflows/publish_package.yml to automatically publish a GitHub package when a new version is released (we just need to add valid credentials in the secrets), but that anyway doesn't really solve the problem, because GitHub Packages can not be used on Actions at the moment, because they require authentication (unless that was fixed). https://github.com/f-meloni/danger-swift-swiftlint-action looked then the easiest solution to the problem, and is easy to maintain

ffried commented 4 years ago

@f-meloni I see. Thanks for explaining! Haven't tried myself, but according to this help page, it should be possible to use GitHub Packages together with GitHub Actions...

Is there a package available somewhere that I could use to try this?

f-meloni commented 4 years ago

@ffried https://github.com/f-meloni/danger-swift-swiftlint-action creates a package with the same version that is on docker hub on every tag, then if you want to try, you can use the latest version at https://github.com/f-meloni/danger-swift-swiftlint-action/packages/191011

ffried commented 4 years ago

@f-meloni Apparently the problem is not using GitHub Packages with GitHub Actions in general, but specifically using a Docker GitHub Package the run-image for a step (with the using: keyword): https://github.community/t/use-docker-images-from-github-package-registry/16135/12

Sad, that GitHub can't make their services work with each other.

TheCoordinator commented 3 years ago

Using the new docker package here in the Github registry and still getting the same error I'm afraid. Hard to say what exactly is going wrong?

https://github.com/orgs/danger/packages/container/package/danger-swift-with-swiftlint

Here is my danger file

import Danger
import Foundation

let danger = Danger()

let allSourceFiles = danger.git.modifiedFiles + danger.git.createdFiles
let allSwiftFiles = allSourceFiles.filter { $0.fileType == .swift }

let additions = danger.github.pullRequest.additions ?? 0
let deletions = danger.github.pullRequest.deletions ?? 0

let bigPRThreshold = 400

if additions + deletions > bigPRThreshold {
    warn("> Pull Request size seems relatively large. If this Pull Request contains multiple changes, please split each into separate PR will helps faster, easier review.")
}

let coreSourceChanged = allSourceFiles.first { $0.hasPrefix("App/Core/Sources/Entities") }
let coreTestsChanged = allSourceFiles.first { $0.hasPrefix("App/Core/Tests/Entities") }

if coreSourceChanged != nil && coreTestsChanged == nil {
    warn("Consider adding tests for Core Entities updates")
}

SwiftLint.lint(.modifiedAndCreatedFiles(directory: "App"), inline: true)