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

[CP] Update vm_snapshot_analysis to 0.7.6 #133658

Closed
sstrickl opened this issue Aug 30, 2023 · 11 comments
Closed

[CP] Update vm_snapshot_analysis to 0.7.6 #133658

sstrickl opened this issue Aug 30, 2023 · 11 comments
Assignees
Labels
cp: approved Approved cherry-pick request cp: merge-to-stable Cherry-picks that should be merged to stable cp: merged Cherry-pick has been merged to the release branch. cp: review Cherry-picks in the review queue

Comments

@sstrickl
Copy link
Contributor

Issue Link

#132695

Commit Hash

3b8f6c4

Target

stable

PR Link

#133657

Changelog Description

Build fails when using the --analyze-size argument.

Impacted Users

All Flutter developers

Impact Description

When using the --analyze-size argument, builds fail with a crash in package:vm_snapshot_analysis/v8_profile.dart.

Workaround

Manually updating the flutter package dependencies or using the beta branch.

Risk

low

Test Coverage

yes

Validation Steps

No response

@sstrickl sstrickl added the cp: review Cherry-picks in the review queue label Aug 30, 2023
@sstrickl sstrickl changed the title [CP] <title> [CP] Update vm_snapshot_analysis to 0.7.6 Aug 30, 2023
@sstrickl
Copy link
Contributor Author

Note that this isn't a direct cherry-pick of 3b8f6c4, but only updates the pinned vm_snapshot_analysis version.

@itsjustkevin
Copy link
Contributor

@sstrickl is there any reason why we wouldn’t pick the entire change?

Have we tested this cherry-pick with the full DEPS roll?

Reaching out to @christopherfujino for initial review.

@sstrickl
Copy link
Contributor Author

The only reason I didn't just cherry pick 3b8f6c4 was just trying to limit the amount of package changes hitting stable, when only the one for vm_snapshot_analysis is needed to fix the issue linked above. However, that change landed the day after stable was forked, so I would be surprised if it actually caused any issues. I'm happy to change the PR to just cherry pick that change wholesale instead if that would be preferred.

@itsjustkevin
Copy link
Contributor

Thanks for explaining @sstrickl! We already cut for the next release, so we have time to work though the pros and cons of each approach 🙂.

@christopherfujino
Copy link
Member

The only reason I didn't just cherry pick 3b8f6c4 was just trying to limit the amount of package changes hitting stable, when only the one for vm_snapshot_analysis is needed to fix the issue linked above. However, that change landed the day after stable was forked, so I would be surprised if it actually caused any issues. I'm happy to change the PR to just cherry pick that change wholesale instead if that would be preferred.

@sstrickl I think you did the right thing. Also, I love your avatar. @itsjustkevin I'll add an action item for our next 1:1 to discuss codifying exactly how to handle these situations.

@CaseyHillers CaseyHillers removed their assignment Aug 30, 2023
@christopherfujino
Copy link
Member

@itsjustkevin I think this slipped through the cracks and was missed for the 3.13.3 stable release.

@itsjustkevin
Copy link
Contributor

@christopherfujino and @sstrickl this did slip through and that is 100% on me. So we are aligned on submitting the cherry-pick as is?

@sstrickl
Copy link
Contributor Author

sstrickl commented Sep 8, 2023

Yep, sounds like it :)

@christopherfujino
Copy link
Member

@christopherfujino and @sstrickl this did slip through and that is 100% on me. So we are aligned on submitting the cherry-pick as is?

Yes, looks good to go

@itsjustkevin itsjustkevin added cp: approved Approved cherry-pick request cp: merge-to-stable Cherry-picks that should be merged to stable cp: merged Cherry-pick has been merged to the release branch. labels Sep 12, 2023
@itsjustkevin itsjustkevin reopened this Sep 13, 2023
@itsjustkevin
Copy link
Contributor

Closing this once it is released.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cp: approved Approved cherry-pick request cp: merge-to-stable Cherry-picks that should be merged to stable cp: merged Cherry-pick has been merged to the release branch. cp: review Cherry-picks in the review queue
Projects
None yet
Development

No branches or pull requests

5 participants