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

groupNameMappingFile is not refreshed #17

Open
ArloL opened this issue Sep 20, 2013 · 3 comments
Open

groupNameMappingFile is not refreshed #17

ArloL opened this issue Sep 20, 2013 · 3 comments

Comments

@ArloL
Copy link
Contributor

ArloL commented Sep 20, 2013

I use a filter on my html and jsp files to automatically use the latest css file. Sadly these files are not automatically updated when I make a change in the less files.

Steps to reproduce

  • Clone https://github.com/ArloL/m2e-wro4j-bug-demo2/tree/master
  • Import project into an Eclipse with m2e-wro4j
  • Build project
  • Look under target/m2e-wtp/web-resources/css and see css file A
  • Look at target/m2e-wtp/web-resources/index.jsp and see reference to css file A
  • Change src/main/less/main.less and save
  • See two css files under target/m2e-wtp/web-resources/css
  • See reference to css file A under target/m2e-wtp/web-resources/index.jsp

Expected behaviour:
Reference to css file B after changing main.less.

You can actually see that naming.properties was changed. If you edit it yourself the login.jsp file is updated.

@fbricon
Copy link
Member

fbricon commented Sep 20, 2013

looks like a cool feature, I didn't know about it.

That will probably require some changes in m2e-wtp though, which needs to detect filters have been modified outside the eclipse incremental build. But I'm afraid making it work would have a terrible performance impact on incremental builds, since basically, every time you change/optimize one of the web resources, filtering would have to be performed on all webResource directories content.

@ArloL
Copy link
Contributor Author

ArloL commented Sep 23, 2013

Why does m2e-wtp have to detect changes outside of the incremental build? Isn't m2e-wro4j part of said build?

According to my tests, m2e-wtp already detects changes in the filters. You can reproduce this with the above steps following these:

  • Open naming.properties and make a change, save
  • Say Yes on whether you really want to change it
  • See reference to css file B under target/m2e-wtp/web-resources/index.jsp

m2e-wro4j already refreshes the groupNameMappingFile here. But it just doesn't get through to m2e-wtp somehow.

IMO the performance impact of the filtering is negligible in comparison with the build time of wro4j. A wro4j build is already triggered every time I change/optimize a web resource and that step takes "forever".

@fbricon
Copy link
Member

fbricon commented Sep 23, 2013

An incremental build is caused by a user change. User changes can obviously cause other IDE generated changes, but these are not part of the original change delta.

So when you manually modify a known maven filter, as you said, that file exists in the resource delta sent to the maven resource filtering build participant, there we can force a full resource filtering (https://github.com/eclipse/m2e.wtp/blob/master/org.eclipse.m2e.wtp/src/org/eclipse/m2e/wtp/internal/filtering/ResourceFilteringBuildParticipant.java#L167).

Simply "refreshing" the groupNameMappingFile is not enough. It merely makes the IDE know it's updated. This change occurs during a build event already, so it won't trigger another one (that would likely cause endless build loops).

Now, since the filter change occurs outside a normal build delta, I can see 2 ways to deal with it :

  • either m2e-wtp needs to keep track of all project filters modifications 'manually' and act accordingly. But it's really easy to screw that up and end up with memory leaks.
  • or m2e-wro4j needs to pass a list a interesting resources to m2e's BuildContext, shared between all build participants. That list would be leveraged by m2e-wtp's resource filtering participant, which runs after m2e-wro4j's one. I prefer that approach.
    Either way, m2e-wtp would need to be modified.

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

No branches or pull requests

2 participants