Add support for ESRP Release to Maven Central #29153
Conversation
ghost
added
the
| $sourcesArtifact = $packageDetail.AssociatedArtifacts | Where-Object { ($_.Classifier -eq "sources") -and ($_.Type -eq "jar") } | ||
|
|
||
| [AssociatedArtifact[]]$additionalArtifacts = $packageDetail.AssociatedArtifacts | Where-Object { | ||
| ($_ -ne $pomArtifact) -and |
There was a problem hiding this comment.
Nit: -NotIn might be simpler here.
There was a problem hiding this comment.
This is copied from the artifact handling in Publish-MavenPackages.ps1. I'm intentionally changing little until I can make a refactor PR to unify the mvn cli argument handling between GPG signing and deployment.
| $commaDelimitedTypes = "" | ||
|
|
||
| if ($null -ne $packageDetail.AdditionalArtifacts) { | ||
| foreach($additionalArtifact in $packageDetail.AdditionalArtifacts) { |
There was a problem hiding this comment.
Nit: Probably simpler here to use an array and then use -join rather than constructing the string and then cutting the leading comma off.
There was a problem hiding this comment.
Agreed. Same as before though. I'd rather address in a refactor pr than add a different implementation in this file.
azure-sdk-for-java/eng/scripts/Publish-MavenPackages.ps1
Lines 110 to 122 in d55e2cf
|
Bypassing check enforcer. The change tested well with this pipeline run: |
|
/check-enforcer override |
| GroupID: ${{artifact.groupId}} | ||
| ArtifactDirectory: $(Pipeline.Workspace)/${{parameters.ArtifactName}}-signed | ||
| OutputDirectory: $(Pipeline.Workspace)/EsrpPackages | ||
| Target: EsrpRelease |
There was a problem hiding this comment.
Looking to see what the difference is between these two blocks it seems like we could just default things like the output and artifact name based on the Target and simply have one call to the java-publishing.yml while setting the Target via a variable we define above.
| ArtifactPath: $(Pipeline.Workspace)/${{parameters.ArtifactName}}-signed | ||
| - ${{ if eq(parameters.UseEsrpRelease, 'true') }}: | ||
| - template: /eng/pipelines/templates/steps/java-publishing.yml | ||
| parameters: |
There was a problem hiding this comment.
Similar feedback here about using a variable instead of duplicating the template parameters.
| PackageLocation: $(PackageLocation) | ||
| Owners: $(Build.RequestedForEmail) | ||
| Approvers: 'wesh@microsoft.com' |
There was a problem hiding this comment.
Is this required to be listed here? Also does this mean I need to approve all the releases? Can we use an alias or the bot account instead?
| Target: MavenCentral | ||
| BuildToolsPath: $(Pipeline.Workspace)/azure-sdk-build-tools | ||
| JavaRepoRoot: $(Pipeline.Workspace)/azure-sdk-for-java | ||
| - template: /eng/common/pipelines/templates/steps/publish-artifact.yml |
There was a problem hiding this comment.
Do we think it is worth adding this publish artifact step in the java-publishing.yml template? It seems like we are doing these 2 steps connected in the places I see here.
| function Get-RandomRepositoryDirectory() { | ||
| Write-Host "Getting random repository directory." | ||
| $randomSubDirectoryName = [System.IO.Path]::GetRandomFileName() | ||
| $randomRepositoryDirectory = New-Item -Type Directory -Path $env:TEMP -Name $randomSubDirectoryName |
There was a problem hiding this comment.
Is there a reason we want to use Temp here? As opposed to just a directory under the working directory?
| } | ||
|
|
||
| function ConvertTo-DeploymentDetails($PackageDetail) { | ||
| $pomArtifact = $packageDetail.AssociatedArtifacts | Where-Object { ($null -eq $_.Classifier) -and ($_.Type -eq "pom") } |
There was a problem hiding this comment.
nit: We should consider using .Where({...}) over | Where-Object as it will ensure you always end up with an array.
Add support for publishing individual packages to maven central via ESRP Release.
The release task has approver statically set. For now, releases are auto approved so this won't matter. We may need to find a way to have a dynamic set of approvers added to the task based on the library being released.