Issue 5725: Added support for commonsrc+pluginsrc folders #197
Conversation
Can build .proto files into engine + plugin source Can generate a plugin .jar file
| apt-get install -y --no-install-recommends \ | ||
| # setuptools for protobuf builder | ||
| python-setuptools | ||
|
|
There was a problem hiding this comment.
I should move this higher up in the Dockerfile, together with the other python related stuff.
| @@ -145,7 +151,10 @@ | |||
| this.platform = platform; | |||
| this.sdk = sdk; | |||
|
|
|||
| // TODO: Add a way to merge these platform configs: common -> platform -> arch-platform | |||
| this.commonPlatformConfig = config.platforms.get("common"); | |||
There was a problem hiding this comment.
Not ideal to have two variables, but I'll add a merge function later.
| v = templateExecutor.execute(v, envContext); | ||
| processExecutor.putEnv(k, v); | ||
| } | ||
| } |
There was a problem hiding this comment.
If we have a merge function for platform configs, this wouldn't be needed.
| @@ -320,6 +339,9 @@ static boolean isListOfStrings(List<Object> list) { | |||
| private Map<String, Object> context(Map<String, Object> manifestContext) throws ExtenderException { | |||
| Map<String, Object> context = new HashMap<>(config.context); | |||
|
|
|||
| if (commonPlatformConfig != null) { | |||
| context = Extender.mergeContexts(context, commonPlatformConfig.context); | |||
| } | |||
There was a problem hiding this comment.
The "context" is a simple Map<String, Object>, and we already have a merge function for those.
| return includes; | ||
| } | ||
|
|
||
| private File addCompileFileCpp_Internal(int index, File extDir, File src, Map<String, Object> manifestContext, String cmd, List<String> commands) throws IOException, InterruptedException, ExtenderException { |
There was a problem hiding this comment.
The old name "compileFile" was now a misnomer, since it only sets up the command.
| LOGGER.info("No C++ source found for plugin. Skipping {}", extDir); | ||
| } else { | ||
| // We leave the C++ protobuf support for a later date | ||
| List<File> generatedFiles = new ArrayList<>();//generateProtoSrcForPlugin(extDir, manifestContext, protoFiles, "cpp"); |
There was a problem hiding this comment.
Currently disabled until later.
We want to be able to use C++ code to transform data (e.g. textures)
| String command = templateExecutor.execute(commonPlatformConfig.jarCmd, context); | ||
| processExecutor.execute(command); | ||
|
|
||
| outputFiles.add(outputJar); |
There was a problem hiding this comment.
List the output files that are to be sent back to the client.
| for (String extensionSymbol : keys) { | ||
| symbols.add(extensionSymbol); | ||
|
|
||
| List<String> symbols = getSortedKeys(manifestConfigs.keySet()); |
There was a problem hiding this comment.
We sort them so that it's easier to compare builds when debugging
| @@ -1715,6 +1971,7 @@ else if (platform.contains("web")) { | |||
| outputFiles.addAll(buildAndroid(platform)); | |||
| } | |||
| outputFiles.addAll(buildEngine()); | |||
| outputFiles.addAll(buildPipelinePlugin()); | |||
There was a problem hiding this comment.
Send back all generated plugins (currently .jar file and "_pb2.py" files)
| @@ -86,4 +87,34 @@ public void setCwd(File cwd) { | |||
| public void putLog(String msg) { | |||
| output.append(msg); | |||
| } | |||
|
|
|||
| public static void executeCommands(ProcessExecutor processExecutor, List<String> commands) throws IOException, InterruptedException, ExtenderException { | |||
There was a problem hiding this comment.
Moved from Extender.java
|
|
||
| static final String FOLDER_ENGINE_SRC = "src"; // source for the engine library | ||
| static final String FOLDER_PLUGIN_SRC = "pluginsrc";// source for the pipeline/format plugins | ||
| static final String FOLDER_COMMON_SRC = "commonsrc";// common source shared between both types |
There was a problem hiding this comment.
These names are now part of the api.
Are they good enough?
There was a problem hiding this comment.
Is plugin the word we are going to use? When talking to users or when users discuss Defold I see the following three used to describe what we call a Library project in the documentation:
- (Native) extension
- library
- plugin
- dependency
And soon an extension project will also contain source code for the editor right? That folder would probably be named editorsrc?
What about buildsrc, buildersrc or pipelinesrc?
commonsrc is good I think. It could also be named sharedsrc but I believe we use common elsewhere to indicate shared things.
There was a problem hiding this comment.
"plugin" was a collecting term for all plugins (bob and editor)
"buildsrc" is too ambigous I think. All source is built.
"pipelinesrc" is better.
I don't think we'll want a separate "editorsrc". I consider that part still a "plugin"
There was a problem hiding this comment.
I think it's good to separate the meanings of the the terms (as we already do).
I wouldn't want to call everything "extensions" or "libraries".
Also, I don't really consider them confusing.
But I agree that we should be more specific when referring to them
- (Defold) library
- Dependency is the link to a library
- (Native) extension - statically linked source code. Avaialble through the Defold Library system.
- Plugin (a separate file/bundle to add code)
There was a problem hiding this comment.
I chose "pluginsrc" over "pipelinesrc" because it was shorter, but more importantly because "plugin" is a well known concept. But as you mention, it might be confusing for others.
| objs.add(ExtenderUtil.getRelativePath(jobDirectory, o)); | ||
| i++; | ||
| } | ||
| ProcessExecutor.executeCommands(processExecutor, commands); // in parallel |
There was a problem hiding this comment.
Should the name of the function indicate parallelism somehow? So the comments aren't necessary
Can build .proto files into engine + plugin source
Can generate a plugin .jar file