Fix null pointer panic in listFiles for empty repos and git worktrees #202
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fgksgf
force-pushed
the
fix-null-pointer-panic
branch
from
cb458af to
2b1a039
Compare
… invalid HEAD Fixes a panic that occurs when calling repo.Head().Hash() in empty git repositories, git worktrees, or repositories with invalid HEAD references. The issue happened in pkg/header/check.go where head could be nil, causing a null pointer dereference when calling head.Hash(). This commonly occurs in the following scenarios: - Empty git repositories with no initial commits - Git worktrees with detached HEAD or invalid branch references - Corrupted repositories with invalid HEAD pointers - New worktrees created from non-existent commits or branches Changes: - Add proper null check for head reference before calling head.Hash() - Add error handling for repo.Head() to gracefully handle edge cases - Skip git-based file discovery when repository has no commits or invalid HEAD - Add test cases for empty repositories and worktree scenarios in check_test.go - Ensure graceful fallback to glob-based file discovery in problematic git states Resolves panic at pkg/header/check.go:99 when working with empty git repositories or git worktree environments with invalid HEAD references. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
fgksgf
force-pushed
the
fix-null-pointer-panic
branch
from
2b1a039 to
6b7c669
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR prevents a nil pointer panic in listFiles by adding HEAD nil checks and error handling, and it adds tests for empty repositories and detached HEAD worktrees.
- Added nil‐check and error handling around
repo.Head()and commit lookup inpkg/header/check.go - Ensured fallback to file discovery via worktree status when HEAD is invalid
- Introduced unit tests for empty repos and detached HEAD scenarios in
pkg/header/check_test.go
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/header/check.go | Added head == nil check and error handling around repo.Head()/CommitObject to avoid panic |
| pkg/header/check_test.go | Added tests covering empty repo and detached HEAD worktree cases; imported go-git packages |
Comments suppressed due to low confidence (1)
pkg/header/check_test.go:20
- The test file uses os and filepath but these packages are not imported. Add
"os"and"path/filepath"to the import block so functions likeos.MkdirTempandfilepath.Joinresolve correctly.
import (
Comment on lines
+147
to
+149
| if len(fileList) == 0 { | ||
| t.Error("Expected to find at least one file") | ||
| } |
There was a problem hiding this comment.
[nitpick] Instead of a manual if check and t.Error, use require.NotEmpty(t, fileList) for consistency and clearer failure messages in your tests.
Suggested change
| if len(fileList) == 0 { | |
| t.Error("Expected to find at least one file") | |
| } | |
| require.NotEmpty(t, fileList, "Expected to find at least one file") |
Copilot uses AI. Check for mistakes.
| if file == nil { | ||
| return errors.New("file pointer is nil") | ||
| head, err := repo.Head() | ||
| if err != nil || head == nil { |
There was a problem hiding this comment.
[nitpick] This nested if/else block could be simplified with an early continue or extracted helper to reduce indentation and improve readability.
Copilot uses AI. Check for mistakes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a null pointer panic that occurs when using SkyWalking Eyes in empty git repositories or git worktree environments. The panic happens in
pkg/header/check.gowhen attempting to callhead.Hash()on a nil HEAD reference.Problem
The issue occurs in the following scenarios:
The original code assumed
repo.Head()would always return a valid HEAD reference, but this is not true in edge cases, leading to a panic when callinghead.Hash().Solution
head.Hash()repo.Head()to gracefully handle edge cases