Another bike shed :art: We're adding a new goal to...
# general
h
Another bike shed 🎨 We're adding a new goal to automate upgrading Pants versions by updating BUILD files to fix deprecations for you. Trying to come up with the name, currently between
./pants mend
vs
./pants serge
. Feedback welcomed! 🧡
At first, I thought of
fix
or
upgrade
. But I think they're too generic. This goal only is relevant to upgrading Pants versions, not some generic "fix" functionality
b
In sewing, one mends something that is damaged or weak, whereas one serges to add polished presentation, clean lines, strength at the edges, and preserve stretchiness.
And a mend never looks as good as the original or can be expected to be as durable. Whereas serging is done upfront, rather than as a repair.
h
My two cents: I suspect the majority of users don't know what "serging" is, nor the negative connotation of
mend
. Prefer a term that people can automatically understand, even if it's slightly inaccurate
βž• 1
h
Yeah, I think
mend
πŸ‘ 1
b
something like
self-update
?
b
I run a Slack for techies who craft (We So Crafty; DM me anyone who wants an invite) and mis-use of terminology is something that make folks cranky. So I'd say either accept that
mend
will be read forever by some people as "Pants damages stuff then makes you run a command to fix what it damages" or choose a neutral term that more accurately reflects what's happening.
h
"Pants damages stuff then makes you run a command to fix what it"
I mean, tbh, we sort of do with deprecations - deprecations are annoying for current users! We try to be careful only doing them when it will make the future a lot brighter for everyone, but yes, it's annoying / damages things
b
There's also a downside in leaning too far into analogies / puns / etc. with software naming. You tend to run through the obvious ones early on, and then it becomes increasingly difficult to find usable names within the "pun" domain. (I used to work at a company that was somewhat notorious for this). It also can become difficult for users to understand what a thing actually does if the name is relatively jargony from a completely unrelated field.
πŸ‘ 1
(saying this as a person that legitimately loves
pants tailor
πŸ˜… )
b
Yeah, I'm completely in favor of avoiding all clothing/sewing jargon. That's why I suggest a neutral term.
πŸ‘ 1
self-update
is such a common choice for package managers. If it's setting the correct expectation, +1 for that.
And it is free of the negative connotation of
mend
b
possibly
migrate
, depending on precisely what the command does
βž• 1
h
migrate
might be more accurate. Thus far, all it does is update some BUILD files. It does not change
pants.toml
, such as changing your
pants_version
- it's not 100% self-contained, you still have to do some manual things
b
I'd wanna be checking docs first to see if
migrate
is gonna move files around. But that's not a bad thing to check.
b
yeah, that's what I was thinking. If it messed around with
pants.toml
as well, then
self-update
might be better. That reads as a bit more self-contained and hands-off to me.
b
update
vs
self-update
?
b
maybe so πŸ€”
h
Trying to think if we'd ever get to a world where it's fully self-contained..a challenge is that many of the deprecations require you to make a choice, like how to configure an option in pants.toml. But I think we can go that direction! Make this goal be interactive: explain the decision and tradeoffs to you, and then ask you how you'd like to proceed. Always have an option to punt on the deprecation, like that we can even add it to
[GLOBAL].ignore_warnings
for you
b
(arguing against myself now, I'm perpetually mixing up
update
and
upgrade
whenever I deal with package managers, and I always end up picking the wrong one.)
πŸ’― 2
@hundreds-father-404 interactive is always cool
βž• 1
h
I definitely am a -1 for `update`/`upgrade`, too ambiguous if it's upgrading your own code/tools vs Pants itself. I like
self-upgrade
(or
self-update
) more
Any thoughts on
migrate
vs.
self-migrate
vs.
self-update
?
b
Migration is movement, so (a) and (b) seem like inaccurate connotations.
self-update
, yup
πŸ‘ 1
h
Cool, I like
self-update
a lot! I still think
./pants mend
is really cute and it sparks joy for me hehe, but you both raise good points
Thanks for the suggestion @brash-baker-91190 and for the feedback @busy-vase-39202!
πŸ‘ 2
b
Heh. That's the darn tricky thing about humor: no matter how effective for some people, it falls flat for some others. That's why I prefer clarity over jokes for commands.
βž• 1
b
πŸ‘ Yup, clarity wins at the end of the day. (sadly, as I love a good pun / dad joke) C'est la vie...
πŸ’― 2
βž• 2
b
Hahah our resident dad joke champ is @fast-nail-55400. And let's face it, Pants' name itself practically is one too. ("pants" = "python ant")
b
🀯
TIL... that is amazing
h
Woah that's awesome. I just realized the way I wrote this goal allows plugin authors to automate making arbitrary edits to BUILD files. All you have to do is write a rule that takes in
list[str]
(the original lines) and returns
list[str]
. Pants pipes your "fixer" on to any others, like how
./pants fmt
works cc @proud-dentist-22844
😎 1
p
migrate
βž•
self-update
I would expect that to update the
./pants
script and maybe bump the version.
self-update
should not be touching my config.
πŸ€” 1
You could do
./pants build-updates
though
h
Interesting, thanks Jacob. Also with the insight that this goal can be used by plugin authors for programmatically rewriting BUILD files...I'm not as certain anymore we want this 100% self-contained goal that pauses for your input and then updates pants.toml accordingly etc Good idea! What do you think of
update-build-files
? And then if/when we decide to do that fully self-contained experience, we can deprecate
update-build-files
in favor of
self-update
Which makes me wonder if
tailor
really should have been
generate-build-files
....(not too late to change that)
p
Eg consider
poetry self update
which upgrades poetry. https://python-poetry.org/docs/#updating-poetry
βž• 1
This is awesome. Could this be combined with tailor so that you have one goal for managing build files?
h
I'm wondering that exact same thing right now, but I'm thinking we want separate?
./pants tailor
means that you generate BUILD files where you didn't have them before, which means that you cause Pants to run on files it wasn't running on before Whereas this new goal is only meant to automate fixing BUILD files you already have
p
./pants uptailor
./pants tailor --update
Splitting it into
./pants generate-build-files
and
./pants update-build-files
Would be fine too. @busy-vase-39202 would renaming tailor be a good thing?
πŸ’― 1
h
./pants tailor --update
Possibly, although it does give me pause that some of the options would only be useful depending on which mode you're running in. For example,
--build-file-name
is only relevant to generating new BUILD files, so it's unused with
--update
. It's a smell to have to document "This option only is used if
--update
is also set"
βž• 1
I like
generate-build-files
more than
tailor
. It tells you exactly what it does, even though
tailor
is clever / fun
p
Maybe tailor can live on in the docs with the heading "Tailoring your BUILD files" with a section on generate and another on update
πŸ‘ 1
h
Interesting! I was just typing out about maybe calling it
upgrade-build-files
rather than
update-build-files
, with the logic that the goal is really only for upgrading and a one-time operation by codebase admins, not something you run every day But, it makes a ton of sense to run
./pants upgrade-build-files
in your CI! It avoids your org's developers unintentionally using deprecated functionality that you fixed but they re-add back
w
Would it be good to follow what Terraform does/did? They made specific scripts to perform the upgrade like "0.12upgrade" https://www.terraform.io/docs/cli/commands/0.12upgrade.html . Could have a temporary goal that only lives in the next version up or similar. I say that since the behavior of a "mend" may change from version to version and you might not want the code to live forever.
h
@worried-salesclerk-37834 that's how this started, actually! Pants 2.8 renames some targets like
python_library
to
python_sources
, which we knew we needed to automate fixing. I at first wrote a script that you manually download and run, then @happy-kitchen-89482 pointed out we have this amazing build tool designed precisely for situations like this Indeed, a lot of the "fixers" in this goal will only live in 1 or 2 Pants releases. After the deprecation is done, we'll delete that fixer from our code
w
I said "script" but I meant "goal". So "pants 2.7upgrade ::" or similar. Or maybe specific to the fix "pants topythonsources ::" Obviously the downsides ergonomics of typing that out and also having to have a specific upgrade path. But you're clear on what's being done and the behavior of a generic "mend" or "upgrade" goal then shouldn't change over time. And if you intend to remove the upgrade code at any point, you still need the upgrade path. And then it can be ambiguous.
(sorry while catching up I missed the last few messages which kind of suggest the same thing)
h
If we're going for clarity then
update-buildfiles
or similar is clearer than
self-update
, which sounds like we might be upgrading the Pants version
πŸ‘ 1
h
Agreed, I'm thinking
upgrade-build-files
. Somewhat arbitrary on
update
vs
upgrade
, but we're using upgrade now to talk about changing your Pants version. And https://fossbytes.com/whats-the-difference-between-update-and-upgrade/ claims that's the right word
@happy-kitchen-89482 what do you think about renaming
tailor
to
generate-build-files
for clarity and consistency?
h
Let's not worry about that for now
let's focus on the task at hand
I don't think
upgrade
is the right term here
we're not upgrading a version or anything like that, we're editing build files, they aren't versioned
h
I'm fine with
update-build-files
then, don't have a strong opinion there
h
Since you only run it once after upgrading the Pants version, it's OK if the name is clunky
βž• 1
h
(And possibly run in CI to check that teammates didn't make regressions)
c
fix-deprecations
?
πŸ‘ 1
h
Oh I like that even more I think! And then it does allow that future where we have an interactive flow for deprecations that require decision making
c
Yea, felt it was pretty close to what this command will do, fwict. Also, serge is not a word I knew, so looked like sarge to me πŸ˜‚ Upgrade would work too, if making it a bit more specific
upgrade-build-files
.
πŸ‘ 1
p
I like update-build-files better than fix-deprecations considering that I will use it on a regular basis to bulk edit build files with my own fixers.
πŸ‘ 2
h
I do like
fix-deprecations
because it will also allow in the future to fix things other than BUILD files, such as pants.toml
πŸ‘ 1
h
Oh Jacob, not sure this was obvious, but thanks to the rules API's design, you can do a bunch of cool things in your fixer like run a process or inspect a target's fields/dependencies The tricky part is going to be how to safely rewrite the BUILD file..I've been wondering about how to safely do things like remove a kwarg while not breaking trailing commas, commands, etc Which makes me realize this is how we can hook up Black to BUILD files!! Imo it would be better if that happened via
fmt
and
lint
, but that really really messes with the Target API and has super awkward edges. I tried adding it last month and it was going to take a lot of big redesigns to do. Instead, I can probably wire up Black to update-build-files in an hour this weekend :)
@happy-kitchen-89482 if we use this goal to run Black, I think update-build-files makes more sense? Again, not optimal to not use fmt, but trying to be pragmatic about how to unblock a huge win from fundamental changes we don't have the time for We can add --update-build-files-check for lint mode (like --tailor-check that I want to add)
h
There is a big semantic difference between "~heuristically updating BUILD files due to deprecations via regex" and running a reliable formatter that operates on the AST. In that the former requires manual verification and the latter does not.
And that the former happens once per upgrade and the latter happens all the time
But 🀷
It sounds like to do this ~safely we'll have to somehow run black anyway
So it may make sense to just do that
h
In my head, the story of the update-build-files goal is:
BUILD files are annoying and I don't want to deal with them, which I usually don't thanks to dep inference
. When I do have to, I want to make sure they're in tip-top shape. That they're using the newest idioms and pretty formatted. Another thing to consider with black is that you always have BUILD files but might not have Python in your repo or might not want to use Pants for it. That is, you might want to format BUILD files without activating pants.backend.Python, which would have consequences like ./pants tailor now insisting on creating Python targets for code you don't want
c
There'll still be room for a fix-deprecations in the future, that can call update-build-files as one of the steps..
βž• 1
h
Agreed! I'm thinking now that it's actually really useful for
update-build-files
to not be interactive, especially so that you can safely run it in CI
fix-deprecations
(or
self-update
) would need to be interactive, so I think having both goals could make sense in the future
πŸ‘ 2
FYI followup to rename
tailor
to `generate-build-files`: https://github.com/pantsbuild/pants/pull/13302 Feedback/reviews welcomed!
c
Oh, too bad πŸ˜• I do see the motivation, and can agree with it (was just a bit fond of the
tailor
name…) πŸ‘
h
Still open for discussion, of course! We could choose to stick with
tailor
. This is only meant to prompt that discussion
πŸ‘ 1
c
There is good symmetry with
update-build-files
and
generate-build-files
.. almost so, that it feels that they could be merged into one and the same?
Also, as
tailor
modifies BUILD files in place, the
generate
name doesn’t really convey that, feels like it risks replacing if there is pre-existing files..
where as
update
is very clear on that matter.
h
feels like it risks replacing if there is pre-existing files..
The goal only adds new targets, does not modify any existing ones
c
Yeah, was referring to the BUILD files themselves… as
generate-build-files
feels like it works on them as an atomic unit…
πŸ‘ 1
The goal only adds new targets, does not modify any existing ones
Am aware, just trying to point out what I feel the name may imply, if I didn’t know this.. πŸ™‚
πŸ‘ 1
h
add-target-definitions? add-missing-targets?
h
Yeah I had similar thoughts, on the PR
Let's discuss there?
πŸ‘€ 1
f
./pants build-Xtreme
πŸ˜‚ 2
h
tl;dr from the issue: for now, we should indeed stick with
tailor
! We realized we should merge
tailor
and
update-build-files
into one goal, and that that goal also should have the option to automate safe deprecations in
pants.toml
(only safe ones)
πŸ‘ 3