From b2248faa2b8358bfef222f2fd7cb3b2dc50692eb Mon Sep 17 00:00:00 2001 From: github-actions Date: Wed, 11 Dec 2024 17:57:43 +0100 Subject: [PATCH 1/3] Fixes a number of errors in the bndrun handling --- Signed-off-by: github-actions Signed-off-by: github-actions --- .../maven/lib/resolve/BndrunContainer.java | 4 +- .../aQute/bnd/build/model/BndEditModel.java | 48 +++++++++++-------- .../src/bndtools/editor/BndEditor.java | 9 +++- .../editor/project/RunRequirementsPart.java | 4 +- 4 files changed, 40 insertions(+), 25 deletions(-) diff --git a/biz.aQute.bnd.maven/src/aQute/bnd/maven/lib/resolve/BndrunContainer.java b/biz.aQute.bnd.maven/src/aQute/bnd/maven/lib/resolve/BndrunContainer.java index 3b6a60a44f..8d2a865d8f 100644 --- a/biz.aQute.bnd.maven/src/aQute/bnd/maven/lib/resolve/BndrunContainer.java +++ b/biz.aQute.bnd.maven/src/aQute/bnd/maven/lib/resolve/BndrunContainer.java @@ -203,7 +203,9 @@ public Bndrun init(File runFile, String task, File workingDir) throws Exception run.setParent(getProcessor(workspace)); run.clear(); run.forceRefresh(); // setBase must be called after forceRefresh - run.setBase(temporaryDir); + // this should imho not be there, it should + // be the parent directory of the properties file, as it was. + // run.setBase(temporaryDir); run.getInfo(workspace); setRunrequiresFromProjectArtifact(run); setEEfromBuild(run); diff --git a/biz.aQute.bndlib/src/aQute/bnd/build/model/BndEditModel.java b/biz.aQute.bndlib/src/aQute/bnd/build/model/BndEditModel.java index 823c4fca32..e9b6e38238 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/build/model/BndEditModel.java +++ b/biz.aQute.bndlib/src/aQute/bnd/build/model/BndEditModel.java @@ -16,7 +16,6 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; @@ -263,6 +262,7 @@ public ImportPattern error( private final PropertyChangeSupport propChangeSupport = new PropertyChangeSupport( this); private Properties properties = new UTF8Properties(); + private UTF8Properties reference = new UTF8Properties(); private final Map objectProperties = new HashMap<>(); private final Map changesToSave = new TreeMap<>(); private Processor owner; @@ -379,6 +379,7 @@ public BndEditModel(BndEditModel model) { this.inputFile = model.inputFile; this.workspace = model.workspace; this.properties.putAll(model.properties); + this.reference.putAll(model.properties); this.changesToSave.putAll(model.changesToSave); setOwner(model.getOwner()); } @@ -439,7 +440,9 @@ public void loadFrom(File file) throws IOException { public void loadFrom(InputStream inputStream) throws IOException { try { properties.clear(); + reference.clear(); properties.load(inputStream); + reference.putAll(properties); objectProperties.clear(); changesToSave.clear(); @@ -1248,6 +1251,13 @@ private void doRemoveObject(String name, T oldValue, T newValue, Converter void doSetObject(String name, T oldValue, T newValue, Converter formatter) { objectProperties.put(name, newValue); String v = formatter.convert(newValue); + if (v == null) { + if (oldValue == null) { + return; + } + doRemoveObject(name, oldValue, newValue, formatter); + return; + } changesToSave.put(name, v); dirty = true; propChangeSupport.firePropertyChange(name, oldValue, newValue); @@ -1416,7 +1426,6 @@ public Processor getProperties() throws Exception { return getPropertiesInternal(true); } - /** * @param expandMacros set to true if macros e.g. '${.}' should * be expanded. In the UI when editing the model, macros should @@ -1432,7 +1441,10 @@ private Processor getPropertiesInternal(boolean expandMacros) throws IOException currentProperties.load(changes, null, null); } - Set ownerLocalKeys = owner.getPropertyKeys(false); + Set ownerLocalKeys = reference.keySet() + .stream() + .map(String.class::cast) + .collect(Collectors.toSet()); Set editedLocalKeys = currentProperties.keySet() .stream() .map(String.class::cast) @@ -1440,35 +1452,28 @@ private Processor getPropertiesInternal(boolean expandMacros) throws IOException Collection deleted = Logic.remove(ownerLocalKeys, editedLocalKeys); - Set inheritedKeysWithoutOwner = new HashSet<>(editedLocalKeys); - - Processor parent = owner.getParent(); - if (parent != null) { - - // we leave out the keys of the owner - inheritedKeysWithoutOwner.addAll(parent.getPropertyKeys(true)); - } - File source = getBndResource(); Processor dummy = new Processor(owner) { @Override public Set getPropertyKeys(boolean inherit) { - if (inherit) { - return Collections.unmodifiableSet(inheritedKeysWithoutOwner); - } - return super.getPropertyKeys(false); + Set keys = super.getPropertyKeys(inherit); + keys.removeAll(deleted); + return keys; } @Override public String getProperty(String key, String deflt, String separator) { - if (!inheritedKeysWithoutOwner.contains(key)) - return deflt; + String value; + if (deleted.contains(key)) { + key = "IN_BNDEDIT_MODEL_DELETED_KEY_" + key; + } if (expandMacros) { - return super.getProperty(key, deflt, separator); + value = super.getProperty(key, deflt, separator); } else { - return super.getUnprocessedProperty(key, deflt); + value = super.getUnprocessedProperty(key, deflt); } + return value; } }; dummy.setBase(owner.getBase()); @@ -1477,7 +1482,8 @@ public String getProperty(String key, String deflt, String separator) { if (expandMacros) { currentProperties = currentProperties.replaceHere(dummy.getBase()); } - dummy.setProperties(currentProperties); + dummy.getProperties() + .putAll(currentProperties); return dummy; } diff --git a/bndtools.core/src/bndtools/editor/BndEditor.java b/bndtools.core/src/bndtools/editor/BndEditor.java index 65cd78a9b9..2f20a08547 100644 --- a/bndtools.core/src/bndtools/editor/BndEditor.java +++ b/bndtools.core/src/bndtools/editor/BndEditor.java @@ -14,6 +14,7 @@ import org.bndtools.api.ILogger; import org.bndtools.api.Logger; +import org.bndtools.api.RunMode; import org.bndtools.api.editor.IBndEditor; import org.bndtools.api.launch.LaunchConstants; import org.bndtools.core.jobs.JobUtil; @@ -619,8 +620,12 @@ private Promise loadEditModel(File inputFile, BndEditModel model) thr Processor p = workspace.readLocked(() -> workspace.findProcessor(inputFile) .orElseGet(() -> { try { - Bndrun bndrun = Bndrun.createBndrun(workspace, inputFile); - return bndrun; + Run run; + if (inputResource == null) + run = Bndrun.createBndrun(workspace, inputFile); + else + run = LaunchUtils.createRun(inputResource, RunMode.EDIT); + return run; } catch (Exception e) { throw Exceptions.duck(e); } diff --git a/bndtools.core/src/bndtools/editor/project/RunRequirementsPart.java b/bndtools.core/src/bndtools/editor/project/RunRequirementsPart.java index dfd9e3f5e0..97cf78a95e 100644 --- a/bndtools.core/src/bndtools/editor/project/RunRequirementsPart.java +++ b/bndtools.core/src/bndtools/editor/project/RunRequirementsPart.java @@ -166,7 +166,9 @@ private void doResolve() { @Override protected void doCommitToModel(List requires) { - model.setRunRequires(requires); + if (isDirty()) { + model.setRunRequires(requires); + } } @Override From 6167461982951b2675b5afd361e33a8628bf2af7 Mon Sep 17 00:00:00 2001 From: github-actions Date: Thu, 12 Dec 2024 17:10:23 +0100 Subject: [PATCH 2/3] cleanup --- Signed-off-by: github-actions Signed-off-by: github-actions --- .../test/test/BndEditModelTest.java | 88 ++++++++ .../bndtools-resolve-reproducer/UNLICENSE | 24 +++ .../bndtools-resolve-reproducer/bnd.bnd | 1 + .../debug-standalone.bndrun | 17 ++ .../bndtools-resolve-reproducer/debug.bndrun | 11 + .../bndtools-resolve-reproducer/pom.xml | 120 +++++++++++ .../bndtools-resolve-reproducer/run.bndrun | 5 + .../bndtools/bndrun/reproducer/Activator.java | 7 + .../aQute/bnd/build/model/BndEditModel.java | 201 +++++++++--------- .../aQute/bnd/build/model/package-info.java | 2 +- .../src/aQute/bnd/osgi/Processor.java | 24 ++- .../src/aQute/bnd/osgi/Verifier.java | 2 +- .../baseline/BundleVersionErrorHandler.java | 2 +- .../src/bndtools/release/ReleaseHelper.java | 4 +- 14 files changed, 390 insertions(+), 118 deletions(-) create mode 100644 biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/UNLICENSE create mode 100644 biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/bnd.bnd create mode 100644 biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/debug-standalone.bndrun create mode 100644 biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/debug.bndrun create mode 100644 biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/pom.xml create mode 100644 biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/run.bndrun create mode 100644 biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/src/main/java/org/example/bndtools/bndrun/reproducer/Activator.java diff --git a/biz.aQute.bndlib.tests/test/test/BndEditModelTest.java b/biz.aQute.bndlib.tests/test/test/BndEditModelTest.java index 89ebecfc43..cada35aa31 100644 --- a/biz.aQute.bndlib.tests/test/test/BndEditModelTest.java +++ b/biz.aQute.bndlib.tests/test/test/BndEditModelTest.java @@ -6,8 +6,10 @@ import java.io.ByteArrayInputStream; import java.io.File; +import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.Test; import org.osgi.framework.namespace.IdentityNamespace; @@ -255,6 +257,92 @@ public void testGetPropertiesWithOnlyWorkspace() throws Exception { assertThat(p.getProperty("foo")).isEqualTo("FOO"); } + /** + * Test the testresources/bndtools-resolve-reproducer project (m2). There + * were issues with the inheritance and inclusion of properties. + */ + + static final File REPRODUCER = IO.getFile("testresources/bndtools-resolve-reproducer"); + static final File DEBUG_BNDRUN = IO.getFile(REPRODUCER, "debug.bndrun"); + static final Map PROPERTIES = Map.of("-runbundles", + "org.apache.felix.gogo.command;version='[1.1.2,1.1.3)',org.apache.felix.gogo.runtime;version='[1.1.6,1.1.7)',org.apache.felix.gogo.shell;version='[1.1.4,1.1.5)'", // + "-resolve.effective", "active", // + "-runfw", "org.eclipse.osgi;version='3.21.0'", // + "-runproperties.debug", "osgi.console=,osgi.console.enable.builtin=false", // + "-runrequires", "bnd.identity;id='org.example.bndtools.bndrun.reproducer'", // + "-runee", "JavaSE-17", // + "-runrequires.debug", + "osgi.identity;filter:='(osgi.identity=org.apache.felix.gogo.runtime)',osgi.identity;filter:='(osgi.identity=org.apache.felix.gogo.command)'" // + + ); + + @Test + public void testBasicReproducer() throws Exception { + assertThat(DEBUG_BNDRUN).isFile(); + Run run = Run.createRun(null, DEBUG_BNDRUN); + assertThat(run.getProperties()).containsExactlyInAnyOrderEntriesOf(PROPERTIES); + BndEditModel model = new BndEditModel(run); + + assertThat(model.getAllPropertyNames()).containsExactlyInAnyOrder("-runbundles", "-runrequires.debug", + "-runproperties.debug", "-include"); + + } + + @Test + public void testBasicReproducerRemove() throws Exception { + assertThat(DEBUG_BNDRUN).isFile(); + Run run = Run.createRun(null, DEBUG_BNDRUN); + BndEditModel model = new BndEditModel(run); + + assertThat(model.getProperties() + .getProperties()).containsKey("-runbundles"); + assertThat(model.getDocumentChanges()).isEmpty(); + assertThat(model.getDocumentProperties() + .keySet()).containsExactlyInAnyOrder("-runbundles", "-runrequires.debug", "-runproperties.debug", + "-include"); + + model.setRunBundles(Collections.emptyList()); + + assertThat(model.getRunBundles()).isEmpty(); + assertThat(model.getDocumentChanges()).containsKey("-runbundles"); + assertThat(model.getProperties() + .getProperties()).doesNotContainKey("-runbundles"); + + Document d = new Document(IO.collect(DEBUG_BNDRUN)); + assertThat(d.get()).contains("-runbundles"); + + model.saveChangesTo(d); + + assertThat(model.getDocumentChanges()).isEmpty(); + assertThat(d.get()).doesNotContain("-runbundles"); + assertThat(model.getDocumentProperties() + .keySet()).containsExactlyInAnyOrder("-runrequires.debug", "-runproperties.debug", "-include"); + } + + @Test + public void testBasicReproducerAdd() throws Exception { + assertThat(DEBUG_BNDRUN).isFile(); + Run run = Run.createRun(null, DEBUG_BNDRUN); + BndEditModel model = new BndEditModel(run); + + assertThat(model.getProperties() + .getProperties()).doesNotContainKey("-runframework"); + + model.setRunFramework("none"); + + assertThat(model.getDocumentProperties()).doesNotContainKey("-runframework"); + + assertThat(model.getProperties() + .getProperties()).containsKey("-runframework"); + + assertThat(model.getRunFramework()).isEqualTo("none"); + Document d = new Document(IO.collect(DEBUG_BNDRUN)); + assertThat(d.get()).doesNotContain("-runframework"); + model.saveChangesTo(d); + assertThat(model.getDocumentChanges()).isEmpty(); + assertThat(d.get()).contains("-runframework"); + } + private String getPortablePath(File base) { String path = base.getAbsolutePath(); if (File.separatorChar != '/') { diff --git a/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/UNLICENSE b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/UNLICENSE new file mode 100644 index 0000000000..8a31d238c6 --- /dev/null +++ b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/UNLICENSE @@ -0,0 +1,24 @@ +This is free and unencumbered software released into the public domain. + +Anyone is free to copy, modify, publish, use, compile, sell, or +distribute this software, either in source code form or as a compiled +binary, for any purpose, commercial or non-commercial, and by any +means. + +In jurisdictions that recognize copyright laws, the author or authors +of this software dedicate any and all copyright interest in the +software to the public domain. We make this dedication for the benefit +of the public at large and to the detriment of our heirs and +successors. We intend this dedication to be an overt act of +relinquishment in perpetuity of all present and future rights to this +software under copyright law. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR +OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, +ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +OTHER DEALINGS IN THE SOFTWARE. + +For more information, please refer to . diff --git a/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/bnd.bnd b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/bnd.bnd new file mode 100644 index 0000000000..416feff78b --- /dev/null +++ b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/bnd.bnd @@ -0,0 +1 @@ +#Bundle-Activator: org.example.bndtools.bndrun.reproducer.Activator \ No newline at end of file diff --git a/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/debug-standalone.bndrun b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/debug-standalone.bndrun new file mode 100644 index 0000000000..594e077037 --- /dev/null +++ b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/debug-standalone.bndrun @@ -0,0 +1,17 @@ +-include: ~run.bndrun +-standalone: target/index.xml +-resolve.effective: active +-runfw: org.eclipse.osgi;version='3.21.0' +-runee: JavaSE-17 +-runproperties.debug: \ + osgi.console=,\ + osgi.console.enable.builtin=false +-runrequires.debug: \ + osgi.identity;filter:='(osgi.identity=org.apache.felix.gogo.shell)',\ + osgi.identity;filter:='(osgi.identity=org.apache.felix.gogo.runtime)',\ + osgi.identity;filter:='(osgi.identity=org.apache.felix.gogo.command)' +-runbundles: \ + org.apache.felix.gogo.command;version='[1.1.2,1.1.3)',\ + org.apache.felix.gogo.runtime;version='[1.1.6,1.1.7)',\ + org.apache.felix.gogo.shell;version='[1.1.4,1.1.5)',\ + org.example.bndtools.bndrun.reproducer;version='[1.0.0,1.0.1)' \ No newline at end of file diff --git a/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/debug.bndrun b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/debug.bndrun new file mode 100644 index 0000000000..b1b7a2996c --- /dev/null +++ b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/debug.bndrun @@ -0,0 +1,11 @@ +-include: ~run.bndrun +-runproperties.debug: \ + osgi.console=,\ + osgi.console.enable.builtin=false +-runrequires.debug: \ + osgi.identity;filter:='(osgi.identity=org.apache.felix.gogo.runtime)',\ + osgi.identity;filter:='(osgi.identity=org.apache.felix.gogo.command)' +-runbundles: \ + org.apache.felix.gogo.command;version='[1.1.2,1.1.3)',\ + org.apache.felix.gogo.runtime;version='[1.1.6,1.1.7)',\ + org.apache.felix.gogo.shell;version='[1.1.4,1.1.5)' diff --git a/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/pom.xml b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/pom.xml new file mode 100644 index 0000000000..1ded91bf68 --- /dev/null +++ b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/pom.xml @@ -0,0 +1,120 @@ + + + 4.0.0 + org.example + org.example.bndtools.bndrun.reproducer + 1.0-SNAPSHOT + + 17 + UTF-8 + 7.1.0 + + + + org.eclipse.platform + org.eclipse.osgi + 3.21.0 + + + org.apache.felix + org.apache.felix.gogo.shell + 1.1.4 + + + org.apache.felix + org.apache.felix.gogo.runtime + 1.1.6 + + + org.apache.felix + org.apache.felix.gogo.command + 1.1.2 + + + + + + biz.aQute.bnd + bnd-maven-plugin + ${bnd.version} + + + + bnd-process + + + + + + org.apache.maven.plugins + maven-jar-plugin + 3.4.2 + + + + ${project.build.outputDirectory}/META-INF/MANIFEST.MF + + + + + + biz.aQute.bnd + bnd-indexer-maven-plugin + ${bnd.version} + + true + REQUIRED + false + + + + + index + + + + + + biz.aQute.bnd + bnd-resolver-maven-plugin + ${bnd.version} + + false + + run.bndrun + debug.bndrun + debug-standalone.bndrun + + + + + resolve + + resolve + + + + verify + + verify + + + + + + biz.aQute.bnd + bnd-export-maven-plugin + ${bnd.version} + + + + export + + + + + + + \ No newline at end of file diff --git a/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/run.bndrun b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/run.bndrun new file mode 100644 index 0000000000..91febbe97c --- /dev/null +++ b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/run.bndrun @@ -0,0 +1,5 @@ +-resolve.effective: active +-runfw: org.eclipse.osgi;version='3.21.0' +-runee: JavaSE-17 +-runrequires: bnd.identity;id='org.example.bndtools.bndrun.reproducer' +-runbundles: org.example.bndtools.bndrun.reproducer;version='[1.0.0,1.0.1)' \ No newline at end of file diff --git a/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/src/main/java/org/example/bndtools/bndrun/reproducer/Activator.java b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/src/main/java/org/example/bndtools/bndrun/reproducer/Activator.java new file mode 100644 index 0000000000..87c5f6f113 --- /dev/null +++ b/biz.aQute.bndlib.tests/testresources/bndtools-resolve-reproducer/src/main/java/org/example/bndtools/bndrun/reproducer/Activator.java @@ -0,0 +1,7 @@ +package org.example.bndtools.bndrun.reproducer; + +import org.osgi.framework.BundleActivator; +import org.osgi.framework.BundleContext; + +public class Activator{ +} diff --git a/biz.aQute.bndlib/src/aQute/bnd/build/model/BndEditModel.java b/biz.aQute.bndlib/src/aQute/bnd/build/model/BndEditModel.java index e9b6e38238..26459b3d0c 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/build/model/BndEditModel.java +++ b/biz.aQute.bndlib/src/aQute/bnd/build/model/BndEditModel.java @@ -68,7 +68,6 @@ import aQute.bnd.properties.PropertiesLineReader; import aQute.bnd.version.Version; import aQute.lib.collections.Iterables; -import aQute.lib.collections.Logic; import aQute.lib.io.IO; import aQute.lib.utf8properties.UTF8Properties; @@ -261,8 +260,7 @@ public ImportPattern error( private String name; private final PropertyChangeSupport propChangeSupport = new PropertyChangeSupport( this); - private Properties properties = new UTF8Properties(); - private UTF8Properties reference = new UTF8Properties(); + private final UTF8Properties documentProperties = new UTF8Properties(); private final Map objectProperties = new HashMap<>(); private final Map changesToSave = new TreeMap<>(); private Processor owner; @@ -270,6 +268,7 @@ public ImportPattern error( private IDocument document; private long lastChangedAt; private Workspace workspace; + private final boolean effective; // Converter resolveModeFormatter = // EnumFormatter.create(ResolveMode.class, ResolveMode.manual); @@ -362,12 +361,12 @@ public ImportPattern error( } - /** * Default constructor */ public BndEditModel() { setOwner(new Processor()); + this.effective = false; } /** @@ -376,11 +375,21 @@ public BndEditModel() { * @param model the source */ public BndEditModel(BndEditModel model) { + this(model, false); + } + + /** + * Copy constructor with an override for effective + * + * @param model + * @param effective + */ + public BndEditModel(BndEditModel model, boolean effective) { this.inputFile = model.inputFile; this.workspace = model.workspace; - this.properties.putAll(model.properties); - this.reference.putAll(model.properties); + this.documentProperties.putAll(model.documentProperties); this.changesToSave.putAll(model.changesToSave); + this.effective = effective; setOwner(model.getOwner()); } @@ -390,6 +399,7 @@ public BndEditModel(Workspace workspace) { } public BndEditModel(IDocument document) throws IOException { + this.effective = false; loadFrom(document); } @@ -439,10 +449,8 @@ public void loadFrom(File file) throws IOException { public void loadFrom(InputStream inputStream) throws IOException { try { - properties.clear(); - reference.clear(); - properties.load(inputStream); - reference.putAll(properties); + documentProperties.clear(); + documentProperties.load(inputStream); objectProperties.clear(); changesToSave.clear(); @@ -472,21 +480,36 @@ public void saveChangesTo(IDocument document) { updateDocument(document, propertyName, stringValue); - // - // Ensure that properties keeps reflecting the current document - // value - // - String value = cleanup(stringValue); - if (value == null) - value = ""; - - if (propertyName != null) - properties.setProperty(propertyName, value); + updateProperty(propertyName, stringValue, documentProperties); iter.remove(); } } + private void updateProperty(String propertyName, String stringValue, UTF8Properties properties) { + if (propertyName == null) + return; + + if (stringValue == null) { + properties.remove(propertyName); + } else { + try { + properties.load(propertyName + ": " + stringValue, null, null); + } catch (IOException e) { + assert false; + } + } + } + + private UTF8Properties updatedProperties() { + UTF8Properties updated = new UTF8Properties(); + updated.putAll(documentProperties); + for (Map.Entry entry : changesToSave.entrySet()) { + updateProperty(entry.getKey(), entry.getValue(), updated); + } + return updated; + } + private static IRegion findEntry(IDocument document, String name) throws Exception { PropertiesLineReader reader = new PropertiesLineReader(document); LineType type = reader.next(); @@ -543,7 +566,7 @@ private static void updateDocument(IDocument document, String name, String value } public List getAllPropertyNames() { - return StreamSupport.stream(Iterables.iterable(properties.propertyNames(), String.class::cast) + return StreamSupport.stream(Iterables.iterable(documentProperties.propertyNames(), String.class::cast) .spliterator(), false) .collect(toList()); } @@ -564,7 +587,7 @@ public Object genericGet(String propertyName) { Converter converter = converters.get(propertyName); if (converter == null) converter = new NoopConverter<>(); - return doGetObject(propertyName, converter); + return doGetObject(propertyName, converter, false); } public void genericSet(String propertyName, Object value) { @@ -751,7 +774,7 @@ public void addPrivatePackage(String packageName) { } private boolean hasPrivatePackageInstruction() { - return properties.containsKey(Constants.PRIVATEPACKAGE); + return documentProperties.containsKey(Constants.PRIVATEPACKAGE); } @SuppressWarnings("unchecked") @@ -836,11 +859,11 @@ public void setImportPatterns(List patterns) { } public List getBuildPath() { - return doGetObject(Constants.BUILDPATH, buildPathConverter); + return doGetObject(Constants.BUILDPATH, buildPathConverter, true); } public List getTestPath() { - return doGetObject(Constants.TESTPATH, buildPathConverter); + return doGetObject(Constants.TESTPATH, buildPathConverter, true); } public void setBuildPath(List paths) { @@ -868,7 +891,7 @@ public void setTestPath(List paths) { } public List getRunBundles() { - return doGetObject(Constants.RUNBUNDLES, clauseListConverter); + return doGetObject(Constants.RUNBUNDLES, clauseListConverter, true); } public void setRunBundles(List paths) { @@ -877,7 +900,7 @@ public void setRunBundles(List paths) { } public List getRunBundlesDecorator() { - return doGetObject(aQute.bnd.osgi.Constants.RUNBUNDLES_DECORATOR, clauseListConverter); + return doGetObject(aQute.bnd.osgi.Constants.RUNBUNDLES_DECORATOR, clauseListConverter, true); } public void setRunBundlesDecorator(List paths) { @@ -912,7 +935,7 @@ public void setSubBndFiles(List subBndFiles) { } public Map getRunProperties() { - return doGetObject(Constants.RUNPROPERTIES, propertiesConverter); + return doGetObject(Constants.RUNPROPERTIES, propertiesConverter, true); } /* @@ -1013,7 +1036,6 @@ public Map> getPluginsProperties() { return getPropertiesMapInternal(Constants.PLUGIN, false); } - /** * @param stem * @param expandMacros controls whether or not macros like (${.}) will be @@ -1105,7 +1127,7 @@ private void setProperties(String stem, Maptrue if the given propertyKey is physically in the - * local {@link #properties} + * local {@link #documentProperties} */ private boolean isLocalPropertyKey(String key) { return doGetObject(key, headerClauseListConverter) != null; @@ -1168,7 +1190,7 @@ public void setRunFw(String clause) { } public List getRunRequires() { - return doGetObject(Constants.RUNREQUIRES, requirementListConverter); + return doGetObject(Constants.RUNREQUIRES, requirementListConverter, true); } public void setRunRequires(List requires) { @@ -1177,7 +1199,7 @@ public void setRunRequires(List requires) { } public List getRunBlacklist() { - return doGetObject(Constants.RUNBLACKLIST, requirementListConverter); + return doGetObject(Constants.RUNBLACKLIST, requirementListConverter, true); } public void setRunBlacklist(List requires) { @@ -1217,6 +1239,10 @@ public void setIgnoreStandalone(List headers) { } private R doGetObject(String name, Converter converter) { + return doGetObject(name, converter, false); + } + + private R doGetObject(String name, Converter converter, boolean merged) { try { R result; if (objectProperties.containsKey(name)) { @@ -1226,11 +1252,16 @@ private R doGetObject(String name, Converter co } else if (changesToSave.containsKey(name)) { result = converter.convert(changesToSave.get(name)); objectProperties.put(name, result); - } else if (properties.containsKey(name)) { - result = converter.convert(properties.getProperty(name)); - objectProperties.put(name, result); } else { - result = converter.convert(null); + if (effective) { + String value = merged ? owner.mergeProperties(name) : owner.getProperty(name); + if (value != null && value.isBlank()) + value = null; + result = converter.convert(value); + } else { + result = converter.convert(documentProperties.getProperty(name)); + objectProperties.put(name, result); + } } return result; @@ -1241,7 +1272,7 @@ private R doGetObject(String name, Converter co private void doRemoveObject(String name, T oldValue, T newValue, Converter formatter) { objectProperties.remove(name); - properties.remove(name); + documentProperties.remove(name); String v = formatter.convert(newValue); changesToSave.put(name, v); dirty = true; @@ -1249,15 +1280,11 @@ private void doRemoveObject(String name, T oldValue, T newValue, Converter void doSetObject(String name, T oldValue, T newValue, Converter formatter) { - objectProperties.put(name, newValue); - String v = formatter.convert(newValue); - if (v == null) { - if (oldValue == null) { - return; - } - doRemoveObject(name, oldValue, newValue, formatter); - return; + if (effective) { + throw new IllegalArgumentException("Read only because set to effective"); } + String v = formatter.convert(newValue); + objectProperties.put(name, newValue); changesToSave.put(name, v); dirty = true; propChangeSupport.firePropertyChange(name, oldValue, newValue); @@ -1379,7 +1406,7 @@ public void addIncludeResource(String resource) { } private boolean hasIncludeResourceHeaderLikeInstruction() { - return properties.containsKey(Constants.INCLUDE_RESOURCE); + return documentProperties.containsKey(Constants.INCLUDE_RESOURCE); } public Project getProject() { @@ -1434,77 +1461,30 @@ public Processor getProperties() throws Exception { * @throws IOException */ private Processor getPropertiesInternal(boolean expandMacros) throws IOException { - UTF8Properties currentProperties = new UTF8Properties(); - currentProperties.putAll(properties); - if (!changesToSave.isEmpty()) { - String changes = changesToString(); - currentProperties.load(changes, null, null); - } - - Set ownerLocalKeys = reference.keySet() - .stream() - .map(String.class::cast) - .collect(Collectors.toSet()); - Set editedLocalKeys = currentProperties.keySet() - .stream() - .map(String.class::cast) - .collect(Collectors.toSet()); - - Collection deleted = Logic.remove(ownerLocalKeys, editedLocalKeys); + UTF8Properties currentProperties = updatedProperties(); + UTF8Properties actualProperties = expandMacros ? currentProperties.replaceHere(owner.getBase()) + : currentProperties; File source = getBndResource(); - Processor dummy = new Processor(owner) { - @Override - public Set getPropertyKeys(boolean inherit) { - Set keys = super.getPropertyKeys(inherit); - keys.removeAll(deleted); - return keys; - } + Map changes = getDocumentChanges(); + Processor dummy = new Processor(owner) { @Override - public String getProperty(String key, String deflt, String separator) { - String value; - if (deleted.contains(key)) { - key = "IN_BNDEDIT_MODEL_DELETED_KEY_" + key; + public String getUnexpandedProperty(String key) { + if (changes.containsKey(key)) { + return actualProperties.getProperty(key); } - - if (expandMacros) { - value = super.getProperty(key, deflt, separator); - } else { - value = super.getUnprocessedProperty(key, deflt); - } - return value; + return super.getUnexpandedProperty(key); } }; dummy.setBase(owner.getBase()); dummy.setPropertiesFile(owner.getPropertiesFile()); - if (expandMacros) { - currentProperties = currentProperties.replaceHere(dummy.getBase()); - } dummy.getProperties() - .putAll(currentProperties); + .putAll(actualProperties); return dummy; } - String changesToString() { - StringBuilder sb = new StringBuilder(); - changesToSave.forEach((key, value) -> sb.append(key) - .append(':') - .append(' ') - .append(value) - .append('\n') - .append('\n')); - return sb.toString(); - } - - private String cleanup(String value) { - if (value == null) - return null; - - return value.replaceAll("\\\\\n", ""); - } - private static List disjunction(final Collection collection, final Collection remove) { final List list = new ArrayList<>(); for (final E obj : collection) { @@ -1667,4 +1647,19 @@ public void setProject(Project project) { setOwner(project); } + /** + * Return the document properties + */ + + public Properties getDocumentProperties() { + return documentProperties; + } + + /** + * Return if this model is handling effective properties (and this read only) or actual document properties. + */ + + public boolean isEffective() { + return effective; + } } diff --git a/biz.aQute.bndlib/src/aQute/bnd/build/model/package-info.java b/biz.aQute.bndlib/src/aQute/bnd/build/model/package-info.java index 34925e8671..3dd3c3d9e2 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/build/model/package-info.java +++ b/biz.aQute.bndlib/src/aQute/bnd/build/model/package-info.java @@ -1,4 +1,4 @@ -@Version("4.4.0") +@Version("4.5.0") package aQute.bnd.build.model; import org.osgi.annotation.versioning.Version; diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/Processor.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/Processor.java index ef511f5369..e91613a544 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/Processor.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/Processor.java @@ -643,14 +643,6 @@ public String getProperty(String key) { return getProperty(key, null); } - public String getUnexpandedProperty(String key) { - if (filter != null && filter.contains(key)) { - Object raw = getProperties().get(key); - return (raw instanceof String string) ? string : null; - } - return getProperties().getProperty(key); - } - public void mergeProperties(File file, boolean overwrite) { if (file.isFile()) { try { @@ -975,18 +967,28 @@ public static boolean isTrue(String value) { } /** - * Get a property without preprocessing it with a proper default + * Get a property without preprocessing it with a proper default. This is + * the ONLY place where we access the properties. * * @param key * @param deflt */ + @Deprecated public String getUnprocessedProperty(String key, String deflt) { + String v = getUnexpandedProperty(key); + if (v == null) + return deflt; + else + return v; + } + + public String getUnexpandedProperty(String key) { if (filter != null && filter.contains(key)) { Object raw = getProperties().get(key); - return (raw instanceof String string) ? string : deflt; + return (raw instanceof String string) ? string : null; } - return getProperties().getProperty(key, deflt); + return getProperties().getProperty(key, null); } /** diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/Verifier.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/Verifier.java index 0b8d7c4f40..180618f1d5 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/Verifier.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/Verifier.java @@ -464,7 +464,7 @@ else if (!analyzer.isImported(packageRef)) { } } else if (getParent() != null) { // If we have access to the parent we can do deeper checking - String raw = getParent().getUnprocessedProperty(BUNDLE_ACTIVATOR, null); + String raw = getParent().getUnexpandedProperty(BUNDLE_ACTIVATOR); if (raw != null) { // The activator was specified, but nothing showed up. if (raw.isEmpty()) { diff --git a/bndtools.builder/src/org/bndtools/builder/handlers/baseline/BundleVersionErrorHandler.java b/bndtools.builder/src/org/bndtools/builder/handlers/baseline/BundleVersionErrorHandler.java index 7ef858e597..04dc5de1e0 100644 --- a/bndtools.builder/src/org/bndtools/builder/handlers/baseline/BundleVersionErrorHandler.java +++ b/bndtools.builder/src/org/bndtools/builder/handlers/baseline/BundleVersionErrorHandler.java @@ -43,7 +43,7 @@ public List generateMarkerData(IProject project, Processor model, Lo for (Builder builder : pb.getSubBuilders()) { if (builder.getBsn() .equals(info.bsn)) { - String currentVersion = builder.getUnprocessedProperty(Constants.BUNDLE_VERSION, null); + String currentVersion = builder.getUnexpandedProperty(Constants.BUNDLE_VERSION); FileLine loc = builder.getHeader(Constants.BUNDLE_VERSION, currentVersion); Map attribs = new HashMap<>(); diff --git a/bndtools.release/src/bndtools/release/ReleaseHelper.java b/bndtools.release/src/bndtools/release/ReleaseHelper.java index 988ffbb432..bc5fa69771 100644 --- a/bndtools.release/src/bndtools/release/ReleaseHelper.java +++ b/bndtools.release/src/bndtools/release/ReleaseHelper.java @@ -512,7 +512,9 @@ public static List getBsnsWithBundleVersionMacro(List pr try { Builder builder = diff.getProject() .getSubBuilder(baseline.getBsn()); - String bundleVersion = builder.getUnprocessedProperty(Constants.BUNDLE_VERSION, ""); + String bundleVersion = builder.getUnexpandedProperty(Constants.BUNDLE_VERSION); + if (bundleVersion == null) + bundleVersion = ""; if (bundleVersion.startsWith("${")) { MacroInfo info = new MacroInfo(); info.projectDiff = diff; From 0ed37df5b49cb20f6af2a871b5f2b33a6df0d40e Mon Sep 17 00:00:00 2001 From: github-actions Date: Fri, 13 Dec 2024 14:52:32 +0100 Subject: [PATCH 3/3] Improved and added test cases --- Signed-off-by: github-actions Signed-off-by: github-actions --- .../test/test/BndEditModelTest.java | 81 ++++++++++--------- 1 file changed, 41 insertions(+), 40 deletions(-) diff --git a/biz.aQute.bndlib.tests/test/test/BndEditModelTest.java b/biz.aQute.bndlib.tests/test/test/BndEditModelTest.java index cada35aa31..c483e44779 100644 --- a/biz.aQute.bndlib.tests/test/test/BndEditModelTest.java +++ b/biz.aQute.bndlib.tests/test/test/BndEditModelTest.java @@ -10,6 +10,8 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; import org.junit.jupiter.api.Test; import org.osgi.framework.namespace.IdentityNamespace; @@ -21,12 +23,15 @@ import aQute.bnd.build.Workspace; import aQute.bnd.build.model.BndEditModel; import aQute.bnd.build.model.clauses.ExportedPackage; +import aQute.bnd.build.model.clauses.VersionedClause; import aQute.bnd.osgi.Constants; import aQute.bnd.osgi.Processor; import aQute.bnd.osgi.resource.CapReqBuilder; import aQute.bnd.properties.Document; +import aQute.lib.collections.ExtList; import aQute.lib.io.IO; import aQute.lib.strings.Strings; +import aQute.lib.utf8properties.UTF8Properties; public class BndEditModelTest { static CapReqBuilder cp = new CapReqBuilder(IdentityNamespace.IDENTITY_NAMESPACE); @@ -275,6 +280,8 @@ public void testGetPropertiesWithOnlyWorkspace() throws Exception { "osgi.identity;filter:='(osgi.identity=org.apache.felix.gogo.runtime)',osgi.identity;filter:='(osgi.identity=org.apache.felix.gogo.command)'" // ); + static final Set EXISTING_KEYS = Set.of("-runbundles", "-runrequires.debug", + "-runproperties.debug", "-include"); @Test public void testBasicReproducer() throws Exception { @@ -283,64 +290,58 @@ public void testBasicReproducer() throws Exception { assertThat(run.getProperties()).containsExactlyInAnyOrderEntriesOf(PROPERTIES); BndEditModel model = new BndEditModel(run); - assertThat(model.getAllPropertyNames()).containsExactlyInAnyOrder("-runbundles", "-runrequires.debug", - "-runproperties.debug", "-include"); + assertThat(model.getAllPropertyNames()).containsAll(EXISTING_KEYS); + + test("-runbundles", m -> m.setRunBundles(Collections.emptyList()), null); + test("-runfw", m -> m.setRunFw("foobar"), "foobar"); + test("-runbundles", m -> m.setRunBundles(new ExtList<>(new VersionedClause("foobar"))), "foobar"); + test("Bundle-Version", m -> m.setBundleVersion("1.2.3"), "1.2.3"); + test("-runfw", m -> m.setRunFw(null), null); } - @Test - public void testBasicReproducerRemove() throws Exception { + void test(String key, Consumer c, String value) throws Exception { assertThat(DEBUG_BNDRUN).isFile(); Run run = Run.createRun(null, DEBUG_BNDRUN); BndEditModel model = new BndEditModel(run); + String oldValue = PROPERTIES.get(key); + boolean isInFile = EXISTING_KEYS.contains(key); - assertThat(model.getProperties() - .getProperties()).containsKey("-runbundles"); assertThat(model.getDocumentChanges()).isEmpty(); - assertThat(model.getDocumentProperties() - .keySet()).containsExactlyInAnyOrder("-runbundles", "-runrequires.debug", "-runproperties.debug", - "-include"); - model.setRunBundles(Collections.emptyList()); + c.accept(model); + + assertThat(model.getDocumentChanges()).containsEntry(key, value); + if (oldValue != null && isInFile) + assertThat(model.getDocumentProperties()).containsEntry(key, oldValue); - assertThat(model.getRunBundles()).isEmpty(); - assertThat(model.getDocumentChanges()).containsKey("-runbundles"); assertThat(model.getProperties() - .getProperties()).doesNotContainKey("-runbundles"); + .getProperties() + .get(key)).isEqualTo(value); Document d = new Document(IO.collect(DEBUG_BNDRUN)); - assertThat(d.get()).contains("-runbundles"); - - model.saveChangesTo(d); - - assertThat(model.getDocumentChanges()).isEmpty(); - assertThat(d.get()).doesNotContain("-runbundles"); - assertThat(model.getDocumentProperties() - .keySet()).containsExactlyInAnyOrder("-runrequires.debug", "-runproperties.debug", "-include"); - } - - @Test - public void testBasicReproducerAdd() throws Exception { - assertThat(DEBUG_BNDRUN).isFile(); - Run run = Run.createRun(null, DEBUG_BNDRUN); - BndEditModel model = new BndEditModel(run); + UTF8Properties p = new UTF8Properties(); + p.load(d.get(), null, null); - assertThat(model.getProperties() - .getProperties()).doesNotContainKey("-runframework"); - - model.setRunFramework("none"); + if (isInFile && oldValue != null) + assertThat(p.get(key)).isEqualTo(oldValue); + else + assertThat(p.get(key)).isNull(); - assertThat(model.getDocumentProperties()).doesNotContainKey("-runframework"); + model.saveChangesTo(d); - assertThat(model.getProperties() - .getProperties()).containsKey("-runframework"); + if (value != null) { + assertThat(d.get()).contains(value); + } else { + assertThat(d.get()).doesNotContain(key); + } - assertThat(model.getRunFramework()).isEqualTo("none"); - Document d = new Document(IO.collect(DEBUG_BNDRUN)); - assertThat(d.get()).doesNotContain("-runframework"); - model.saveChangesTo(d); assertThat(model.getDocumentChanges()).isEmpty(); - assertThat(d.get()).contains("-runframework"); + if (value != null) + assertThat(model.getDocumentProperties()).containsEntry(key, value); + else + assertThat(model.getDocumentProperties()).doesNotContainKey(key); + } private String getPortablePath(File base) {