Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When using -am more modules are built than the user might expect #44

Closed
famod opened this issue Jul 8, 2018 · 9 comments
Closed

When using -am more modules are built than the user might expect #44

famod opened this issue Jul 8, 2018 · 9 comments
Assignees
Milestone

Comments

@famod
Copy link
Member

famod commented Jul 8, 2018

Given a project with four modules A, B, C and D:

  • B and C depend on A
  • D depends on B and C
  • B contains changed/added files
  • A, C and D are unchanged
  • -am is used

then this extension behaves differently than what one might expect:

  • the extension reduces the project list to B and D
  • B because it contains changes
  • D because it depends on B
  • Maven applies -am to the project list: B and D
  • Maven ends up building everthing A (ok, upstream of B), B (ok, changed), D (ok, downstream of B) but also C because D depends on C

In other words this extension is effectively calling mvn ... -pl :B,:D -am whereas a user without this extension would probably call mvn ... -pl :B -am -amd which would only build A, B and D.

I have to add that the current "wider" approach is not entirely wrong or useless! In fact I do need it in a Jenkins pull request (PR) pipeline job which merges the PR with the target branch (I can elaborate more in detail if needed).

So I'd suggest we make this behaviour configurable. Ideas for the property name?

@vackosar
Copy link
Collaborator

vackosar commented Jul 9, 2018

To name this feature we should probably focus on what the user will have in mind when he will be looking for this feature. To do that we probably should try to list the use cases.

I think I made it such that it builds C because C may not exist in the maven repo local or remote or may exist but could be outdated. So the use case is to rebuild everything needed freshly.

Above use case would suggest name like 'rebuild' or 'dont-rely-on-repo'. Emmm, not sure.

What do u think?

@famod
Copy link
Member Author

famod commented Jul 18, 2018

Hm, as this is only relevant when using -am, the name of the feature/property should be related.

Something like alsoMakeMode=[changed|all] or =[min|full] or ...

Implementation-wise I see two different approaches for the changed/min/whatever mode:

  1. Either pass only the directly changed modules to ProjectDependencyGraph.getUpstreamProjects(...)
  2. Or only set the directly changed modules on the MavenSession but also set the "MakeBehaviour" on the MavenSession's MavenExecutionRequest to make-upstream

The second option makes me wonder whether GIB should in general auto-disable itself when make-downstream or make-both is selected by the user (-amd)...

@famod
Copy link
Member Author

famod commented Jan 20, 2019

@deradam & @ambition-consulting You both reacted to this issue with a "thumbs up".
Is this an urgent feature for you? If yes, we could defer release 3.8 until this is implemented.
Thanks in advance for your feedback!

@ambition-consulting
Copy link

Not urgent, but definitely desirable.

@famod
Copy link
Member Author

famod commented Jan 21, 2019

As noone is forcing us to push release 3.8 out today or tomorrow, I am going to have a look at this more in detail so that we can hopefully include this in 3.8.

@famod famod added this to the release-3.8 milestone Jan 21, 2019
@famod famod self-assigned this Jan 21, 2019
@famod famod mentioned this issue Jan 21, 2019
famod added a commit that referenced this issue Feb 17, 2019
…ties

Issue #44:  Introduce buildUpstreamMode & buildDownstream
@famod
Copy link
Member Author

famod commented Feb 18, 2019

Starting with Release 3.8, gib.buildUpstreamMode=changed will have the desired effect.

See: https://github.com/vackosar/gitflow-incremental-builder#gibbuildupstreammode

@famod famod closed this as completed Feb 18, 2019
@ambition-consulting
Copy link

Hi there,

thanks for the change. I have encountered a problem.

Assume the following maven dependency chain:
A -> B -> C

B gets changed.

'mvn clean install -pl C -am"

The ideal behaviour would be:

  • gib realising what is changed - works
  • maven build starting from the changed module - does not work
  • Instead, I can see the logs of "[INFO] gitflow-incremental-builder exiting..." before the reactor build does the normal thing

In a project with 50 modules with various dependencies, it would really pay off building only those modules changed, dependent modules on the changed, and only required for the target module.

@famod
Copy link
Member Author

famod commented Mar 5, 2019

@ambition-consulting Thanks for your feedback!

Can you please open another issue? IMHO, although related, this issue and the one you just reported are not entirely the same thing.

@ambition-consulting
Copy link

understood, will do

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

No branches or pull requests

3 participants