Skip to content
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

Support for Swift package manager #7525

Merged
merged 7 commits into from
Jul 25, 2023

Conversation

deivid-rodriguez
Copy link
Contributor

This PR implements #1245.

For now just a draft but most of the functionality I intend to ship initially should be already there.

@github-actions github-actions bot added L: ruby:bundler RubyGems via bundler L: elixir:hex Elixir packages via hex L: terraform Terraform packages L: rust:cargo Rust crates via cargo labels Jul 6, 2023
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/swift-support branch 9 times, most recently from 4b502b8 to d699384 Compare July 11, 2023 08:52
@deivid-rodriguez
Copy link
Contributor Author

One question for @stevapple (maybe @0xTim knows too?).

So, our updater runs under Linux, and since we're running Swift directly, I've encountered a problem where we're adding Linux specific dependencies to some manifests.

For example, whenever we find this dependency.

Can you think of any workaround? I was thinking we could maybe ask upstream for some feature that allows to resolve under a specific platform, like swift package update --platform macOS or something. But regardless of that, it'd be nice to figure out a way to somehow override this #if os() macro.

That said, what I've been able to see from some forum research is that this may be an old hack and not what's recommended right now. Maybe the right way now is conditional dependencies. If that's the case, maybe it's best to let libraries using this old hack fix themselves.

@stevapple
Copy link

stevapple commented Jul 13, 2023

The only way to deal with such hack is to syntactically parse the file with SwiftSyntax, IIRC. It’s hard and not worth the effort.
Some packages may rely on this hack because their dependencies used it and some because they’re directly using platform-specific APIs in the manifest, so removing or bypassing can be likely breaking.

I would suggest to leave it alone and let the library author fix it. These packages are also not likely to have Package.resolved committed, because it could differ from platform to platform.

I've encountered a problem where we're adding Linux specific dependencies to some manifests.

I don’t think it’s a big problem… SwiftPM will remove unnecessary packages and add necessary ones during resolution.

@deivid-rodriguez
Copy link
Contributor Author

Thanks for the input, that's reassuring!

@deivid-rodriguez
Copy link
Contributor Author

Not fully done yet, but happy to get some early feedback here!

@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/swift-support branch 3 times, most recently from 0477599 to 67cde3a Compare July 18, 2023 17:13
@github-actions github-actions bot removed L: ruby:bundler RubyGems via bundler L: elixir:hex Elixir packages via hex L: terraform Terraform packages L: rust:cargo Rust crates via cargo labels Jul 18, 2023
@deivid-rodriguez
Copy link
Contributor Author

In order to keep this PR smaller and easier to review, I removed Security Update support and versioning strategies from this initial PR. Will pull those changes later.

Unless CI proves me wrong, this should be ready for a review now!

@deivid-rodriguez deivid-rodriguez marked this pull request as ready for review July 18, 2023 18:44
@deivid-rodriguez deivid-rodriguez requested a review from a team as a code owner July 18, 2023 18:44
@deivid-rodriguez deivid-rodriguez added F: language-support Issues specific to a particular language or ecosystem; may be paired with an L: label. L: swift Swift packages labels Jul 18, 2023
Copy link
Member

@jakecoffman jakecoffman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, maybe we can start some smoke tests too? 👏

@deivid-rodriguez
Copy link
Contributor Author

Yes, I'll create some smoke tests tomorrow!

@deivid-rodriguez
Copy link
Contributor Author

deivid-rodriguez commented Jul 19, 2023

Smoke test created at dependabot/smoke-tests#106.

My understanding is that we need to merge this and let an initial updater image for Swift be published so that the smoke test can pass.

EDIT: And also a new release of https://github.com/dependabot/cli that supports pulling the swift updater.

@deivid-rodriguez
Copy link
Contributor Author

CLI with support for pulling the Swift updater has been released.

Once CI is green, I'll deploy this change, and then see if the smoke tests pass.

@deivid-rodriguez
Copy link
Contributor Author

Actually I just pushed a small trick to verify smoke tests before merging.

@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/swift-support branch 3 times, most recently from ffc365d to 9b1f0c2 Compare July 19, 2023 14:41
@deivid-rodriguez
Copy link
Contributor Author

The smoke test is passing now! And it actually detected a real issue! I will next merge the smoke test PR, regenerate smoke tests cache, and then edit this PR to run against the default branch of the smoke tests repo.

@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/swift-support branch 3 times, most recently from e4aef8e to e855a4f Compare July 19, 2023 16:12
@deivid-rodriguez deivid-rodriguez merged commit e389471 into main Jul 25, 2023
91 checks passed
@deivid-rodriguez deivid-rodriguez deleted the deivid-rodriguez/swift-support branch July 25, 2023 09:12
brettfo pushed a commit to brettfo/dependabot-core that referenced this pull request Oct 11, 2023
…wift-support

Support for Swift package manager
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: language-support Issues specific to a particular language or ecosystem; may be paired with an L: label. L: swift Swift packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants