Skip to content

Commit

Permalink
DEF-3658 Fixed issue of using incomplete context when resolving conte…
Browse files Browse the repository at this point in the history 
…xt variables (#113)
  • Loading branch information
@mathiaswking
mathiaswking authored Nov 28, 2018
1 parent 0a0656e commit 439bf75
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 23 deletions.
46 changes: 24 additions & 22 deletions server/src/main/java/com/defold/extender/Extender.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
class Extender {
private static final Logger LOGGER = LoggerFactory.getLogger(Extender.class);
private final Configuration config;
private final AppManifestConfiguration appManifest;
private final String appManifestPath;
private final String platform;
private final File sdk;
Expand Down Expand Up @@ -66,36 +65,39 @@ class Extender {
if (appManifests.size() > 1 ) {
throw new ExtenderException("Only one app.manifest allowed!");
}

AppManifestConfiguration appManifest = null;
AppManifestConfiguration baseVariantManifest = null;

if (appManifests.isEmpty()) {
this.appManifestPath = "";
this.appManifest = new AppManifestConfiguration();
appManifest = new AppManifestConfiguration();
} else {
this.appManifestPath = ExtenderUtil.getRelativePath(this.uploadDirectory, appManifests.get(0));
AppManifestConfiguration appManifest = Extender.loadYaml(this.jobDirectory, appManifests.get(0), AppManifestConfiguration.class);
appManifest = Extender.loadYaml(this.jobDirectory, appManifests.get(0), AppManifestConfiguration.class);

// An completely empty manifest will yield a null pointer in result from Extender.loadYaml
this.appManifest = (appManifest != null) ? appManifest : new AppManifestConfiguration();
appManifest = (appManifest != null) ? appManifest : new AppManifestConfiguration();

// An manifest with no platform keyword will yield a null-pointer for this.appManifest.platforms
// This happens if we get a manifest with just the context keyword given.
if (this.appManifest.platforms == null) {
this.appManifest.platforms = new HashMap<String, AppManifestPlatformConfig>();
if (appManifest.platforms == null) {
appManifest.platforms = new HashMap<String, AppManifestPlatformConfig>();
}

// To avoid null pointers later on
if (this.appManifest.platforms.get(platform) == null) {
this.appManifest.platforms.put(platform, new AppManifestPlatformConfig());
if (appManifest.platforms.get(platform) == null) {
appManifest.platforms.put(platform, new AppManifestPlatformConfig());
}
if (this.appManifest.platforms.get(platform).context == null) {
this.appManifest.platforms.get(platform).context = new HashMap<String, Object>();
if (appManifest.platforms.get(platform).context == null) {
appManifest.platforms.get(platform).context = new HashMap<String, Object>();
}

if (this.appManifest.context != null && this.appManifest.context.get(BASE_VARIANT_KEYWORD) instanceof String)
if (appManifest.context != null && appManifest.context.get(BASE_VARIANT_KEYWORD) instanceof String)
{
String baseVariant = (String)this.appManifest.context.get(BASE_VARIANT_KEYWORD);
String baseVariant = (String)appManifest.context.get(BASE_VARIANT_KEYWORD);
File baseVariantFile = new File(sdk.getPath() + "/extender/variants/" + baseVariant + ".appmanifest");

if (!baseVariantFile.exists()) {
throw new ExtenderException("Base variant " + baseVariant + " not found!");
}
Expand Down Expand Up @@ -127,7 +129,7 @@ class Extender {
this.useWine = alternatePlatform.contains("wine32");

this.platformConfig = getPlatformConfig(alternatePlatform);
this.appManifestContext = ExtenderUtil.getAppManifestContext(this.appManifest, platform, baseVariantManifest);
this.appManifestContext = ExtenderUtil.getAppManifestContext(appManifest, platform, baseVariantManifest);
LOGGER.info("Using context for platform: " + alternatePlatform);

processExecutor.setCwd(jobDirectory);
Expand Down Expand Up @@ -446,8 +448,8 @@ private List<File> linkEngine(List<String> symbols, Map<String, Object> manifest

Map<String, Object> mainContext = context(manifestContext);

extSymbols = ExtenderUtil.pruneItems( extSymbols, ExtenderUtil.getAppManifestItems(appManifest, platform, "includeSymbols"), ExtenderUtil.getAppManifestItems(appManifest, platform, "excludeSymbols") );
mainContext.put("symbols", ExtenderUtil.pruneItems( (List<String>)mainContext.get("symbols"), ExtenderUtil.getAppManifestItems(appManifest, platform, "includeSymbols"), ExtenderUtil.getAppManifestItems(appManifest, platform, "excludeSymbols")));
extSymbols = ExtenderUtil.pruneItems( extSymbols, ExtenderUtil.getStringList(mainContext, "includeSymbols"), ExtenderUtil.getStringList(mainContext, "excludeSymbols") );
mainContext.put("symbols", ExtenderUtil.pruneItems( ExtenderUtil.getStringList(mainContext, "symbols"), ExtenderUtil.getStringList(mainContext, "includeSymbols"), ExtenderUtil.getStringList(mainContext, "excludeSymbols")));
mainContext.put("ext", ImmutableMap.of("symbols", extSymbols));

String main = templateExecutor.execute(config.main, mainContext);
Expand Down Expand Up @@ -492,8 +494,8 @@ private List<File> linkEngine(List<String> symbols, Map<String, Object> manifest
}
}

extLibs = ExtenderUtil.pruneItems( extLibs, ExtenderUtil.getAppManifestItems(appManifest, platform, "includeLibs"), ExtenderUtil.getAppManifestItems(appManifest, platform, "excludeLibs"));
extJsLibs = ExtenderUtil.pruneItems( extJsLibs, ExtenderUtil.getAppManifestItems(appManifest, platform, "includeJsLibs"), ExtenderUtil.getAppManifestItems(appManifest, platform, "excludeJsLibs"));
extLibs = ExtenderUtil.pruneItems( extLibs, ExtenderUtil.getStringList(mainContext, "includeLibs"), ExtenderUtil.getStringList(mainContext, "excludeLibs"));
extJsLibs = ExtenderUtil.pruneItems( extJsLibs, ExtenderUtil.getStringList(mainContext, "includeJsLibs"), ExtenderUtil.getStringList(mainContext, "excludeJsLibs"));

String writeExePattern = platformConfig.writeExePattern;
if (writeExePattern == null ) {
Expand All @@ -511,8 +513,8 @@ private List<File> linkEngine(List<String> symbols, Map<String, Object> manifest
context.put("src", objects);
context.put("tgt", ExtenderUtil.getRelativePath(jobDirectory, exe));
context.put("ext", ImmutableMap.of("libs", extLibs, "libPaths", extLibPaths, "frameworks", extFrameworks, "frameworkPaths", extFrameworkPaths, "jsLibs", extJsLibs));
context.put("engineLibs", ExtenderUtil.pruneItems((List<String>) context.getOrDefault("engineLibs", new ArrayList<>()), ExtenderUtil.getAppManifestItems(appManifest, platform, "includeLibs"), ExtenderUtil.getAppManifestItems(appManifest, platform, "excludeLibs")) );
context.put("engineJsLibs", ExtenderUtil.pruneItems((List<String>) context.getOrDefault("engineJsLibs", new ArrayList<>()), ExtenderUtil.getAppManifestItems(appManifest, platform, "includeJsLibs"), ExtenderUtil.getAppManifestItems(appManifest, platform, "excludeJsLibs")) );
context.put("engineLibs", ExtenderUtil.pruneItems(ExtenderUtil.getStringList(context, "engineLibs"), ExtenderUtil.getStringList(mainContext, "includeLibs"), ExtenderUtil.getStringList(mainContext, "excludeLibs")) );
context.put("engineJsLibs", ExtenderUtil.pruneItems(ExtenderUtil.getStringList(context, "engineJsLibs"), ExtenderUtil.getStringList(mainContext, "includeJsLibs"), ExtenderUtil.getStringList(mainContext, "excludeJsLibs")) );

// WINE->clang transition pt1: in the transition period from link.exe -> lld, we want to make sure we can write "foo" as opposed to "foo.lib"
context.put("libs", patchLibs((List<String>) context.get("libs")));
Expand Down Expand Up @@ -754,8 +756,8 @@ private File[] buildClassesDex(List<File> extraJars) throws ExtenderException {
Map<String, Object> context = context(empty);
context.put("classes_dex", classesDex.getAbsolutePath());
context.put("classes_dex_dir", buildDirectory.getAbsolutePath());
context.put("jars", ExtenderUtil.pruneItems( extJars, ExtenderUtil.getAppManifestItems(appManifest, platform, "includeJars"), ExtenderUtil.getAppManifestItems(appManifest, platform, "excludeJars")));
context.put("engineJars", ExtenderUtil.pruneItems( (List<String>)context.get("engineJars"), ExtenderUtil.getAppManifestItems(appManifest, platform, "includeJars"), ExtenderUtil.getAppManifestItems(appManifest, platform, "excludeJars")) );
context.put("jars", ExtenderUtil.pruneItems( extJars, ExtenderUtil.getStringList(appManifestContext, "includeJars"), ExtenderUtil.getStringList(appManifestContext, "excludeJars")));
context.put("engineJars", ExtenderUtil.pruneItems( (List<String>)context.get("engineJars"), ExtenderUtil.getStringList(appManifestContext, "includeJars"), ExtenderUtil.getStringList(appManifestContext, "excludeJars")) );

String command = templateExecutor.execute(platformConfig.dxCmd, context);
try {
Expand Down
27 changes: 26 additions & 1 deletion server/src/main/java/com/defold/extender/ExtenderUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,18 @@ static void debugPrint(Map<String, Object> map, int indent) {
}
}

static void debugPrint(String name, List<String> l) {
if (l == null) {
System.out.println(String.format("%s: <null>", name));
return;
}
System.out.print(String.format("%s: [", name));
for (String v : l) {
System.out.print(String.format("%s, ", v));
}
System.out.println("]");
}

static File[] listFilesMatching(File dir, String regex) {
if(!dir.isDirectory()) {
throw new IllegalArgumentException(dir+" is not a directory.");
Expand Down Expand Up @@ -210,6 +222,19 @@ static Boolean getAppManifestBoolean(AppManifestConfiguration manifest, String p
return default_value;
}

private static boolean isListOfStrings(List<Object> list) {
return list != null && list.stream().allMatch(o -> o instanceof String);
}

static List<String> getStringList(Map<String, Object> context, String key) throws ExtenderException
{
Object v = context.getOrDefault(key, new ArrayList<>());
if (v instanceof List && isListOfStrings((List<Object>)v)) {
return (List<String>)v;
}
throw new ExtenderException(String.format("The context variables only support strings or lists of strings. Key %s: %s (type %s)", key, v.toString(), v.getClass().getCanonicalName()));
}

// Does a regexp match on the filename for each file found in a directory
static public List<String> collectFilesByName(File dir, String re) {
List<String> result = new ArrayList<>();
Expand Down Expand Up @@ -247,4 +272,4 @@ static public List<String> collectFilesByPath(File dir, String re) {
}
return result;
}
}
}

0 comments on commit 439bf75

Please sign in to comment.