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

Backport: Follow P2 contract of cached file's extension (#2938) #2944

Merged
merged 1 commit into from
Oct 28, 2023

Conversation

basilevs
Copy link
Contributor

@basilevs basilevs commented Oct 21, 2023

Fixes #2938

This is a backport of #2945

  • Current implementation of CacheManager injected from Tycho into P2 names cached downloads internally based on their URLs
  • When HTTP server performs a redirect, URL and cache file name change to reflect that.
  • Some servers (Github) redirect to URLs that do not follow any particular convention on path or file names.
  • Tycho caches such files with invalid names.
  • The invalid name is then returned from cache layer and confuses P2.
  • P2 relies on correct file extension to parse cached files.
  • Files with invalid names are considered to be XML
  • P2 fails to parse files with invalid names with an error "Content is not allowed in prolog" even if the file content is a valid JAR.

This change ensures that CacheManager injected into P2 adheres to the contract and returns cached files with extensions matching the (original, before redirect) request.

eclipse-equinox/p2#355 discusses fix on P2 side.

Copy link
Member

@laeubi laeubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good thanks for the enhancement!

@laeubi laeubi added the backport-to-tycho-4.0.x Can be added to a PR to trigger an automatic backport of the change label Oct 21, 2023
@github-actions
Copy link

github-actions bot commented Oct 21, 2023

Test Results

   567 files  +  3     567 suites  +3   4h 31m 51s ⏱️ - 28m 0s
   370 tests +  4     363 ✔️ +  4    6 💤 ±0  0 ±0  1 🔥 ±0 
1 110 runs  +12  1 090 ✔️ +12  19 💤 ±0  0 ±0  1 🔥 ±0 

For more details on these errors, see this check.

Results for commit d544578. ± Comparison against base commit 486e6e4.

♻️ This comment has been updated with latest results.

@laeubi laeubi changed the base branch from tycho-4.0.x to master October 21, 2023 17:09
@laeubi laeubi changed the base branch from master to tycho-4.0.x October 21, 2023 17:09
@laeubi
Copy link
Member

laeubi commented Oct 21, 2023

@basilevs can you please target the master branch for this (backport will then be performed automatically), also if you think your done please mark this ready for review.

@basilevs basilevs changed the title Follow P2 contract of cached file's extension (#2938) Backport: Follow P2 contract of cached file's extension (#2938) Oct 21, 2023
@basilevs
Copy link
Contributor Author

@basilevs can you please target the master branch for this (backport will then be performed automatically), also if you think your done please mark this ready for review.

Created #2945 for master. Waiting for test results.

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

laeubi commented Oct 22, 2023

@basilevs build seems fine now, can you rebase and squash your commits?

This is a backport from 5.0.0

P2 relies on correct file extensions to parse cached files.
This change prevents cached file name from changing over HTTP redirect.

See
eclipse-equinox/p2#355

Fix HttpServer concurrency bug.
URLs returned from HttpServer.getAccessedUrls() are now stripped of
context prefix. No callers have used these values until now.
Fix concurrency bug in test utility class HttpServer request logging.
Add verification of request logs to ensure that metadata repository does
indeed unjars fresh, non-cached artifact. Ensure that cached XML file
does not produce false negative in tests.
Refactor tests.
@basilevs basilevs force-pushed the issue_2938_reproducer branch from d4d443d to d544578 Compare October 22, 2023 18:37
@basilevs basilevs marked this pull request as ready for review October 22, 2023 18:40
@laeubi laeubi merged commit 8dc3455 into eclipse-tycho:tycho-4.0.x Oct 28, 2023
4 of 6 checks passed
@basilevs basilevs deleted the issue_2938_reproducer branch October 28, 2023 16:46
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

Successfully merging this pull request may close these issues.

2 participants