refactor: split packageinfo.WritePackageInfo into one function for modules and one for packages#363
Open
hartblanc wants to merge 13 commits into
Open
refactor: split packageinfo.WritePackageInfo into one function for modules and one for packages#363hartblanc wants to merge 13 commits into
hartblanc wants to merge 13 commits into
Conversation
added 13 commits
June 27, 2026 14:14
…l to WritePackageInfo
…n advance for FromBuildPackageForModule
…n advance for fromBuildPackage
…nd packages.Package as pkg
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change is a refactor.
It is in preparation for fixing issues #352 - #362.
I have attempted to keep the individual commits as small as possible. They should be significantly easier to review than the whole PR.
I have added some tests early on to reduce the risk of regressions with the refactoring. I have avoided adding test cases where the behaviour is currently buggy (e.g. I did not include a test case for external test packages given #362).
The primary aim of the refactor is to separate the codepaths for writing packageinfo for modules (i.e. the module_info command of please_go) and for stand-alone packages (i.e. the package_info command of please_go).
module_infois called forgo_moduleandgo_stdlibtargets.package_infois called for all other types of targets.These two groups of targets have very different characteristics:
module_infotargets have many packages for a single target, whereaspackage_infotargets should only return a single package.module_inforules are only ever depended on, they do not have sources that need to be queried by the package driver directly. They are only ever included as dependencies.module_inforules have their source code primarily built outside of please by standard go tooling. Therefore, they adhere to standard go package constraints that please does not enforce (e.g. one package per directory). They also use features of the go build tooling that please does not support for other types of targets (module vendoring).These differences in characteristics heavily influence the way that I intend to handle the two types of packages when addressing the issues I have raised. I also believe the co-location of these two codepaths has contributed to the bugs that are present today (e.g. when attempting to introduce tests for
package_infotargets we accidentally introduced some bugs in themodule_infotargets).