https://pantsbuild.org/ logo
Join Slack
Powered by
I’m getting a <lint failure> on a `BUILD` file tha...
# development
a
I’m getting a lint failure on a 
BUILD
file that I can’t replicate on my M1 (pwith py3.9) or on a linux box (with py3.7). There’s no real error in the CI output other than what the file is. How can I diagnose this?
w
that is the new 
update-build-files
goal, apparently cc @hundreds-father-404
a
It seems like Format with Black is an instruction, which it does not appear to be, actually
Should that task be invoked by 
./pants fmt
too?
h
Fix by running 
update-build-files
I can update the output to say "Run 
update-build-files
to fix these issues"?
a
I think that would help.
h
Should that task be invoked by ./pants fmt too?
In a perfect world, yeah. Turns out it is really really tricky to hook up Black for BUILD files to 
fmt
https://github.com/pantsbuild/pants/pull/13279
a
Is there a Dreadful Hack that could be put in place instead? 🙂
h
No, it's really not feasible to hook it up to 
fmt
and 
lint
without major breaking changes. It violates a bunch of very core assumptions like how running 
./pants list path/to/BUILD
returns all targets who are defined there
The awkwardness of this is hopefully mitigated by not touching BUILD files very frequently
a
fair
w
hm. an awkward hack to get it into 
fmt
might eventually allow us to adjust those semantics. i don’t expect much of a slippery slope there.
h
What do you mean? 
core/goals/fmt.py
does not handle the Specs code that translates 
path/to/BUILD
into all targets defined there, that is done beforehand. 
fmt.py
can't hack that code
w
it should be able to… 
tailor
does
from a “semantic meaning” / “goal to be accomplished” perspective, separate goals for formatting source files and BUILD files doesn’t really make sense.
☝️ 1
the “update-build-files” goal started as a way to remove deprecated names/code, and that is clearly a new semantic meaning. but formatting is something else.
but i don’t think that this is time critical for 2.8.x (other than mentioning 
update-build-files
in the release notes to fix deprecations)
➕ 1
h
tailor does
By doing a bunch of weird special casing of 
Specs
and ignoring our 
graph.py
handling of things like 
--changed-since
. NB that 
./pants --changed-since=HEAD tailor
errors It's probably possible to put intense special casing into 
fmt.py
and 
lint.py
, but it would be very involved and would mean that those goals behave differently than 
test
and 
package
the “update-build-files” goal started as a way to remove deprecated names/code,
My mental model for the goal is:
Make Pants config files be in tip-top shape.
Where there's a difference between your own code vs. Pants config files like BUILD and pants.toml
w
sure. and in that regard, removing deprecations is very much like `pyupgrade`… sortof odd to go directly in 
fmt
, and so probably better in a separate goal like 
mend
or 
fix
h
but i don’t think that this is time critical for 2.8.x
Agreed, I don't have the time to try to hack Black on BUILD files into 
fmt
and 
lint
. I tried doing that last month and it was going to be much too hard Doing it via 
update-build-files
was an easy win for a feature users have been asking about for months. If folks feel strongly that it shouldn't live in 
update-build-files
, I can revert that PR? But won't have time to do it with 
fmt
and in that regard, removing deprecations is very much like pyupgrade… sortof odd to go directly in fmt , and so probably better in a separate goal like mend or fix
Key difference is pyupgrade runs on your own code, not Pants config files
w
i’m not sure that that is a key difference. maybe.
Doing it via 
update-build-files
 was an easy win for a feature users have been asking about for months. If folks feel strongly that it shouldn’t live in 
update-build-files
, I can revert that PR? But won’t have time to do it with 
fmt
this might be good, but i don’t feel strongly about it? while removing deprecations, folks will end up reformatting all their BUILD files as well. usually that would be two commits, in order to be a bit easier to review
(it’s a different semantic meaning)
h
while removing deprecations, folks will end up reformatting all their BUILD files as well. usually that would be two commits, in order to be a bit easier to review
If there are BUILD files that weren't already formatted, yep. Which is why we'll recommend setting 
--no-fmt
the first time. (In fact, I can update our deprecation message to say that!) But it was 10 vs 2 votes that 
--fmt
should be enabled by default, so went with that. Once your repo is already formatted, then bumping to a new Pants version and running 
update-build-files
should mean that you are only fixing deprecations because Black no-ops. This is only a first-time adoption quirk
but i don’t feel strongly about it
I really want to have Pants support running Black on BUILD files, particularly because the 
overrides
field has so much nesting and I hate manually spending time fixing that formatting Trying to be pragmatic about how we can get that win in the face of a lot of time/priority constraints
w
yea, it’s definitely good. just a question of carefully identifying the scope of new goals. as an example, 
validate
is an odd goal out right now, because it overlaps semantically with other goals. it exposes an implementation detail that it is a separate goal
➕ 1
and it would be very good for the fix/mend/update goal to not end up pigeonholed the same way, where it only works on BUILD files, and so we have others
so, a (temporary) tradeoff of hacks to keep our externals easier to document and making it clear what is recommended to do in CI and commithooks… vs internal semantic clarity
h
I suspect but don't have definitive proof that a user would not want 
./pants fmt
(or 
./pants fix
) to update 
pants.toml
by removing safe deprecations. Same with not wanting those goals to add missing target definitions That is, that there's value in separating operating on user code vs. Pants config Maybe we should bring that question to #general?
w
well, it very much depends what the docs for 
./pants fix
say… i would expect that based on the name, but perhaps that’s because i’m familiar with 
cargo fix
i would not expect 
fmt
to. but i’m definitely not suggesting that deprecation removal should be there (for the same reason i’m sketched out by 
pyupgrade
being there: putting that in a dedicated goal would be much more comfortable)
h
should I ask in general? I can put the question here first for you to vet, ik the way I word it will impact responses
w
sure.
@hundreds-father-404: are you sure that you wouldn’t expect 
./pants fix
to do that though?
clearly not 
fmt
, but something called 
fix
…?
keeping in mind that 
go
, 
rust
, and 
scala
all have “fix” tools (from go’s ancestry, i think)
h
Hello, another poll! We're trying to understand if you find it useful and intuitive to distinguish between operating on your own code vs. Pants config files (BUILD files, pants.toml). We now have these operations on Pants config files: • add missing target definitions (
./pants tailor
) • fix safe deprecations, like renaming target types • format BUILD files with Black • plugin hook for you to automate your own changes to BUILD files Option A: a goal like 
update-pants-config
does those 4 things, only on Pants's config. 
fmt
, 
lint
, and maybe 
fix
are only for your own code Option B: a new 
fix
goal will add missing target definitions and fix safe deprecations, in addition to running tools like Pyupgrade and Autoflake on your own code. 
fmt
will run Black on both your own code and BUILD files Comments welcomed too!
🅱️ 1
🅰️ 1
w
Option A: a goal like “`update-pants-config`” which does those 3 things, only on Pants’ config. 
fmt
, 
lint
, and maybe 
fix
 are only for your own code
yea, looks good.
@hundreds-father-404: but before doing this, i’d ask again: you’re sure that you wouldn’t expect 
fix
to also operate on BUILD files?
i mean, maybe you want to ask regardless of what you think. that’s fine.
@hundreds-father-404: one other bit to maybe put in the question is that a benefit of fewer goals is shorter CLI invokes and CI configs
👍 1
h
definitely it being 
fix
rather than 
fmt
changes my answer. I think it probably does make sense that 
fmt
formats everything
Hm, yeah, I think I see your point now
So, that being said: being able to merge 
validate
, 
tailor
, and 
update-build-files
into `fmt`/`lint`/`fix` is not super realistic for Pants 2.8, including because it requires Specs changes How do you feel about landing as is, with the vision that we fix this all to unify?
(We might want to add 
fix
goal before we stabilize Pants 2.8 and Pyupgrade + Autoflake using 
fmt
..)
w
imo, as long as we don’t advertise 
update-build-files
as for being used for anything other than the upgrade, we’re fine for 2.8.x
i.e., let’s not push people on putting it in CI, since it’s likely to change further.
h
hm but when it is changed, we should respect the deprecation policy. it's a stable feature, not gated behind experimental backend I don't see a reason to under-document it, even if we plan to make it more natural
w
sure. it’s likely fine to just deprecate it if we want to move it to 
fix
or 
mend
or whatever
but basically, right now we would not be advertising it as something that you should run on an ongoing basis.
h
Why would you not run it on an ongoing basis in CI? It makes sure that your developers don't accidentally use deprecated things, like if they didn't get the memo that 
python_library
is now 
python_sources
. And it ensures your BUILD files are formatted
Ack that it's clunky to have to have 
./pants update-build-files --check
in addition to `fmt`/`lint`! But that doesn't mean there's no utility, only that it's clunky
w
Yea, I didn't say there was no utility. Just that advertising it a bunch makes less sense if it's going to change a bunch in the next release.
And if in the next release, 
fmt
does BUILD files as well, people would be undoing those CI config edits.
(hypothetically: not time sensitive either)
h
people would be undoing those CI config edits.
Sure, but that's not A Huge Deal imo. I agree it would have been much better if we could have done this via 
fmt
and 
fix
right away, but we couldn't. So, in the meantime, recommend users do the useful thing even if it's a bit clunky and that clunkiness will soon* go away *Not sure on soon. Merging `validate`/`update-build-files`/`tailor` into `fmt`/`fix` is going to require lots of changes and very likely it wouldn't make it into 2.9
Open in Slack
PreviousNext
Powered by