https://pantsbuild.org/ logo
Powered by
#general
  • b

    bored-art-40741

    04/19/2016, 6:30 PM
    Not sure what you mean
  • b

    bored-art-40741

    04/19/2016, 6:30 PM
    Oh, just set it to false?
  • b

    bored-art-40741

    04/19/2016, 6:31 PM
    Yeah I guess we could just get rid of the option entirely and instead set 
    cache_target_dirs
    to 
    False
    . But then we'd have to split back out the implementation of 
    _should_cache
    too
  • w

    witty-crayon-22786

    04/19/2016, 6:33 PM
    @bored-art-40741: i mean that it might not be a boolean option
  • w

    witty-crayon-22786

    04/19/2016, 6:33 PM
    or perhaps there are two options: "create_target_dir" vs "cache_target_dir"
  • b

    bored-art-40741

    04/19/2016, 6:35 PM
    You don't need two, I don't think
  • b

    bored-art-40741

    04/19/2016, 6:35 PM
    We could in fact just flip 
    cache_target_dirs
    to 
    False
    in 
    jvm_compile
    and ditch the extra option
  • w

    witty-crayon-22786

    04/19/2016, 6:37 PM
    @bored-art-40741: then you'd need to actually create the target dirs.
  • w

    witty-crayon-22786

    04/19/2016, 6:37 PM
    which is non-trivial.
  • b

    bored-art-40741

    04/19/2016, 6:38 PM
    Ah, I see
  • w

    witty-crayon-22786

    04/19/2016, 6:38 PM
    the target_dirs are where the "native" incremental support is now
  • b

    bored-art-40741

    04/19/2016, 6:39 PM
    Okay, I'm going to vote leaving this as it is now then--it swaps out one unused option for a more specific one that gets the job done and no more
  • w

    witty-crayon-22786

    04/19/2016, 6:39 PM
    @bored-art-40741: what about having a 
    vts.cache()
    method or something?
  • b

    bored-art-40741

    04/19/2016, 6:39 PM
    If we ever have another user of this functionality we can think about extending the API
  • w

    witty-crayon-22786

    04/19/2016, 6:39 PM
    or 
    vts.update(cache=bool)
  • b

    bored-art-40741

    04/19/2016, 6:39 PM
    That won't work
  • b

    bored-art-40741

    04/19/2016, 6:39 PM
    Not without a fair amount of threading
  • b

    bored-art-40741

    04/19/2016, 6:40 PM
    The VTS needs to know about the 
    cache
    , which is generated by the 
    Task
    instance
  • b

    bored-art-40741

    04/19/2016, 6:40 PM
    Based on options and such
  • w

    witty-crayon-22786

    04/19/2016, 6:40 PM
    it would be more uniform, because the same way that manually calling 
    vts.update
    is optional, pre-writing to the cache would be optional
  • w

    witty-crayon-22786

    04/19/2016, 6:41 PM
    and you wouldn't multi-write, because it would only write if the vts wasn't already up-to-date
  • b

    bored-art-40741

    04/19/2016, 6:41 PM
    Sure, but it would be a much larger change than I'm particularly comfortable making here
  • w

    witty-crayon-22786

    04/19/2016, 6:41 PM
    actually... what about just doing that here? only write to the cache if the vts isn't already 'up to date'
  • w

    witty-crayon-22786

    04/19/2016, 6:41 PM
    and drop the 
    invalidated
    param
  • b

    bored-art-40741

    04/19/2016, 6:41 PM
    I wanted to keep the scope of this to the only existing user of this functionality: zinc compilation. We really shouldn't be encouraging other users to cache by hand
  • b

    bored-art-40741

    04/19/2016, 6:41 PM
    I don't understand how that helps, or even really what you mean
  • w

    witty-crayon-22786

    04/19/2016, 6:42 PM
    i was pretty happy to kill all the special cases in zinc =P
  • w

    witty-crayon-22786

    04/19/2016, 6:42 PM
    so re-introducing them carefully would be nice
  • b

    bored-art-40741

    04/19/2016, 6:43 PM
    I still don't see how what you're suggesting helps
  • w

    witty-crayon-22786

    04/19/2016, 6:43 PM
    anyway... you have the shipit. but would be good to make it more uniform with the manual 
    update
    call somehow
1...117611771178...2420Latest