Fix DeclarationAssembler to correctly merge fragments with different offsets
PiperOrigin-RevId: 298723049
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/file/DeclarationAssembler.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/file/DeclarationAssembler.java
index 2a843ba..e5c4919 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/file/DeclarationAssembler.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/file/DeclarationAssembler.java
@@ -114,6 +114,20 @@
ByteBufferFragment merged = ByteBufferFragment.merge(fragments);
+ int newOffset;
+ if (Iterables.getLast(list).getBufferOffset() == firstOffset) {
+ // If all fragment offsets were the same (which is the case if all their originating buffers
+ // were the same), then the merged fragment has the start index of the first originating
+ // fragment, and the end index of the last originating fragment.
+ // The old offset can be used without issue.
+ newOffset = firstOffset;
+ } else {
+ // If fragment offsets differed, then the new merged fragment has a different offset.
+ // In this case, the merged fragment has relative indices [0, len), which means that its
+ // offset should reflect the absolute "real" start offset.
+ newOffset = first.getFragmentOffset();
+ }
+
int previousEnd = 0;
for (Range<Integer> range : interestingRanges) {
int idx =
@@ -123,11 +137,11 @@
// starting from the connection point between first and second fragments.
Preconditions.checkState(idx > previousEnd);
declarationConsumer.declaration(
- new ByteFragmentAtOffset(firstOffset, merged.subFragment(previousEnd, idx + 1)));
+ new ByteFragmentAtOffset(newOffset, merged.subFragment(previousEnd, idx + 1)));
previousEnd = idx + 1;
}
}
declarationConsumer.declaration(
- new ByteFragmentAtOffset(firstOffset, merged.subFragment(previousEnd, merged.length())));
+ new ByteFragmentAtOffset(newOffset, merged.subFragment(previousEnd, merged.length())));
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/DeclarationAssemblerTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/DeclarationAssemblerTest.java
index 480d1ca..770ecaf 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/DeclarationAssemblerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/DeclarationAssemblerTest.java
@@ -16,13 +16,17 @@
package com.google.devtools.build.lib.bazel.rules.ninja;
import static com.google.common.truth.Truth.assertThat;
+import static java.nio.charset.StandardCharsets.ISO_8859_1;
+import com.google.common.base.Strings;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.bazel.rules.ninja.file.ByteBufferFragment;
import com.google.devtools.build.lib.bazel.rules.ninja.file.ByteFragmentAtOffset;
import com.google.devtools.build.lib.bazel.rules.ninja.file.DeclarationAssembler;
import com.google.devtools.build.lib.bazel.rules.ninja.file.GenericParsingException;
import com.google.devtools.build.lib.bazel.rules.ninja.file.NinjaSeparatorFinder;
+import com.google.devtools.build.lib.util.Pair;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
@@ -54,6 +58,38 @@
doTwoBuffersTest("abc$", "\ndef", "abc$\ndef");
}
+ @Test
+ public void testMergeTwoDifferentBuffers() throws Exception {
+ List<Pair<Integer, String>> offsetStringPairList = Lists.newArrayList();
+ String unrelatedFirstBuffer = Strings.repeat(" ", 100);
+ String s1 = "hello";
+ String s2 = "goodbye";
+ byte[] chars1 = (unrelatedFirstBuffer + s1).getBytes(ISO_8859_1);
+ byte[] chars2 = s2.getBytes(ISO_8859_1);
+
+ DeclarationAssembler assembler =
+ new DeclarationAssembler(
+ (byteFragmentAtOffset) -> {
+ offsetStringPairList.add(
+ new Pair<>(
+ byteFragmentAtOffset.getFragmentOffset(),
+ byteFragmentAtOffset.getFragment().toString()));
+ },
+ NinjaSeparatorFinder.INSTANCE);
+
+ assembler.wrapUp(
+ Lists.newArrayList(
+ new ByteFragmentAtOffset(
+ 0,
+ new ByteBufferFragment(
+ ByteBuffer.wrap(chars1), unrelatedFirstBuffer.length(), chars1.length)),
+ new ByteFragmentAtOffset(
+ chars1.length, new ByteBufferFragment(ByteBuffer.wrap(chars2), 0, s2.length()))));
+
+ assertThat(Iterables.getOnlyElement(offsetStringPairList))
+ .isEqualTo(new Pair<>(unrelatedFirstBuffer.length(), "hellogoodbye"));
+ }
+
private static void doTwoBuffersTest(String s1, String s2, String... expected)
throws GenericParsingException, IOException {
List<String> list = Lists.newArrayList();
