kumarshantanu / lein-sub

Leiningen plugin for executing tasks on sub-projects
67 stars 10 forks source link

Transitive dependency support #11

Open ilya-pi opened 10 years ago

ilya-pi commented 10 years ago

Hi Kumar,

Neat plugin — thank you!

We are using it in a rather big project with transitive dependencies, and we though it would be nice if lein-sub would've resolved them out-of-the box in a more or less automatic manner.

I've adjusted subtest to represent this situation.

So, dependency tree looks the following way:

common-child -> []
child -> [common-child]
child2 -> [common-child]
child3 -> [child2]

and once you run lein sub install in \subtest it will give you output like:

bash-3.2$ lein sub install
Building project  [subtest/common-child v0.3.0-1-g4ddbb83]
Created/lein-sub/subtest/common-child/target/provided/common-child-v0.3.0-1-g4ddbb83.jar
Wrote/lein-sub/subtest/common-child/pom.xml
Building project  [subtest/child v0.3.0-1-g4ddbb83]
Created/lein-sub/subtest/child/target/provided/child-v0.3.0-1-g4ddbb83.jar
Wrote/lein-sub/subtest/child/pom.xml
Building project  [subtest/child2 v0.3.0-1-g4ddbb83]
Created/lein-sub/subtest/child2/target/provided/child2-v0.3.0-1-g4ddbb83.jar
Wrote/lein-sub/subtest/child2/pom.xml
Building project  [subtest/child3 v0.3.0-1-g4ddbb83]
Created/lein-sub/subtest/child3/target/provided/child3-v0.3.0-1-g4ddbb83.jar
Wrote/lein-sub/subtest/child3/pom.xml

Might you make dependency tree as:

common-child -> []
child -> [common-child]
child2 -> [common-child, child2]
child3 -> [child2]

It will go like:

bash-3.2$ lein sub install
Building project  [subtest/common-child v0.3.0-1-g4ddbb83]
Created/lein-sub/subtest/common-child/target/provided/common-child-v0.3.0-1-g4ddbb83.jar
Wrote/lein-sub/subtest/common-child/pom.xml
Building project  [subtest/child v0.3.0-1-g4ddbb83]
Created/lein-sub/subtest/child/target/provided/child-v0.3.0-1-g4ddbb83.jar
Wrote/lein-sub/subtest/child/pom.xml
Cycle dependecy, cannot resolve proper build order for projects: #{[subtest/child3 "v0.3.0-1-g4ddbb83"] [subtest/child2 "v0.3.0-1-g4ddbb83"]}

And if you will make the dependency tree as:

common-child -> [child3]
child -> [common-child]
child2 -> [common-child, child2]
child3 -> [child2]

Output would be:

bash-3.2$ lein sub install
Cycle dependecy, cannot resolve proper build order for projects: #{[subtest/child "v0.3.0-1-g4ddbb83"] [subtest/child3 "v0.3.0-1-g4ddbb83"] [subtest/child2 "v0.3.0-1-g4ddbb83"] [subtest/common-child "v0.3.0-1-g4ddbb83"]}

So, as you see — it builds sub-projects in a recursion, unless finished or a cycle was encountered.

kumarshantanu commented 10 years ago

Did you look at #5 discussion thread? There are several issues around implicit recursive behavior, which I want to avoid. Is this pull request different from what one can achieve using lein-sub and Leiningen profiles/aliases or lein-cascade?

ilya-pi commented 10 years ago

Hi Kumar. Yes, I did — it is about different things. There is no execution recursion in this case (I've misused it, addressing to the algorithm I use to determine proper build order).

I didn't change resolve-subprojects function at all, one that is responsible to figure out which subprojects will be built.

In this pull request I'm only addressing build order. So, if you have two subprojects: A and B, and B have a dependency on A, then A will be built first, and only then B.

I guess you can accomplish it with lein-cascade by specifying explicit build order for all your sub-projects on top of already existing dependency tree.

Currently, what we do is:

lein sub -s "common/common" install
lein sub install

Or

lein sub -s "common/common" install
lein sub -s "util/util" install
lein sub install

Depending on how complex dependencies are.

Well, once again — it is a very lightweight feature, I'm not addressing "set" of the sub-projects that will be built in any way, only the order in which they are built.

kumarshantanu commented 10 years ago

I think I now understand what this PR is trying to do. Please let me know if I got it wrong. Since it (1) discovers the dependency between sub-projects, and (2) reorders their builds silently, there's some implicit magic (complected) going on. I appreciate the discovery part, which I'd rather like to be invoked with a command-line switch e.g. -d -- it should probably only display the recommended order of build. The second part, which is to reorder the builds is something I believe should be left to the user.

Please let me know what you think? The file README.md shows that sub-projects can be specified in a vector, which preserves order -- one can use this to specify the order in which the build should happen. We ought to fulfill that assumption.

ilya-pi commented 10 years ago

Yep, you got it totally right! Now, thinking about — I think it is better to keep the build order as specified in the :sub vector, while allowing user to switch to dependency discovery mode.

So I've added -d option, as you proposed, that basically overrides order in the :sub vector.

I've also added a paragraph to README.md on this:

You can force dependency discovery between sub-projects (overrides build order specified in the :sub vector):

$ lein sub clean -d

I though it would be just natural, that you can override build order with -d the same way you can override :sub vector with -s; and might you only want to learn about the advised build order or check whether there is a cycle, without installing/deploying a thing, you can simply run lein sub clean -d, which will give you a list like:

$ lein sub clean -d
Building project  [subtest/common-child v0.3.0-3-g0158fca]
Building project  [subtest/child v0.3.0-3-g0158fca]
Building project  [subtest/child2 v0.3.0-3-g0158fca]
Building project  [subtest/child3 v0.3.0-3-g0158fca]

Please let me know what you think about it!

ilya-pi commented 10 years ago

Kumar, any feedback on this?

kumarshantanu commented 10 years ago

Sorry, was pulled away into other things. Will come back shortly on this.

ilya-pi commented 10 years ago

Cool!

kumarshantanu commented 10 years ago

I have few observations:

  1. The discovery can be a separate function, e.g. (defn discover-order ..) which may be consumed by resolve-subprojects, instead of perform-in-order, because discovery and effect are complected in the latter. Please let me know if I am missing anything.
  2. As a convention, options e.g. -d should come before the sub-command: lein sub -d clean
  3. -d and -s must be composable unless we decide that they should be mutually exclusive. I'd suggest using tools.cli to parse the arguments. E.g. lein sub -d -s "foo:bar" clean
  4. When using -d, it must print the discovered order it inferred on the screen, so that the user can know the new order. (What if the user only wants discovery, and no effect? -d does not provide that option in this case -- that's why my original suggestion was to only print the discovered order so that the user can use the output in a new command with the -s option.)
  5. Since -d would be a new switch, it should go into a new minor version (0.4.0-SNAPSHOT in this case.)

If these observations are in conflict with the PR, fee free to create a clean new PR. Also, I will be happy to discuss with an open mind if you don't agree with any of my observations.