Skip to content

Conversation

@thaystg
Copy link
Member

@thaystg thaystg commented Mar 28, 2025

Fix applying hot reload changes when the delta table has a bigger len than the baseline table.

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2396448

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)
  • src/mono/mono/component/hot_reload.c: Language not supported

@lewing
Copy link
Member

lewing commented Mar 29, 2025

failures are unrelated. can we add a test for this?

@thaystg
Copy link
Member Author

thaystg commented Mar 31, 2025

failures are unrelated. can we add a test for this?

I tried but I couldn't reproduce.

@thaystg
Copy link
Member Author

thaystg commented Apr 1, 2025

failures are unrelated. can we add a test for this?

added the test case.

@thaystg thaystg requested a review from a team April 1, 2025 16:51
Copy link
Member

@lewing lewing 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 fix the whitespace additions in src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs

@thaystg
Copy link
Member Author

thaystg commented Apr 1, 2025

/backport to release/9.0-staging

@github-actions
Copy link
Contributor

github-actions bot commented Apr 1, 2025

Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/14204570483

@github-actions
Copy link
Contributor

github-actions bot commented Apr 1, 2025

@thaystg backporting to "release/9.0-staging" failed, the patch most likely resulted in conflicts:

$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Fix applying hot reload changes when the delta table has a bigger len than the baselike table.
Applying: Fixing test failures.
Applying: Fixing assertion condition
Applying: Fix braces
Applying: Addign test case
.git/rebase-apply/patch:30117: trailing whitespace.
        [ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] 
.git/rebase-apply/patch:30126: trailing whitespace.
        [ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] 
.git/rebase-apply/patch:30135: trailing whitespace.
        [ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] 
.git/rebase-apply/patch:30144: trailing whitespace.
                
.git/rebase-apply/patch:30153: trailing whitespace.
                
warning: squelched 7 whitespace errors
warning: 12 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
M	src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj
Auto-merging src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
CONFLICT (content): Merge conflict in src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0005 Addign test case
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@thaystg
Copy link
Member Author

thaystg commented Apr 2, 2025

/ba-g BuildAnalysis malfunction, all the errors are already reported

@thaystg thaystg merged commit 2c5b3ee into dotnet:main Apr 2, 2025
106 of 112 checks passed
@@ -0,0 +1,38 @@
using System;
Copy link
Member

Choose a reason for hiding this comment

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

All .cs files in the repo should have license headers.


public static void Method2(int x2)
{
// Example body for Method2
Copy link
Member

Choose a reason for hiding this comment

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

We should not be committing 100 kBs of copilot generated boilerplate comments into the repo. This and the other file should be composed from one-liners like:

...
        public static void Method3() { }
        public static void Method4() { }
...

@github-actions github-actions bot locked and limited conversation to collaborators May 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants