-
-
Notifications
You must be signed in to change notification settings - Fork 141
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
Fails on Xcode 12 #365
Comments
Interesting, will try it on Xcode 12 and see if I can get something useful |
I tried with the Dangerfile in this repo and got swift run danger-swift pr https://github.com/danger/swift/pull/314 --dangerfile Dangerfile.swift
[4/4] Merging module Logger
Starting Danger PR on danger/swift#314
You don't have a DANGER_GITHUB_API_TOKEN set up, this is optional, but TBH, you want to do this
Check out: http://danger.systems/js/guides/the_dangerfile.html#working-on-your-dangerfile
Danger: ✓ passed review, received no feedback. I also tried with a test warning
Looking at the stacktrace you posted
Makes me think that there might be some string in the warnings, messages, markdowns or failures that might be making it crash for some reason when is encoded in the JSON file that is returned as result of the Dangerfile execution |
Hmm interesting @f-meloni... I've completely cleared out the dangerfile though with only a single message with normal text and still have the same issue... Let me try running danger as you did on this repo and see what happens |
@f-meloni I was able to successfully do it from this repo like you were... and I was able to reproduce the crash on the repo as well. This is really strange but the difference in one letter of a message will crash. Here are two examples of a Dangerfile: Succeeds:
Fails:
The only difference is the "m" in the message... how could this be crashing it? Maybe related to the length of the string? |
From what I can gather... any string over 15 characters long crashes! |
Mmmmm
The only thing I see here is that it tried to encode the violation message and then crashes, it might be a bug in the |
|
The same string you just posted crashes for me (Xcode-beta 6). So ya... could be a bug with encoding or something, but that seems like it would be a much bigger issue in the betas since we use features like that all over the place in our apps. |
So it seems it's not directly related to messaging, since the following also crashes:
Seems like an issue parsing the Dangerfile in general. Removing the enum works fine. |
Does the enum crash have the same stacktrace? |
Looks like a different stacktrace (but similar starts). Enum Stacktrace:
Message stacktrace:
|
from the stack: looks like its around json parsing.
maybe a null or number is being coerced into a string? |
I'm struggling to figure out exactly why it's failing. Best I've come up with so far is that the crash is happening in |
I saw simliar issues today when trying |
So I'm convinced this might be an issue on Swift itself, the behaviour is not so different to what we had on #214 some totally random code makes it crash, in that case where some static function. I will make some tries on a branch to see if something like using swift 5.3 toolchain or not using at exit can make it work |
@f-meloni thanks for looking into it. I've been trying on the 5.3 toolchain and also specifying the macos version since at some point I saw a crash due to the executable trying to use macos 11 toolchain even though I'm still on 10.15.x by adding the following to Package.swift:
At this point I can sum up the issue as:
Maybe there's a step in between that I'm not sure about that you can find where this "corruption" is happening 🤷 . But yes, I agree that it is a Swift bug of some sort, not directly a Danger issue. |
@ecerney I've pushed a branch called |
@f-meloni That fixes it! I ran danger on both this project, and on an internal PR / Dangerfile and it worked. I haven't gone through and looked at all your changes on the branch yet to see why it fixes it, but it does! Note: I don't have Swift 5.1.1 on this machine, so I changed the .swift-version to 5.3 and it worked fine 👍 |
Great! It looses some performances, because generates the JSON every time you write a warning/failure/message, but is something really minor, then is a valid workarounds in case they don't fix it with the next beta/GM |
Ah I see, you are just dumping the results every time they are updated instead of waiting for the process to exit... Nice work around. Obviously it sucks to have to write to disk after every Violation, but 🤷 |
So I was able to run everything fine when building from your branch locally... but for some reason it fails when pulling in the branch from my repo's package.swift:
0 swift 0x00000001052c0a85 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
1 swift 0x00000001052bfa85 llvm::sys::RunSignalHandlers() + 85
2 swift 0x00000001052c103f SignalHandler(int) + 111
3 libsystem_platform.dylib 0x00007fff6d3625fd _sigtramp + 29
4 libsystem_platform.dylib 0x000000010750aa00 _sigtramp + 18446603343101592608
5 libswiftCore.dylib 0x00007fff6caae946 swift_conformsToProtocolImpl(swift::TargetMetadata<swift::InProcess> const*, swift::TargetProtocolDescriptor<swift::InProcess> const*) + 86
6 libswiftCore.dylib 0x00007fff6ca7d5ea swift::_conformsToProtocol(swift::OpaqueValue const*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetProtocolDescriptorRef<swift::InProcess>, swift::TargetWitnessTable<swift::InProcess> const**) + 42
7 libswiftCore.dylib 0x00007fff6ca812cb _conformsToProtocols(swift::OpaqueValue const*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetExistentialTypeMetadata<swift::InProcess> const*, swift::TargetWitnessTable<swift::InProcess> const**) + 251
8 libswiftCore.dylib 0x00007fff6ca8063e _dynamicCastToExistential(swift::OpaqueValue*, swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetExistentialTypeMetadata<swift::InProcess> const*, swift::DynamicCastFlags) + 398
9 libswiftFoundation.dylib 0x00007fff6cc4abab $s10Foundation13__JSONEncoder33_12768CA107A31EF2DCE034FD75B541C9LLC4box_ySo8NSObjectCSgSE_pKF + 315
10 libswiftFoundation.dylib 0x00007fff6cc4a628 $s10Foundation11JSONEncoderC6encodeyAA4DataVxKSERzlF + 456
11 libswiftFoundation.dylib 0x00007fff6cd3cede $s10Foundation11JSONEncoderC6encodeyAA4DataVxKSERzlFTj + 14
12 libDanger.dylib 0x000000010a6b69f8 $s6Danger17dumpResultsAtExit33_DA57A8538BDE0544FE0FD943A1DB0C2BLL_4pathyAA0A6RunnerC_SStF0B0L_yyF + 552
13 libsystem_c.dylib 0x00007fff6d213143 __cxa_finalize_ranges + 326
14 libsystem_c.dylib 0x00007fff6d213412 exit + 55
15 libdyld.dylib 0x00007fff6d169cd0 start + 8
16 libdyld.dylib 0x000000000000001b start + 18446603338685965139
danger-results://var/folders/fn/gwhkhbj526b_qlrl0sm8jhnxtnzk2_/T/danger-response.json 😓 |
DumpAtExit doesn't exist anymore in my branch, so it looks like is still using the last released version 🤔 |
Oh shoot you're right I didn't see that. Strange. The build folder has your latest code in the checkout though... will try to figure out why it may be using some other executable or something |
@f-meloni I just created my own fork of your branch, edited one of the debug printouts and pointed my Package.swift to my fork/branch and I see my updated output, but still see the |
Is possible that you are caching the |
Ya I deleted all caches and it still failed. I think there is something weird going on with SPM. Here's what I was seeing:
I noticed that even though I wasn't calling anything with SPM, SPM was still building the danger-swift project which somehow makes everything work. Seems like something is going on that I'm not familiar with. Example From my project directory (fails):
From danger-swift directory (works):
|
Any update on this issue, guys? 🙏 |
No update from me other than I had to take a break trying to find a work around and just disabled danger for our Xcode 12 builds :/. If I get more time this week I'll continue to look into it but it seems like just a Swift bug |
I've opened #372 |
I've released danger-swift 3.5.0, please give it a try and tell me if it works. |
Edit: See below comment. Looks like an old homebrew version of danger-swift was being used instead of one created with SPM |
Ok @f-meloni I figured out that I had a libDanger installed from homebrew... so I deleted that, and now get the error:
So it looks like SPM isn't building a libDanger anymore from the danger-swift package... any idea why? Edit: |
Try to uninstall it from brew, go on the folder where the Package.swift is, run |
Thanks for all the help @f-meloni... Got our CI up and running with the latest danger-swift and looks like everything's good. Still need to mess with the run command / danger structure because I am still relying on the homebrew version (--cwd isn't working quite right but only spent a few minutes trying), but at least for now everything works great on Xcode 12. |
I think this is solved now, closing, if you have any issue feel free to re open it |
I'm still trying to debug why exactly it's failing, but thought I'd post the issue here while I look into it in case someone else has seen the same thing.
Issue:
Since switching to Xcode-12 beta, danger stopped working on bitrise (and locally). The command I'm running locally is:
And the result is:
This issue does not happen if I change the toolchain back to Xcode 11.
The text was updated successfully, but these errors were encountered: