gnodet-bot commented on code in PR #13180:
URL: https://github.com/apache/maven/pull/13180#discussion_r4276883912


##########
api/maven-api-core/src/main/java/org/apache/maven/api/build/report/BuildReport.java:
##########
@@ -0,0 +1,208 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.build.report;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+import org.apache.maven.api.BuildEnvironment;
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.services.BuilderProblem;
+
+/**
+ * A structured report of a Maven build execution, persisted to
+ * {@code target/build-report.json} at the end of every build.
+ * <p>
+ * The report captures metadata, per-module results (including mojo execution
+ * timings), and any failures. It is intended to be consumed by tools, IDEs,
+ * CI systems, and LLM agents without having to re-run the build or parse
+ * console output.
+ *
+ * @since 4.1.0
+ * @see ModuleReport
+ * @see FailureReport
+ */
+@Experimental
+@Immutable
+public interface BuildReport {
+
+    /**
+     * The invocation context of this build: flags, properties, and platform 
settings
+     * that were active when the session started.
+     *
+     * @return the build environment, never {@code null}
+     * @since 4.1.0
+     */
+    @Nonnull
+    BuildEnvironment environment();
+
+    /**
+     * Schema version of the report format. Consumers should check this
+     * to handle forward compatibility.
+     *
+     * @return the format version, currently {@code 1}
+     */
+    int formatVersion();
+
+    /**
+     * The overall build status.
+     *
+     * @return the build outcome, never {@code null}
+     */
+    @Nonnull
+    BuildStatus status();
+
+    /**
+     * Wall-clock duration of the entire build.
+     *
+     * @return the total duration, never {@code null}
+     */
+    @Nonnull
+    Duration duration();
+
+    /**
+     * When the build started (wall-clock time).
+     *
+     * @return the start instant, never {@code null}
+     */
+    @Nonnull
+    Instant startTime();
+
+    /**
+     * The Maven version that produced this report.
+     *
+     * @return the Maven version string, never {@code null}
+     */
+    @Nonnull
+    String mavenVersion();
+
+    /**
+     * The Java version used for the build.
+     *
+     * @return the Java version string, never {@code null}
+     */
+    @Nonnull
+    String javaVersion();
+
+    /**
+     * The goals or phases that were requested.
+     *
+     * @return the list of goals, never {@code null}
+     */
+    @Nonnull
+    List<String> goals();
+
+    /**
+     * The GAV of the top-level project ({@code groupId:artifactId:version}).
+     *
+     * @return the project identifier, never {@code null}
+     */
+    @Nonnull
+    String project();
+
+    /**
+     * Whether this was a multi-module (reactor) build.
+     *
+     * @return {@code true} for multi-module builds
+     */
+    boolean multiModule();
+
+    /**
+     * The degree of concurrency ({@code -T} flag), or 1 for sequential builds.
+     *
+     * @return the thread count
+     */
+    int threads();

Review Comment:
   💡 **`goals()` and `threads()` overlap with `environment().goals()` and 
`environment().threads()`**
   
   `BuildReport` already exposes `environment()` which returns a 
`BuildEnvironment` — and `BuildEnvironment` has both `goals()` and `threads()`. 
Surfacing them again directly on `BuildReport` creates two sources of truth for 
the same data. If they are convenience aliases (delegates to `environment()`), 
the Javadoc should say so explicitly. If they can diverge from the environment 
values, that distinction also needs to be documented.



##########
api/maven-api-core/src/main/java/org/apache/maven/api/BuildEnvironment.java:
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api;
+
+import java.util.List;
+import java.util.Map;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Describes the invocation context of a Maven build: the flags, properties, 
and
+ * environment settings that were active when the build started.
+ *
+ * <p>An instance is available via {@link Session#buildEnvironment()} during 
the build,
+ * and is also recorded in the structured build report for post-mortem 
analysis and
+ * reproducibility.
+ *
+ * <h2>What is captured</h2>
+ * <ul>
+ *   <li>Goals and lifecycle phases requested ({@link #goals()})</li>
+ *   <li>User properties passed via {@code -Dkey=value} ({@link 
#userProperties()}),
+ *       with sensitive keys redacted — see {@link #userProperties()} for the 
denylist</li>
+ *   <li>A curated subset of system properties relevant to reproducibility
+ *       ({@link #systemInfo()}): OS name/arch/version, Java vendor and VM 
name/version,
+ *       Maven home, and available processors</li>
+ *   <li>Local repository path ({@link #localRepository()})</li>
+ *   <li>Explicitly activated or deactivated profiles ({@link 
#activeProfiles()})</li>
+ *   <li>Selected projects ({@link #selectedProjects()}) and resume-from
+ *       ({@link #resumeFrom()})</li>
+ *   <li>Reactor failure behavior ({@link #reactorFailureBehavior()})</li>
+ *   <li>Offline mode ({@link #offline()}) and snapshot update policy
+ *       ({@link #updateSnapshots()})</li>
+ *   <li>Degree of concurrency ({@link #threads()})</li>
+ *   <li>Transfer-progress suppression ({@link #noTransferProgress()}) and 
batch mode ({@link #batchMode()})</li>
+ * </ul>
+ *
+ * <h2>What is not yet captured</h2>
+ * <p>The following information is not currently available through the Maven 4 
API
+ * and therefore cannot be recorded here. It may be added in future versions as
+ * the API evolves:
+ * <ul>
+ *   <li><b>Raw command-line arguments</b> ({@code args[]}): the CLI bootstrap 
layer
+ *       does not surface these through {@code MavenExecutionRequest} or the 
session</li>
+ *   <li><b>Implicitly activated profiles</b>: profiles activated by OS, JDK, 
or
+ *       property conditions rather than by explicit {@code -P} flag; computing
+ *       these requires per-project activation and is not available at session 
level</li>
+ * </ul>
+ *
+ * @since 4.1.0
+ * @see Session#buildEnvironment()
+ */
+@Experimental
+@Immutable
+public interface BuildEnvironment {
+
+    /**
+     * The goals or lifecycle phases that were requested on the command line.
+     *
+     * @return the requested goals/phases, never {@code null}
+     */
+    @Nonnull
+    List<String> goals();
+
+    /**
+     * User properties passed via {@code -Dkey=value} on the command line or 
via
+     * {@code --define}.
+     *
+     * <p>Sensitive keys are redacted and replaced with {@code "***"}. A key is
+     * considered sensitive if its lower-case form contains any of:
+     * {@code password}, {@code passwd}, {@code secret}, {@code token},
+     * {@code apikey}, {@code api_key}, {@code credential}, {@code passphrase}.
+     *
+     * @return the user properties, never {@code null}; values of sensitive 
keys
+     *         are replaced with {@code "***"}
+     */
+    @Nonnull
+    Map<String, String> userProperties();
+
+    /**
+     * A curated subset of system properties capturing platform and JVM 
identity,
+     * relevant for build reproducibility analysis.
+     *
+     * <p>The following keys are included when present:
+     * {@code os.name}, {@code os.arch}, {@code os.version},
+     * {@code java.vendor}, {@code java.vm.name}, {@code java.vm.version},
+     * {@code maven.home}, {@code user.home}, {@code user.name},
+     * and {@code available.processors} (from {@code Runtime.getRuntime()}).
+     *
+     * <p>The full {@code System.getProperties()} map is intentionally not 
captured:
+     * it is large, mostly irrelevant, and may contain sensitive values.
+     *
+     * @return the curated system info map, never {@code null}
+     */
+    @Nonnull
+    Map<String, String> systemInfo();
+
+    /**
+     * The path to the local repository used for this build.
+     *
+     * @return the local repository path string, never {@code null}
+     */
+    @Nonnull
+    String localRepository();
+
+    /**
+     * Profiles explicitly activated ({@code -P profileId}) or deactivated
+     * ({@code -P !profileId}) on the command line.
+     *
+     * <p>This list reflects only explicit {@code -P} selections. Profiles 
activated
+     * implicitly by OS, JDK, or property conditions are not included here — 
see
+     * the class-level javadoc for details.
+     *
+     * @return the explicitly selected profiles, never {@code null}; may be 
empty
+     */
+    @Nonnull
+    List<String> activeProfiles();
+
+    /**
+     * Projects explicitly selected via {@code -pl} / {@code --projects}.
+     *
+     * @return the selected project selectors (e.g. {@code ":my-module"}),
+     *         never {@code null}; empty for a full reactor build
+     */
+    @Nonnull
+    List<String> selectedProjects();
+
+    /**
+     * The project to resume from, as specified via {@code -rf} / {@code 
--resume-from}.
+     *
+     * @return the resume-from selector, or {@code null} if not specified
+     */
+    String resumeFrom();

Review Comment:
   ⚠️ **Missing `@Nullable` annotation on `resumeFrom()`**
   
   The Javadoc says _"or `null` if not specified"_, but the method has no 
`@Nullable` annotation. Every other nullable-returning method in the Maven API 
carries this annotation to make the null contract explicit for callers and 
static analysis tools. The file imports `@Nonnull` but not `@Nullable`.
   
   ```suggestion
       @Nullable
       String resumeFrom();
   ```
   
   And add the import alongside `@Nonnull`:
   ```
   import org.apache.maven.api.annotations.Nullable;
   ```



##########
its/core-it-support/maven-it-helper/src/main/java/org/apache/maven/it/Verifier.java:
##########
@@ -451,17 +451,24 @@ private String doGetLocalRepositoryWithSettings(String 
settingsXml) {
             if (!Files.isRegularFile(settingsFile)) {
                 throw new IllegalArgumentException("settings xml does not 
exist: " + settingsXml);
             }
-            return executorTool.localRepository(executorRequest()
-                    .cwd(tempBasedir)
+            ExecutorRequest.Builder builder = executorRequest()
+                    .cwd(basedir)

Review Comment:
   ⚠️ **Stale comment contradicts the new code**
   
   The comment at lines 447–448 says:
   > `// but we should not use basedir, as it may contain extensions.xml or a 
project, that Maven will eagerly`
   
   But this PR changes `.cwd(tempBasedir)` to `.cwd(basedir)` on the very next 
line. Either the comment is stale and should be removed/updated, or the 
`.cwd(basedir)` change reintroduces the problem the comment was guarding 
against. Please clarify the intent and update the comment accordingly.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to