https://pantsbuild.org/ logo
Join Slack
Powered by
<@U06A03HV1> starting cherry picks to 1.26.x for a...
# announce
e
@witty-crayon-22786 starting cherry picks to 1.26.x for an rc1 with tokio compat and notify.
w
soo, it took until today for me to realize that y’all are going to need https://github.com/pantsbuild/pants/pull/9636 if you want to safely disable watchman… otherwise notify won’t watch invalidation globs
i just stripped out a few things to try and make it a bit easier to cherry pick
e
Oh I see. Thanks
I’m going to review it today
Will we also need to cherry-pick the PR that moves the python side engine files around?
w
…
not if you can avoid it
it shouldn’t have relevant changes in it. but it’s going to make it messy
e
I’d prefer not to but some of the changes in the PR touch moved files.
I guess I can manually apply them to the previous file name, or maybe git is smart enough to know they moved.
w
*fingers crossed
e
Yeah…w/r/t the unifying retries PR do you think that is skippable as well?
w
for processes? i’d be surprised if that collided…
e
Just because I think we changed some stuff in the scheduler to unify retries of uncacheable nodes and roots, I was thinking that might collide with the scheduler changes for the invalidation handled in the engine thing.
hopefully not though
w
oh, that. yea, that probably will as well.
e
I’m happy with the way the invalidation globs PR looks, do you have plans to land it soon?
w
ah, yes… had been waiting on reviews from Greg and John, but can let them review post-facto.
sorry about that.
merging.
e
Okay thanks! got everything up to there cherry picked
w
merged
@early-needle-54791: so, subtext on https://github.com/pantsbuild/pants/pull/9754 is that i have a suspicion that i actually observed that hang, although i haven’t reproed it to confirm.
you’ll probably want to pick it.
e
Im suprised the change surface area is so small.
I thought the raw event api was much different
But thanks, should be an easy pick
w
it could be larger… i think that we don’t strictly need the channel anymore
but wanted to avoid that here since it didn’t feel like a strictly necessary change.
e
appreciate that.
I assume you have tested manually with the new version. I remember there was much more event fidelity with the immediate watcher
But sounds like that shouldn’t be a problem, based on your description in the PR
We are planning to hot fix the notify changes soon so the sooner it lands the sooner I can pick.
(obviously wait for ci though)
actually I’ll just pick from your PR.
w
pushed a test-only fix
e
saw it, and picked it
A random go contrib test is failing now.
w
yea, reproducibly it looks like. investigating after figuring out my new laptop. sigh.
e
K
w
phew, the last test failure also looks like a bug in the code under test rather than in the filesystem (you can no longer get away with creating a file and then hoping pants hasn’t noticed it yet a few lines later!)
e
Okay, phew indeed
Is the fix simple enough?
w
yes!
or, easily explained.
e
K. I’m somewhat surprised I’m seeing the same error on the cherry-pick branch because notify is disabled by default. Maybe just the polling for changed files through the engine sped up invalidation enough
w
it’s not disabled by default, afaik
it was very briefly disabled by default in master, but you fixed that
e
Im the 1.25.x-twtr branch it is still disabled by default
I’m fairly certain.
oops I’m wrong 🙂
I should change that
w
e
I don’t think thats strictly necessary to cherry pick to our branch
w
maybe not. causes a failure if someone does the equivalent of 
./pants $goal *
… ie, expanding a glob near the root.
2 Views
Open in Slack
PreviousNext
Powered by