Introduce TaskValue to avoid out-of-graph execution by dwijnand · Pull Request #114 · sbt/sbt-buildinfo

dwijnand · 2018-01-29T15:12:21Z

.. also upgrade to sbt 1.1.0 & cleanup.

eed3si9n · 2018-01-30T01:30:51Z

src/sbt-test/sbt-buildinfo/task/build.sbt

+val projInTaskGraph2 = project dependsOn projInTaskGraph1 settings (
+  BuildInfoPlugin.buildInfoDefaultSettings,
+  addBuildInfoToConfig(Test),
+  buildInfoKeys in Test += (fullClasspath in Compile).taskValue


In sbt/sbt#2943 I'm injecting .taskValue as macro when the LHS is Initialize[Seq[Task[...]]].
Would you be able to take advantage of that if buildInfoKeys were SettingKey[Seq[Task[String]], and you unify other miscellaneous build info entries to a Task[...] instance?

I'm not sure - maybe. But how are you going to change the type of buildInfoKeys?

We can just change the type and bump up minor version.

What about any plugin that depends on sbt-buildinfo?

I am personally unaware of any such plugin, and I don't feel obligation to keep bincompat as long as we bump to 0.8.0.

I don't think this is going to work because then this no longer works:

buildInfoKeys := Seq(name, version, scalaVersion, sbtVersion)

which is what the README details.

we're going to have to look into the macro-to-expand-to-taskValue way.

or (and this was the same argument as sbt/sbt#2943) own the fact that TaskKeys and Task values are different types in sbt, and there are different situations for using either.

Perhaps:

buildInfoKeys in Test := BuildInfoKey.fromKeys(name, fullClasspath in Compile)

and

buildInfoKeys in Test ++= BuildInfoKey.fromKeys(name, fullClasspath in Compile)

sbt 1 requires java 8+ where MaxPermSize is no longer present.

These exist to avoid having to explain the concept of the .taskValue method and sbt.Task to users.

Introduce BuildInfoKey.outOfGraphTaskExecution as a fallback.

dwijnand · 2018-02-09T17:57:02Z

@eed3si9n have a look, and let me know what you think now

eed3si9n

overall LGTM
minor comment about outOfGraph..

eed3si9n · 2018-02-12T16:52:07Z

src/main/scala/sbtbuildinfo/package.scala

+    def of(x: Any): BuildInfoKey = macro BuildInfoKeyMacros.ofImpl
+    def ofN(xs: Any*): Seq[BuildInfoKey] = macro BuildInfoKeyMacros.ofNImpl
+
+    def outOfGraphTaskExecution[A](key: TaskKey[A]): Entry[A] = Task(key)


Maybe shorter name like outOfGraph(..)? Also is this tested ?

I went with the more verbose name, to both make the intention explicit and to discourage usage.

It's tested via the identical deprecated implicit conversion. Is that ok? Or do you want me to switch to this one? Or do you want me to add both?

@eed3si9n wdyt?

outOfGraphUnsafe?

When discussing the design of new in task graph execution, in sbt#114, we completely didn't think of backing the existing TaskKey[A] to BuildInfoKey.Entry[A] implicit conversion with the macro! Fixes sbt#117

dwijnand requested a review from eed3si9n January 29, 2018 15:12

dwijnand force-pushed the task-value branch from fbadf45 to 582a0c2 Compare January 29, 2018 15:15

eed3si9n reviewed Jan 30, 2018

View reviewed changes

dwijnand mentioned this pull request Jan 31, 2018

Cleanups #115

Merged

dwijnand force-pushed the task-value branch 2 times, most recently from 596721d to 3205c7d Compare January 31, 2018 16:02

dwijnand added 6 commits February 9, 2018 17:22

Upgrade to sbt 1.1.0

4bae2cc

Remove -XX:MaxPermSize from scriptedLaunchOpts

e9b952e

sbt 1 requires java 8+ where MaxPermSize is no longer present.

Make BuildInfoResult generation Task-based

49f8aa0

Introduce TaskValue to avoid out-of-graph execution

81b66e7

Introduce BuildInfoKey.of/ofN macros

46d7c09

These exist to avoid having to explain the concept of the .taskValue method and sbt.Task to users.

Deprecate the implicit conv from TaskKey to BuildInfoKey

c83a2d3

Introduce BuildInfoKey.outOfGraphTaskExecution as a fallback.

dwijnand force-pushed the task-value branch from 3205c7d to c83a2d3 Compare February 9, 2018 17:56

eed3si9n requested changes Feb 12, 2018

View reviewed changes

Change the name to BuildInfoKey.outOfGraphUnsafe

c090dd5

dwijnand force-pushed the task-value branch from ee6d700 to c090dd5 Compare February 14, 2018 17:28

eed3si9n approved these changes Feb 14, 2018

View reviewed changes

dwijnand merged commit bb40207 into sbt:master Feb 14, 2018

dwijnand deleted the task-value branch February 14, 2018 19:30

This was referenced Feb 23, 2018

Introduce API to serialise execution of tasks #113

Closed

BuildInfoKey.of doesn't work with BuildInfoKey.map #117

Closed

dwijnand mentioned this pull request Feb 23, 2018

Implicit convert TaskKey with the macro! #119

Merged

Conversation

dwijnand commented Jan 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dwijnand Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dwijnand Feb 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dwijnand commented Feb 9, 2018

Uh oh!

eed3si9n left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dwijnand Feb 1, 2018 •

edited

Loading

dwijnand Feb 7, 2018 •

edited

Loading