Rollback of commit d7b64bd03100300b79cd33d04904ce9b0e6a5332.
*** Reason for rollback ***
Some rules write the toString representation of lists of files to disk and then read it back again. This breaks that.
--
MOS_MIGRATED_REVID=104524336
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 56da7ab..c4fd52e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -273,7 +273,14 @@
sb.append(obj).append(".");
}
sb.append(func);
- Printer.printList(sb, args, "(", ", ", ")", /* singletonTerminator */ null);
+ String backup = sb.toString();
+ try {
+ Printer.printList(sb, args, "(", ", ", ")", /* singletonTerminator */ null);
+ } catch (OutOfMemoryError ex) {
+ // export_files might lead to an OOM error (e.g. when deserializing very large packages).
+ // TODO(bazel-dev): make the Printer limit its own output.
+ return backup + "(<too long>)";
+ }
return sb.toString();
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
index 55e8556..22dcec8 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
@@ -70,10 +70,33 @@
return kind == Kind.TUPLE;
}
+ private static char startChar(Kind kind) {
+ switch(kind) {
+ case LIST: return '[';
+ case TUPLE: return '(';
+ }
+ return '[';
+ }
+
+ private static char endChar(Kind kind) {
+ switch(kind) {
+ case LIST: return ']';
+ case TUPLE: return ')';
+ }
+ return ']';
+ }
+
@Override
public String toString() {
StringBuilder sb = new StringBuilder();
- Printer.printList(sb, exprs, isTuple(), '"');
+ sb.append(startChar(kind));
+ String sep = "";
+ for (Expression e : exprs) {
+ sb.append(sep);
+ sb.append(e);
+ sep = ", ";
+ }
+ sb.append(endChar(kind));
return sb.toString();
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Printer.java b/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
index f607b11..0fa2964 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
@@ -13,10 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.syntax;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -30,8 +27,6 @@
import java.util.Set;
import java.util.SortedMap;
import java.util.TreeMap;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
/**
* (Pretty) Printing of Skylark values
@@ -40,19 +35,6 @@
private static final char SKYLARK_QUOTATION_MARK = '"';
- /*
- * Maximum number of list elements that should be printed via printList().
- */
- @VisibleForTesting
- static final int CRITICAL_LIST_ELEMENTS_COUNT = 10;
-
- /*
- * printList() will start to shorten the values of list elements when their string length reaches
- * this value.
- */
- @VisibleForTesting
- static final int CRITICAL_LIST_ELEMENTS_STRING_LENGTH = 128;
-
private Printer() {
}
@@ -275,54 +257,21 @@
String after,
String singletonTerminator,
char quotationMark) {
- return printList(buffer, list, before, separator, after, singletonTerminator, quotationMark,
- CRITICAL_LIST_ELEMENTS_COUNT, CRITICAL_LIST_ELEMENTS_STRING_LENGTH);
- }
-
- private static Appendable printList(
- Appendable buffer,
- Iterable<?> list,
- String before,
- String separator,
- String after,
- String singletonTerminator,
- char quotationMark,
- int maxItemsToPrint,
- int criticalItemsStringLength) {
boolean printSeparator = false; // don't print the separator before the first element
- boolean skipArgs = false;
- int items = Iterables.size(list);
int len = 0;
- // We don't want to print "1 more arguments", hence we don't skip arguments if there is only one
- // above the limit.
- int itemsToPrint = (items - maxItemsToPrint == 1) ? items : maxItemsToPrint;
-
- LengthLimitedAppendable appendable =
- LengthLimitedAppendable.create(buffer, criticalItemsStringLength);
- append(appendable, before);
- appendable.enforceLimit();
+ append(buffer, before);
for (Object o : list) {
- // We don't want to print "1 more arguments", even if we hit the string limit.
- if (len == itemsToPrint || (appendable.hasHitLimit() && len < items - 1)) {
- skipArgs = true;
- break;
- }
if (printSeparator) {
- append(appendable, separator);
+ append(buffer, separator);
}
- write(appendable, o, quotationMark);
+ write(buffer, o, quotationMark);
printSeparator = true;
len++;
}
- appendable.ignoreLimit();
- if (skipArgs) {
- append(appendable, separator);
- append(appendable, String.format("<%d more arguments>", items - len));
- }
if (singletonTerminator != null && len == 1) {
- append(appendable, singletonTerminator);
+ append(buffer, singletonTerminator);
}
- return append(appendable, after);
+ return append(buffer, after);
}
public static Appendable printList(Appendable buffer, Iterable<?> list, String before,
@@ -494,134 +443,4 @@
}
return buffer;
}
-
- /**
- * Helper class for {@code Appendable}s that want to limit the length of their input.
- *
- * <p>Instances of this class act as a proxy for one {@code Appendable} object and decide whether
- * the given input (or parts of it) can be written to the underlying {@code Appendable}, depending
- * on whether the specified maximum length has been met or not.
- */
- private static final class LengthLimitedAppendable implements Appendable {
-
- private static final ImmutableSet<Character> SPECIAL_CHARS =
- ImmutableSet.of(',', ' ', '"', '\'', ':', '(', ')', '[', ']', '{', '}');
-
- private static final Pattern ARGS_PATTERN = Pattern.compile("<\\d+ more arguments>");
-
- private final Appendable original;
- private int limit;
- private boolean ignoreLimit;
- private boolean previouslyShortened;
-
- private LengthLimitedAppendable(Appendable original, int limit) {
- this.original = original;
- this.limit = limit;
- }
-
- public static LengthLimitedAppendable create(Appendable original, int limit) {
- // We don't want to overwrite the limit if original is already an instance of this class.
- return (original instanceof LengthLimitedAppendable)
- ? (LengthLimitedAppendable) original : new LengthLimitedAppendable(original, limit);
- }
-
- @Override
- public Appendable append(CharSequence csq) throws IOException {
- if (ignoreLimit || hasOnlySpecialChars(csq)) {
- // Don't update limit.
- original.append(csq);
- previouslyShortened = false;
- } else {
- int length = csq.length();
- if (length <= limit) {
- limit -= length;
- original.append(csq);
- } else {
- original.append(csq, 0, limit);
- // We don't want to append multiple ellipses.
- if (!previouslyShortened) {
- original.append("...");
- }
- appendTrailingSpecialChars(csq, limit);
- previouslyShortened = true;
- limit = 0;
- }
- }
- return this;
- }
-
- /**
- * Appends any trailing "special characters" (e.g. brackets, quotation marks) in the given
- * sequence to the output buffer, regardless of the limit.
- *
- * <p>For example, let's look at foo(['too long']). Without this method, the shortened result
- * would be foo(['too...) instead of the prettier foo(['too...']).
- *
- * <p>If the input string was already shortened and contains "<x more arguments>", this part
- * will also be appended.
- */
- private void appendTrailingSpecialChars(CharSequence csq, int limit) throws IOException {
- int length = csq.length();
- Matcher matcher = ARGS_PATTERN.matcher(csq);
- // We assume that everything following the "x more arguments" part has to be copied, too.
- int start = matcher.find() ? matcher.start() : length;
- // Find the left-most non-arg char that has to be copied.
- for (int i = start - 1; i > limit; --i) {
- if (isSpecialChar(csq.charAt(i))) {
- start = i;
- } else {
- break;
- }
- }
- if (start < length) {
- original.append(csq, start, csq.length());
- }
- }
-
- /**
- * Returns whether the given sequence denotes characters that are not part of the value of an
- * argument.
- *
- * <p>Examples are brackets, braces and quotation marks.
- */
- private boolean hasOnlySpecialChars(CharSequence csq) {
- for (int i = 0; i < csq.length(); ++i) {
- if (!isSpecialChar(csq.charAt(i))) {
- return false;
- }
- }
- return true;
- }
-
- private boolean isSpecialChar(char c) {
- return SPECIAL_CHARS.contains(c);
- }
-
- @Override
- public Appendable append(CharSequence csq, int start, int end) throws IOException {
- return append(csq.subSequence(start, end));
- }
-
- @Override
- public Appendable append(char c) throws IOException {
- return append(String.valueOf(c));
- }
-
- public boolean hasHitLimit() {
- return limit <= 0;
- }
-
- public void enforceLimit() {
- ignoreLimit = false;
- }
-
- public void ignoreLimit() {
- ignoreLimit = true;
- }
-
- @Override
- public String toString() {
- return original.toString();
- }
- }
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java b/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
index 92e3d5e..925503e 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
@@ -18,7 +18,6 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
-import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
@@ -29,7 +28,6 @@
import java.util.Arrays;
import java.util.HashMap;
import java.util.IllegalFormatException;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@@ -168,73 +166,4 @@
assertThat(Printer.repr(dict, '\''))
.isEqualTo("{1: ('foo', 'bar'), 2: ['foo', 'bar'], 'foo': []}");
}
-
- @Test
- public void testListLimitStringLength() throws Exception {
- String limit = Strings.repeat("x", Printer.CRITICAL_LIST_ELEMENTS_STRING_LENGTH);
- String half = Strings.repeat("x", Printer.CRITICAL_LIST_ELEMENTS_STRING_LENGTH / 2);
-
- List<String> list = Arrays.asList(limit + limit);
-
- // String is way too long -> shorten.
- assertThat(Printer.str(list)).isEqualTo("[\"" + limit + "...\"]");
-
- LinkedList<List<String>> nestedList = new LinkedList<>();
- nestedList.add(list);
-
- // Same as above, but with one additional level of indirection.
- assertThat(Printer.str(nestedList)).isEqualTo("[[\"" + limit + "...\"]]");
-
- // The inner list alone would meet the limit, but because of the first element, it has to be
- // shortened.
- assertThat(Printer.str(Arrays.asList(half, Arrays.asList(limit))))
- .isEqualTo("[\"" + half + "\", [\"" + half + "...\"]]");
-
- // String is too long, but the ellipsis make it even longer.
- assertThat(Printer.str(Arrays.asList(limit + "x"))).isEqualTo("[\"" + limit + "...\"]");
-
- // We hit the limit exactly -> everything is printed.
- assertThat(Printer.str(Arrays.asList(limit))).isEqualTo("[\"" + limit + "\"]");
-
- // Exact hit, but with two arguments -> everything is printed.
- assertThat(Printer.str(Arrays.asList(half, half)))
- .isEqualTo("[\"" + half + "\", \"" + half + "\"]");
-
- // First argument hits the limit -> remaining argument is shortened.
- assertThat(Printer.str(Arrays.asList(limit, limit))).isEqualTo("[\"" + limit + "\", \"...\"]");
-
- String limitMinusOne = limit.substring(0, limit.length() - 1);
-
- // First arguments is one below the limit -> print first character of remaining argument.
- assertThat(Printer.str(Arrays.asList(limitMinusOne, limit)))
- .isEqualTo("[\"" + limitMinusOne + "\", \"x...\"]");
-
- // First argument hits the limit -> we skip the remaining two arguments.
- assertThat(Printer.str(Arrays.asList(limit, limit, limit)))
- .isEqualTo("[\"" + limit + "\", <2 more arguments>]");
- }
-
- @Test
- public void testListLimitTooManyArgs() throws Exception {
- StringBuilder builder = new StringBuilder();
- List<Integer> maxLength = new LinkedList<>();
-
- int next;
- for (next = 0; next < Printer.CRITICAL_LIST_ELEMENTS_COUNT; ++next) {
- maxLength.add(next);
- if (next > 0) {
- builder.append(", ");
- }
- builder.append(next);
- }
-
- // There is one too many, but we print every argument nonetheless.
- maxLength.add(next);
- assertThat(Printer.str(maxLength)).isEqualTo("[" + builder + ", " + next + "]");
-
- // There are two too many, hence we don't print them.
- ++next;
- maxLength.add(next);
- assertThat(Printer.str(maxLength)).isEqualTo("[" + builder + ", <2 more arguments>]");
- }
}
