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.

Sign up for GitHub

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

Jump to bottom

cmd/go/internal/load: prevent calling ImportErrorf when the err is *module.InvalidPathError #50089

Closed
wants to merge 3 commits into from
Closed

cmd/go/internal/load: prevent calling ImportErrorf when the err is *module.InvalidPathError #50089

wants to merge 3 commits into from

Conversation

SilverRainZ
Copy link
Contributor

  1. The call of setError is unnecessary bacause setError do nothing when
    p.Error is not nil.
  2. The call of ImportErrorf may panic in this situation
    (double quotes appear in the import path).

Fixes #49137

@SilverRainZ
Copy link
Contributor Author

Why this PR is not imported to Gerrit?

@ianlancetaylor
Copy link
Member

Why this PR is not imported to Gerrit?

See https://golang.org/issue/49696.

@gopherbot
Copy link
Contributor

This PR (HEAD: df89974) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/372398 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Bryan Mills:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 5cbf11a) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/372398 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@SilverRainZ
Copy link
Contributor Author

@gopherbot /comments Regression test has been added : )

@SilverRainZ
Copy link
Contributor Author

@bcmills Any progress?

@gopherbot
Copy link
Contributor

Message from 张盛宇:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from 张盛宇:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan Mills:

Patch Set 2: Run-TryBot+1 Trust+1

(4 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 2: TryBot-Result-1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from 张盛宇:

Patch Set 2:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from 张盛宇:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 7b23d67) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/372398 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from 张盛宇:

Patch Set 2:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from 张盛宇:

Patch Set 3:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from 张盛宇:

Patch Set 3:

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Ian Lance Taylor:

Patch Set 3: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 3:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 3:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 3: TryBot-Result-1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@SilverRainZ
cmd/go/internal/load: prevent calling ImportErrorf when the err is *m…
0e49fb4 
…odule.InvalidPathError

1. The call of setError is unnecessary bacause setError do nothing when
p.Error is not nil.
2. The call of ImportErrorf may panic in this situation
(double quotes appear in the import path).

Fixes golang#49137
@SilverRainZ
cmd/go: add tests for loading invalid import path
990c14c
@SilverRainZ
cmd/go: match entire error message for test case load_invalid_import_…
4307863 
…path
@gopherbot
Copy link
Contributor

This PR (HEAD: 4307863) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/372398 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from 张盛宇:

Patch Set 4:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from 张盛宇:

Patch Set 4:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Srinivas Pokala:

Patch Set 4:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from 张盛宇:

Patch Set 4:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan Mills:

Patch Set 4:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from [email protected]:

Patch Set 4:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/372398.
After addressing review feedback, remember to publish your drafts!

@seankhliao seankhliao closed this Feb 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cmd/go: build panics when passed =" as build path
4 participants
@SilverRainZ @ianlancetaylor @gopherbot @seankhliao