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.

Sign up for GitHub

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

Jump to bottom

COCOON-2347 #4

Closed
wants to merge 13 commits into from
Closed

COCOON-2347 #4

wants to merge 13 commits into from

Conversation

ggruber4711
Copy link

Needed changes to build against
spring 4.x
JDK 1.8

@ggruber4711
COCOON-2347
e7dcaa5 
Needed changes to build against
spring 4.x
JDK 1.8
ilgrosso
ilgrosso reviewed Apr 4, 2018
View reviewed changes
README.txt Outdated
$ cocoon.sh debug

Then tell your IDE to connect to JVM debug on port localhost:5005.
This branch tries to implement migration to
Copy link
Member

Choose a reason for hiding this comment

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

Please revert the changes to this file, no need to alter the README, as this branch will eventually be merged back to trunk.

Copy link
Author

Choose a reason for hiding this comment

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

OK, I will do that.

Copy link
Author

Choose a reason for hiding this comment

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

done

ilgrosso
ilgrosso reviewed Apr 4, 2018
View reviewed changes
...coon-expression-language-api/src/main/java/org/apache/cocoon/el/objectmodel/ObjectModel.java Outdated
* @return a {@link MultiMap} that contains all stored values in all context in LIFO-compliant order.
* Returned {@link MultiMap} is {@link org.apache.commons.collections.Unmodifiable}.
*/
org.apache.cocoon.el.util.MultiMap getAll();
Copy link
Member

Choose a reason for hiding this comment

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

Didn't you import this class? Why then use the package name?

Copy link
Author

Choose a reason for hiding this comment

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

also fixed.

@ilgrosso
Copy link
Member

ilgrosso commented Apr 4, 2018

Thanks for this PR @ggruber4711 - could you ensure it works well with JDK 6 and JDK 7 as well?

@ggruber4711
Copy link
Author

About the JDK6/7 compatiblity. There is an issue with the testcases in cocoon-template-impl which relies on proper translation of 'january' to polish, which however changed between JDK1.7 and JDK1.8. I will change the testcase to use a german translation instead. Then it should run on older JDKs also.

@ilgrosso
Copy link
Member

ilgrosso commented Apr 4, 2018

I will change the testcase to use a german translation instead.

Good.

Thanks for your changes: I have troubles when applying the changes in

  • core/cocoon-sitemap/cocoon-sitemap-impl/src/test/java/org/apache/cocoon/AbstractTestCase.java
  • core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/core/container/spring/avalon/BridgeElementParser.java

Please review such files so that only the actual changes are reported (currently, it seems that the whole file is changed)

@ggruber4711
COCOON-2347
526cc3d 
changed expected test-results to work with java-versions 6-8
used german locale instead of polish for testing
@ggruber4711
Copy link
Author

I have changed the testcases in cocoon-template-impl to work for JDK 1.6 to JDK 1.8

@ggruber4711
COCOON-2374
6521eb7 
needed to avoid problem with
java.lang.IllegalStateException:
org.apache.cocoon.MockWebApplicationContext@7d898981 has not been
refreshed yet
@ggruber4711
COCOON-2374
4e4aba2 
needed to avoid problem with
java.lang.IllegalStateException:
org.apache.cocoon.MockWebApplicationContext@7d898981 has not been
refreshed yet
@ggruber4711
COCOON-2347
be97630 
set scope to prototype if it is not singleton to work with spring 4.x
@ggruber4711
COCOON-2347 set scope to prototype, if it is no singleton -> should fix
314cc52 
compatibility with spring 4.x
@ilgrosso
Copy link
Member

ilgrosso commented Apr 4, 2018

I have the following error when building via mvn clean verify -P allblocks with Oracle JDK 6 on Linux:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.3:compile (default-compile) on project cocoon-portal-impl: Compilation failure: Compilation failure:
[ERROR] /home/ilgrosso/work/cocoon/2_2_COCOON-2347/blocks/cocoon-portal/cocoon-portal-impl/src/main/java/org/apache/cocoon/portal/spring/AspectsBeanDefinitionParser.java:[95,89] forName(java.lang.String,java.lang.ClassLoader) in org.springframework.util.ClassUtils cannot be applied to (java.lang.String)
[ERROR] /home/ilgrosso/work/cocoon/2_2_COCOON-2347/blocks/cocoon-portal/cocoon-portal-impl/src/main/java/org/apache/cocoon/portal/spring/AspectsBeanDefinitionParser.java:[108,108] cannot find symbol
[ERROR] symbol  : variable BEAN_NAME_DELIMITERS
[ERROR] location: class org.springframework.beans.factory.xml.BeanDefinitionParserDelegate
[ERROR] -> [Help 1]

FYI, on JDK 7 / JDK 8 the command above fails earlier because package com.sun.image.codec.jpeg does not exist in CaptchaReader.java.

mvn clean verify (e.g. without -P allblocks) works fine with all JDKs.

@ggruber4711
COCOON-2347
03956c7 
fixing compile errors after changing to Spring 4.x
@ggruber4711
Copy link
Author

Fixed issue with cocoon-portal-impl

@ilgrosso
Copy link
Member

ilgrosso commented Apr 4, 2018

I have now troubles when patching blocks/cocoon-portal/cocoon-portal-impl/src/main/java/org/apache/cocoon/portal/spring/AspectsBeanDefinitionParser.java: please apply the same fix you did above for AbstractTestCase.java

@ggruber4711
Copy link
Author

ok updated AspectsBeanDefinitionParser.java again.

@ilgrosso
Copy link
Member

ilgrosso commented Apr 4, 2018

LGTM, let me get the diff to SVN...

asfgit pushed a commit that referenced this pull request Apr 4, 2018
@ilgrosso
[COCOON-2347] Fixes made by Gabriel Gruber @ggruber4711 - This fixes #4
c87a744 
git-svn-id: https://svn.apache.org/repos/asf/cocoon/branches/BRANCH_2_2_COCOON-2347@1828352 13f79535-47bb-0310-9956-ffa450edef68
@ilgrosso
Copy link
Member

ilgrosso commented Apr 4, 2018

Merged with c87a744

Please close this PR (I don't have karma to do that), thanks!

@ggruber4711 ggruber4711 closed this Apr 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilgrosso ilgrosso ilgrosso left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@ggruber4711 @ilgrosso