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

[BRE-113] Publish SWIFT sdk releases to bitwarden/sdk-swift #956

Merged
merged 38 commits into from
Aug 26, 2024

Conversation

michalchecinski
Copy link
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/BRE-113

📔 Objective

Automate publishing SWIFT SDK to bitwarden/sdk-swift repo. Thanks @mimartin12 for the initial work on that!

📸 Screenshots

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation
    team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed
    issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

mimartin12 and others added 29 commits October 5, 2023 14:20
- Add default environment variables
- Update branch check
- Point Bitwarden actions to main
- Move github-release step to last
- Create release on sdk-swift project
Thank you!

Co-authored-by: Vince Grassia <[email protected]>
- Using the requirement of hosting these artifacts with GitHub releases, push each commit in main to the sdk-swift repo.
- Supports this implementation with requirements of GitHub releases as storage, along with pushing commits from main to sdk-swift #268 (comment)
@michalchecinski michalchecinski requested a review from a team as a code owner August 14, 2024 09:38
Copy link
Contributor

github-actions bot commented Aug 14, 2024

Logo
Checkmarx One – Scan Summary & Details615b682f-c497-4722-8b0a-622fc260b2be

New Issues

Severity Issue Source File / Package Checkmarx Insight
MEDIUM Unpinned Actions Full Length Commit SHA /build-swift.yml: 83 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-swift.yml: 70 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-swift.yml: 139 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Copy link

codecov bot commented Aug 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.96%. Comparing base (4b17741) to head (e670576).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #956      +/-   ##
==========================================
- Coverage   57.98%   57.96%   -0.02%     
==========================================
  Files         197      197              
  Lines       13647    13651       +4     
==========================================
  Hits         7913     7913              
- Misses       5734     5738       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- name: Get Package Version
id: retrieve-version
run: |
VERSION=$(grep -o '^version = ".*"' crates/bws/Cargo.toml | grep -Eo "[0-9]+\.[0-9]+\.[0-9]+")
Copy link
Member

Choose a reason for hiding this comment

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

This should be the bitwarden crate not the cli.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops.... Fixed 😃

Copy link
Member

Choose a reason for hiding this comment

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

@michalchecinski it occured to me know that the bitwarden verison is inherited from the workspace version. https://github.com/bitwarden/sdk/blob/main/Cargo.toml#L8

Copy link
Contributor Author

@michalchecinski michalchecinski Aug 19, 2024

Choose a reason for hiding this comment

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

@Hinton got it! Also, should the version for SWIFT SDK be the same as Rust crates?

Copy link
Member

Choose a reason for hiding this comment

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

I think we can use that for now.

@michalchecinski michalchecinski requested a review from Hinton August 19, 2024 10:02
Comment on lines 142 to 156
- name: Update files
run: |
# Update BitwardenFFI path
sed -i '' 's|.binaryTarget(name: "BitwardenFFI", path: "BitwardenFFI.xcframework")|.binaryTarget(\
name: "BitwardenFFI",\
url: "https://github.com/bitwarden/sdk-swift/releases/download/v${{ env._RELEASE_NAME }}/BitwardenFFI-${{ env._PKG_VERSION }}-${{ needs.validate.outputs.short_sha }}.xcframework.zip",|' sdk/languages/swift/Package.swift

# Run swiftformat
swiftformat sdk/languages/swift/Package.swift

# Copy files to local sdk-swift repo path
cp --verbose -rf sdk/languages/swift/README.md sdk-swift/README.md
cp --verbose -rf sdk/languages/swift/Package.swift sdk-swift/Package.swift
cp --verbose -rf sdk/languages/swift/Sources sdk-swift/Sources
cp --verbose -rf sdk/languages/swift/Tests sdk-swift/Tests
Copy link
Member

@Hinton Hinton Aug 19, 2024

Choose a reason for hiding this comment

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

Are we calculating the checksum somewhere?'

I usually have to edit these two lines

url: "https://bwlivefronttest.blob.core.windows.net/sdk/de3b6e2-BitwardenFFI.xcframework.zip",
checksum: "37e884d820907af85e788b5e0ab483bfc5aa0299e5d6b39865ad70659322a202"),

Copy link
Member

Choose a reason for hiding this comment

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

To calculate the value I use swift package compute-checksum <path>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought I did it... It's fixed now. :)

@michalchecinski michalchecinski requested a review from Hinton August 20, 2024 11:15
vgrassia
vgrassia previously approved these changes Aug 20, 2024
Copy link
Member

@vgrassia vgrassia left a comment

Choose a reason for hiding this comment

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

LGTM, please wait for Oscar to review.

fi

- name: Calculate swift file checksum
run: |
CHECKSUM=$(swift package compute-checksum BitwardenFFI-${{ steps.version.outputs.version }}-${{ steps.set-sha.outputs.short_sha }}.xcframework)
Copy link
Member

Choose a reason for hiding this comment

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

Is this the hash of the zip file? I always run the compute-checksum on the already zipped framework.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I thought that needs to be on pre-zipped file. I'll change that.

@michalchecinski michalchecinski requested a review from Hinton August 21, 2024 13:59
Hinton
Hinton previously approved these changes Aug 21, 2024
@michalchecinski michalchecinski enabled auto-merge (squash) August 22, 2024 11:45
@Hinton Hinton requested a review from vgrassia August 26, 2024 09:58
@michalchecinski michalchecinski merged commit 5ff95a8 into main Aug 26, 2024
57 checks passed
@michalchecinski michalchecinski deleted the BRE-113-Publish-Swift-SDK-releases-to-sdk-swift branch August 26, 2024 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants