Support for nested subprojects #5

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
3 participants

erickg commented May 25, 2012

Hi Kumar. I am using the lein-sub plugin in a large project, which contains modules that contain multiple nested sub projects, so I had to make some modifications to allow lein-sub to work properly with more than one sub level in the project tree. I think it might be of use to the larger community.

Cheers!

Erick

Owner

kumarshantanu commented May 26, 2012

Hi Erick, thanks for the pull request. It looks good, but I found an issue with the way it invokes task on the nested sub-projects. Here is how to reproduce it:

Create nested projects foo-->bar-->baz:

$ lein2 new foo
$ cd foo
$ lein2 new bar
$ cd bar
$ lein2 new baz

Edit project.clj of each to include the :sub entries. Now create JAR artifacts in each:

$ cd foo
$ lein2 jar
$ cd bar
$ lein2 jar
$ cd baz
$ lein2 jar

Now reach the toplevel foo and try to invoke the task on sub-projects:

$ cd ../..  # make sure you are in foo
$ lein2 sub clean

This should clean both bar and baz, but only baz is cleaned. Please let me know if my assumption is incorrect. This seems to be arising from ee6e120 and 83d4401 wherein the expression if nested-subprojects seems to be making a boolean choice between nested-sub-projects and the current sub-project.

apply tasks to *all* projects in the directory hierarchy, not just th…
…e leaf projects, and make this recursive behaviour an option via the :recursive option

erickg commented May 27, 2012

Hi Kumar, yeah good catch. This scenario simply is not being used by us in our project, so I simply didn't notice. For us, only the leaf sub projects contain build products, and the "intermediate" sub projects are just "bridging" or bundling all the nested sub projects together.. something like this:

top -> libraries -> foo
-> bar
-> baz

So in our case, "libraries", has no build products itself.. its only purpose is to bind the sub projects down the tree..

Alas, I agree that this is incomplete and when one calls "sub " the reasonable thing to do is to both recurse the entire tree structure, but also call the task on each an every one of the sub projects, not just on the "leaf" ones.

The fix is really simple, but before we go ahead, I would ask you.. shouldn't this also apply to the top directory?.. I mean, when one does "lein sub " , task is not called on the top directory itself. You have to do "lein sub jar, jar" or something along those lines. To be consistent, it sounds to me like the expected behavior in the sub projects should also apply to the top project, no?.. the fix here would be slightly different, but I find it personally a more elegant solution both in the way the code turns out and the behaviour resembles that of other build tools.

What do u think?

Erick

Am 26.05.2012 um 23:35 schrieb Shantanu Kumar:

Hi Erick, thanks for the pull request. It looks good, but I found an issue with the way it invokes task on the nested sub-projects. Here is how to reproduce it:

Create nested projects foo-->bar-->baz:

$ lein2 new foo
$ cd foo
$ lein2 new bar
$ cd bar
$ lein2 new baz

Edit project.clj of each to include the :sub entries. Now create JAR artifacts in each:

$ cd foo
$ lein2 jar
$ cd bar
$ lein2 jar
$ cd baz
$ lein2 jar

Now reach the toplevel foo and try to invoke the task on sub-projects:

$ cd ../..  # make sure you are in foo
$ lein2 sub clean

This should clean both bar and baz, but only baz is cleaned. Please let me know if my assumption is incorrect. This seems to be arising from ee6e120 and 83d4401 wherein the expression if nested-subprojects seems to be making a boolean choice between nested-sub-projects and the current sub-project.


Reply to this email directly or view it on GitHub:
#5 (comment)

Owner

kumarshantanu commented May 28, 2012

Hi Erick, That's a good proposal. Do you think lein sub -r <taskname> would be a good alternative command-line switch to represent the recursive application of the task? I mean, the recursive application should take place only when somebody specifies -r. The rationale behind this is to let the user decide the scope for applying the task.

Cheers!

Shantanu

erickg commented May 28, 2012

Not sure I follow.. but in general terms I think if someone is using the sub plugin, it is cause they want this recursive behaviour no?.. or did I misunderstand you?. I mean, it is like when you have a Maven or Make project, you expect that by simply typing "make" at the top, somehow the entire project with all its subcomponents is built. That we have to use a separate plugin for this is already a bit of a nuisance for users. Ideally this functionality should be a part of leiningen itself :-)

Am 28.05.2012 um 12:51 schrieb Shantanu Kumar:

Hi Erick, That's a good proposal. Do you think lein sub -r <taskname> would be a good alternative command-line switch to represent the recursive application of the task? I mean, the recursive application should take place only when somebody specifies -r. The rationale behind this is to let the user decide the scope for applying the task.

Cheers!

Shantanu


Reply to this email directly or view it on GitHub:
#5 (comment)

Owner

kumarshantanu commented May 28, 2012

Yes, it would have been nice of Leiningen to support this out of the box. However, it is a bit complicated for lein-sub -- it is possible to use Leiningen hooks to invoke lein-sub with scope: https://groups.google.com/group/leiningen/browse_thread/thread/1223cd092b83f007/cef32b294960b678 (search for comments by "Shantanu Kumar")

It is for this reason that I am hesitant to go for recursive behavior by default. Optional recursive behavior using -r gives the flexibility to do both. What do you think?

erickg commented May 28, 2012

Sorry, that is what I don't understand.. what is the alternative to recursive behavior?.. the standard and sole reason d'être for lein-sub is to apply tasks recursively?.. sorry I am just not following you.

Erick

Am 28.05.2012 um 14:09 schrieb Shantanu Kumar:

Yes, it would have been nice of Leiningen to support this out of the box. However, it is a bit complicated for lein-sub -- it is possible to use Leiningen hooks to invoke lein-sub with scope: https://groups.google.com/group/leiningen/browse_thread/thread/1223cd092b83f007/cef32b294960b678 (search for comments by "Shantanu Kumar")

It is for this reason that I am hesitant to go for recursive behavior by default. Optional recursive behavior using -r gives the flexibility to do both. What do you think?


Reply to this email directly or view it on GitHub:
#5 (comment)

Owner

kumarshantanu commented May 28, 2012

OK, so it's like this: lein-sub currently executes a target for only one level of sub-projects, i.e. only those which are the immediate children. You suggested recursive application, which is a different behavior from the current setting. My point is that both behaviors (i.e. existing behavior, and the behavior you proposed) have their respective places, so recursive application should be accommodated without destroying the existing behavior.

You already know the utility of recursive behavior, so I will try and explain why the existing behavior is important too. When a target is applied to just one-level, that is an instrument of control over how a complex project might be built using Leiningen hooks and a version of lein-sub that can apply a target to just one level. That intends to be fine-grained control, one baby step at a time. Such controls at each sub-project level can lead to a sophisticated nested build. Combine that with Leiningen 2.0 profiles and you get a powerful mix. The thread I linked to in my previous comment discusses some aspects of that.

Hope that helps. Feel free to ask if I skipped anything essential.

Owner

erickg commented on 75560ac May 28, 2012

Ah ok, I understand now what you mean. The confusion arose from the fact that in my head, the current behaviour is already "recursive".. i.e. to just one level :-) anyhow.. your point is clear now, and I have updated the pull request to include your idea. The :recursive option when calling lein sub will enable recursive behaviour (i.e. task is applied to every sub project in the hierarchy) and when not specified the previous behaviour is preserved, that is, the task will be run just on the directories included in the current level's :sub key

Hello,

I am working on a project with the same structure as @erickg and I was wondering if there was any plan to merge this feature. Or if there is another tool I could use.

Thanks!

Owner

kumarshantanu commented Sep 21, 2013

@shanewilson Can you try https://github.com/kumarshantanu/lein-cascade and see if you can compose lein-cascade and lein-sub together to have complex nested builds?

@kumarshantanu Thanks, lein-cascade seems pretty cool.

I got it working by doing:

# Parent  
:cascade {"test" [["sub" "cascade" "test"]]}

# Sub Parent
:cascade {"test" [["sub" "test"]]}

And then in the top parent dir I can run:

lein cascade test

and it runs tests on all the children. Is that about right?

Owner

kumarshantanu commented Sep 21, 2013

@shanewilson That's correct. One problem lein-sub currently has is, it runs the same command to all sub-projects regardless of whether the user wants so. Adding an option to lein-sub to specify sub-project directories, e.g. -s modules/m1:modules/m2 would probably fix that - still thinking about it though.

Owner

kumarshantanu commented Sep 21, 2013

I haven't announced lein-cascade publicly yet, but its intent is to address the points @erickg and I discussed in this thread above. I realized, (1) combining task dependencies with lein-sub would result in a complected design, and (2) Leiningen aliases are not very handy for lengthy multi-stage dependencies. I hope the division of responsibility between lein-cascade and lein-sub would keep it clean.

Owner

kumarshantanu commented Sep 22, 2013

I have released 0.3.0 that adds the -s <subproject dirs> command line option and updated the README with a pointer on how to achieve recursive behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment