Skip to content
This repository has been archived by the owner on Apr 1, 2022. It is now read-only.

Gomod: ignore local modules in a multi-module project #171

Merged
merged 2 commits into from
Dec 28, 2020

Conversation

cnr
Copy link
Contributor

@cnr cnr commented Dec 16, 2020

The replace directive in gomodules projects is problematic: in addition to its typically-bad use, it's also used to emulate multi-project structures if the replace target is a directory, rather than the usual package/version combo:

replace foo => bar v2 // typical, bad
replace foo => ../foo // multi-project!

the Right Way™️ to fix this is to do what we've done with maven projects previously, which is to construct multi-project "closures" and extract dependency graphs from those. I've done this for gomodules projects on another branch: https://github.com/fossas/spectrometer/tree/go-multi-module-projects

This is a simpler approach. It handles the vast majority of usecases seen in the wild. We improve our fallback static-analysis strategy by ignoring local packages.

The best-analysis case (via running commands with the go cli) was already covered in a previous PR.

@cnr cnr requested a review from skilly-lily December 16, 2020 22:23
@cnr cnr changed the title Gomod ignore local modules Gomod: ignore local modules in a multi-module project Dec 16, 2020
Copy link
Contributor

@skilly-lily skilly-lily left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -20,6 +20,8 @@ replace repo/B => alias/repo/B v0.1.0
replace (
repo/C => alias/repo/C v0.0.0-20180207000608-000000000003
repo/E => alias/repo/E v0.0.0-20170808103936-000000000005+incompatible
foo v0 => ../foo
bar => /foo/bar/baz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Are the differences in leading whitespace significant? If not, can we normalize them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was completely unintentional, but they're accidentally significant insofar as proving our parser is whitespace-insensitive -- so I guess I'll keep them

@cnr cnr merged commit 2736544 into master Dec 28, 2020
@cnr cnr deleted the gomod-ignore-local-modules branch December 28, 2020 20:13
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