-
Notifications
You must be signed in to change notification settings - Fork 9
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
Updating to sbt 1.x #11
Changes from all commits
dd51cc2
01cfb18
f6208a1
4409bc5
92cecd6
8d71217
daea8d4
c376caa
e1079e0
8de5f0a
97926b3
d9c1ff4
72f330d
e1db434
ed08482
3b6f970
958b3ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
sbt.version = 0.13.16 | ||
sbt.version = 1.0.2 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,7 +128,7 @@ object ReleaseEarly { | |
|
||
/* Sbt bug: `Def.sequential` here produces 'Illegal dynamic reference' when | ||
* used inside `Def.taskDyn`. This is reported upstream, unclear if it can be fixed. */ | ||
private val StableDef = new sbt.TaskSequential {} | ||
private val StableDef = new sbt.internal.TaskSequential {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that the part where this is used could be simplified: Could you explain the logic behind this code? You want to run all tasks from the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code gets a sequence of tasks that may or may not have been modified by users and execute them inside a dynamic task via There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I simplified it just a bit: 7af1257. Couldn't do anything about that strange |
||
|
||
// See https://github.com/dwijnand/sbt-dynver/issues/23. | ||
val isSnapshot: Def.Initialize[Boolean] = Def.setting { | ||
|
@@ -144,18 +144,18 @@ object ReleaseEarly { | |
} else currentPassword | ||
} | ||
|
||
val releaseEarlyPublishTo: Def.Initialize[Option[sbt.Resolver]] = { | ||
Def.setting { | ||
val releaseEarlyPublishTo: Def.Initialize[Task[Option[sbt.Resolver]]] = { | ||
Def.taskDyn { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this a task instead of a dynamic setting? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because in sbt-1.0 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really wonder why. I just went throught the code in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, makes sense. @dwijnand No need for you to intervene! |
||
// It is not necessary to use a dynamic setting here. | ||
val logger = Keys.sLog.value | ||
if (PrivateKeys.releaseEarlyIsSonatype.value) { | ||
if (PrivateKeys.releaseEarlyIsSonatype.value) Def.task { | ||
// Sonatype requires instrumentation of publishTo to work. | ||
// Reference: https://github.com/xerial/sbt-sonatype#buildsbt | ||
val projectVersion = Keys.version.value | ||
if (isOldSnapshot(projectVersion)) | ||
logger.error(Feedback.UnrecognisedPublisher) | ||
Some(sbt.Opts.resolver.sonatypeStaging) | ||
} else (Keys.publishTo in Bintray.bintray).value | ||
} else (Keys.publishTo in Bintray.bintray) | ||
} | ||
} | ||
|
||
|
@@ -229,7 +229,7 @@ object ReleaseEarly { | |
Pgp.PgpKeys.publishSigned | ||
// Else, use the normal hijacked bintray publish task | ||
else Keys.publish | ||
} dependsOn (Bintray.bintrayEnsureLicenses) | ||
} | ||
|
||
private def sonatypeRelease(state: sbt.State): Def.Initialize[Task[Unit]] = { | ||
// sbt-sonatype needs these task to run sequentially :( | ||
|
@@ -239,7 +239,7 @@ object ReleaseEarly { | |
logger.info(Feedback.logReleaseSonatype(projectName)) | ||
// Trick to make sure that 'sonatypeRelease' does not change the name | ||
import Sonatype.{sonatypeRelease => _, sonatypeOpen => _} | ||
val toRun = s";sonatypeOpen $projectName;publishSigned;sonatypeRelease" | ||
val toRun = s";sonatypeOpen $projectName;$projectName/publishSigned;sonatypeRelease" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this was the fix for that CI failure? Another sbt scoping trap... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it was a legit issue... once you go the command way, you need to manually speciify which command you really meant. Glad I found this, it was a real bug. |
||
runCommandAndRemaining(toRun)(state) | ||
() | ||
} | ||
|
@@ -250,7 +250,6 @@ object ReleaseEarly { | |
* Both have to be executed one after the other on and exclusively, | ||
* meaning that concurrency is not accepted. */ | ||
val releaseEarlyClose: Def.Initialize[Task[Unit]] = Def.taskDyn { | ||
val state = Keys.state.value | ||
val logger = Keys.streams.value.log | ||
val projectName = Keys.name.value | ||
if (!PrivateKeys.releaseEarlyIsSonatype.value) { | ||
|
@@ -282,13 +281,10 @@ object ReleaseEarly { | |
val msg = Feedback.skipRelease(Keys.name.value) | ||
Def.task(logger.info(msg)) | ||
} else { | ||
logger.info(Feedback.logReleaseEarly(Keys.name.value)) | ||
val steps = ThisPluginKeys.releaseEarlyProcess.value | ||
// Return task with unit value at the end | ||
val initializedSteps = steps.map(_.toTask) | ||
Def.taskDyn { | ||
logger.info(Feedback.logReleaseEarly(Keys.name.value)) | ||
StableDef.sequential(initializedSteps, Def.task(())) | ||
} | ||
StableDef.sequential(steps.map(_.toTask), Def.task(())) | ||
} | ||
} | ||
|
||
|
@@ -322,8 +318,7 @@ object ReleaseEarly { | |
val releaseEarlyEnableSyncToMaven: Def.Initialize[Boolean] = | ||
Def.setting(true) | ||
|
||
val releaseEarlyNoGpg: Def.Initialize[Boolean] = | ||
Def.setting(false) | ||
val releaseEarlyNoGpg: Def.Initialize[Boolean] = Def.setting(false) | ||
|
||
val releaseEarlySyncToMaven: Def.Initialize[Task[Unit]] = { | ||
Def.taskDyn { | ||
|
@@ -380,71 +375,61 @@ trait Helper { | |
) | ||
} | ||
|
||
def checkRequirementsTask: Def.Initialize[Task[Unit]] = Def.task { | ||
def checkRequirementsTask: Def.Initialize[Task[Unit]] = Def.taskDyn { | ||
import ReleaseEarlyPlugin.{autoImport => ThisPluginKeys} | ||
import scala.util.control.Exception.catching | ||
|
||
val logger = Keys.streams.value.log | ||
val projectName = Keys.name.value | ||
val useSonatype = PrivateKeys.releaseEarlyIsSonatype.value | ||
|
||
logger.info(Feedback.logCheckRequirements(projectName)) | ||
|
||
val bintrayCredentials = { | ||
if (useSonatype) { | ||
logger.debug(Feedback.skipBintrayCredentialsCheck(projectName)) | ||
None | ||
} else { | ||
catching(classOf[NoSuchElementException]) | ||
.opt(bintray.BintrayKeys.bintrayEnsureCredentials.value) | ||
} | ||
def check(checks: (Boolean, String)*): Unit = { | ||
val errors = checks.collect { case (true, feedback) => logger.error(feedback) } | ||
if (errors.nonEmpty) sys.error(Feedback.fixRequirementErrors) | ||
} | ||
|
||
val sonatypeCredentials = { | ||
if (useSonatype) { | ||
getSonatypeCredentials.orElse { | ||
// Get extra credentials from optional environment variables | ||
val extraCredentials = getExtraSonatypeCredentials | ||
extraCredentials.foreach(persistExtraSonatypeCredentials) | ||
extraCredentials | ||
} | ||
} else { | ||
logger.debug(Feedback.skipBintrayCredentialsCheck(projectName)) | ||
None | ||
// Using Sonatype publisher | ||
if (PrivateKeys.releaseEarlyIsSonatype.value) Def.task { | ||
logger.debug(Feedback.skipBintrayCredentialsCheck(projectName)) | ||
val sonatypeCredentials = getSonatypeCredentials.orElse { | ||
// Get extra credentials from optional environment variables | ||
val extraCredentials = getExtraSonatypeCredentials | ||
extraCredentials.foreach(persistExtraSonatypeCredentials) | ||
extraCredentials | ||
} | ||
} | ||
|
||
// If not interactive, it means input has to come from environment | ||
val missingBintrayCredentials = !useSonatype && bintrayCredentials.isEmpty | ||
val missingSonatypeCredentials = ( | ||
// True if sonatype is the underlying publisher | ||
useSonatype && ( | ||
// Or if Bintray publishes a stable version under interactive mode | ||
val missingSonatypeCredentials = { | ||
sonatypeCredentials.isEmpty && | ||
!Keys.isSnapshot.value && | ||
sonatypeCredentials.isEmpty && | ||
!Keys.state.value.interactive | ||
!Keys.state.value.interactive | ||
} | ||
|
||
val sonatypeInconsistentState = ThisPluginKeys.releaseEarlyNoGpg.value | ||
check( | ||
(missingSonatypeCredentials, Feedback.missingSonatypeCredentials), | ||
(sonatypeInconsistentState, Feedback.SonatypeInconsistentGpgState) | ||
) | ||
) | ||
|
||
val ignoreGpg = ReleaseEarlyPlugin.autoImport.releaseEarlyNoGpg.value | ||
val syncToMaven = ReleaseEarlyPlugin.autoImport.releaseEarlyEnableSyncToMaven.value | ||
val bintrayInconsistentState = !useSonatype && syncToMaven && ignoreGpg | ||
val sonatypeInconsistentState = useSonatype && ignoreGpg | ||
// Using Bintray publisher | ||
} else Def.task { | ||
val missingBintrayCredentials = { | ||
catching(classOf[NoSuchElementException]) | ||
.opt(bintray.BintrayKeys.bintrayEnsureCredentials.value) | ||
.isEmpty | ||
} | ||
|
||
val Checks = List( | ||
(missingBintrayCredentials, Feedback.missingBintrayCredentials), | ||
(missingSonatypeCredentials, Feedback.missingSonatypeCredentials), | ||
(bintrayInconsistentState, Feedback.BintrayInconsistentGpgState), | ||
(sonatypeInconsistentState, Feedback.SonatypeInconsistentGpgState) | ||
) | ||
val bintrayInconsistentState = | ||
ThisPluginKeys.releaseEarlyEnableSyncToMaven.value && | ||
ThisPluginKeys.releaseEarlyNoGpg.value | ||
|
||
val hasErrors = Checks.foldLeft(false) { | ||
case (hasError, (predicate, feedback)) => | ||
if (predicate) { logger.error(feedback); true } else hasError | ||
check( | ||
(missingBintrayCredentials, Feedback.missingBintrayCredentials), | ||
(bintrayInconsistentState, Feedback.BintrayInconsistentGpgState) | ||
) | ||
} | ||
|
||
if (hasErrors) sys.error(Feedback.fixRequirementErrors) | ||
} | ||
|
||
def validatePomTask: Def.Initialize[Task[Unit]] = Def.task { | ||
def validatePomTask: Def.Initialize[Task[Unit]] = Def.taskDyn { | ||
val logger = Keys.streams.value.log | ||
logger.info(Feedback.logValidatePom(Keys.name.value)) | ||
|
||
|
@@ -463,10 +448,12 @@ trait Helper { | |
} else hasError | ||
} | ||
|
||
if (hasErrors) sys.error(Feedback.fixRequirementErrors) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why has this new line be added here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just some freestyle formatting 😜 I just noticed that you have a Ah, and this line was moved from below to be called before that resulting def-task is returned. If it fails here, it doesn't change anything, so I needed to move it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But don't we have now two repeated lines in the same source? This is a new addition, it does not replace or remove anything. The same line seems to exist in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand. This is in the |
||
|
||
// Ensure licenses before releasing | ||
val useBintray = !PrivateKeys.releaseEarlyIsSonatype.value | ||
if (useBintray) bintray.BintrayKeys.bintrayEnsureLicenses.value | ||
if (hasErrors) sys.error(Feedback.fixRequirementErrors) | ||
if (useBintray) bintray.BintrayKeys.bintrayEnsureLicenses | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this check is unnecessarily duplicated in Does it mean that it checks it even for sonatype? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that we're already doing the checks in the right place, that line should be deleted 😉 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 77b7b70 |
||
else Def.task(()) | ||
} | ||
|
||
def runCommandAndRemaining(command: String): State => State = { st: State => | ||
|
@@ -480,7 +467,7 @@ trait Helper { | |
nextState.remainingCommands.toList match { | ||
case Nil => nextState | ||
case head :: tail => | ||
runCommand(head, nextState.copy(remainingCommands = tail)) | ||
runCommand(head.commandLine, nextState.copy(remainingCommands = tail)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh wow, this is weird. Why is this required? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think before it took a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll have a closer look then, don't worry! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM |
||
} | ||
} | ||
runCommand(command, st.copy(remainingCommands = Nil)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the plan is to cross publish for both sbt 0.13 and sbt 1.0 I suggest to replacing these with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what's @jvican's plan, but I actually don't see any point in cross publishing for 0.13 and 1.0 (now that 1.0 has been released). Especially if the code needs to be altered, it only makes it harder to maintain.
And if some users are "stuck" on sbt 0.13, they can happily use previous version of the plugin until they are ready to upgrade.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, let's not cross-build. If this really becomes a problem for users, we can rethink it in the future. Those stuck in 0.13.x can use 1.2.
This decision will obviously require a 2.0 release.