Hi Tamás,
Thank you for hacking around this so rapidly. I am not familiar with the maven 
code base, so let me know if I have misinterpreted the change: Is your hack to:

  1.  Retain the full multi-module build dependency graph such that the build 
graph always remains consistent regardless of whether a “-pl” arg has been 
provided
  2.  Introduce a skip build option to satisfy the functionality of the “-pl” 
arg: The reactor will report the module has been built but the build is 
basically a no-op
If so, this does resolve the issue but at the cost of losing the opportunity to 
gain extra parallelism in the build.
What I was wondering may be possible (without any idea of how feasible it would 
be to implement) would be the opportunity to recognise in the multi-build 
distinct build graphs and then keep isolation in the reactor between these 
graphs. To elaborate, in the reproducer scenario there would be two distinct 
graphs:

  *   "testsupport-module-1" followed by "app"
  *   "testsupport-module-2" followed by "module-b"
Ideally these two graphs build in parallel to each other and even if ultimately 
there is a dependency between them they NEVER resolve that dependency from 
within reactor they instead always resolve from the external Maven cache. The 
key statement here is that if a user presents a list of modules to be built 
then they really need to be sure that the DIRECT dependencies between these 
modules truly represent the build graph(s) they want. For tools like 
gitflow-incremental-builder which are designed to carefully work out change 
sets and what needs to build this offers a really powerful opportunity to 
optimise builds. I appreciate this functionality could also be seen as a gotcha 
(although tbf this already exists).
Thanks,
Joe


On 2024/02/08 10:09:31 Tamás Cservenák wrote:
> Seems we are on track with this. To prove my last-night theory I created a
> "hack" (is really just that) and guess what?
> It makes reproducer behave "as expected":
> https://github.com/apache/maven/pull/1406
>
> T
>
> On Wed, Feb 7, 2024 at 10:05 PM Tamás Cservenák <ta...@cservenak.net> wrote:
>
> > Howdy,
> >
> > Thank you very much, the reproducer works. Did not dig thru it fully, but
> > here are some related issues:
> >
> > https://issues.apache.org/jira/browse/MNG-8028 (funny thing, I created
> > this few weeks ago)
> > https://issues.apache.org/jira/browse/MNG-6300
> >
> > Will report back tomorrow (EU TZ)
> > T
> >
> >
> > On Wed, Feb 7, 2024 at 7:48 PM Joseph Leonard <
> > joseph.leon...@alfasystems.com> wrote:
> >
> >> Hi Tamás,
> >> I have created a simple example here:
> >> https://github.com/josple/mvn-multibuild-issue
> >> Hopefully the README is clear enough – let me know if I can clarify
> >> anything.
> >> Thanks,
> >> Joe
> >>
> >> On 2024/02/07 17:33:08 Tamás Cservenák wrote:
> >> > Howdy,
> >> >
> >> > In that case, there is something fishy with the project, my blind guess
> >> > would be some "hidden" inter-module dependency maybe?
> >> >
> >> > Can you provide access to source, or, if not feasible, could you provide
> >> > some reproducer and publish it on Github/Gitlab/whatever (maybe even
> >> just
> >> > send it privately as ML strips off attachments and images) for us to see
> >> > this in action?
> >> >
> >> > Thanks
> >> > T
> >> >
> >> > On Wed, Feb 7, 2024 at 6:29 PM Joseph Leonard <
> >> > joseph.leon...@alfasystems.com> wrote:
> >> >
> >> > > Hi Tamás,
> >> > > We have previously played around a bit with mvnd but not takari
> >> directly –
> >> > > I will have a play with it. With regards to this issue, using the
> >> takari
> >> > > smart builder unfortunately doesn’t resolve the issue.
> >> > > Joe
> >> > >
> >> > > On 2024/02/07 11:41:22 Tamás Cservenák wrote:
> >> > > > Can you please try smart builder instead?
> >> > > > https://github.com/takari/takari-smart-builder
> >> > > >
> >> > > > (note: smart builder is used by mvnd as well)
> >> > > >
> >> > > > The difference between the two can be seen here:
> >> > > > http://takari.io/book/30-team-maven.html#takari-smart-builder
> >> > > >
> >> > > > On Wed, Feb 7, 2024 at 11:50 AM Joseph Leonard <
> >> > > > joseph.leon...@alfasystems.com> wrote:
> >> > > >
> >> > > > > Hi Tamás,
> >> > > > > Yeah, this was unexpected to me initially as well. From what I
> >> can tell
> >> > > > > the Maven reactor only considers direct dependencies (i.e. not
> >> > > transitive
> >> > > > > dependencies) between the modules in the reactor when working out
> >> the
> >> > > build
> >> > > > > graph. For example if you have a simple linear dependency chain
> >> of:
> >> > > > > One --> Two --> Three --> Four --> Five
> >> > > > > Then invoking “mvn clean verify -pl One,Two,Four,Five -T 2 will
> >> result
> >> > > in
> >> > > > > two ‘graphs’ being built in parallel ([One,Two] and [Four,Five]).
> >> I
> >> > > assume
> >> > > > > this is as designed because it actually offers quite powerful
> >> > > functionality
> >> > > > > to improve the parallelism in your build. An example of where
> >> this is
> >> > > legit
> >> > > > > is when:
> >> > > > >
> >> > > > >   *   “Four” has a test scope dependency on “Five”
> >> > > > >   *   “One” has a test scoped dependency on “Two”
> >> > > > > If you made a src code change to “Five” and “Two” then it would be
> >> > > safe to
> >> > > > > build [One,Two] and [Four,Five] in parallel because you know the
> >> > > changes
> >> > > > > within these graphs cannot impact each other.
> >> > > > > Joe
> >> > > > >
> >> > > > > On 2024/02/06 21:37:42 Tamás Cservenák wrote:
> >> > > > > > Howdy,
> >> > > > > >
> >> > > > > > To me this looks like Maven is not aware that the App depends on
> >> > > > > ModuleB...
> >> > > > > > Are they "plain dependency" linked? Or what kind of dependency
> >> we
> >> > > talk
> >> > > > > > about here?
> >> > > > > > In short: why would App start while ModuleB (upstream dep) is
> >> not
> >> > > done?
> >> > > > > > Something is fishy here.
> >> > > > > >
> >> > > > > > T
> >> > > > > >
> >> > > > > >
> >> > > > > > On Tue, Feb 6, 2024 at 11:40 AM Joseph Leonard <
> >> > > > > > joseph.leon...@alfasystems.com> wrote:
> >> > > > > >
> >> > > > > > > Hi all,
> >> > > > > > >
> >> > > > > > > It would be great to get any thoughts on whether the
> >> following is a
> >> > > > > defect:
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > Issue details:
> >> > > > > > > tl;dr
> >> > > > > > >
> >> > > > > > > Maven can resolve dependencies either from:
> >> > > > > > >
> >> > > > > > >   *   an external repo
> >> > > > > > >   *   a class directory of a module being built within the
> >> reactor
> >> > > > > > >   *   a packaged jar of a module being built within the
> >> reactor
> >> > > > > > >
> >> > > > > > > If you run a concurrent multi-module build it is possible to
> >> get a
> >> > > race
> >> > > > > > > condition whereby the build of module Foo may resolve module
> >> Bar
> >> > > from
> >> > > > > > > either of the three resolution channels. This inconsistency
> >> can
> >> > > result
> >> > > > > in
> >> > > > > > > the Maven war plugin sometimes failing to build a functional
> >> war
> >> > > file.
> >> > > > > I
> >> > > > > > > would expect a consistent resolution would always take place.
> >> > > > > > >
> >> > > > > > > Full details
> >> > > > > > > Scenario
> >> > > > > > >
> >> > > > > > > Consider you have a repo with the following structure:
> >> > > > > > >
> >> > > > > > >                        App
> >> > > > > > >
> >> > > > > > >                      /     \
> >> > > > > > >
> >> > > > > > >                     /       \
> >> > > > > > >
> >> > > > > > >        (compile scope)      (test scope)
> >> > > > > > >
> >> > > > > > >                   /           \
> >> > > > > > >
> >> > > > > > >                 \/_           _\/
> >> > > > > > >
> >> > > > > > >              ModuleA      TestSupportModule1
> >> > > > > > >
> >> > > > > > >                 /
> >> > > > > > >
> >> > > > > > >                /
> >> > > > > > >
> >> > > > > > >     (compile scope)
> >> > > > > > >
> >> > > > > > >              /
> >> > > > > > >
> >> > > > > > >            \/_
> >> > > > > > >
> >> > > > > > >         ModuleB
> >> > > > > > >
> >> > > > > > >            /
> >> > > > > > >
> >> > > > > > >           /
> >> > > > > > >
> >> > > > > > >     (test scope)
> >> > > > > > >
> >> > > > > > >         /
> >> > > > > > >
> >> > > > > > >       \/_
> >> > > > > > >
> >> > > > > > > TestSupportModule2
> >> > > > > > >
> >> > > > > > > If you were to make a src code change to the following test
> >> support
> >> > > > > > > modules:
> >> > > > > > >
> >> > > > > > >   *   TestSupportModule1
> >> > > > > > >   *   TestSupportModule2
> >> > > > > > >
> >> > > > > > > Then the minimum number of modules we need to build to verify
> >> the
> >> > > > > change
> >> > > > > > > set is OK is:
> >> > > > > > >
> >> > > > > > >   *   TestSupportModule1
> >> > > > > > >   *   TestSupportModule2
> >> > > > > > >   *   ModuleB
> >> > > > > > >   *   App
> >> > > > > > >
> >> > > > > > > i.e. there is no requirement to build ModuleA because we know
> >> that
> >> > > > > none of
> >> > > > > > > the src code changes could impact the classpaths used in its
> >> maven
> >> > > > > build.
> >> > > > > > >
> >> > > > > > > We know that despite 'App' depending (transitively) on ModuleB
> >> > > there
> >> > > > > is no
> >> > > > > > > need for the 'App' build to wait for ModuleB to complete its
> >> build
> >> > > > > because
> >> > > > > > > the src code change to TestSupportModule2 will not impact any
> >> of
> >> > > the
> >> > > > > > > classpaths used in the App maven build. Therefore to get the
> >> most
> >> > > > > efficient
> >> > > > > > > build possible we ideally would invoke Maven to run with 2
> >> threads
> >> > > and
> >> > > > > with
> >> > > > > > > instruction to build two distinct 'dependency graphs':
> >> > > > > > >
> >> > > > > > >   *   TestSupportModule1 followed by ModuleB
> >> > > > > > >   *   TestSupportModule1 followed by App
> >> > > > > > >
> >> > > > > > > The following Maven command achieves exactly what we want
> >> because
> >> > > the
> >> > > > > > > reactor build order is based only on the direct (i.e.
> >> > > non-transitive)
> >> > > > > > > dependencies of the modules provided to the reactor in the
> >> build
> >> > > > > command.
> >> > > > > > > Therefore the absence of ModuleA results in two distinct
> >> > > 'dependency
> >> > > > > > > graphs':
> >> > > > > > >
> >> > > > > > > mvn clean verify -pl
> >> > > TestSupportModule1,TestSupportModule2,ModuleB,App
> >> > > > > -T 2
> >> > > > > > >
> >> > > > > > > Note: In reality the code base I maintain has a very large
> >> > > monobuild
> >> > > > > with
> >> > > > > > > 100s of modules and this type of build optimisation makes a
> >> > > significant
> >> > > > > > > difference to the speed of our monobuild (we use
> >> > > > > > >
> >> > > > >
> >> > >
> >> https://github.com/gitflow-incremental-builder/gitflow-incremental-builder
> >> > > > > > > to automate the logic of determining which modules to include
> >> in
> >> > > the
> >> > > > > > > reactor based on our change set).
> >> > > > > > >
> >> > > > > > > Issue
> >> > > > > > >
> >> > > > > > > We have encountered an issue in the above scenario because the
> >> > > 'App'
> >> > > > > build
> >> > > > > > > has a race condition with the ModuleB build which will result
> >> in
> >> > > one
> >> > > > > of the
> >> > > > > > > following three outcomes:
> >> > > > > > >
> >> > > > > > >   *   If the 'App' build starts before the ModuleB build has
> >> > > compiled
> >> > > > > its
> >> > > > > > > src classes then the 'App' build will resolve ModuleB from the
> >> > > external
> >> > > > > > > repo (i.e. equivalent to ModuleB not being in the reactor at
> >> all)
> >> > > > > > >   *   If the 'App' build starts after ModuleB has compiled
> >> its src
> >> > > > > classes
> >> > > > > > > but before it has packaged these classes into a jar then the
> >> 'App'
> >> > > > > build
> >> > > > > > > will resolve ModuleB's target/classes directory
> >> > > > > > >   *   If the 'App' build starts after ModuleB has packaged
> >> its jar
> >> > > file
> >> > > > > > > then the 'App' build will resolve ModuleB's target/ModuleB.jar
> >> > > file.
> >> > > > > > >
> >> > > > > > > In many scenarios this dependency resolution inconsistency
> >> doesn't
> >> > > > > > > represent a challenge. However, it does cause an issue in our
> >> case
> >> > > > > because
> >> > > > > > > the 'App' POM has its Maven packaging stanza configured to
> >> war and
> >> > > in
> >> > > > > the
> >> > > > > > > scenario where ModuleB's target/classes directory is resolved
> >> by
> >> > > the
> >> > > > > 'App'
> >> > > > > > > then this results in the resultant 'App' war file being
> >> packaged
> >> > > with a
> >> > > > > > > completely empty ModuleB.jar file.
> >> > > > > > >
> >> > > > > > > Proposed solution
> >> > > > > > >
> >> > > > > > > Ideally we would like the Maven reactor to retain isolation
> >> > > between the
> >> > > > > > > two distinct 'dependency graphs' it constructs at
> >> instantiation
> >> > > > > throughout
> >> > > > > > > the entire Maven build. This would mean, in the simple example
> >> > > above,
> >> > > > > that
> >> > > > > > > the 'App' would always resolves ModuleB from the external repo
> >> > > > > (regardless
> >> > > > > > > of whether the reactor has built ModuleB or not in a separate
> >> > > > > 'dependency
> >> > > > > > > graph' in the reactor).
> >> > > > > > >
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > Joseph Leonard
> >> > > > > > > Manager
> >> > > > > > >
> >> > > > > > > Alfa
> >> > > > > > > ________________________________
> >> > > > > > > e: joseph.leon...@alfasystems.com | w: alfasystems.com<
> >> > > > > > > https://www.alfasystems.com>
> >> > > > > > > t: +44 (0)20 7588 1800 | Moor Place, 1 Fore Street Avenue,
> >> London,
> >> > > EC2Y
> >> > > > > > > 9DT, GB
> >> > > > > > > ________________________________
> >> > > > > > >
> >> > > > > > > The contents of this communication are not intended to be
> >> binding
> >> > > or
> >> > > > > > > constitute any form of offer or acceptance or give rise to any
> >> > > legal
> >> > > > > > > obligations on behalf of the sender or Alfa. The views or
> >> opinions
> >> > > > > > > expressed represent those of the author and not necessarily
> >> those
> >> > > of
> >> > > > > Alfa.
> >> > > > > > > This email and any attachments are strictly confidential and
> >> are
> >> > > > > intended
> >> > > > > > > solely for use by the individual or entity to whom it is
> >> > > addressed. If
> >> > > > > you
> >> > > > > > > are not the addressee (or responsible for delivery of the
> >> message
> >> > > to
> >> > > > > the
> >> > > > > > > addressee) you may not copy, forward, disclose or use any
> >> part of
> >> > > the
> >> > > > > > > message or its attachments. At present the integrity of email
> >> > > across
> >> > > > > the
> >> > > > > > > internet cannot be guaranteed and messages sent via this
> >> medium are
> >> > > > > > > potentially at risk. All liability is excluded to the extent
> >> > > permitted
> >> > > > > by
> >> > > > > > > law for any claims arising as a result of the use of this
> >> medium to
> >> > > > > > > transmit information by or to Alfa or its affiliates.
> >> > > > > > >
> >> > > > > > > Alfa Financial Software Ltd
> >> > > > > > > Reg. in England No: 0248 2325
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
>

Reply via email to