Skip to content

Set default branch to master if not set from git #1558

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.

Sign up for GitHub

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

Merged
4 commits merged into from
Apr 20, 2021
Merged

Set default branch to master if not set from git #1558

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dc088b4
Set default branch to master if not set from git
sima-zhu Apr 14, 2021
5e99df1
Added log messages
sima-zhu Apr 14, 2021
c64759b
Silent the failure if not able fetch default branch from git
sima-zhu Apr 15, 2021
f615e57
Update git-push-changes.yml
sima-zhu Apr 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eng/common/pipelines/templates/steps/git-push-changes.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ steps:

- task: PowerShell@2
displayName: Push changes
condition: and(succeeded(), eq(variables['HasChanges'], 'true'))
condition: and(succeeded(), ne(variables['HasChanges'], 'false'))
Copy link
Member

@weshaggard weshaggard Apr 15, 2021

Choose a reason for hiding this comment

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

Why this change? We want to make sure we don't accidently start pushing these changes unless someone has explicitly set it to true.

Copy link
Contributor Author

@sima-zhu sima-zhu Apr 15, 2021

Choose a reason for hiding this comment

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

Several cases here:

  1. Step Check for changes executes:
  • git diff success: Then HasChange=true, we run git push on purpose.
  • git diff fail: Then HasChange=false, we skip git push on purpose.
  1. Step Check for changes skips, then HasChanges not set, we will run git push step anyway:
  • If people intended to check in some commits to branch, then they have to literally put PushArgs: -f. (This is the purpose of the changes. In my case, I fetch some commits in master, and then merge the commits into live branch)
  • If PushArgs not set, then the script will fail. We have to take care of this case which not fail previously.

Copy link
Member

@weshaggard weshaggard Apr 16, 2021

Choose a reason for hiding this comment

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

  1. That is has intended.
  2. If you set the value to skip the check for changes then you are in charge of setting HasChanges to true if there are changes that are interesting. We want to avoid blindly pushing changes in a case where someone doesn't set HasChanges at all.

As far as I'm aware this logic is working exactly has intended and if there is some scenario where you feel it is working please give me the specific case as I suspect that case needs to be address individually instead of changing the common template logic.

Copy link
Contributor Author

@sima-zhu sima-zhu Apr 19, 2021

Choose a reason for hiding this comment

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

Totally agree with you as the changes here might breaking some feature. I removed it from this PR. Will open a new one if it is needed.

inputs:
pwsh: true
workingDirectory: ${{ parameters.WorkingDirectory }}
Expand Down
7 changes: 6 additions & 1 deletion eng/common/pipelines/templates/steps/set-default-branch.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ parameters:
steps:
- pwsh: |
$setDefaultBranch = (git remote show ${{ parameters.RemoteRepo }} | Out-String) -replace "(?ms).*HEAD branch: (\w+).*", '$1'
if ($LASTEXITCODE -ne 0) {
Write-Host "Not able to fetch the default branch from git command. Set to master."
$setDefaultBranch = 'master'
}
Write-Host "Setting DefaultBranch=$setDefaultBranch"
echo "##vso[task.setvariable variable=DefaultBranch]$setDefaultBranch"
Write-Host "##vso[task.setvariable variable=DefaultBranch]$setDefaultBranch"
displayName: "Setup Default Branch"
workingDirectory: ${{ parameters.workingDirectory }}
ignoreLASTEXITCODE: true