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

Populate default methods, when interfaces are in classpath and not in input directory #101

Closed
wants to merge 0 commits into from

Conversation

teras
Copy link

@teras teras commented Jun 15, 2016

Here is a proposal. I tried to be as lees intrusive as possible. If you wish we can discuss the patch.

@josejuanmontiel
Copy link

Hi,

i suppose that PR try to mitigate the known limitation of: "Also, backporting default methods won't work across module or dependency boundaries."

I'm trying to use retrolamnda with this project: https://code.onehippo.org/cms-community/hippo-cms

I add this (inside build tag) in parent pom.xml

<plugins>
    <plugin>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>3.1</version>
        <configuration>
            <source>1.8</source>
            <target>1.8</target>
            <testExcludes>
                <pattern>**/*Test.java</pattern>
            </testExcludes>
        </configuration>
    </plugin>
    <plugin>
        <groupId>net.orfjackal.retrolambda</groupId>
        <artifactId>retrolambda-maven-plugin</artifactId>
        <version>2.3.1-SNAPSHOT</version>
        <executions>
            <execution>
                <goals>
                    <goal>process-main</goal>
                    <!-- <goal>process-test</goal> -->
                </goals>
            </execution>
        </executions>
        <configuration>
            <target>1.7</target>
        </configuration>
    </plugin>   
</plugins>

after compile your fork (notice 2.3.1-SNAPSHOT) but

[ERROR] /home/jose/git/hippocms/hippo-cms/perspectives/src/main/java/org/hippoecm/frontend/plugins/cms/browse/service/IBrowserSection.java:[30,7] error: cannot access IconProvider

IBrowserSection it's inside hippo-cms-perspectives and IconProvider insede hippo-cms-api, a

<dependency>
  <groupId>org.onehippo.cms7</groupId>
  <artifactId>hippo-cms-api</artifactId>
  <version>${project.version}</version>
</dependency>

for the first module..

when you talk "classpath" talk about "maven dependency"?

To solve my issue i was going to try the maven resources plugin: copy-resources, but i saw your PR and i thought it could solve my problem... maybe not for maven pluging, only command line?

Thanks.

@teras
Copy link
Author

teras commented Oct 9, 2016

I have tried it as a maven plugin and works fine (actually this is the way I use it).
From what I undestand is that the problem is that you are not using the PR at all, but the official retrolambda plugin. Which doesn't do that.

@josejuanmontiel
Copy link

Hi, thanks for answer.

The first problem was i didn't read the logs...

WARNING: The interface org/hippoecm/frontend/skin/IconProvider has a default method "getIcon" but backporting default methods is not enabled

and the second was i don't understood, that the problem was in compilation phase (because the previous backporting of the default method)

[INFO] --- maven-compiler-plugin:3.1:compile (default-compile) @ hippo-cms-perspectives ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 138 source files to /home/jose/git/hippocms/hippo-cms/perspectives/target/classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /home/jose/git/hippocms/hippo-cms/perspectives/src/main/java/org/hippoecm/frontend/plugins/cms/browse/service/IBrowserSection.java:[30,7] error: cannot access IconProvider

But when i "enable defafult method backport in configuration of maven"

        <configuration>
            <defaultMethods>true</defaultMethods>
            ...

The error changed

[ERROR] /home/jose/git/hippocms/hippo-cms/perspectives/src/main/java/org/hippoecm/frontend/plugins/cms/browse/section/BrowsingSectionPlugin.java:[38,7] error: BrowsingSectionPlugin is not abstract and does not override abstract method getIcon(String,IconSize) in IconProvider
[ERROR] /home/jose/git/hippocms/hippo-cms/perspectives/src/main/java/org/hippoecm/frontend/plugins/cms/browse/section/SearchingSectionPlugin.java:[63,7] error: SearchingSectionPlugin is not abstract and does not override abstract method getIcon(String,IconSize) in IconProvider

because decompiling IconProvider...

public abstract interface IconProvider extends IClusterable {
public abstract Component getIcon(String paramString, IconSize paramIconSize);
}

from original source

public interface IconProvider extends IClusterable {
default Component getIcon(final String id, IconSize size) {
return null;
}
}

and

public class BrowsingSectionPlugin extends RenderPlugin<DocumentCollection> implements IBrowserSection {

and

@Override
public ResourceReference getIcon(IconSize type) {
    return null;
}

What do you think... i think my problem isn't with classpath and it's about how default methods are backported...

Did you succed compile the hole proyect?

@teras
Copy link
Author

teras commented Oct 9, 2016

You mean the retrolambda project?
I think you need IntelliJ and even then, I had to make slight modifications to the buildfiles to let me compile it.
But apart from that, it does compile.

@josejuanmontiel
Copy link

Nop, i refer to compile https://code.onehippo.org/cms-community/hippo-cms with retrolambda and default method activated.

i could compiled your PR without problem...

And now, i thing my problem never was about classloading, and yes about this #58

If you wanna talk about this, send me a mail (it's in my github profile)... maybe this issue isn't the best place for now..

@teras
Copy link
Author

teras commented Oct 9, 2016

I'd suggest to try to compile it with the classfiles unzipped first. Then try with this patch.
If you are successful with the former but not the latter we can discuss it some more

@josejuanmontiel
Copy link

Hi @teras can you tell me some public proyect where you use this PR? Thx

@teras
Copy link
Author

teras commented Oct 10, 2016

I don't have any public, sorry.

@josejuanmontiel
Copy link

Hi, could you try with my code repo (hippocms) to ser what happen to me?

Thanks in advance.

@josejuanmontiel
Copy link

@teras here is a full history of my problem... i'm going to try to reproduce with a small mutliproject.. but do you think that could be possible if the name of the default method exist (with another arguments) the retrolambda library not implemente, because think that exists? And thats my problem? Because your PR could search in the maven classpath and solve multimodule problem... but not for my...

@orfjackal what do you think about this?

Thanks to all.

@josejuanmontiel
Copy link

Hi @teras can you try defaultproject.zip this simple project?

is it supposed to works?

defaultproject/module2/src/main/java/com/test/retrolambda/module2/ChildClass.java:[5,8] com.test.retrolambda.module2.ChildClass is not abstract and does not override abstract method getIcon(java.lang.String,java.lang.String) in com.test.retrolambda.module1.ParentClass

Please, could you try it? Could be great that improve this PR to make it work "multimodule" integration via "classpath of maven" ;)

@luontola
Copy link
Owner

Related to #98 and #44

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.

3 participants