hubcio commented on code in PR #4714: URL: https://github.com/apache/iggy/pull/4714#discussion_r4455666855
########## Package.swift: ########## @@ -0,0 +1,57 @@ +// swift-tools-version: 6.0 +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// SwiftPM resolves packages only from a repository root. CI keeps this in sync with foreign/swift/Package.swift. + +import PackageDescription + +let package = Package( + name: "iggy", + platforms: [ + .macOS(.v13), + .iOS(.v16), + .tvOS(.v16), + .watchOS(.v9), + .visionOS(.v1), + ], + products: [ + .library(name: "Iggy", targets: ["Iggy"]) + ], + dependencies: [ + .package(url: "https://github.com/apple/swift-nio.git";, from: "2.80.0"), + .package(url: "https://github.com/apple/swift-nio-ssl.git";, from: "2.29.0"), + .package(url: "https://github.com/apple/swift-log.git";, from: "1.6.0"), + .package(url: "https://github.com/apple/swift-crypto.git";, "3.0.0"..<"4.0.0"), Review Comment: warning: capping swift-crypto below 4.0.0 means an app that also pulls jwt-kit 5.8.0 (swift-crypto 4.1.0..<6.0.0) cannot resolve. the SDK only uses SHA256, so widen to `"3.0.0"..<"6.0.0"` in both manifests before the first stable tag. ########## foreign/swift/README.md: ########## @@ -36,20 +36,20 @@ The SDK depends on [SwiftNIO](https://github.com/apple/swift-nio), ## Installation -Swift Package Manager fetches a package from the root of a git repository, so the SDK -cannot be depended on straight from this monorepo's subdirectory. Add it through the -package mirror, which carries every release as a `v<version>` tag: +Swift Package Manager resolves a package from the root of a git repository, so the Iggy repository root carries a `Package.swift` that publishes this SDK. Depend on the Iggy repository itself, replacing `0.1.10` with a released version: ```swift // Package.swift dependencies: [ - .package(url: "https://github.com/apache/iggy-swift.git";, from: "0.1.1") + .package(url: "https://github.com/apache/iggy.git";, from: "0.1.10") Review Comment: nit: `from:` is up-to-next-major, so while the SDK is 0.x a `swift package update` also pulls 0.2.0 and later minors. recommend `.upToNextMinor(from: "0.1.10")` until 1.0. ########## .github/actions/swift/post-merge/action.yml: ########## @@ -0,0 +1,107 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: swift-post-merge +description: Swift post-merge tag preparation github iggy actions + +inputs: + version: + description: "Version for tagging (without the 'v' prefix)" + required: true + tag: + description: "Git tag the publish workflow creates for this version" + required: true + dry_run: + description: "Dry run mode" + required: false + default: "false" + +runs: + using: "composite" + steps: + # Build metadata cannot distinguish versions with the same SwiftPM precedence. + - name: Validate version and tag + shell: bash + env: + VERSION: ${{ inputs.version }} + TAG: ${{ inputs.tag }} + run: | + set -euo pipefail + PRERELEASE='(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)' + SEMVER="^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(-${PRERELEASE}(\.${PRERELEASE})*)?$" + if ! [[ "$VERSION" =~ $SEMVER ]]; then + echo "❌ Invalid Swift SDK version: $VERSION" + echo "Expected X.Y.Z or X.Y.Z-prerelease, without build metadata" + exit 1 + fi + if [ "$TAG" != "v${VERSION}" ]; then + echo "❌ Tag $TAG does not match version $VERSION, expected v${VERSION}" + exit 1 + fi + echo "✅ Version $VERSION, tag $TAG" + + # SwiftPM consumers receive the root manifest and legal files with the repository. + - name: Check package and legal files + shell: bash + run: | + set -euo pipefail + for file in Package.swift LICENSE NOTICE foreign/swift/LICENSE foreign/swift/NOTICE; do + if [ ! -f "$file" ]; then + echo "❌ Missing $file" + exit 1 + fi + done + echo "✅ Root manifest and legal files present" + + - name: Setup Swift + uses: ./.github/actions/utils/setup-swift + with: + cache-key: "release" + package-path: "." + + # An unpinned release build checks dependency resolution before the tag exists. + - name: Build the published package + shell: bash + run: swift build -c release --product Iggy + + - name: Summary + shell: bash + env: + VERSION: ${{ inputs.version }} + TAG: ${{ inputs.tag }} + DRY_RUN: ${{ inputs.dry_run }} + run: | + { + echo "## 🏷️ Swift Package Tag" + echo "" + echo "| Property | Value |" + echo "|----------|-------|" + echo "| **Version** | \`${VERSION}\` |" + echo "| **Git Tag** | \`${TAG}\` |" + echo "| **Package URL** | \`https://github.com/${{ github.repository }}.git\` |" + echo "" + echo "\`\`\`swift" + echo ".package(url: \"https://github.com/${{ github.repository }}.git\", exact: \"${VERSION}\")" + echo ".product(name: \"Iggy\", package: \"iggy\")" + echo "\`\`\`" + echo "" + if [ "$DRY_RUN" = "true" ]; then + echo "**Note:** Dry run, no tag will be created" + else + echo "**Note:** The publish workflow creates the tag after this step" Review Comment: nit: this note promises a tag on every non-dry run, but the action never gets `skip_tag_creation` or `should_tag`, so it can announce a tag that is never created. pass both in, or drop the note. ########## Package.swift: ########## @@ -0,0 +1,57 @@ +// swift-tools-version: 6.0 +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// SwiftPM resolves packages only from a repository root. CI keeps this in sync with foreign/swift/Package.swift. + +import PackageDescription + +let package = Package( Review Comment: warning: a cold resolve mirror-clones the whole repo, where 3352 of 4808 refs are `refs/pull/*` (`git ls-remote`), then checks out the 67 MiB tree (`git ls-tree`) for a 0.8 MiB SDK. document that cost or compare it with a swift-only repo before merge. ########## foreign/swift/README.md: ########## @@ -36,20 +36,20 @@ The SDK depends on [SwiftNIO](https://github.com/apple/swift-nio), ## Installation -Swift Package Manager fetches a package from the root of a git repository, so the SDK -cannot be depended on straight from this monorepo's subdirectory. Add it through the -package mirror, which carries every release as a `v<version>` tag: +Swift Package Manager resolves a package from the root of a git repository, so the Iggy repository root carries a `Package.swift` that publishes this SDK. Depend on the Iggy repository itself, replacing `0.1.10` with a released version: ```swift // Package.swift dependencies: [ - .package(url: "https://github.com/apache/iggy-swift.git";, from: "0.1.1") + .package(url: "https://github.com/apache/iggy.git";, from: "0.1.10") Review Comment: nit: `from: "0.1.10"` resolves nothing today - post-merge only creates `v0.1.10-edge.1`, and swiftpm skips prereleases for this range. say there is no stable release yet and show the `exact:` snapshot form until there is. ########## .github/actions/swift/post-merge/action.yml: ########## @@ -0,0 +1,107 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: swift-post-merge +description: Swift post-merge tag preparation github iggy actions + +inputs: + version: + description: "Version for tagging (without the 'v' prefix)" + required: true + tag: + description: "Git tag the publish workflow creates for this version" + required: true + dry_run: + description: "Dry run mode" + required: false + default: "false" + +runs: + using: "composite" + steps: + # Build metadata cannot distinguish versions with the same SwiftPM precedence. + - name: Validate version and tag + shell: bash + env: + VERSION: ${{ inputs.version }} + TAG: ${{ inputs.tag }} + run: | + set -euo pipefail + PRERELEASE='(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)' + SEMVER="^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(-${PRERELEASE}(\.${PRERELEASE})*)?$" + if ! [[ "$VERSION" =~ $SEMVER ]]; then + echo "❌ Invalid Swift SDK version: $VERSION" + echo "Expected X.Y.Z or X.Y.Z-prerelease, without build metadata" + exit 1 + fi + if [ "$TAG" != "v${VERSION}" ]; then + echo "❌ Tag $TAG does not match version $VERSION, expected v${VERSION}" + exit 1 + fi + echo "✅ Version $VERSION, tag $TAG" + + # SwiftPM consumers receive the root manifest and legal files with the repository. + - name: Check package and legal files + shell: bash + run: | + set -euo pipefail + for file in Package.swift LICENSE NOTICE foreign/swift/LICENSE foreign/swift/NOTICE; do + if [ ! -f "$file" ]; then + echo "❌ Missing $file" + exit 1 + fi + done + echo "✅ Root manifest and legal files present" + + - name: Setup Swift + uses: ./.github/actions/utils/setup-swift + with: + cache-key: "release" + package-path: "." + + # An unpinned release build checks dependency resolution before the tag exists. + - name: Build the published package + shell: bash + run: swift build -c release --product Iggy Review Comment: warning: with no lockfile, `swift build` runs the newest upstream `Package.swift` manifests unsandboxed on linux, in a job with `contents: write` and `id-token: write`. run this build in a job without write permissions and make the tag step depend on it. ########## foreign/swift/README.md: ########## @@ -544,3 +544,15 @@ for package in examples/swift/getting-started bdd/swift/tests; do swift format lint --strict --recursive --configuration foreign/swift/.swift-format "$package/Sources" "$package/Package.swift" done ``` + +### Releasing + +`IggyVersion.sdkVersion` in `Sources/Iggy/Iggy.swift` is the SDK version. Change it only with `scripts/bump-version.sh sdk-swift` from the repository root, which also regenerates the golden vectors that embed it: `--edge` for the next development snapshot, `--strip-edge` for the version that goes to a release vote. Review Comment: nit: `--edge` alone fails after a stable release - `compute_next_version` rejects it on a stable version like `0.1.10`. document `--patch --edge` for the first snapshot after a release. ########## foreign/swift/README.md: ########## @@ -36,20 +36,20 @@ The SDK depends on [SwiftNIO](https://github.com/apple/swift-nio), ## Installation -Swift Package Manager fetches a package from the root of a git repository, so the SDK -cannot be depended on straight from this monorepo's subdirectory. Add it through the -package mirror, which carries every release as a `v<version>` tag: +Swift Package Manager resolves a package from the root of a git repository, so the Iggy repository root carries a `Package.swift` that publishes this SDK. Depend on the Iggy repository itself, replacing `0.1.10` with a released version: ```swift // Package.swift dependencies: [ - .package(url: "https://github.com/apache/iggy-swift.git";, from: "0.1.1") + .package(url: "https://github.com/apache/iggy.git";, from: "0.1.10") ], targets: [ - .target(name: "MyApp", dependencies: [.product(name: "Iggy", package: "iggy-swift")]) + .target(name: "MyApp", dependencies: [.product(name: "Iggy", package: "iggy")]) ] ``` +Stable Swift releases use `vX.Y.Z` tags after PMC approval. An `X.Y.Z-edge.N` tag is a development snapshot of unreleased changes, not an Apache release. A version range such as `from: "0.1.10"` never selects a snapshot. The first resolve clones the whole Iggy repository, which SwiftPM then caches. Review Comment: nit: the legacy `0.0.4` to `0.0.52` tags parse as versions with no root manifest, so `from: "0.0.52"` fails until a stable `v0.1.x` tag exists. note that requirements must start at the first stable `v0.1.x` tag. ########## scripts/bump-version.sh: ########## @@ -589,6 +628,20 @@ cmd_bump() { fi done < <(get_version_files "$component") + if [[ -n "$swift_backup" ]]; then + if [[ ${#changed_files[@]} -gt 0 ]]; then + echo " Regenerating ${SWIFT_GOLDEN_FIXTURE}" + if ! regenerate_swift_golden; then Review Comment: nit: there is no `INT`/`TERM` trap here, so ctrl-c during `cargo run` leaves a bumped `Iggy.swift`, a stale `golden.json` and two temp files behind. add one trap that restores `Iggy.swift` and removes both temp files. ########## scripts/bump-version.sh: ########## @@ -563,6 +589,19 @@ cmd_bump() { echo -e "${CYAN}New:${NC} ${new_ver}" echo "" + local swift_backup="" Review Comment: simplification: drop the `Iggy.swift` backup and the temp fixture file - run the generator on `golden.json` directly like the golden lane does, and on failure call `write_version "$SWIFT_VERSION_FILE" swift "$current"`. that also stops the backup leaking when the check at line 624 returns early. ########## .github/actions/swift/pre-merge/action.yml: ########## @@ -45,6 +45,37 @@ runs: for package in examples/swift/getting-started bdd/swift/tests; do swift format lint --strict --recursive --configuration foreign/swift/.swift-format "$package/Sources" "$package/Package.swift" done + swift format lint --strict --configuration foreign/swift/.swift-format Package.swift + + # Consumers and CI must build the same library from the two manifests. + - name: Check the root manifest matches the SDK manifest + shell: bash + if: inputs.task == 'lint' + run: | + set -euo pipefail + root_manifest=$(swift package --package-path . dump-package) + sdk_manifest=$(swift package --package-path foreign/swift dump-package) + if ! jq -e '.products | length == 1 and .[0].name == "Iggy"' <<< "$root_manifest" >/dev/null \ Review Comment: simplification: the three jq gates plus the projection diff can become one jq projection per side that maps the expected `Iggy` path to a sentinel, then a single `diff`. same coverage, about 14 fewer lines. ########## scripts/bump-version.sh: ########## @@ -563,6 +589,19 @@ cmd_bump() { echo -e "${CYAN}New:${NC} ${new_ver}" echo "" + local swift_backup="" + if [[ "$component" == "sdk-swift" && $apply -eq 1 ]]; then Review Comment: nit: with `--dry-run` this skips the cargo check and leaves `golden.json` out of the file list and the `git add` hint. run the check and list the fixture in dry-run mode too. also at line 631. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
