yeah.

does your branch kill the idea of (capital P)

Platform

?

in favor of a singular

PythonIdentity

?

might make sense to conjoin them - then point to an existing interpreter if available/needed

then iff sdist execution is required, error out if a local interpreter is not found - else gleefully resolve bdists ad nauseum since the spec is all thats needed there

that would make great sense to me

that could improve the whole cross platform “translation” opaqueness too

separately from the internal release, my goals right now are to get 6022 (pydist tag build) and 5998 (conan) mergeable and both happen to be failing with the error described in 6099 (and i suspect this may at least give insight into what the remaining osx shard failure on 6022 was, if not fix it) so let me know if i can help out on any of the above without being one cook too many because i can justify spending a thursday on it (but not a blocker right now, so no rush)

ah, didn’t realize it was affecting more diffs than just those two

@witty-crayon-22786 I’ve been having fun with https://github.com/pantsbuild/pants/pull/6095. You raise a good point - I think that dirtying is technically safe (because happens to only be done under the graph lock), but this isn’t guaranteed by the type system. I think the way to fix that, if we wanted to, is to take the dirty bit off of

EntryState

, and put dirty `EntryId`s into an

Arc<RwLock<HashSet<EntryId>>>

which is shared between the

Graph

and each

Entry

, so that when batch invalidations happen,

Graph

can just graph the lock and invalidate them all, but when `Node`s complete, they can try for the lock as they need in an ad-hoc way… I’ve just hacked that together, and it seems to work… How does it sound to you?

It opens up deadlock possibilities, as

Entry

will have to acquire both its

EntryState

mutex and its dirty-set mutex, which isn’t ideal…

in a meeting... in 30 minutes

No worries 🙂

@average-vr-56795: and the goal there is to make it type safe, yea?

i ... am ... i think ok with violating typesafety for the benefit of code clarity -temporarily- (edit: who knows how temporary it would be), given a sufficiently large comment?

if it doesn't affect clarity, then that's probably fine too.

@enough-analyst-54434: are you going to have time to continue working on the "PythonIdentity" change?

CI is currently not useful, so i might ask Kris to land a temporary fix if need be

Yeah. Haven't worked the last week but working today and tomorrow.

great, thank you!

Yeah - he should not have cancelled his PR, this is a 3 part fix. 1st staunching is in pants, then pex, then pick up new pex in pants.

I'll resurrect a more succinct version of his PR presently

ok, thank you.

@witty-crayon-22786 https://github.com/pantsbuild/pants/pull/6095/commits/5fb71da8dec80031b2817330c5b0560b33dbdac0 is roughly what removing

dirty

from

EntryState

would look like

I’m not convinced it actually ends up being more safe, or clear 😞 I was excited to be using rust for some serious concurrency stuff, which is meant to be its wheelhouse, but it feels like the moment you actually have higher level races than guarding against non-atomic writes, it doesn’t actually help that much 😞

imo, let's put large comments on both

invalidate

and

complete

about the invariant, and leave it as is in the PR

(ie, rely on

complete

happening under the lock)

... alternatively, you could make it type safe in an awkward way by requiring a mutable reference to the graph in

Entry::complete

?

@average-vr-56795: er, so... talk to me about why we need to be under the lock here

a dependent must have been marked dirty if it's dependencies have been