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

bugfix: Include semanticdb plugin path in java options processorpath #2072

Merged
merged 12 commits into from Jun 28, 2023

Conversation

gersonsosa
Copy link
Collaborator

For projects that use javac "-processorpath" option append semanticdb plugin path to avoid compilation errors

Fixes:
#2059

For projects that use javac processorpath option append semanticdb
plugin path to avoid compilation errors
@gersonsosa gersonsosa changed the title Include semanticdb plugin path in java options processorpath bugfix: Include semanticdb plugin path in java options processorpath Jun 12, 2023
@tgodzik
Copy link
Contributor

tgodzik commented Jun 15, 2023

Just got back to work this week, I am running the jobs and will take a look at the PR! Sorry for the delay.

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

Thanks for the great work! Added a couple of comments, but let's see if we can make the tests pass 🥇

gersonsosa and others added 3 commits June 18, 2023 07:30
updats sbt to 1.7.2 to match all other projects
removes unnecessary appending "/" before path
Co-authored-by: Tomasz Godzik <tgodzik@users.noreply.github.com>
@gersonsosa gersonsosa requested a review from tgodzik June 18, 2023 05:42
@gersonsosa gersonsosa requested a review from tgodzik June 18, 2023 20:20
@tgodzik
Copy link
Contributor

tgodzik commented Jun 20, 2023

I added you to triage role in the repo, so that I don't have to click approve every time. Let me know if you need help figuring out the tests!

@gersonsosa
Copy link
Collaborator Author

@tgodzik thank you, I think what's missing is adding bloop to the sample project 😅

@tgodzik
Copy link
Contributor

tgodzik commented Jun 21, 2023

I think we are almost there, but the windows tests are failing with:

--P:semanticdb:sourceroot:D:\a\bloop\bloop\frontend\src\test\resources\scala-java-processorpath
+-P:semanticdb:sourceroot:C:\Users\runneradmin\AppData\Local\Temp\bloop-test-workspace14905355409242014016

sourceroot is being set to the original resource directory and not the temporary workspace.

use project path instead of workspace path to assert semanticdb options
@tgodzik
Copy link
Contributor

tgodzik commented Jun 21, 2023

Weirdly enough it seems that on Windows we don't actually run within the created temporary workspace 🤔

I don't see any obvious reason for that, I can take a look on a windows machine tomorrow.

@gersonsosa
Copy link
Collaborator Author

@tgodzik I see, not running in a workspace is the reason for the difference in the test when asserting the workspace path.

I ran the tests on a windows machine, looks like the Compiler is unable to locate the semanticdb plugin on windows
I added some mode debugging and I get this from the compilation results

Compilation failed with results:
Failed(List(ProblemPerPhase(Problem(-1,Error,plug-in not found: semanticdb,,,Optional.empty),None)),None,10732,bloop.Compiler$$anon$3@776647f6)

After some trial and error the issue is that in windows you must use ; instead of : to separate classpath or processorpath entries for javac.
I'll try to make changes to fix it later today.

@gersonsosa
Copy link
Collaborator Author

@tgodzik I'm not sure if the last failure of the pipeline is related to the changes, I would appreciate you to take a look and let me know how to proceed 👍🏼

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

LGTM! The failing test was not related.

@tgodzik tgodzik merged commit 6e81f06 into scalacenter:main Jun 28, 2023
17 checks passed
@gersonsosa gersonsosa deleted the fix-java-options-processorpath branch June 28, 2023 17:20
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.

None yet

2 participants