Skip to main content

Notice: this Wiki will be going read only early in 2024 and edits will no longer be possible. Please see: https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/wikis/Wiki-shutdown-plan for the plan.

Jump to: navigation, search

CDT/Obsolete/CoreBuildCommandLauncher

< CDT‎ | Obsolete
Revision as of 16:00, 23 September 2017 by Slewis.composent.com (Talk | contribs) (Created page with "==What Is The Issue?== [https://bugs.eclipse.org/bugs/show_bug.cgi?id=513589 bug513589] was introduced to support the use of docker containers for doing CDT builds. Jeff Jo...")

(diff) ← Older revision | Latest revision (diff) | Newer revision → (diff)

What Is The Issue?

bug513589 was introduced to support the use of docker containers for doing CDT builds. Jeff Johnston has implemented org.eclipse.cdt.docker.launcher, which depends upon linuxtools project's docker APIs for the managed builder. It does this by creating a ContainerCommandLauncher class implementing ICommandLauncher which delegates any build/clean commands to an underlying docker container. Impls of ICommandLauncher like ContainerCommandLauncher are introduced via the ICommandLauncherFactory extension point.

What About Core Builder?

It seems that the core builder is the future for cdt building.

With the relatively new core build (api in package org.eclipse.cdt.core.build) the architecture is somewhat different from the managed builder. When the org.eclipse.cdt.core.build.CBuilder.build method is called (via core build infrastructure as an IncrementalProjectBuilder extension), it gets the org.eclipse.cdt.core.build.ICBuildConfiguration adapter from the IBuildConfiguration on the project, and then simply calls ICBuildConfiguration.build(...). Same with clean. This is implemented in the org.eclipse.cdt.core.build.CBuilder class. The ICBuildConfiguration instance associated with a project upon construction is thus the entry point for building each project. This approach is clean and simple, and allows more extensibility for building different types of projects. The reason is that each project type (CMake, Yocto, Qt, etc) can easily provide it's own special implementation of ICBuildConfiguration (via the ICBuildConfigurationProvider extension point) which does it's build/clean in a custom way.

The ICBuildConfiguration interface extends IAdaptable, meaning that it has a getAdapter(Class<?>) method. This means that existing ICBuildConfiguration impls can be adapted at runtime rather than extended at compile time, making it possible to add functionality (adapters) to existing ICBuildConfigurations without updating existing impls.

ICommandLauncher and Core Builder

Currently, there is no connection between the core builder and an ICommandLauncher impl. I think this is the primary change that has to be made in CDT core to support docker-based project builds via core builder, but also any other future/other impls of ICBuildConfiguration.build/clean via core builder. The association has to be between ICBuildConfiguration and ICommandLauncher because these are the two types that are both in org.eclipse.cdt.core bundle. It would not make any sense to require docker launcher to make this API association, because there may/should eventually be other impls of ICommandLauncher that use non-docker containers or some other remoting protocol (e.g. lsp) or toolchain-specific docker containers (e.g. yocto). Also I expect it's possible that multiple types of ICommandLauncher could be available to a project and the user could switch between them via ui -> configuration changes.

Note in contrast to the managed builder that a factory extension point (ICommandLauncherFactory) *alone* won't work well for the core builder, because the core builder delegates the build/clean to the project-specific ICBuildConfiguration instance. For a ICommandLauncherFactory to work, there would have to be a new factory extension for each new type of ICBuildConfiguration. Further, an extension-based association would be static, and would not support runtime switching among different types of ICommandLauncher.

Proposal

I propose that a runtime association be made between ICBuildConfiguration and ICommandLauncher. There several ways to implement such an association:

1) Create an adapter interface (ICBuildCommandLauncherConfiguration with a single method ICommandLauncher.getCommandLauncher()) and implement to ICBuildConfiguration.getAdapter(ICBuildCommandLauncherConfiguration.class) to return non-null instances. This is what I initially proposed via bug522376 in gerrit change 105239.

2) Add ICommandLauncher getCommandLauncher() to ICBuildConfiguration. This is what I was proposing in bug522376 comment3.

1 is more complicated for the programmer (i.e. getAdapter(ICBuildCommandLauncherConfiguration.class) has to be called first, then getCommandLauncher() to check for a non-null ICommandLauncher, but according to Eclipse API migration rules 2 requires a major rather than minor version change for org.eclipse.cdt.core.

Getting some association (1 or 2) between ICBuildConfiguration and ICommandLauncher is necessary to support docker container support for core builder-based projects, but any other possible type of ICommandLauncher (e.g. yocto-specific customized ContainerCommandLauncher).

Second Problem

Assume that we select '2' above and add ICommandLauncher getCommandLauncher() to ICBuildConfiguration interface. Unfortunately this would still mean that each ICBuildConfiguration impl ...that wanted to provide a non-null ICommandLauncher (within the ICBuildConfiguration.build/clean methods) would have to specify the type when the ICBuildConfiguration impl was created (upon workbench startup). That is, the ICBuildConfiguration provider would have to create an instance of a desired type at the type of creation of ICBuildConfiguration. That also means that every core build configuration type would have to do that separately, and that they would have to specify the types at compile time. And if they used ContainerCommandLauncher for example, they would have a compile-time dependency on docker.launcher, meaning that docker.launcher would be required to be present just to create a (e.g.) CMake ICBuildConfiguration. Not very good.

What about IToolChain?

The core build has the concept of an IToolChain, which is an extension point that allows new tool chains to be introduced via additional plugins and existing toolchains to be reused across project types and their associated ICBuildConfiguration impls. An IToolChain is associated with a ICBuildConfiguration typically upon construction by the cbuild configuration provider. There is also a method: IToolChain getToolChain() on ICBuildConfiguration to get the associated tool chain for that core build configuration instance.

IToolChain also extends IAdaptable, making it possible to create an adapter allowing an association between an IToolChain and ICommandLauncher. As described in bug522376 comment 3 it would be possible to create an IToolChainCommandLauncherAdapter (or perhaps just ICommandLauncherAdapter) with signature:

ICommandLauncher getCommandLauncher();

that could be used via IToolChain.getAdapter(IToolChainCommandLauncherAdapter.class) to delegate the getCommandLauncher() to an IToolChain rather than to the ICBuildConfiguration. Via the IAdapterFactory, it would also allow ICommandLaunchers (like ContainerCommandLauncher) to be made available without modifying the IToolChain impl. For example as described in bug522376 comment 3 the docker launcher plugin could make available an IToolChainCommandLauncherAdapter that returned a ContainerCommandLauncher without modifying existing IToolChain impls but rather by setting up a IAdapterFactory that adapted all IToolChains to IToolChainCommandLauncherAdapter and returned a ContainerCommandLauncher. Then there would be no compile-time dependencies on docker.launcher except within docker.launcher code...so if docker.launcher was not present, there would be no adapter for IToolChainCommandLauncherAdapter...IToolChain.getAdapter(IToolChainCommandLauncherAdapter.class) returns null.

Copyright © Eclipse Foundation, Inc. All Rights Reserved.