Remove usage of the sequence number from target files

Currently there is an attribute 'sequenceNumber' that should change
"whenever something in the target that affects the set of features and
bundles that would be resolved" but effectively this never worked very
well:

- A sequence number is not mandatory
- Target files are often edited by humans and it is easy to forget to
increment those
- Updating it was never public API so only with some operations it was
possible to implicitly change that
- it is only used to check if a P2 profile is up-to-date where the
sequence number is a very bad indication as actually even things outside
the target can influence it
- even if the sequence number does not matches, P2TargetUtils performed
some other checks if maybe the profile has not changed, so P2TargetUtils
is already aware of changes without it

To summarize if the sequence number did not changed the profile is
considered clean (what might be wrong), if it changed we ignore it and
check our self if it changed.

To prevent us from strange errors and uncertainties, this now completely
removes the 'sequenceNumber' and corresponding code leaving the
responsibility to detect something changes completely to the
implementation of the target location.

Author: laeubi

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetDefinition.java

@@ -124,8 +124,6 @@ public class TargetDefinition implements ITargetDefinition {
 	private TargetFeature[] fFeatures;
 	private TargetBundle[] fOtherBundles;
 
-	private int fSequenceNumber = -1;
-
 	/**
 	 * Constructs a target definition based on the given handle.
 	 */
@@ -210,7 +208,6 @@ public String getWS() {
 
 	@Override
 	public void setArch(String arch) {
-		incrementSequenceNumber();
 		fArch = arch;
 		if (fRoot != null && arch != null && !arch.isEmpty()) {
 			Element archNode = TargetDefinitionDocumentTools.getChildElement(fRoot,
@@ -224,7 +221,6 @@ public void setArch(String arch) {
 
 	@Override
 	public void setNL(String nl) {
-		incrementSequenceNumber();
 		fNL = nl;
 		if (fRoot != null && nl != null && !nl.isEmpty()) {
 			Element nlNode = TargetDefinitionDocumentTools.getChildElement(fRoot,
@@ -250,7 +246,6 @@ public void setName(String name) {
 
 	@Override
 	public void setOS(String os) {
-		incrementSequenceNumber();
 		fOS = os;
 		if (fRoot != null && os != null && !os.isEmpty()) {
 			Element nlNode = TargetDefinitionDocumentTools.getChildElement(fRoot,
@@ -296,7 +291,6 @@ public void setVMArguments(String args) {
 
 	@Override
 	public void setWS(String ws) {
-		incrementSequenceNumber();
 		fWS = ws;
 		if (fRoot != null && ws != null && !ws.isEmpty()) {
 			Element nlNode = TargetDefinitionDocumentTools.getChildElement(fRoot,
@@ -310,7 +304,6 @@ public void setWS(String ws) {
 
 	@Override
 	public void setTargetLocations(ITargetLocation[] locations) {
-		incrementSequenceNumber();
 		// Clear the feature model cache as it is based on the bundle container locations
 		fFeatures = null;
 		fOtherBundles = null;
@@ -770,7 +763,6 @@ void setContents(InputStream stream) throws CoreException {
 			fProgramArgs = null;
 			fVMArgs = null;
 			fWS = null;
-			fSequenceNumber = 0;
 			fDocument = null;
 			fRoot = null;
 			TargetDefinitionPersistenceHelper.initFromXML(this, stream);
@@ -819,7 +811,6 @@ public NameVersionDescriptor[] getImplicitDependencies() {
 
 	@Override
 	public void setImplicitDependencies(NameVersionDescriptor[] bundles) {
-		incrementSequenceNumber();
 		if (bundles != null && bundles.length == 0) {
 			bundles = null;
 		}
@@ -1106,26 +1097,6 @@ public void setUIMode(int mode) {
 		}
 	}
 
-	/**
-	 * Returns the current sequence number of this target.  Sequence numbers change
-	 * whenever something in the target that affects the set of features and bundles that
-	 * would be resolved.
-	 *
-	 * @return the current sequence number
-	 */
-	public int getSequenceNumber() {
-		return fSequenceNumber;
-	}
-
-	/**
-	 * Increases the current sequence number.
-	 * @see TargetDefinition#getSequenceNumber()
-	 * @return the current sequence number after it has been increased
-	 */
-	public int incrementSequenceNumber() {
-		return ++fSequenceNumber;
-	}
-
 	private void removeElement(String... childNames) {
 		if (fRoot != null) {
 			TargetDefinitionDocumentTools.removeElement(fRoot, childNames);

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPersistence36Helper.java

@@ -41,7 +41,7 @@ public class TargetPersistence36Helper {
 
 	/* Example of Software location in Target XML
 
-	<?xml version="1.0" encoding="UTF-8"?><?pde version="3.6"?><target name="SoftwareSiteTarget" sequenceNumber="6">
+	<?xml version="1.0" encoding="UTF-8"?><?pde version="3.6"?><target name="SoftwareSiteTarget">
 	<locations>
 	<location includeAllPlatforms="false" includeMode="slicer" includeSource="true" type="InstallableUnit">
 	<unit id="org.eclipse.egit.feature.group" version="0.11.3"/>

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPersistence38Helper.java

@@ -54,7 +54,7 @@ public class TargetPersistence38Helper {
 	<?xml version="1.0" encoding="UTF-8" standalone="no"?>
 	<?pde version="3.8"?>
 
-	<target name="test" sequenceNumber="9">
+	<target name="test">
 	<locations>
 	<location path="${eclipse_home}" type="Directory"/>
 	<location path="${eclipse_home}" type="Profile"/>

ui/org.eclipse.pde.genericeditor.extension.tests/src/org/eclipse/pde/genericeditor/extension/tests/AttributeNameCompletionTests.java

@@ -28,7 +28,7 @@ public class AttributeNameCompletionTests extends AbstractTargetEditorTest {
 	public void testAttributeNameSuggestions() throws Exception {
 		Map<Integer, String[]> expectedProposalsByOffset = new HashMap<>();
 		// target
-		expectedProposalsByOffset.put(8, new String[] { "name", "sequenceNumber" });
+		expectedProposalsByOffset.put(8, new String[] { "name" });
 		// location
 		expectedProposalsByOffset.put(33, new String[] { "followRepositoryReferences", "includeAllPlatforms",
 				"includeConfigurePhase", "includeMode", "includeSource", "type" });

ui/org.eclipse.pde.genericeditor.extension/src/org/eclipse/pde/internal/genericeditor/target/extension/autocomplete/TagCompletionProposal.java

@@ -24,8 +24,7 @@ public class TagCompletionProposal extends TargetCompletionProposal {
 
 	static {
 		tagStartingAttributesAndValues.put(ITargetConstants.TARGET_TAG, new Attribute[] {
-				new Attribute(ITargetConstants.TARGET_NAME_ATTR, null),
-				new Attribute(ITargetConstants.TARGET_SEQ_NO_ATTR, "1") });
+				new Attribute(ITargetConstants.TARGET_NAME_ATTR, null) });
 		tagStartingAttributesAndValues.put(ITargetConstants.LOCATION_DIRECTORY_COMPLETION_LABEL, new Attribute[] {
 				new Attribute(ITargetConstants.LOCATION_PATH_ATTR, null),
 				new Attribute(ITargetConstants.LOCATION_TYPE_ATTR, ITargetConstants.LOCATION_TYPE_ATTR_VALUE_DIRECTORY) });

ui/org.eclipse.pde.genericeditor.extension/src/org/eclipse/pde/internal/genericeditor/target/extension/autocomplete/processors/AttributeNameCompletionProcessor.java

@@ -47,7 +47,7 @@ public class AttributeNameCompletionProcessor extends DelegateProcessor {
 	private static final String ATTRIBUTE_NAME_FIND = "(?:\\s*(\\w*)\\s*=\\s*\".*?\")";// $NON-NLS-1$
 	private static final Pattern ATT_NAME_PATTERN = Pattern.compile(ATTRIBUTE_NAME_FIND);
 
-	private final String[] target = new String[] { ITargetConstants.TARGET_NAME_ATTR, ITargetConstants.TARGET_SEQ_NO_ATTR };
+	private final String[] target = new String[] { ITargetConstants.TARGET_NAME_ATTR };
 	private final String[] locations = new String[] {};
 	private final String[] location = new String[] { ITargetConstants.LOCATION_INCLUDE_PLATFORMS_ATTR,
 			ITargetConstants.LOCATION_INCLUDE_CONFIG_PHASE_ATTR, ITargetConstants.LOCATION_FOLLOW_REPOSITORY_REFERENCES_ATTR,

ui/org.eclipse.pde.genericeditor.extension/src/org/eclipse/pde/internal/genericeditor/target/extension/model/ITargetConstants.java

@@ -48,7 +48,6 @@ public interface ITargetConstants {
 	String UNIT_ID_ATTR = "id"; //$NON-NLS-1$
 	String UNIT_VERSION_ATTR = "version"; //$NON-NLS-1$
 	String TARGET_NAME_ATTR = "name"; //$NON-NLS-1$
-	String TARGET_SEQ_NO_ATTR = "sequenceNumber"; //$NON-NLS-1$
 	String LOCATION_INCLUDE_PLATFORMS_ATTR = "includeAllPlatforms"; //$NON-NLS-1$
 	String LOCATION_INCLUDE_CONFIG_PHASE_ATTR = "includeConfigurePhase"; //$NON-NLS-1$
 	String LOCATION_FOLLOW_REPOSITORY_REFERENCES_ATTR = "followRepositoryReferences";

ui/org.eclipse.pde.genericeditor.extension/src/org/eclipse/pde/internal/genericeditor/target/extension/reconciler/presentation/TargetPlatformAttributeRule.java

@@ -15,17 +15,16 @@
 
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.INCLUDE_DEPENDENCY_DEPTH;
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.INCLUDE_DEPENDENCY_SCOPES;
+import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.LOCATION_FOLLOW_REPOSITORY_REFERENCES_ATTR;
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.LOCATION_INCLUDE_CONFIG_PHASE_ATTR;
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.LOCATION_INCLUDE_MODE_ATTR;
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.LOCATION_INCLUDE_PLATFORMS_ATTR;
-import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.LOCATION_FOLLOW_REPOSITORY_REFERENCES_ATTR;
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.LOCATION_INCLUDE_SOURCE_ATTR;
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.LOCATION_TYPE_ATTR;
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.MISSING_MANIFEST;
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.REPOSITORY_LOCATION_ATTR;
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.TARGET_JRE_PATH_ATTR;
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.TARGET_NAME_ATTR;
-import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.TARGET_SEQ_NO_ATTR;
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.UNIT_ID_ATTR;
 import static org.eclipse.pde.internal.genericeditor.target.extension.model.ITargetConstants.UNIT_VERSION_ATTR;
 
@@ -42,7 +41,7 @@ public class TargetPlatformAttributeRule extends WordRule {
 
 	private static final String[] ATTRIBUTES = new String[] { TARGET_NAME_ATTR, UNIT_VERSION_ATTR, UNIT_ID_ATTR,
 			LOCATION_INCLUDE_PLATFORMS_ATTR, LOCATION_INCLUDE_MODE_ATTR, LOCATION_TYPE_ATTR, REPOSITORY_LOCATION_ATTR,
-			TARGET_JRE_PATH_ATTR, TARGET_SEQ_NO_ATTR, LOCATION_INCLUDE_CONFIG_PHASE_ATTR,
+			TARGET_JRE_PATH_ATTR, LOCATION_INCLUDE_CONFIG_PHASE_ATTR,
 			LOCATION_FOLLOW_REPOSITORY_REFERENCES_ATTR, LOCATION_INCLUDE_SOURCE_ATTR, INCLUDE_DEPENDENCY_DEPTH,
 			INCLUDE_DEPENDENCY_SCOPES, MISSING_MANIFEST };
 	private final IToken attributeToken = new Token(

ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/TargetDefinitionPersistenceTests.java

@@ -482,39 +482,6 @@ private static void validateTypeAndLocation(ITargetLocation container, Class<?>
 		assertEquals(rawLocation, container.getLocation(false));
 	}
 
-	/**
-	 * Tests that we increment the sequence number correctly when target is
-	 * modified contents.
-	 */
-	@Test
-	public void testSequenceNumberChange() throws Exception {
-		ITargetDefinition target = readOldTarget("featureLocations");
-
-		assertEquals("Wrong name", "Features", target.getName());
-		TargetDefinition targetDef = (TargetDefinition) target;
-		int currentSeqNo = targetDef.getSequenceNumber();
-		target.setArch("x86");
-		asssertSequenceNumber("Arch", currentSeqNo, targetDef);
-
-		currentSeqNo = targetDef.getSequenceNumber();
-		target.setOS("win32");
-		asssertSequenceNumber("OS", currentSeqNo, targetDef);
-
-		currentSeqNo = targetDef.getSequenceNumber();
-		target.setNL("hi_IN");
-		asssertSequenceNumber("NL", currentSeqNo, targetDef);
-
-		currentSeqNo = targetDef.getSequenceNumber();
-		target.setWS("carbon");
-		asssertSequenceNumber("WS", currentSeqNo, targetDef);
-
-		ITargetLocation[] newContainers = new ITargetLocation[1];
-		newContainers[0] = new DirectoryBundleContainer("Path");
-		currentSeqNo = targetDef.getSequenceNumber();
-		target.setTargetLocations(newContainers);
-		asssertSequenceNumber("Bundle Containers", currentSeqNo, targetDef);
-	}
-
 	@Test
 	public void testIncludeSource() throws Exception {
 		ITargetDefinition target = readOldTarget("SoftwareSiteTarget");
@@ -684,9 +651,4 @@ private void assertTargetDefinitionsEqual(ITargetDefinition targetA, ITargetDefi
 		assertTrue("Target content not equal", ((TargetDefinition) targetA).isContentEqual(targetB));
 	}
 
-	private void asssertSequenceNumber(String name, int currentSeqNo, TargetDefinition targetDef) {
-		assertEquals("Sequence number did not increment after updating '" + name + "'", currentSeqNo + 1,
-				targetDef.getSequenceNumber());
-	}
-
 }

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/ToggleIncludeHandler.java

@@ -101,9 +101,6 @@ public IStatus toggle(ITargetDefinition target, TreePath[] treePath) {
 					include.stream());
 		}
 		target.setIncluded(stream.toArray(NameVersionDescriptor[]::new));
-		if (target instanceof TargetDefinition) {
-			((TargetDefinition) target).incrementSequenceNumber();
-		}
 		return Status.OK_STATUS;
 	}