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

Support sub bundles build #940

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Support sub bundles build #940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ui/org.eclipse.pde.core/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %name
Bundle-SymbolicName: org.eclipse.pde.core; singleton:=true
Bundle-Version: 3.17.300.qualifier
Bundle-Version: 3.17.400.qualifier
Copy link
Member

Choose a reason for hiding this comment

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

Why have you changed it? This causes now API error because it was already changes for the 4.31 release.

Please consider quickly revert this before next IBuild kicks in.

Copy link
Member

Choose a reason for hiding this comment

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

I've set #948

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why have you changed it? This causes now API error because it was already changes for the 4.31 release.

Why it not failed this PR?

Copy link
Member

Choose a reason for hiding this comment

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

I guess this is yet another API tools mystery.

Copy link
Member

Choose a reason for hiding this comment

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

There was another issue with API error not being reported, see eclipse-jdt/eclipse.jdt.ui#914 .
Note that eclipse-jdt/eclipse.jdt.ui#914 is reported in SDK build and in the IDE.
So it could be some new tycho build issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was another issue with API error not being reported

The green build reports it as a warning

[2023-11-29T07:02:13.259Z] [WARNING] [API WARNING] File MANIFEST.MF at line 5: The service version is increased unnecessarily since either the major or minor or service version is already increased (location: /home/jenkins/agent/workspace/eclipse.pde_PR-940/ui/org.eclipse.pde.core/META-INF/MANIFEST.MF)

So it could be some new tycho build issue.

API tools is provided by PDE not Tycho, Tycho just provides a wrapper around it, so either it is configured differently WARNING vs ERROR or the configuration is not picket up by Tycho @vik-chand might better know if/how/where this is to be configured.

Copy link
Member

Choose a reason for hiding this comment

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

Tycho just provides a wrapper around it

That is what I've meant with SDK/IDE build showing error.

I have no idea if it is the wrapper or the tycho config or whatever else.
API tooling in the IDE works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have now debugged the case and the problem is that ApiProblem.getSeverity() always return the default as it passes null as a project... Why it works in IDE/SDK ist because here Error Markers are used and when ApiTools converts ApiProblem > Error Marker it does IGNORE ApiProblem.getSeverity() and instead computes the value again but this time using current build project... will replicate this behaviour with Tycho as well ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually ApiProblem.getSeverity() is only used in two places:

  1. ApiProblem.toString (what will print wrong value then or at least ignores the project settings)
  2. In a org.eclipse.pde.api.tools.problems.tests.ApiProblemTests.testGetSeverity()

so actually I think that method can completely be deleted to prevent further confusion...

Bundle-Activator: org.eclipse.pde.internal.core.PDECore
Bundle-Vendor: %provider-name
Bundle-Localization: plugin
Expand Down
30 changes: 29 additions & 1 deletion ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/bnd/BndBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*******************************************************************************/
package org.eclipse.pde.internal.core.bnd;

import java.io.File;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
Expand Down Expand Up @@ -42,6 +44,8 @@

import aQute.bnd.build.Project;
import aQute.bnd.build.ProjectBuilder;
import aQute.bnd.osgi.Builder;
import aQute.bnd.osgi.Jar;

public class BndBuilder extends IncrementalProjectBuilder {

Expand Down Expand Up @@ -154,8 +158,32 @@ public void addClasspath(aQute.bnd.osgi.Jar jar) {
builder.setBase(bnd.getBase());
ProjectJar jar = new ProjectJar(project, CLASS_FILTER);
builder.setJar(jar);
// build the main jar
builder.build();
new BndErrorReporter(project, bnd).validateContent(monitor);
new BndErrorReporter(project, bnd, project.getFile(BndProject.INSTRUCTIONS_FILE))
.validateContent(monitor);
// now build sub jars
List<Builder> subBuilders = builder.getSubBuilders();
if (subBuilders.size() > 0) {
for (Builder subBuilder : subBuilders) {
File outputFile = subBuilder.getOutputFile(null);
if (outputFile != null) {
Jar subJar = subBuilder.build();
subJar.write(outputFile);
for (IFile file : project.getWorkspace().getRoot()
.findFilesForLocationURI(outputFile.toURI())) {
file.refreshLocal(IResource.DEPTH_ZERO, monitor);
}
File propertiesFile = subBuilder.getPropertiesFile();
if (propertiesFile != null) {
for (IFile file : project.getWorkspace().getRoot()
.findFilesForLocationURI(propertiesFile.toURI())) {
new BndErrorReporter(project, subBuilder, file).validateContent(monitor);
}
}
}
}
}
}
if (monitor.isCanceled()) {
return;
Expand Down
6 changes: 3 additions & 3 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/bnd/BndErrorReporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
*******************************************************************************/
package org.eclipse.pde.internal.core.bnd;

import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.jface.text.IDocument;
import org.eclipse.pde.internal.core.builders.CompilerFlags;
import org.eclipse.pde.internal.core.builders.ErrorReporter;
import org.eclipse.pde.internal.core.builders.PDEMarkerFactory;
import org.eclipse.pde.internal.core.natures.BndProject;

import aQute.bnd.properties.IRegion;
import aQute.bnd.properties.LineType;
Expand All @@ -32,8 +32,8 @@ public class BndErrorReporter extends ErrorReporter {
private BndDocument bndDocument;
private final Report report;

public BndErrorReporter(IProject project, Report report) {
super(project.getFile(BndProject.INSTRUCTIONS_FILE));
public BndErrorReporter(IProject project, Report report, IFile file) {
super(file);
this.report = report;
IDocument document = createDocument(fFile);
if (document != null) {
Expand Down
Loading