mirror of
https://github.com/PowerShell/PowerShell
synced 2026-06-08 12:12:50 +00:00
[release/v7.6] Add merge conflict marker detection to linux-ci workflow and refactor existing actions to use reusable get-changed-files action (#26530)
This commit is contained in:
Travis Plunk
@@ -80,6 +80,12 @@ runs:
|
||||
Invoke-CIFinish
|
||||
shell: pwsh
|
||||
|
||||
- name: Install Pester
|
||||
run: |-
|
||||
Import-Module ./tools/ci.psm1
|
||||
Install-CIPester
|
||||
shell: pwsh
|
||||
|
||||
- name: Validate Package Names
|
||||
run: |-
|
||||
# Run Pester tests to validate package names
|
||||
|
||||
@@ -0,0 +1,159 @@
|
||||
---
|
||||
applyTo:
|
||||
- "**/*.ps1"
|
||||
- "**/*.psm1"
|
||||
---
|
||||
|
||||
# PowerShell Automatic Variables - Naming Guidelines
|
||||
|
||||
## Purpose
|
||||
|
||||
This instruction provides guidelines for avoiding conflicts with PowerShell's automatic variables when writing PowerShell scripts and modules.
|
||||
|
||||
## What Are Automatic Variables?
|
||||
|
||||
PowerShell has built-in automatic variables that are created and maintained by PowerShell itself. Assigning values to these variables can cause unexpected behavior and side effects.
|
||||
|
||||
## Common Automatic Variables to Avoid
|
||||
|
||||
### Critical Variables (Never Use)
|
||||
|
||||
- **`$matches`** - Contains the results of regular expression matches. Overwriting this can break regex operations.
|
||||
- **`$_`** - Represents the current object in the pipeline. Only use within pipeline blocks.
|
||||
- **`$PSItem`** - Alias for `$_`. Same rules apply.
|
||||
- **`$args`** - Contains an array of undeclared parameters. Don't use as a regular variable.
|
||||
- **`$input`** - Contains an enumerator of all input passed to a function. Don't reassign.
|
||||
- **`$LastExitCode`** - Exit code of the last native command. Don't overwrite unless intentional.
|
||||
- **`$?`** - Success status of the last command. Don't use as a variable name.
|
||||
- **`$$`** - Last token in the last line received by the session. Don't use.
|
||||
- **`$^`** - First token in the last line received by the session. Don't use.
|
||||
|
||||
### Context Variables (Use with Caution)
|
||||
|
||||
- **`$Error`** - Array of error objects. Don't replace, but can modify (e.g., `$Error.Clear()`).
|
||||
- **`$PSBoundParameters`** - Parameters passed to the current function. Read-only.
|
||||
- **`$MyInvocation`** - Information about the current command. Read-only.
|
||||
- **`$PSCmdlet`** - Cmdlet object for advanced functions. Read-only.
|
||||
|
||||
### Other Common Automatic Variables
|
||||
|
||||
- `$true`, `$false`, `$null` - Boolean and null constants
|
||||
- `$HOME`, `$PSHome`, `$PWD` - Path-related variables
|
||||
- `$PID` - Process ID of the current PowerShell session
|
||||
- `$Host` - Host application object
|
||||
- `$PSVersionTable` - PowerShell version information
|
||||
|
||||
For a complete list, see: https://learn.microsoft.com/powershell/module/microsoft.powershell.core/about/about_automatic_variables
|
||||
|
||||
## Best Practices
|
||||
|
||||
### ❌ Bad - Using Automatic Variable Names
|
||||
|
||||
```powershell
|
||||
# Bad: $matches is an automatic variable used for regex capture groups
|
||||
$matches = Select-String -Path $file -Pattern $pattern
|
||||
|
||||
# Bad: $args is an automatic variable for undeclared parameters
|
||||
$args = Get-ChildItem
|
||||
|
||||
# Bad: $input is an automatic variable for pipeline input
|
||||
$input = Read-Host "Enter value"
|
||||
```
|
||||
|
||||
### ✅ Good - Using Descriptive Alternative Names
|
||||
|
||||
```powershell
|
||||
# Good: Use descriptive names that avoid conflicts
|
||||
$matchedLines = Select-String -Path $file -Pattern $pattern
|
||||
|
||||
# Good: Use specific names for arguments
|
||||
$arguments = Get-ChildItem
|
||||
|
||||
# Good: Use specific names for user input
|
||||
$userInput = Read-Host "Enter value"
|
||||
```
|
||||
|
||||
## Naming Alternatives
|
||||
|
||||
When you encounter a situation where you might use an automatic variable name, use these alternatives:
|
||||
|
||||
| Avoid | Use Instead |
|
||||
|-------|-------------|
|
||||
| `$matches` | `$matchedLines`, `$matchResults`, `$regexMatches` |
|
||||
| `$args` | `$arguments`, `$parameters`, `$commandArgs` |
|
||||
| `$input` | `$userInput`, `$inputValue`, `$inputData` |
|
||||
| `$_` (outside pipeline) | Use a named parameter or explicit variable |
|
||||
| `$Error` (reassignment) | Don't reassign; use `$Error.Clear()` if needed |
|
||||
|
||||
## How to Check
|
||||
|
||||
### PSScriptAnalyzer Rule
|
||||
|
||||
PSScriptAnalyzer has a built-in rule that detects assignments to automatic variables:
|
||||
|
||||
```powershell
|
||||
# This will trigger PSAvoidAssignmentToAutomaticVariable
|
||||
$matches = Get-Something
|
||||
```
|
||||
|
||||
**Rule ID**: PSAvoidAssignmentToAutomaticVariable
|
||||
|
||||
### Manual Review
|
||||
|
||||
When writing PowerShell code, always:
|
||||
1. Avoid variable names that match PowerShell keywords or automatic variables
|
||||
2. Use descriptive, specific names that clearly indicate the variable's purpose
|
||||
3. Run PSScriptAnalyzer on your code before committing
|
||||
4. Review code for variable naming during PR reviews
|
||||
|
||||
## Examples from the Codebase
|
||||
|
||||
### Example 1: Regex Matching
|
||||
|
||||
```powershell
|
||||
# ❌ Bad - Overwrites automatic $matches variable
|
||||
$matches = [regex]::Matches($content, $pattern)
|
||||
|
||||
# ✅ Good - Uses descriptive name
|
||||
$regexMatches = [regex]::Matches($content, $pattern)
|
||||
```
|
||||
|
||||
### Example 2: Select-String Results
|
||||
|
||||
```powershell
|
||||
# ❌ Bad - Conflicts with automatic $matches
|
||||
$matches = Select-String -Path $file -Pattern $pattern
|
||||
|
||||
# ✅ Good - Clear and specific
|
||||
$matchedLines = Select-String -Path $file -Pattern $pattern
|
||||
```
|
||||
|
||||
### Example 3: Collecting Arguments
|
||||
|
||||
```powershell
|
||||
# ❌ Bad - Conflicts with automatic $args
|
||||
function Process-Items {
|
||||
$args = $MyItems
|
||||
# ... process items
|
||||
}
|
||||
|
||||
# ✅ Good - Descriptive parameter name
|
||||
function Process-Items {
|
||||
[CmdletBinding()]
|
||||
param(
|
||||
[Parameter(ValueFromRemainingArguments)]
|
||||
[string[]]$Items
|
||||
)
|
||||
# ... process items
|
||||
}
|
||||
```
|
||||
|
||||
## References
|
||||
|
||||
- [PowerShell Automatic Variables Documentation](https://learn.microsoft.com/powershell/module/microsoft.powershell.core/about/about_automatic_variables)
|
||||
- [PSScriptAnalyzer Rules](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/docs/Rules/README.md)
|
||||
- [PowerShell Best Practices](https://learn.microsoft.com/powershell/scripting/developer/cmdlet/strongly-encouraged-development-guidelines)
|
||||
|
||||
## Summary
|
||||
|
||||
**Key Takeaway**: Always use descriptive, specific variable names that clearly indicate their purpose and avoid conflicts with PowerShell's automatic variables. When in doubt, choose a longer, more descriptive name over a short one that might conflict.
|
||||
@@ -0,0 +1,201 @@
|
||||
---
|
||||
applyTo:
|
||||
- "tools/ci.psm1"
|
||||
- "build.psm1"
|
||||
- "tools/packaging/**/*.psm1"
|
||||
- ".github/**/*.yml"
|
||||
- ".github/**/*.yaml"
|
||||
---
|
||||
|
||||
# Guidelines for PowerShell Code Organization
|
||||
|
||||
## When to Move Code from YAML to PowerShell Modules
|
||||
|
||||
PowerShell code in GitHub Actions YAML files should be kept minimal. Move code to a module when:
|
||||
|
||||
### Size Threshold
|
||||
- **More than ~30 lines** of PowerShell in a YAML file step
|
||||
- **Any use of .NET types** like `[regex]`, `[System.IO.Path]`, etc.
|
||||
- **Complex logic** requiring multiple nested loops or conditionals
|
||||
- **Reusable functionality** that might be needed elsewhere
|
||||
|
||||
### Indicators to Move Code
|
||||
1. Using .NET type accelerators (`[regex]`, `[PSCustomObject]`, etc.)
|
||||
2. Complex string manipulation or parsing
|
||||
3. File system operations beyond basic reads/writes
|
||||
4. Logic that would benefit from unit testing
|
||||
5. Code that's difficult to read/maintain in YAML format
|
||||
|
||||
## Which Module to Use
|
||||
|
||||
### ci.psm1 (`tools/ci.psm1`)
|
||||
**Purpose**: CI/CD-specific operations and workflows
|
||||
|
||||
**Use for**:
|
||||
- Build orchestration (invoking builds, tests, packaging)
|
||||
- CI environment setup and configuration
|
||||
- Test execution and result processing
|
||||
- Artifact handling and publishing
|
||||
- CI-specific validations and checks
|
||||
- Environment variable management for CI
|
||||
|
||||
**Examples**:
|
||||
- `Invoke-CIBuild` - Orchestrates build process
|
||||
- `Invoke-CITest` - Runs Pester tests
|
||||
- `Test-MergeConflictMarker` - Validates files for conflicts
|
||||
- `Set-BuildVariable` - Manages CI variables
|
||||
|
||||
**When NOT to use**:
|
||||
- Core build operations (use build.psm1)
|
||||
- Package creation logic (use packaging.psm1)
|
||||
- Platform-specific build steps
|
||||
|
||||
### build.psm1 (`build.psm1`)
|
||||
**Purpose**: Core build operations and utilities
|
||||
|
||||
**Use for**:
|
||||
- Compiling source code
|
||||
- Resource generation
|
||||
- Build configuration management
|
||||
- Core build utilities (New-PSOptions, Get-PSOutput, etc.)
|
||||
- Bootstrap operations
|
||||
- Cross-platform build helpers
|
||||
|
||||
**Examples**:
|
||||
- `Start-PSBuild` - Main build function
|
||||
- `Start-PSBootstrap` - Bootstrap dependencies
|
||||
- `New-PSOptions` - Create build configuration
|
||||
- `Start-ResGen` - Generate resources
|
||||
|
||||
**When NOT to use**:
|
||||
- CI workflow orchestration (use ci.psm1)
|
||||
- Package creation (use packaging.psm1)
|
||||
- Test execution
|
||||
|
||||
### packaging.psm1 (`tools/packaging/packaging.psm1`)
|
||||
**Purpose**: Package creation and distribution
|
||||
|
||||
**Use for**:
|
||||
- Creating distribution packages (MSI, RPM, DEB, etc.)
|
||||
- Package-specific metadata generation
|
||||
- Package signing operations
|
||||
- Platform-specific packaging logic
|
||||
|
||||
**Examples**:
|
||||
- `Start-PSPackage` - Create packages
|
||||
- `New-MSIPackage` - Create Windows MSI
|
||||
- `New-DotnetSdkContainerFxdPackage` - Create container packages
|
||||
|
||||
**When NOT to use**:
|
||||
- Building binaries (use build.psm1)
|
||||
- Running tests (use ci.psm1)
|
||||
- General utilities
|
||||
|
||||
## Best Practices
|
||||
|
||||
### Keep YAML Minimal
|
||||
```yaml
|
||||
# ❌ Bad - too much logic in YAML
|
||||
- name: Check files
|
||||
shell: pwsh
|
||||
run: |
|
||||
$files = Get-ChildItem -Recurse
|
||||
foreach ($file in $files) {
|
||||
$content = Get-Content $file -Raw
|
||||
if ($content -match $pattern) {
|
||||
# ... complex processing ...
|
||||
}
|
||||
}
|
||||
|
||||
# ✅ Good - call function from module
|
||||
- name: Check files
|
||||
shell: pwsh
|
||||
run: |
|
||||
Import-Module ./tools/ci.psm1
|
||||
Test-SomeCondition -Path ${{ github.workspace }}
|
||||
```
|
||||
|
||||
### Document Functions
|
||||
Always include comment-based help for functions:
|
||||
```powershell
|
||||
function Test-MyFunction
|
||||
{
|
||||
<#
|
||||
.SYNOPSIS
|
||||
Brief description
|
||||
.DESCRIPTION
|
||||
Detailed description
|
||||
.PARAMETER ParameterName
|
||||
Parameter description
|
||||
.EXAMPLE
|
||||
Test-MyFunction -ParameterName Value
|
||||
#>
|
||||
[CmdletBinding()]
|
||||
param(
|
||||
[Parameter(Mandatory)]
|
||||
[string] $ParameterName
|
||||
)
|
||||
# Implementation
|
||||
}
|
||||
```
|
||||
|
||||
### Error Handling
|
||||
Use proper error handling in modules:
|
||||
```powershell
|
||||
try {
|
||||
# Operation
|
||||
}
|
||||
catch {
|
||||
Write-Error "Detailed error message: $_"
|
||||
throw
|
||||
}
|
||||
```
|
||||
|
||||
### Verbose Output
|
||||
Use `Write-Verbose` for debugging information:
|
||||
```powershell
|
||||
Write-Verbose "Processing file: $filePath"
|
||||
```
|
||||
|
||||
## Module Dependencies
|
||||
|
||||
- **ci.psm1** imports both `build.psm1` and `packaging.psm1`
|
||||
- **build.psm1** is standalone (minimal dependencies)
|
||||
- **packaging.psm1** imports `build.psm1`
|
||||
|
||||
When adding new functions, consider these import relationships to avoid circular dependencies.
|
||||
|
||||
## Testing Modules
|
||||
|
||||
Functions in modules should be testable:
|
||||
```powershell
|
||||
# Test locally
|
||||
Import-Module ./tools/ci.psm1 -Force
|
||||
Test-MyFunction -Parameter Value
|
||||
|
||||
# Can be unit tested with Pester
|
||||
Describe "Test-MyFunction" {
|
||||
It "Should return expected result" {
|
||||
# Test implementation
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Migration Checklist
|
||||
|
||||
When moving code from YAML to a module:
|
||||
|
||||
1. ✅ Determine which module is appropriate (ci, build, or packaging)
|
||||
2. ✅ Create function with proper parameter validation
|
||||
3. ✅ Add comment-based help documentation
|
||||
4. ✅ Use `[CmdletBinding()]` for advanced function features
|
||||
5. ✅ Include error handling
|
||||
6. ✅ Add verbose output for debugging
|
||||
7. ✅ Test the function independently
|
||||
8. ✅ Update YAML to call the new function
|
||||
9. ✅ Verify the workflow still works end-to-end
|
||||
|
||||
## References
|
||||
|
||||
- PowerShell Advanced Functions: https://learn.microsoft.com/powershell/module/microsoft.powershell.core/about/about_functions_advanced
|
||||
- Comment-Based Help: https://learn.microsoft.com/powershell/scripting/developer/help/writing-help-for-windows-powershell-scripts-and-functions
|
||||
@@ -67,6 +67,20 @@ jobs:
|
||||
with:
|
||||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
|
||||
merge_conflict_check:
|
||||
name: Check for Merge Conflict Markers
|
||||
runs-on: ubuntu-latest
|
||||
if: github.event_name == 'pull_request' && (startsWith(github.repository_owner, 'azure') || github.repository_owner == 'PowerShell')
|
||||
permissions:
|
||||
pull-requests: read
|
||||
contents: read
|
||||
steps:
|
||||
- name: checkout
|
||||
uses: actions/checkout@v5
|
||||
|
||||
- name: Check for merge conflict markers
|
||||
uses: "./.github/actions/infrastructure/merge-conflict-checker"
|
||||
|
||||
ci_build:
|
||||
name: Build PowerShell
|
||||
runs-on: ubuntu-latest
|
||||
@@ -162,6 +176,48 @@ jobs:
|
||||
runner_os: ubuntu-latest
|
||||
test_results_artifact_name: testResults-xunit
|
||||
|
||||
infrastructure_tests:
|
||||
name: Infrastructure Tests
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: checkout
|
||||
uses: actions/checkout@v5
|
||||
with:
|
||||
fetch-depth: 1
|
||||
|
||||
- name: Install Pester
|
||||
shell: pwsh
|
||||
run: |
|
||||
Import-Module ./tools/ci.psm1
|
||||
Install-CIPester
|
||||
|
||||
- name: Run Infrastructure Tests
|
||||
shell: pwsh
|
||||
run: |
|
||||
$testResultsFolder = Join-Path $PWD "testResults"
|
||||
New-Item -ItemType Directory -Path $testResultsFolder -Force | Out-Null
|
||||
|
||||
$config = New-PesterConfiguration
|
||||
$config.Run.Path = './test/infrastructure/'
|
||||
$config.Run.PassThru = $true
|
||||
$config.TestResult.Enabled = $true
|
||||
$config.TestResult.OutputFormat = 'NUnitXml'
|
||||
$config.TestResult.OutputPath = "$testResultsFolder/InfrastructureTests.xml"
|
||||
$config.Output.Verbosity = 'Detailed'
|
||||
|
||||
$result = Invoke-Pester -Configuration $config
|
||||
|
||||
if ($result.FailedCount -gt 0 -or $result.Result -eq 'Failed') {
|
||||
throw "Infrastructure tests failed"
|
||||
}
|
||||
|
||||
- name: Publish Test Results
|
||||
uses: "./.github/actions/test/process-pester-results"
|
||||
if: always()
|
||||
with:
|
||||
name: "InfrastructureTests"
|
||||
testResultsFolder: "${{ github.workspace }}/testResults"
|
||||
|
||||
## Temporarily disable the CodeQL analysis on Linux as it doesn't work for .NET SDK 10-rc.2.
|
||||
# analyze:
|
||||
# name: CodeQL Analysis
|
||||
@@ -173,22 +229,6 @@ jobs:
|
||||
# contents: read
|
||||
# security-events: write
|
||||
# with:
|
||||
# runner_os: ubuntu-latest
|
||||
|
||||
ready_to_merge:
|
||||
name: Linux ready to merge
|
||||
needs:
|
||||
- xunit_tests
|
||||
- linux_test_elevated_ci
|
||||
- linux_test_elevated_others
|
||||
- linux_test_unelevated_ci
|
||||
- linux_test_unelevated_others
|
||||
# - analyze
|
||||
- linux_packaging
|
||||
if: always()
|
||||
uses: PowerShell/compliance/.github/workflows/ready-to-merge.yml@v1.0.0
|
||||
with:
|
||||
needs_context: ${{ toJson(needs) }}
|
||||
linux_packaging:
|
||||
name: Linux Packaging
|
||||
needs:
|
||||
@@ -203,3 +243,20 @@ jobs:
|
||||
fetch-depth: 0
|
||||
- name: Linux Packaging
|
||||
uses: "./.github/actions/test/linux-packaging"
|
||||
|
||||
ready_to_merge:
|
||||
name: Linux ready to merge
|
||||
needs:
|
||||
- xunit_tests
|
||||
- linux_test_elevated_ci
|
||||
- linux_test_elevated_others
|
||||
- linux_test_unelevated_ci
|
||||
- linux_test_unelevated_others
|
||||
- linux_packaging
|
||||
- merge_conflict_check
|
||||
- infrastructure_tests
|
||||
# - analyze
|
||||
if: always()
|
||||
uses: PowerShell/compliance/.github/workflows/ready-to-merge.yml@v1.0.0
|
||||
with:
|
||||
needs_context: ${{ toJson(needs) }}
|
||||
|
||||
@@ -191,6 +191,14 @@ jobs:
|
||||
$macOSRuntime = if ([System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture -eq 'Arm64') { 'osx-arm64' } else { 'osx-x64' }
|
||||
Start-PSPackage -Type osxpkg -ReleaseTag $releaseTag -MacOSRuntime $macOSRuntime -SkipReleaseChecks
|
||||
shell: pwsh
|
||||
|
||||
- name: Install Pester
|
||||
if: success()
|
||||
run: |-
|
||||
Import-Module ./tools/ci.psm1
|
||||
Install-CIPester
|
||||
shell: pwsh
|
||||
|
||||
- name: Test package contents
|
||||
if: success()
|
||||
run: |-
|
||||
|
||||
Reference in New Issue
Block a user