Works around missing Module.dir for vendored Modules #178
Conversation
|
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. |
| if isVendorModule(newModule(p.Module), pkgDir) { | ||
| splits := strings.SplitN(pkgDir, "/vendor/", 2) | ||
| moduleSplits := strings.Split(p.Module.Path, "/") | ||
| // There seems to be 3 common cases in terms of go module directory structure: |
There was a problem hiding this comment.
I am not overly happy/attached to this code, but tried to document what I saw when running these build thru our build pipelines. I think the main fix here is by stopping the traversal up at the vendor directory. We could remove all this and just use that instead of trying to be "smart" by making these assumptions. I would not guarantee these are assumptions are even correct. There could def be other cases where the folder structure is deeper or shallower for legit reason.
Looking for feedback here for sure.
jaxesn
force-pushed
the
jgw/fix-missing-license
branch
from
204f087 to
33b3528
Compare
| // - "k8s.io" case follow k8s.io/<module> | ||
| // - "go.starlark.net" (and probably lots others not exactly sure why/when this happens) case where go.starlark.net is the only folder in the structure | ||
| // To account for the above, the stopAt is set to the the greatest depth up to 3 subfolders | ||
| // Alternative: Since license files should only exists in the context of module folders within |
There was a problem hiding this comment.
I like this alternative solution. The assumption that vendor folder and module dir folders do not have license files makes sense and is simpler than assuming a depth <= 3.
What do you think?
There was a problem hiding this comment.
I think so too, we can always adjust it if/when running into a case of a license file being in vendor, which seems less likely than depending on the current assumptions holding true.
Im going to change it in my build and run it thru our pipelines and just make sure nothing new pops up and Ill update this PR.
Fixes: #143
When scanning for a license for a particular module, go-licenses will traverse up the directory structure looking for LICENSE (or some variant of) file. The find code tries to use the module's rootdir as a guard rail for this traversal up. The intent being that a license should be found at some point within the module's root, not above.
In the case of vendored modules, the Module.Dir returned from the golang tools package is actually empty. I do not know exactly why this is, but it seems to be something about about what metadata exists, or does not exist, in the vendor directory for these modules. Since Module.Dir is empty and is used as the stopping point for traversal up the directory structure, if a module is missing a LICENSE the code will continue searching until it ultimately finds the LICENSE in the root of the repo, which in these cases is incorrect since these dependencies should have their own licenses.
Since the root license is found, go-licenses does not inform the user of the missing license for that module, which it would normally do. In addition, this results in an odd side effect when running the csv subcommand. When go-licenses finds multiple packages using the same LICENSE file it groups them together and uses this to determine its final name in the csv file, typically github.com//. Since deps missing LICENSES files match with the root package, the csv file ends up having github.com listed in one of the rows since its the commonAncestor. I believe this matching of packages is to handle legit cases where modules have subfolders/submodules which are licensed together.
The library code already had specific handling for vendored modules later in the process. This was extract out to be shared during the finding license process so that when vendor modules are detected the rootDir is inferred based on the vendor directory and the package path to ensure the traversal stops at the module root.