Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix-artifacts-metadata doesn't fix the md5.checksum #2875

Closed
henryju opened this issue Oct 2, 2023 · 5 comments
Closed

fix-artifacts-metadata doesn't fix the md5.checksum #2875

henryju opened this issue Oct 2, 2023 · 5 comments

Comments

@henryju
Copy link

henryju commented Oct 2, 2023

Hi,

Using Tycho 4.0.2.

We are including some third-party dependencies in our p2 repository, and we are signing all artifacts with our own certificate. As a result, the checksums are incorrect, so we are using the fix-artifacts-metadata goal to recompute all hashes.
A user reported that the sh256 checksum is correct, while the md5 one is still the old one from the source (Orbit) repository.

I think I will simply exclude Orbit artifacts from our sign process, since they are already signed anyway, but maybe there is an issue to fix in Tycho as well?

@laeubi
Copy link
Member

laeubi commented Oct 2, 2023

@henryju I think the issue is that P2 has dropped md5 support, so I think Tycho must remove the md5 when mirroring things.

@mdaloia
Copy link
Contributor

mdaloia commented May 27, 2024

@laeubi We came across this same issue using 4.0.7 (current latest).

It seems that the checksums are re-calculated but only for those that the current director version used by Tycho knows that it needs to be published.
The problem is that old checksums (like the md5) are left there with an outdated value and if you use a p2 director application that it is old (before md5 was disable) it fails the check. For example:

  • Modifying a jar:
[INFO] --- antrun:3.1.0:run (touch-jar) @ product ---
[INFO] Executing tasks
[WARNING]      [echo] File original MD5 = e88a1e99c0c9ee29b96b3159b54e419a
[WARNING]      [echo] File original SHA-1 = 0ef3b09190da2fb64d81985372e44c974ccd8468
[WARNING]      [echo] File original SHA-256 = c49f54f646587f98b9cbfd5239f34e3f52d3f85a1aa1f68b416192a563bca3ec
[WARNING]      [echo] File original SHA-512 = 22971289b7d28d47a102b339f7b896f3c4be517bc9bcb48d8ad2b5f298503cb0f57822c199a5961fb74fabecfefc22d4051dd531259301808dd024cfbf018e2d
[INFO]     [unzip] Expanding: /Users/mdaloia/tycho-issue/product/target/repository/plugins/org.eclipse.nebula.cwt_1.1.0.202402011804.jar into /Users/mdaloia/tycho-issue/product/target/tmp
[INFO]       [zip] Building zip: /Users/mdaloia/tycho-issue/product/target/repository/plugins/org.eclipse.nebula.cwt_1.1.0.202402011804.jar
[WARNING]      [echo] File new MD5 = d6f70d9bb0184a5bf86cafb942c35fd7
[WARNING]      [echo] File new SHA-1 = 9d689f6f2ba4f4ac026c6011c809e697d0e3617f
[WARNING]      [echo] File new SHA-256 = e25c8fb8f3ca117a93d230926dd6d9932203d16fa4b4064e504da36809a90041
[WARNING]      [echo] File new SHA-512 = bfb3f37f3003916acd9bff52791475fa0ea59c06bf209724e04d2a573f0252f18b8e1cceb10fd19a911621a7a460bc96f6e1fad27b7cd0e27beb49be3fbfbe8e
[INFO] Executed tasks
  • and running fix-artifacts-metadata (see the download.md5 and download.checksum.md5 values):
    <artifact classifier='osgi.bundle' id='org.eclipse.nebula.cwt' version='1.1.0.202402011804'>
      <properties size='10'>
        <property name='artifact.size' value='163175'/>
        <property name='download.size' value='163175'/>
        <property name='maven-groupId' value='org.eclipse.nebula'/>
        <property name='maven-artifactId' value='org.eclipse.nebula.cwt'/>
        <property name='maven-version' value='1.1.0-SNAPSHOT'/>
        <property name='download.md5' value='e88a1e99c0c9ee29b96b3159b54e419a'/>
        <property name='download.checksum.md5' value='e88a1e99c0c9ee29b96b3159b54e419a'/>
        <property name='download.checksum.sha-256' value='e25c8fb8f3ca117a93d230926dd6d9932203d16fa4b4064e504da36809a90041'/>
        <property name='download.checksum.sha-512' value='bfb3f37f3003916acd9bff52791475fa0ea59c06bf209724e04d2a573f0252f18b8e1cceb10fd19a911621a7a460bc96f6e1fad27b7cd0e27beb49be3fbfbe8e'/>
        <property name='download.checksum.sha-1' value='9d689f6f2ba4f4ac026c6011c809e697d0e3617f'/>
      </properties>
    </artifact>
  • trying to use the Update Site with an old p2 director:
An error occurred while collecting items to be installed
	session context was:(profile=DefaultProfile, phase=org.eclipse.equinox.internal.p2.engine.phases.Collect, operand=, action=).
	Problems downloading artifact: osgi.bundle,org.eclipse.nebula.cwt,1.1.0.202402011804.
		MD5 hash is not as expected. Expected: e88a1e99c0c9ee29b96b3159b54e419a and found d6f70d9bb0184a5bf86cafb942c35fd7.

Alternatives I can think to fix fix-artifacts-metadata goal:

  1. Remove all the checksum properties for an artifact and store only the new ones calculated with current checksum algorithms.
  2. Add an option in the goal configuration to exclude writing certain properties (if they are present) and let users to determine which ones to remove. This could also be used for other kind of cases.
  3. Recompute all the current checksums already present in the metadata for an artifact.

References:

@laeubi
Copy link
Member

laeubi commented May 31, 2024

MD5 (and even SHA1) are marked as "do not use for publish", so we should either remove or update them if they are present, I wonder if there is an example project or even an integration-test to demonstrate the issue?

laeubi added a commit to laeubi/tycho that referenced this issue May 31, 2024
Currently Tycho/P2 only *adds* checksums but as some are now disabled
for publish this can lead to the case where an existing checksum is kept
as-is.

This now performs and additional check if there are old checksums and
remove them when recreate the repository.

Fix eclipse-tycho#2875
@laeubi
Copy link
Member

laeubi commented May 31, 2024

I created now a PR, but think we should have at laest one testcase to verify it actually works.

@henryju it seems in your referenced issue you was able to replicate this with some slf4j bundle, would you mind to try providing a reproducer?

laeubi added a commit to laeubi/tycho that referenced this issue May 31, 2024
Currently Tycho/P2 only *adds* checksums but as some are now disabled
for publish this can lead to the case where an existing checksum is kept
as-is.

This now performs and additional check if there are old checksums and
remove them when recreate the repository.

Fix eclipse-tycho#2875
@henryju
Copy link
Author

henryju commented Jun 3, 2024

@laeubi sorry I am away from Eclipse plugin development those days, so I won't have the time to set up a proper IT.

The minimal reproducer should be something like:

In the target platform:

<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
      <unit id="org.slf4j.api" version="0.0.0"/>
      <repository location="https://download.eclipse.org/tools/orbit/downloads/drops/R20220531185310/repository/"/>
    </location>

In the feature/feature.xml:

<plugin
         id="org.slf4j.api"
         download-size="0"
         install-size="0"
         version="0.0.0"
         unpack="false"/>

In the site/pom.xml:

<plugin>
        <groupId>org.eclipse.tycho</groupId>
        <artifactId>tycho-p2-repository-plugin</artifactId>
        <executions>
          <execution>
            <id>update</id>
            <goals>
              <goal>fix-artifacts-metadata</goal>
            </goals>
          </execution>
          <execution>
            <id>verify</id>
            <goals>
              <goal>verify-repository</goal>
            </goals>
          </execution>
        </executions>
      </plugin>

mdaloia added a commit to mdaloia/tycho that referenced this issue Jun 3, 2024
mdaloia added a commit to mdaloia/tycho that referenced this issue Jun 3, 2024
This integration test shows that `download.md5` and
`download.checksum.md5` are present after running the
`fix-artifacts-metadata` goal.

If these are not removed and the content of the artifact changed then,
after the recalculation done by this goal, new p2 libs will see an
outdated value for these old checksum algorithms and will fail the
installation due to a checksum mismatch.
mdaloia added a commit to mdaloia/tycho that referenced this issue Jun 3, 2024
This integration test shows that `download.md5` and
`download.checksum.md5` are present after running the
`fix-artifacts-metadata` goal when they shouldn't.

If these are not removed and the content of the artifact changed then,
after the recalculation done by this goal, new p2 libs will see an
outdated value for these old checksum algorithms and will fail the
installation due to a checksum mismatch.
laeubi added a commit to laeubi/tycho that referenced this issue Jun 6, 2024
Currently Tycho/P2 only *adds* checksums but as some are now disabled
for publish this can lead to the case where an existing checksum is kept
as-is.

This now performs and additional check if there are old checksums and
remove them when recreate the repository.

Fix eclipse-tycho#2875
laeubi pushed a commit to laeubi/tycho that referenced this issue Jun 6, 2024
This integration test shows that `download.md5` and
`download.checksum.md5` are present after running the
`fix-artifacts-metadata` goal when they shouldn't.

If these are not removed and the content of the artifact changed then,
after the recalculation done by this goal, new p2 libs will see an
outdated value for these old checksum algorithms and will fail the
installation due to a checksum mismatch.
@laeubi laeubi closed this as completed in ae2aae1 Jun 7, 2024
laeubi pushed a commit that referenced this issue Jun 7, 2024
This integration test shows that `download.md5` and
`download.checksum.md5` are present after running the
`fix-artifacts-metadata` goal when they shouldn't.

If these are not removed and the content of the artifact changed then,
after the recalculation done by this goal, new p2 libs will see an
outdated value for these old checksum algorithms and will fail the
installation due to a checksum mismatch.
eclipse-tycho-bot pushed a commit that referenced this issue Jun 7, 2024
Currently Tycho/P2 only *adds* checksums but as some are now disabled
for publish this can lead to the case where an existing checksum is kept
as-is.

This now performs and additional check if there are old checksums and
remove them when recreate the repository.

Fix #2875

(cherry picked from commit ae2aae1)
eclipse-tycho-bot pushed a commit that referenced this issue Jun 7, 2024
This integration test shows that `download.md5` and
`download.checksum.md5` are present after running the
`fix-artifacts-metadata` goal when they shouldn't.

If these are not removed and the content of the artifact changed then,
after the recalculation done by this goal, new p2 libs will see an
outdated value for these old checksum algorithms and will fail the
installation due to a checksum mismatch.

(cherry picked from commit b88a16e)
eclipse-tycho-bot pushed a commit that referenced this issue Jun 7, 2024
Currently Tycho/P2 only *adds* checksums but as some are now disabled
for publish this can lead to the case where an existing checksum is kept
as-is.

This now performs and additional check if there are old checksums and
remove them when recreate the repository.

Fix #2875

(cherry picked from commit ae2aae1)
eclipse-tycho-bot pushed a commit that referenced this issue Jun 7, 2024
This integration test shows that `download.md5` and
`download.checksum.md5` are present after running the
`fix-artifacts-metadata` goal when they shouldn't.

If these are not removed and the content of the artifact changed then,
after the recalculation done by this goal, new p2 libs will see an
outdated value for these old checksum algorithms and will fail the
installation due to a checksum mismatch.

(cherry picked from commit b88a16e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants