fix: go install cosmovisor v1.1.+

[tac0turtle]

Description

reverts #10442 as it breaks go install.

closes #11199

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• included the correct type prefix in the PR title
• added ! to the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• followed the guidelines for building modules
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• updated the relevant documentation or specification
• reviewed "Files changed" and left comments if necessary
• confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage
• manually tested (if applicable)

[codecov]

Codecov Report

Merging #11212 (cefe99b) into master (5c44e5d) will decrease coverage by 0.03%.
The diff coverage is 94.44%.

❗ Current head cefe99b differs from pull request most recent head 65b7648. Consider uploading reports for the commit 65b7648 to get more accurate results

@@            Coverage Diff             @@
##           master   #11212      +/-   ##
==========================================
- Coverage   66.04%   66.00%   -0.04%     
==========================================
  Files         666      671       +5     
  Lines       69238    69118     -120     
==========================================
- Hits        45730    45624     -106     
+ Misses      20827    20819       -8     
+ Partials     2681     2675       -6     

Impacted Files	Coverage Δ
cosmovisor/process.go	54.54% <50.00%> (ø)
cosmovisor/args.go	84.88% <100.00%> (ø)
cosmovisor/scanner.go	78.31% <100.00%> (ø)
cosmovisor/upgrade.go	75.72% <100.00%> (ø)
x/group/keeper/grpc_query.go	61.53% <0.00%> (-2.52%)	⬇️
x/gov/keeper/keeper.go	75.64% <0.00%> (-1.47%)	⬇️
x/upgrade/keeper/grpc_query.go	75.60% <0.00%> (-1.14%)	⬇️
x/upgrade/module.go	66.66% <0.00%> (-0.78%)	⬇️
x/group/types.go	47.34% <0.00%> (-0.51%)	⬇️
store/v2/multi/store.go	76.23% <0.00%> (-0.49%)	⬇️
... and 36 more

[robert-zaremba]

I would prefer to find a better solution than reverting the change.
We didn't want to copy the expected struct between the packages, and rather import it from the x/upgrade package.

Moreover there are new changes coming which require to import x/upgrade.

[tac0turtle]

I would prefer to find a better solution than reverting the change. We didn't want to copy the expected struct between the packages, and rather import it from the x/upgrade package.

Moreover there are new changes coming which require to import x/upgrade.

if we want to support go install then the only workaround I see is to copy the generated go file into cosmovisor. this way its not manual duplication but auto generated

[peterbourgon]

Any module which will be imported by other modules cannot effectively use replace directives in its go.mod file, because replace is not transitive. So anything which removes replace directives is a step in the right direction.

[tac0turtle]

@alexanderbez @AmauryM @aaronc would like to hear from you guys here. Not having go install work is a pain for many node operators. Id like to duplicate the code until the fix lands since the fix timeline is unknown

[alexanderbez]

I agree @marbar3778. Having go install work is definitely a desirable thing to have in terms of operator UX.

However, I'm not really privy to the core of the issue here though. Can someone summarize so I could perhaps provide some feedback?

Is there a proposed solution that prevents reverting but also allows go install to work?

[robert-zaremba]

I think we can remove the replace. I've started a PR: #11218
But forgot about it because we had a Regen sprint and in the meantime Marko removed the PR (it should be converted into draft).

[robert-zaremba]

I think the right solution would be to consistently use regen gogo proto fork without using the replace.

[robert-zaremba]

We can make this as a temporal fix. But we still need a solution for #10378

[tac0turtle]

We can make this as a temporal fix. But we still need a solution for #10378

#10925 this would be the fix, but timeline is unclear at this time.

[robert-zaremba]

#10925 this would be the fix, but timeline is unclear at this time.

Exactly. @aaronc just shared it with me. I also wrote about this idea yesterday: #11212 (comment). We can handle it in our team.

[tac0turtle]

should I close this or can we merge it?

I think the fix of renaming the generated code will take a bit of time, id rather have go install working now for users.

[robert-zaremba]

we started working on it.