Skip to content

Conversation

@smarter
Copy link
Contributor

@smarter smarter commented Aug 28, 2018

This PR requires a release of sbt util with sbt/util#175 merged.

After having a look at https://discuss.lightbend.com/t/patching-strategy/1924 I have no idea whether I'm supposed to open things against 1.2.x or 1.x, so I'm opening against 1.2.x since that's what I care about right now.

@smarter smarter requested review from eed3si9n and jvican August 28, 2018 16:07
@typesafe-tools
Copy link

A validation involving this pull request is in progress...

@typesafe-tools
Copy link

The validator has checked the following projects against Scala 2.12,
tested using dbuild, projects built on top of each other.

Project Reference Commit
sbt 1.2.x sbt/sbt@6eb3d9f
zinc pull/588/head 62d15ad
io 1.2.x sbt/io@fb0acb1
librarymanagement 1.2.x sbt/librarymanagement@f42ec6f
util pull/175/head sbt/util@15522a0
website 1.2.x

❌ The result is: FAILED
(restart)

@smarter
Copy link
Contributor Author

smarter commented Aug 28, 2018

The validator is still broken:

[scala-212] --== End Extracting dependencies for scala-212 ==--
[error] java.lang.RuntimeException: 
[error] 
[error] Fatal: multiple projects have the same artifacts visible in the same space.
[error] 
[error]   org.scala-sbt#zinc-classpath  from sbt212-zinc and sbt212-zinc, both visible in space "sbt212"
[error]   org.scala-sbt#zinc-apiinfo  from sbt212-zinc and sbt212-zinc, both visible in space "sbt212"
[error]   org.scala-sbt#zinc-classfile  from sbt212-zinc and sbt212-zinc, both visible in space "sbt212"
[error]   org.scala-sbt#compiler-bridge  from sbt212-zinc and sbt212-zinc, both visible in space "sbt212"
[error] 
[error] 	at scala.sys.package$.error(package.scala:27)
[error] 	at com.typesafe.dbuild.model.RepeatableDBuildConfig.<init>(RepeatableBuild.scala:158)
[error] 	at com.typesafe.dbuild.model.RepeatableDBuildConfigH$.fromExtractionOutcome(RepeatableBuild.scala:71)
Shutting down, please wait...
Result: EXTRACTION FAILED (Cause: RuntimeException: )

Copy link
Member

@jvican jvican left a comment

Choose a reason for hiding this comment

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

👍 on the addition of rendered, aside from the comment this LGTM

import sbinary.Format
import xsbti.{ Position, Problem, Severity }

// TODO: Remove this method once a version of sbinary containing
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to update the text analysis format since it's not supported anymore and it's not used by any build tool that I know of (and if somebody is using it, it's fine that they don't see the new additions to positions). We should probably remove this from this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You don't need to update the text analysis format since it's not supported anymore

Then why isn't it deprecated? If it was, I wouldn't have wasted a day implementing this.

We should probably remove this from this PR

At this point I would just keep it since it avoids some deprecation warnings in the build of Zinc.

}
}

private def fromString(value: String): Option[String] =
Copy link
Member

Choose a reason for hiding this comment

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

Is indentation off here compared to fromInt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean? They're both indented the same way.

Copy link
Member

Choose a reason for hiding this comment

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

Safari plays tricks on me 😛

This was forgotten in sbt#580. Also bump sbinary to 0.5.0 to make
`asProduct13` from sbt/sbinary#17 available.
@smarter smarter merged commit 335878c into sbt:1.2.x Aug 30, 2018
smarter added a commit to dotty-staging/dotty that referenced this pull request Sep 14, 2018
Dotty has its own logic for displaying problems with the proper file
path, position, and caret, but if we store this information in
Problem#message we end up with duplicated information in the output
since Zinc will prepend/append similar things (see
sbt.internal.inc.ProblemStringFormats). So far, we worked around this in
Dotty by using an empty position in the sbt bridge reporter, but this
means that crucial semantic information that could be used by a Build
Server Protocol implementation and other tools is lost.

Thanks to sbt/zinc#588 we can now fully
customize how the message is displayed to the user using
Problem#rendered, so we can now store the position information without
any adverse effect.
smarter added a commit to dotty-staging/dotty that referenced this pull request Sep 14, 2018
Dotty has its own logic for displaying problems with the proper file
path, position, and caret, but if we store this information in
Problem#message we end up with duplicated information in the output
since Zinc will prepend/append similar things (see
sbt.internal.inc.ProblemStringFormats). So far, we worked around this in
Dotty by using an empty position in the sbt bridge reporter, but this
means that crucial semantic information that could be used by a Build
Server Protocol implementation and other tools is lost.

Thanks to sbt/zinc#588 we can now fully
customize how the message is displayed to the user using
Problem#rendered, so we can now store the position information without
any adverse effect.
smarter added a commit to dotty-staging/dotty that referenced this pull request Sep 14, 2018
Dotty has its own logic for displaying problems with the proper file
path, position, and caret, but if we store this information in
Problem#message we end up with duplicated information in the output
since Zinc will prepend/append similar things (see
sbt.internal.inc.ProblemStringFormats). So far, we worked around this in
Dotty by using an empty position in the sbt bridge reporter, but this
means that crucial semantic information that could be used by a Build
Server Protocol implementation and other tools is lost.

Thanks to sbt/zinc#588 we can now fully
customize how the message is displayed to the user using
Problem#rendered, so we can now store the position information without
any adverse effect.
smarter added a commit to dotty-staging/dotty that referenced this pull request Sep 14, 2018
Dotty has its own logic for displaying problems with the proper file
path, position, and caret, but if we store this information in
Problem#message we end up with duplicated information in the output
since Zinc will prepend/append similar things (see
sbt.internal.inc.ProblemStringFormats). So far, we worked around this in
Dotty by using an empty position in the sbt bridge reporter, but this
means that crucial semantic information that could be used by a Build
Server Protocol implementation and other tools is lost.

Thanks to sbt/zinc#588 we can now fully
customize how the message is displayed to the user using
Problem#rendered, so we can now store the position information without
any adverse effect.
smarter added a commit to dotty-staging/dotty that referenced this pull request Sep 14, 2018
Dotty has its own logic for displaying problems with the proper file
path, position, and caret, but if we store this information in
Problem#message we end up with duplicated information in the output
since Zinc will prepend/append similar things (see
sbt.internal.inc.ProblemStringFormats). So far, we worked around this in
Dotty by using an empty position in the sbt bridge reporter, but this
means that crucial semantic information that could be used by a Build
Server Protocol implementation and other tools is lost.

Thanks to sbt/zinc#588 we can now fully
customize how the message is displayed to the user using
Problem#rendered, so we can now store the position information without
any adverse effect.
smarter added a commit to dotty-staging/dotty that referenced this pull request Sep 14, 2018
Dotty has its own logic for displaying problems with the proper file
path, position, and caret, but if we store this information in
Problem#message we end up with duplicated information in the output
since Zinc will prepend/append similar things (see
sbt.internal.inc.ProblemStringFormats). So far, we worked around this in
Dotty by using an empty position in the sbt bridge reporter, but this
means that crucial semantic information that could be used by a Build
Server Protocol implementation and other tools is lost.

Thanks to sbt/zinc#588 we can now fully
customize how the message is displayed to the user using
Problem#rendered, so we can now store the position information without
any adverse effect.
smarter added a commit to dotty-staging/dotty that referenced this pull request Sep 14, 2018
Dotty has its own logic for displaying problems with the proper file
path, position, and caret, but if we store this information in
Problem#message we end up with duplicated information in the output
since Zinc will prepend/append similar things (see
sbt.internal.inc.ProblemStringFormats). So far, we worked around this in
Dotty by using an empty position in the sbt bridge reporter, but this
means that crucial semantic information that could be used by a Build
Server Protocol implementation and other tools is lost.

Thanks to sbt/zinc#588 we can now fully
customize how the message is displayed to the user using
Problem#rendered, so we can now store the position information without
any adverse effect.
smarter added a commit to dotty-staging/dotty that referenced this pull request Sep 14, 2018
Dotty has its own logic for displaying problems with the proper file
path, position, and caret, but if we store this information in
Problem#message we end up with duplicated information in the output
since Zinc will prepend/append similar things (see
sbt.internal.inc.ProblemStringFormats). So far, we worked around this in
Dotty by using an empty position in the sbt bridge reporter, but this
means that crucial semantic information that could be used by a Build
Server Protocol implementation and other tools is lost.

Thanks to sbt/zinc#588 we can now fully
customize how the message is displayed to the user using
Problem#rendered, so we can now store the position information without
any adverse effect.
smarter added a commit to dotty-staging/dotty that referenced this pull request Sep 14, 2018
Dotty has its own logic for displaying problems with the proper file
path, position, and caret, but if we store this information in
Problem#message we end up with duplicated information in the output
since Zinc will prepend/append similar things (see
sbt.internal.inc.ProblemStringFormats). So far, we worked around this in
Dotty by using an empty position in the sbt bridge reporter, but this
means that crucial semantic information that could be used by a Build
Server Protocol implementation and other tools is lost.

Thanks to sbt/zinc#588 we can now fully
customize how the message is displayed to the user using
Problem#rendered, so we can now store the position information without
any adverse effect.
eed3si9n pushed a commit to eed3si9n/scala that referenced this pull request May 14, 2019
Dotty has its own logic for displaying problems with the proper file
path, position, and caret, but if we store this information in
Problem#message we end up with duplicated information in the output
since Zinc will prepend/append similar things (see
sbt.internal.inc.ProblemStringFormats). So far, we worked around this in
Dotty by using an empty position in the sbt bridge reporter, but this
means that crucial semantic information that could be used by a Build
Server Protocol implementation and other tools is lost.

Thanks to sbt/zinc#588 we can now fully
customize how the message is displayed to the user using
Problem#rendered, so we can now store the position information without
any adverse effect.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants