https://pantsbuild.org/ logo
w

witty-crayon-22786

11/05/2021, 9:11 PM
the message is rendered when the rule completes, not when it is depended on. so you probably do have multiple copies of the rule running with different Params
👀 1
h

hundreds-father-404

11/05/2021, 9:13 PM
Interesting. So maybe this
ProcessCacheScope.ALWAYS
was a red herring To debug, log the
FallibleBuildGoPackageRequest
, right?
w

witty-crayon-22786

11/05/2021, 9:14 PM
or at the head of the rule, yea
👍 1
and fwiw, even failures are memoized within a run
💯 1
(everything except an exception)
h

hundreds-father-404

11/05/2021, 9:18 PM
Hm the rule is only executing once, but streaming result still happens twice!
Copy code
diff --git a/pants.toml b/pants.toml
index 2c60b314d..f4effe200 100644
--- a/pants.toml
+++ b/pants.toml
@@ -51,7 +51,7 @@ pants_ignore.add = [
 
 build_ignore.add = [
   # Disable Go targets by default so Pants developers do not need Go installed.
-  "testprojects/src/go/**",
+#  "testprojects/src/go/**",
 ]
 
 # NB: Users must still set `--remote-cache-{read,write}` to enable the remote cache.
diff --git a/src/python/pants/backend/go/util_rules/build_pkg.py b/src/python/pants/backend/go/util_rules/build_pkg.py
index 9839ce5d9..31dfa58a5 100644
--- a/src/python/pants/backend/go/util_rules/build_pkg.py
+++ b/src/python/pants/backend/go/util_rules/build_pkg.py
@@ -2,6 +2,7 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 from __future__ import annotations
 
+import logging
 import os.path
 from dataclasses import dataclass
 
@@ -21,6 +22,7 @@ from pants.util.frozendict import FrozenDict
 from pants.util.logging import LogLevel
 from pants.util.strutil import path_safe
 
+logger = logging.getLogger(__name__)
 
 class BuildGoPackageRequest(EngineAwareParameter):
     def __init__(
@@ -175,6 +177,7 @@ class BuiltGoPackage:
 # (triggered by `FallibleBuiltGoPackage` subclassing `EngineAwareReturnType`).
 @rule(desc="Compile with Go", level=LogLevel.DEBUG)
 async def build_go_package(request: BuildGoPackageRequest) -> FallibleBuiltGoPackage:
+    <http://logger.info|logger.info>(request.import_path)
     maybe_built_deps = await MultiGet(
         Get(FallibleBuiltGoPackage, BuildGoPackageRequest, build_request)
         for build_request in request.direct_dependencies
diff --git a/testprojects/src/go/pants_test/bar/bar.go b/testprojects/src/go/pants_test/bar/bar.go
index 1f590eb92..bf7a686e3 100644
--- a/testprojects/src/go/pants_test/bar/bar.go
+++ b/testprojects/src/go/pants_test/bar/bar.go
@@ -7,5 +7,6 @@ func GenUuid() string {
 }
 
 func Quote(s string) string {
+    x := "foo"
 	return ">> " + s + " <<"
 }
Copy code
❯ ./pants check testprojects/src/go::
14:17:56.06 [INFO] main
14:17:56.36 [INFO] <http://github.com/pantsbuild/pants/testprojects/src/go/pants_test/bar|github.com/pantsbuild/pants/testprojects/src/go/pants_test/bar>
14:17:56.36 [INFO] <http://github.com/pantsbuild/pants/testprojects/src/go/pants_test|github.com/pantsbuild/pants/testprojects/src/go/pants_test>
14:17:56.36 [INFO] <http://github.com/google/uuid|github.com/google/uuid>
oh...... 🤦 I set up so that a dependee renders the failure of their dependency. Sorry for the noise! Nothing wrong the engine, operator error
w

witty-crayon-22786

11/05/2021, 9:24 PM
pheew
h

hundreds-father-404

11/05/2021, 9:25 PM
That does raise a (less interesting) question though. Any UX opinions on how this should render? I think something like this:
Copy code
Completed: Compile with Go - <http://github.com/pantsbuild/pants/testprojects/src/go/pants_test|github.com/pantsbuild/pants/testprojects/src/go/pants_test> failed (exit code 1).
Dependency failed to compile: <http://github.com/pantsbuild/pants/testprojects/src/go/pants_test/bar|github.com/pantsbuild/pants/testprojects/src/go/pants_test/bar>
w

witty-crayon-22786

11/05/2021, 9:26 PM
no need to report the failed dependency… it will always already have been rendered.
(i don’t think that we do for the JVM…? not sure though)
… oh. i guess that we do.
in a deep graph, it would be very noisy to render all of the “dependency failed to compile” stuff
so probably both of those should just remain silent when a dependency fails.
h

hundreds-father-404

11/05/2021, 9:28 PM
Hm, it also is semantically incorrect to claim it succeeded. We could add special casing that it wasn't attempted? Like log at debug level, rather than
Error
w

witty-crayon-22786

11/05/2021, 9:28 PM
check
will still report a line per root
h

hundreds-father-404

11/05/2021, 9:28 PM
Nack,
check
only reports for the entire compiler
w

witty-crayon-22786

11/05/2021, 9:28 PM
We could add special casing that it wasn’t attempted? Like log at debug level, rather than 
Error
sure: we already special case that though: https://github.com/pantsbuild/pants/blob/3874cd6f683692ac109f41a61eb26265d67f3696/src/python/pants/jvm/compile.py#L73-L85 … so expanding that to look at the error type and do nothing seems fine.
Nack, 
check
 only reports for the entire compiler
it shouldn’t probably. just for the roots. pretty sure that’s the case for JVM
you get streaming output for the rest
h

hundreds-father-404

11/05/2021, 9:32 PM
Yes, but all
check
reports is the compiler name + error code. No std{out,err}. If you say
./pants go check pkg:
and it fails due to a transitive dep, would you expect the results to explicitly mention
pkg:
? Or it's enough to say that
pkg/dep
had failed so the user can infer
pkg
also failed
w

witty-crayon-22786

11/05/2021, 9:32 PM
it will show a line per root target, won’t it…?
i guess that is also noisy in a bunch of cases… per-compiler is probably fine.
h

hundreds-father-404

11/05/2021, 9:33 PM
Not
check
! Only our streaming of
BuildGoPackageRequest
is per-target
w

witty-crayon-22786

11/05/2021, 9:33 PM
k, yea.
i think just don’t stream anything for the dependency-failed case.
1
h

hundreds-father-404

11/05/2021, 9:33 PM
Cool, I definitely agree. It's wayyyy too noisy otherwise when doing
check ::
and a downstream dep fails to compile. Thanks!
w

witty-crayon-22786

11/05/2021, 9:33 PM
if you make that edit, fixing the JVM while you’re at it would be great too.
👍 1
h

hundreds-father-404

11/05/2021, 9:34 PM
Will cherry-pick to 2.8. A little annoyed I did a release last night rather than waiting