Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove third_party directory #6416

Merged
merged 1 commit into from
Mar 22, 2023
Merged

Remove third_party directory #6416

merged 1 commit into from
Mar 22, 2023

Conversation

lbernick
Copy link
Member

@lbernick lbernick commented Mar 21, 2023

This commit removes the third_party directory, and updates hack/update-deps.sh to not store licenses in the third_party directory. It also removes symlinks to third_party in the kodata folders of code in cmd/, which results in third_party not being included in the images built from these directories.

The default build_test runner validates licenses stored in the vendor directory.

The reason we have the third_party directory is because some of our dependencies may have licenses that require distributing source code along with built images. However, we already include the (compressed) vendor directory in built images (see the release pipeline).

The reason to remove third_party is because it is redundant with the vendor directory, included in unit tests, and CI does not enforce keeping it up to date. As an alternative to this commit, we could instead enforce that third_party is kept up to date via hack/verify-codegen.sh (the approach taken by #6342).

/kind misc
Closes #6210
Closes #6352
Closes #6015

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • n/a Has Docs included if any changes are user facing
  • n/a Has Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
  • Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
  • n/a Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

Images built as part of releases no longer contain contents of third_party/. (Images still contain contents of vendor/.)

@tekton-robot tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/misc Categorizes issue or PR as a miscellaneuous one. labels Mar 21, 2023
@tekton-robot tekton-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 21, 2023
@lbernick lbernick changed the title [WIP] Remove third_party directory Remove third_party directory Mar 22, 2023
@tekton-robot tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 22, 2023
This commit removes the third_party directory, and updates hack/update-deps.sh
to not store licenses in the third_party directory. It also removes symlinks
to third_party in the kodata folders of code in cmd/, which results in third_party
not being included in the images built from these directories.

The default build_test runner validates licenses stored in the vendor directory
(https://github.com/tektoncd/plumbing/blob/c6cc7570d1254ac6796b08f20adcaf10a80d38a4/scripts/presubmit-tests.sh#L205-L208).

The reason we have the third_party directory is because some of our dependencies
may have licenses that require distributing source code along with built images.
However, we already include the (compressed) vendor directory in built images
(see the [release pipeline](https://github.com/tektoncd/pipeline/blob/bf26bfb35745b403ac8ca760068f4d102e441940/tekton/publish.yaml#L139-L152)).

The reason to remove third_party is because it is redundant with the vendor directory,
included in unit tests, and CI does not enforce keeping it up to date.
As an alternative to this commit, we could instead enforce that third_party is kept up to date
via hack/verify-codegen.sh.
@vdemeester
Copy link
Member

/retest
/approve

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 22, 2023
Copy link
Member

@wlynch wlynch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 22, 2023
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dibyom, vdemeester, wlynch

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [dibyom,vdemeester,wlynch]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot merged commit fb76f21 into tektoncd:main Mar 22, 2023
dibyom added a commit to dibyom/triggers that referenced this pull request Apr 19, 2023
The third_party directory contains source code of some of our third_party
dependencies that need to be distributed with the built images as required by
their licenses. However, our publish-triggers-release task [1] already includes the
source code of all of our dependencies in the image rendering the separate
third_party directory redundant.

This is a port of tektoncd/pipeline#6416

[1] https://github.com/tektoncd/triggers/blob/2713832b025b804db4d8d7af48e4f944511b490f/tekton/publish.yaml#L104-L117

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
dibyom added a commit to dibyom/triggers that referenced this pull request May 12, 2023
The third_party directory contains source code of some of our third_party
dependencies that need to be distributed with the built images as required by
their licenses. However, our publish-triggers-release task [1] already includes the
source code of all of our dependencies in the image rendering the separate
third_party directory redundant.

This is a port of tektoncd/pipeline#6416

[1] https://github.com/tektoncd/triggers/blob/2713832b025b804db4d8d7af48e4f944511b490f/tekton/publish.yaml#L104-L117

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
tekton-robot pushed a commit to tektoncd/triggers that referenced this pull request May 31, 2023
The third_party directory contains source code of some of our third_party
dependencies that need to be distributed with the built images as required by
their licenses. However, our publish-triggers-release task [1] already includes the
source code of all of our dependencies in the image rendering the separate
third_party directory redundant.

This is a port of tektoncd/pipeline#6416

[1] https://github.com/tektoncd/triggers/blob/2713832b025b804db4d8d7af48e4f944511b490f/tekton/publish.yaml#L104-L117

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/misc Categorizes issue or PR as a miscellaneuous one. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
7 participants