Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: gazelle failing on Windows with "panic: runtime error: invalid memory address or nil pointer dereference" #1872

Merged
merged 7 commits into from
May 19, 2024

Conversation

bartoszpop
Copy link
Contributor

@bartoszpop bartoszpop commented May 1, 2024

This pull request fixes bazel run gazelle failing on Windows with "panic: runtime error: invalid memory address or nil pointer dereference" if there is a path with at least 2-levels deep directory.

Fixes #1871

@bartoszpop bartoszpop requested a review from f0rmiga as a code owner May 1, 2024 15:57
Copy link

google-cla bot commented May 1, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@bartoszpop bartoszpop requested a review from rickeylev as a code owner May 1, 2024 16:13
@@ -126,7 +126,7 @@ type Configs map[string]*Config

// ParentForPackage returns the parent Config for the given Bazel package.
func (c *Configs) ParentForPackage(pkg string) *Config {
dir := filepath.Dir(pkg)
dir := path.Dir(pkg)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at the code, I am not sure it is working the way you think. path.Dir will just split at / if I understand things correctly and filepath.Path is much more sophisticated and I am surprised that this is not working correctly.

I don't have a Windows machine, but the code change looks suspicious.

Copy link
Contributor Author

@bartoszpop bartoszpop May 13, 2024

Choose a reason for hiding this comment

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

@aignas the same issue I have fixed here. My initial approach was to use cfgs[filepath.FromSlash(rel)] but a more targeted fix was proposed in this comment.

Basically, it fails because pythonconfig.go#ParentForPackage delegates to filepath.Dir(pkg) to replace the separator character "\" with "/", hence for a 2-levels deep path, e.g. rootDir\nestedDir1\nestedDir2, a parent is searched with "/" (rootDir/nestedDir1) but Configs contains a key with "\" (rootDir\nestedDir1).

Copy link
Collaborator

@aignas aignas May 13, 2024

Choose a reason for hiding this comment

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

Ah, I see, thanks for the context, it was much needed here. Could you please rebase and add Changelog notes to the Changelog.md please?

EDIT: Would it be possible to add a test for this?

@aignas
Copy link
Collaborator

aignas commented May 17, 2024

The PR looks good to me and I would love to merge it since we have cut a 0.32.2 release. Could you please rebase and update the CHANGELOG.md please?

@bartoszpop
Copy link
Contributor Author

The PR looks good to me and I would love to merge it since we have cut a 0.32.2 release. Could you please rebase and update the CHANGELOG.md please?

@aignas sorry for the delay, I'll do it by the end of this week.

@aignas aignas enabled auto-merge May 19, 2024 03:03
@aignas aignas added this pull request to the merge queue May 19, 2024
Merged via the queue into bazelbuild:main with commit 8d31c5f May 19, 2024
4 checks passed
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.

Gazelle fails on Windows with "panic: runtime error: invalid memory address or nil pointer dereference"
2 participants