Skip to content

Also output resolved paths for symlinks#92

Merged
MusicalNinjaDad merged 4 commits into
mainfrom
symlink
Nov 4, 2025
Merged

Also output resolved paths for symlinks#92
MusicalNinjaDad merged 4 commits into
mainfrom
symlink

Conversation

@MusicalNinjaDad

@MusicalNinjaDad MusicalNinjaDad commented Nov 4, 2025

Copy link
Copy Markdown
Owner

Summary by Sourcery

Display resolved filesystem paths for symlinks in output logs and test expectations.

Enhancements:

  • Include resolved symlink targets in test expected stdout for binaries, interpreters, and library dependencies
  • Show original and resolved source paths when linking files in the logging output
  • Introduce P_ld_linux_resolved to represent the resolved interpreter path

Chores:

  • Update CHANGELOG to reflect symlink resolution output changes

@sourcery-ai

sourcery-ai Bot commented Nov 4, 2025

Copy link
Copy Markdown

Reviewer's Guide

This PR enhances link output by including resolved symlink paths in both runtime logging and test expectations by conditionally appending the original and resolved paths.

Sequence diagram for enhanced symlink resolution in link logging

sequenceDiagram
    participant "link()"
    participant "filepath.EvalSymlinks"
    participant "log.Default()"
    "link()"->>"filepath.EvalSymlinks": Resolve sourcePath
    "link()"->>"log.Default()": Print operation with original and resolved sourcePath if different
Loading

Sequence diagram for test output generation with resolved symlink paths

sequenceDiagram
    participant "generateOutput()"
    participant "filepath.EvalSymlinks"
    participant "TestCase"
    "generateOutput()"->>"filepath.EvalSymlinks": Resolve lib path
    "generateOutput()"->>"TestCase": Append lib (resolved) -> destination to ExpectedStdout
Loading

Class diagram for updated symlink and path resolution logic

classDiagram
    class TestDetails {
        +string Path
        +bool Symlink
        +BinDetails Bin
        +string SnagTo
        +string SnagAs
    }
    class BinDetails {
        +ElfDetails Elf
        +bool HasInterpreter
    }
    class ElfDetails {
        +string Path
        +string Interpreter
        +[]string Dependencies
    }
    class TestCase {
        +[]string ExpectedStdout
        +map[string]string ExpectedFiles
        +string Dest
    }
    TestDetails --> BinDetails
    BinDetails --> ElfDetails
    TestCase --> TestDetails

    class Paths {
        +const P_ld_linux
        +var P_ld_linux_resolved
    }
Loading

File-Level Changes

Change Details Files
Include resolved source paths in test expected stdout for binaries, interpreters, and dependencies
  • Add conditional for symlinked binaries to include Elf.Path
  • Conditionally include resolved interpreter path if it differs
  • Evaluate and conditionally include resolved dependency paths
internal/testing/testiterator.go
Enhance link() logging to report resolved symlink source
  • Add conditional to log originalSourcePath alongside resolved sourcePath when they differ
snaggle.go
Introduce resolved interpreter path constant
  • Add P_ld_linux_resolved variable initialized via filepath.EvalSymlinks
internal/paths.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@MusicalNinjaDad MusicalNinjaDad linked an issue Nov 4, 2025 that may be closed by this pull request
@codecov

codecov Bot commented Nov 4, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.41379% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.94%. Comparing base (6e5b83d) to head (6d07c66).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/testing/testiterator.go 68.00% 6 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #92      +/-   ##
==========================================
- Coverage   73.39%   72.94%   -0.45%     
==========================================
  Files          12       12              
  Lines         639      658      +19     
==========================================
+ Hits          469      480      +11     
- Misses        117      123       +6     
- Partials       53       55       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `internal/paths.go:14` </location>
<code_context>
 // Path to interpreter
 const P_ld_linux = "/lib64/ld-linux-x86-64.so.2"

+var P_ld_linux_resolved, _ = filepath.EvalSymlinks(P_ld_linux)
+
 // Paths to common libraries
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Consider handling errors from EvalSymlinks when initializing P_ld_linux_resolved.

Explicitly handling the EvalSymlinks error will help avoid silent failures and improve reliability. Consider logging the error or falling back to P_ld_linux if resolution fails.

Suggested implementation:

```golang
var P_ld_linux_resolved = resolveLdLinux()

func resolveLdLinux() string {
	resolved, err := filepath.EvalSymlinks(P_ld_linux)
	if err != nil {
		// Log the error and fall back to the original path
		fmt.Fprintf(os.Stderr, "Warning: failed to resolve symlink for %s: %v\n", P_ld_linux, err)
		return P_ld_linux
	}
	return resolved
}

```

You will need to import the "fmt" and "os" packages at the top of the file if they are not already imported:

```go
import (
	"fmt"
	"os"
	"path/filepath"
)
```
</issue_to_address>

### Comment 2
<location> `internal/testing/testiterator.go:227` </location>
<code_context>
 func generateOutput(bin TestDetails, tc *TestCase, inplace bool) {
 	if !inplace {
 		snaggedBin := filepath.Join(tc.Dest, bin.SnagTo, bin.SnagAs)
</code_context>

<issue_to_address>
**issue (complexity):** Consider extracting the repeated mapping logic into a helper function to simplify generateOutput.

```go
// helper to reduce duplication in generateOutput
func formatMapping(src, hint, dest string) string {
    if hint != "" && hint != src {
        return fmt.Sprintf("%s (%s) -> %s", src, hint, dest)
    }
    return fmt.Sprintf("%s -> %s", src, dest)
}

func generateOutput(bin TestDetails, tc *TestCase, inplace bool) {
    if !inplace {
        snagged := filepath.Join(tc.Dest, bin.SnagTo, bin.SnagAs)
        hint := ""
        if bin.Symlink {
            hint = bin.Bin.Elf.Path
        }
        tc.ExpectedStdout = append(tc.ExpectedStdout,
            formatMapping(bin.Path, hint, snagged),
        )
        tc.ExpectedFiles[bin.Path] = snagged
    }

    if bin.Bin.HasInterpreter {
        snagged := filepath.Join(tc.Dest, bin.Bin.Elf.Interpreter)
        hint := P_ld_linux_resolved
        if P_ld_linux == P_ld_linux_resolved {
            hint = ""
        }
        tc.ExpectedStdout = append(tc.ExpectedStdout,
            formatMapping(bin.Bin.Elf.Interpreter, hint, snagged),
        )
        tc.ExpectedFiles[bin.Bin.Elf.Interpreter] = snagged
    }

    for _, lib := range bin.Bin.Elf.Dependencies {
        snagged := filepath.Join(tc.Dest, "lib64", filepath.Base(lib))
        resolved, _ := filepath.EvalSymlinks(lib)
        hint := resolved
        if lib == resolved {
            hint = ""
        }
        tc.ExpectedStdout = append(tc.ExpectedStdout,
            formatMapping(lib, hint, snagged),
        )
        tc.ExpectedFiles[lib] = snagged
    }
}
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@MusicalNinjaDad MusicalNinjaDad enabled auto-merge (squash) November 4, 2025 16:00
@MusicalNinjaDad MusicalNinjaDad merged commit 9e37153 into main Nov 4, 2025
12 of 14 checks passed
@MusicalNinjaDad MusicalNinjaDad deleted the symlink branch November 4, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improved stdout from link()

1 participant