Skip to content

bugfix - apply autoImports for global plugins at global configuration stage#2399

Merged
dwijnand merged 3 commits intosbt:0.13from
timcharper:0.13
Jan 19, 2016
Merged

bugfix - apply autoImports for global plugins at global configuration stage#2399
dwijnand merged 3 commits intosbt:0.13from
timcharper:0.13

Conversation

@timcharper
Copy link
Contributor

No description provided.

@eed3si9n eed3si9n added the ready label Jan 18, 2016
@typesafe-tools
Copy link

Can one of the admins verify this patch?

@timcharper timcharper changed the title Amend contributing build instructions to help solve potential build problems bugfix - apply autoImports for global plugins at global configuration stage Jan 18, 2016
@timcharper
Copy link
Contributor Author

I've confirmed that this change resolves issue #2120 as I've experienced it. I'm not deeply familiar with the sbt code base, and am not sure what kind of consequences my change might have.

One consequence is that it seems that it will scan for autoPlugins twice; not sure how much more expensive that will be.

I don't know what the consequence of not passing an UpdateReport, or Resolvers. None seem to be available at the buildGlobalSettings stage of initialization.

@eed3si9n
Copy link
Member

Could you add your test in here?
https://github.com/sbt/sbt/tree/0.13/sbt/src/sbt-test/project/global-plugin

See http://eed3si9n.com/testing-sbt-plugins for the explanation on scripted.

@timcharper
Copy link
Contributor Author

Yeah, I'll have to work on that later. I didn't want to try and add tests until at least my approach was validated by somebody who knows what they're doing.

Copy link
Member

Choose a reason for hiding this comment

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

Why not cache this in LoadBuildConfiguration as a lazy val?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think I can, since PluginDiscovery requires a ClassLoader, and I don't think a ClassLoader is available in LoadBuildConfiguration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, hey, there is one there! I'll update the patch.

Like I said in the chat room... I don't know what I'm doing and just threw up a patch to get some validation. Please don't expect this to be me saying "here's a finished patch, please merge it right now". :)

@eed3si9n
Copy link
Member

Plz add release note to https://github.com/sbt/sbt/tree/0.13/notes/0.13.10

@timcharper
Copy link
Contributor Author

added release note and moved detectedPlugin related code to LoadBuildConfiguration. Tests still remaining.

@timcharper
Copy link
Contributor Author

I've defined an AutoPlugin for the project/global-plugin scripted test, but I'm unable to define a configuration block that is loaded by loadGlobalSettings.

☝️ January 18, 2016 2:37 AM

@eed3si9n
Copy link
Member

Here's what I have locally:

global-plugin/global/g.sbt

serverName := "x.lan"

global-plugin/global/plugins/A.scala

package test

import sbt._
import Keys._

object Global extends sbt.AutoPlugin {
  override def trigger = allRequirements
  override def requires = plugins.JvmPlugin

  val x = 3
  object autoImport {
    val serverName = settingKey[String]("")
  }
}

result

[info] [info] Compiling 1 Scala source to /private/var/folders/7g/l96y5q310h90xsz109qqjpmc0000gn/T/sbt_a1dedef7/global-plugin/global/plugins/target/scala-2.10/sbt-0.13/classes...
[info] java.lang.ClassNotFoundException: test.Global$
[info]  at java.net.URLClassLoader$1.run(URLClassLoader.java:202)
[info]  at java.security.AccessController.doPrivileged(Native Method)
[info]  at java.net.URLClassLoader.findClass(URLClassLoader.java:190)
[info]  at java.lang.ClassLoader.loadClass(ClassLoader.java:306)
[info]  at java.lang.ClassLoader.loadClass(ClassLoader.java:247)
[info]  at java.lang.Class.forName0(Native Method)
[info]  at java.lang.Class.forName(Class.java:249)
[info]  at sbt.ModuleUtilities$.getObject(ModuleUtilities.scala:14)
[info]  at sbt.ModuleUtilities$.getCheckedObject(ModuleUtilities.scala:20)
[info]  at sbt.ModuleUtilities$$anonfun$getCheckedObjects$1.apply(ModuleUtilities.scala:23)
[info]  at sbt.ModuleUtilities$$anonfun$getCheckedObjects$1.apply(ModuleUtilities.scala:23)
[info]  at scala.collection.immutable.Stream.map(Stream.scala:376)
[info]  at sbt.ModuleUtilities$.getCheckedObjects(ModuleUtilities.scala:23)
[info]  at sbt.PluginDiscovery$.loadModules(PluginDiscovery.scala:129)
[info]  at sbt.PluginDiscovery$.binarySourceModules(PluginDiscovery.scala:123)
[info]  at sbt.PluginDiscovery$.discover$1(PluginDiscovery.scala:28)
[info]  at sbt.PluginDiscovery$.discoverAll(PluginDiscovery.scala:37)
[info]  at sbt.LoadBuildConfiguration.pluginDiscovery$lzycompute(Load.scala:968)
[info]  at sbt.LoadBuildConfiguration.pluginDiscovery(Load.scala:968)
[info]  at sbt.LoadBuildConfiguration.detectedPlugins$lzycompute(Load.scala:970)
[info]  at sbt.LoadBuildConfiguration.detectedPlugins(Load.scala:969)
[info]  at sbt.Load$.buildGlobalSettings(Load.scala:80)
[info]  at sbt.Load$.loadGlobalSettings(Load.scala:71)
[info]  at sbt.Load$.defaultWithGlobal(Load.scala:65)
[info]  at sbt.Load$.defaultLoad(Load.scala:34)
[info]  at sbt.BuiltinCommands$.liftedTree1$1(Main.scala:492)
[info]  at sbt.BuiltinCommands$.doLoadProject(Main.scala:492)
[info]  at sbt.BuiltinCommands$$anonfun$loadProjectImpl$2.apply(Main.scala:484)
[info]  at sbt.BuiltinCommands$$anonfun$loadProjectImpl$2.apply(Main.scala:484)
[info]  at sbt.Command$$anonfun$applyEffect$1$$anonfun$apply$2.apply(Command.scala:59)
[info]  at sbt.Command$$anonfun$applyEffect$1$$anonfun$apply$2.apply(Command.scala:59)
[info]  at sbt.Command$$anonfun$applyEffect$2$$anonfun$apply$3.apply(Command.scala:61)
[info]  at sbt.Command$$anonfun$applyEffect$2$$anonfun$apply$3.apply(Command.scala:61)
[info]  at sbt.Command$.process(Command.scala:93)
[info]  at sbt.MainLoop$$anonfun$1$$anonfun$apply$1.apply(MainLoop.scala:96)
[info]  at sbt.MainLoop$$anonfun$1$$anonfun$apply$1.apply(MainLoop.scala:96)
[info]  at sbt.State$$anon$1.process(State.scala:184)
[info]  at sbt.MainLoop$$anonfun$1.apply(MainLoop.scala:96)
[info]  at sbt.MainLoop$$anonfun$1.apply(MainLoop.scala:96)
[info]  at sbt.ErrorHandling$.wideConvert(ErrorHandling.scala:17)
[info]  at sbt.MainLoop$.next(MainLoop.scala:96)
[info]  at sbt.MainLoop$.run(MainLoop.scala:89)
[info]  at sbt.MainLoop$$anonfun$runWithNewLog$1.apply(MainLoop.scala:68)
[info]  at sbt.MainLoop$$anonfun$runWithNewLog$1.apply(MainLoop.scala:63)
[info]  at sbt.Using.apply(Using.scala:24)
[info]  at sbt.MainLoop$.runWithNewLog(MainLoop.scala:63)
[info]  at sbt.MainLoop$.runAndClearLast(MainLoop.scala:46)
[info]  at sbt.MainLoop$.runLoggedLoop(MainLoop.scala:30)
[info]  at sbt.MainLoop$.runLogged(MainLoop.scala:22)
[info]  at sbt.StandardMain$.runManaged(Main.scala:54)
[info]  at sbt.xMain.run(Main.scala:29)
[info]  at xsbt.boot.Launch$$anonfun$run$1.apply(Launch.scala:109)
[info]  at xsbt.boot.Launch$.withContextLoader(Launch.scala:128)
[info]  at xsbt.boot.Launch$.run(Launch.scala:109)
[info]  at xsbt.boot.Launch$$anonfun$apply$1.apply(Launch.scala:35)
[info]  at xsbt.boot.Launch$.launch(Launch.scala:117)
[info]  at xsbt.boot.Launch$.apply(Launch.scala:18)
[info]  at xsbt.boot.Boot$.runImpl(Boot.scala:41)
[info]  at xsbt.boot.Boot$.main(Boot.scala:17)
[info]  at xsbt.boot.Boot.main(Boot.scala)
[info] [error] java.lang.ClassNotFoundException: test.Global$
[info] [error] Use 'last' for the full log.
[info] Project loading failed: (r)etry, (q)uit, (l)ast, or (i)gnore?
[error] x project / global-plugin
[error]    {line 2}  Command failed: Remote sbt initialization failed
[trace] Stack trace suppressed: run last sbtRoot/*:scripted for the full output.
[error] (sbtRoot/*:scripted) Failed tests:
[error]     project / global-plugin

@timcharper
Copy link
Contributor Author

I'm trying that exact same thing and it doesn't seem to work. I'll try again. Are you running the test with scripted project/global-plugin ?

@eed3si9n
Copy link
Member

scripted project/global-plugin

Yes.

@eed3si9n
Copy link
Member

Here's how my LoadBuildConfiguration looks like:

final case class LoadBuildConfiguration(
    stagingDirectory: File,
    classpath: Seq[Attributed[File]],
    loader: ClassLoader,
    compilers: Compilers,
    evalPluginDef: (sbt.BuildStructure, State) => PluginData,
    definesClass: DefinesClass,
    delegates: sbt.LoadedBuild => Scope => Seq[Scope],
    scopeLocal: ScopeLocal,
    pluginManagement: PluginManagement,
    injectSettings: Load.InjectSettings,
    globalPlugin: Option[GlobalPlugin],
    extraBuilds: Seq[URI],
    log: Logger) {
  lazy val (globalPluginClasspath, globalPluginLoader) = Load.pluginDefinitionLoader(this, Load.globalPluginClasspath(globalPlugin))
  lazy val globalPluginNames = if (globalPluginClasspath.isEmpty) Nil else Load.getPluginNames(globalPluginClasspath, globalPluginLoader)
  private[sbt] lazy val globalPluginDefs = {
    val pluginData = globalPlugin match {
      case Some(x) => PluginData(x.data.fullClasspath, x.data.internalClasspath, Some(x.data.resolvers), Some(x.data.updateReport), Nil)
      case None    => PluginData(globalPluginClasspath, Nil, None, None, Nil)
    }
    val baseDir = globalPlugin match {
      case Some(x) => x.base
      case _       => stagingDirectory
    }
    Load.loadPluginDefinition(baseDir, this, pluginData)
  }
  lazy val detectedGlobalPlugins = globalPluginDefs.detected
}

Copy link
Member

Choose a reason for hiding this comment

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

Do we still need this?

@timcharper timcharper force-pushed the 0.13 branch 4 times, most recently from b132387 to ade8962 Compare January 18, 2016 21:48
@timcharper
Copy link
Contributor Author

I've added integration tests to assert that both legacy-plugin-defined-keys and auto-plugin autoImports are available for global configuration files.

This patch should be ready to go. Everything works in usage and in the scripted project/global-plugin.

Copy link
Member

Choose a reason for hiding this comment

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

Can you mention #2120 as well. And mention it in the commit ("Fixes #2120") so it auto-closes the ticket.

Tim Harper added 2 commits January 18, 2016 15:02
… stage

Previously, the autoimports for globally defined plugins were not
imported for global configuration files, although they were imported for
project configuration files.

This patch causes an additional plugin discovery phase to happen during
global config evaluation, so that auto-plugins can be detected and their
imports subsequently included.
@eed3si9n
Copy link
Member

LGTM pending Travis.
Also please forward port this PR to 0.1.x branch (cherry pick and open another pull request).

@dwijnand
Copy link
Member

Same 👍 Thanks @timcharper

and yeah please pull request this against 1.0.x as well.

@timcharper
Copy link
Contributor Author

Sure!

I'll do so once the tests pass and are merged into 0.13. That okay?

@dwijnand
Copy link
Member

Sure.

@timcharper
Copy link
Contributor Author

It seems like the failure in travis-ci is unrelated to my change:

https://travis-ci.org/sbt/sbt/jobs/103215626

[info] Could not create file /tmp/sbt_b2c09a85/info/target/streams/$global/ivyConfiguration/$global/streams/outjava.io.IOException: No such file or directory
[info]  at sbt.ErrorHandling$.translate(ErrorHandling.scala:10)
[info]  at sbt.IO$.touch(IO.scala:196)
[info]  at sbt.std.Streams$$anon$3$$anon$2.make(Streams.scala:129)
[info]  at sbt.std.Streams$$anon$3$$anon$2.text(Streams.scala:113)
[info]  at sbt.std.Streams$$anon$3$$anon$2.log(Streams.scala:124)
[info]  at sbt.std.TaskStreams$class.log(Streams.scala:56)
[info]  at sbt.std.Streams$$anon$3$$anon$2.log$lzycompute(Streams.scala:102)
[info]  at sbt.std.Streams$$anon$3$$anon$2.log(Streams.scala:102)
[info]  at sbt.Classpaths$$anonfun$mkIvyConfiguration$1.apply(Defaults.scala:1644)
[info]  at sbt.Classpaths$$anonfun$mkIvyConfiguration$1.apply(Defaults.scala:1643)
[info]  at scala.Function10$$anonfun$tupled$1.apply(Function10.scala:35)
[info]  at scala.Function10$$anonfun$tupled$1.apply(Function10.scala:34)
[info]  at scala.Function1$$anonfun$compose$1.apply(Function1.scala:47)
[info]  at sbt.$tilde$greater$$anonfun$$u2219$1.apply(TypeFunctions.scala:40)
[info]  at sbt.std.Transform$$anon$4.work(System.scala:63)
[info]  at sbt.Execute$$anonfun$submit$1$$anonfun$apply$1.apply(Execute.scala:228)
[info]  at sbt.Execute$$anonfun$submit$1$$anonfun$apply$1.apply(Execute.scala:228)
[info]  at sbt.ErrorHandling$.wideConvert(ErrorHandling.scala:17)
[info]  at sbt.Execute.work(Execute.scala:237)
[info]  at sbt.Execute$$anonfun$submit$1.apply(Execute.scala:228)
[info]  at sbt.Execute$$anonfun$submit$1.apply(Execute.scala:228)
[info]  at sbt.ConcurrentRestrictions$$anon$4$$anonfun$1.apply(ConcurrentRestrictions.scala:159)
[info]  at sbt.CompletionService$$anon$2.call(CompletionService.scala:28)
[info]  at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
[info]  at java.util.concurrent.FutureTask.run(FutureTask.java:166)
[info]  at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
[info]  at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
[info]  at java.util.concurrent.FutureTask.run(FutureTask.java:166)
[info]  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1146)
[info]  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
[info]  at java.lang.Thread.run(Thread.java:701)
[info] Caused by: java.io.IOException: No such file or directory
[info]  at java.io.UnixFileSystem.createFileExclusively(Native Method)
[info]  at java.io.File.createNewFile(File.java:959)
[info]  at sbt.IO$$anonfun$1.apply$mcZ$sp(IO.scala:196)
[info]  at sbt.IO$$anonfun$1.apply(IO.scala:196)
[info]  at sbt.IO$$anonfun$1.apply(IO.scala:196)
[info]  at sbt.ErrorHandling$.translate(ErrorHandling.scala:10)
[info]  at sbt.IO$.touch(IO.scala:196)
[info]  at sbt.std.Streams$$anon$3$$anon$2.make(Streams.scala:129)
[info]  at sbt.std.Streams$$anon$3$$anon$2.text(Streams.scala:113)
[info]  at sbt.std.Streams$$anon$3$$anon$2.log(Streams.scala:124)
[info]  at sbt.std.TaskStreams$class.log(Streams.scala:56)
[info]  at sbt.std.Streams$$anon$3$$anon$2.log$lzycompute(Streams.scala:102)
[info]  at sbt.std.Streams$$anon$3$$anon$2.log(Streams.scala:102)
[info]  at sbt.Classpaths$$anonfun$mkIvyConfiguration$1.apply(Defaults.scala:1644)
[info]  at sbt.Classpaths$$anonfun$mkIvyConfiguration$1.apply(Defaults.scala:1643)
[info]  at scala.Function10$$anonfun$tupled$1.apply(Function10.scala:35)
[info]  at scala.Function10$$anonfun$tupled$1.apply(Function10.scala:34)
[info]  at scala.Function1$$anonfun$compose$1.apply(Function1.scala:47)
[info]  at sbt.$tilde$greater$$anonfun$$u2219$1.apply(TypeFunctions.scala:40)
[info]  at sbt.std.Transform$$anon$4.work(System.scala:63)
[info]  at sbt.Execute$$anonfun$submit$1$$anonfun$apply$1.apply(Execute.scala:228)
[info]  at sbt.Execute$$anonfun$submit$1$$anonfun$apply$1.apply(Execute.scala:228)
[info]  at sbt.ErrorHandling$.wideConvert(ErrorHandling.scala:17)
[info]  at sbt.Execute.work(Execute.scala:237)
[info]  at sbt.Execute$$anonfun$submit$1.apply(Execute.scala:228)
[info]  at sbt.Execute$$anonfun$submit$1.apply(Execute.scala:228)
[info]  at sbt.ConcurrentRestrictions$$anon$4$$anonfun$1.apply(ConcurrentRestrictions.scala:159)
[info]  at sbt.CompletionService$$anon$2.call(CompletionService.scala:28)
[info]  at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
[info]  at java.util.concurrent.FutureTask.run(FutureTask.java:166)
[info]  at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
[info]  at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
[info]  at java.util.concurrent.FutureTask.run(FutureTask.java:166)
[info]  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1146)
[info]  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
[info]  at java.lang.Thread.run(Thread.java:701)
[info] [error] (*:ivyConfiguration) Could not create file /tmp/sbt_b2c09a85/info/target/streams/$global/ivyConfiguration/$global/streams/outjava.io.IOException: No such file or directory
[info] [error] Total time: 0 s, completed Jan 19, 2016 2:34:17 AM
[error] x dependency-management / info
[error]    {line 4}  Command failed: clean failed

@eed3si9n
Copy link
Member

Job restarted.

@timcharper
Copy link
Contributor Author

It seems like 0.13 has a bunch of stuff that is not merged into 1.0.x, so I'm interpetting "send a pull request" with these changes you mean cherry pick them on to 1.0.x

@eed3si9n
Copy link
Member

It seems like 0.13 has a bunch of stuff that is not merged into 1.0.x, so I presume by "send a pull request" with these patches you mean cherry pick them on to 1.0.x

Yes.

dwijnand added a commit that referenced this pull request Jan 19, 2016
bugfix - apply autoImports for global plugins at global configuration stage
@dwijnand dwijnand merged commit 35aed2b into sbt:0.13 Jan 19, 2016
@dwijnand dwijnand removed the ready label Jan 19, 2016
dwijnand added a commit that referenced this pull request Jan 19, 2016
Forward port pull request #2399 to 1.0.x
@eed3si9n eed3si9n added this to the 0.13.10 milestone Jan 21, 2016
@eed3si9n
Copy link
Member

https://gitter.im/sbt/sbt?at=56a3c957586242210adf1d87

is anybody seeing this with 0.13.10-RC1?

 /home/fommil/.sbt/0.13/global.sbt:8: error: reference to useGpgAgent is ambiguous;
it is imported twice in the same scope by
import com.typesafe.sbt.pgp.PgpKeys._
and import _root_.com.typesafe.sbt.SbtPgp.autoImport._
useGpgAgent := true

is there now an implicit dependency on the pgp plugin?

Us importing autoImport._ might break source compatibility.

"not doing a wildcard import" is apparently a workaround, but this is a sticky situation because global settings are used across all 0.13 builds.

eed3si9n added a commit to eed3si9n/sbt that referenced this pull request Jan 25, 2016
We are hiding a bug fix on global setting that was not importing auto
imports because fixing this via sbt#2399 breaks the source
compatibility: sbt#2415
I've split out the relevant portion of the scripted test into
global-settings, and marked it pending.
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.

4 participants