Skip to content

Commit 533fe37

Browse files
Error when buf.yaml dep is missing from buf.lock
When a module is added to buf.yaml deps but buf dep update has not been run, the module is absent from buf.lock entirely. Previously this caused two bad outcomes depending on whether any proto files imported from the missing dep: - If proto files imported from it: a confusing protoc-level "file not found" error with no mention of buf.lock or what to do. - If proto files did not yet import from it: MalformedDepsForWorkspace would flag it as MalformedDepTypeUnused, suggesting the user remove the dep — the opposite of the right advice. Fix this by checking, during workspace construction, that every module declared in buf.yaml deps has a corresponding entry in buf.lock. If any are missing, return an actionable error immediately: "buf.testing/acme/extension" declared in buf.yaml deps but not present in buf.lock; run "buf dep update" to update the lock file The check is added to both the v1 and v2 workspace construction paths in workspace_provider.go and requires no network call — it is a pure comparison of module full names between the two config files. Two test cases are added: one where the missing dep is not yet used in any proto file (previously: silent success with a misleading "unused" warning), and one where the missing dep is actively imported (previously: cryptic "file not found" from the compiler).
1 parent 54e9748 commit 533fe37

8 files changed

Lines changed: 159 additions & 0 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
version: v2
2+
deps:
3+
- name: buf.testing/acme/date
4+
commit: ffded0b4cf6b47cab74da08d291a3c2f
5+
digest: b5:24ed4f13925cf89ea0ae0127fa28540704c7ae14750af027270221b737a1ce658f8014ca2555f6f7fcd95ea84e071d33f37f86cc36d07fe0d0963329a5ec2462
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
version: v2
2+
modules:
3+
- path: finance/bond/proto
4+
name: buf.testing/acme/bond
5+
deps:
6+
- buf.testing/acme/date
7+
- buf.testing/acme/extension
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
syntax = "proto3";
2+
3+
package acme.bond.v2;
4+
5+
import "google/protobuf/duration.proto";
6+
7+
message Bond {
8+
google.protobuf.Duration duration = 4;
9+
string name = 6;
10+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
version: v2
2+
deps:
3+
- name: buf.testing/acme/date
4+
commit: ffded0b4cf6b47cab74da08d291a3c2f
5+
digest: b5:24ed4f13925cf89ea0ae0127fa28540704c7ae14750af027270221b737a1ce658f8014ca2555f6f7fcd95ea84e071d33f37f86cc36d07fe0d0963329a5ec2462
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
version: v2
2+
modules:
3+
- path: finance/bond/proto
4+
name: buf.testing/acme/bond
5+
deps:
6+
- buf.testing/acme/date
7+
- buf.testing/acme/extension
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
syntax = "proto3";
2+
3+
package acme.bond.v2;
4+
5+
import "acme/extension/v1/extension.proto";
6+
7+
message Bond {
8+
option (acme.extension.v1.experimental) = true;
9+
10+
string name = 1;
11+
}

private/buf/bufworkspace/workspace_provider.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"fmt"
2121
"io/fs"
2222
"log/slog"
23+
"slices"
2324

2425
"buf.build/go/standard/xlog/xslog"
2526
"buf.build/go/standard/xslices"
@@ -323,6 +324,8 @@ func (w *workspaceProvider) getWorkspaceForBucketAndModuleDirPathsV1Beta1OrV1(
323324
v1WorkspaceTargeting *v1Targeting,
324325
) (*workspace, error) {
325326
moduleSetBuilder := bufmodule.NewModuleSetBuilder(ctx, w.logger, w.moduleDataProvider, w.commitProvider)
327+
var hadBufLock bool
328+
lockModuleFullNames := make(map[string]struct{})
326329
for _, moduleBucketAndTargeting := range v1WorkspaceTargeting.moduleBucketsAndTargeting {
327330
mappedModuleBucket := moduleBucketAndTargeting.bucket
328331
moduleTargeting := moduleBucketAndTargeting.moduleTargeting
@@ -356,13 +359,15 @@ func (w *workspaceProvider) getWorkspaceForBucketAndModuleDirPathsV1Beta1OrV1(
356359
default:
357360
return nil, syserror.Newf("unknown FileVersion: %v", fileVersion)
358361
}
362+
hadBufLock = true
359363
for _, depModuleKey := range bufLockFile.DepModuleKeys() {
360364
// DepModuleKeys from a BufLockFile is expected to have all transitive dependencies,
361365
// and we can rely on this property.
362366
moduleSetBuilder.AddRemoteModule(
363367
depModuleKey,
364368
false,
365369
)
370+
lockModuleFullNames[depModuleKey.FullName().String()] = struct{}{}
366371
}
367372
}
368373
v1BufYAMLObjectData, err := bufconfig.GetBufYAMLV1Beta1OrV1ObjectDataForPrefix(ctx, bucket, moduleTargeting.moduleDirPath)
@@ -386,6 +391,9 @@ func (w *workspaceProvider) getWorkspaceForBucketAndModuleDirPathsV1Beta1OrV1(
386391
// configs, however, we return this error as a safety check
387392
return nil, fmt.Errorf("no module config found for module at: %q", moduleTargeting.moduleDirPath)
388393
}
394+
if moduleFullName := moduleConfig.FullName(); moduleFullName != nil {
395+
lockModuleFullNames[moduleFullName.String()] = struct{}{}
396+
}
389397
moduleSetBuilder.AddLocalModule(
390398
mappedModuleBucket,
391399
moduleBucketAndTargeting.bucketID,
@@ -410,6 +418,11 @@ func (w *workspaceProvider) getWorkspaceForBucketAndModuleDirPathsV1Beta1OrV1(
410418
),
411419
)
412420
}
421+
if hadBufLock {
422+
if missingFromLock := depsInYAMLMissingFromLock(v1WorkspaceTargeting.allConfiguredDepModuleRefs, lockModuleFullNames); len(missingFromLock) > 0 {
423+
return nil, fmt.Errorf("%s declared in buf.yaml deps but not present in buf.lock; run \"buf dep update\" to update the lock file", xstrings.SliceToHumanStringQuoted(missingFromLock))
424+
}
425+
}
413426
moduleSet, err := moduleSetBuilder.Build()
414427
if err != nil {
415428
return nil, err
@@ -438,6 +451,7 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2(
438451
remotePolicyKeys []bufpolicy.PolicyKey
439452
policyNameToRemotePluginKeys map[string][]bufplugin.PluginKey
440453
)
454+
lockModuleFullNames := make(map[string]struct{})
441455
bufLockFile, err := bufconfig.GetBufLockFileForPrefix(
442456
ctx,
443457
bucket,
@@ -465,6 +479,7 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2(
465479
depModuleKey,
466480
false,
467481
)
482+
lockModuleFullNames[depModuleKey.FullName().String()] = struct{}{}
468483
}
469484
remotePluginKeys = bufLockFile.RemotePluginKeys()
470485
remotePolicyKeys = bufLockFile.RemotePolicyKeys()
@@ -502,6 +517,9 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2(
502517
return nil, fmt.Errorf("multiple module configs found with the same description: %s", moduleDescription)
503518
}
504519
seenModuleDescriptions[moduleDescription] = struct{}{}
520+
if moduleFullName := moduleConfig.FullName(); moduleFullName != nil {
521+
lockModuleFullNames[moduleFullName.String()] = struct{}{}
522+
}
505523
moduleSetBuilder.AddLocalModule(
506524
mappedModuleBucket,
507525
moduleBucketAndTargeting.bucketID,
@@ -518,6 +536,11 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2(
518536
bufmodule.LocalModuleWithDescription(moduleDescription),
519537
)
520538
}
539+
if bufLockFile != nil {
540+
if missingFromLock := depsInYAMLMissingFromLock(v2Targeting.bufYAMLFile.ConfiguredDepModuleRefs(), lockModuleFullNames); len(missingFromLock) > 0 {
541+
return nil, fmt.Errorf("%s declared in buf.yaml deps but not present in buf.lock; run \"buf dep update\" to update the lock file", xstrings.SliceToHumanStringQuoted(missingFromLock))
542+
}
543+
}
521544
moduleSet, err := moduleSetBuilder.Build()
522545
if err != nil {
523546
return nil, err
@@ -578,6 +601,19 @@ func (w *workspaceProvider) getWorkspaceForBucketModuleSet(
578601
), nil
579602
}
580603

604+
// depsInYAMLMissingFromLock returns the sorted list of module full name strings
605+
// that appear in configuredDepModuleRefs but are absent from lockModuleFullNames.
606+
func depsInYAMLMissingFromLock(configuredDepModuleRefs []bufparse.Ref, lockModuleFullNames map[string]struct{}) []string {
607+
var missing []string
608+
for _, ref := range configuredDepModuleRefs {
609+
if _, ok := lockModuleFullNames[ref.FullName().String()]; !ok {
610+
missing = append(missing, ref.FullName().String())
611+
}
612+
}
613+
slices.Sort(missing)
614+
return missing
615+
}
616+
581617
// This formats a module name based on its module config entry in the v2 buf.yaml:
582618
// `path: foo, includes: ["foo/v1, "foo/v2"], excludes: "foo/v1/internal"`.
583619
//

private/buf/bufworkspace/workspace_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,84 @@ func TestUnusedDep(t *testing.T) {
247247
require.Equal(t, MalformedDepTypeUnused, malformedDeps[1].Type())
248248
}
249249

250+
func TestDepMissingFromLock(t *testing.T) {
251+
t.Parallel()
252+
ctx := t.Context()
253+
254+
workspaceProvider := testNewWorkspaceProvider(
255+
t,
256+
bufmoduletesting.ModuleData{
257+
Name: "buf.testing/acme/date",
258+
DirPath: "testdata/basic/bsr/buf.testing/acme/date",
259+
},
260+
bufmoduletesting.ModuleData{
261+
Name: "buf.testing/acme/extension",
262+
DirPath: "testdata/basic/bsr/buf.testing/acme/extension",
263+
},
264+
)
265+
266+
storageosProvider := storageos.NewProvider()
267+
bucket, err := storageosProvider.NewReadWriteBucket("testdata/basic/workspace_dep_missing_from_lock")
268+
require.NoError(t, err)
269+
bucketTargeting, err := buftarget.NewBucketTargeting(
270+
ctx,
271+
slogtestext.NewLogger(t),
272+
bucket,
273+
".",
274+
nil,
275+
nil,
276+
buftarget.TerminateAtControllingWorkspace,
277+
)
278+
require.NoError(t, err)
279+
280+
_, err = workspaceProvider.GetWorkspaceForBucket(
281+
ctx,
282+
bucket,
283+
bucketTargeting,
284+
)
285+
require.ErrorContains(t, err, `"buf.testing/acme/extension" declared in buf.yaml deps but not present in buf.lock`)
286+
require.ErrorContains(t, err, `buf dep update`)
287+
}
288+
289+
func TestDepMissingFromLockUsed(t *testing.T) {
290+
t.Parallel()
291+
ctx := t.Context()
292+
293+
workspaceProvider := testNewWorkspaceProvider(
294+
t,
295+
bufmoduletesting.ModuleData{
296+
Name: "buf.testing/acme/date",
297+
DirPath: "testdata/basic/bsr/buf.testing/acme/date",
298+
},
299+
bufmoduletesting.ModuleData{
300+
Name: "buf.testing/acme/extension",
301+
DirPath: "testdata/basic/bsr/buf.testing/acme/extension",
302+
},
303+
)
304+
305+
storageosProvider := storageos.NewProvider()
306+
bucket, err := storageosProvider.NewReadWriteBucket("testdata/basic/workspace_dep_missing_from_lock_used")
307+
require.NoError(t, err)
308+
bucketTargeting, err := buftarget.NewBucketTargeting(
309+
ctx,
310+
slogtestext.NewLogger(t),
311+
bucket,
312+
".",
313+
nil,
314+
nil,
315+
buftarget.TerminateAtControllingWorkspace,
316+
)
317+
require.NoError(t, err)
318+
319+
_, err = workspaceProvider.GetWorkspaceForBucket(
320+
ctx,
321+
bucket,
322+
bucketTargeting,
323+
)
324+
require.ErrorContains(t, err, `"buf.testing/acme/extension" declared in buf.yaml deps but not present in buf.lock`)
325+
require.ErrorContains(t, err, `buf dep update`)
326+
}
327+
250328
func TestDuplicatePath(t *testing.T) {
251329
t.Parallel()
252330
ctx := t.Context()

0 commit comments

Comments
 (0)