https://pantsbuild.org/ logo
Powered by
#development
  • w

    witty-crayon-22786

    05/10/2018, 5:05 PM
    hm... i think 
    wire
    is an alternative protoc compiler.
  • a

    average-vr-56795

    05/10/2018, 5:07 PM
    How horrendous would it be having the 
    wire
    rule implementation manually look at its raw arg, rather than the snapshot, to construct its command line?
  • w

    witty-crayon-22786

    05/10/2018, 5:07 PM
    ...not. at all.
  • a

    average-vr-56795

    05/10/2018, 5:08 PM
    This feels like a nice simplification, unless I’m missing something big…
  • w

    witty-crayon-22786

    05/10/2018, 5:08 PM
    yea, me too. i think that that was the primary case in which i wanted to preserve ordering.
  • a

    average-vr-56795

    05/10/2018, 5:09 PM
    I didn’t run into any other cases when sorting the file lists of all of twitter’s, and all of the pants repo’s, BUILD files
  • w

    witty-crayon-22786

    05/10/2018, 5:09 PM
    ...hah
  • w

    witty-crayon-22786

    05/10/2018, 5:09 PM
    alrighty then.
  • w

    witty-crayon-22786

    05/10/2018, 5:10 PM
    ok, big queue of potential simplifications here.
  • w

    witty-crayon-22786

    05/10/2018, 5:10 PM
    well, simplification first, then optimization.
  • a

    average-vr-56795

    05/10/2018, 5:10 PM
    In that case I will definitely do the digest of 
    Directory
    thing, rather than the lifting of 
    Snapshot
    thing, and we can follow up with the simplifying of `Snapshot`s whenever we find time
  • w

    witty-crayon-22786

    05/10/2018, 5:11 PM
    i could try removing ordering today to unblock you to do other things tomorrow?
  • a

    average-vr-56795

    05/10/2018, 5:12 PM
    I wouldn’t complain, but I’m happy to just use 
    Digest
    of 
    Directory
    as my fundamental unit of files, and have a 
    project to Snapshot
    intrinsic so that `Snapshot`s are only ever produced to be consumed, and never to be passed back into a rule
  • w

    witty-crayon-22786

    05/10/2018, 5:12 PM
    yep, makes sense.
  • a

    average-vr-56795

    05/10/2018, 5:13 PM
    Removing ordering just allows us push the 
    project Directory to Snapshot
    out to a later stage where we’re currently doing it eagerly
  • w

    witty-crayon-22786

    05/10/2018, 5:13 PM
    but... i feel like going "`Digest` only" to represent Snapshots might be blocked on ordering?
  • w

    witty-crayon-22786

    05/10/2018, 5:13 PM
    because if at any point the Snapshot object doesn't exist, you lose the PathStats and the ordering.
  • a

    average-vr-56795

    05/10/2018, 5:14 PM
    That’s fine - once you cross into a remote execution world, I’m happy for files to just be a 
    Directory
    not a 
    Snapshot
  • a

    average-vr-56795

    05/10/2018, 5:14 PM
    And for us to give you back a (consistent) arbitrary order if you project that into a 
    Snapshot
  • a

    average-vr-56795

    05/10/2018, 5:15 PM
    We’re basically saying: If you care about ordering of files in a 
    Directory
    , keep that out of band
  • w

    witty-crayon-22786

    05/10/2018, 5:16 PM
    hm. i think that unless you special case handling somewhere, you're going to lose ordering between capturing a Snapshot and consuming if you don't actually materialize the Snapshot object
  • w

    witty-crayon-22786

    05/10/2018, 5:16 PM
    for sources globs
  • w

    witty-crayon-22786

    05/10/2018, 5:16 PM
    basically, if the intrinsic becomes 
    PathGlobs -> Digest
    , then you've lost ordering
  • w

    witty-crayon-22786

    05/10/2018, 5:17 PM
    (with a separate intrinsic for 
    Digest -> Snapshot
    )
  • a

    average-vr-56795

    05/10/2018, 5:18 PM
    My plan isn’t to change those intrinsics (that’s a future problem), but to make the remote execution stuff consume a 
    Digest
    not a 
    Snapshot
    - so if you already have a 
    Snapshot
    you just grab the 
    Digest
    field, and if you are capturing from a 
    PathGlobs
    you’re going to go via a 
    Snapshot
    , but if you happen to have a 
    Digest
    we don’t need to expand it
  • w

    witty-crayon-22786

    05/10/2018, 5:19 PM
    oh. ok
  • w

    witty-crayon-22786

    05/10/2018, 5:19 PM
    yea, fire away.
  • a

    average-vr-56795

    05/10/2018, 5:19 PM
    i.e. you can go 
    PathGlobs -> Snapshot -> Digest
    or 
    ExecuteProcessResult -> Digest
  • a

    average-vr-56795

    05/10/2018, 5:19 PM
    Cool, I’ll put that together tomorrow 🙂 Thanks, this has been a really helpful discussion!
  • w

    witty-crayon-22786

    05/10/2018, 5:20 PM
    i'll create a ticket for removing ordering from Snapshot
1...484485486...906Latest