Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-52

Packages should list all direct dependencies in their ups table files

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None
    • Location:
      this ticket

      Description

      We have been allowing developers to list as many or as few direct dependencies as they wish in the ups file of a package, as long as the chain of dependencies includes everything needed. I propose that we recommend that every package list all direct dependencies in the ups file because:

      • It is more robust against changes in dependent packages. If package A depends on B and C, and B depends on C, then it is sufficient for A to only list B as a dependency. But if B is later changed to not rely on C then this breaks A in a way that is surprising.
      • It makes the dependencies of the package explicit to readers of the code.
      • It reduces the need to understand the dependency tree of dependent packages.

      This is clearly a contentious issue so I am directly assigning it to K-T

        Attachments

          Issue Links

            Activity

            Hide
            ktl Kian-Tat Lim added a comment -

            There has been some recent discussion of this RFC in HipChat. I adopted it on 2015-06-10, based partly on " I don't care enough about this to carry the fight further" and "but I'm not going to try to block this RFC because of it" and because I thought (and still think) that transparency of dependencies and which ones need to be specified is important both for programs and for people. I regret that it has not been implemented by incorporation into the developer documents; I will try (after this week) to find an appropriate place for it.

            As stated before, the issue with "umbrella" packages like pipe_tasks or afw is that it is difficult to determine whether a given "import lsst.x" statement in Python is legal or not based solely on a "setupRequired()" of the umbrella package in the table file. That information has to come from external documentation somewhere or a search through multiple potential locations, rather than a single, package-local, machine- and human-readable location.

            I think the current state of our packaging is such that (direct) build-only and test-only dependencies are appropriate to mention; this distinguishes "standard" LSST packages from other packages (usually third-party ones) that do not require scons and sconsUtils, and a small amount of boilerplate does not seem highly objectionable. We need a better mechanism for handling system dependencies, but that is a separate RFC.

            Show
            ktl Kian-Tat Lim added a comment - There has been some recent discussion of this RFC in HipChat. I adopted it on 2015-06-10, based partly on " I don't care enough about this to carry the fight further" and "but I'm not going to try to block this RFC because of it" and because I thought (and still think) that transparency of dependencies and which ones need to be specified is important both for programs and for people. I regret that it has not been implemented by incorporation into the developer documents; I will try (after this week) to find an appropriate place for it. As stated before, the issue with "umbrella" packages like pipe_tasks or afw is that it is difficult to determine whether a given " import lsst.x " statement in Python is legal or not based solely on a " setupRequired() " of the umbrella package in the table file. That information has to come from external documentation somewhere or a search through multiple potential locations, rather than a single, package-local, machine- and human-readable location. I think the current state of our packaging is such that (direct) build-only and test-only dependencies are appropriate to mention; this distinguishes "standard" LSST packages from other packages (usually third-party ones) that do not require scons and sconsUtils , and a small amount of boilerplate does not seem highly objectionable. We need a better mechanism for handling system dependencies, but that is a separate RFC.
            Hide
            rhl Robert Lupton added a comment -

            This RFC seems to me to be a result of misunderstanding our tools, and I therefore feel that it's a poor decision, but it is taken and I won't try to reverse it.

            Firstly, you almost never need to know whether a given import lsst.x statement is legal, I certainly never consult the table file to ascertain this.

            Secondly, if you do need to know the dependencies of the package you are working on,

            eups list --dependencies -r .

            will tell you; but again I never need to do this. If we are worried about dependencies for packaging, once again we should be using the tools that understand table files rather than adding extra entries that do not actually change anything.

            Show
            rhl Robert Lupton added a comment - This RFC seems to me to be a result of misunderstanding our tools, and I therefore feel that it's a poor decision, but it is taken and I won't try to reverse it. Firstly, you almost never need to know whether a given import lsst.x statement is legal, I certainly never consult the table file to ascertain this. Secondly, if you do need to know the dependencies of the package you are working on, eups list --dependencies -r . will tell you; but again I never need to do this. If we are worried about dependencies for packaging, once again we should be using the tools that understand table files rather than adding extra entries that do not actually change anything.
            Hide
            mjuric Mario Juric added a comment -

            Robert, I think there's a misunderstanding of the problem – take look at the original request, this is about making the stack more robust against a situation where A explicitly uses modules from B and C, but only declares the dependence on B. As B has an internal dependence on C, everything seems to work, until, say, B is rewritten not to use C upon which the build of A breaks. To avoid that, each package should declare all things it explicitly depends on.

            I believe this is a relatively common practice, whether with include files or libraries or packages.

            Show
            mjuric Mario Juric added a comment - Robert, I think there's a misunderstanding of the problem – take look at the original request, this is about making the stack more robust against a situation where A explicitly uses modules from B and C, but only declares the dependence on B. As B has an internal dependence on C, everything seems to work, until, say, B is rewritten not to use C upon which the build of A breaks. To avoid that, each package should declare all things it explicitly depends on. I believe this is a relatively common practice, whether with include files or libraries or packages.
            Hide
            tjenness Tim Jenness added a comment -

            This RFC is hanging around because of DM-7140 – it would seem that documenting this should be pretty quick.

            Show
            tjenness Tim Jenness added a comment - This RFC is hanging around because of DM-7140 – it would seem that documenting this should be pretty quick.
            Hide
            jsick Jonathan Sick added a comment -

            This RFC has been implemented in https://github.com/lsst/templates/tree/master/project_templates/stack_package (and refined as part of DM-17155).

            Show
            jsick Jonathan Sick added a comment - This RFC has been implemented in https://github.com/lsst/templates/tree/master/project_templates/stack_package  (and refined as part of DM-17155 ).

              People

              • Assignee:
                ktl Kian-Tat Lim
                Reporter:
                rowen Russell Owen
                Watchers:
                Gregory Dubois-Felsmann, Jim Bosch, John Swinbank, Jonathan Sick, Joshua Hoblitt, Kian-Tat Lim, Mario Juric, Robert Lupton, Russell Owen, Simon Krughoff, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                11 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Planned End:

                  Summary Panel