SwiftPackageIndex-Server
SwiftPackageIndex-Server copied to clipboard
SwiftPackageIndex
Re-analyze version skipping reanalysis of some versions, maybe?
We ran a re-analysis last night, and the feature isn't working as expected. At least it appears not to be!
The good news is that there are now 1,763 rows in versions covering 88 packages with has_binary_targets set. So, the analysis is definitely setting data.
The other good news is that where the flag has been set on the default branch version, the flag is correctly displayed on the page. See SemverBridge or Firebase for examples.
There was a bit of a bad smell, though. There are 88 distinct packages with the flag set on any of their versions, but only 43 where it is set on the default branch version. This may not be a problem, but it struck me as suspicious that over 50% of packages that had a binary target at some point no longer have it.
I investigated some examples:
- Pulse has no flag on the package page. I looked back in the version history in our database and it was v0.15.0 that had the flag set, and sure enough, there’s a binary target in that Package.swift - ✅ No problem found.
- Facebook is a problem, but one which is potentially caused by a complicated
Package.swiftfile. There are 16 versions in the database flagged as having binary targets, but the default branch version does not. ThePackage.swiftis so complex I can’t manually compile it in my head, though 😂 - ⚠️ Needs investigation. - OnfidoAuth looks like a legitimate problem. All tagged versions apart from 0.2.4, and the default branch version have the flag set, but the
Package.swiftfile on the default branch definitely has a binary target - 🛑 Confirmed error. - Appfigurate is the same deal. Four versions with the flag set but not the default branch. However, look at the
Package.swiftfile on the default branch - 🛑 Confirmed error.
I stopped looking for more examples at this point.
The only other thing that could be causing this bug is a problem with our re-analyze versions task. To check this. I ran a re-analyze for OnfidoAuth.
Is the suspense killing you?
…
…
There seems to be a bug in re-analyze. After running for Onfido again, all versions have the flag set.
😭
This is going to be a hard one to debug.
/cc @samdeane
Just re-analysed Facebook too, and it has now flagged the default branch version correctly.
I have merged #1946 to gate the display of the flag so we're able to roll code out still without this being inconsistent. I'm also going to run re-analysis on production today and see if the same issue happens there.
This is going to be a hard one to debug.
It shouldn't be too bad. If it's reproducible, we can run re-analyze-versions against a copy, or just re-enact the state locally by editing db content.
This is going to be a hard one to debug.
It shouldn't be too bad. If it's reproducible, we can run re-analyze-versions against a copy, or just re-enact the state locally by editing db content.
The prod analysis should tell us more. The reason I said it'd be hard to debug is if it only happens when running analysis over thousands of packages. Every individual package I re-analysed alone worked fine.
Is this related to the problem I found where re-analyse only touches some branches/versions? I think we discussed it @daveverwer, but I can't remember the exact details right now.
I think this is the relevant discussion: https://discord.com/channels/844491846452379649/844491937662763008/1001474049798586472
I found that I had to run analyze to get the main branch, and re-analyze to get the branches tagged with version numbers. Possibly you need a new mode for re-analyze which explicitly forces them all to get processed?
Yes, that's definitely the case. Re-analyse only deals with existing versions while analyse captures new ones. Since analyse is running continuously, that should convert everything eventually.
Alright, apologies for the delay in resuming this. Now that everything has had a few days on production, we should be in a stable state, but there are still issues where this flag is not being set correctly.
On prod there are 95 packages with some binary targets. Of those 95, 33 are showing false for has_binary_targets on the latest default branch.
Here's how those break down (from manually looking at each Package.swift):
✅ No binary targets ✅
- https://github.com/bitrise-io/trace-cocoa-sdk.git
- https://github.com/growthbook/growthbook-swift.git
- https://github.com/imgly/pesdk-ios-build.git
- https://github.com/imgly/vesdk-ios-build.git
- https://github.com/johnno1962/HotReloading.git
- https://github.com/johnno1962/InjectionScratch.git
- https://github.com/kean/Pulse.git
- https://github.com/mapbox/mapbox-maps-ios.git
- https://github.com/mapbox/turf-swift.git
- https://github.com/splitio/ios-client.git
- https://github.com/stripe/stripe-ios.git
❌ Has binary targets ❌
- https://github.com/AgoraIO/AgoraAudio_macOS.git
- https://github.com/alexpiezo/WebRTC.git
- https://github.com/Alviere/alviere-cards-ios.git
- https://github.com/Alviere/alviere-core-ios.git
- https://github.com/Alviere/alviere-payments-ios.git
- https://github.com/Alviere/alviere-remittances-ios.git
- https://github.com/braintree/braintree_ios.git
- https://github.com/FelixHerrmann/intercom-ios.git
- https://github.com/googleanalytics/google-tag-manager-ios-sdk.git
- https://github.com/imgly/imglykit-sp.git
- https://github.com/Instabug/Instabug-SP.git
- https://github.com/kewlbear/Python-iOS.git
- https://github.com/krzysztofzablocki/sourcery.git
- https://github.com/mapbox/mapbox-events-ios.git
- https://github.com/oozoofrog/SwiftDemangle.git
- https://github.com/pspdfkit/instant-sp.git
- https://github.com/pspdfkit/pspdfkitocr-sp.git
- https://github.com/stripe/stripe-terminal-ios.git
- https://github.com/SwiftyTesseract/libtesseract.git
- https://github.com/thegrizzlylabs/geniusscan-sdk-spm.git
- https://github.com/tink-ab/tink-money-manager-ios.git
Finally, https://github.com/jedisct1/swift-sodium.git is an interesting case. It has binary targets on iOS/macOS/tvOS/watchOS but not on Linux, where we do the analysis. It's not a big deal but it's an interesting edge case.
So there are 21 packages that should be showing has_binary_targets but are not.
Uploading latest DB dump into Discord with this data if anyone wants to look at this
Yes, that's definitely the case. Re-analyse only deals with existing versions while analyse captures new ones. Since analyse is running continuously, that should convert everything eventually.
Will this not eventually sort out the problem @daveverwer?
Alternatively if a new re-analyse mode would help, we could make an issue for that and I could do it. If there are still packages missing after that, we’d know that something else was broken.
Yes, that's definitely the case. Re-analyse only deals with existing versions while analyse captures new ones. Since analyse is running continuously, that should convert everything eventually.
Will this not eventually sort out the problem @daveverwer?
That's why I left it several days before re-checking this. The analysis should have passed by every default branch in that time and there were still not showing a correct has_binary status from their default branch after that time. I think there's a bug, highlighted in those 21 packages (or however many it is now).
Alternatively if a new re-analyse mode would help, we could make an issue for that and I could do it. If there are still packages missing after that, we’d know that something else was broken.
I don't think a new mode is needed. The re-analyze doing old versions and regular-analyze doing default branches is fine. It's just not right that after several days the default branch versions (which should have been through several cycles of analysis) are still not showing correct information.
That's not to say it's your responsibility to fix this @samdeane! Only work on it if you're interested in it, but as you said you might want to tackle another back-end problem, this struck me as something you might be interested in.
I don't think a new mode is needed. The re-analyze doing old versions and regular-analyze doing default branches is fine. It's just not right that after several days the default branch versions (which should have been through several cycles of analysis) are still not showing correct information.
Right, yes. I’m happy to take a look.
I guess the first thing to establish is whether manually running analyse on the failing ones fixes them (implying that it’s somehow not being run automatically), or not (implying that there’s a bug in the detection). Should be easy enough for me to figure that out and proceed accordingly.
The database dump from the 9th should match the state of the investigation I did since I took it to snapshot the state at the point of my investigation. I'll DM you a link to it.
Just an update on my status - still happy to look at this, but quite tied up with some contracting (and a ton of other things) currently, so it might be a few weeks! I’m assuming that it is non-urgent, but shout if that is wrong (or ever changes).
No problem, Sam. I wonder if we should remove the gate from the binary targets feature to at least get that live? I postponed it originally as I didn't want to deal with people noticing (and reporting) discrepancies, but it affects only a small number, and I feel bad that your work hasn't been seen yet.
Don't feel bad, it's fine. I did some stuff for Drew & Alex on Agenda once that took about nine months to see the light of day so this is comparatively quick 😆.