You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2022/01/07 17:06:42 UTC
[GitHub] [trafficcontrol] ocket8888 opened a new pull request #6455: Remove vendored dependencies
ocket8888 opened a new pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455
Now that we have switched to Go modules¹, there's no need to commit dependencies in the vendoring directory to the repository
<hr/>
## Which Traffic Control components are affected by this PR?
- Documentation
- Traffic Control Cache Config (T3C, formerly ORT)
- Traffic Control Health Client (tc-health-client)
- Traffic Ops Client (Go)
- Traffic Monitor
- Traffic Ops
- Traffic Stats
- Grove
- CDN in a Box
- Automation (GHA, anything that relies on Go)
## What is the best way to verify this PR?
Make sure everything still works and there are no license issues.
## PR submission checklist
- [ ] This PR has tests
- [x] This PR has documentation
- [ ] This PR has a CHANGELOG.md entryhave a changelog entry.
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#discussion_r780409959
##########
File path: go.sum
##########
@@ -1317,6 +1317,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Review comment:
I'm not sure, but it also passes _with_ those changes. If it passes under both conditions there must be something wrong with the check, right? Surely it shouldn't be able to pass in both scenarios
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c edited a comment on pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
rob05c edited a comment on pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#issuecomment-1007586324
Vendoring prevents the build from breaking in the future, when people delete their github repos that the project depends on.
Internet resources disappearing is a common problem, and it makes it impossible to build older versions, if a project doesn't vendor and include everything necessary to build without the internet. See: `left-pad`.
It also makes it possible to build if someone doesn't have an internet connection, or is in an area where one of the dependency URLs is blocked by a firewall.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c edited a comment on pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
rob05c edited a comment on pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#issuecomment-1007586324
Vendoring prevents the build from breaking in the future, when people delete their github repos that the project depends on.
Internet resources disappearing is a common problem, and it makes it impossible to build older versions, if a project doesn't vendor and include everything necessary to build without the internet. See: `left-pad`.
It also makes it possible to build if someone doesn't have an internet connection, or is in an area where one of the dependency URLs is blocked by a firewall.
I really think the project should vendor, for those reasons.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c commented on pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
rob05c commented on pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#issuecomment-1007586324
Vendoring prevents the build from breaking in the future, when people delete their github repos that TC depends on.
Internet resources disappearing is a common problem, and it makes it impossible to build older versions, if we don't vendor and include everything necessary to build without the internet. See: `left-pad`.
It also makes it possible to build if someone doesn't have an internet connection, or is in an area where one of the dependency URLs is blocked by a firewall.
I really think we should vendor, for those reasons.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#discussion_r780407198
##########
File path: go.sum
##########
@@ -1317,6 +1317,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Review comment:
Does the _Check Go Modules_ action pass without these changes to `go.mod` and `go.sum`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#discussion_r780417255
##########
File path: go.sum
##########
@@ -1317,6 +1317,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Review comment:
That change is inserted by `go mod tidy`, though. I did not manually alter either `go.mod` or `go.sum`. If the check (and all other tests) pass after the change, shouldn't it stay in? The master branch also updates those files whenever I build a Go component, so the underlying issue isn't introduced in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#discussion_r780407198
##########
File path: go.sum
##########
@@ -1317,6 +1317,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Review comment:
Does the _Check Go Modules_ action pass without these changes to `go.mod` and `go.sum`?
##########
File path: go.sum
##########
@@ -1317,6 +1317,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Review comment:
ec4e9b44eb upgrades `golang.org/x/tools` to `v0.1.8` and commits the corresponding `go.sum`. I also get those `go.sum` changes if I run `go mod tidy` after upgrading `golang.org/x/tools` to `v0.1.8`. So the check is not incorrect, just the https://github.com/apache/trafficcontrol/blob/ec4e9b44eb326b4f7d9a436cd7ec1300aa516b9a/go.mod#L78 line.
##########
File path: go.sum
##########
@@ -1317,6 +1317,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Review comment:
> If you download this changeset and run `go mod tidy`, does it change anything for you?
no
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c commented on pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
rob05c commented on pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#issuecomment-1007586324
Vendoring prevents the build from breaking in the future, when people delete their github repos that TC depends on.
Internet resources disappearing is a common problem, and it makes it impossible to build older versions, if we don't vendor and include everything necessary to build without the internet. See: `left-pad`.
It also makes it possible to build if someone doesn't have an internet connection, or is in an area where one of the dependency URLs is blocked by a firewall.
I really think we should vendor, for those reasons.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#discussion_r780409959
##########
File path: go.sum
##########
@@ -1317,6 +1317,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Review comment:
I'm not sure, but it also passes _with_ those changes. If it passes under both conditions there must be something wrong with the check, right? Surely it shouldn't be able to pass in both scenarios
##########
File path: go.sum
##########
@@ -1317,6 +1317,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Review comment:
That change is inserted by `go mod tidy`, though. I did not manually alter either `go.mod` or `go.sum`. If the check (and all other tests) pass after the change, shouldn't it stay in? The master branch also updates those files whenever I build a Go component, so the underlying issue isn't introduced in this PR.
##########
File path: go.sum
##########
@@ -1317,6 +1317,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Review comment:
That change is inserted by `go mod tidy`, though. I did not manually alter either `go.mod` or `go.sum`. If the check (and all other tests) pass after the change, shouldn't it stay in? The master branch also updates those files whenever I build a Go component, so the underlying issue isn't introduced in this PR. If you download this changeset and run `go mod tidy`, does it change anything for you?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#issuecomment-1011182095
> this seems like it needs a mailing list discussion
Alright, no problem. I really just didn't expect anyone to come to the defense of vendoring :P
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
rawlinp commented on pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#issuecomment-1010394639
Yeah, this seems like it needs a mailing list discussion. There are pros and cons to keeping the vendored dependencies in the repo (@rob05c pointed out a big one). Another pro of vendoring dependencies is that it helps to understand the amount of code changed whenever a dependency is upgraded/added/removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#discussion_r780417255
##########
File path: go.sum
##########
@@ -1317,6 +1317,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Review comment:
That change is inserted by `go mod tidy`, though. I did not manually alter either `go.mod` or `go.sum`. If the check (and all other tests) pass after the change, shouldn't it stay in? The master branch also updates those files whenever I build a Go component, so the underlying issue isn't introduced in this PR. If you download this changeset and run `go mod tidy`, does it change anything for you?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#discussion_r780413186
##########
File path: go.sum
##########
@@ -1317,6 +1317,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Review comment:
ec4e9b44eb upgrades `golang.org/x/tools` to `v0.1.8` and commits the corresponding `go.sum`. I also get those `go.sum` changes if I run `go mod tidy` after upgrading `golang.org/x/tools` to `v0.1.8`. So the check is not incorrect, just the https://github.com/apache/trafficcontrol/blob/ec4e9b44eb326b4f7d9a436cd7ec1300aa516b9a/go.mod#L78 line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c edited a comment on pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
rob05c edited a comment on pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#issuecomment-1007586324
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#discussion_r780419199
##########
File path: go.sum
##########
@@ -1317,6 +1317,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Review comment:
> If you download this changeset and run `go mod tidy`, does it change anything for you?
no
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c edited a comment on pull request #6455: Remove vendored dependencies
Posted by GitBox <gi...@apache.org>.
rob05c edited a comment on pull request #6455:
URL: https://github.com/apache/trafficcontrol/pull/6455#issuecomment-1007586324
Vendoring prevents the build from breaking in the future, when people delete their github repos that TC depends on.
Internet resources disappearing is a common problem, and it makes it impossible to build older versions, if we don't vendor and include everything necessary to build without the internet. See: `left-pad`.
It also makes it possible to build if someone doesn't have an internet connection, or is in an area where one of the dependency URLs is blocked by a firewall.
I really think the project should vendor, for those reasons.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org