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

Ensure that the JSP support works with newer dependencies #311

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

merks
Copy link
Contributor

@merks merks commented Sep 14, 2023

@github-actions
Copy link

Test Results

     24 files  ±0       24 suites  ±0   12m 26s ⏱️ + 1m 3s
2 150 tests ±0  2 106 ✔️ ±0  44 💤 ±0  0 ±0 
2 194 runs  ±0  2 150 ✔️ ±0  44 💤 ±0  0 ±0 

Results for commit 6204eed. ± Comparison against base commit 10c3708.

@merks
Copy link
Contributor Author

merks commented Sep 14, 2023

@akurtakov

I'm not sure who else might want to review this? The "continuous-integration/jenkins/pr-head" build passed....

@akurtakov
Copy link
Member

What is the reason to restrict jasper version to [9,10) ? It might be good to keep the current version in the range.

@merks
Copy link
Contributor Author

merks commented Sep 14, 2023

What is the reason to restrict jasper version to [9,10) ? It might be good to keep the current version in the range.

I did that originally the first time I made the changes, but this time around when I did the changes, I noticed that this initializer class is not in the older bundle:

image

It is present in the 8.x version:

image

But I don't see a 7.x version here:

https://repo1.maven.org/maven2/org/mortbay/jasper/apache-jsp/

So I'm not sure when it was first existing...

@merks
Copy link
Contributor Author

merks commented Sep 14, 2023

I suppose it's also possible to wrap the initializer call in a try block that catches LinkageError and then leave the range more open. That would make it harder to notice problems...

@akurtakov
Copy link
Member

That's good enough explanation. If/when you have the whole story with jsps working please merge this one together with the rest so we get less build issues.

@merks
Copy link
Contributor Author

merks commented Sep 14, 2023

These are the outstanding changes. Remove (replace) the references to org.apache.jasper.glassfish here:

image

image

image

image

image

I think I need to commit this PR before I do the above removals...

Locally the help system works without org.apache.jasper.glassfish in the installation:

image

@merks
Copy link
Contributor Author

merks commented Sep 14, 2023

I'm going to proceed with all these changes so that tonight's I-Build will contain all the changes...

@merks merks merged commit 1a51ae9 into eclipse-equinox:master Sep 14, 2023
20 of 23 checks passed
@merks merks deleted the issue-aggr-jsp-1251 branch September 14, 2023 14:41
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