Circular warning logic caused $! to be set, breaking test autorun.

jruby · Apr 15, 2015 · 088b8d4 · 088b8d4
1 parent f2d2fa8
commit 088b8d4
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 28 deletions.
diff --git a/core/src/main/java/org/jruby/runtime/ThreadContext.java b/core/src/main/java/org/jruby/runtime/ThreadContext.java
@@ -48,6 +48,7 @@
 import org.jruby.internal.runtime.methods.DynamicMethod;
 import org.jruby.lexer.yacc.ISourcePosition;
 import org.jruby.parser.StaticScope;
+import org.jruby.runtime.backtrace.BacktraceData;
 import org.jruby.runtime.backtrace.BacktraceElement;
 import org.jruby.runtime.backtrace.RubyStackTraceElement;
 import org.jruby.runtime.backtrace.TraceType;
@@ -635,11 +636,18 @@ public IRubyObject getConstant(String internedName) {
         return getCurrentStaticScope().getConstant(internedName);
     }
 
-    private static void addBackTraceElement(Ruby runtime, RubyArray backtrace, RubyStackTraceElement element) {
-        RubyString str = RubyString.newString(runtime, element.mriStyleString());
-        backtrace.append(str);
+    /**
+     * Render the current backtrace as a string to the given StringBuilder. This will honor the currently-configured
+     * backtrace format and content.
+     *
+     * @param sb the StringBuilder to which to render the backtrace
+     */
+    public void renderCurrentBacktrace(StringBuilder sb) {
+        TraceType traceType = runtime.getInstanceConfig().getTraceType();
+        BacktraceData backtraceData = traceType.getBacktrace(this, false);
+        traceType.getFormat().renderBacktrace(backtraceData.getBacktrace(runtime), sb, false);
     }
-    
+
     /**
      * Create an Array with backtrace information for Kernel#caller
      * @param level
@@ -656,7 +664,8 @@ public IRubyObject createCallerBacktrace(int level, Integer length, StackTraceEl
         RubyArray newTrace = runtime.newArray(trace.length);
 
         for (int i = level; i - level < trace.length; i++) {
-            addBackTraceElement(runtime, newTrace, trace[i - level]);
+            RubyString str = RubyString.newString(runtime, trace[i - level].mriStyleString());
+            newTrace.append(str);
         }
 
         if (RubyInstanceConfig.LOG_CALLERS) TraceType.dumpCaller(newTrace);

diff --git a/core/src/main/java/org/jruby/runtime/backtrace/BacktraceData.java b/core/src/main/java/org/jruby/runtime/backtrace/BacktraceData.java
@@ -1,7 +1,6 @@
 package org.jruby.runtime.backtrace;
 
 import org.jruby.Ruby;
-import org.jruby.compiler.JITCompiler;
 import org.jruby.util.JavaNameMangler;
 
 import java.io.Serializable;
@@ -37,12 +36,12 @@ public BacktraceData(StackTraceElement[] javaTrace, BacktraceElement[] rubyTrace
 
     public RubyStackTraceElement[] getBacktrace(Ruby runtime) {
         if (backtraceElements == null) {
-            backtraceElements = transformBacktrace(runtime.getBoundMethods());
+            backtraceElements = constructBacktrace(runtime.getBoundMethods());
         }
         return backtraceElements;
     }
 
-    private RubyStackTraceElement[] transformBacktrace(Map<String, Map<String, String>> boundMethods) {
+    private RubyStackTraceElement[] constructBacktrace(Map<String, Map<String, String>> boundMethods) {
         List<RubyStackTraceElement> trace = new ArrayList<RubyStackTraceElement>(javaTrace.length);
 
         // used for duplicating the previous Ruby frame when masking native calls

diff --git a/core/src/main/java/org/jruby/runtime/backtrace/TraceType.java b/core/src/main/java/org/jruby/runtime/backtrace/TraceType.java
@@ -28,6 +28,14 @@ public TraceType(Gather gather, Format format) {
         this.format = format;
     }
 
+    public Gather getGather() {
+        return gather;
+    }
+
+    public Format getFormat() {
+        return format;
+    }
+
     /**
      * Get a normal Ruby backtrace, using the current Gather type.
      *
@@ -214,6 +222,10 @@ public enum Format {
             public String printBacktrace(RubyException exception, boolean console) {
                 return printBacktraceMRI(exception, console);
             }
+
+            public void renderBacktrace(RubyStackTraceElement[] elts, StringBuilder buffer, boolean color) {
+                renderBacktraceMRI(elts, buffer, color);
+            }
         },
 
         /**
@@ -223,9 +235,14 @@ public String printBacktrace(RubyException exception, boolean console) {
             public String printBacktrace(RubyException exception, boolean console) {
                 return printBacktraceJRuby(exception, console);
             }
+
+            public void renderBacktrace(RubyStackTraceElement[] elts, StringBuilder buffer, boolean color) {
+                renderBacktraceJRuby(elts, buffer, color);
+            }
         };
 
         public abstract String printBacktrace(RubyException exception, boolean console);
+        public abstract void renderBacktrace(RubyStackTraceElement[] elts, StringBuilder buffer, boolean color);
     }
 
     protected static String printBacktraceMRI(RubyException exception, boolean console) {
@@ -304,16 +321,9 @@ protected static String printBacktraceMRI(RubyException exception, boolean conso
 
     protected static String printBacktraceJRuby(RubyException exception, boolean console) {
         Ruby runtime = exception.getRuntime();
-        RubyStackTraceElement[] frames = exception.getBacktraceElements();
-        if (frames == null) frames = new RubyStackTraceElement[0];
-
-        // find longest method name
-        int longestMethod = 0;
-        for (RubyStackTraceElement frame : frames) {
-            longestMethod = Math.max(longestMethod, frame.getMethodName().length());
-        }
 
         StringBuilder buffer = new StringBuilder();
+        boolean color = console && runtime.getInstanceConfig().getBacktraceColor();
 
         // exception line
         String message = exception.message(runtime.getCurrentContext()).toString();
@@ -325,8 +335,21 @@ protected static String printBacktraceJRuby(RubyException exception, boolean con
                 .append(": ")
                 .append(message)
                 .append('\n');
-
-        boolean color = console && runtime.getInstanceConfig().getBacktraceColor();
+
+        RubyStackTraceElement[] frames = exception.getBacktraceElements();
+        if (frames == null) frames = RubyStackTraceElement.EMPTY_ARRAY;
+        renderBacktraceJRuby(frames, buffer, color);
+
+
+        return buffer.toString();
+    }
+
+    private static void renderBacktraceJRuby(RubyStackTraceElement[] frames, StringBuilder buffer, boolean color) {
+        // find longest method name
+        int longestMethod = 0;
+        for (RubyStackTraceElement frame : frames) {
+            longestMethod = Math.max(longestMethod, frame.getMethodName().length());
+        }
 
         // backtrace lines
         boolean first = true;
@@ -341,7 +364,7 @@ protected static String printBacktraceJRuby(RubyException exception, boolean con
                 }
                 first = false;
             }
-            
+
             buffer.append("  ");
 
             // method name
@@ -355,16 +378,28 @@ protected static String printBacktraceJRuby(RubyException exception, boolean con
                     .append(frame.getFileName())
                     .append(':')
                     .append(frame.getLineNumber());
-            
+
             if (color) {
                 buffer.append(CLEAR_COLOR);
             }
-            
+
             buffer
                     .append('\n');
         }
+    }
 
-        return buffer.toString();
+    private static void renderBacktraceMRI(RubyStackTraceElement[] trace, StringBuilder buffer, boolean color) {
+        for (int i = 0; i < trace.length; i++) {
+            RubyStackTraceElement element = trace[i];
+
+            buffer
+                    .append(element.getFileName())
+                    .append(':')
+                    .append(element.getLineNumber())
+                    .append(":in `")
+                    .append(element.getMethodName())
+                    .append("'\n");
+        }
     }
 
     public static IRubyObject generateMRIBacktrace(Ruby runtime, RubyStackTraceElement[] trace) {

diff --git a/core/src/main/java/org/jruby/runtime/load/LoadService.java b/core/src/main/java/org/jruby/runtime/load/LoadService.java
@@ -461,13 +461,12 @@ private void unlock(String requireName) {
     }
 
     protected void warnCircularRequire(String requireName) {
+        StringBuilder sb = new StringBuilder();
+
+        runtime.getCurrentContext().renderCurrentBacktrace(sb);
+
         runtime.getWarnings().warn("loading in progress, circular require considered harmful - " + requireName);
-        // it's a hack for c:rb_backtrace impl.
-        // We should introduce new method to Ruby.TraceType when rb_backtrace is widely used not only for this purpose.
-        RaiseException ex = new RaiseException(runtime, runtime.getRuntimeError(), null, false);
-        String trace = runtime.getInstanceConfig().getTraceType().printBacktrace(ex.getException(), runtime.getPosix().isatty(FileDescriptor.err));
-        // rb_backtrace dumps to stderr directly.
-        System.err.print(trace.replaceFirst("[^\n]*\n", ""));
+        runtime.getErr().print(sb.toString());
     }
 
     /**