Maven 2 & 3

embedder returns stale maven project state

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 3.0-alpha-2
  • Fix Version/s: 3.0-alpha-3
  • Component/s: Embedding
  • Labels:
    None
  • Complexity:
    Intermediate
  • Number of attachments :
    6

Description

Embedder returns stale maven project state after project's pom.xml changed and re-read. This is extremely common scenario when using embedder in IDE like m2e. See attached sample project that demonstrates one of many possible ways to trigger this problem.

The problem appears to be related to static MavenProject cache in DefaultMavenProjectBuilder, which means that even recreating embedder instance is not going to purge state cache entries.

Also note that using artifact#getId does not provide enough context to uniquely identify cached MavenProject instances. This is particularly true for IDEs where the same embedder instance is likely to be used to process unrelated projects. Cached MavenProject instances should be identified at least by the following attributes

  • project artifact id
  • values of all properties
  • all active profiles
  • content of all pom.xml files directly and indirectly involved in construction of maven project instance
  • IDE specific state, like m2e "enable workspace dependency resolution"
  • likely more...
  1. File
    MNG-4041.diff
    04/Jun/09 9:40 PM
    76 kB
    Igor Fedorenko
  2. File
    MNG-4041-c.diff
    08/Jun/09 2:56 PM
    4 kB
    Igor Fedorenko
  3. File
    MNG-4041-c.diff
    06/Jun/09 8:04 PM
    6 kB
    Igor Fedorenko
  4. File
    MNG-4041-d.diff
    20/Jun/09 8:13 AM
    2 kB
    Igor Fedorenko
  5. File
    MNG-4041-e.diff
    20/Jun/09 1:29 PM
    14 kB
    Igor Fedorenko

Issue Links

relates to

Improvement - An improvement or enhancement to an existing feature or task. MNG-4194 API to safely release of plugin realms

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Igor Fedorenko added a comment -

Attached replaces MavenProject cache with two simpler (relatively speaking) caches – maven metadata source and pluginDescriptor/pluginRealm. It also defers plugin dependency resolution, plugin realm construction and component discovery until mojo execution. This eliminates unnecessary artifact resolution and classloading overhead for plugins that are never used (site plugin during clean/install build, for example). The patch is against maven trunk.

Show
Igor Fedorenko added a comment - Attached replaces MavenProject cache with two simpler (relatively speaking) caches – maven metadata source and pluginDescriptor/pluginRealm. It also defers plugin dependency resolution, plugin realm construction and component discovery until mojo execution. This eliminates unnecessary artifact resolution and classloading overhead for plugins that are never used (site plugin during clean/install build, for example). The patch is against maven trunk.
Hide
Permalink
Milos Kleint added a comment -

it would be nice to have an option to put a custom caching strategy in place. With the old 2.1 snapshots, there was a Workspace*someting* component. it was purged after each call to project resolution. Changing it to timed weak references I was able to speed up the loading of multiple projects significantly, while cleaning up in the long run..

Show
Milos Kleint added a comment - it would be nice to have an option to put a custom caching strategy in place. With the old 2.1 snapshots, there was a Workspace*someting* component. it was purged after each call to project resolution. Changing it to timed weak references I was able to speed up the loading of multiple projects significantly, while cleaning up in the long run..
Hide
Permalink
Jason van Zyl added a comment -

Patch applied and m2eclipse is working well with this in place.

Show
Jason van Zyl added a comment - Patch applied and m2eclipse is working well with this in place.
Hide
Permalink
Igor Fedorenko added a comment -

Milos, both caches have flush() method, so it is possible to purge them when needed. It is also possible to provide custom cache implementations using container customization hooks (this is what I plan to do in m2e). I am not sure if it makes sense to change default implementation to use weak references.

Show
Igor Fedorenko added a comment - Milos, both caches have flush() method, so it is possible to purge them when needed. It is also possible to provide custom cache implementations using container customization hooks (this is what I plan to do in m2e). I am not sure if it makes sense to change default implementation to use weak references.
Hide
Permalink
Igor Fedorenko added a comment -

Attached MNG-4041-c.diff patch fixes couple of problems in the original implementation

  • Plugin CacheKey equals method considers repositories now (thank you for spotting this, Benjamin).
  • Plugins that differ only in <executions/> use the same plugin realm.
Show
Igor Fedorenko added a comment - Attached MNG-4041 -c.diff patch fixes couple of problems in the original implementation Plugin CacheKey equals method considers repositories now (thank you for spotting this, Benjamin). Plugins that differ only in <executions/> use the same plugin realm.
Hide
Permalink
Igor Fedorenko added a comment -

Revised version of the second patch. After some further discussions we came to the conclusion that plugin/executions should not be considered when caching neither plugin realms nor plugin descriptors.

Show
Igor Fedorenko added a comment - Revised version of the second patch. After some further discussions we came to the conclusion that plugin/executions should not be considered when caching neither plugin realms nor plugin descriptors.
Hide
Permalink
Benjamin Bentmann added a comment -

Applied additional patch in r782776, thanks Igor!

Show
Benjamin Bentmann added a comment - Applied additional patch in r782776 , thanks Igor!
Hide
Permalink
Milos Kleint added a comment -

Igor: allowing custom cache implementations in the container is more than enough, I never meant to suggest tat weak references shall be used by default..

Show
Milos Kleint added a comment - Igor: allowing custom cache implementations in the container is more than enough, I never meant to suggest tat weak references shall be used by default..
Hide
Permalink
Igor Fedorenko added a comment -

Attached MNG-4041-d.diff corrects a bug in plugin dependencies comparison, which could result in caching of duplicate realms and/or wrong cache hits.

Show
Igor Fedorenko added a comment - Attached MNG-4041 -d.diff corrects a bug in plugin dependencies comparison, which could result in caching of duplicate realms and/or wrong cache hits.
Hide
Permalink
Benjamin Bentmann added a comment -

Commited in r786811, good catch!

Show
Benjamin Bentmann added a comment - Commited in r786811 , good catch!
Hide
Permalink
Igor Fedorenko added a comment -

Attached MNG-4041-e.diff implements missing hashCode/equals methods in ArtifactFilter classes. ArtifactFilters are used in hash maps by DefaultMavenMetadataCache and missing hashCode/equals was causing duplicate cache entries.

Show
Igor Fedorenko added a comment - Attached MNG-4041 -e.diff implements missing hashCode/equals methods in ArtifactFilter classes. ArtifactFilters are used in hash maps by DefaultMavenMetadataCache and missing hashCode/equals was causing duplicate cache entries.
Hide
Permalink
Benjamin Bentmann added a comment -

Commited in r786882. Thanks but in the future please try to remember to add the Apache license header to new files.

Show
Benjamin Bentmann added a comment - Commited in r786882 . Thanks but in the future please try to remember to add the Apache license header to new files.

People

  • Assignee:
    Unassigned
    Reporter:
    Igor Fedorenko
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: