Avoid pruning unused dependencies for buf dep update#2966
Merged
Conversation
buf dep updatebuf dep update
bufdev
reviewed
May 11, 2024
Member
Author
|
Found something in testing today: when pulling in a new dependency that is not used yet, we do not pull in its transitive dependencies... I think this is expected/fine since you may not end up needing all transitive dependencies from the new dependency. But I wanted to note this. |
saquibmian
reviewed
May 13, 2024
Contributor
saquibmian
left a comment
There was a problem hiding this comment.
I don't think this is right.
Co-authored-by: Saquib Mian <smian@buf.build>
bufdev
reviewed
May 14, 2024
pkwarren
reviewed
May 15, 2024
Member
pkwarren
left a comment
There was a problem hiding this comment.
Changes look good - two minor comments.
Co-authored-by: Philip K. Warren <pkwarren@users.noreply.github.com>
pkwarren
approved these changes
May 15, 2024
saquibmian
approved these changes
May 15, 2024
bufdev
approved these changes
May 15, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When running
buf mod update, we want to include in thebuf.lockdepsthat have been configured in
buf.yamland may not be used yet. This is tosupport workflows where a user can configure a dependency and
buf dep updatebefore they use the newly configured dep in their proto definitions.
Prunedoes the following things right now:buf.lockbuf.yamlthat are present inbuf.lockThe first step is necessary for
buf dep updatefor tamper-proofing and validatingthe build. However, the latter two steps are unnecessary -- we want to keep
unused dependencies and their transitive dependencies, and since
dep updategets the dependencies from
buf.yamland writes them intobuf.lock, therewill not be dependencies present not from the
buf.yaml, so that step is ano-op.
So instead of running
Prune, we validate the build after the logic fordep udpateand we log out the unused dependencies as a warning to the users (but we do not
remove them).