Skip to content

Commit

Permalink
Extract a small macro for the two jars created by genrule.
Browse files Browse the repository at this point in the history
RELNOTES=n/a
PiperOrigin-RevId: 698509165
  • Loading branch information
netdpb authored and Flogger Team committed Nov 20, 2024
1 parent febddca commit b3f5472
Show file tree
Hide file tree
Showing 13 changed files with 111 additions and 59 deletions.
2 changes: 1 addition & 1 deletion MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ bazel_dep(name = "rules_java", version = "7.12.1")
bazel_dep(name = "google_bazel_common")
git_override(
module_name = "google_bazel_common",
commit = "e2204d625d97dda88112af0b0a9e56ed7712d15c",
commit = "a7761f9d45b65283274d8d86abb3b87e9fc87258",
remote = "https://github.com/google/bazel-common",
)
46 changes: 12 additions & 34 deletions api/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ load("@google_bazel_common//tools/jarjar:jarjar.bzl", "jarjar_library")
load("@google_bazel_common//tools/javadoc:javadoc.bzl", "javadoc_library")
load("@rules_java//java:defs.bzl", "java_binary", "java_import", "java_library")
load("//tools:maven.bzl", "pom_file")
load(":javac.bzl", "genjar")

SYSTEM_BACKEND_SRCS = glob(["src/main/java/com/google/common/flogger/backend/system/**/*.java"])

Expand Down Expand Up @@ -62,48 +63,25 @@ java_import(
deps = [":stack_getter_common"],
)

genrule(
genjar(
name = "gen_stack_getter_java_lang_access_impl",
srcs = STACK_GETTER_JAVA_LANG_ACCESS_IMPL_SRCS + [
":stack_getter_common",
srcs = STACK_GETTER_JAVA_LANG_ACCESS_IMPL_SRCS,
toolchains = ["@rules_java//toolchains:jdk_8"],
deps = [
":checks",
":stack_getter_common",
],
outs = ["stack_getter_java_lang_access_impl.jar"],
cmd = """
set -eu
TMPDIR=$$(mktemp -d)
OUTPUT=$$PWD/$@
STACK_GETTER_COMMON=$(location :stack_getter_common)
CHECKS=$(location :checks)
JAVABASE=$$PWD/$(JAVABASE)
JAR=$$PWD/$(JAVABASE)/bin/jar
$$JAVABASE/bin/javac -d $$TMPDIR -source 8 -target 8 -cp $${STACK_GETTER_COMMON}:$${CHECKS} -implicit:none \
$(location src/main/java/com/google/common/flogger/util/JavaLangAccessStackGetter.java)
cd $$TMPDIR && $$JAR cf $$OUTPUT com/google/common/flogger/util/*StackGetter*.class
""",
toolchains = ["@rules_java//toolchains:jdk_8"],
)

genrule(
genjar(
name = "gen_stack_getter_stack_walker_impl",
srcs = STACK_GETTER_STACK_WALKER_IMPL_SRCS + [
":stack_getter_common",
srcs = STACK_GETTER_STACK_WALKER_IMPL_SRCS,
toolchains = ["@rules_java//toolchains:remote_jdk11"],
deps = [
":checks",
":stack_getter_common",
"@google_bazel_common//third_party/java/jspecify_annotations",
],
outs = ["stack_getter_stack_walker_impl.jar"],
cmd = """
set -eu
TMPDIR=$$(mktemp -d)
OUTPUT=$$PWD/$@
STACK_GETTER_COMMON=$(location :stack_getter_common)
CHECKS=$(location :checks)
JAVABASE=$$PWD/$(JAVABASE)
JAR=$$PWD/$(JAVABASE)/bin/jar
$$JAVABASE/bin/javac -d $$TMPDIR -source 8 -target 8 -cp $${STACK_GETTER_COMMON}:$${CHECKS} -implicit:none \
$(location src/main/java/com/google/common/flogger/util/StackWalkerStackGetter.java)
cd $$TMPDIR && $$JAR cf $$OUTPUT com/google/common/flogger/util/*StackGetter*.class
""",
toolchains = ["@rules_java//toolchains:remote_jdk11"],
)

java_library(
Expand Down
46 changes: 46 additions & 0 deletions api/javac.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Copyright (C) 2024 The Flogger Authors.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

def genjar(name, srcs = [], deps = [], **kwargs):
"""Compiles Java source files to a jar file using a genrule.
Args:
name: The name of the target. The output jar file will be named <name>.jar.
srcs: The Java source files to compile.
deps: The Java libraries to depend on.
**kwargs: Additional arguments to pass to genrule.
"""
native.genrule(
name = name,
srcs = srcs + deps,
outs = [name + ".jar"],
cmd =
"""
set -eu
TMPDIR="$$(mktemp -d)"
OUTPUT="$$PWD/$@"
JAVABASE="$$PWD/$(JAVABASE)"
JAR="$$PWD/$(JAVABASE)/bin/jar"
"$$JAVABASE/bin/javac" -d "$$TMPDIR" \
-source 8 -target 8 -implicit:none \
-cp "{classpath}" {srcs}
cd "$$TMPDIR" && "$$JAR" cf "$$OUTPUT" .
unzip -l "$$OUTPUT"
false # for testing externally
""".format(
srcs = " ".join(["$(location %s)" % src for src in srcs]),
classpath = ":".join(["$(location %s)" % dep for dep in deps]),
),
**kwargs
)
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.common.flogger.backend;

import static java.util.Objects.requireNonNull;
import static org.objectweb.asm.Opcodes.ACC_FINAL;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
Expand Down Expand Up @@ -108,7 +109,7 @@ public static void main(String[] args) throws IOException {

// Write the class to the output file.
Path path = Paths.get(args[0]);
Files.createDirectories(path.getParent());
Files.createDirectories(requireNonNull(path.getParent(), "path must have a parent: " + path));
try (JarOutputStream jar =
new JarOutputStream(Files.newOutputStream(path, StandardOpenOption.CREATE_NEW))) {
ZipEntry entry = new ZipEntry("com/google/common/flogger/backend/PlatformProvider.class");
Expand Down
13 changes: 8 additions & 5 deletions api/src/main/java/com/google/common/flogger/LogContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import static com.google.common.flogger.util.CallerFinder.getStackForCallerOf;
import static com.google.common.flogger.util.Checks.checkNotNull;
import static com.google.common.flogger.util.Checks.checkState;
import static java.util.concurrent.TimeUnit.NANOSECONDS;

import com.google.common.flogger.DurationRateLimiter.RateLimitPeriod;
Expand Down Expand Up @@ -435,16 +436,18 @@ public final LogSite getLogSite() {

@Override
public final Object[] getArguments() {
if (templateContext == null) {
throw new IllegalStateException("cannot get arguments unless a template context exists");
checkState(templateContext != null, "cannot get arguments unless a template context exists");
if (args == null) {
throw new IllegalStateException("cannot get arguments before calling log()");
}
return args;
}

@Override
public final Object getLiteralArgument() {
if (templateContext != null) {
throw new IllegalStateException("cannot get literal argument if a template context exists");
checkState(templateContext == null, "cannot get literal argument if a template context exists");
if (args == null) {
throw new IllegalStateException("cannot get literal argument before calling log()");
}
return args[0];
}
Expand Down Expand Up @@ -683,7 +686,7 @@ private boolean shouldLog() {
if (rateLimitStatus != null) {
// We check rate limit status even if it is "DISALLOW" to update the skipped logs count.
int skippedLogs = RateLimitStatus.checkStatus(rateLimitStatus, logSiteKey, metadata);
if (shouldLog && skippedLogs > 0) {
if (shouldLog && skippedLogs > 0 && metadata != null) {
metadata.addValue(Key.SKIPPED_LOG_COUNT, skippedLogs);
}
// checkStatus() returns -1 in two cases:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ protected Random initialValue() {
}
};

// Android annotates ThreadLocal.get() with @RecentlyNullable, but random.get() is never null.
@SuppressWarnings("nullness")
private static Random getRandom() {
return random.get();
}

// Visible for testing.
final AtomicInteger pendingCount = new AtomicInteger();

Expand All @@ -72,7 +78,7 @@ RateLimitStatus sampleOneIn(int rateLimitCount) {
// zero and the number of logs will end up statistically close to 1-in-N (even if at
// times they can be "bursty" due to the action of other rate limiting mechanisms).
int pending;
if (random.get().nextInt(rateLimitCount) == 0) {
if (getRandom().nextInt(rateLimitCount) == 0) {
pending = pendingCount.incrementAndGet();
} else {
pending = pendingCount.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,12 @@ private static Platform loadFirstAvailablePlatform(String[] platformClass) {
return (Platform) Class.forName(clazz).getConstructor().newInstance();
} catch (Throwable e) {
// Catch errors so if we can't find _any_ implementations, we can report something useful.
// Unwrap any generic wrapper exceptions for readability here (extend this as needed).
if (e instanceof InvocationTargetException) {
e = e.getCause();
}
errorMessage.append('\n').append(clazz).append(": ").append(e);
errorMessage
.append('\n')
.append(clazz)
.append(": ")
// Unwrap any wrapper exceptions for readability here (extend as needed).
.append(e instanceof InvocationTargetException ? e.getCause() : e);
}
}
throw new IllegalStateException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;
import org.jspecify.annotations.Nullable;

/**
* Common backend to handle everything except formatting of log message and metadata. This is an
Expand Down Expand Up @@ -146,7 +145,7 @@ public final void log(LogRecord record, boolean wasForced) {
// would get a lot of extra stack frames to search through). However for Flogger, the LogSite
// was already determined in the "shouldLog()" method because it's needed for things like
// rate limiting. Thus we don't have to care about using iterative methods vs recursion here.
private static void publish(@Nullable Logger logger, LogRecord record) {
private static void publish(Logger logger, LogRecord record) {
// Annoyingly this method appears to copy the array every time it is called, but there's
// nothing much we can do about this (and there could be synchronization issues even if we
// could access things directly because handlers can be changed at any time). Most of the
Expand All @@ -155,9 +154,9 @@ private static void publish(@Nullable Logger logger, LogRecord record) {
handler.publish(record);
}
if (logger.getUseParentHandlers()) {
logger = logger.getParent();
if (logger != null) {
publish(logger, record);
Logger parent = logger.getParent();
if (parent != null) {
publish(parent, record);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,12 @@ public final void setParameters(Object @Nullable [] parameters) {
super.setParameters(parameters);
}

@Override
@SuppressWarnings("nullness") // setParameters() above guarantees this is never null.
public final Object[] getParameters() {
return super.getParameters();
}

@Override
public final void setMessage(@Nullable String message) {
if (message == null) {
Expand Down
9 changes: 7 additions & 2 deletions api/src/main/java/com/google/common/flogger/context/Tags.java
Original file line number Diff line number Diff line change
Expand Up @@ -513,8 +513,12 @@ private int mergeTagMaps(
int newEntryIndex = 0;
while (lhsEntry != null || rhsEntry != null) {
// Nulls count as being *bigger* than anything (since they indicate the end of the array).
int signum = (lhsEntry == null) ? 1 : (rhsEntry == null) ? -1 : 0;
if (signum == 0) {
int signum;
if (lhsEntry == null) {
signum = 1;
} else if (rhsEntry == null) {
signum = -1;
} else {
// Both entries exist and must be compared.
signum = lhsEntry.getKey().compareTo(rhsEntry.getKey());
if (signum == 0) {
Expand All @@ -528,6 +532,7 @@ private int mergeTagMaps(
continue;
}
}

// Signum is non-zero and indicates which entry to process next (without merging).
if (signum < 0) {
valueStart = copyEntryAndValues(lhsEntry, newEntryIndex++, valueStart, array, offsets);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public static SimpleParameter of(int index, FormatChar formatChar, FormatOptions
// We can safely test FormatSpec with '==' because the factory methods always return the default
// instance if applicable (and the class has no visible constructors).
if (index < MAX_CACHED_PARAMETERS && options.isDefault()) {
return DEFAULT_PARAMETERS.get(formatChar)[index];
return checkNotNull(DEFAULT_PARAMETERS.get(formatChar), "default parameter")[index];
}
return new SimpleParameter(index, formatChar, options);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,15 @@ protected RecursionDepth initialValue() {
}
};

// Android annotates ThreadLocal.get() with @RecentlyNullable, but holder.get() is never null.
@SuppressWarnings("nullness")
private static RecursionDepth getRecursionDepth() {
return holder.get();
}

/** Do not call this method directly, use {@code Platform.getCurrentRecursionDepth()}. */
public static int getCurrentDepth() {
return holder.get().value;
return getRecursionDepth().value;
}

/** Do not call this method directly, use {@code Platform.getCurrentRecursionDepth()}. */
Expand All @@ -48,7 +54,7 @@ public int getValue() {

/** Internal API for use by core Flogger library. */
public static RecursionDepth enterLogStatement() {
RecursionDepth depth = holder.get();
RecursionDepth depth = getRecursionDepth();
// Can only reach 0 if it wrapped around completely or someone is manipulating the value badly.
// We really don't expect 2^32 levels of recursion however, so assume it's a bug.
if (++depth.value == 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.lang.StackWalker.StackFrame;
import java.util.function.Predicate;
import java.util.stream.Stream;
import org.jspecify.annotations.Nullable;

/**
* StackWalker based implementation of the {@link StackGetter} interface.
Expand All @@ -39,9 +40,9 @@ public StackWalkerStackGetter() {
}

@Override
public StackTraceElement callerOf(Class<?> target, int skipFrames) {
public @Nullable StackTraceElement callerOf(Class<?> target, int skipFrames) {
checkArgument(skipFrames >= 0, "skipFrames must be >= 0");
return STACK_WALKER.walk(
return STACK_WALKER.<@Nullable StackTraceElement>walk(
s ->
filterStackTraceAfterTarget(isTargetClass(target), skipFrames, s)
.findFirst()
Expand Down

0 comments on commit b3f5472

Please sign in to comment.