[PM-9022] scaffold the extension and build pipeline #9948
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Electron builder modifies the .plist in the extension which causes issues with the signing process. Copying and re-signing manually avoids this because it bypasses the electron builder for the extension
coroiu
commented
Jul 3, 2024
| @@ -68,6 +68,7 @@ bitwarden_license/bit-web/src/app/billing @bitwarden/team-billing-dev | |||
| ## Platform team files ## | |||
| apps/browser/src/platform @bitwarden/team-platform-dev | |||
| apps/cli/src/platform @bitwarden/team-platform-dev | |||
| apps/desktop/macos @bitwarden/team-platform-dev | |||
There was a problem hiding this comment.
thought: we might want to assign some this folder to team-autofill-dev
There was a problem hiding this comment.
Added autofill as owner of the autofill-extension
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #9948 +/- ##
=======================================
Coverage 33.48% 33.48%
=======================================
Files 2845 2845
Lines 89046 89046
Branches 16982 16982
=======================================
Hits 29817 29817
Misses 56889 56889
Partials 2340 2340 ☔ View full report in Codecov by Sentry. |
|
New Issues
Fixed Issues
|
Hinton
reviewed
Jul 8, 2024
| if (copyPlugIn) { | ||
| let shouldResign = false; | ||
|
|
||
| // cannot use extraFiles because it modifies the extensions .plist and makes it invalid |
There was a problem hiding this comment.
question: Can you elaborate a bit on what modifications they do?
I was hoping we could finally get rid of the after sign script and re-evaluate if newer electron builders supports this out of the box.
There was a problem hiding this comment.
It adds an Electron ASAR section, and I couldn't find a way to selectively disable ASAR for extraFiles
I'm honestly not entirely sure why that invalidates the .plist file since we should be re-signing the whole plugin. But nonetheless it fails to sign it with the error message saying that .plist was modified
There was a problem hiding this comment.
Should we open a issue for this?
There was a problem hiding this comment.
I'm not entirely sure what the issue is so we'd have put in a fair amount of work just to report it. I really don't wanna do that right now
There was a problem hiding this comment.
question: What is the desktop project?
There was a problem hiding this comment.
This is just a project used with xcodebuild to build the extension. Why? Are there simpler ways?
apps/desktop/macos/autofill-extension/autofill_extension.entitlements
Outdated
Show resolved
Hide resolved
Hinton
reviewed
Jul 8, 2024
|
@Hinton sorry I re-requested a review from you before I noticed that our dev build is failing due to a missing provisioning profile |
Hinton
previously approved these changes
Jul 10, 2024
This way we don't include the extension in any build and can avoid the signing issues it brings
coltonhurst
previously approved these changes
Nov 11, 2024
Hinton
reviewed
Nov 11, 2024
There was a problem hiding this comment.
You will want to add xcuserdata to gitignore.
| if (copyPlugIn) { | ||
| let shouldResign = false; | ||
|
|
||
| // cannot use extraFiles because it modifies the extensions .plist and makes it invalid |
There was a problem hiding this comment.
Should we open a issue for this?
vgrassia
approved these changes
Nov 13, 2024
coroiu
added a commit
that referenced
this pull request
Nov 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
PM-9022 [Desktop][Swift Extension] Scaffold the extension and build pipeline
📔 Objective
Only add the extension for locally built MAS DEV buildsThe extension is no longer included in any build to avoid the signing issues⏰ Reminders before review
🦮 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