Api based dependency check #51
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This updates our dependency check to use the API to collect all dependencies instead of crawling each package. It brings down total runtime of this part of the nightly job from 8.5 hours to 5-6 minutes.
If run as is, it finds 60 new packages, but I've also added a parameter to limit the additions per nightly run. We could set this to, say,
--limit 10such that these 60 new packages would be added over the course of 6 days. The idea is to have some sort of limit in place so we don't have too many new packages in one go hitting the build system. (Although 60 isn't really unmanageable either.)Two bigger questions are the following:
In the original dependency check, we skipped packages that had no products. I've dropped this, because it's not a check we apply to packages that are added manually either.
We also ignore packages that are forks. If we dropped this constraint, we'd add another ~160 packages. We may want to consider dropping this constraint, too, to ensure our coverage of packages and all their dependencies is as complete as possible. However, we may want to review how we're displaying and ranking forked packages before we add lots of them.
I don't think we have any check in place to ignore when packages are added manually, so the main reason to do this is the large influx of forks we'd add.
This whole change in backwards compatible in that it adds a new command
check-dependencies2without removing the old one. So this PR is safe to merge and only once we merge SwiftPackageIndex/PackageList#6057 will it take effect.