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

[MNG-8461] Initial settings method must restore context state #2004

Merged
merged 2 commits into from
Dec 22, 2024

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Dec 20, 2024

Effective settings are (should be) created twice, once for "early boot" of Plexus when extensions are loaded up, and then again when Maven "boots".

Bug was that early call "corrupted" (inited settings) in context causing that 2nd required call (due spy) was omitted. This resulted in lack of settings related spy events firing (as we do have IT for spy but it does not test settings events).


https://issues.apache.org/jira/browse/MNG-8461

Related: alextu/maven4-reproducer#1

Effective settings are (should be) created twice, once for
"early boot" of Plexus when extensions are loaded up, and
then again when Maven "boots".

Bug was that early call "corrupted" (inited settings) in
context causing that 2nd required call (due spy) was
omitted.

---

https://issues.apache.org/jira/browse/MNG-8461
@cstamas cstamas added this to the 4.0.0-rc-3 milestone Dec 20, 2024
@cstamas cstamas requested a review from gnodet December 20, 2024 13:59
@cstamas cstamas self-assigned this Dec 20, 2024
Copy link
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An IT would be good. Can we adapt the reproducer ?

@cstamas
Copy link
Member Author

cstamas commented Dec 20, 2024

I think we can... but am now looking at MavenITmng4936EventSpyTest and it seems it does test for this thing but did not fail?

@cstamas
Copy link
Member Author

cstamas commented Dec 20, 2024

So, we need to dig in, why reproducer fails, and why IT succeeds. Locally running the related IT I have this (mng-4936/target/spy.log):

fixed: Apache Maven 4.0.0-rc-3-SNAPSHOT (a9f6f3b)

init
event: org.apache.maven.api.services.SettingsBuilderRequest$SettingsBuilderRequestBuilder$DefaultSettingsBuilderRequest
event: org.apache.maven.internal.impl.DefaultSettingsBuilder$DefaultSettingsBuilderResult
event: org.apache.maven.api.services.ToolchainsBuilderRequest$ToolchainsBuilderRequestBuilder$DefaultToolchainsBuilderRequest
event: org.apache.maven.internal.impl.DefaultToolchainsBuilder$DefaultToolchainsBuilderResult
event: org.apache.maven.execution.DefaultMavenExecutionRequest
event: org.apache.maven.lifecycle.internal.DefaultExecutionEvent
event: org.apache.maven.lifecycle.internal.DefaultExecutionEvent
event: org.apache.maven.lifecycle.internal.DefaultExecutionEvent
event: org.apache.maven.lifecycle.internal.DefaultExecutionEvent
event: org.apache.maven.lifecycle.internal.DefaultExecutionEvent
event: org.apache.maven.execution.DefaultMavenExecutionResult
close

master: Apache Maven 4.0.0-rc-3-SNAPSHOT (575ad37)

init
event: org.apache.maven.api.services.SettingsBuilderRequest$SettingsBuilderRequestBuilder$DefaultSettingsBuilderRequest
event: org.apache.maven.internal.impl.DefaultSettingsBuilder$DefaultSettingsBuilderResult
event: org.apache.maven.api.services.ToolchainsBuilderRequest$ToolchainsBuilderRequestBuilder$DefaultToolchainsBuilderRequest
event: org.apache.maven.internal.impl.DefaultToolchainsBuilder$DefaultToolchainsBuilderResult
event: org.apache.maven.execution.DefaultMavenExecutionRequest
event: org.apache.maven.lifecycle.internal.DefaultExecutionEvent
event: org.apache.maven.lifecycle.internal.DefaultExecutionEvent
event: org.apache.maven.lifecycle.internal.DefaultExecutionEvent
event: org.apache.maven.lifecycle.internal.DefaultExecutionEvent
event: org.apache.maven.lifecycle.internal.DefaultExecutionEvent
event: org.apache.maven.execution.DefaultMavenExecutionResult
close

Basically they are same.

@cstamas cstamas marked this pull request as ready for review December 20, 2024 15:19
@cstamas
Copy link
Member Author

cstamas commented Dec 20, 2024

Still cannot figure, and I starting to doubt in our ITs... The only difference I found so far:

  • reproducer uses .mvn/extensions.xml to load up spy listener
  • IT uses -Dmaven.ext.class.path=spy-0.1.jar so it is much earlier loaded up to system (also, due this IT is forked)

Still, as the code change shows, the event should not be emitted, but it seems it was? Checked also, the IT is forked, and forked executor is used, so it cannot be some "leftover state" in embedded executor...

@cstamas cstamas merged commit bebc3d4 into apache:master Dec 22, 2024
13 checks passed
@cstamas cstamas deleted the MNG-8461 branch December 22, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants