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

Package Registry client implementation does not support pagination using Link headers #8215

Open
1 task done
ehyche opened this issue Jan 14, 2025 · 8 comments
Open
1 task done
Labels

Comments

@ehyche
Copy link

ehyche commented Jan 14, 2025

Is it reproducible with SwiftPM command-line tools: swift build, swift test, swift package etc?

  • Confirmed reproduction steps with SwiftPM CLI. The description text must include reproduction steps with either of command-line SwiftPM commands, swift build, swift test, swift package etc.

Description

In the specification for the Swift Package Registry Service, the specification for the GET /{scope}/{name} endpoint says:

A server MAY paginate results by responding with a Link header field that includes any of the following relations:

and provides these examples of a Link header:

Link: <https://packages.example.com/mona/HashMap/5.0.3>; rel="latest-version",
      <https://packages.example.com/mona/HashMap?page=1>; rel="first",
      <https://packages.example.com/mona/HashMap?page=2>; rel="previous",
      <https://packages.example.com/mona/HashMap?page=4>; rel="next",
      <https://packages.example.com/mona/HashMap?page=10>; rel="last"

However, it does not appear that the SPM Package Registry client implementation supports pagination for the GET /{scope}/{name} endpoint.

This is a burden on Swift Package Registry Service (SPRS) implementations. For example, if you were implementing an SPRS service which proxied the Github API, you would most likely use this Github API endpoint to implement the GET /{scope}/{name}.

Take an example of implementing GET /{scope}/{name} for swift-syntax.

  1. You get the request for GET /swiftlang/swift-syntax from SPM to your SPRS
  2. You proxy this request to the Github API: GET https://api.github.com/repos/swiftlang/swift-syntax/tags
  3. This response uses the default value of per_page which is 30, and responds with the first 30 tags. Here is the Link header in that response:
Link: <https://api.github.com/repositories/143079594/tags?per_page=30&page=2>; rel="next", <https://api.github.com/repositories/143079594/tags?per_page=30&page=47>; rel="last"

This means there are 47 pages of tags for swift-syntax. So since the SPM client does not support pagination, then you had to make 47 requests to the GIthub API to fetch all of the tags before you can produce a response for the SPM client.

You could always use a per_page of 100 (which is the maximum), but that only reduces the number of requests you have to make to 14.

It would be much better if the SPRS could respond with its own Link header to the GET /{scope}/{name} response and the SPM client would respect that and make multiple GET /{scope}/{name} calls in order to fetch all of the package releases (tags).

When you request GET https://api.github.com/repos/swiftlang/swift-syntax/tags
That endpoint has a maximum per_page of 100meaning that for repositories with a LOT of tags (like swift-syntax), then your implementation would have to make

Expected behavior

I would expect the SPM Package Registry client to respect the Link header with next, last attributes and make multiple requests to the GET /{scope}/{name} endpoint in order to fetch all of the tags.

Actual behavior

The SPM Package Registry client makes ONE request to GET /{scope}/{name}. If the expected tag is not in that first page of tags, then package resolution fails.

Steps to reproduce

  1. Clone https://github.com/hollyoops/swiftPM-registry-service
  2. npm install
  3. set GITHUB_ACCESS_TOKEN="Your github token" && npm start
  4. swift package-registry set --global http://127.0.0.1:3001
  5. Create a Package.swift which depends on swiftlang.swift-syntax with tag 600.0.1 (the latest version)
  6. Observe package resolution fail, since it says it can't find the release 600.0.1

Swift Package Manager version/commit hash

6.0.0-dev

Swift & OS version (output of swift --version ; uname -a)

swift-driver version: 1.115.1 Apple Swift version 6.0.3 (swiftlang-6.0.3.1.10 clang-1600.0.30.1)
Target: arm64-apple-macosx15.0
Darwin ML-H71R34NL7K 24.2.0 Darwin Kernel Version 24.2.0: Fri Dec  6 19:02:41 PST 2024; root:xnu-11215.61.5~2/RELEASE_ARM64_T6030 arm64
@ehyche ehyche added the bug label Jan 14, 2025
@plemarquand
Copy link
Contributor

Thanks for the detailed reproduction steps, I'm seeing a failure to resolve as well.

Note that swiftPM-registry-service isn't forwarding the Link headers that contain next and last from GitHub, but even after adding that forwarding I'm seeing a failure to resolve.

@ehyche
Copy link
Author

ehyche commented Jan 14, 2025

@plemarquand : I am putting together a PR for swiftPM-registry-service that DOES forward the next and last Link headers from Github. That's how I discovered this bug. Once I got the forwarding implemented, and I could see it work via curl, then I turned SPM on it and it never used the next Link url that I provided.

I'll update this thread once I have the PR up for swiftPM-registry-service.

@plemarquand
Copy link
Contributor

Great, thanks! I have a rough fix in progress in SPM that iterates the next Links during package resolution.

I used swiftPM-registry-service for testing and had to rewrite the next/last Links so they pointed back to the swiftPM-registry-service server instead of directly to GitHub in order to have the proper Accept header.

If you'd like I can put up a draft PR when my patch is a bit further along and you could check it out, build it and then try to do a package resolve with the change in against your local swiftPM-registry-service to verify both changes.

plemarquand added a commit to plemarquand/swift-package-manager that referenced this issue Jan 14, 2025
In [4.1 List Package
Releases](https://github.com/swiftlang/swift-package-manager/blob/main/Documentation/PackageRegistry/Registry.md#41-list-package-releases)
it is stated that a server may respond with a `Link` header that contains
a pointer to a subsequent page of results. SPM is not checking for this
link in the `Link` header and so if a registry returns paginated results
only the first page of versions is searched when resolving.

Respect the `next` link in the `Link` header by loading the next page of
results and building up a list of versions, continuing until there is no
`next` link present in the `Link` header of the last result.

Issue: swiftlang#8215
@ehyche
Copy link
Author

ehyche commented Jan 14, 2025

@plemarquand : sounds good. I made the same change - rewriting the Link URLs. But I am guessing your change is probably better than mine - I am a Swift developer, and this was my first venture into TypeScript. 😄

@ehyche
Copy link
Author

ehyche commented Jan 15, 2025

@plemarquand : I put up a PR in the swiftPM-registry-service repository which returns correct Link headers in the response to the GET /{scope}/{name} request:

hollyoops/swiftPM-registry-service#2

You can test your SPM draft PR against a service running against swiftPM-registry-service running on my branch.

plemarquand added a commit to plemarquand/swift-package-manager that referenced this issue Jan 15, 2025
In [4.1 List Package
Releases](https://github.com/swiftlang/swift-package-manager/blob/main/Documentation/PackageRegistry/Registry.md#41-list-package-releases)
it is stated that a server may respond with a `Link` header that contains
a pointer to a subsequent page of results. SPM is not checking for this
link in the `Link` header and so if a registry returns paginated results
only the first page of versions is searched when resolving.

Respect the `next` link in the `Link` header by loading the next page of
results and building up a list of versions, continuing until there is no
`next` link present in the `Link` header of the last result.

Issue: swiftlang#8215
@plemarquand
Copy link
Contributor

@ehyche looking good! I spun up swiftPM-registry-service with your patch and did a swift package resolve with #8219. Resolution is now successful and I see in the logs that all the pages are being iterated.

@ehyche
Copy link
Author

ehyche commented Jan 15, 2025

@plemarquand : thanks so much for the quick action on this! What kind of turnaround time do you typically see on:

  • Getting SPM PRs merged
  • Having new SPM releases adopted by Xcode

I won't really be able to use this fix until it makes it into Xcode.

@plemarquand
Copy link
Contributor

plemarquand commented Jan 15, 2025

The SPM PRs don't take too long to merge, pending review. As for when it will be released in Xcode I'm not sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants