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

Suppress "no changes" output by default #92

Merged
merged 6 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions dependency-guard/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ testing {
useJUnitJupiter()
dependencies {
// gradleTest test suite depends on the production code in tests
implementation(project)
implementation(project())
implementation(libs.truth)
}

targets {
all {
configureEach {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a small opportunistic improvement as all eagerly configures targets and configureEach is lazy

testTask.configure {
shouldRunAfter(test)
dependsOn(deleteOldGradleTests)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,40 @@ class PluginTest {
args = arrayOf(":lib:dependencyGuard")
)

assertThat(result.output)
.doesNotContain("No Dependency Changes Found in :lib for configuration \"compileClasspath\"")

project.assertFileExistsWithContentEqual(
filename = "lib/dependencies/compileClasspath.txt",
contentFile = "simple/list_before_update.txt",
)

project.assertFileExistsWithContentEqual(
filename = "lib/dependencies/compileClasspath.tree.txt",
contentFile = "simple/tree_before_update.txt",
)
}

@ParameterizedPluginTest
fun `guard with no dependencies changes - verbose`(
args: ParameterizedPluginArgs,
): Unit = SimpleProject(tree = true).use { project ->
// create baseline
build(
gradleVersion = args.gradleVersion,
withConfigurationCache = args.withConfigurationCache,
project = project,
args = arrayOf(":lib:dependencyGuard")
)

// check with no dependencies changes
val result = build(
gradleVersion = args.gradleVersion,
withConfigurationCache = args.withConfigurationCache,
project = project,
args = arrayOf(":lib:dependencyGuard", "-PdependencyGuard.verbose=true")
)

assertThat(result.output)
.contains("No Dependency Changes Found in :lib for configuration \"compileClasspath\"")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,20 @@ import org.gradle.api.GradleException
import org.gradle.api.Project
import org.gradle.api.artifacts.result.ResolvedComponentResult
import org.gradle.api.file.DirectoryProperty
import org.gradle.api.model.ObjectFactory
import org.gradle.api.provider.MapProperty
import org.gradle.api.provider.Property
import org.gradle.api.provider.Provider
import org.gradle.api.provider.ProviderFactory
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.OutputDirectory
import org.gradle.api.tasks.TaskAction
import javax.inject.Inject

internal abstract class DependencyGuardListTask : DefaultTask() {
internal abstract class DependencyGuardListTask @Inject constructor(
objects: ObjectFactory,
providers: ProviderFactory
) : DefaultTask() {

init {
group = DependencyGuardPlugin.DEPENDENCY_GUARD_TASK_GROUP
Expand Down Expand Up @@ -61,6 +67,10 @@ internal abstract class DependencyGuardListTask : DefaultTask() {
@get:Input
abstract val monitoredConfigurationsMap: MapProperty<DependencyGuardConfiguration, Provider<ResolvedComponentResult>>

@get:Input
val logVerbosely: Property<Boolean> = objects.property(Boolean::class.java)
.convention(providers.gradleProperty("dependencyGuard.verbose").map { it.toBoolean() }.orElse(false))

@get:OutputDirectory
abstract val projectDirectoryDependenciesDir: DirectoryProperty

Expand All @@ -86,19 +96,24 @@ internal abstract class DependencyGuardListTask : DefaultTask() {
when (diffResult) {
is DependencyListDiffResult.DiffPerformed.HasDiff -> {
// Print to console in color
println(diffResult.createDiffMessage(withColor = true))
logger.error(diffResult.createDiffMessage(withColor = true))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm still understanding the Gradle Logging https://docs.gradle.org/current/userguide/logging.html

Why is this considered an error? Because of the priority that we want to have the user see this? Based on the docs QUIET seems like the right level as it is an Important information messages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

error is always displayed to the user, and some folks may instrument their builds to capture and save/send/etc error level logs. Would it not be an error?

Copy link
Collaborator

Choose a reason for hiding this comment

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

A detected difference would be an error from a task result perspective.

So, error is good. Thanks 👍


// Add to exception message without color
exceptionMessage.appendLine(diffResult.createDiffMessage(withColor = false))
}

is DependencyListDiffResult.DiffPerformed.NoDiff -> {
// Print no diff message
println(diffResult.noDiffMessage)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for getting rid of these println statements. This is the artifact of a developer that didn't know how to write Gradle plugins, and println "worked just fine" before. This is much nicer, thanks!

val message = diffResult.noDiffMessage
if (logVerbosely.get()) {
logger.lifecycle(message)
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we just delete these lines, and then get rid of the verbose option?

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'm fine with that too, was just trying to leave existing functionality possible if it was wanted

Copy link
Collaborator

Choose a reason for hiding this comment

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

Appreciate it. Because it is just this case, I personally will not miss it. The goal of the dependencyGuard task is to succeed if no diffs, so there shouldn't need to be this extra logging.

If someone has a different option, an issue could be created for discussion, but this makes sense here IMO.

logger.debug(message)
}
}

is DependencyListDiffResult.BaselineCreated -> {
println(diffResult.baselineCreatedMessage(true))
logger.lifecycle(diffResult.baselineCreatedMessage(true))
}
}

Expand Down
Binary file modified gradle/wrapper/gradle-wrapper.jar
Binary file not shown.
3 changes: 2 additions & 1 deletion gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.5.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6.1-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
18 changes: 14 additions & 4 deletions gradlew
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
# Darwin, MinGW, and NonStop.
#
# (3) This script is generated from the Groovy template
# https://github.com/gradle/gradle/blob/master/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt
# https://github.com/gradle/gradle/blob/HEAD/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt
# within the Gradle project.
#
# You can find Gradle at https://github.com/gradle/gradle/.
Expand All @@ -80,10 +80,10 @@ do
esac
done

APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit

APP_NAME="Gradle"
# This is normally unused
# shellcheck disable=SC2034
APP_BASE_NAME=${0##*/}
APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit

# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'
Expand Down Expand Up @@ -143,12 +143,16 @@ fi
if ! "$cygwin" && ! "$darwin" && ! "$nonstop" ; then
case $MAX_FD in #(
max*)
# In POSIX sh, ulimit -H is undefined. That's why the result is checked to see if it worked.
# shellcheck disable=SC3045
MAX_FD=$( ulimit -H -n ) ||
warn "Could not query maximum file descriptor limit"
esac
case $MAX_FD in #(
'' | soft) :;; #(
*)
# In POSIX sh, ulimit -n is undefined. That's why the result is checked to see if it worked.
# shellcheck disable=SC3045
ulimit -n "$MAX_FD" ||
warn "Could not set maximum file descriptor limit to $MAX_FD"
esac
Expand Down Expand Up @@ -205,6 +209,12 @@ set -- \
org.gradle.wrapper.GradleWrapperMain \
"$@"

# Stop when "xargs" is not available.
if ! command -v xargs >/dev/null 2>&1
then
die "xargs is not available"
fi

# Use "xargs" to parse quoted args.
#
# With -n1 it outputs one arg per line, with the quotes and backslashes removed.
Expand Down
15 changes: 9 additions & 6 deletions gradlew.bat
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
@rem limitations under the License.
@rem

@if "%DEBUG%" == "" @echo off
@if "%DEBUG%"=="" @echo off
@rem ##########################################################################
@rem
@rem Gradle startup script for Windows
Expand All @@ -25,7 +25,8 @@
if "%OS%"=="Windows_NT" setlocal

set DIRNAME=%~dp0
if "%DIRNAME%" == "" set DIRNAME=.
if "%DIRNAME%"=="" set DIRNAME=.
@rem This is normally unused
set APP_BASE_NAME=%~n0
set APP_HOME=%DIRNAME%

Expand All @@ -40,7 +41,7 @@ if defined JAVA_HOME goto findJavaFromJavaHome

set JAVA_EXE=java.exe
%JAVA_EXE% -version >NUL 2>&1
if "%ERRORLEVEL%" == "0" goto execute
if %ERRORLEVEL% equ 0 goto execute

echo.
echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
Expand Down Expand Up @@ -75,13 +76,15 @@ set CLASSPATH=%APP_HOME%\gradle\wrapper\gradle-wrapper.jar

:end
@rem End local scope for the variables with windows NT shell
if "%ERRORLEVEL%"=="0" goto mainEnd
if %ERRORLEVEL% equ 0 goto mainEnd

:fail
rem Set variable GRADLE_EXIT_CONSOLE if you need the _script_ return code instead of
rem the _cmd.exe /c_ return code!
if not "" == "%GRADLE_EXIT_CONSOLE%" exit 1
exit /b 1
set EXIT_CODE=%ERRORLEVEL%
if %EXIT_CODE% equ 0 set EXIT_CODE=1
if not ""=="%GRADLE_EXIT_CONSOLE%" exit %EXIT_CODE%
exit /b %EXIT_CODE%

:mainEnd
if "%OS%"=="Windows_NT" endlocal
Expand Down