Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

fix: build plugin incorrectly combines stdout&stderr #3909

Merged
merged 1 commit into from
Nov 14, 2022

Conversation

garethjevans
Copy link
Contributor

What this PR does / why we need it

Which issue(s) this PR fixes

Fixes #3907

Describe testing done for PR

  • check out local plugin project
  • go clean -modcache
  • make build-local
    (builds successfully where as would previously fail)

Release note

Builder plugin no longer combines stdout & stderr when reading the plugin descriptor.

@garethjevans garethjevans requested a review from a team as a code owner November 14, 2022 10:37
@randomvariable randomvariable added the ok-to-merge PRs should be labelled with this before merging label Nov 14, 2022
@codecov
Copy link

codecov bot commented Nov 14, 2022

Codecov Report

Merging #3909 (237be80) into main (65ded46) will decrease coverage by 0.93%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3909      +/-   ##
==========================================
- Coverage   46.94%   46.01%   -0.94%     
==========================================
  Files         402      427      +25     
  Lines       40243    41794    +1551     
==========================================
+ Hits        18894    19230     +336     
- Misses      19600    20794    +1194     
- Partials     1749     1770      +21     
Impacted Files Coverage Δ
...oller-manager/controllers/v3_cascade_controller.go 61.90% <0.00%> (-11.43%) ⬇️
packageclients/pkg/packageclient/package_update.go 83.57% <0.00%> (-1.43%) ⬇️
cmd/cli/plugin/cluster/delete.go 12.50% <0.00%> (ø)
cmd/cli/plugin/cluster/get_machinehealthcheck.go 11.42% <0.00%> (ø)
cmd/cli/plugin/cluster/list.go 11.36% <0.00%> (ø)
cmd/cli/plugin/cluster/set_node_pool.go 14.63% <0.00%> (ø)
...cluster/delete_machinehealthcheck_control_plane.go 16.66% <0.00%> (ø)
cmd/cli/plugin/cluster/credentials_update.go 9.23% <0.00%> (ø)
cmd/cli/plugin/cluster/get.go 6.27% <0.00%> (ø)
cmd/cli/plugin/cluster/osimage.go 100.00% <0.00%> (ø)
... and 17 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@randomvariable randomvariable merged commit a8c2eac into vmware-tanzu:main Nov 14, 2022
@garethjevans garethjevans deleted the failing-builder branch November 14, 2022 16:37
@marckhouzam
Copy link
Contributor

A little late to the party, but I wanted to say thanks for this fix! This bug has been causing problems to many different people and we had trouble tracking it down. Basically, this PR will fix the error many have seen intermittently of:

✖  🐼 - building plugin "cmd/cli/plugin/tkr" failed - invalid character 'F' looking for beginning of value

It turns out that the tkr plugin prints error messages if an unreachable cluster is configured for tanzu.
Now that the builder plugin ignores error printouts, the build will no longer fail when this happens.

So, again, thanks for this fix @garethjevans 🎉

@garethjevans
Copy link
Contributor Author

no problem @marckhouzam

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Builder plugin fails when there are dependencies to download
4 participants