Only use ci-friendly version if there is such property defined

Currently it can happen that one uses one of the "trigger properties" that enable Tycho pomless feature to assume that a cifriendly version is to be used even though ci-friendly versions are actually not used at all. This tries to differentiate the two cases by checking if there is actually a property defined that could be used in a ci-friendly way.
eclipse-tycho · Jan 5, 2024 · 9352449 · 9352449
1 parent 64b3ccf
commit 9352449
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 24 deletions.
diff --git a/tycho-build/src/main/java/org/eclipse/tycho/build/TychoGraphBuilder.java b/tycho-build/src/main/java/org/eclipse/tycho/build/TychoGraphBuilder.java
@@ -84,7 +84,7 @@ public Result<ProjectDependencyGraph> build(MavenSession session) {
 				if (properties.getProperty(TychoCiFriendlyVersions.PROPERTY_BUILDQUALIFIER_FORMAT) != null
 						|| properties.getProperty(TychoCiFriendlyVersions.PROPERTY_FORCE_QUALIFIER) != null
 						|| properties.getProperty(TychoCiFriendlyVersions.BUILD_QUALIFIER) != null) {
-					tychoMapping.setSnapshotFormat("${" + TychoCiFriendlyVersions.BUILD_QUALIFIER + "}");
+					tychoMapping.setSnapshotProperty(TychoCiFriendlyVersions.BUILD_QUALIFIER);
 				}
 			}
 		}

diff --git a/tycho-build/src/main/java/org/eclipse/tycho/build/bnd/BndWorkspaceMapping.java b/tycho-build/src/main/java/org/eclipse/tycho/build/bnd/BndWorkspaceMapping.java
@@ -23,11 +23,11 @@
 import java.util.stream.Stream;
 
 import org.apache.maven.model.Model;
-import org.apache.maven.model.Parent;
 import org.codehaus.plexus.component.annotations.Component;
 import org.eclipse.tycho.TychoConstants;
 import org.eclipse.tycho.pomless.AbstractTychoMapping;
 import org.eclipse.tycho.pomless.NoParentPomFound;
+import org.eclipse.tycho.pomless.ParentModel;
 import org.sonatype.maven.polyglot.mapping.Mapping;
 
 import aQute.bnd.build.Project;
@@ -123,7 +123,7 @@ protected void initModel(Model model, Reader artifactReader, Path cnfFolder) thr
 	}
 
 	@Override
-	protected Parent findParent(Path projectRoot, Map<String, ?> projectOptions) throws IOException {
+	protected ParentModel findParent(Path projectRoot, Map<String, ?> projectOptions) throws IOException {
 		try {
 			return super.findParent(projectRoot, projectOptions);
 		} catch (NoParentPomFound e) {

diff --git a/tycho-extras/tycho-pomless/src/main/java/org/eclipse/tycho/pomless/AbstractTychoMapping.java b/tycho-extras/tycho-pomless/src/main/java/org/eclipse/tycho/pomless/AbstractTychoMapping.java
@@ -67,7 +67,8 @@ public abstract class AbstractTychoMapping implements Mapping, ModelReader {
 
     private static final String PARENT_POM_DEFAULT_VALUE = System.getProperty(TYCHO_POMLESS_PARENT_PROPERTY, "..");
     private static final String QUALIFIER_SUFFIX = ".qualifier";
-    private static final String MODEL_PARENT = "TychoMapping.model.parent";
+
+    private Map<Path, ParentModel> parentModelCache = new HashMap<Path, ParentModel>();
 
     @Requirement
     protected PlexusContainer container;
@@ -79,7 +80,7 @@ public abstract class AbstractTychoMapping implements Mapping, ModelReader {
     private boolean extensionMode;
     @SuppressWarnings("unused")
     private File multiModuleProjectDirectory;
-    private String snapshotFormat;
+    private String snapshotProperty;
 
     @Override
     public File locatePom(File dir) {
@@ -155,7 +156,7 @@ private Model read(Reader artifactReader, Path artifactFile, Map<String, ?> opti
         model.setPackaging(getPackaging());
         initModel(model, artifactReader, artifactFile);
         if (model.getParent() == null) {
-            model.setParent(findParent(artifactFile.getParent(), options));
+            model.setParent(findParent(artifactFile.getParent(), options).parentReference());
         }
         if (model.getVersion() == null && model.getParent() != null) {
             //inherit version from parent if not given
@@ -172,11 +173,10 @@ protected Path getRealArtifactFile(Path polyglotArtifactFile) {
         return polyglotArtifactFile;
     }
 
-    protected Parent findParent(Path projectRoot, Map<String, ?> projectOptions) throws IOException {
-        Parent parent = (Parent) projectOptions.get(MODEL_PARENT);
-        if (parent != null) {
-            //if the parent is given by the options we don't need to search it!
-            return parent;
+    protected synchronized ParentModel findParent(Path projectRoot, Map<String, ?> projectOptions) throws IOException {
+        ParentModel cached = parentModelCache.get(projectRoot);
+        if (cached != null) {
+            return cached;
         }
         Properties buildProperties = getBuildProperties(projectRoot);
         // assumption parent pom must be physically located in parent directory if not given by build.properties
@@ -198,24 +198,27 @@ protected Parent findParent(Path projectRoot, Map<String, ?> projectOptions) thr
         Model parentModel = parentPom.getReader().read(parentPom.getPomFile(), options);
         Parent parentReference = new Parent();
         String groupId = parentModel.getGroupId();
-        if (groupId == null) {
+        Parent grandParent = parentModel.getParent();
+        if (groupId == null && grandParent != null) {
             // must be inherited from grandparent
-            groupId = parentModel.getParent().getGroupId();
+            groupId = grandParent.getGroupId();
         }
         parentReference.setGroupId(groupId);
         parentReference.setArtifactId(parentModel.getArtifactId());
         String version = parentModel.getVersion();
-        if (version == null) {
+        if (version == null && grandParent != null) {
             // must be inherited from grandparent
-            version = parentModel.getParent().getVersion();
+            version = grandParent.getVersion();
         }
         parentReference.setVersion(version);
         parentReference
                 .setRelativePath(projectRoot.toRealPath().relativize(parentPom.getPomFile().toPath()).toString());
         logger.debug("Derived parent for path " + projectRoot + " is groupId: " + parentReference.getGroupId()
                 + ", artifactId: " + parentReference.getArtifactId() + ", relativePath: "
                 + parentReference.getRelativePath());
-        return parentReference;
+        ParentModel model = new ParentModel(parentReference, parentModel);
+        parentModelCache.put(projectRoot, model);
+        return model;
     }
 
     /**
@@ -342,18 +345,38 @@ private static void setLocation(Model model, Path modelSource) {
         model.setLocation("", new InputLocation(0, 0, inputSource));
     }
 
-    protected String getPomVersion(String pdeVersion) {
+    protected String getPomVersion(String pdeVersion, Model model, Path projectRoot) {
         String pomVersion = pdeVersion;
         if (pdeVersion.endsWith(QUALIFIER_SUFFIX)) {
             String unqualifiedVersion = pdeVersion.substring(0, pdeVersion.length() - QUALIFIER_SUFFIX.length());
-            if (isExtensionMode() && snapshotFormat != null) {
-                return unqualifiedVersion + snapshotFormat;
+            //we need to check that this property is actually defined!
+            if (isExtensionMode() && modelHasProperty(snapshotProperty, model, projectRoot)) {
+                return unqualifiedVersion + "${" + snapshotProperty + "}";
             }
             return unqualifiedVersion + "-SNAPSHOT";
         }
         return pomVersion;
     }
 
+    private boolean modelHasProperty(String property, Model model, Path projectRoot) {
+        if (property == null) {
+            //nothing we can check assume it is NOT present...
+            return false;
+        }
+        Properties properties = model.getProperties();
+        String string = properties.getProperty(property);
+        if (string != null) {
+            return true;
+        }
+        try {
+            ParentModel parent = findParent(projectRoot.getParent(), Map.of());
+            return modelHasProperty(property, parent.parentModel(),
+                    projectRoot.resolve(parent.parentReference().getRelativePath()));
+        } catch (IOException e) {
+            return false;
+        }
+    }
+
     public boolean isExtensionMode() {
         return extensionMode;
     }
@@ -367,8 +390,8 @@ public void setMultiModuleProjectDirectory(File multiModuleProjectDirectory) {
         this.multiModuleProjectDirectory = multiModuleProjectDirectory;
     }
 
-    public void setSnapshotFormat(String snapshotFormat) {
-        this.snapshotFormat = snapshotFormat;
+    public void setSnapshotProperty(String snapshotFormat) {
+        this.snapshotProperty = snapshotFormat;
     }
 
     static Optional<Path> getLocation(Map<String, ?> options) {

diff --git a/tycho-extras/tycho-pomless/src/main/java/org/eclipse/tycho/pomless/ParentModel.java b/tycho-extras/tycho-pomless/src/main/java/org/eclipse/tycho/pomless/ParentModel.java
@@ -0,0 +1,20 @@
+/*******************************************************************************
+ * Copyright (c) 2024 Christoph Läubrich and others.
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     Christoph Läubrich - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.tycho.pomless;
+
+import org.apache.maven.model.Model;
+import org.apache.maven.model.Parent;
+
+public record ParentModel(Parent parentReference, Model parentModel) {
+
+}
diff --git a/tycho-extras/tycho-pomless/src/main/java/org/eclipse/tycho/pomless/TychoBundleMapping.java b/tycho-extras/tycho-pomless/src/main/java/org/eclipse/tycho/pomless/TychoBundleMapping.java
@@ -95,7 +95,7 @@ protected void initModel(Model model, Reader artifactReader, Path artifactFile)
         // groupId is inherited from parent pom
         model.setArtifactId(bundleSymbolicName);
         String bundleVersion = getRequiredHeaderValue("Bundle-Version", manifestHeaders, manifestFile);
-        model.setVersion(getPomVersion(bundleVersion));
+        model.setVersion(getPomVersion(bundleVersion, model, artifactFile));
         String prefix;
         if (isTestBundle(bundleSymbolicName, bundleRoot)) {
             model.setPackaging(PACKAGING_TEST);

diff --git a/tycho-extras/tycho-pomless/src/main/java/org/eclipse/tycho/pomless/TychoFeatureMapping.java b/tycho-extras/tycho-pomless/src/main/java/org/eclipse/tycho/pomless/TychoFeatureMapping.java
@@ -47,7 +47,7 @@ public float getPriority() {
     @Override
     protected void initModelFromXML(Model model, Element xml, Path artifactFile) throws IOException {
         model.setArtifactId(getRequiredXMLAttributeValue(xml, "id"));
-        model.setVersion(getPomVersion(getRequiredXMLAttributeValue(xml, "version")));
+        model.setVersion(getPomVersion(getRequiredXMLAttributeValue(xml, "version"), model, artifactFile));
 
         Path featureProperties = artifactFile.getParent().resolve("feature.properties");
         Supplier<Properties> properties = getPropertiesSupplier(featureProperties);

diff --git a/...-extras/tycho-pomless/src/main/java/org/eclipse/tycho/pomless/TychoRepositoryMapping.java b/...-extras/tycho-pomless/src/main/java/org/eclipse/tycho/pomless/TychoRepositoryMapping.java
@@ -99,7 +99,7 @@ protected void initModelFromXML(Model model, Element xml, Path artifactFile) thr
             model.setArtifactId(getRequiredXMLAttributeValue(xml, PRODUCT_UID_ATTRIBUTE));
             String version = getXMLAttributeValue(xml, PRODUCT_VERSION_ATTRIBUTE);
             if (version != null) {
-                model.setVersion(getPomVersion(version));
+                model.setVersion(getPomVersion(version, model, artifactFile));
             }
             String name = getXMLAttributeValue(xml, PRODUCT_NAME_ATTRIBUTE);
             model.setName(PRODUCT_NAME_PREFIX + (name != null ? name : model.getArtifactId()));