Remove vendor folder

[vcastellm]

Description

Go mod takes care of getting the packages.

Changes include

• Chore

Testing

• I have tested this code with the official test suite
• I have tested this code manually

Manual tests

Go build

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[vcastellm]

I have read the CLA Document and I hereby sign the CLA

[codecov]

Codecov Report

Merging #755 (d64c3ba) into develop (bf909f5) will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #755      +/-   ##
===========================================
+ Coverage    52.48%   52.49%   +0.01%     
===========================================
  Files          130      130              
  Lines        17069    17069              
===========================================
+ Hits          8959     8961       +2     
+ Misses        7466     7464       -2     
  Partials       644      644              

Impacted Files	Coverage Δ
network/server.go	76.18% <0.00%> (+0.42%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[zivkovicmilos]

Hey @vcastellm,

The vendoring mechanism we have in Edge is originally part of the legacy repo features when go.mod packaging wasn't very prevalent. The reason we stuck around with vendoring, and why we need to stick around with vendoring is because we need to preserve exact package versions because some of the packages Edge relies on critically are unstable, as it's been proven true in the past multiple times. Keeping vendoring is not optional at this point because of the unstable packaging, not to mention it requires lots of documentation and setup changes

[vcastellm]

Hey @vcastellm,

The vendoring mechanism we have in Edge is originally part of the legacy repo features when go.mod packaging wasn't very prevalent. The reason we stuck around with vendoring, and why we need to stick around with vendoring is because we need to preserve exact package versions because some of the packages Edge relies on critically are unstable, as it's been proven true in the past multiple times. Keeping vendoring is not optional at this point because of the unstable packaging, not to mention it requires lots of documentation and setup changes

Please @zivkovicmilos refer to this conversation https://0xpolygon.slack.com/archives/C02EZ0KUGF4/p1663921633680319 I've to disagree with these points, go mod ensures we preserve exact package version down to commit-ids, so It's a bad practice in modern Go repos to include the vendor folder in the repository, unless very specific reason, which "maintaining exact versions" is not one of them.

Can you please elaborate on what documentation and setup changes needs to be adapted?

PD: just try it for yourself, the CI and local execution runs just fine.

[zivkovicmilos]

@vcastellm

I've gone over the convo you've linked, and @epikichi is right about the package availability thing - you can't really guarantee package availability if you rely on a 3rd party proxy service, even if it's the default Golang one. You're actually right about the package versions - we can lock them down in the mod file.

Bundling these packages with the repo itself, although bulkier, makes it easier to do dependency management, because now users won't have to additionally download every single package, and indirect package to build and run Edge locally apart from releases. Not only does it save bandwidth, but you can be sure that the code we ship works with the exact packages we (literally) ship. It also makes it easier to debug other packages, you don't have to hunt them down on your fs, or add additional replace directives, since they are automatically linked locally in the vendor folder when go looks for them. You also don't risk having previously modified packages on your fs interfere with Edge, since Edge brings the packages with it, regardless of what you have in your bin folder.

It's just a quality of life thing we keep up with the codebase. Most of our dependency headaches are handled by dependabot nowadays, apart from the occasional libp2p breaking changes that we need to resolve manually (ex. #730).

We'd need to update our license script generator steps (add a vendor step) to accommodate for this:
https://github.com/0xPolygon/polygon-edge/blob/develop/generate_dependency_licenses.sh

I've also gone over our docs now to see if there's anything that would be affected, but judging by the commands we list (specifically in the install section), it seems like they're unaffected since those go commands handle dependency fetch in the background regardless if we use vendoring or not.

I would not rush this decision, since it's a bigger repo change, and I would definitely not base it on a 15min Slack conversation. We can talk more about this next week 👍

[vcastellm]

@vcastellm

I've gone over the convo you've linked, and @epikichi is right about the package availability thing - you can't really guarantee package availability if you rely on a 3rd party proxy service, even if it's the default Golang one. You're actually right about the package versions - we can lock them down in the mod file.

As everything in IT it's all tradeoffs, using the official proxy is completely worth it in exchange of a much lighter repo. Just look to major Go projects (hashicorp ones for instance), none of them are vendoring.

Bundling these packages with the repo itself, although bulkier, makes it easier to do dependency management, because now users won't have to additionally download every single package, and indirect package to build and run Edge locally apart from releases.

Go mod is fully integrated with go tooling, just need to run go build it automatically fetches al deps, the package manager makes it transparent for the user, so the user doesn't need no additional downloads one by one. IMO this is not a valid argument.

Not only does it save bandwidth, but you can be sure that the code we ship works with the exact packages we (literally) ship.

Bandwidth is negligible in this case, you are going to download the packages anyway from one source or the other, just a few extra tcp packets. The packages will be the exact packages we ship due to proxy caching again.

It also makes it easier to debug other packages, you don't have to hunt them down on your fs, or add additional replace directives, since they are automatically linked locally in the vendor folder when go looks for them. You also don't risk having previously modified packages on your fs interfere with Edge, since Edge brings the packages with it, regardless of what you have in your bin folder.

Go and VSCode tooling help you with this, again tradeoffs, it's again worth a lighter repo that store > 4k files in the repo, you can use go mod vendor in local to gather all packages temporarily in your local fs.

It's easier to modify your local vendor package than a normal one, VScode opens them as read-only, so this argument looks invalid to me.

It's just a quality of life thing we keep up with the codebase. Most of our dependency headaches are handled by dependabot nowadays, apart from the occasional libp2p breaking changes that we need to resolve manually (ex. #730).

We'd need to update our license script generator steps (add a vendor step) to accommodate for this: https://github.com/0xPolygon/polygon-edge/blob/develop/generate_dependency_licenses.sh

OK, I can do that.

I've also gone over our docs now to see if there's anything that would be affected, but judging by the commands we list (specifically in the install section), it seems like they're unaffected since those go commands handle dependency fetch in the background regardless if we use vendoring or not.

Correct, glad you realized

I would not rush this decision, since it's a bigger repo change, and I would definitely not base it on a 15min Slack conversation. We can talk more about this next week 👍

Actually several senior Go professionals has given its 👍 , it's just a matter of testing, we need to be enough confident in our CI to accomodate this change without investing too much time. The blast radius of the change is well known, the consequences are pretty controlled, and can be easily verified.

I would ask you not to block this change for long time without having a strong argument against it.

Thanks.