Skip to content

Add Merge-Main.ps1 script#30676

Merged
moarychan merged 6 commits into
feature/spring-boot-3from
pahallis/spring-merge-script
Sep 28, 2022
Merged

Add Merge-Main.ps1 script#30676
moarychan merged 6 commits into
feature/spring-boot-3from
pahallis/spring-merge-script

Conversation

@hallipr

@hallipr hallipr commented Aug 27, 2022

Copy link
Copy Markdown
Member

Add script for merging main into the long-lived spring feature branches.

Given a feature/spring branch like:

eng/
    drift.ps1
    versioning/
        external_dependencies.txt  (with changes)
        drift.txt
sdk/
    not-spring/
         pom.xml
         drift.md
    spring/
         pom.xml
         v3.txt

And a main branch like:

eng/
    versioning/
        external_dependencies.txt  (with conflicting changes)
sdk/
    not-spring/
         pom.xml
    something-new/
         pom.xml
    spring/
         pom.xml
         v2.txt

After running Merge-Main.ps1, we would expect an in-progress merge commit like:

Changes to be committed:
        deleted:    eng/drift.ps1
        deleted:    eng/versioning/drift.txt
        deleted:    sdk/not-spring/drift.md
        modified:   sdk/not-spring/pom.xml
        new file:   sdk/something-new/pom.xml

Unmerged paths:
  (use "git add <file>..." to mark resolution)
        both modified:   eng/versioning/external_dependencies.txt

The net effect being:

  • the sdk/spring folder remained untouched even though it differs from main.
  • txt files in eng/versioning are merged from main
  • except for sdk/spring, any file not in main is deleted from the current branch
  • all other files are copied from main, overwriting the current branch
  • the merge is waiting for remaining conflicts to be manually resolved

@ghost ghost added the azure-spring All azure-spring related issues label Aug 27, 2022
@hallipr
hallipr requested a review from benbp August 27, 2022 22:04
Comment thread sdk/spring/scripts/Merge-Main.ps1 Outdated
Comment on lines +18 to +24
&git merge $Source --no-ff --no-commit

# update paths matching "theirs" except for "ours" and "merge" to the state in $Source
&git restore -s $Source --staged --worktree -- ":(top,glob)$Theirs" $ourExcludes $mergeExcludes

# update paths matching "ours" except for "merge" to their pre-merge state
&git restore -s (git rev-parse HEAD) --staged --worktree -- ":(top,glob)$Ours" $mergeExcludes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be useful to handle exit codes at each of these steps.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, is the ampersand necessary?

@hallipr hallipr Sep 1, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added an error message function and removed the ampersands

@JimSuplizio
JimSuplizio self-requested a review September 1, 2022 21:52
@moarychan moarychan added this to the 2022-09 milestone Sep 2, 2022
Comment thread sdk/spring/scripts/Merge-Main.ps1
Comment thread sdk/spring/scripts/Merge-Main.ps1 Outdated
param(
[string]$Source = 'main',
[string[]]$Theirs = @('**'), # paths to always overwrite
[string[]]$Ours = @('sdk/spring'), # paths to never merge or overwrite

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

spring-experimental should be excluded like spring, the new service directory sdk/jdbc is coming in the main branch, which may be excluded later.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added spring-experimental to $Ours.

@moarychan moarychan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, I will test the script later, thanks @hallipr

@saragluna saragluna modified the milestones: 2022-09, 2022-10 Sep 6, 2022
@hallipr
hallipr requested a review from weshaggard as a code owner September 15, 2022 00:06
@hallipr
hallipr force-pushed the pahallis/spring-merge-script branch from b45b3b0 to 9919c82 Compare September 15, 2022 00:06
@hallipr
hallipr force-pushed the pahallis/spring-merge-script branch from 9919c82 to adb4a2c Compare September 15, 2022 00:18
@hallipr
hallipr requested a review from moarychan September 15, 2022 00:35
# This script can be run locally from the root of the repo:
# .\eng\scripts\Merge-Branch.ps1 -SourceBranch 'main' -Theirs '**' -Ours 'sdk/template' -Merge 'sdk/template/ci.yml', '**/README.md'
#
# This would merge main into the local branch, making the working folder look like main. It not overwrite sdk\template.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
# This would merge main into the local branch, making the working folder look like main. It not overwrite sdk\template.
# This would merge main into the local branch, making the working folder look like main. It will not overwrite sdk\template.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed in pr #31222

[string[]]$Merge # paths to merge or overwrite
)

$theirIncludes = @($Theirs | ForEach-Object { ":(top,glob)$_" })

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What are these magic include filters? Can you link to the git documentation covering them in a comment?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Comments added to script in pr #31222

" Use `"git commit --no-edit`" to complete the merge with the default merge message`n" `
" Use `"git commit -m <message>`" to complete the merge with a custom message"

exit 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't there still potential for merge conflicts? If so we should try to detect that and output a message about those conflicts.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

git outputs the appropriate message. It appears in the output just above our "Merge commit started" message

@moarychan

Copy link
Copy Markdown
Member

@hallipr , please note I have renamed the target branch name to feature/spring-boot-3.

@moarychan

Copy link
Copy Markdown
Member

Hi @hallipr , I will sync code from main to feature/spring-boot-3 now, please help to raise a new PR if any comments should be resolved.

@moarychan
moarychan merged commit 5b57146 into feature/spring-boot-3 Sep 28, 2022
@moarychan
moarychan deleted the pahallis/spring-merge-script branch September 28, 2022 10:34
weshaggard added a commit to weshaggard/azure-sdk-tools that referenced this pull request Jun 8, 2023
Use some form of Merge-Branch.ps1 to do the partial sync

see Azure/azure-sdk-for-java#30676
see Azure/azure-rest-api-specs-pr#12958
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

azure-spring All azure-spring related issues

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants