add artifacts archive file format #3295

tomerd · 2021-02-20T00:22:43Z

motivation: to support the package extensions feature, we need to support binary dependencies with tools that can be distributed across different platforms

changes:

introduce new binary dependency type "artifactsArchive" with the extension "arar"
the structure of the archive is inspired by XCFramework, and it can contain multiple tools/executables targeting multiple platforms and architectures using target triples to indicate such support
update BuildPlan code to parse "arar" files and extract the tools available to the target in a [String: AbsolutePath] which * can be later hooks to be used by extensions
refactor XCFramework code
update call-sites and tests and add new tests

the new archive file name, extension and structure are all up to debate and would take final shape in an evolution proposal, but we can start discussing them now in preparation to such proposal.

the file structure proposed

<file>.arar
|- info.json
|- <artifact identifier> <-- an archive may have several artifacts
|   |   
|   <group identifer> <-- psoudo target triplet
|   |   |- executable
|   |   |- executable supporting files
|   |
|   <group identifer> 
|       |- executable
|       |- executable supporting files
|
|— <artifact identifier>
|   | ...

having the metadata file look like

{
    "schemaVersion": 1.0,
    "availableArtifacts": {
        "<artifact identifier>": {
            "version": "<version number>",
            "type": "<artifact type (e.g. executble, library)>",
            "variants": [ 
                    {
                        "exectuablePath": "<path to exectuable>",
                        "supportedTriples": ["<triplet identifier>"]
                    }
            ]
        }
    }
}

concrete example:

protocol-bufffer-compiler.exar
|- info.json
|— protocol-bufffer-compiler
|     |
|     x86_64-osx
|     |   |- protoc executable for macOS
|     | 
|     x86_64-linux
|     |   |- protoc executable for Linux

{
    "schemaVersion": 1.0,
    "availableArtifacts": {
        "protocol-bufffer-compiler": {
            "version": "3.5.1",
            "type": "executable",
            "variants": [
                {
                    "path": "x86_64-osx/protoc-3.5.1",
                    "supportedTriples": ["x86_64-apple-macosx"]
                },
                {
                    "path": "x86_64-linux/protoc-3.5.1",
                    "supportedTriples": ["x86_64-unknown-linux-gnu"]
                }
            ]
    }
}

open questions:

how do we deal with system dependencies these tools may have? we could require that the archive metadata includes information on such dependencies and do basic validation for the user, but this could get very messy.
- imo we should lean on SwiftPM systemLibrary to express such dependencies and not invent something new
what about versioning? do we need to add metadata about the tool version?
- I suggest we add a metadata field to indicate the version. I can't see how we practically use this field but at least we can make it available to the extension that can do some validation?
We are likely to extend the ArtifactsArchive file format to carry other types of artifacts beyond executables. Additional fields are likely to be required to support such artifact types, e.g. headers path for libraries.

tomerd · 2021-02-20T00:22:55Z

@swift-ci please smoke test

tomerd · 2021-02-20T00:41:11Z

@swift-ci please smoke test

Sources/Build/BuildPlan.swift

Sources/PackageModel/Target.swift

motivation: to support the package extensions feature, we need to support binary dependencies with tools that can be distributed across different platforms changes: * introuce new binary dependency type "toolsArchive" with the extension "toar" * the structure of the archive is inspired by XCFramework, and it can contain multiple tools/executables targeting multiple platforms and architectures using target triplets to indicate such support * update BuildPlan code to parse "toar" files and extract the tools available to the target in a [String: AbsolutePath] which can be later hooks to be used by extensions * refactory XCFramework code * update call-sites and tests and add new tests

tomerd · 2021-02-22T00:45:43Z

@swift-ci please smoke test

tomerd · 2021-02-22T00:53:21Z

@swift-ci please smoke test

Sources/Build/ArtifactsArchiveMetadata.swift

abertelrud · 2021-02-22T02:07:19Z

Sources/Build/ArtifactsArchiveMetadata.swift

+
+    public struct Variant: Equatable {
+        let path: String
+        let supportedTriplets: [Triple]


I suggest supportedTriples for consistency.

abertelrud · 2021-02-22T02:09:11Z

Sources/Build/ArtifactsArchiveMetadata.swift

+extension ArtifactsArchiveMetadata: Decodable {
+    enum CodingKeys: String, CodingKey {
+        case schemaVersion
+        case artifacts = "availableArtifacts"


Nit: should this just be "artifacts"? Are there any artifacts in the archive that aren't available?

I just saw the example of the existing XCFrameworks format in the unit test, which uses AvailableLibraries, so now I understand where this comes from. I could go either way on this: if we expect to move to arar as the future format that can handle all types of artifacts than maybe it's more important to pick names we think are clear than to be consistent with XCFrameworks. Could go either way for me.

abertelrud · 2021-02-22T02:09:26Z

Sources/Build/ArtifactsArchiveMetadata.swift

+        case type
+        case version
+        case variants
+        case supportedTriplets


I suggest supportedTriples to be consistent.

Also: since an Artifact has Variants, and both have supportedTriples, is this one a union of all the ones for all the variants?

hmm no, this is a typo

abertelrud · 2021-02-22T02:17:06Z

Sources/Build/BuildPlan.swift

+                    switch binaryTarget.kind {
+                    case .xcframework:
+                        let libraries = try self.parseXCFramework(for: binaryTarget)
+                        libraries.forEach { library in


Nit: what is our stance on for _ in vs forEach in the SwiftPM codebase? Historically I think we've been using for _ in, which I personally find much easier to read, but we should probably choose one and use the same in the codebase.

abertelrud · 2021-02-22T02:20:32Z

Sources/Build/BuildPlan.swift

            }

            let libraryDirectory = target.artifactPath.appending(component: library.libraryIdentifier)
-            let binaryPath = libraryDirectory.appending(component: library.libraryPath)
+            let libraryPath = libraryDirectory.appending(component: library.libraryPath)
            let headersPath = library.headersPath.map({ libraryDirectory.appending(component: $0) })


I realize this is existing code but this one should probably be .appending(RelativePath($0)) as well, if the path can be a relative path.

abertelrud

Left a few comments, which are mostly suggestions but saw a couple of places of appending(component: )) in existing code that should probably be appending(RelativePath()) now if we support subpaths in those keys.

abertelrud · 2021-02-22T02:20:41Z

Sources/Build/BuildPlan.swift

            }

            let libraryDirectory = target.artifactPath.appending(component: library.libraryIdentifier)
-            let binaryPath = libraryDirectory.appending(component: library.libraryPath)
+            let libraryPath = libraryDirectory.appending(component: library.libraryPath)


I realize this is existing code but this one should probably be .appending(RelativePath($0)) as well, if the path can be a relative path.

Sources/Build/BuildPlan.swift

abertelrud · 2021-02-22T02:23:55Z

Sources/Build/ArtifactsArchiveMetadata.swift

+extension ArtifactsArchiveMetadata: Decodable {
+    enum CodingKeys: String, CodingKey {
+        case schemaVersion
+        case artifacts = "availableArtifacts"


I just saw the example of the existing XCFrameworks format in the unit test, which uses AvailableLibraries, so now I understand where this comes from. I could go either way on this: if we expect to move to arar as the future format that can handle all types of artifacts than maybe it's more important to pick names we think are clear than to be consistent with XCFrameworks. Could go either way for me.

tomerd · 2021-02-22T02:47:51Z

@swift-ci please smoke test

abertelrud

Looks great to me! Thanks for taking this on!

tomerd requested review from abertelrud, friedbunny and neonichu as code owners February 20, 2021 00:22

tomerd added the ready Author believes the PR is ready to be merged & any feedback has been addressed label Feb 20, 2021