allow sub-classes of project to provide repos

[chrisrueger]

Closes #6481

• add method Project.getRepositories()
• override it in BndRun to provide a different list of plugins than Project

[chrisrueger]

@laeubi as a base for discussion for #6481
Would that help you?

[laeubi]

Looks useful!

[chrisrueger]

@laeubi ok thanks for having a look.
I am not sure about if my default implementation in BndRun is good as I have it now.

Currently it is almost doing the same as what Workspace (aQute.bnd.build.Workspace.WorkspaceData.WorkspaceData(Workspace) and aQute.bnd.build.Workspace.initRepositories()) is doing (which is what gets returns by Workspace.getRepositories()

The "problem" with Workspace.getRepositories() returns cached data (see the Memoize in aQute.bnd.build.Workspace.WorkspaceData.WorkspaceData(Workspace)), so it is only fetched once.

I think in case of my change in BndRun.getRepositories() it maybe fine, since it is more a one of kind of thing and does not need that heavy caching compared to Project which is called all over the place.

Or should I first call something like super.getRepositories() to get the stuff from the parent Project and then add whatever we added specifically for the current run instance?

Thoughts welcome

[pkriens]

I think this is going to have backward compatibility issues. The idea that repositories come from the workspace is pretty deep engrained in a lot of code. A bndrun is actually a Project AND a Workspace.

This needs a lot of testing with existing builds.

[chrisrueger]

I think this is going to have backward compatibility issues. The idea that repositories come from the workspace is pretty deep engrained in a lot of code. A bndrun is actually a Project AND a Workspace.

This needs a lot of testing with existing builds.

Ok thanks for your feedback. It was more of a idea / proposal at first.
So do you see a more defensive approach?

Maybe: should sub-classes like BndRun just be able to "add" Repositories instead of passing the complete list? (so that the Workspace repos would always be used as it is at the moment, but sub-classes can add additional ones.)

[pkriens]

Bndrun is special, the -standalone was a hack. But we really needed to make a synthetic workspace I recall.

I see the usefulness of adding repositories on a project but I see a lot of issues on that road. Key thing is that is must be backward compatible. And the plugins have a peculiar inheritance model. Since repositories can take significant initialization time you also have to handle the issue that the repo might not be ready. We handle that for workspace repos.

Lots of bears ...

[pkriens]

Also notice that this repository will not be visible in the repositoriesview and it might be problematic for the resource view.

[laeubi]

I know that there are cornercases but wanted to mention that I explicitly use this as some kind of "commandline action" or better as running something programatically thats also why I try to avoid to pollute the original workspace.

As you say bndruns are project+workspace, I wonder if I can add the plugin to the bndruns workspace without affecting the one passed in the constructor?

[pkriens]

You should be able to add a new repository with addBasicPlugin(Object) to any Processor. These are automatically added to the PluginContainer, also after a refresh

[chrisrueger]

You should be able to add a new repository with addBasicPlugin(Object) to any Processor.

I think this is what @laeubi is trying to do but it fails:

#6481 (comment)

Bndrun run = ...
run.addBasicPluging(new MyRepositoryPlugin())

But that does not seem to work, because the Repositories of

bnd/biz.aQute.bndlib/src/aQute/bnd/build/Project.java

Line 1281 in 5549ecf

List<RepositoryPlugin> plugins = workspace.getRepositories();

are cached (Memoize)

These are automatically added to the PluginContainer, also after a refresh

Maybe that is the missing piece in the puzzle... How would that refresh look?

Bndrun run = ...
run.addBasicPluging(new MyRepositoryPlugin())
run.refresh()

or something like that?
or project.refreshAll() maybe, which seems to refresh the workspace too.

bnd/biz.aQute.bndlib/src/aQute/bnd/build/Project.java

Line 2699 in 5549ecf

public void refreshAll() {

[laeubi]

Just take this simple testcase:

public class TestMe {

	public static void main(String[] args) throws Exception {
		Workspace workspace = Workspace.createDefaultWorkspace();
		Bndrun bndrun = Bndrun.createBndrun(workspace, null);
		bndrun.addBasicPlugin(new MyPlugin());
		System.out.println("Bndrun: " + bndrun.getPlugin(MyPlugin.class));
		System.out.println("Workspace: " + workspace.getPlugin(MyPlugin.class));
		workspace.refresh();
		System.out.println("Workspace after refresh: " + workspace.getPlugin(MyPlugin.class));
	}

	private static final class MyPlugin {

	}
}

Output is:

Bndrun: biz.aQute.resolve.TestMe$MyPlugin@4df50bcc
Workspace: null
Workspace after refresh: null

As said it works perfectly fine for the resolve case, because that is using getPlugins( ... ) while getBundle is using workspace.getPlugins(...) ... and thus things added to the Bndrun will not be returned.

Repositories are memorized, yes but in this case the RepositoryPlugins itself can not be found, so a refresh do not change much here...

[chrisrueger]

Just take this simple testcase:

Thanks for the testcase @laeubi

@pkriens what do you think? Is the expectation of this testcase reasonable?

[laeubi]

@pkriens what do you think? Is the expectation of this testcase reasonable?

The testacse does not expect anything, it just shows that adding a plugin to a Bndrun and then query for a plugin through workspace does not return the plugin. This is actually expected to me.
But it highlights the problem that calling workspace.getRepositories() will miss RepoPlugins added to the Bndrun directly so bndrun.getBundle() returns nothing for such bundles (that is unexpected for me)

[chrisrueger]

that is unexpected for me

you are right, I was not precise in my words. I just wanted to find out, if we all agree that this is the problem we are trying to solve.

[chrisrueger]

@laeubi I tried adding your example as a testcase in e56d802

Does it describe the problem correctly?

Feel free to suggest changes or improvements.