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

    witty-crayon-22786

    03/22/2016, 7:21 PM
    they don't let you address the duped class, i understand
  • j

    jolly-chef-92794

    03/22/2016, 7:23 PM
    What if I made the 
    transitive
    argument to 
    target()
    “private”, ie rename to 
    _transitive
    , and make a new 
    intransitive(spec)
    alias equivalent to 
    scoped(spec, transitive=False)
    as the preferred way to define transitive deps?
  • w

    witty-crayon-22786

    03/22/2016, 7:24 PM
    that would be an improvement, yes. but unlike scope, transitivity does seem like an edge property
  • w

    witty-crayon-22786

    03/22/2016, 7:24 PM
    ya know?
  • w

    witty-crayon-22786

    03/22/2016, 7:25 PM
    so: scope on target? absolutely!
  • w

    witty-crayon-22786

    03/22/2016, 7:25 PM
    transitivity on target... very unclear what it means.
  • j

    jolly-chef-92794

    03/22/2016, 7:26 PM
    I agree that transitivity is more an edge property than a vertex property, however it’s easier to handle behind the scenes as a property on an intermediate dependency, since we’re already using scopes that way and they benefit from similar logic
  • j

    jolly-chef-92794

    03/22/2016, 7:27 PM
    We can hide that from users with an 
    intransitive()
    alias
  • w

    witty-crayon-22786

    03/22/2016, 7:27 PM
    well... in that case, making 
    _transitive
    private/experimental might be alright if it's a thing that can be evolved.
  • j

    jolly-chef-92794

    03/22/2016, 7:28 PM
    As long as we don’t change the user-facing api of using 
    intransitive()
    , we’d be free to completely change how it was implemented under the covers if it seemed expedient
  • w

    witty-crayon-22786

    03/22/2016, 7:29 PM
    hm. then i think it's even more a question of whether the intransitive deps should be a in a separate list
  • j

    jolly-chef-92794

    03/22/2016, 7:30 PM
    My hesitance with that is classpath ordering related
  • w

    witty-crayon-22786

    03/22/2016, 7:31 PM
    could require the target to be in both lists?
  • j

    jolly-chef-92794

    03/22/2016, 7:31 PM
    With the 
    scoped_dependencies=[]
    list I proposed originally, we could get around the ordering problem by just putting all dependencies in the 
    scoped_dependencies
    list, but giving most of them the default scope
  • j

    jolly-chef-92794

    03/22/2016, 7:33 PM
    That makes it harder on the implementation side. The easiest/cleanest way to implement it would be just to concatenate all the entries in 
    intransitive_dependencies
    to 
    dependencies
    using Target's 
    traversable_dependency_specs
    property. Which has the advantage of not having to change all the existing dependency graph computation
  • j

    jolly-chef-92794

    03/22/2016, 7:34 PM
    That’s why I ended up having to rewrite the search function used by 
    Target.closure
    from scratch in the first version of this patch, but when I switched from 
    scoped_dependencies
    to putting the scope and intransitive property on the Target, I was able to use the existing code
  • w

    witty-crayon-22786

    03/22/2016, 7:34 PM
    yea.
  • j

    jolly-chef-92794

    03/22/2016, 7:34 PM
    Which was good because it turned out to be tricky to perfectly handle all the cases of bfs vs postorder dfs vs preorder dfs
  • w

    witty-crayon-22786

    03/22/2016, 7:35 PM
    alright. how about private 
    _transitive
    , and explicitly marking the 
    intransitive
    wrapper experimental?
  • j

    jolly-chef-92794

    03/22/2016, 7:35 PM
    Sound good to me
  • w

    witty-crayon-22786

    03/22/2016, 7:37 PM
    @jolly-chef-92794: where did you see issues with 
    respect_intransitive
    ? it seems like in any reasonable case it should be safe to apply
  • w

    witty-crayon-22786

    03/22/2016, 7:38 PM
    ...in the sense that if the dep is being included transitively in some other path, it still comes in
  • j

    jolly-chef-92794

    03/22/2016, 7:39 PM
    Most targets don’t want to respect intransitive currently
  • j

    jolly-chef-92794

    03/22/2016, 7:40 PM
    If you have a java library 
    A
    that intransitively depends on java library 
    B
    that intransitively depends on java library 
    C
    ,
  • j

    jolly-chef-92794

    03/22/2016, 7:40 PM
    The zinc compiler will first compile 
    B
    with 
    B
    as the root when computing the closure, so it will correctly get 
    C
  • j

    jolly-chef-92794

    03/22/2016, 7:40 PM
    And then compile 
    A
    with 
    A
    as the root, so it will correctly get 
    B
    but not 
    C
    , when respect_intransitive is True
  • j

    jolly-chef-92794

    03/22/2016, 7:40 PM
    which is all well and good
  • j

    jolly-chef-92794

    03/22/2016, 7:41 PM
    However, if you’ve just asked to compile 
    A
    , the ivy resolve needs to know that it should get everything A transitively depends on
  • j

    jolly-chef-92794

    03/22/2016, 7:41 PM
    Else we might not resolve jars that 
    C
    needs
  • w

    witty-crayon-22786

    03/22/2016, 7:41 PM
    ah.
1...108110821083...2420Latest