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

Fix #525: Spuriously duplicate options #536

Merged
merged 8 commits into from
Jan 11, 2022

Conversation

jchyb
Copy link
Contributor

@jchyb jchyb commented Jan 10, 2022

This PR fixes the issue and adds a unit test based on the original issue snippet. It is worth noting that I decided to fix only the spuriously duplicate options - users still can duplicate them on will f.e. by providing multiple of the same using directives in different files or by adding an already defined option via cli. On that occasion they will most likely still receive the same warning.

I did not want to limit users in defining duplicate options as that would require a more strict control of a scalac api, f.e. perhaps one day scalac will have an option that will be able to be defined multiple times (like our verbose) and then our hypothetical assumption would become incorrect.

@jchyb jchyb linked an issue Jan 10, 2022 that may be closed by this pull request
@alexarchambault
Copy link
Contributor

alexarchambault commented Jan 10, 2022

Just tried something slightly different in the commits I just pushed, as the CI errors looked legit… Feel free to revert them, if ever my approach doesn't pan out well.

@alexarchambault
Copy link
Contributor

The idea of my approach is that CrossSources basically splits its buildOptions: Seq[HasBuildRequirements[BuildOptions]] field in two: some options go in def sharedOptions, the others go into the ScopedSources returned by def scopedSources. My changes make sure options go only in one of those, not both (as was the case before it seems).

@jchyb
Copy link
Contributor Author

jchyb commented Jan 11, 2022

Thank you for the fixes, I must have messed something up with the scopes before. I'll be more careful next time

@alexarchambault
Copy link
Contributor

Thank you for the fixes, I must have messed something up with the scopes before. I'll be more careful next time

No problem, that's a super hairy area of the code base. As the commit history shows, I definitely didn't address that in the first try 😅

@alexarchambault alexarchambault merged commit 5644277 into VirtusLab:master Jan 11, 2022
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.

Setting scalacOptions twice causing warnings
3 participants