Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

Always remove self_package import #343

Merged
merged 4 commits into from
Jan 14, 2020
Merged

Always remove self_package import #343

merged 4 commits into from
Jan 14, 2020

Conversation

linzhp
Copy link
Contributor

@linzhp linzhp commented Nov 6, 2019

When users provide -self_package, gomock should always exclude that import from generated mock. The pkg.Name can be incorrectly determined in reflect mode:

mock/mockgen/reflect.go

Lines 206 to 211 in 0b73a1d

pkg := &model.Package{
// NOTE: This behaves contrary to documented behaviour if the
// package name is not the final component of the import path.
// The reflect package doesn't expose the package name, though.
Name: path.Base({{printf "%q" .ImportPath}}),
}

We should not reset outputPackagePath when it is passed from -self_package.

Fixing #342

@codyoss
Copy link
Member

codyoss commented Jan 3, 2020

Not sure why this is working on CI, but if you run go install ./... && ./ci/check_go_generate.sh locally this fails.

cc @cvgw @nguyenfilip Something we should look into

@codyoss
Copy link
Member

codyoss commented Jan 3, 2020

This may having nothing to do with this PR btw, but would like to get it fixed before evaluating further PRs.

@codyoss
Copy link
Member

codyoss commented Jan 12, 2020

@linzhp thanks for your patience. I wanted to make sure we got our build properly working on master before we accepted any more PRs. Would you mind rebasing this as there are now some conflicts.

@linzhp
Copy link
Contributor Author

linzhp commented Jan 13, 2020

@codyoss rebased

@linzhp
Copy link
Contributor Author

linzhp commented Jan 13, 2020

Are you planning another release soon? It's been 8 months

@codyoss
Copy link
Member

codyoss commented Jan 13, 2020

Yes, originally planned for tomorrow: https://github.com/golang/mock/milestone/1
Should land sometime this week. This package recently switched maintainers, so there was a bit of a lull.

@codyoss
Copy link
Member

codyoss commented Jan 13, 2020

@linzhp One more thing, would you mind adding a testcase under mockgen/internal/tests. All it would need to be is a source file with a go: generate directive and the generated code. Our ci will try to regenerate all these and check for diffs to make sure this does not break again in the future.

@codyoss codyoss added this to the v1.4.0 milestone Jan 13, 2020
@linzhp
Copy link
Contributor Author

linzhp commented Jan 13, 2020

@codyoss Added a test case

@codyoss codyoss merged commit e00cb15 into golang:master Jan 14, 2020
@codyoss
Copy link
Member

codyoss commented Jan 14, 2020

@linzhp Thank you for your patience and contribution! Should have this rolled up in a release later this week 🎆

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants