Merge pull request #983 from meshery/fix/syncmodutil-pin-shared-deps-for-plugin-abi

fix(syncmodutil): pin shared deps via replace to keep Go plugin ABI stable

Author: fitzergerald

cmd/syncmodutil/README.md

@@ -2,10 +2,32 @@
 
 go run github.com/meshery/meshkit/cmd/syncmodutil < path to src go mod >  < path to destination go mod >
 
+Pass `--err` as a third argument to fail (non-zero exit) when the destination
+`require` block already contains a different version than the source for any
+shared module, instead of rewriting it in place.
+
+## What it does
+
+1. Rewrites every shared `require` line in the destination `go.mod` to match
+   the source's version.
+2. Appends a `replace` block that pins every module in the source module
+   graph to the source's exact selected version. Source-declared `replace`
+   directives are preserved verbatim and take precedence over pinning.
+
+Step 2 is what makes the result safe for **Go plugin builds**. Without it, a
+`go mod tidy` run after sync can upgrade transitive dependencies past what
+the host binary is linked against — causing `plugin.Open` to fail at runtime
+with `plugin was built with a different version of package ...`. Because
+`replace` directives override `require` resolution, the destination module
+is guaranteed to compile against the same versions as the source no matter
+what `tidy` does afterwards.
 
 # Caveats
 1. Always perform a build test of destination go module after syncing to make sure that nothing breaks.
 2. If destination go module relied on a specific version having similar API contract but different internal logic, then you may be in trouble.
+3. The emitted `replace` block is intentionally broad (it pins every module
+   in the source graph). Unused pins are harmless — `go mod tidy` keeps them
+   but they have no effect on modules the destination does not import.
 
 
 ## Flow

cmd/syncmodutil/internal/modsync/sync.go

@@ -71,22 +71,150 @@ func (g *GoMod) SyncRequire(f io.Reader, throwerr bool) (gomod string, err error
 			}
 		}
 	}
-	if len(g.ReplacedVersions) != 0 {
-		data = append(data, "replace(")
+
+	// Emit a replace block that pins every package in the source (host)
+	// module graph to its exact source-selected version. Pinning via
+	// `replace` — rather than relying on `require` alone — is required for
+	// Go plugin ABI compatibility. Without it, the `go mod tidy` step that
+	// callers run after this tool can upgrade transitive dependencies past
+	// what the host binary is linked against (for example, a direct
+	// dependency on github.com/99designs/gqlgen pulling in a newer
+	// github.com/vektah/gqlparser/v2 than the host uses), causing
+	// plugin.Open to fail at runtime with:
+	//   "plugin was built with a different version of package <pkg>".
+	// Because replace directives override require resolution, this
+	// guarantees the destination module is compiled against the exact same
+	// versions as the source. Source-declared replaces take precedence over
+	// pinning and are emitted verbatim.
+	replacedMods := make(map[string]bool, len(g.ReplacedVersions))
+	for _, r := range g.ReplacedVersions {
+		if len(r) >= 1 {
+			replacedMods[r[0].Name] = true
+		}
+	}
+
+	// Compute the set of modules we will emit a replace directive for, so
+	// that any pre-existing replace in the destination for the same module
+	// can be stripped first. Leaving both in place would cause `go mod tidy`
+	// to fail with "multiple replacements for <module>".
+	emitModules := make(map[string]bool, len(g.RequiredVersions)+len(g.ReplacedVersions))
+	for _, required := range g.RequiredVersions {
+		if !replacedMods[required.Name] {
+			emitModules[required.Name] = true
+		}
+	}
+	for _, r := range g.ReplacedVersions {
+		if len(r) >= 1 {
+			emitModules[r[0].Name] = true
+		}
+	}
+	data = stripConflictingReplaces(data, emitModules)
+
+	if len(g.RequiredVersions) > 0 || len(g.ReplacedVersions) > 0 {
+		data = append(data, "replace (")
+	}
+
+	pinned := make(map[string]bool, len(g.RequiredVersions))
+	for _, required := range g.RequiredVersions {
+		if pinned[required.Name] || replacedMods[required.Name] {
+			continue
+		}
+		pinned[required.Name] = true
+		data = append(data, fmt.Sprintf("\t%s => %s %s", required.Name, required.Name, required.Version))
 	}
 
-	//Add all the replaced versions from source to destination. Running go mod tidy after the utility will perform the cleanup in the destination go.mod and remove all the unwanted replace statements.
-	//Instead of trying to intelligently perform diffs, it is better to let the go mod tidy do the cleanup.
+	// Add all the replaced versions from source to destination. Running go
+	// mod tidy after the utility will perform the cleanup in the destination
+	// go.mod and remove unused entries. Instead of trying to intelligently
+	// perform diffs, it is better to let `go mod tidy` do the cleanup.
 	for _, replaced := range g.ReplacedVersions {
-		data = append(data, fmt.Sprintf("\t%s %s => %s %s", replaced[0].Name, replaced[0].Version, replaced[1].Name, replaced[1].Version))
+		data = append(data, formatReplaceLine(replaced))
 	}
-	if len(g.ReplacedVersions) != 0 {
+
+	if len(g.RequiredVersions) > 0 || len(g.ReplacedVersions) > 0 {
 		data = append(data, ")")
 	}
 	gomod = strings.Join(data, "\n")
 	return
 }
 
+// stripConflictingReplaces removes any replace directive from the
+// destination go.mod lines whose "from" module is in the conflicts set.
+// Both forms are handled: single-line (`replace foo => bar v1`) and lines
+// inside an existing `replace (...)` block. Replaces for modules not in
+// conflicts are preserved — including destination-specific overrides that
+// the source does not touch. Empty replace blocks that may be left behind
+// are valid go.mod syntax; `go mod tidy` cleans them up.
+func stripConflictingReplaces(data []string, conflicts map[string]bool) []string {
+	if len(conflicts) == 0 {
+		return data
+	}
+	out := make([]string, 0, len(data))
+	inReplaceBlock := false
+	for _, line := range data {
+		trim := strings.TrimSpace(line)
+
+		if !inReplaceBlock && (trim == "replace (" || trim == "replace(") {
+			inReplaceBlock = true
+			out = append(out, line)
+			continue
+		}
+		if inReplaceBlock && trim == ")" {
+			inReplaceBlock = false
+			out = append(out, line)
+			continue
+		}
+
+		if strings.Contains(trim, "=>") {
+			var rest string
+			switch {
+			case inReplaceBlock:
+				rest = trim
+			default:
+				// go.mod allows arbitrary whitespace after the `replace`
+				// keyword (e.g. `replace\tfoo => bar`), so tokenize rather
+				// than match a fixed "replace " prefix. Skip any other line
+				// containing "=>" (e.g. an in-comment arrow).
+				fields := strings.Fields(trim)
+				if len(fields) == 0 || fields[0] != "replace" {
+					out = append(out, line)
+					continue
+				}
+				rest = strings.TrimSpace(strings.TrimPrefix(trim, fields[0]))
+			}
+			parts := strings.SplitN(rest, "=>", 2)
+			from := strings.Fields(strings.TrimSpace(parts[0]))
+			if len(from) >= 1 && conflicts[from[0]] {
+				continue
+			}
+		}
+
+		out = append(out, line)
+	}
+	return out
+}
+
+// formatReplaceLine renders a single replace directive. The "from" version
+// is optional (e.g. `foo => ../foo`); the "to" version is absent for
+// local-path replacements.
+func formatReplaceLine(r []Package) string {
+	if len(r) < 2 {
+		return ""
+	}
+	from, to := r[0], r[1]
+
+	left := from.Name
+	if from.Version != "" {
+		left = from.Name + " " + from.Version
+	}
+
+	right := to.Name
+	if to.Version != "" {
+		right = to.Name + " " + to.Version
+	}
+	return "\t" + left + " => " + right
+}
+
 // NewGoMod takes an io.Reader to a go.mod and returns GoMod struct
 func New(f io.Reader) (*GoMod, error) {
 	b, err := io.ReadAll(f)
@@ -130,6 +258,7 @@ func getRequiredVersionsFromString(s string) (p []Package) {
 	return p
 }
 func getReplacedVersionsFromString(s string) (p [][]Package) {
+	// Block form: replace ( ... )
 	reps := ReplacePatternRegex.FindAllString(s, -1)
 	for _, req := range reps {
 		data := getStringWithinCharacters(req, '(', ')')
@@ -146,6 +275,36 @@ func getReplacedVersionsFromString(s string) (p [][]Package) {
 			}
 		}
 	}
+
+	// Single-line form: `replace foo [v] => bar [v]` outside any block.
+	// go.mod allows arbitrary whitespace after the `replace` keyword, so
+	// tokenize rather than match a fixed prefix.
+	inBlock := false
+	for _, line := range strings.Split(s, "\n") {
+		trim := strings.TrimSpace(line)
+		if !inBlock && (trim == "replace (" || trim == "replace(") {
+			inBlock = true
+			continue
+		}
+		if inBlock {
+			if trim == ")" {
+				inBlock = false
+			}
+			continue
+		}
+		if !strings.Contains(trim, "=>") {
+			continue
+		}
+		fields := strings.Fields(trim)
+		if len(fields) == 0 || fields[0] != "replace" {
+			continue
+		}
+		rest := strings.TrimSpace(strings.TrimPrefix(trim, fields[0]))
+		p0 := getPackagesAndVersionsFromPackageVersions(rest)
+		if len(p0) != 0 {
+			p = append(p, p0)
+		}
+	}
 	return p
 }
 func getPackagesAndVersionsFromPackageVersions(pkg string) (p []Package) {