Merge pull request #43575 from ballerina-platform/master

Sync branches

compiler/ballerina-lang/src/main/java/io/ballerina/projects/internal/BlendedManifest.java

@@ -35,6 +35,7 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -132,11 +133,12 @@ public static BlendedManifest from(DependencyManifest dependencyManifest,
                     }
                 }
             } else {
+                Collection<String> moduleNames = existingDepOptional.isPresent() ?
+                        existingDepOptional.get().modules : Collections.emptyList();
                 depContainer.add(depInPkgManifest.org(), depInPkgManifest.name(), new Dependency(
                         depInPkgManifest.org(), depInPkgManifest.name(), depInPkgManifest.version(),
                         DependencyRelation.UNKNOWN, REPOSITORY_NOT_SPECIFIED,
-                        moduleNames(new DependencyManifest.Package(depInPkgManifest.name(), depInPkgManifest.org(),
-                                depInPkgManifest.version())), DependencyOrigin.USER_SPECIFIED));
+                        moduleNames, DependencyOrigin.USER_SPECIFIED));
                 continue;
             }
 

compiler/ballerina-lang/src/test/java/io/ballerina/projects/test/resolution/packages/ExistingProjectTests.java

@@ -60,9 +60,13 @@ public static Object[][] testCaseProvider() {
                 // 14. package contains 2 dependencies one of which is in Ballerina toml file thats not local
                 {"suite-existing_project", "case-0014", true},
                 {"suite-existing_project", "case-0014", false},
-                // 15. package updates transitive dependency from the Ballerian toml file that is not local
+                // 15. package updates transitive dependency from the Ballerina toml file that is not local
                 {"suite-existing_project", "case-0015", true},
-                {"suite-existing_project", "case-0015", false}
+                {"suite-existing_project", "case-0015", false},
+                // 16. package name is hierarchical, there are new versions in the central,
+                // and the older version is specified in Ballerina.toml and Dependencies.toml
+                {"suite-existing_project", "case-0016", true},
+                {"suite-existing_project", "case-0016", false}
         };
     }
 }

compiler/ballerina-lang/src/test/java/io/ballerina/projects/test/resolution/packages/internal/PackageResolutionTestCaseBuilder.java

@@ -156,8 +156,9 @@ private static DependencyManifest getDependencyManifest(Path dependenciesTomlPat
             }
 
             PackageDescriptor pkgDesc = Utils.getPkgDescFromNode(node.name().value(), null);
+            List<DependencyManifest.Module> modules = Utils.getDependencyModules(pkgDesc, attrs.get("modules"));
             recordedDeps.add(new DependencyManifest.Package(pkgDesc.name(), pkgDesc.org(), pkgDesc.version(),
-                    scope.getValue(), isTransitive, Collections.emptyList(), Collections.emptyList()));
+                    scope.getValue(), isTransitive, Collections.emptyList(), modules));
         }
         return DependencyManifest.from("2.0.0", null, recordedDeps, Collections.emptyList());
     }

compiler/ballerina-lang/src/test/java/io/ballerina/projects/test/resolution/packages/internal/Utils.java

@@ -17,6 +17,7 @@
  */
 package io.ballerina.projects.test.resolution.packages.internal;
 
+import io.ballerina.projects.DependencyManifest;
 import io.ballerina.projects.DependencyResolutionType;
 import io.ballerina.projects.ModuleName;
 import io.ballerina.projects.PackageDependencyScope;
@@ -26,6 +27,10 @@
 import io.ballerina.projects.PackageVersion;
 import io.ballerina.projects.environment.ModuleLoadRequest;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
 /**
  * Contains utility methods used throughout the test framework.
  *
@@ -49,6 +54,19 @@ public static PackageDependencyScope getDependencyScope(Object scopeValue) {
         }
     }
 
+    public static List<DependencyManifest.Module> getDependencyModules(PackageDescriptor pkgDesc, Object modulesValue) {
+        if (modulesValue == null) {
+            return Collections.emptyList();
+        }
+        List<DependencyManifest.Module> modules = new ArrayList<>();
+        String modulesStr = modulesValue.toString();
+        String[] moduleNames = modulesStr.split(",");
+        for (String moduleName : moduleNames) {
+            modules.add(new DependencyManifest.Module(pkgDesc.org().value(), pkgDesc.name().value(), moduleName));
+        }
+        return modules;
+    }
+
     public static ModuleLoadRequest getModuleLoadRequest(String name,
                                                          PackageDependencyScope scope,
                                                          DependencyResolutionType resolutionType) {

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/case-0016/Ballerina_toml.dot

@@ -0,0 +1,3 @@
+digraph "BallerinaToml" {
+    "samjs/qux.foo:1.0.2"
+}

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/case-0016/Dependencies_toml.dot

@@ -0,0 +1,5 @@
+digraph "example1" {
+    "samejs/app:0.1.0" -> "samjs/qux.foo:1.0.2"
+
+    "samjs/qux.foo:1.0.2" [modules = "qux.foo"]
+}

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/case-0016/app.dot

@@ -0,0 +1,3 @@
+digraph "samejs/app:0.1.0" {
+    "samjs/qux.foo"
+}

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/case-0016/case-description.md

@@ -0,0 +1,13 @@
+# Package name is hierarchical, there are new versions in the central, and the older version is specified in Ballerina.toml and Dependencies.toml
+
+1. User's package has `samjs/qux.foo:1.0.2` as a dependency in Dependencies.toml.
+2. A newer version `samjs/qux.foo:1.0.5` has been released to central.
+3. User specifies `samjs/qux.foo:1.0.2` in Ballerina.toml
+4. User now builds the package
+
+## Expected behavior
+
+### Sticky == true
+No changes to Dependency graph
+### Sticky == false
+Dependency graph should be updated to have `samjs/qux.foo:1.0.5`

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/case-0016/expected-graph-nosticky.dot

@@ -0,0 +1,3 @@
+digraph "example1" {
+    "samejs/app:0.1.0" -> "samjs/qux.foo:1.0.5"
+}

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/case-0016/expected-graph-sticky.dot

@@ -0,0 +1,3 @@
+digraph "example1" {
+    "samejs/app:0.1.0" -> "samjs/qux.foo:1.0.2"
+}

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/repositories/central.dot

@@ -102,5 +102,11 @@ digraph central {
     subgraph "ballerina/http:1.4.0" {
         "ballerina/http:1.4.0" -> "ballerina/io:1.0.2"
     }
+
+    subgraph "samjs/qux.foo:1.0.2" {
+    }
+
+    subgraph "samjs/qux.foo:1.0.5" {
+    }
 }
 

misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/BreakpointProcessor.java

@@ -22,17 +22,15 @@
 import com.sun.jdi.ThreadReference;
 import com.sun.jdi.event.BreakpointEvent;
 import com.sun.jdi.request.BreakpointRequest;
-import com.sun.jdi.request.EventRequest;
 import com.sun.jdi.request.StepRequest;
 import org.ballerinalang.debugadapter.breakpoint.BalBreakpoint;
 import org.ballerinalang.debugadapter.breakpoint.LogMessage;
 import org.ballerinalang.debugadapter.breakpoint.TemplateLogMessage;
-import org.ballerinalang.debugadapter.config.ClientConfigHolder;
-import org.ballerinalang.debugadapter.config.ClientLaunchConfigHolder;
 import org.ballerinalang.debugadapter.evaluation.BExpressionValue;
 import org.ballerinalang.debugadapter.evaluation.DebugExpressionEvaluator;
 import org.ballerinalang.debugadapter.evaluation.EvaluationException;
 import org.ballerinalang.debugadapter.evaluation.EvaluationExceptionKind;
+import org.ballerinalang.debugadapter.jdi.JDIUtils;
 import org.ballerinalang.debugadapter.jdi.JdiProxyException;
 import org.ballerinalang.debugadapter.jdi.StackFrameProxyImpl;
 import org.ballerinalang.debugadapter.jdi.ThreadReferenceProxyImpl;
@@ -48,6 +46,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
@@ -107,9 +106,10 @@ void processBreakpointEvent(BreakpointEvent bpEvent) {
         // need to internally step out if we are at the last line of a function, in order to ignore having debug hits
         // on the last line.
         if (requireStepOut(bpEvent)) {
-            activateDynamicBreakPoints((int) bpEvent.thread().uniqueID(), DynamicBreakpointMode.CALLER);
+            activateDynamicBreakPoints((int) bpEvent.thread().uniqueID(), DynamicBreakpointMode.CALLER, true);
+            context.setPrevInstruction(DebugInstruction.STEP_OVER);
             context.getDebuggeeVM().resume();
-        } else if (context.getLastInstruction() != null && context.getLastInstruction() != DebugInstruction.CONTINUE) {
+        } else if (context.getPrevInstruction() != null && context.getPrevInstruction() != DebugInstruction.CONTINUE) {
             jdiEventProcessor.notifyStopEvent(bpEvent);
         } else if (fileBreakpoints == null || !fileBreakpoints.containsKey(lineNumber)) {
             jdiEventProcessor.notifyStopEvent(bpEvent);
@@ -168,19 +168,13 @@ private void processAdvanceBreakpoints(BreakpointEvent event, BalBreakpoint brea
     /**
      * Responsible for clearing dynamic(temporary) breakpoints used for step over instruction and for restoring the
      * original user breakpoints before proceeding with the other debug instructions.
-     *
-     * @param instruction debug instruction
      */
-    void restoreUserBreakpoints(DebugInstruction instruction) {
+    void restoreUserBreakpoints() {
         if (context.getDebuggeeVM() == null) {
             return;
         }
-
         context.getEventManager().deleteAllBreakpoints();
-        if (instruction == DebugInstruction.CONTINUE || instruction == DebugInstruction.STEP_OVER) {
-            context.getDebuggeeVM().allClasses().forEach(referenceType ->
-                    activateUserBreakPoints(referenceType, false));
-        }
+        context.getDebuggeeVM().allClasses().forEach(classRef -> activateUserBreakPoints(classRef, false));
     }
 
     /**
@@ -191,13 +185,6 @@ void restoreUserBreakpoints(DebugInstruction instruction) {
      */
     void activateUserBreakPoints(ReferenceType referenceType, boolean shouldNotify) {
         try {
-            // avoids setting break points if the server is running in 'no-debug' mode.
-            ClientConfigHolder configHolder = context.getAdapter().getClientConfigHolder();
-            if (configHolder instanceof ClientLaunchConfigHolder
-                    && ((ClientLaunchConfigHolder) configHolder).isNoDebugMode()) {
-                return;
-            }
-
             String qualifiedClassName = getQualifiedClassName(referenceType);
             if (!userBreakpoints.containsKey(qualifiedClassName)) {
                 return;
@@ -206,6 +193,7 @@ void activateUserBreakPoints(ReferenceType referenceType, boolean shouldNotify)
             for (BalBreakpoint breakpoint : breakpoints.values()) {
                 List<Location> locations = referenceType.locationsOfLine(breakpoint.getLine());
                 if (!locations.isEmpty()) {
+                    // TODO: should we consider the last location instead?
                     Location loc = locations.get(0);
                     BreakpointRequest bpReq = context.getEventManager().createBreakpointRequest(loc);
                     bpReq.enable();
@@ -230,23 +218,32 @@ void activateUserBreakPoints(ReferenceType referenceType, boolean shouldNotify)
      * Activates dynamic/temporary breakpoints (which will be used to process the STEP-OVER instruction) via Java Debug
      * Interface(JDI).
      *
-     * @param threadId ID of the active java thread in oder to configure dynamic breakpoint on the active stack trace
-     * @param mode     dynamic breakpoint mode
+     * @param threadId ID of the active java thread in oder to configure dynamic breakpoint on the active stack
+     *                 trace.
+     * @param mode     dynamic breakpoint mode.
+     * @param validate If true, validates whether the dynamic breakpoints are already applied before activating dynamic
+     *                 breakpoints. This is to optimize the performance by avoiding redundant breakpoint activations.
      */
-    void activateDynamicBreakPoints(int threadId, DynamicBreakpointMode mode) {
-        ThreadReferenceProxyImpl threadReference = context.getAdapter().getAllThreads().get(threadId);
+    void activateDynamicBreakPoints(int threadId, DynamicBreakpointMode mode, boolean validate) {
         try {
+            ThreadReferenceProxyImpl threadReference = context.getAdapter().getAllThreads().get(threadId);
             List<StackFrameProxyImpl> jStackFrames = threadReference.frames();
             List<BallerinaStackFrame> validFrames = jdiEventProcessor.filterValidBallerinaFrames(jStackFrames);
 
             if (mode == DynamicBreakpointMode.CURRENT && !validFrames.isEmpty()) {
-                configureBreakpointsForMethod(validFrames.get(0));
+                Location currentLocation = validFrames.get(0).getJStackFrame().location();
+                Optional<Location> prevLocation = context.getPrevLocation();
+                if (!validate || prevLocation.isEmpty() || !isWithinSameSource(currentLocation, prevLocation.get())) {
+                    doActivateDynamicBreakPoints(currentLocation);
+                }
+                context.setPrevLocation(currentLocation);
             }
+
             // If the current function is invoked within another ballerina function, we need to explicitly set another
             // temporary breakpoint on the location of its invocation. This is supposed to handle the situations where
             // the user wants to step over on an exit point of the current function.
             if (mode == DynamicBreakpointMode.CALLER && validFrames.size() > 1) {
-                configureBreakpointsForMethod(validFrames.get(1));
+                doActivateDynamicBreakPoints(validFrames.get(1).getJStackFrame().location());
             }
         } catch (JdiProxyException e) {
             LOGGER.error(e.getMessage());
@@ -257,16 +254,35 @@ void activateDynamicBreakPoints(int threadId, DynamicBreakpointMode mode) {
         }
     }
 
+    private void doActivateDynamicBreakPoints(Location location) {
+        context.getEventManager().deleteAllBreakpoints();
+        configureBreakpointsForMethod(location);
+        context.setPrevLocation(location);
+    }
+
+    /**
+     * Checks whether the given two locations are within the same source file.
+     *
+     * @param currentLocation current location
+     * @param prevLocation    previous location
+     * @return true if the given two locations are within the same source file, false otherwise
+     */
+    private boolean isWithinSameSource(Location currentLocation, Location prevLocation) {
+        try {
+            return Objects.equals(currentLocation.sourcePath(), prevLocation.sourcePath());
+        } catch (AbsentInformationException e) {
+            return false;
+        }
+    }
+
     /**
      * Configures temporary(dynamic) breakpoints for all the lines within the method, which encloses the given stack
      * frame location. This strategy is used when processing STEP_OVER requests.
      *
-     * @param balStackFrame stack frame which contains the method information
+     * @param currentLocation current stack frame location
      */
-    private void configureBreakpointsForMethod(BallerinaStackFrame balStackFrame) {
+    private void configureBreakpointsForMethod(Location currentLocation) {
         try {
-            Location currentLocation = balStackFrame.getJStackFrame().location();
-            ReferenceType referenceType = currentLocation.declaringType();
             List<Location> allLocations = currentLocation.method().allLineLocations();
             Optional<Location> firstLocation = allLocations.stream()
                     .filter(location -> location.lineNumber() > 0)
@@ -278,7 +294,7 @@ private void configureBreakpointsForMethod(BallerinaStackFrame balStackFrame) {
 
             int nextStepPoint = firstLocation.get().lineNumber();
             do {
-                List<Location> locations = referenceType.locationsOfLine(nextStepPoint);
+                List<Location> locations = currentLocation.method().locationsOfLine(nextStepPoint);
                 if (!locations.isEmpty() && (locations.get(0).lineNumber() > firstLocation.get().lineNumber())) {
                     // Checks whether there are any user breakpoint configured for the same location, before adding the
                     // dynamic breakpoint.
@@ -291,7 +307,7 @@ private void configureBreakpointsForMethod(BallerinaStackFrame balStackFrame) {
                 }
                 nextStepPoint++;
             } while (nextStepPoint <= lastLocation.get().lineNumber());
-        } catch (AbsentInformationException | JdiProxyException e) {
+        } catch (AbsentInformationException e) {
             LOGGER.error(e.getMessage());
         }
     }
@@ -362,8 +378,7 @@ private BExpressionValue evaluateExpressionSafely(String expression, ThreadRefer
         // the new event. If this new EventSet is in 'SUSPEND_ALL' mode, then a deadlock will occur because no one
         // will resume the EventSet. Therefore to avoid this, we are disabling possible event requests before doing
         // the condition evaluation.
-        context.getEventManager().classPrepareRequests().forEach(EventRequest::disable);
-        context.getEventManager().breakpointRequests().forEach(BreakpointRequest::disable);
+        JDIUtils.disableJDIRequests(context);
 
         ThreadReferenceProxyImpl thread = context.getAdapter().getAllThreads().get((int) threadReference.uniqueID());
         List<BallerinaStackFrame> validFrames = jdiEventProcessor.filterValidBallerinaFrames(thread.frames());
@@ -377,16 +392,16 @@ private BExpressionValue evaluateExpressionSafely(String expression, ThreadRefer
         evaluator.setExpression(expression);
         BExpressionValue evaluationResult = evaluator.evaluate();
 
-        // As we are disabling all the breakpoint requests before evaluating the user's conditional
+        // As we disabled all the breakpoint requests before evaluating the user's conditional
         // expression, need to re-enable all the breakpoints before continuing the remote VM execution.
-        restoreUserBreakpoints(context.getLastInstruction());
+        JDIUtils.enableJDIRequests(context);
         return evaluationResult;
     }
 
     private boolean requireStepOut(BreakpointEvent event) {
         try {
-            if (context.getLastInstruction() != DebugInstruction.STEP_OVER
-                    && context.getLastInstruction() != DebugInstruction.STEP_OUT) {
+            if (context.getPrevInstruction() != DebugInstruction.STEP_OVER
+                    && context.getPrevInstruction() != DebugInstruction.STEP_OUT) {
                 return false;
             }
             Location currentLocation = event.location();
@@ -415,7 +430,7 @@ private void notifyBreakPointChangesToClient(BalBreakpoint balBreakpoint) {
     /**
      * Dynamic Breakpoint Options.
      */
-    enum DynamicBreakpointMode {
+    public enum DynamicBreakpointMode {
         /**
          * Configures dynamic breakpoints only for the current method (active stack frame).
          */