-
Notifications
You must be signed in to change notification settings - Fork 520
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
[main] Hot Restart fixes #18317
[main] Hot Restart fixes #18317
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
<Touch AlwaysCreate="true" Files="$(HotRestartContentStampDir)$(AssemblyName).hotrestartapp.stamp" /> | ||
<Touch AlwaysCreate="true" Files="$(HotRestartAppContentDir)$(AssemblyName).hotrestartapp" /> |
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.
Could you add comments in the code explaining why these two are needed?
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.
Could you add comments in the code explaining why these two are needed?
Will add them and update the PR
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.
@rolfbjarne I've just added some comments.
Please find here more details on the deployment process (incremental VS full installation) and also on the runtime execution of the Hot Restart app, to understand better about the markers and stamps:
Outputs="$(HotRestartAppBundlePath)\_CodeSignature\CodeResources"> | ||
Outputs="@(_CodeSignHotRestartInputs -> '%(Outputs)')"> |
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.
Does this mean that code signing does not create the _CodeSignature\CodeResources
file in the app bundle?
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.
Does this mean that code signing does not create the
_CodeSignature\CodeResources
file in the app bundle?
Yes, it creates it. But for some reason it always shows me this message and never satisfies de inputs/outputs statement:
Input file "bin\Debug\net8.0-ios\ios-arm64\MauiApp35.app\Info.plist" is newer than output file "C:\Users\maagno\AppData\Local\Temp\Xamarin\HotRestart\Bundles\1.0.103.11\4fdcd990\MauiApp35.app_CodeSignature\CodeResources".
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.
Maybe an earlier task always writes Info.plist?
94131d2
to
e424ae4
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Outputs="$(HotRestartAppBundlePath)\_CodeSignature\CodeResources"> | ||
Outputs="@(_CodeSignHotRestartInputs -> '%(Outputs)')"> |
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.
Maybe an earlier task always writes Info.plist?
… work again Fixes: - Added missing app markers back: - Added .stamp files to make incremental deployments work again and to avoid re-installing the application. We use .stamp file to know which files to copy on incremental deployments and also to avoid unnecessary app installations - Added .hotrestartapp file back to identify the main app entry point. We need this since the main entry point to dynamically load the app might change between Forms and MAUI (could be a .dll or an .exe), so we need a way to let the Hot Restart app to know which is the main assembly to load - Fixed outputs in _CodesignHotRestartAppBundle target: the codesign was being executed always, causing the incremental builds to not work as expected. - Ensure the _CodesignHotRestartAppBundle target is executed before the copy of the content files and not after: Hot Restart content files doesn't affect the code signing, so they don't need to be copied before the signing process. Copying the content files before the code sign was causing unwanted behaviors and errors since the code sign logic will try to clear the signing folder before the execution, to avoid mixing old and new content
Bring latest isignsharp fix: xamarin/isignsharp@71220d4
@rolfbjarne I just updated the PR to fix the missing package version. I hope it works this time |
💻 [CI Build] Windows Integration Tests passed 💻✅ All Windows Integration Tests passed. Pipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻✅ All tests on macOS M1 - Mac Big Sur (11.5) passed. Pipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻✅ All tests on macOS M1 - Mac Ventura (13.0) passed. Pipeline on Agent |
📚 [PR Build] Artifacts 📚Packages generatedView packagesPipeline on Agent |
✅ API diff for current PR / commitLegacy Xamarin (No breaking changes)
NET (empty diffs)
✅ API diff vs stableLegacy Xamarin (No breaking changes).NET (No breaking changes)✅ Generator diffGenerator diff is empty Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🔥 [CI Build] Test results 🔥Test results❌ Tests failed on VSTS: simulator tests 0 tests crashed, 1 tests failed, 234 tests passed. Failures❌ monotouch tests [attempt 2]
Html Report (VSDrops) Download Successes✅ bcl: All 69 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
Test failure is unrelated (https://github.com/xamarin/maccore/issues/868). |
Update Hot Restart client to 1.0.119: bring latest isignsharp fix https://github.com/xamarin/isignsharp/commit/71220d490a83761a63bf005e326d3834e8b18d00
Added missing app markers back:
Fixed outputs in _CodesignHotRestartAppBundle target: the codesign was being executed always, causing the incremental builds to not work as expected.
Ensure the _CodesignHotRestartAppBundle target is executed before the copy of the content files and not after: Hot Restart content files doesn't affect the code signing, so they don't need to be copied before the signing process. Copying the content files before the code sign was causing unwanted behaviors and errors since the code sign logic will try to clear the signing folder before the execution, to avoid mixing old and new content