@hundreds-father-404: what are your thoughts on whether or not first-party package analysis should fail due to syntax errors? my inclination is that it should not fail; later compilation will fail but package analysis shouldn’t. (context is that first_party_pkg_test has a unit test that looks for failure of analysis due to a syntax error in a go file)

It should behave how

FallibleFirstPartyPkgInfo

was before: gracefully handle the error. Don't crash Pants, but also indicate it was a failure so that callers can decide what to do

check

will output it as a failure, for example.

package

should crash and burn

but

check

will try to compile it and the failure will be the compile failure

Nack,

check

will not compile if analysis failed. Check out

go/goals/check.py

context is my PR to add go:embed comment scanning. my point is that if that PR changes the behavior to allow analysis to succeed, then

check

will try and compile

Oh, I don't see the benefit? Perhaps that the error message would be better coming from Go rather than from our own parser? It does simplify the code a little if we have

FirstPartyPkgInfo

always be infallabile, altho that isn't reason enough imo to make the semantic change

the semantic change is part of adopting the rules_go parsing code as-is

I tend to agree that dep inference shouldn't fail on parse errors (a warning is appropriate)

one issue is that the import extraction could have failed due to the parse error

Dep inference has that exact behavior right now, Benjy I think the only impact of Tom's change would be: • Dep inference no longer warns when there was a parse error •

check

,

test

, and

package

attempt to compile bad packages, rather than early aborting Does that sound right?

that sounds right.

That sounds like what is already happening with

FallibleFirstPartyPkgInfo

?

yeah and was easy enough to keep the existing semantics

Yes, that sounds right to me. I think that's what people will expect.