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

update sbt versions #313

Merged
merged 3 commits into from
Feb 16, 2021
Merged

update sbt versions #313

merged 3 commits into from
Feb 16, 2021

Conversation

larsrh
Copy link
Contributor

@larsrh larsrh commented Feb 16, 2021

Subsumes #304 and #301

@larsrh larsrh mentioned this pull request Feb 16, 2021
@larsrh
Copy link
Contributor Author

larsrh commented Feb 16, 2021

The fact that the tests are failing here is very unsettling 😰

@SethTisue
Copy link
Contributor

Partial investigation:

  • the problem is reproducible locally, it's not some CI-only glitch
  • it's the sbt version bump, not the sbt-dotty bump
  • 2.13 passes, 2.11 and 2.12 fail

@SethTisue
Copy link
Contributor

oh, I found a ticket on this: sbt/sbt#5934

@gabro
Copy link
Member

gabro commented Feb 16, 2021

I admit I've never bothered to upgrade Sbt because I didn't want to get into that issue 😅

Wild guess: something related to escaping of output? It seems that the value names are not being printed

@mzuehlke
Copy link
Contributor

Updates didn't worked since sbt version 1.4.0.
see #213

@SethTisue
Copy link
Contributor

something related to escaping of output? It seems that the value names are not being printed

It's not a printing problem. In MacroCompatScala2.scala, in clueImpl, if I println(text) after val text = ..., it's printing empty strings (on 2.11 and 2.12 only). So the Clue values themselves incorrectly contain empty strings.

@gabro
Copy link
Member

gabro commented Feb 16, 2021

Wow. The fact that this has something to do with sbt is deeply unnerving 😳

I'll take a look when I have a minute

@larsrh
Copy link
Contributor Author

larsrh commented Feb 16, 2021

It looks like -Yrangepos is not passed to the scalacOptions in test. However, I failed to fix it locally. Maybe a useful hint, though.

@gabro
Copy link
Member

gabro commented Feb 16, 2021

@larsrh Nice find, that would explain the differences between 2.11/2.12 and 2.13

@mpilquist
Copy link

It's definitely the -Yrangepos setting. On current head, we see it on testsJVM project when running +show Test/scalacOptions. Doing same with this PR shows -Yrangepos missing. Not sure what changed upstream to cause this but we could patch easily by setting Test / scalacOptions := (Compile / scalacOptions).value

@gabro
Copy link
Member

gabro commented Feb 16, 2021

I can confirm the workaround proposed by @mpilquist works locally.

Copy link
Member

@gabro gabro left a comment

Choose a reason for hiding this comment

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

Code LGTM, just a minor comment 👍

build.sbt Show resolved Hide resolved
Copy link
Member

@gabro gabro left a comment

Choose a reason for hiding this comment

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

🙏 thank you @larsrh and everyone who contributed to this distributed bug hunt!

@gabro gabro merged commit 3aad999 into scalameta:main Feb 16, 2021
This was referenced Feb 16, 2021
@larsrh larsrh deleted the update/sbt branch February 16, 2021 20:38
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.

5 participants