modify-composite-repository: NPE in createP2Index for in-place edits (all 5.0.x)

[john-aa]

modify-composite-repository: NPE in createP2Index for in-place edits (all 5.0.x)

Summary

tycho-p2-repository-plugin:modify-composite-repository always fails with a
NullPointerException when modifying a composite repository in place, which is the
documented default behaviour of the goal.

The composite metadata is written correctly first, and the goal then throws while writing
p2.index. So the repository is mutated and the build fails.

Affected versions

5.0.0, 5.0.1, 5.0.2, 5.0.3 — every published version containing the goal. The identical call
site is present in all four. The goal does not exist in 4.0.13, so this is not a regression:
the default mode has never worked.

Reproducer

pom.xml:

<project xmlns="http://maven.apache.org/POM/4.0.0">
  <modelVersion>4.0.0</modelVersion>
  <groupId>test</groupId>
  <artifactId>p2test</artifactId>
  <version>1.0.0</version>
  <packaging>pom</packaging>

  <build>
    <plugins>
      <plugin>
        <groupId>org.eclipse.tycho</groupId>
        <artifactId>tycho-p2-repository-plugin</artifactId>
        <version>5.0.3</version>
        <executions>
          <execution>
            <id>t</id>
            <phase>initialize</phase>
            <goals><goal>modify-composite-repository</goal></goals>
            <configuration>
              <repository>
                <id>t</id>
                <url>${project.baseUri}repo/</url>
              </repository>
              <repositoryKind>metadata</repositoryKind>
              <childrenToAdd>
                <child>../child</child>
              </childrenToAdd>
              <validateChildren>false</validateChildren>
            </configuration>
          </execution>
        </executions>
      </plugin>
    </plugins>
  </build>
</project>

mkdir repo
mvn initialize

No child repository is required (validateChildren=false), and the target may be an empty
directory or an existing composite — both fail identically.

Actual result

[ERROR] Failed to execute goal org.eclipse.tycho:tycho-p2-repository-plugin:5.0.3:modify-composite-repository (t)
        on project p2test: Execution t of goal ... failed:
        Cannot invoke "java.nio.file.Path.resolve(String)" because "directory" is null

Caused by: java.lang.NullPointerException: Cannot invoke "java.nio.file.Path.resolve(String)" because "directory" is null
    at org.eclipse.tycho.p2maven.tools.P2RepositoryDataManipulator.createP2Index (P2RepositoryDataManipulator.java:282)
    at org.eclipse.tycho.p2maven.tools.P2RepositoryDataManipulator.modifyCompositeRepository (P2RepositoryDataManipulator.java:213)
    at org.eclipse.tycho.plugins.p2.repository.ModifyCompositeRepositoryMojo.execute (ModifyCompositeRepositoryMojo.java:146)

compositeContent.xml is written to the repository directory before the throw; only
p2.index is missing.

Expected result

The composite is modified in place and p2.index is written alongside it, as when an explicit
outputLocation is used.

Root cause

In org.eclipse.tycho.p2maven.tools.P2RepositoryDataManipulator (artifact p2-maven-plugin),
in-place editing is represented by a null outputLocation:

createDescriptor, lines 72-74:

if (outputLocation != null) {
    if (location.getScheme().equals("file") && Files.isSameFile(outputLocation, Path.of(location))) {
        outputLocation = null; // Signal in-place edit
    }
    ...

modifyOutputCompositeRepository resolves that null into a real path, lines 237-245:

Path outputLocation = descriptor.outputLocation();
if (outputLocation == null) {
    try {
        outputLocation = Path.of(repository.getURL());
    } catch (Exception e) {
        throw new IllegalStateException(...);
    }
}

but that resolution is a local variable, and is never written back to the descriptor.
modifyCompositeRepository then passes the raw, still-null field, line 213:

createP2Index(repository.outputLocation(), repository.isMetadata(), repository.isArtifact());

and createP2Index dereferences it, line 282:

try (OutputStream output = Files.newOutputStream(directory.resolve("p2.index"))) {

So the fallback exists at one call site and is missing at the other.

Why every configuration fails

Because line 73 normalises "output location equals repository" to null, there is no
configuration that both edits in place and produces a p2.index:

Configuration	Result
outputLocation omitted	NPE
outputLocation = the repository directory	NPE (normalised to null at line 73)
repositoryKind = metadata / artifact / unset	NPE in all cases
target is a new directory / an existing composite	NPE in both
location given as absolute path, relative path, or file: URI	NPE in all forms
outputLocation = a different, empty directory	works

Workaround

Set outputLocation to a separate empty directory and copy the generated files over the
repository afterwards. The directory must be empty, or createDescriptor lines 78-82 throw
IllegalStateException: Output location is not empty.

Suggested fix

Resolve the effective output location once in modifyCompositeRepository and pass it to
createP2Index — mirroring xzCompress(ModifiedRepositoryDescriptor, Path) at line 158,
which already takes the resolved path as a parameter:

Path effectiveOutput = repository.outputLocation();
if (effectiveOutput == null) {
    effectiveOutput = Path.of(repository.repository().getURL());
}
createP2Index(effectiveOutput, repository.isMetadata(), repository.isArtifact());

Hoisting the existing fallback out of modifyOutputCompositeRepository so both call sites
share it would avoid the duplication.

Given the goal currently mutates the repository before failing, making the p2.index write
part of the same success path would also make the operation atomic from the caller's point of
view.

Environment

• Tycho 5.0.3 (also verified in the sources of 5.0.0, 5.0.1, 5.0.2)
• Apache Maven 3.9.9
• Java 21.0.11 (Eclipse Adoptium)
• Windows 11 x86_64