Don't await a StackOverflowError on a redirection loop

[thahnen]

In case of a redirection loop for URI of a p2 repository, fail fast on a redirection loop. In this case provide a warning to the user and let the rest be handled by Tycho. Otherwise there might be a StackOverflowError due to the recursion used and no check for whether the base URI is the same as the redirected URI.

This links to #4451 but does not fix it, only raising visibility for such cases.

[github-actions]

Test Results

  618 files  ±0    618 suites  ±0   4h 40m 5s ⏱️ + 21m 18s
  450 tests ±0    441 ✅  - 1   8 💤 ±0  1 ❌ +1 
1 350 runs  ±0  1 324 ✅  - 1  25 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 1b76b15. ± Comparison against base commit e59ede5.

♻️ This comment has been updated with latest results.

[laeubi]

Any chance you can enhance the existing integration test to show the behavior?

[thahnen]

@laeubi which one is the corresponding one?

I was thinking about creating a new one with a web server redirecting to itself to mimic this. But I haven't finished yet reading the documentation of Jetty ^^

[laeubi]

If you search for HttpServer references in the test you should find some test that already do something in that direction.

[thahnen]

Hi @laeubi, sorry for the late response. I will take some time later today to create an IT or to update a fitting existing one.

[thahnen]

Sadly, for whatever reason I wasn't able to run the ITs from within Eclipse itself due to the launch configuration tycho-its - prepare test resources failing with an exception from both the launcher as well as when running the test from the command line mvn clean verify -f tycho-its/pom.xml -Dtest=P2RedirectLoopTest:
prepare_test_resources.log

I have to await the CI and then wait and see.

[laeubi]

In eclipse you can simpel right click and choose to run as unit test. But due to the nature of tycho/maven it is usually better to use maven CLI with remote debugging.

For this simply do

• mvn clean install -T1C -DskipTests
• cd tycho-its
• mvn clean install -Pits -Dtest=<full class name of test>

[thahnen]

Hey @laeubi,

I have been trying for a few days now to create an IT for this behavior but with no success. Taking a look at all P2-related ITs, I could not really find a way to hook into one, so I created one on my own.

But even having a Jetty where the p2.index was reachable and linked to a compositeContent.xml and compositeArtifacts.xml on another server that would create the redirection loop did not work.
My initial idea of just having a target platform linking to a server that would redirect to itself for every file would not work because it failed earlier by not finding a p2 metadata repository 😞

[laeubi]

@thahnen can you maybe share what "did not work"? Maybe just upload the test and explain what you wanted to behave differently and we can probably find a solution.

[thahnen]

Good idea! Currently OOF, will do so once I‘m back again.

[laeubi]

@thahnen there will be anew Tycho release probably end of Feb do you like to complete this so we can include the fix?

[thahnen]

Hey @laeubi, I sadly found no time lately. I will put this back on my TODO list to complete this. Thanks for the heads up!

[laeubi]

@thahnen as a gentle reminder we prepare for a final Tycho 4 release so if you want to see this feature there it would be good to finish this soon. Otherwise it could go into of course in the 5.x release without any problem.

[thahnen]

Hey @laeubi, thanks for the reminder, I did not find the time to pick it up yet. But for the general implementation without reproducer tests I could do so today - I struggled the last time again heavely with trying to create a reproducer so I'm most certain that I will not do that here and stick with the implementation I currently have. Will resolve the merge conflicts tho.