Skip to content

Fix #11076: Use bootstrapped scalaInstance for community build #11082

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

Closed
wants to merge 1 commit into from

Conversation

griggt
Copy link
Contributor

@griggt griggt commented Jan 13, 2021

This PR seems to fix the issue, however it is not at all clear to me whether there are better/alternative solutions.

Fixes #11076

@griggt griggt marked this pull request as ready for review January 13, 2021 06:49
@nicolasstucki
Copy link
Contributor

I double-checked that the fix works for the case I encountered. It looks good. Not sure if there is a better alternative. @smarter WDYT?

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

The problem is that the community-build project uses a macro: https://github.com/lampepfl/dotty/blob/master/community-build/src/scala/dotty/communitybuild/FieldsImpl.scala, macros require classloading which means the version of the standard library in the scalaInstance must match the one used in the project (so bootstrapped in both case), which in turns means that all bootstrapped projects have to be compiled. This is fine for the community-build since we need the bootstrapped projects compiled anyway to run it, so we can make the change proposed in this PR but it would need to be documented better (and ideally it should avoid duplicating code and comment with the other scalaInstance). Alternatively, we could get rid of the macro in the community-build project: IMO it's really overkill to use a macro here, the list of project could be built by adding entries into a mutable set for example.

@nicolasstucki
Copy link
Contributor

We should get rid of the macro and just list them. We already list those in other parts of the code.

@nicolasstucki
Copy link
Contributor

I added a commit to remove the macro in #11074

@griggt griggt closed this Jan 15, 2021
@griggt griggt deleted the fix-#11076 branch March 17, 2021 00:16
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.

community-docs CI is using the non-bootstrapped library
3 participants