gnodet-bot commented on code in PR #12695: URL: https://github.com/apache/maven/pull/12695#discussion_r4059835799
########## api/maven-api-core/src/main/java/org/apache/maven/api/BuildEnvironment.java: ########## @@ -0,0 +1,202 @@ +/* + * 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> + * </ul> Review Comment: **[Medium] "What is captured" list is missing `batchMode()` and `noTransferProgress()`.** The "What is not yet captured" section was updated to remove these two fields, but the "What is captured" `<ul>` (above) was not updated to add them. A reader of the class Javadoc sees the summary list and will not know these fields exist. ```suggestion * <li>Degree of concurrency ({@link #threads()})</li> * <li>Transfer-progress suppression ({@link #noTransferProgress()}) and batch mode ({@link #batchMode()})</li> * </ul> ``` ########## impl/maven-impl/src/main/java/org/apache/maven/impl/standalone/ApiRunner.java: ########## @@ -284,6 +285,75 @@ interface LocalRepoProvider { */ static class DefaultSession extends AbstractSession { + // NOTE: If BuildEnvironment gains new abstract methods, update this constant. + // DefaultBuildEnvironment (maven-core) cannot be used here — module boundary. + private static final BuildEnvironment EMPTY_BUILD_ENVIRONMENT = new BuildEnvironment() { + @Override + public List<String> goals() { + return List.of(); + } + + @Override + public Map<String, String> userProperties() { + return Map.of(); + } + + @Override + public Map<String, String> systemInfo() { + return Map.of(); + } + + @Override + public String localRepository() { + return ""; + } + + @Override + public List<String> activeProfiles() { + return List.of(); + } + + @Override + public List<String> selectedProjects() { + return List.of(); + } + + @Override + public String resumeFrom() { + return null; + } + + @Override + public String reactorFailureBehavior() { + return "FAIL_FAST"; + } + + @Override + public boolean offline() { + return false; + } + + @Override + public boolean updateSnapshots() { + return false; + } + + @Override + public boolean noTransferProgress() { + return false; + } + + @Override + public boolean batchMode() { + return false; + } + + @Override + public int threads() { + return 1; + } + }; + private final Map<String, String> systemProperties; private final Instant startTime = MonotonicClock.now(); private Settings settings; Review Comment: **[Low] Identical 70-line `EMPTY_BUILD_ENVIRONMENT` anonymous class duplicated in `SessionStub`.** The same anonymous class implementing all `BuildEnvironment` methods with no-op/empty defaults exists verbatim in both `ApiRunner.DefaultSession` (this file) and `SessionStub`. The `ApiRunner` copy carries the comment `// NOTE: If BuildEnvironment gains new abstract methods, update this constant.` — but `SessionStub` has no such reminder. This has already caused a silent miss once: when `batchMode()` and `noTransferProgress()` were added, `SessionStub` required a separate fix commit (`5a244435c1`). With two independent copies, the next new method will require the same manual double-update. Consider extracting to a shared `EmptyBuildEnvironment` package-private class in `maven-api-core` or a test-helper module, so there is exactly one place to update. -- 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]
