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

Issue 2551 check dependencies failure #45

Merged
merged 3 commits into from
Aug 12, 2023

Conversation

finestructure
Copy link
Member

@finestructure finestructure commented Aug 12, 2023

I believe this changed when we adopted Swift 5.9 for validation. There's been a subtle format change in the package dump json from

          "location" : {
            "remote" : [
              "https://github.com/apple/swift-argument-parser"
            ]
          },

to

          "location" : {
            "remote" : [
              {
                "urlString" : "https://github.com/JohnSundell/Ink.git"
              }
            ]
          },

which tripped us up. We're now supporting both versions.

I've made a note to try and detect changes like this somehow between Swift versions.

Fixes SwiftPackageIndex/SwiftPackageIndex-Server#2551

Copy link
Member

@daveverwer daveverwer left a comment

Choose a reason for hiding this comment

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

We could have a snapshot test of the output from dump-package run on a non-trivial package. There would be a lot of artefacts for that test, and it might be slow to run, so it could even be a separate CI stage that just does that one job, and only run on CI, maybe.

@daveverwer
Copy link
Member

I'm not sure what the up-to-date plans are with deprecating dump-package are at this stage, but we still have #1172 hanging over our heads as well.

@finestructure
Copy link
Member Author

❯ swift run validator check-dependencies -i test.json
Building for debugging...
[444/444] Linking validator
Build complete! (5.57s)
Warning: Using anonymous authentication -- you will quickly run into rate limiting issues

Cache loaded (0 entries)
Checking dependencies (1 packages) ...
Dependencies for https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server.git ...
  - https://github.com/JohnSundell/Ink.git
  - https://github.com/swift-server-community/SwiftPrometheus.git
  - https://github.com/SwiftPackageIndex/DependencyResolution.git
  - https://github.com/SwiftPackageIndex/SPIManifest.git
  - https://github.com/SwiftPackageIndex/SemanticVersion.git
  - https://github.com/apple/swift-package-manager.git
  - https://github.com/dankinsoid/VaporToOpenAPI.git
  - https://github.com/pointfreeco/swift-custom-dump.git
  - https://github.com/pointfreeco/swift-parsing.git
  - https://github.com/pointfreeco/swift-snapshot-testing.git
  - https://github.com/scinfu/SwiftSoup.git
  - https://github.com/soto-project/soto.git
  - https://github.com/vapor/fluent-postgres-driver.git
  - https://github.com/vapor/fluent.git
  - https://github.com/vapor/vapor.git

@finestructure finestructure merged commit df7a672 into main Aug 12, 2023
3 checks passed
@finestructure finestructure deleted the issue-2551-check-dependencies-failure branch August 12, 2023 11:36
@finestructure
Copy link
Member Author

We could have a snapshot test of the output from dump-package run on a non-trivial package. There would be a lot of artefacts for that test, and it might be slow to run, so it could even be a separate CI stage that just does that one job, and only run on CI, maybe.

Yeah, although we'd have to do that in SPI-Server, not here, because we hardly ever update validator and wouldn't notice.

@daveverwer
Copy link
Member

daveverwer commented Aug 12, 2023

Yeah, although we'd have to do that in SPI-Server, not here, because we hardly ever update validator and wouldn't notice.

Yes, sorry. I didn't say it, but I meant for it to be in the context of SPI-Server 👍 That way, as the base image moves forward, we're covered

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

Successfully merging this pull request may close these issues.

SPI-Server dependency not picked up
2 participants