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

Guava + failureaccess resolution error #675

Closed
jonahgraham opened this issue Jul 21, 2023 · 9 comments
Closed

Guava + failureaccess resolution error #675

jonahgraham opened this issue Jul 21, 2023 · 9 comments

Comments

@jonahgraham
Copy link
Contributor

Linux Tools has recently been updating their dependencies to 3rd party to use Maven. CDT started having a build failure resolving the target platform with an error like this:

Error:  Cannot resolve target definition:
Error:    Software being installed: org.eclipse.linuxtools.docker.feature.feature.group 5.12.0.202307201237
Error:    Missing requirement: com.google.guava 32.1.1.jre requires 'java.package; com.google.common.util.concurrent.internal [1.0.0,2.0.0)' but it could not be found
Error:    Cannot satisfy dependency: org.eclipse.linuxtools.docker.core 5.12.0.202307201237 depends on: osgi.bundle; com.google.guava 32.1.1
Error:    Cannot satisfy dependency: org.eclipse.linuxtools.docker.feature.feature.group 5.12.0.202307201237 depends on: org.eclipse.equinox.p2.iu; org.eclipse.linuxtools.docker.core [5.12.0.202307201237,5.12.0.202307201237]

The cdt target platform is pretty big, so this is my attempt to create a minimum version:

<target name="cdt" sequenceNumber="143">
	<locations>
		<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
			<repository location="https://download.eclipse.org/eclipse/updates/4.29-I-builds/I20230705-1800/"/>
			<unit id="org.eclipse.equinox.executable.feature.group" version="0.0.0"/>
		</location>
		<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
			<repository location="https://download.eclipse.org/linuxtools/updates-docker-nightly/"/>
			<unit id="org.eclipse.linuxtools.docker.core" version="0.0.0"/>
		</location>
		<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
			<repository location="https://download.eclipse.org/tools/cdt/releases/11.2" />
		</location>
		<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
			<repository location="https://download.eclipse.org/tools/orbit/downloads/latest-R/" />
		</location>
		<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
			<repository location="https://download.eclipse.org/tools/orbit/downloads/drops/R20201118194144/repository/" />
		</location>

	</locations>
	<targetJRE path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-17"/>
</target>

With the above target file you get the resolution error Missing requirement: com.google.guava 32.1.1.jre requires 'java.package; com.google.common.util.concurrent.internal [1.0.0,2.0.0)' but it could not be found - com.google.common.util.concurrent.internal is provided by failureaccess, so if I add:

		<location includeDependencyDepth="none" includeDependencyScopes="compile" includeSource="true" label="DirectFromMaven" missingManifest="error" type="Maven">
			<dependencies>
				<dependency>
					<groupId>com.google.guava</groupId>
					<artifactId>failureaccess</artifactId>
					<version>1.0.1</version>
					<type>jar</type>
				</dependency>
			</dependencies>
		</location>

I expect it to resolve properly. But it doesn't work. However adding:

		<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
			<repository location="https://download.eclipse.org/tools/orbit/simrel/orbit-aggregation/nightly"/>
			<unit id="com.google.guava.failureaccess" version="0.0.0" />
		</location>

does work fine. The JAR file for both those resolutions are the same.

jonahgraham added a commit to jonahgraham/cdt that referenced this issue Jul 21, 2023
The commit resolves the resolution error that was raised in
eclipse-linuxtools/org.eclipse.linuxtools#232

I have raised an issue in PDE here:
eclipse-pde/eclipse.pde#675
jonahgraham added a commit to eclipse-cdt/cdt that referenced this issue Jul 21, 2023
The commit resolves the resolution error that was raised in
eclipse-linuxtools/org.eclipse.linuxtools#232

I have raised an issue in PDE here:
eclipse-pde/eclipse.pde#675
@HannesWell
Copy link
Member

This is strange.
In m2e we use guava 32.0.1 with failure-access and I didn't notice any issue:
https://github.com/eclipse-m2e/m2e-core/blob/master/target-platform/target-platform.target

I can try your example in the next days and see if I find something.

@jonahgraham
Copy link
Contributor Author

This is strange.

Yes - I think it is just weird, probably some strange interaction. Linuxtools uses Guava 32.1.1.jre it their build and it works fine. I don't know how to make it standalone reproducible so that it isn't subject to the changes in the various nightly p2 urls my example references.

I can try your example in the next days and see if I find something.

Thanks!

@merks
Copy link
Contributor

merks commented Jul 22, 2023

This very minimal example also reproduces the problem:

<target name="minimal" sequenceNumber="0">
	<locations>
		<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
			<repository location="https://download.eclipse.org/linuxtools/updates-docker-nightly/"/>
			<unit id="com.google.guava" version="0.0.0"/>
		</location>

		<location includeDependencyDepth="none" includeDependencyScopes="compile" includeSource="true" label="DirectFromMaven" missingManifest="error" type="Maven">
			<dependencies>
				<dependency>
					<groupId>com.google.guava</groupId>
					<artifactId>failureaccess</artifactId>
					<version>1.0.1</version>
					<type>jar</type>
				</dependency>
			</dependencies>
		</location>
	</locations>
</target>

I have a theory for why that might be...

@merks
Copy link
Contributor

merks commented Jul 22, 2023

This one works:

<target name="minimal2" sequenceNumber="0">
	<locations>
		<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
			<repository location="https://download.eclipse.org/linuxtools/updates-docker-nightly/"/>
			<unit id="com.google.guava" version="0.0.0"/>
		</location>

		<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
			<repository location="https://download.eclipse.org/tools/orbit/simrel/maven-osgi/nightly/latest"/>
			<unit id="com.google.guava.failureaccess" version="0.0.0"/>
		</location>
	</locations>
</target>

And interestingly, the resolution shows this:

image

Which might lead one to believe that com.google.guava.failureaccess IU is available in the linuxtools repository, but of course that's not the case or we would not have this problem. That pretty much confirms my theory...

Looking in the debugger why this works, we can see that during the resolution of the InstallableUnit location, both repositories are made available for the planner:

image

But for the failure case, that's of course not the case:

image

For Oomph's targlets, I also ran into this problem when I added support for composing other *.target files. For that case I needed to resolve the other target and make its IUs available as if they were provided by a p2 repository during subsequent overall resolution of the composed targlets. But PDE is doing no such thing; each location is effectively resolved independently, though with special case handling for IUBundleContainer by org.eclipse.pde.internal.core.target.P2TargetUtils.getMetadataRepositories(ITargetDefinition), which gathers all the update sites.

I suppose PDE might be made smarter such that it resolves all non-IUBundleContainer containers first, and is able to synthesis a p2 repository with all the resolved IUs and make those available when resolving the remaining IUBundleContainers. That sounds quite non-trivial...

@akurtakov

I'm sure the linuxtools repository will want to fix this problem because otherwise consumers will not be able to install the tools from this repository without some other repository providing the missing requirements. Also, the repository contains two versions of com.google.guava which is likely also not desired:

image

@laeubi
Copy link
Contributor

laeubi commented Jul 22, 2023

This is not magic or strange at all. P2 Locations simply don't care about other (non UI location) at all see:

(By the way Tycho does not suffer from this restriction)

I suppose PDE might be made smarter such that it resolves all non-IUBundleContainer containers first, and is able to synthesis a p2 repository with all the resolved IUs and make those available when resolving the remaining IUBundleContainers. That sounds quite non-trivial...

Actually it is not that complex but would require a two phase resolve operation, first collect and resolve all direct items and then perform the planner operation afterwards, that's how Tycho do it. But all this is behavioral change, probably breaking change and that's where it becomes not so trivial for anyone not liking getting punished for that.

@jonahgraham
Copy link
Contributor Author

This is therefore a duplicate of #207 and I will close it as such. I think as we encourage other projects to migrate more people will see issues like this.

I will leave @akurtakov to decide whether to reopen eclipse-linuxtools/org.eclipse.linuxtools#232 to make linuxtools repo self contained.

@jonahgraham jonahgraham closed this as not planned Won't fix, can't repro, duplicate, stale Jul 22, 2023
@akurtakov
Copy link
Member

I am on vacation and hope to not forget when I am back to add the missing guava dep.

@laeubi
Copy link
Contributor

laeubi commented Jul 22, 2023

This is therefore a duplicate of #207 and I will close it as such. I think as we encourage other projects to migrate more people will see issues like this.

As said its is actually possible, but it will need conses if it should be changed as otherwise will end up in endless discussions and frustrated contributors ;-)

@HannesWell
Copy link
Member

This is not magic or strange at all. P2 Locations simply don't care about other (non UI location) at all see:

Right. Thanks for pointing out, I forgot that again.

In general I think it would be good to add support for that, but lets continue the discussion about that in #207.

davmac314 pushed a commit to davmac314/cdt that referenced this issue Sep 24, 2023
The commit resolves the resolution error that was raised in
eclipse-linuxtools/org.eclipse.linuxtools#232

I have raised an issue in PDE here:
eclipse-pde/eclipse.pde#675
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

5 participants