Skip to content

Commit

Permalink
Merge branch 'master' into TIMOB-25678
Browse files Browse the repository at this point in the history
  • Loading branch information
@garymathews
garymathews committed Oct 22, 2018
2 parents fe43726 + 748cd1c commit f48267f
Show file tree
Hide file tree
Showing 24 changed files with 1,078 additions and 360 deletions.
17 changes: 5 additions & 12 deletions Jenkinsfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def isGreenKeeper = env.BRANCH_NAME.startsWith('greenkeeper/') || 'greenkeeper[b

// These values could be changed manually on PRs/branches, but be careful we don't merge the changes in. We want this to be the default behavior for now!
// target branch of windows SDK to use and test suite to test with
def targetBranch = isPR ? env.CHANGE_TARGET : (env.BRANCH_NAME ?: 'master')
def targetBranch = isGreenKeeper ? 'master' : (isPR ? env.CHANGE_TARGET : (env.BRANCH_NAME ?: 'master'))
def includeWindows = isMainlineBranch // Include Windows SDK if on a mainline branch, by default
// Note that the `includeWindows` flag also currently toggles whether we build for all OSes/platforms, or just iOS/Android for macOS
def runDanger = isPR // run Danger.JS if it's a PR by default. (should we also run on origin branches that aren't mainline?)
Expand Down Expand Up @@ -78,6 +78,8 @@ def unitTests(os, nodeVersion, npmVersion, testSuiteBranch) {
if ('ios'.equals(os)) {
// Gather the crash report(s)
def home = sh(returnStdout: true, script: 'printenv HOME').trim()
// wait 1 minute, sometimes it's delayed in writing out crash reports to disk...
sleep time: 1, unit: 'MINUTES'
def crashFiles = sh(returnStdout: true, script: "ls -1 ${home}/Library/Logs/DiagnosticReports/").trim().readLines()
for (int i = 0; i < crashFiles.size(); i++) {
def crashFile = crashFiles[i]
Expand Down Expand Up @@ -154,14 +156,7 @@ timestamps {

// Install dependencies
timeout(5) {
// FIXME Do we need to do anything special to make sure we get os-specific modules only on that OS's build/zip?
if (isGreenKeeper) {
sh 'npm install'
sh 'npm install -g sgtcoolguy/greenkeeper-lockfile#jenkins-multibranch'
sh 'greenkeeper-lockfile-update'
} else {
sh 'npm ci'
}
sh 'npm ci'
}
// Run npm test, but record output in a file and check for failure of command by checking output
if (fileExists('npm_test.log')) {
Expand All @@ -177,8 +172,6 @@ timestamps {
stash allowEmpty: true, name: 'test-report-ios'
stash allowEmpty: true, name: 'test-report-android'
error readFile('npm_test.log')
} else if (isGreenKeeper) {
sh 'greenkeeper-lockfile-upload' // FIXME: This will get pushed up even if unit tests on sims fails later...
}
}

Expand Down Expand Up @@ -460,7 +453,7 @@ timestamps {
withEnv(['ghprbGhRepository=appcelerator/titanium_mobile',"ghprbPullId=${env.CHANGE_ID}", "ZIPFILE=${basename}-osx.zip", "BUILD_STATUS=${currentBuild.currentResult}"]) {
// FIXME Can't pass along env variables properly, so we cheat and write them as a JSON file we can require
sh 'node -p \'JSON.stringify(process.env)\' > env.json'
sh returnStatus: true, script: 'npx danger' // Don't fail build if danger fails. We want to retain existing build status.
sh returnStatus: true, script: 'npx danger ci' // Don't fail build if danger fails. We want to retain existing build status.
} // withEnv
} // nodejs
deleteDir()
Expand Down
3 changes: 3 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ see the LICENSE file for specific details.
*[Download Pre-built Titanium](http://builds.appcelerator.com/#master)*

# Table of Contents

[![Greenkeeper badge](https://badges.greenkeeper.io/appcelerator/titanium_mobile.svg)](https://greenkeeper.io/)

1. [Features](#features)
2. [Hyperloop](#hyperloop)
3. [Alloy](#alloy)
Expand Down
164 changes: 163 additions & 1 deletion ...roll-apt/src/java/org/appcelerator/kroll/annotations/generator/KrollBindingGenerator.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* Appcelerator Titanium Mobile
* Copyright (c) 2010-2013 by Appcelerator, Inc. All Rights Reserved.
* Copyright (c) 2010-Present by Appcelerator, Inc. All Rights Reserved.
* Licensed under the terms of the Apache Public License
* Please see the LICENSE included with this distribution for details.
*/
Expand All @@ -16,7 +16,9 @@
import java.net.URISyntaxException;
import java.net.URL;
import java.security.CodeSource;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.jar.JarFile;
Expand Down Expand Up @@ -282,10 +284,170 @@ protected void generateBindings() throws ParseException, IOException
String v8ProxySource = proxyName + ".cpp";

saveTypeTemplate(v8HeaderTemplate, v8ProxyHeader, root);
validateProxyShape(root);
saveTypeTemplate(v8SourceTemplate, v8ProxySource, root);
}
}

private void validateProxyShape(Map<Object, Object> root)
{
Map<String, Object> proxyAttrs = (Map<String, Object>) root.get("proxyAttrs");
List<String> propertyAccessors = (List<String>) proxyAttrs.get("propertyAccessors");
if (propertyAccessors == null) {
propertyAccessors = Collections.EMPTY_LIST;
}
Map<String, Object> methods = (Map<String, Object>) root.get("methods");
if (methods == null) {
methods = Collections.EMPTY_MAP;
}
Map<String, Object> dynamicProperties = (Map<String, Object>) root.get("dynamicProperties");
if (dynamicProperties == null) {
dynamicProperties = Collections.EMPTY_MAP;
}
if (methods.size() > 0 || dynamicProperties.size() > 0) {
String proxyClassName = (String) root.get("proxyClassName");
for (String propertyName : propertyAccessors) {

// if a property is defined as both dynamic *and* accessor, then developer needs to choose.
// If there's both a set and getProperty, just remove the property accessor
// if there's no setProperty impl, then need to add one and remove from propertyAccessors
if (dynamicProperties.containsKey(propertyName)) {
System.err.println(
"Clashing property definition in proxy.propertyAccessors and a @Kroll.set/getProperty annotations for property '"
+ proxyClassName + "." + propertyName + "'.");
Map<String, Object> dynamicProperty = (Map<String, Object>) dynamicProperties.get(propertyName);
if ((Boolean) dynamicProperty.get("set")) { // there's a setter
// is getter defined?
if ((Boolean) dynamicProperty.get("get")) {
System.err.println(
"Likely fix is to remove from proxy.propertyAccessors listing, as both getter and setter methods are defined.");
} else {
System.err.println(
"Likely fix is to remove from proxy.propertyAccessors listing, as a setter method is already defined. A getter IS NOT defined, so you may want to add a @Kroll.getProperty implementation as well (or rely n the default getter generated, which uses #onPropertyChanged() to react).");
}
System.exit(1);
} else {
// NO SETTER! (must have getter)
// This may be a valid usage pattern: override getProperty, wants the "default" set implementation
System.err.println(
"This will use the 'default' implementation for a setter and treat the property as readwrite, with a non-default getter.");
System.err.println(
"This is not an error, but you may want to consider adding a @Kroll.setProperty implementation and then removing from proxy.propertyAccessors listing.");
System.err.println();
}
// Don't check for clashing methods, since we handle that for dynamic properties in next loop (and we have a dynamic property with same name)
continue;
}

// If there's a method matching auto-generated property accessor, then dev needs to either:
// - rename the method
// - remove property accessor and ensure the method is marked with @Kroll.get/setProperty annotation (basically go from "JS" property to "dynamic" property)
String upperProp = Character.toUpperCase(propertyName.charAt(0)) + propertyName.substring(1);
boolean hasClashingGetter = methods.containsKey("get" + upperProp);
boolean hasClashingSetter = methods.containsKey("set" + upperProp);
if (hasClashingGetter && hasClashingSetter) {
System.err.println(
"Clashing method definitions in proxy.propertyAccessors and @Kroll.method annotations for property accessors on '"
+ proxyClassName + "." + propertyName + " - get" + upperProp + "() and set" + upperProp
+ "()'.");
System.err.println(
"Likely fix is to remove from proxy.propertyAccessors listing, remove @Kroll.method annotation and add @Kroll.getProperty/@Kroll.setProperty annotations to the methods.");
System.err.println("Alternately, please rename the methods to avoid the clash.");
System.exit(1);
}
if (hasClashingGetter) {
// There's a getter due to propertyAccessor entry, but also a method with same name
System.err.println(
"Clashing method definition in proxy.propertyAccessors and a @Kroll.method annotation for property accessor '"
+ proxyClassName + "#get" + upperProp + "()'.");
System.err.println(
"Likely fix is to remove @Kroll.method annotation and add @Kroll.getProperty annotation to method.");
System.err.println("Alternately, please rename the method to avoid the clash.");
System.exit(1);
}
if (hasClashingSetter) {
// There's a setter due to propertyAccessor entry, but also a method with same name
System.err.println(
"Clashing method definition in proxy.propertyAccessors and a @Kroll.method annotation for property accessor '"
+ proxyClassName + "#set" + upperProp + "()'.");
System.err.println(
"Likely fix is to remove @Kroll.method annotation and add @Kroll.setProperty annotation to method.");
System.err.println("Alternately, please rename the method to avoid the clash.");
System.exit(1);
}
}

for (String propertyName : dynamicProperties.keySet()) {
Map<String, Object> dynamicProperty = (Map<String, Object>) dynamicProperties.get(propertyName);
String getterName = (String) dynamicProperty.get("getMethodName");
boolean hasGetter = true;
if (getterName == null) {
hasGetter = false;
getterName = "get" + Character.toUpperCase(propertyName.charAt(0)) + propertyName.substring(1);
}

// Turns out implying @Kroll.method from @Kroll.getProperty/setProperty is a pain in the ass (as they have properties that can change their names)!
// so let's warn when they're not paired up

// method has @Kroll.getProperty but no @Kroll.method. This is ok in some cases, but generally we want getter accessors until they get removed in SDK 9
if (hasGetter && !methods.containsKey(getterName)) {
// There are rare cases where we don't want this, like Ti.Android.R (I assume we don't want Ti.Android#getR())
System.err.println(
"Property has getter defined with @Kroll.getProperty on " + proxyClassName + "#" + getterName
+ "(), but has no @Kroll.method annotation. Consider adding one to expose the getter accessor to JS.");
System.err.println();
}

// there's no getProperty defined, but there's a method with the target name
if (!hasGetter && methods.containsKey(getterName)) {
System.err.println("There is no getter assigned to property " + propertyName
+ ", but there is a @Kroll.method annotation on the default getter method name: "
+ proxyClassName + "#" + getterName + "()");
System.err.println("Likely fix is to add the @Kroll.getProperty to this method so that obj."
+ propertyName + " returns the same value as obj." + getterName + "();");
System.err.println("Alternately, please rename the method to avoid the clash.");
System.err.println();
System.exit(1);
}

String setterName = (String) dynamicProperty.get("setMethodName");
boolean hasSetter = true;
if (setterName == null) {
hasSetter = false;
setterName = "set" + Character.toUpperCase(propertyName.charAt(0)) + propertyName.substring(1);
}

if (hasSetter && !methods.containsKey(setterName)) {
// method has @Kroll.setProperty but no @Kroll.method. This is ok in some cases, but generally we want setter accessors unitl they get removed in SDK 9
System.err.println(
"Property has setter defined with @Kroll.getProperty on " + proxyClassName + "#" + setterName
+ "(), but has no @Kroll.method annotation. Consider adding one to expose the setter accessor to JS.");
System.err.println();
System.exit(1);
}

// there's no setProperty defined, but there's a method with the target name
if (!hasSetter && methods.containsKey(setterName)) {
// ensure target method has typical single argument, if not then we can't expose as property setter
Map<String, Object> method = (Map<String, Object>) methods.get(setterName);
List<Map> args = (List<Map>) method.get("args");
if (args.size() == 1) {
System.err.println(
"There is no setter assigned to property " + propertyName
+ ", but there is a @Kroll.method annotation on the default setter method name: "
+ proxyClassName + "#" + setterName + "()");
System.err.println("Likely fix is to add the @Kroll.setProperty to this method so that obj."
+ propertyName + " = value executes the same code as obj." + setterName
+ "(value);");
System.err.println("Alternately, please rename the method to avoid the clash.");
System.err.println();
System.exit(1);
}
}
}
}
}

public static void main(String[] args) throws Exception
{
if (args.length < 4) {
Expand Down
10 changes: 8 additions & 2 deletions ...oid/kroll-apt/src/java/org/appcelerator/kroll/annotations/generator/ProxyBindingV8.cpp.fm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,22 +208,28 @@ Local<FunctionTemplate> ${className}::getProxyTemplate(Isolate* isolate)
titanium::Proxy::onPropertyChanged,
</#if>
Local<Value>(), DEFAULT,
<#if property.set>
<#if property.set || proxyAttrs.propertyAccessors?seq_contains(name)>
static_cast<v8::PropertyAttribute>(v8::DontDelete)
<#else>
static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontDelete)
</#if>
);
);
</@Proxy.listDynamicProperties>

// Accessors --------------------------------------------------------------
<@Proxy.listPropertyAccessors ; isFirst, name, getter, setter>
<#if !dynamicProperties?? || !dynamicProperties?keys?seq_contains(name)>
instanceTemplate->SetAccessor(
NEW_SYMBOL(isolate, "${name}"),
titanium::Proxy::getProperty,
titanium::Proxy::onPropertyChanged);
</#if>
<#if !methods?? || !methods?keys?seq_contains(getter)>
DEFINE_PROTOTYPE_METHOD_DATA(isolate, t, "${getter}", titanium::Proxy::getProperty, NEW_SYMBOL(isolate, "${name}"));
</#if>
<#if !methods?? || !methods?keys?seq_contains(setter)>
DEFINE_PROTOTYPE_METHOD_DATA(isolate, t, "${setter}", titanium::Proxy::onPropertyChanged, NEW_SYMBOL(isolate, "${name}"));
</#if>
</@Proxy.listPropertyAccessors>

<#if interceptor??>
Expand Down

0 comments on commit f48267f

Please sign in to comment.