I have a draft PR (https://github.com/pantsbuild/pants/pull/10433) that tries to fix this by making the

cacheable

method on

Node

depend on

Node::Item

, i.e. making its signature

fn cacheable(&self, result: Option<&Self::Item>) -> bool;

@witty-crayon-22786 the code seems to compile without changing the signature of

cacheable

to remove the

self

, although it's possible we don't need that anyway

we use

cacheable

in more places in

graph

htan I thought we did

so, here’s the thing.

hm. ok, this will take some thinking. sorry, can respond in a bit

or tomorrow if you're heading off for the day

to avoid forgetting to call both, you could add a method to

Entry

in

<http://entry.rs|entry.rs>

that would call both on

&self

the thing about having one cacheable method that takes both self and the item is that it isn’t strictly necessary for

self

to match the item (ie, it doesn’t matter that self is a ProcessRequest and the item is a ProcessResult… you use self and the item for independent checks, basically)

but minor.

or rather, that it will depend on that with my change?

I'm not sure it matters much whether where's a separate `cacheable_item`method or that code is part of the

cacheable

implementation, I think it could go either way

I just don't have the solution in the draft PR working yet, and I think that's because in the several call sites of

Node::cacheable

, we don't necessarily have the node output value availalable. at least right now with that version of the code the bug in pantsd is not fixed

lemme know if you want me to take a look at the draft