From d2753d249b121aab17314be482923ec6ae639439 Mon Sep 17 00:00:00 2001 From: Doria Keung Date: Tue, 3 Dec 2024 14:41:34 -0500 Subject: [PATCH] Address comments --- .../bufimage/bufimageutil/bufimageutil.go | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/private/bufpkg/bufimage/bufimageutil/bufimageutil.go b/private/bufpkg/bufimage/bufimageutil/bufimageutil.go index 47602a4ce8..68b4d0d796 100644 --- a/private/bufpkg/bufimage/bufimageutil/bufimageutil.go +++ b/private/bufpkg/bufimage/bufimageutil/bufimageutil.go @@ -269,23 +269,18 @@ func ImageFilteredByTypesWithOptions(image bufimage.Image, types []string, opts // Only handle imports and dependencies if there are any. for indexFrom, importPath := range imageFileDescriptor.GetDependency() { path := append(basePath, int32(indexFrom)) - // If there are no required imports, we mark this as deleted. - if importsRequired == nil { - sourcePathRemapper.markDeleted(path) + // We check if the import path exists among required imports. If yes, we + // move and then delete from required imports as we go. + if importsRequired != nil && importsRequired.index(importPath) != -1 { + sourcePathRemapper.markMoved(path, int32(indexTo)) + indexFromTo[int32(indexFrom)] = int32(indexTo) + imageFileDescriptor.Dependency[indexTo] = importPath + indexTo++ + // delete them as we go, so we know which ones weren't in the list + importsRequired.delete(importPath) } else { - // Otherwise, we check if the import path exists among required imports. If yes, we - // move and then delete from required imports as we go. - if i := importsRequired.index(importPath); i != -1 { - sourcePathRemapper.markMoved(path, int32(indexTo)) - indexFromTo[int32(indexFrom)] = int32(indexTo) - imageFileDescriptor.Dependency[indexTo] = importPath - indexTo++ - // delete them as we go, so we know which ones weren't in the list - importsRequired.delete(importPath) - } else { - // Path did not exist in required imports, we mark as deleted. - sourcePathRemapper.markDeleted(path) - } + // Path did not exist in required imports, we mark as deleted. + sourcePathRemapper.markDeleted(path) } } imageFileDescriptor.Dependency = imageFileDescriptor.Dependency[:indexTo] @@ -293,6 +288,8 @@ func ImageFilteredByTypesWithOptions(image bufimage.Image, types []string, opts // Add any other imports (which may not have been in the list because // they were picked up via a public import). The filtered files will not // use public imports. + // The imports are added in the order they are encountered when importing + // to maintain a deterministic ordering. if importsRequired != nil { imageFileDescriptor.Dependency = append(imageFileDescriptor.Dependency, importsRequired.keys()...) }