Skip to content

rename some vars to prevent shadowing "path" import - #35

Merged
thaJeztah merged 1 commit into
moby:mainfrom
thaJeztah:less_shadow
Jul 15, 2026
Merged

rename some vars to prevent shadowing "path" import#35
thaJeztah merged 1 commit into
moby:mainfrom
thaJeztah:less_shadow

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

No description provided.

Comment thread archive.go
return breakoutError(fmt.Errorf("invalid symlink %q -> %q", dstPath, hdr.Linkname))
}
if err := os.Symlink(hdr.Linkname, path); err != nil {
if err := os.Symlink(hdr.Linkname, dstPath); err != nil {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yup; this needs fixing, but unrelated.

Comment thread archive.go
@@ -861,25 +861,25 @@ loop:
// The only exception is when it is a directory *and* the file from
// the layer is also a directory. Then we want to merge them (i.e.
// just apply the metadata from the layer).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yup; this needs fixing, but unrelated.

@codecov-commenter

codecov-commenter commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 59.37500% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.04%. Comparing base (2cd730e) to head (6f8816a).
⚠️ Report is 33 commits behind head on main.

Files with missing lines Patch % Lines
archive.go 56.81% 6 Missing and 13 partials ⚠️
xattr_supported.go 50.00% 5 Missing ⚠️
archive_unix.go 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #35      +/-   ##
==========================================
- Coverage   66.35%   65.04%   -1.32%     
==========================================
  Files          42       42              
  Lines        2027     2031       +4     
==========================================
- Hits         1345     1321      -24     
- Misses        497      535      +38     
+ Partials      185      175      -10     

☔ View full report in Codecov by Harness.
📢 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.

@thaJeztah
thaJeztah requested review from tonistiigi and vvoland July 14, 2026 18:21
@thaJeztah

Copy link
Copy Markdown
Member Author

@AkihiroSuda if you have time for quick LGTM on this one; CodeQL failures are expected; I'm working on reviewing some of the other PRs related to that, so this is to make the way free for using path.xxx functions in some places.

Comment thread archive.go Outdated
return breakoutError(fmt.Errorf("invalid hardlink %q -> %q", targetPath, hdr.Linkname))
}
if err := os.Link(targetPath, path); err != nil {
if err := os.Link(targetPath, dstPath); err != nil {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

kinda confusing to have both "targetPath" and "dstPath"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hm.. yeah, perhaps linkTarget for the existing one?

@AkihiroSuda

Copy link
Copy Markdown
Member

Alternatively import "path" could be changed to import pkgpath "path"

@thaJeztah

Copy link
Copy Markdown
Member Author

Yeah, was considering that as alternative; that said, there's various places where path also wasn't very descriptive.

Oh! Of course now needs a rebase 😂

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
thaJeztah merged commit f83d63b into moby:main Jul 15, 2026
11 of 12 checks passed
@thaJeztah
thaJeztah deleted the less_shadow branch July 15, 2026 07:47
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.

5 participants