-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[WIP] registry publish api with libzip #6125
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
base: main
Are you sure you want to change the base?
Conversation
motivation: progress on registry publishing implementationn changes: * implement compression using ZipArchiver * implement code flow to archive package for publishing per spec * add tests
@@ -59,6 +59,7 @@ endif() | |||
find_package(dispatch QUIET) | |||
find_package(Foundation QUIET) | |||
find_package(SQLite3 REQUIRED) | |||
find_package(libzip REQUIRED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is different from zlib, right? I was thinking since we already depend on zlib and have plans to add it to update-checkout-config.json
to always build it with the toolchain, maybe we could rely on that one? That's assuming it provides all of the necessary functionality.
} | ||
} | ||
|
||
struct Compressor: Cancellable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it stores references to FileSystem
and ThreadSafeBox
, this probably has reference semantics and should be a final class
.
} | ||
} | ||
|
||
struct Validator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto for reference semantics
import TSCTestSupport | ||
import XCTest | ||
|
||
class ArchiverTests: XCTestCase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
class ArchiverTests: XCTestCase { | |
final class ArchiverTests: XCTestCase { |
import TSCTestSupport | ||
import XCTest | ||
|
||
class LibzipArchiverTests: XCTestCase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
class LibzipArchiverTests: XCTestCase { | |
final class LibzipArchiverTests: XCTestCase { |
.systemLibrary(name: "SPMSQLite3", pkgConfig: "sqlite3"), | ||
.systemLibrary(name: "SPMLibzip", pkgConfig: "libzip"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming nit: looks like SwiftPM is already an established abbreviation in our codebase and documentation, maybe this could be named SwiftPMLibzip
for consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot of "SPM" as well, especially in type names, but I agree that we should clean that up at some point.
motivation: progress on registry publishing implementationn
changes: