-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle non boolean values for requires arc option #263
Handle non boolean values for requires arc option #263
Conversation
Don't proceed to resolve dependencies if the project contains no Podfiles
@@ -10,8 +10,8 @@ build_artifact() { | |||
echo "[build] Creating standalone extender server artifact at ${ARTIFACT_DIR}..." | |||
rm -rf ${ARTIFACT_DIR} | |||
mkdir -p ${ARTIFACT_DIR} | |||
cp ${SERVER_DIR}/build/docker/extender-0.1.0.jar ${ARTIFACT_DIR}/extender.jar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use the path for standalone builds. The jar files are copied in build.gradle in task buildStandalone()
@@ -703,6 +703,9 @@ private void buildPods() throws IOException, InterruptedException, ExtenderExcep | |||
// We use the same mechanism as when building the extension and create a | |||
// manifest context for each pod | |||
Map<String, Object> manifestContext = new HashMap<>(); | |||
manifestContext = ExtenderUtil.mergeContexts(manifestContext, this.platformConfig.context); | |||
manifestContext = ExtenderUtil.mergeContexts(manifestContext, this.platformVariantConfig.context); | |||
manifestContext = ExtenderUtil.mergeContexts(manifestContext, this.platformAppConfig.context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want the platformAppConfig as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest we use the already merged mergedAppContext
from loadManifests().
You can make a copy of that and then override your variables below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Done!
@@ -578,14 +588,20 @@ public ResolvedPods resolveDependencies(File jobDirectory, String platform) thro | |||
throw new ExtenderException("Unsupported platform " + platform); | |||
} | |||
|
|||
List<File> podFiles = ExtenderUtil.listFilesMatchingRecursive(jobDirectory, "Podfile"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exit the service early if the job doesn't contain any Podfiles
@@ -703,6 +703,9 @@ private void buildPods() throws IOException, InterruptedException, ExtenderExcep | |||
// We use the same mechanism as when building the extension and create a | |||
// manifest context for each pod | |||
Map<String, Object> manifestContext = new HashMap<>(); | |||
manifestContext = ExtenderUtil.mergeContexts(manifestContext, this.platformConfig.context); | |||
manifestContext = ExtenderUtil.mergeContexts(manifestContext, this.platformVariantConfig.context); | |||
manifestContext = ExtenderUtil.mergeContexts(manifestContext, this.platformAppConfig.context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest we use the already merged mergedAppContext
from loadManifests().
You can make a copy of that and then override your variables below.
File armFrameworkDir = new File(frameworkDir, "ios-arm64_armv7"); | ||
if (armFrameworkDir.exists()) { | ||
Path from = new File(armFrameworkDir, frameworkName + ".framework").toPath(); | ||
File arm64_armv7FrameworkDir = new File(frameworkDir, "ios-arm64_armv7"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to look for multiple variations of folder names inside the frameworks
@@ -618,7 +617,7 @@ public ResolvedPods resolveDependencies(File jobDirectory, String platform) thro | |||
List<PodSpec> pods = installPods(workingDir); | |||
copyPodFrameworks(pods, frameworksDir); | |||
|
|||
dumpDir(jobDirectory, 0); | |||
// dumpDir(jobDirectory, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a DM_KEEP_JOB_FOLDER for debugging.
I suggest we piggyback on that?
This PR also includes a fix for missing
env
in the context (this issue popped up after the big refactor @JCash did).Fixes #262