Remove usage of the sequence number from target files

[laeubi]

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 and is nowhere used in such a way:

• 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.

[eclipse-pde-bot]

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

ui/org.eclipse.pde.genericeditor.extension.tests/META-INF/MANIFEST.MF
ui/org.eclipse.pde.genericeditor.extension.tests/pom.xml

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch

From dbf1799189f640ab6ddf7ffff627c13480ad818b Mon Sep 17 00:00:00 2001
From: Eclipse PDE Bot <[email protected]>
Date: Tue, 31 Dec 2024 13:35:53 +0000
Subject: [PATCH] Version bump(s) for 4.35 stream


diff --git a/ui/org.eclipse.pde.genericeditor.extension.tests/META-INF/MANIFEST.MF b/ui/org.eclipse.pde.genericeditor.extension.tests/META-INF/MANIFEST.MF
index 7ff765d764..1e8406f358 100644
--- a/ui/org.eclipse.pde.genericeditor.extension.tests/META-INF/MANIFEST.MF
+++ b/ui/org.eclipse.pde.genericeditor.extension.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: Tests for Generic Target Platform Editor
 Bundle-SymbolicName: org.eclipse.pde.genericeditor.extension.tests
-Bundle-Version: 1.2.500.qualifier
+Bundle-Version: 1.2.600.qualifier
 Bundle-Vendor: Eclipse.org
 Bundle-RequiredExecutionEnvironment: JavaSE-17
 Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.29.0,4.0.0)",
diff --git a/ui/org.eclipse.pde.genericeditor.extension.tests/pom.xml b/ui/org.eclipse.pde.genericeditor.extension.tests/pom.xml
index e99337cff1..6a338c2f1b 100644
--- a/ui/org.eclipse.pde.genericeditor.extension.tests/pom.xml
+++ b/ui/org.eclipse.pde.genericeditor.extension.tests/pom.xml
@@ -18,7 +18,7 @@
     <relativePath>../../</relativePath>
   </parent>
   <artifactId>org.eclipse.pde.genericeditor.extension.tests</artifactId>
-  <version>1.2.500-SNAPSHOT</version>
+  <version>1.2.600-SNAPSHOT</version>
   <packaging>eclipse-test-plugin</packaging>
 
   <properties>
-- 
2.47.1

Further information are available in Common Build Issues - Missing version increments.

[github-actions]

Test Results

   285 files  ±0     285 suites  ±0   46m 15s ⏱️ - 3m 49s
 3 585 tests  - 1   3 509 ✅  - 1   76 💤 ±0  0 ❌ ±0 
10 947 runs   - 3  10 716 ✅  - 3  231 💤 ±0  0 ❌ ±0 

Results for commit 176b613. ± Comparison against base commit cb0d691.

This pull request removes 1 test.

AllPDETests org.eclipse.pde.ui.tests.target.AllTargetTests org.eclipse.pde.ui.tests.target.TargetDefinitionPersistenceTests ‑ testSequenceNumberChange

♻️ This comment has been updated with latest results.

[merks]

We discussed my small concern that we might just change the checkProfile method to return false when the sequencer numbers are not equal. But given the sequence number seems poorly managed and poorly behave since forever, I'm not sure keeping it has any real significant value. I also checked that the Target DSL generator won't be broken by the changes, and that looks fine too.

[laeubi]

This seem to affect a small area in API tools :-(

Therefore I decided to start first with a much smaller approach here:

• Remove the check for sequence number from P2TargetUtils.checkProfile #1541

in the long run I still think we should get rid of the sequenceNumber altogether at it seems not working reliable.

[HannesWell]

in the long run I still think we should get rid of the sequenceNumber altogether at it seems not working reliable.

Yes, absolutely! For the reasons you listed in your initial message. In general I always found it odd that this was even 'necessary' and PDE should be smart enough to detect changes without such a counter that has to be persisted in the file.
Just in case you missed it: I recently extended the sync-check and everything relevant in IU-locations should now be checked:
#1438

I can also have a look at this in detail, probably at the end of this week, but I like the overall direction very much.

[laeubi]

With all the preliminary changes now we should be able to finally remove the sequence number all togehter!