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

Adapt to removed support for loading pack.gz packaged artifacts in P2 #3168

Merged

Conversation

HannesWell
Copy link
Member

Eclipse P2 got its support for loading pack.gz packed artifacts from repositories finally removed in
eclipse-equinox/p2#310.

In order to fix the tests, remove packed duplicates from and replace only packed ecf artifact with unpacked eclipse.core.runtime artifact in test repository at \resources\repositories\packgz.

This is required for #3043 respectively #3125.

Copy link

github-actions bot commented Dec 6, 2023

Test Results

382 files  ±0  382 suites  ±0   2h 40m 24s ⏱️ - 9m 47s
376 tests ±0  370 ✔️ ±0    6 💤 ±0  0 ±0 
752 runs  ±0  739 ✔️ ±0  13 💤 ±0  0 ±0 

Results for commit 45575c9. ± Comparison against base commit b193066.

♻️ This comment has been updated with latest results.

@HannesWell HannesWell force-pushed the remove-packgz-from-tests branch from d46451b to a99c076 Compare December 6, 2023 23:53
@HannesWell
Copy link
Member Author

The LocalArtifactRepositoryP2APITest still needs some adjustments since it explicitly relays on packed artifacts.
Will continue tomorrow.

@laeubi
Copy link
Member

laeubi commented Dec 7, 2023

Maybe we should keep the pack.gz items in the repository to tests they are really ignored and just adjust the number in the assertions? If a test relies on packgz to work and can't be adjusted it is maybe time to delete it.

@akurtakov akurtakov force-pushed the remove-packgz-from-tests branch from a99c076 to bdfe7a5 Compare December 7, 2023 06:35
@akurtakov
Copy link
Member

@HannesWell Are you gonna be working on this one today?

@HannesWell
Copy link
Member Author

I will check which tests not make sense anymore and remove or adjust them based on the gone support of packgz.

@HannesWell Are you gonna be working on this one today?

I'm sorry but I won't have time for this this evening, but will continue tomorrow.

Eclipse P2 got its support for loading pack.gz packed artifacts from
repositories finally removed in
eclipse-equinox/p2#310.

In order to fix the tests, remove packed duplicates from and replace
only packed ecf artifact with unpacked eclipse.core.runtime artifact in
test repository at '\resources\repositories\packgz'.
@HannesWell HannesWell force-pushed the remove-packgz-from-tests branch from bdfe7a5 to 45575c9 Compare December 9, 2023 15:30
@HannesWell
Copy link
Member Author

I now also adapted the test cases in LocalArtifactRepositoryP2APITest to reflect the fact that there a no two descriptors/real artifacts for the same artifact-key anymore (I'm not aware of a scenario where this can happen any more now that pack.gz is gone).
Test cases that are pointless with only one artifact per key are now removed.
Locally all cases in that class passed, lets hope that they also pass in the CI and that there are no more cases that fail.

@HannesWell HannesWell added the backport-to-tycho-4.0.x Can be added to a PR to trigger an automatic backport of the change label Dec 9, 2023
@laeubi
Copy link
Member

laeubi commented Dec 9, 2023

I'm not aware of a scenario where this can happen any more now that pack.gz is gone).

It is not really gone... all already released updatesites still contain such artifacts thats why they should be ignored, that means even if a repository contains pack.gz it is simply not used.

@HannesWell
Copy link
Member Author

I'm puzzled why FeatureRootfileArtifactRepositoryTest failed in the GH Linux workflow?
Locally it fails for me to, but even on the current master. So I think this is not related to my change.
Will re-start the workflow once all the others terminated.

I'm not aware of a scenario where this can happen any more now that pack.gz is gone).

It is not really gone... all already released updatesites still contain such artifacts thats why they should be ignored, that means even if a repository contains pack.gz it is simply not used.

Of course. I expressed myself unclear. Let me rephrase it: Now that pack.gz artifacts are not read/considered anymore.

@laeubi
Copy link
Member

laeubi commented Dec 9, 2023

Yes Github Linux fails but unless Jenkins linux succeeds its not a problem I want to take a look once we have merged all the cleanup/upgrade stuff, it seems the test must somehow be a TychoPlexusTestcase now because "something" triggers access to Extension registry.

@HannesWell
Copy link
Member Author

Yes Github Linux fails but unless Jenkins linux succeeds its not a problem I want to take a look once we have merged all the cleanup/upgrade stuff, it seems the test must somehow be a TychoPlexusTestcase now because "something" triggers access to Extension registry.

OK, good to know.
Then I will submit this if all other checks (and #3043, which also contains this) pass so that we can continue with the back-port.

@HannesWell
Copy link
Member Author

Jenkins almost fully succeeded, only the following integration tests failed

  • org.eclipse.tycho.test.p2Repository.P2RepositoryDownloadTest.testReactorCanBeVerified
  • org.eclipse.tycho.test.reactor.makeBehaviour.MavenReactorMakeOptionsTest.testCompleteBuild

But they also fail on the master branch so its not related.
Therefore submitting this now.

@HannesWell HannesWell merged commit da254b3 into eclipse-tycho:master Dec 9, 2023
9 of 11 checks passed
Copy link

github-actions bot commented Dec 9, 2023

💚 All backports created successfully

Status Branch Result
tycho-4.0.x

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@HannesWell HannesWell deleted the remove-packgz-from-tests branch December 9, 2023 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-tycho-4.0.x Can be added to a PR to trigger an automatic backport of the change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants