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

Remove obsolete compiler option assignments #3206

Conversation

mehmet-karaman
Copy link
Contributor

This should fix the compilation errors caused by removed CompilerOptions in Jdt.

@cdietrich
Copy link
Contributor

Which is the minimal jdt version
This removal requires
Last time we needed to use reflection

@cdietrich
Copy link
Contributor

@mehmet-karaman can we check if there is more stuff that is marked for removal and is in @iloveeclipse delete queue

@mehmet-karaman
Copy link
Contributor Author

oo .. You're right.. I guess currently it compiles against the latest jdt... but it has to be conform until 2022-03, right?

@mehmet-karaman mehmet-karaman marked this pull request as draft September 25, 2024 15:22
@mehmet-karaman
Copy link
Contributor Author

If the build is green, please don't merge this.

@cdietrich
Copy link
Contributor

Yes. Depending if the team finds time to work on the j17 minimum story

@mehmet-karaman
Copy link
Contributor Author

Are there tests which are running on 2022-03? How we can recognize if we break something on older platform versions?

@cdietrich
Copy link
Contributor

yes the one on ci.eclipse.org/xtext (continuous-integration/jenkins/pr-head)

@cdietrich
Copy link
Contributor

but am not sure how good overage is.

Copy link

github-actions bot commented Sep 25, 2024

Test Results

  6 460 files  +  3 868    6 460 suites  +3 868   3h 9m 4s ⏱️ + 2h 42m 22s
 43 242 tests + 34 497   42 658 ✅ + 33 973    584 💤 +  524   0 ❌ ± 0 
170 304 runs  +137 924  167 936 ✅ +135 803  2 358 💤 +2 111  10 ❌ +10 

Results for commit ffc3b15. ± Comparison against base commit 4e00f55.

♻️ This comment has been updated with latest results.

@LorenzoBettini
Copy link
Contributor

What does that field mean? Is in general only setting the sourceLevel, without the originalSourceLevel enough (even in the past)?

@cdietrich
Copy link
Contributor

@LorenzoBettini
Copy link
Contributor

https://ci.eclipse.org/xtext/job/xtext/job/PR-3206/1/

Ton of test fails

OK, I see.

Another good candidate to be inserted here #3204 ;)

@mehmet-karaman
Copy link
Contributor Author

mehmet-karaman commented Sep 26, 2024

I've contacted @iloveeclipse and Srikant they've said that it is obsolete since version 4.33 .. So for all versions before 4.33 we have to do the reflective workaround. I've also asked if there are any CompilerOptions planned for removal, he said that nothing will be removed for now.

He also added that @cdietrich did similar workarounds for something with JSR .. The same has to be done in this case

@mehmet-karaman
Copy link
Contributor Author

I've found the PR which mentions something with JSR .. I am going to mimik this for the two compiler options..

#3189

@cdietrich
Copy link
Contributor

@mehmet-karaman please note we had to rewrite that one 5 times.
please do the same as its currently on main.
see the

if (INLINE_JSR_BYTECODE != null) {
			try {
				INLINE_JSR_BYTECODE.invoke(this.compilerOptions, true);
			} catch (Throwable e) {
				// ignore
			}
		}

@mehmet-karaman
Copy link
Contributor Author

Just a sidenote:
While checking the occurences, I've seen that JDT marked the following constant as deprecated:
JavaCore.COMPILER_CODEGEN_INLINE_JSR_BYTECODE

Might be we also get problems with that in the future..

@mehmet-karaman mehmet-karaman force-pushed the removeObsolete_CompilerOption_assignments branch from b6fbcae to ffc3b15 Compare September 26, 2024 07:39
@mehmet-karaman mehmet-karaman marked this pull request as ready for review September 26, 2024 09:17
@mehmet-karaman
Copy link
Contributor Author

@cdietrich is this build ok now? Changed the state of this PR from draft to open..

@cdietrich
Copy link
Contributor

Will check when Jenkins is though and I am back from lunch nd meeting

@mehmet-karaman
Copy link
Contributor Author

:) finally all green

@cdietrich cdietrich merged commit 531d040 into eclipse-xtext:main Sep 26, 2024
9 checks passed
@cdietrich
Copy link
Contributor

thx @mehmet-karaman can you rebase the other one

@LorenzoBettini LorenzoBettini mentioned this pull request Sep 26, 2024
2 tasks
@mehmet-karaman mehmet-karaman deleted the removeObsolete_CompilerOption_assignments branch September 26, 2024 11:17
LorenzoBettini added a commit that referenced this pull request Sep 27, 2024
LorenzoBettini added a commit that referenced this pull request Sep 30, 2024
LorenzoBettini added a commit that referenced this pull request Oct 3, 2024
LorenzoBettini added a commit that referenced this pull request Oct 7, 2024
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.

3 participants