Skip to content

Commit ae13f84

Browse files
danengelbrecht-kingdanengelbrecht-king
authored
DEF-2477 Support for context/baseVariant in app.manifest (#100)
* Support for __base_variant in app.manifest * Handle empty manifest files gracefully * Better comments when parsing app manifest * Moved __base_variant keyword into context key/value map Review fixes * Safer merging of base manifests * Removed dead manifest items * Apply base app manifest options first, then add user options
1 parent 7664c66 commit ae13f84

7 files changed

Lines changed: 111 additions & 12 deletions

File tree

‎server/src/main/java/com/defold/extender/Extender.java‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ class Extender {
3737
private List<File> extDirs;
3838
private List<File> manifests;
3939

40+
static final String BASE_VARIANT_KEYWORD = "baseVariant";
4041
static final String FRAMEWORK_RE = "(.+)\\.framework";
4142
static final String JAR_RE = "(.+\\.jar)";
4243
static final String JS_RE = "(.+\\.js)";
@@ -64,12 +65,23 @@ class Extender {
6465
if (appManifests.size() > 1 ) {
6566
throw new ExtenderException("Only one app.manifest allowed!");
6667
}
68+
AppManifestConfiguration baseVariantManifest = null;
69+
6770
if (appManifests.isEmpty()) {
6871
this.appManifestPath = "";
6972
this.appManifest = new AppManifestConfiguration();
7073
} else {
7174
this.appManifestPath = ExtenderUtil.getRelativePath(this.uploadDirectory, appManifests.get(0));
72-
this.appManifest = Extender.loadYaml(this.jobDirectory, appManifests.get(0), AppManifestConfiguration.class);
75+
AppManifestConfiguration appManifest = Extender.loadYaml(this.jobDirectory, appManifests.get(0), AppManifestConfiguration.class);
76+
77+
// An completely empty manifest will yield a null pointer in result from Extender.loadYaml
78+
this.appManifest = (appManifest != null) ? appManifest : new AppManifestConfiguration();
79+
80+
// An manifest with no platform keyword will yield a null-pointer for this.appManifest.platforms
81+
// This happens if we get a manifest with just the context keyword given.
82+
if (this.appManifest.platforms == null) {
83+
this.appManifest.platforms = new HashMap<String, AppManifestPlatformConfig>();
84+
}
7385

7486
// To avoid null pointers later on
7587
if (this.appManifest.platforms.get(platform) == null) {
@@ -78,6 +90,18 @@ class Extender {
7890
if (this.appManifest.platforms.get(platform).context == null) {
7991
this.appManifest.platforms.get(platform).context = new HashMap<String, Object>();
8092
}
93+
94+
if (this.appManifest.context != null && this.appManifest.context.get(BASE_VARIANT_KEYWORD) instanceof String)
95+
{
96+
String baseVariant = (String)this.appManifest.context.get(BASE_VARIANT_KEYWORD);
97+
File baseVariantFile = new File(sdk.getPath() + "/extender/variants/" + baseVariant + ".appmanifest");
98+
if (!baseVariantFile.exists()) {
99+
throw new ExtenderException("Base variant " + baseVariant + " not found!");
100+
}
101+
LOGGER.info("Using base variant: " + baseVariant);
102+
103+
baseVariantManifest = Extender.loadYaml(this.jobDirectory, baseVariantFile, AppManifestConfiguration.class);
104+
}
81105
}
82106

83107
this.platform = platform;
@@ -102,7 +126,7 @@ class Extender {
102126
this.useWine = alternatePlatform.contains("wine32");
103127

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

108132
// LEGACY: Make sure the Emscripten compiler doesn't pollute the environment

‎server/src/main/java/com/defold/extender/ExtenderUtil.java‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,19 +127,28 @@ public boolean accept(File file) {
127127
/** Merges the different levels in the app manifest into one context
128128
* @param manifest The app manifest
129129
* @param platform The platform
130+
* @param optionalBaseVariantManifest The base manifest (optional)
130131
* @return The resource, or null if not found
131132
*/
132-
public static Map<String, Object> getAppManifestContext(AppManifestConfiguration manifest, String platform) throws ExtenderException {
133+
public static Map<String, Object> getAppManifestContext(AppManifestConfiguration manifest, String platform, AppManifestConfiguration optionalBaseVariantManifest) throws ExtenderException {
133134
Map<String, Object> appManifestContext = new HashMap<>();
134135

135-
if (manifest == null || manifest.platforms == null)
136-
return appManifestContext;
137-
138-
if (manifest.platforms.containsKey("common")) {
139-
appManifestContext = Extender.mergeContexts(appManifestContext, manifest.platforms.get("common").context);
136+
if (optionalBaseVariantManifest != null && optionalBaseVariantManifest.platforms != null) {
137+
if (optionalBaseVariantManifest.platforms.containsKey("common")) {
138+
appManifestContext = Extender.mergeContexts(appManifestContext, optionalBaseVariantManifest.platforms.get("common").context);
139+
}
140+
if (optionalBaseVariantManifest.platforms.containsKey(platform)) {
141+
appManifestContext = Extender.mergeContexts(appManifestContext, optionalBaseVariantManifest.platforms.get(platform).context);
142+
}
140143
}
141-
if (manifest.platforms.containsKey(platform)) {
142-
appManifestContext = Extender.mergeContexts(appManifestContext, manifest.platforms.get(platform).context);
144+
145+
if (manifest != null && manifest.platforms != null) {
146+
if (manifest.platforms.containsKey("common")) {
147+
appManifestContext = Extender.mergeContexts(appManifestContext, manifest.platforms.get("common").context);
148+
}
149+
if (manifest.platforms.containsKey(platform)) {
150+
appManifestContext = Extender.mergeContexts(appManifestContext, manifest.platforms.get(platform).context);
151+
}
143152
}
144153

145154
return appManifestContext;

‎server/src/test/java/com/defold/extender/ExtenderTest.java‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ public void testAppManifestContext() throws IOException, ExtenderException {
413413

414414
assertTrue(appManifest != null);
415415

416-
Map<String, Object> context = ExtenderUtil.getAppManifestContext(appManifest, "x86_64-osx");
416+
Map<String, Object> context = ExtenderUtil.getAppManifestContext(appManifest, "x86_64-osx", null);
417417

418418
List<String> expectedItems = new ArrayList<>();
419419
expectedItems.add("-fno-exceptions"); // common
@@ -428,7 +428,7 @@ public void testGetAppmanifest() throws IOException, ExtenderException {
428428

429429
AppManifestConfiguration appManifest = Extender.loadYaml(root, new File("test-data/extendertest.platformnull.appmanifest"), AppManifestConfiguration.class);
430430
// previous issue was that it threw a null pointer exception
431-
ExtenderUtil.getAppManifestContext(appManifest, "x86_64-osx");
431+
ExtenderUtil.getAppManifestContext(appManifest, "x86_64-osx", null);
432432
}
433433

434434
@Test
@@ -441,4 +441,30 @@ public void testGetManifestContext() throws IOException, ExtenderException {
441441
Map<String, Object> manifestContext = Extender.getManifestContext("x86_64-osx", config, manifestConfig);
442442
assertNotEquals(null, manifestContext);
443443
}
444+
445+
@Test
446+
public void testAppManifestContextWithVariant() throws IOException, ExtenderException {
447+
448+
File root = new File("test-data");
449+
File appManifestFile = new File("test-data/extendertest.appmanifest");
450+
File baseManifestFile = new File("test-data/headless.appmanifest");
451+
452+
AppManifestConfiguration appManifest = Extender.loadYaml(root, appManifestFile, AppManifestConfiguration.class);
453+
assertTrue(appManifest != null);
454+
455+
AppManifestConfiguration baseManifest = Extender.loadYaml(root, baseManifestFile, AppManifestConfiguration.class);
456+
assertTrue(baseManifest != null);
457+
458+
Map<String, Object> context = ExtenderUtil.getAppManifestContext(appManifest, "x86_64-osx", baseManifest);
459+
460+
List<String> expectedItems = new ArrayList<>();
461+
expectedItems.add("DefaultSoundDevice"); // base x86-osx
462+
expectedItems.add("AudioDecoderWav"); // base x86-osx
463+
expectedItems.add("AudioDecoderStbVorbis"); // base x86-osx
464+
expectedItems.add("AudioDecoderTremolo"); // base x86-osx
465+
expectedItems.add("SymbolA"); // common
466+
expectedItems.add("SymbolB"); // x86_64-osx
467+
468+
assertEquals( expectedItems, context.get("excludeSymbols") );
469+
}
444470
}

‎server/src/test/java/com/defold/extender/IntegrationTest.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,4 +450,20 @@ public void buildLinkWithoutDotLib() throws IOException, ExtenderClientException
450450

451451
doBuild(sourceFiles);
452452
}
453+
454+
@Test
455+
public void buildEngineAppManifestVariant() throws IOException, ExtenderClientException {
456+
// Testing that the variant parameter can be parse and processed properly.
457+
// This test requires that we have a debug.appmanifest present in the SDK and only
458+
// our test data SDK currently has that, so we can only test it on that version
459+
460+
if (!configuration.platform.equals("a")) {
461+
return;
462+
}
463+
464+
List<ExtenderResource> sourceFiles = Lists.newArrayList(
465+
new FileExtenderResource("test-data/testproject_appmanifest_variant/_app/app.manifest"));
466+
467+
doBuild(sourceFiles);
468+
}
453469
}

‎server/test-data/headless.appmanifest‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
platforms:
2+
x86_64-osx:
3+
context:
4+
excludeLibs: ["record","vpx","sound","tremolo","graphics","hid"]
5+
excludeSymbols: ["DefaultSoundDevice","AudioDecoderWav","AudioDecoderStbVorbis","AudioDecoderTremolo"]
6+
libs: ["record_null","sound_null","graphics_null","hid_null"]

‎server/test-data/sdk/a/defoldsdk/extender/variants/debug.appmanifest‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
platforms:
2+
x86-win32:
3+
context:
4+
use-clang: true
5+
6+
x86_64-win32:
7+
context:
8+
use-clang: true

‎server/test-data/testproject_appmanifest_variant/_app/app.manifest‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
context:
2+
baseVariant: debug
3+
platforms:
4+
x86_64-win32:
5+
context:
6+
use-clang: true
7+
8+
x86-win32:
9+
context:
10+
use-clang: true

0 commit comments

Comments
 (0)