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

x/exp/cmd/gorelease: report when packages can't be loaded without replace, exclude #37559

Open
Tracked in #46371
jayconrod opened this issue Feb 28, 2020 · 2 comments
Open
Tracked in #46371
Labels
FeatureRequest modules NeedsFix Tools
Milestone

Comments

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Feb 28, 2020

replace and exclude directives can be used in go.mod files to temporarily work around problems in dependencies while they are fixed upstream. They're also used in local development. replace and exclude only apply within the main module, so if a module can't be used by another without replace and exclude directives, it shouldn't be released.

gorelease currently loads type information for packages without replace and exclude directives and reports errors. If it sees errors, it should try loading with replace and exclude directives. If that succeeds, gorelease should note that as a possible cause for errors.

@jayconrod jayconrod added NeedsFix FeatureRequest modules Tools labels Feb 28, 2020
@jayconrod jayconrod added this to the Unreleased milestone Feb 28, 2020
@jayconrod jayconrod removed this from the Unreleased milestone Oct 14, 2020
@jayconrod jayconrod added this to the gorelease milestone Oct 14, 2020
@jadekler
Copy link
Member

@jadekler jadekler commented May 14, 2021

gorelease currently loads type information for packages without replace and exclude directives and reports errors.

So, to be clear, today gorelease would say something like "This module does not compile", or something like that?

gorelease currently loads type information for packages without replace and exclude directives and reports errors.

To make sure I understand this properly:

  • gorelease loads information for the module a user is working on.
  • It ignores replace and exclude directives in that module's go.mod.
  • If it cannot build (for example, due to a missing or broken dependency that the exclude or replace would fix), it reports an error.

Is that correct? (I'm mostly not grokking which "packages" the quoted sentence is referring to)

it should try loading with replace and exclude directives

As in, if there's a build error, go back to the user's local go.mod, find the replace and exclude directives, and try to emulate them in the tmp go.mod (from loadLocalModule, presumably?).

If that succeeds, gorelease should note that as a possible cause for errors.

Gotcha - so this feature is "there currently is an error message for this scenario, but we want a better, more descriptive error message tailored to this scenario", as opposed to "there currently is no error message for this scenario, and there should be". Is that correct?

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Jun 25, 2021

Ah, sorry for not answering this earlier.

So, to be clear, today gorelease would say something like "This module does not compile", or something like that?

Today, gorelease will load packages with packages.Load. Each returned *Package will have its Errors field set if there were any errors loading or type-checking the package. For example, say a package in the user's module imported a package in a replaced module and called a function that only exists in the replacement. gorelease will use the original module, not the replacement, so there would be an undefined function error there.

Is that correct? (I'm mostly not grokking which "packages" the quoted sentence is referring to)

Yes, all correct.

As in, if there's a build error, go back to the user's local go.mod, find the replace and exclude directives, and try to emulate them in the tmp go.mod (from loadLocalModule, presumably?).

That was the original idea.

Maybe this is trying to do too much though? I'm not sure how complicated the implementation would be. Perhaps instead, gorelease should attempt to load packages, and if there are errors, it could check if there were any replace or exclude directives, then emit a diagnostic message saying that's possibly the cause? That would be enough to get folks on the right track.

Gotcha - so this feature is "there currently is an error message for this scenario, but we want a better, more descriptive error message tailored to this scenario", as opposed to "there currently is no error message for this scenario, and there should be". Is that correct?

Yeah, currently the user may see an error here that they don't see when building locally. Users of their package would see that error, so it's important that they fix it before the release. We should do what we can to make it less mystifying though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest modules NeedsFix Tools
Projects
None yet
Development

No branches or pull requests

2 participants