ancient-vegetable-10556
10/21/2021, 5:38 PMBUILD
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?witty-crayon-22786
10/21/2021, 5:40 PMupdate-build-files
goal, apparently cc @hundreds-father-404ancient-vegetable-10556
10/21/2021, 5:41 PM./pants fmt
too?hundreds-father-404
10/21/2021, 5:49 PMupdate-build-files
I can update the output to say "Run update-build-files
to fix these issues"?ancient-vegetable-10556
10/21/2021, 5:49 PMhundreds-father-404
10/21/2021, 5:50 PMShould 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/13279ancient-vegetable-10556
10/21/2021, 5:52 PMhundreds-father-404
10/21/2021, 5:54 PMfmt
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 thereancient-vegetable-10556
10/21/2021, 5:55 PMwitty-crayon-22786
10/21/2021, 6:04 PMfmt
might eventually allow us to adjust those semantics. i don’t expect much of a slippery slope there.hundreds-father-404
10/21/2021, 6:29 PMcore/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 codewitty-crayon-22786
10/21/2021, 6:30 PMtailor
doesupdate-build-files
in the release notes to fix deprecations)hundreds-father-404
10/21/2021, 6:35 PMtailor doesBy 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
witty-crayon-22786
10/21/2021, 6:37 PMfmt
, and so probably better in a separate goal like mend
or fix
hundreds-father-404
10/21/2021, 6:38 PMbut i don’t think that this is time critical for 2.8.xAgreed, 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 fixKey difference is pyupgrade runs on your own code, not Pants config files
witty-crayon-22786
10/21/2021, 6:39 PMDoing it viathis 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 reviewwas an easy win for a feature users have been asking about for months. If folks feel strongly that it shouldn’t live inupdate-build-files
, I can revert that PR? But won’t have time to do it withupdate-build-files
fmt
hundreds-father-404
10/21/2021, 6:42 PMwhile 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 reviewIf 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 quirkbut i don’t feel strongly about itI 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 constraintswitty-crayon-22786
10/21/2021, 6:45 PMvalidate
is an odd goal out right now, because it overlaps semantically with other goals. it exposes an implementation detail that it is a separate goalhundreds-father-404
10/21/2021, 6:50 PM./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?witty-crayon-22786
10/21/2021, 6:52 PM./pants fix
say… i would expect that based on the name, but perhaps that’s because i’m familiar with cargo fix
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)hundreds-father-404
10/21/2021, 6:54 PMwitty-crayon-22786
10/21/2021, 6:54 PM./pants fix
to do that though?fmt
, but something called fix
…?go
, rust
, and scala
all have “fix” tools (from go’s ancestry, i think)hundreds-father-404
10/21/2021, 7:00 PM./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!witty-crayon-22786
10/21/2021, 7:04 PMOption A: a goal like “`update-pants-config`” which does those 3 things, only on Pants’ config.,fmt
, and maybelint
are only for your own codefix
fix
to also operate on BUILD files?hundreds-father-404
10/21/2021, 7:07 PMfix
rather than fmt
changes my answer. I think it probably does make sense that fmt
formats everythingvalidate
, 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?fix
goal before we stabilize Pants 2.8 and Pyupgrade + Autoflake using fmt
..)witty-crayon-22786
10/21/2021, 7:11 PMupdate-build-files
as for being used for anything other than the upgrade, we’re fine for 2.8.xhundreds-father-404
10/21/2021, 7:14 PMwitty-crayon-22786
10/21/2021, 7:15 PMfix
or mend
or whateverhundreds-father-404
10/21/2021, 7:17 PMpython_library
is now python_sources
. And it ensures your BUILD files are formatted./pants update-build-files --check
in addition to `fmt`/`lint`! But that doesn't mean there's no utility, only that it's clunkywitty-crayon-22786
10/21/2021, 7:19 PMfmt
does BUILD files as well, people would be undoing those CI config edits.hundreds-father-404
10/21/2021, 7:22 PMpeople 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