Add support to replace directive in go.mod #3693
Conversation
Signed-off-by: Shenoy <[email protected]>
There was a problem hiding this comment.
@shravankshenoy thanks for the PR!
I've added a couple of comments for your consideration, to the best of my knowledge (since I'm not too familiar with go build systems), please review and update the PR accordingly, as applicable.
| @@ -188,6 +259,7 @@ def parse_gomod(location): | |||
|
|
|||
| gomods.require = require | |||
| gomods.exclude = exclude | |||
| gomods.replace = replace | |||
There was a problem hiding this comment.
Also add replace as an attribute of class GoModule
| replace.append( | ||
| GoModule( | ||
| namespace=namespace, | ||
| name=name, | ||
| version=parsed_rep_link.group("version"), | ||
| ) | ||
| ) | ||
|
|
||
| replace.append( | ||
| GoModule( | ||
| namespace=replacement_namespace, | ||
| name=replacement_name, | ||
| version=parsed_rep_link.group("replacement_version"), | ||
| ) | ||
| ) |
There was a problem hiding this comment.
This is not correct, as we are treating the module to be replaced and the replacement module the same way. I think the first module in the replace directive (the module to be replaced) should be added as a dependency with scope as exclude and the second module in the replace directive (the replacement module) should be added with scope as require.
@pombredanne what do you think?
There was a problem hiding this comment.
We need to track the origin and renamed module separately when parsing alright.
And then we would need to only report the replaced modules and not both... and there could be trailing comments in replace that are also useful to track
| @@ -0,0 +1,255 @@ | |||
| module github.com/milvus-io/milvus | |||
There was a problem hiding this comment.
this is quite a large test file, would be nice to truncate this to only have the essential portion required to test the functionality, or find a shorter example. Also add the go.mod files linked in the main issue as tests.
| github.com/bketelsen/crypt => github.com/bketelsen/crypt v0.0.4 // Fix security alert for core-os/etcd | ||
| github.com/expr-lang/expr => github.com/SimFG/expr v0.0.0-20231218130003-94d085776dc5 | ||
| github.com/go-kit/kit => github.com/go-kit/kit v0.1.0 | ||
| github.com/milvus-io/milvus/pkg => ./pkg |
There was a problem hiding this comment.
In the case where the replace directive contains a reference to a local directory, we can try to resolve this in the assemble step of the datafile handler, if there are package manifests in the local directory with package data for the replacement module.
- On the
parsefunction side we store the local references in theextra_dataattribute somehow, with the referenced path. - On the assembly side, we try to walk the codebase for this referenced directory.
- If the referenced directory is present in the codebase, and we have parsed go package data present there, we use that to populate the required dependency list.
|
@shravankshenoy gentle ping |
Fixes #3492
About replace directive in go.mod
Replaces the content of a module at a specific version (or all versions) with another module version or with a local directory. Syntax as shown below
replace module-path [module-version] => replacement-path [replacement-version]Detailed info at https://go.dev/doc/modules/gomod-ref
Changes made
`