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

Fix off issue #22 <?m2e execute onConfiguration?> fails if pom is big… #23

Merged
merged 3 commits into from
Apr 27, 2017

Conversation

barclay-reg
Copy link
Contributor

… enough (8kb)

  • fixed pom parser, as logic to check for x-m-l letters does not respected input tokenization
    ** all other occurences of <? are still ignored, but now: also when they appear behind READ_CHUNK_SIZE

…s if pom is big enough (8kb)

* fixed pom parser, as logic to check for x-m-l letters does not respected input tokenization
@khmarbaise
Copy link
Member

As far as I know there already configuration possibilities for m2e like this:

<pluginManagement>
  <plugins>
    <plugin>
     <groupId>org.eclipse.m2e</groupId>
     <artifactId>lifecycle-mapping</artifactId>
     <version>1.0.0</version>
     <configuration>
       <lifecycleMappingMetadata>
         <pluginExecutions>
           <pluginExecution>
             <pluginExecutionFilter>
               <groupId>some-group-id</groupId>
               <artifactId>some-artifact-id</artifactId>
               <versionRange>[1.0.0,)</versionRange>
               <goals>
                 <goal>some-goal</goal>
               </goals>
             </pluginExecutionFilter>
             <action>
               <ignore/>
             </action>
           </pluginExecution>
         </pluginExecutions>
       </lifecycleMappingMetadata>
     </configuration>
    </plugin>
  </plugins>
</pluginManagement>

@barclay-reg
Copy link
Contributor Author

yes - this was standard up to m2e 1.6, but the new notation (<?m2e) has some advantages - especially in big pom.xml files. Ironically this big pom.xml cannot be parsed at the moment. It's supported by XML standard, so the parser is - in this niche - non standard compliant

@rfscholte rfscholte self-assigned this Apr 14, 2017
@rfscholte
Copy link
Member

I'm missing a unittest to confirm the fix. Would be nice if you can provide that as well.

@rfscholte
Copy link
Member

Seems be be related to https://issues.apache.org/jira/browse/MNG-6216

@barclay-reg
Copy link
Contributor Author

@rfscholte tests added

@rfscholte
Copy link
Member

Thanks. But is seems that if I only apply the test, it is already running succesfully...

fixed unit test, so it fails for the old code
* old unit test implementation was trapped by the buffer resizing (for the one, big 10k chars xml comment), so this was tailored into 10 * 1000 chars xml comments
@barclay-reg
Copy link
Contributor Author

barclay-reg commented Apr 27, 2017

you are correct - test is fixed now, so it fails for the old code. i was trapped by the buffer resizing, which accidentally let the test never fail (buf was alway big enough to avoid the ArrayIndexOutOfBoundException)

@rfscholte rfscholte merged commit dd1c85f into codehaus-plexus:master Apr 27, 2017
@rfscholte
Copy link
Member

Exactly the reason why unittests are required :) Thanks for the patch!

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.

4 participants