Skip to content

Conversation

huydhn
Copy link
Contributor

@huydhn huydhn commented Apr 12, 2024

#3020 is not needed, instead, the test-infra branch needs to be pinned to release/2.3

@huydhn huydhn requested a review from kirklandsign April 12, 2024 21:19
Copy link

pytorch-bot bot commented Apr 12, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/3021

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure

As of commit 5011977 with merge base d3326a2 (image):

NEW FAILURE - The following job has failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 12, 2024
@huydhn huydhn changed the title Fix the wrong test-infra branch on release/0.2 [Release-only] Fix the wrong test-infra branch on release/0.2 Apr 12, 2024
Copy link
Contributor

@mergennachin mergennachin left a comment

Choose a reason for hiding this comment

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

Oh awesome, thanks so much for finding this

Copy link
Contributor

@guangy10 guangy10 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, Huy!

@guangy10
Copy link
Contributor

I feel this is another example where cherry-picks override the release-only changes. Previous one is fixed in #3002. @huydhn do you have any suggestion how we can make the cherry-picking not overriding the release-only changes, or detect it somehow? cc: @dbort @mergennachin

@huydhn huydhn merged commit 31bb2ea into release/0.2 Apr 12, 2024
@huydhn
Copy link
Contributor Author

huydhn commented Apr 12, 2024

Atm, the cherry script run the command with -X theirs to avoid merge conflicts here https://github.com/pytorch/executorch/blob/main/.github/scripts/cherry_pick.py#L119. Maybe that logic could be removed to detect conflicts in this case? -X theirs means that the cherry pick PR will prefer whatever is from the original PR if there are conflicts with the release branch.

@guangy10
Copy link
Contributor

Atm, the cherry script run the command with -X theirs to avoid merge conflicts here https://github.com/pytorch/executorch/blob/main/.github/scripts/cherry_pick.py#L119. Maybe that logic could be removed to detect conflicts in this case? -X theirs means that the cherry pick PR will prefer whatever is from the original PR if there are conflicts with the release branch.

@huydhn Good to know. Thanks for the info! I think removing -X theirs may not be ideal as it may end up lots of false alarm that we have to manually address. I'm thinking of something as simple as a disallowlist where we can either reject the changes (for example if cherry-pick requests are touching .github/workflow, or version.txt, etc) or raise as merge conflict (for example if changes are in setup.py, install_requirements.sh, etc. where we could allow refactoring but would still prefer to lock down to some fixed versions for stability).

@mergennachin mergennachin mentioned this pull request Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants