You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by neykov <gi...@git.apache.org> on 2015/07/17 13:03:19 UTC

[GitHub] incubator-brooklyn pull request: Fix build order - launcher has bu...

GitHub user neykov opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/753

    Fix build order - launcher has build-time depency on jsgui

    Also make the ordering more natural - almost all projects depend on `utils`. `all`, `dist`, `qa` are includes-all modules.
    
    The build would use an older version of `jsgui` (not the one being compiled with the project), or fail if none is available because `launcher` was built before `jsgui`.  Since `launcher` has only build time dependency on `jsgui` it's not convenient to include it in the dependencies section because it would have to be excluded manually in every project that depends on `launcher`.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/neykov/incubator-brooklyn reactor-build-order

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/753.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #753
    
----
commit ea26e42dace05ba1862622c845562fd88db12377
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2015-07-17T10:57:01Z

    Fix build order - launcher has build-time depency on jsgui
    
    Also make the ordering more natural - almost all projects depend on utils. all, dist, qa are includes-all modules.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix build order - launcher has bu...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/753#issuecomment-122294809
  
    Build failure not related, kicked another build.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix build order - launcher has bu...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/753#issuecomment-124330434
  
    Thanks all - merging.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix build order - launcher has bu...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/753#issuecomment-122256865
  
    I don't agree it's a hack - it relies on documented maven behaviour.
    
    1) Maven doesn't provide a mechanism to declare a build-time dependency. Adding a dependency to the `jsgui` war file means that it will be included in all project which include `launcher` as a (transitive) dependency. Also it will appear everywhere dependencies are copied (i.e. when using assembly plugin, as the dist project does). To avoid this `jsgui` would have to be excluded from each dependency to `launcher` which is far from ideal (this includes downstream projects).
    
    2) From http://maven.apache.org/guides/mini/guide-multiple-modules.html#Reactor_Sorting on build order sorting:
    ```
    the order declared in the <modules> element (if no other rule applies)
    ```
    
    Having in mind 1 & 2 I decided that defining the build order in the reactor is the best option.
    
    The solution is a nuisance of course - it shifts the responsibility to keep the build order on the developers, but it's not that big of a deal. Any suggestions to improve it are welcome.
    
    As for breaking the alphabetical sorting - I find it easier to find the projects if they follow build order, but that's a bike shedding discussion and not the reason for the reorder. Some of the modules had to be pulled down to let jsgui build on time. As for examples and QA - it's nice to have dist built as soon as possible when developing, while having the leaf projects built afterwards to confirm the successful changes.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix build order - launcher has bu...

Posted by hzbarcea <gi...@git.apache.org>.
Github user hzbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/753#issuecomment-122248243
  
    In general, build order is not fixed via the order of listing <module>s in the pom.xml but via the dependency mechanism. Modules were initially listed in a sorta alphabetical order to make them easier to find. This is a workable hack, but the builds will require more love for the next releases.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix build order - launcher has bu...

Posted by hzbarcea <gi...@git.apache.org>.
Github user hzbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/753#issuecomment-122278370
  
    @neykov, I suspect you missed my point. If the build system is defined correctly, it should succeed regardless of the order of the modules. That is ensured via the dependency mechanism. The alphabetical order is as good as any, should be completely irrelevant to the build system, but hopefully useful for a human reader.
    
    @rdowner, thanks for clarifying.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix build order - launcher has bu...

Posted by rdowner <gi...@git.apache.org>.
Github user rdowner commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/753#issuecomment-122267948
  
    @neykov in response to your comment *"Maven doesn't provide a mechanism to declare a build-time dependency"*, does `<scope>provided</scope>` have that effect?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix build order - launcher has bu...

Posted by hzbarcea <gi...@git.apache.org>.
Github user hzbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/753#issuecomment-122889979
  
    Good to merge.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix build order - launcher has bu...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/753


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix build order - launcher has bu...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/753#issuecomment-122273243
  
    Works perfectly and much more elegant. Thanks @rdowner - updated the PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---