Revision 98b56e78
Added by Alexis Mousset about 6 years ago
rudder-agent/SOURCES/patches/cfengine/10584-avoid-copy-variable-table.patch | ||
---|---|---|
From 55989299c0969500358169a218203f10c584aa06 Mon Sep 17 00:00:00 2001
|
||
From: Alexis Mousset <contact@amousset.me>
|
||
Date: Fri, 31 Mar 2017 14:06:57 +0200
|
||
Subject: [PATCH] CFE-2524: Avoid copying variable table when expanding a
|
||
promise
|
||
|
||
The idea is to stop copying variable tables, which appears to be
|
||
one of the most heavy operation the agent does, but to always
|
||
lookup the variables in the global table.
|
||
|
||
The main issue with this change is to manage ambiguities
|
||
in the use of the "this" scope. It is used both for special
|
||
variables at promise level and as an alias for variables in the
|
||
current bundle.
|
||
|
||
The general idea is to modify VariableResolve to try to resolve
|
||
unknown "this"-scoped variables as variables in the current bundle.
|
||
|
||
The is a second change to force "this"-scopes variables
|
||
passed as parameters to a bundle to be qualified explicitly in
|
||
the calling bundle.
|
||
|
||
A non-compatible change could be to stop supporting "this" scope
|
||
for non-special bundle, which would allow a clean separation
|
||
and avoid the workarounds.
|
||
|
||
Changelog: Title
|
||
---
|
||
libpromises/eval_context.c | 65 ++++++++---------
|
||
libpromises/expand.c | 3 +-
|
||
libpromises/var_expressions.c | 33 +++++++++
|
||
libpromises/var_expressions.h | 2 +
|
||
libpromises/variable.c | 20 ------
|
||
.../acceptance/01_vars/01_basic/this_variables.cf | 83 ++++++++++++++++++++++
|
||
.../02_functions/nth_datacontainer.cf.expected | 14 ++--
|
||
7 files changed, 157 insertions(+), 63 deletions(-)
|
||
create mode 100644 tests/acceptance/01_vars/01_basic/this_variables.cf
|
||
|
||
diff --git a/libpromises/eval_context.c b/libpromises/eval_context.c
|
||
index f967026..6c305b1 100644
|
||
--- a/libpromises/eval_context.c
|
||
+++ b/libpromises/eval_context.c
|
||
@@ -1334,16 +1334,8 @@ void EvalContextStackPushPromiseFrame(EvalContext *ctx, const Promise *owner, bo
|
||
|
||
EvalContextStackPushFrame(ctx, frame);
|
||
|
||
- if (copy_bundle_context)
|
||
- {
|
||
- frame->data.promise.vars = VariableTableCopyLocalized(ctx->global_variables,
|
||
- EvalContextStackCurrentBundle(ctx)->ns,
|
||
- EvalContextStackCurrentBundle(ctx)->name);
|
||
- }
|
||
- else
|
||
- {
|
||
- frame->data.promise.vars = VariableTableNew();
|
||
- }
|
||
+ // Ignore copy_bundle_context and create an empty table
|
||
+ frame->data.promise.vars = VariableTableNew();
|
||
|
||
if (PromiseGetBundle(owner)->source_path)
|
||
{
|
||
@@ -1915,6 +1907,8 @@ static VariableTable *GetVariableTableForScope(const EvalContext *ctx,
|
||
return frame ? frame->data.body.vars : NULL;
|
||
}
|
||
|
||
+ // "this" variables can be in local or global variable table (when this is used for non-special
|
||
+ // varables), so return local as VariableResolve will try global table anyway.
|
||
case SPECIAL_SCOPE_THIS:
|
||
{
|
||
StackFrame *frame = LastStackFrameByType(ctx, STACK_FRAME_TYPE_PROMISE);
|
||
@@ -2005,6 +1999,7 @@ static void VarRefStackQualify(const EvalContext *ctx, VarRef *ref)
|
||
|
||
case STACK_FRAME_TYPE_PROMISE:
|
||
case STACK_FRAME_TYPE_PROMISE_ITERATION:
|
||
+ // This is necessary to allow special "this" variable to work when used without "this".
|
||
VarRefQualify(ref, NULL, SpecialScopeToString(SPECIAL_SCOPE_THIS));
|
||
break;
|
||
}
|
||
@@ -2054,9 +2049,6 @@ bool EvalContextVariablePut(EvalContext *ctx,
|
||
* means that an unqualified reference will be looked up in the context of the top
|
||
* stack frame. Note that when evaluating a promise, this will qualify a reference
|
||
* to the 'this' scope.
|
||
- *
|
||
- * Ideally, this function should resolve a variable by walking down the stack, but
|
||
- * this is pending rework in variable iteration.
|
||
*/
|
||
static Variable *VariableResolve(const EvalContext *ctx, const VarRef *ref)
|
||
{
|
||
@@ -2065,16 +2057,29 @@ static Variable *VariableResolve(const EvalContext *ctx, const VarRef *ref)
|
||
if (!VarRefIsQualified(ref))
|
||
{
|
||
VarRef *qref = VarRefCopy(ref);
|
||
+ // Warning: This will "qualify" non-scoped variables in a promise to this.variable
|
||
VarRefStackQualify(ctx, qref);
|
||
Variable *ret = VariableResolve(ctx, qref);
|
||
VarRefDestroy(qref);
|
||
return ret;
|
||
}
|
||
|
||
+ /*
|
||
+ * We will make a first lookup that works in almost all cases: will look for local
|
||
+ * or global variables, depending of the current scope.
|
||
+ *
|
||
+ * The only case it does not work is when we have a "this.custom_variable" variable
|
||
+ * because we will search is local promise scope the variable is in the global
|
||
+ * variable table. In this case, we will look for the variable in the current bundle.
|
||
+ */
|
||
+
|
||
+ // Get the variable table associated to the scope
|
||
VariableTable *table = GetVariableTableForScope(ctx, ref->ns, ref->scope);
|
||
+
|
||
+ Variable *var;
|
||
if (table)
|
||
{
|
||
- Variable *var = VariableTableGet(table, ref);
|
||
+ var = VariableTableGet(table, ref);
|
||
if (var)
|
||
{
|
||
return var;
|
||
@@ -2092,6 +2097,17 @@ static Variable *VariableResolve(const EvalContext *ctx, const VarRef *ref)
|
||
}
|
||
}
|
||
|
||
+ // Try to qualify "this." variable to "current_bundle."
|
||
+ const Bundle *last_bundle = EvalContextStackCurrentBundle(ctx);
|
||
+ if (last_bundle && strcmp(ref->scope, SpecialScopeToString(SPECIAL_SCOPE_THIS)) == 0)
|
||
+ {
|
||
+ VarRef *qref = VarRefCopy(ref);
|
||
+ VarRefQualify(qref, last_bundle->ns, last_bundle->name);
|
||
+ var = VariableResolve(ctx, qref);
|
||
+ VarRefDestroy(qref);
|
||
+ return var;
|
||
+ }
|
||
+
|
||
return NULL;
|
||
}
|
||
|
||
@@ -2130,27 +2146,6 @@ const void *EvalContextVariableGet(const EvalContext *ctx, const VarRef *ref, Da
|
||
return var->rval.item;
|
||
}
|
||
}
|
||
- else if (!VarRefIsQualified(ref))
|
||
- {
|
||
- /*
|
||
- * FALLBACK: Because VariableResolve currently does not walk the stack
|
||
- * (rather, it looks at only the top frame), we do an explicit retry
|
||
- * here to qualify an unqualified reference to the current bundle.
|
||
- *
|
||
- * This is overly complicated, and will go away as soon as
|
||
- * VariableResolve can walk the stack (which is pending rework in
|
||
- * variable iteration).
|
||
- */
|
||
- const Bundle *bp = EvalContextStackCurrentBundle(ctx);
|
||
- if (bp)
|
||
- {
|
||
- VarRef *qref = VarRefCopy(ref);
|
||
- VarRefQualify(qref, bp->ns, bp->name);
|
||
- const void *ret = EvalContextVariableGet(ctx, qref, type_out);
|
||
- VarRefDestroy(qref);
|
||
- return ret;
|
||
- }
|
||
- }
|
||
|
||
if (type_out)
|
||
{
|
||
diff --git a/libpromises/expand.c b/libpromises/expand.c
|
||
index dc90fac..fa8a3a4 100644
|
||
--- a/libpromises/expand.c
|
||
+++ b/libpromises/expand.c
|
||
@@ -256,7 +256,8 @@ PromiseResult ExpandPromise(EvalContext *ctx, const Promise *pp,
|
||
* (including body inheritance). */
|
||
Promise *pcopy = DeRefCopyPromise(ctx, pp);
|
||
|
||
- EvalContextStackPushPromiseFrame(ctx, pcopy, true);
|
||
+ // TODO: Remove last parameter?
|
||
+ EvalContextStackPushPromiseFrame(ctx, pcopy, false);
|
||
PromiseIterator *iterctx = PromiseIteratorNew(pcopy);
|
||
|
||
/* 2. Parse all strings (promiser-promisee-constraints), find all
|
||
diff --git a/libpromises/var_expressions.c b/libpromises/var_expressions.c
|
||
index afede51..258d58c 100644
|
||
--- a/libpromises/var_expressions.c
|
||
+++ b/libpromises/var_expressions.c
|
||
@@ -320,6 +320,16 @@ VarRef *VarRefParseFromNamespaceAndScope(const char *qualified_name,
|
||
{
|
||
_ns = NULL;
|
||
}
|
||
+
|
||
+ /*
|
||
+ * Force considering non-special "this." variables as unqualified.
|
||
+ * This allows qualifying bundle parameters passed as reference with a "this" scope
|
||
+ * in the calling bundle.
|
||
+ */
|
||
+ if (is_this_not_special(scope, lval)) {
|
||
+ free(scope);
|
||
+ scope = NULL;
|
||
+ }
|
||
}
|
||
|
||
VarRef *ref = xmalloc(sizeof(VarRef));
|
||
@@ -333,6 +343,29 @@ VarRef *VarRefParseFromNamespaceAndScope(const char *qualified_name,
|
||
return ref;
|
||
}
|
||
|
||
+/*
|
||
+ * This function will return true if the given variable is
|
||
+ * a this.something variable that is an alias to a non-special local variable.
|
||
+ */
|
||
+bool is_this_not_special(const char *scope, const char *lval) {
|
||
+ // TODO: better way to get this list?
|
||
+ const char *special_this_variables[] = {"v","k","this","service_policy","promiser","promiser_uid","promiser_gid","promiser_pid","promiser_ppid","bundle","handle","namespace","promise_filename","promise_dirname","promise_linenumber", NULL};
|
||
+
|
||
+ if (!scope) {
|
||
+ return false;
|
||
+ }
|
||
+
|
||
+ if (SpecialScopeFromString(scope) != SPECIAL_SCOPE_THIS) {
|
||
+ return false;
|
||
+ }
|
||
+
|
||
+ if (IsStrIn(lval, special_this_variables)) {
|
||
+ return false;
|
||
+ }
|
||
+
|
||
+ return true;
|
||
+}
|
||
+
|
||
VarRef *VarRefParse(const char *var_ref_string)
|
||
{
|
||
return VarRefParseFromNamespaceAndScope(var_ref_string, NULL, NULL, CF_NS, '.');
|
||
diff --git a/libpromises/var_expressions.h b/libpromises/var_expressions.h
|
||
index 3639a8f..28f77bb 100644
|
||
--- a/libpromises/var_expressions.h
|
||
+++ b/libpromises/var_expressions.h
|
||
@@ -57,6 +57,8 @@ VarRef *VarRefCopy(const VarRef *ref);
|
||
VarRef *VarRefCopyLocalized(const VarRef *ref);
|
||
VarRef *VarRefCopyIndexless(const VarRef *ref);
|
||
|
||
+bool is_this_not_special(const char *scope, const char *lval);
|
||
+
|
||
VarRef *VarRefParse(const char *var_ref_string);
|
||
|
||
VarRef *VarRefParseFromBundle(const char *var_ref_string, const Bundle *bundle);
|
||
diff --git a/libpromises/variable.c b/libpromises/variable.c
|
||
index bde3994..ebc6e8a 100644
|
||
--- a/libpromises/variable.c
|
||
+++ b/libpromises/variable.c
|
||
@@ -341,23 +341,3 @@ void VariableTableIteratorDestroy(VariableTableIterator *iter)
|
||
free(iter);
|
||
}
|
||
}
|
||
-
|
||
-VariableTable *VariableTableCopyLocalized(const VariableTable *table, const char *ns, const char *scope)
|
||
-{
|
||
- VariableTable *localized_copy = VariableTableNew();
|
||
-
|
||
- VariableTableIterator *iter = VariableTableIteratorNew(table, ns, scope, NULL);
|
||
- Variable *foreign_var = NULL;
|
||
- while ((foreign_var = VariableTableIteratorNext(iter)))
|
||
- {
|
||
- /* TODO why is tags NULL here? Shouldn't it be an exact copy of
|
||
- * foreign_var->tags? */
|
||
- Variable *localized_var = VariableNew(VarRefCopyLocalized(foreign_var->ref),
|
||
- RvalCopy(foreign_var->rval), foreign_var->type,
|
||
- NULL, foreign_var->promise);
|
||
- VarMapInsert(localized_copy->vars, localized_var->ref, localized_var);
|
||
- }
|
||
- VariableTableIteratorDestroy(iter);
|
||
-
|
||
- return localized_copy;
|
||
-}
|
||
diff --git a/tests/acceptance/01_vars/01_basic/this_variables.cf b/tests/acceptance/01_vars/01_basic/this_variables.cf
|
||
new file mode 100644
|
||
index 0000000..638d840
|
||
--- /dev/null
|
||
+++ b/tests/acceptance/01_vars/01_basic/this_variables.cf
|
||
@@ -0,0 +1,83 @@
|
||
+body common control
|
||
+{
|
||
+ inputs => { "../../default.cf.sub" };
|
||
+ bundlesequence => { default("$(this.promise_filename)") };
|
||
+ version => "1.0";
|
||
+}
|
||
+
|
||
+bundle agent test
|
||
+{
|
||
+ vars:
|
||
+ "var" slist => { "var in test" };
|
||
+ "var_test" slist => { "var_test in test" };
|
||
+
|
||
+ methods:
|
||
+ "any" usebundle => inter;
|
||
+}
|
||
+
|
||
+bundle agent inter
|
||
+{
|
||
+ vars:
|
||
+ "var" slist => { "var in inter" };
|
||
+ "k" slist => { "k in inter" };
|
||
+
|
||
+ methods:
|
||
+ "any1" usebundle => final("test1", @(this.var_test));
|
||
+ "any2" usebundle => final("test2", @(var_test));
|
||
+ "any3" usebundle => final("test3", @(this.var));
|
||
+ "any4" usebundle => final("test4", @(var));
|
||
+ "any5" usebundle => final("test5", @(this.k));
|
||
+ "any6" usebundle => final("test6", @(k));
|
||
+ "any7" usebundle => final("test7", @(this.bundle));
|
||
+ "any8" usebundle => final("test8", @(bundle));
|
||
+}
|
||
+
|
||
+bundle agent final(name, param)
|
||
+{
|
||
+ vars:
|
||
+ "${name}" string => "${param}";
|
||
+}
|
||
+
|
||
+bundle agent check
|
||
+{
|
||
+
|
||
+ classes:
|
||
+
|
||
+ "ok1" not => strcmp("var_test in test", "${final.test1}");
|
||
+ "ok2" not => strcmp("var_test in test", "${final.test2}");
|
||
+ "ok3" expression => strcmp("var in inter", "${final.test3}");
|
||
+ "ok4" expression => strcmp("var in inter", "${final.test4}");
|
||
+ "ok5" not => strcmp("k in inter", "${final.test5}");
|
||
+ "ok6" expression => strcmp("k in inter", "${final.test6}");
|
||
+ "ok7" not => strcmp("inter", "${final.test7}");
|
||
+ "ok8" not => strcmp("inter", "${final.test8}");
|
||
+
|
||
+ "ok" expression => "ok1.ok2.ok3.ok4.ok5.ok6.ok7.ok8";
|
||
+
|
||
+ reports:
|
||
+
|
||
+ ok::
|
||
+ "$(this.promise_filename) Pass";
|
||
+
|
||
+ !ok::
|
||
+ "FAIL";
|
||
+
|
||
+ !ok1::
|
||
+ "variable '${final.test1}' = 'var_test in test'";
|
||
+ !ok2::
|
||
+ "variable '${final.test2}' = 'var_test in test'";
|
||
+ !ok3::
|
||
+ "variable '${final.test3}' != 'var in inter'";
|
||
+ !ok4::
|
||
+ "variable '${final.test4}' != 'var in inter'";
|
||
+ !ok5::
|
||
+ "variable '${final.test5}' = 'k in inter'";
|
||
+ !ok6::
|
||
+ "variable '${final.test6}' != 'k in inter'";
|
||
+ !ok7::
|
||
+ "variable '${final.test7}' = 'inter'";
|
||
+ !ok8::
|
||
+ "variable '${final.test8}' = 'inter'";
|
||
+}
|
||
+
|
||
+
|
||
diff --git a/tests/acceptance/01_vars/02_functions/nth_datacontainer.cf.expected b/tests/acceptance/01_vars/02_functions/nth_datacontainer.cf.expected
|
||
index fc702b7..9375b9d 100644
|
||
--- a/tests/acceptance/01_vars/02_functions/nth_datacontainer.cf.expected
|
||
+++ b/tests/acceptance/01_vars/02_functions/nth_datacontainer.cf.expected
|
||
@@ -1,20 +1,20 @@
|
||
jsonstring = {"boolean":true,"boolean_2":false,"integer":20130111,"integer_2":987654321,"list":["chris","dituri","was","here"],"null":null,"object":{"a":true,"b":[1,2,3],"c":"cat","d":108},"string":"Figaro. Figaro. Figaro, Figaro, Figaro... Figaro!","string_2":"Othello? Where art thou now?"}
|
||
keys:json = boolean
|
||
-keys:json = string
|
||
+keys:json = boolean_2
|
||
keys:json = integer
|
||
+keys:json = integer_2
|
||
keys:json = list
|
||
+keys:json = null
|
||
keys:json = object
|
||
-keys:json = integer_2
|
||
+keys:json = string
|
||
keys:json = string_2
|
||
-keys:json = boolean_2
|
||
-keys:json = null
|
||
primitive:json[boolean] = true
|
||
-primitive:json[string] = Figaro. Figaro. Figaro, Figaro, Figaro... Figaro!
|
||
+primitive:json[boolean_2] = false
|
||
primitive:json[integer] = 20130111
|
||
primitive:json[integer_2] = 987654321
|
||
-primitive:json[string_2] = Othello? Where art thou now?
|
||
-primitive:json[boolean_2] = false
|
||
primitive:json[null] = null
|
||
+primitive:json[string] = Figaro. Figaro. Figaro, Figaro, Figaro... Figaro!
|
||
+primitive:json[string_2] = Othello? Where art thou now?
|
||
list:json[0] = chris
|
||
list:json[1] = dituri
|
||
list:json[2] = was
|
rudder-agent/SOURCES/patches/cfengine/12130-avoid-copy-variable-table.patch | ||
---|---|---|
From 248b9f49025cd10546fe5f3bc78f7af83c29df7f Mon Sep 17 00:00:00 2001
|
||
From: Dimitrios Apostolou <jimis@northern.tech>
|
||
Date: Thu, 15 Feb 2018 15:24:38 +0100
|
||
Subject: [PATCH 1/5] whitespace and style
|
||
|
||
---
|
||
cf-agent/files_editline.c | 13 +++++++------
|
||
libpromises/eval_context.c | 16 ++++++++++------
|
||
2 files changed, 17 insertions(+), 12 deletions(-)
|
||
|
||
diff --git a/cf-agent/files_editline.c b/cf-agent/files_editline.c
|
||
index 0edb87e72e..01a0c30eb1 100644
|
||
--- a/cf-agent/files_editline.c
|
||
+++ b/cf-agent/files_editline.c
|
||
@@ -679,12 +679,12 @@ static PromiseResult VerifyLineInsertions(EvalContext *ctx, const Promise *pp, E
|
||
else if (!SelectRegion(ctx, *start, &begin_ptr, &end_ptr, a, edcontext))
|
||
{
|
||
cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, a,
|
||
- "The promised line insertion '%s' could not select an edit region in '%s'", pp->promiser,
|
||
- edcontext->filename);
|
||
+ "The promised line insertion '%s' could not select an edit region in '%s'",
|
||
+ pp->promiser, edcontext->filename);
|
||
result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED);
|
||
return result;
|
||
}
|
||
-
|
||
+
|
||
if (!end_ptr && a.region.select_end && !a.region.select_end_match_eof)
|
||
{
|
||
cfPS(ctx, LOG_LEVEL_VERBOSE, PROMISE_RESULT_INTERRUPTED, pp, a,
|
||
@@ -744,15 +744,16 @@ static PromiseResult VerifyLineInsertions(EvalContext *ctx, const Promise *pp, E
|
||
/* Level */
|
||
/***************************************************************************/
|
||
|
||
-static int SelectRegion(EvalContext *ctx, Item *start, Item **begin_ptr, Item **end_ptr, Attributes a,
|
||
- EditContext *edcontext)
|
||
+static int SelectRegion(EvalContext *ctx, Item *start,
|
||
+ Item **begin_ptr, Item **end_ptr,
|
||
+ Attributes a, EditContext *edcontext)
|
||
/*
|
||
|
||
This should provide pointers to the first and last line of text that include the
|
||
delimiters, since we need to include those in case they are being deleted, etc.
|
||
It returns true if a match was identified, else false.
|
||
|
||
-If no such region matches, begin_ptr and end_ptr should point to NULL
|
||
+If no such region matches, begin_ptr and end_ptr should point to NULL
|
||
|
||
*/
|
||
{
|
||
diff --git a/libpromises/eval_context.c b/libpromises/eval_context.c
|
||
index 409640921f..cef034122f 100644
|
||
--- a/libpromises/eval_context.c
|
||
+++ b/libpromises/eval_context.c
|
||
@@ -130,7 +130,7 @@ struct EvalContext_
|
||
|
||
/* new package promise evaluation context */
|
||
PackagePromiseContext *package_promise_context;
|
||
-
|
||
+
|
||
/* select_end_match_eof context*/
|
||
bool select_end_match_eof;
|
||
|
||
@@ -180,7 +180,7 @@ void AddPackageModuleToContext(const EvalContext *ctx, PackageModuleBody *pm)
|
||
/* First check if the body is there added from previous pre-evaluation
|
||
* iteration. If it is there update it as we can have new expanded variables. */
|
||
ssize_t pm_seq_index;
|
||
- if ((pm_seq_index = SeqIndexOf(ctx->package_promise_context->package_modules_bodies,
|
||
+ if ((pm_seq_index = SeqIndexOf(ctx->package_promise_context->package_modules_bodies,
|
||
pm->name, PackageManagerSeqCompare)) != -1)
|
||
{
|
||
SeqRemove(ctx->package_promise_context->package_modules_bodies, pm_seq_index);
|
||
@@ -602,7 +602,7 @@ void EvalContextHeapPersistentSave(EvalContext *ctx, const char *name, unsigned
|
||
ClassRef ref = IDRefQualify(ctx, name);
|
||
char *key = ClassRefToString(ref.ns, ref.name);
|
||
ClassRefDestroy(ref);
|
||
-
|
||
+
|
||
size_t tags_length = strlen(tags) + 1;
|
||
size_t new_info_size = sizeof(PersistentClassInfo) + tags_length;
|
||
|
||
@@ -882,13 +882,13 @@ void FreePackageManager(PackageModuleBody *manager)
|
||
static
|
||
PackagePromiseContext *PackagePromiseConfigNew()
|
||
{
|
||
- PackagePromiseContext *package_promise_defaults =
|
||
+ PackagePromiseContext *package_promise_defaults =
|
||
xmalloc(sizeof(PackagePromiseContext));
|
||
package_promise_defaults->control_package_module = NULL;
|
||
package_promise_defaults->control_package_inventory = NULL;
|
||
package_promise_defaults->package_modules_bodies =
|
||
SeqNew(5, FreePackageManager);
|
||
-
|
||
+
|
||
return package_promise_defaults;
|
||
}
|
||
|
||
@@ -2019,11 +2019,14 @@ static void VarRefStackQualify(const EvalContext *ctx, VarRef *ref)
|
||
break;
|
||
|
||
case STACK_FRAME_TYPE_BUNDLE:
|
||
- VarRefQualify(ref, last_frame->data.bundle.owner->ns, last_frame->data.bundle.owner->name);
|
||
+ VarRefQualify(ref,
|
||
+ last_frame->data.bundle.owner->ns,
|
||
+ last_frame->data.bundle.owner->name);
|
||
break;
|
||
|
||
case STACK_FRAME_TYPE_PROMISE:
|
||
case STACK_FRAME_TYPE_PROMISE_ITERATION:
|
||
+ // Allow special "this" variables to work when used without "this"
|
||
VarRefQualify(ref, NULL, SpecialScopeToString(SPECIAL_SCOPE_THIS));
|
||
break;
|
||
}
|
||
@@ -2081,6 +2084,7 @@ static Variable *VariableResolve(const EvalContext *ctx, const VarRef *ref)
|
||
{
|
||
assert(ref->lval);
|
||
|
||
+ // RECURSION! Try to qualify non-scoped vars in a promise to "this" scope
|
||
if (!VarRefIsQualified(ref))
|
||
{
|
||
VarRef *qref = VarRefCopy(ref);
|
||
|
||
From cb345d161dc3bb67ff35c24f74749cf3a92b3c0f Mon Sep 17 00:00:00 2001
|
||
From: Dimitrios Apostolou <jimis@northern.tech>
|
||
Date: Thu, 15 Feb 2018 16:32:30 +0100
|
||
Subject: [PATCH 2/5] Log stack type in vartable debug messages.
|
||
|
||
---
|
||
libpromises/eval_context.c | 27 +++++++++++++++++++++++++--
|
||
libpromises/eval_context.h | 3 ++-
|
||
2 files changed, 27 insertions(+), 3 deletions(-)
|
||
|
||
diff --git a/libpromises/eval_context.c b/libpromises/eval_context.c
|
||
index cef034122f..bf7156833c 100644
|
||
--- a/libpromises/eval_context.c
|
||
+++ b/libpromises/eval_context.c
|
||
@@ -52,6 +52,15 @@
|
||
#include <conversion.h> /* DataTypeIsIterable */
|
||
|
||
|
||
+static const char *STACK_FRAME_TYPE_STR[STACK_FRAME_TYPE_MAX] = {
|
||
+ "BUNDLE",
|
||
+ "BODY",
|
||
+ "PROMISE_TYPE",
|
||
+ "PROMISE",
|
||
+ "PROMISE_ITERATION"
|
||
+};
|
||
+
|
||
+
|
||
/**
|
||
Define FuncCacheMap.
|
||
Key: an Rlist (which is linked list of Rvals)
|
||
@@ -1079,6 +1088,9 @@ Rlist *EvalContextGetPromiseCallerMethods(EvalContext *ctx) {
|
||
|
||
case STACK_FRAME_TYPE_PROMISE_TYPE:
|
||
break;
|
||
+
|
||
+ default:
|
||
+ ProgrammingError("Unhandled stack frame type");
|
||
}
|
||
}
|
||
return callers_promisers;
|
||
@@ -1126,6 +1138,9 @@ JsonElement *EvalContextGetPromiseCallers(EvalContext *ctx) {
|
||
JsonObjectAppendString(f, "type", "promise_type");
|
||
JsonObjectAppendString(f, "promise_type", frame->data.promise_type.owner->name);
|
||
break;
|
||
+
|
||
+ default:
|
||
+ ProgrammingError("Unhandled stack frame type");
|
||
}
|
||
|
||
JsonArrayAppendObject(callers, f);
|
||
@@ -1244,7 +1259,8 @@ static void EvalContextStackPushFrame(EvalContext *ctx, StackFrame *frame)
|
||
assert(!frame->path);
|
||
frame->path = EvalContextStackPath(ctx);
|
||
|
||
- LogDebug(LOG_MOD_EVALCTX, "PUSHED FRAME (type %d)", frame->type);
|
||
+ LogDebug(LOG_MOD_EVALCTX, "PUSHED FRAME (type %s)",
|
||
+ STACK_FRAME_TYPE_STR[frame->type]);
|
||
}
|
||
|
||
void EvalContextStackPushBundleFrame(EvalContext *ctx, const Bundle *owner, const Rlist *args, bool inherits_previous)
|
||
@@ -1465,7 +1481,8 @@ void EvalContextStackPopFrame(EvalContext *ctx)
|
||
}
|
||
}
|
||
|
||
- LogDebug(LOG_MOD_EVALCTX, "POPPED FRAME (type %d)", last_frame_type);
|
||
+ LogDebug(LOG_MOD_EVALCTX, "POPPED FRAME (type %s)",
|
||
+ STACK_FRAME_TYPE_STR[last_frame_type]);
|
||
}
|
||
|
||
bool EvalContextClassRemove(EvalContext *ctx, const char *ns, const char *name)
|
||
@@ -1798,6 +1815,9 @@ char *EvalContextStackPath(const EvalContext *ctx)
|
||
PromiseIteratorIndex(frame->data.promise_iteration.iter_ctx));
|
||
}
|
||
break;
|
||
+
|
||
+ default:
|
||
+ ProgrammingError("Unhandled stack frame type");
|
||
}
|
||
}
|
||
|
||
@@ -2029,6 +2049,9 @@ static void VarRefStackQualify(const EvalContext *ctx, VarRef *ref)
|
||
// Allow special "this" variables to work when used without "this"
|
||
VarRefQualify(ref, NULL, SpecialScopeToString(SPECIAL_SCOPE_THIS));
|
||
break;
|
||
+
|
||
+ default:
|
||
+ ProgrammingError("Unhandled stack frame type");
|
||
}
|
||
}
|
||
|
||
diff --git a/libpromises/eval_context.h b/libpromises/eval_context.h
|
||
index 49cfea0f01..adbc11f2d5 100644
|
||
--- a/libpromises/eval_context.h
|
||
+++ b/libpromises/eval_context.h
|
||
@@ -44,7 +44,8 @@ typedef enum
|
||
STACK_FRAME_TYPE_BODY,
|
||
STACK_FRAME_TYPE_PROMISE_TYPE,
|
||
STACK_FRAME_TYPE_PROMISE,
|
||
- STACK_FRAME_TYPE_PROMISE_ITERATION
|
||
+ STACK_FRAME_TYPE_PROMISE_ITERATION,
|
||
+ STACK_FRAME_TYPE_MAX
|
||
} StackFrameType;
|
||
|
||
typedef struct
|
||
|
||
From 7aef85e0911dae5d4cb3ef92fecf46bb2cad928f Mon Sep 17 00:00:00 2001
|
||
From: Dimitrios Apostolou <jimis@northern.tech>
|
||
Date: Thu, 15 Feb 2018 19:55:54 +0100
|
||
Subject: [PATCH 3/5] NULL scope means unscoped variable.
|
||
|
||
---
|
||
libpromises/scope.c | 14 +++++++++-----
|
||
1 file changed, 9 insertions(+), 5 deletions(-)
|
||
|
||
diff --git a/libpromises/scope.c b/libpromises/scope.c
|
||
index 3507d67854..39ebadaab6 100644
|
||
--- a/libpromises/scope.c
|
||
+++ b/libpromises/scope.c
|
||
@@ -67,7 +67,11 @@ const char *SpecialScopeToString(SpecialScope scope)
|
||
|
||
SpecialScope SpecialScopeFromString(const char *scope)
|
||
{
|
||
- if (strcmp("const", scope) == 0)
|
||
+ if (scope == NULL)
|
||
+ {
|
||
+ return SPECIAL_SCOPE_NONE;
|
||
+ }
|
||
+ else if (strcmp("const", scope) == 0)
|
||
{
|
||
return SPECIAL_SCOPE_CONST;
|
||
}
|
||
@@ -99,10 +103,10 @@ SpecialScope SpecialScopeFromString(const char *scope)
|
||
{
|
||
return SPECIAL_SCOPE_BODY;
|
||
}
|
||
- else
|
||
- {
|
||
- return SPECIAL_SCOPE_NONE;
|
||
- }
|
||
+
|
||
+ /* All other scopes fall here, for example all bundle names. It means that
|
||
+ * scope was not special. */
|
||
+ return SPECIAL_SCOPE_NONE;
|
||
}
|
||
|
||
void ScopeAugment(EvalContext *ctx, const Bundle *bp, const Promise *pp, const Rlist *arguments)
|
||
|
||
From 48d83dd277321a43a7934cd7108f6e2129c9072f Mon Sep 17 00:00:00 2001
|
||
From: Alexis Mousset <contact@amousset.me>
|
||
Date: Fri, 31 Mar 2017 14:06:57 +0200
|
||
Subject: [PATCH 4/5] CFE-2524: Speedup evalution by not copying variables
|
||
table when expanding a promise
|
||
|
||
The idea is to stop copying variable tables, which appears to be
|
||
one of the most heavy operation the agent does, but to always
|
||
lookup the variables in the global table.
|
||
|
||
The main issue with this change is to manage ambiguities
|
||
in the use of the "this" scope. It is used both for special
|
||
variables at promise level and as an alias for variables in the
|
||
current bundle.
|
||
|
||
The general idea is to modify VariableResolve to try to resolve
|
||
unknown "this"-scoped variables as variables in the current bundle.
|
||
|
||
Then is a second change to force "this"-scoped variables
|
||
passed as parameters to a bundle to be qualified explicitly in
|
||
the calling bundle.
|
||
|
||
A non-compatible change could be to stop supporting "this" scope
|
||
for non-special bundle, which would allow a clean separation
|
||
and avoid the workarounds.
|
||
|
||
Changelog: Title
|
||
Co-Authored-By: Dimitrios Apostolou <jimis@northern.tech>
|
||
---
|
||
libpromises/eval_context.c | 124 ++++++++++++---------
|
||
libpromises/expand.c | 3 +-
|
||
libpromises/var_expressions.c | 33 ++++++
|
||
libpromises/var_expressions.h | 2 +
|
||
libpromises/variable.c | 20 ----
|
||
.../acceptance/01_vars/01_basic/this_variables.cf | 63 +++++++++++
|
||
.../01_vars/01_basic/this_variables.cf.finish | 3 +
|
||
.../02_functions/nth_datacontainer.cf.expected | 14 +--
|
||
8 files changed, 180 insertions(+), 82 deletions(-)
|
||
create mode 100644 tests/acceptance/01_vars/01_basic/this_variables.cf
|
||
create mode 100644 tests/acceptance/01_vars/01_basic/this_variables.cf.finish
|
||
|
||
diff --git a/libpromises/eval_context.c b/libpromises/eval_context.c
|
||
index bf7156833c..115bbf5f56 100644
|
||
--- a/libpromises/eval_context.c
|
||
+++ b/libpromises/eval_context.c
|
||
@@ -1350,16 +1350,8 @@ void EvalContextStackPushPromiseFrame(EvalContext *ctx, const Promise *owner, bo
|
||
|
||
EvalContextStackPushFrame(ctx, frame);
|
||
|
||
- if (copy_bundle_context)
|
||
- {
|
||
- frame->data.promise.vars = VariableTableCopyLocalized(ctx->global_variables,
|
||
- EvalContextStackCurrentBundle(ctx)->ns,
|
||
- EvalContextStackCurrentBundle(ctx)->name);
|
||
- }
|
||
- else
|
||
- {
|
||
- frame->data.promise.vars = VariableTableNew();
|
||
- }
|
||
+ // Ignore copy_bundle_context and create an empty table
|
||
+ frame->data.promise.vars = VariableTableNew();
|
||
|
||
if (PromiseGetBundle(owner)->source_path)
|
||
{
|
||
@@ -1954,6 +1946,8 @@ static VariableTable *GetVariableTableForScope(const EvalContext *ctx,
|
||
return frame ? frame->data.body.vars : NULL;
|
||
}
|
||
|
||
+ // "this" variables can be in local or global variable table (when this is used for non-special
|
||
+ // varables), so return local as VariableResolve will try global table anyway.
|
||
case SPECIAL_SCOPE_THIS:
|
||
{
|
||
StackFrame *frame = LastStackFrameByType(ctx, STACK_FRAME_TYPE_PROMISE);
|
||
@@ -2094,33 +2088,15 @@ bool EvalContextVariablePut(EvalContext *ctx,
|
||
return true;
|
||
}
|
||
|
||
-/*
|
||
- * Looks up a variable in the the context of the 'current scope'. This basically
|
||
- * means that an unqualified reference will be looked up in the context of the top
|
||
- * stack frame. Note that when evaluating a promise, this will qualify a reference
|
||
- * to the 'this' scope.
|
||
- *
|
||
- * Ideally, this function should resolve a variable by walking down the stack, but
|
||
- * this is pending rework in variable iteration.
|
||
- */
|
||
-static Variable *VariableResolve(const EvalContext *ctx, const VarRef *ref)
|
||
+static Variable *VariableResolve2(const EvalContext *ctx, const VarRef *ref)
|
||
{
|
||
- assert(ref->lval);
|
||
-
|
||
- // RECURSION! Try to qualify non-scoped vars in a promise to "this" scope
|
||
- if (!VarRefIsQualified(ref))
|
||
- {
|
||
- VarRef *qref = VarRefCopy(ref);
|
||
- VarRefStackQualify(ctx, qref);
|
||
- Variable *ret = VariableResolve(ctx, qref);
|
||
- VarRefDestroy(qref);
|
||
- return ret;
|
||
- }
|
||
-
|
||
+ // Get the variable table associated to the scope
|
||
VariableTable *table = GetVariableTableForScope(ctx, ref->ns, ref->scope);
|
||
+
|
||
+ Variable *var;
|
||
if (table)
|
||
{
|
||
- Variable *var = VariableTableGet(table, ref);
|
||
+ var = VariableTableGet(table, ref);
|
||
if (var)
|
||
{
|
||
return var;
|
||
@@ -2141,6 +2117,67 @@ static Variable *VariableResolve(const EvalContext *ctx, const VarRef *ref)
|
||
return NULL;
|
||
}
|
||
|
||
+/*
|
||
+ * Looks up a variable in the the context of the 'current scope'. This
|
||
+ * basically means that an unqualified reference will be looked up in the
|
||
+ * context of the top stack frame.
|
||
+ *
|
||
+ * Note that when evaluating a promise, this
|
||
+ * will qualify a reference to 'this' scope and when evaluating a body, it
|
||
+ * will qualify a reference to 'body' scope.
|
||
+ */
|
||
+static Variable *VariableResolve(const EvalContext *ctx, const VarRef *ref)
|
||
+{
|
||
+ assert(ref->lval);
|
||
+
|
||
+ /* We will make a first lookup that works in almost all cases: will look
|
||
+ * for local or global variables, depending of the current scope. */
|
||
+
|
||
+ Variable *var1 = VariableResolve2(ctx, ref);
|
||
+ if (var1)
|
||
+ {
|
||
+ return var1;
|
||
+ }
|
||
+
|
||
+ /* Try to qualify non-scoped vars to the scope:
|
||
+ "this" for promises, "body" for bodies, current bundle for bundles. */
|
||
+ VarRef *scoped_ref = NULL;
|
||
+ if (!VarRefIsQualified(ref))
|
||
+ {
|
||
+ scoped_ref = VarRefCopy(ref);
|
||
+ VarRefStackQualify(ctx, scoped_ref);
|
||
+ Variable *var2 = VariableResolve2(ctx, scoped_ref);
|
||
+ if (var2)
|
||
+ {
|
||
+ VarRefDestroy(scoped_ref);
|
||
+ return var2;
|
||
+ }
|
||
+ ref = scoped_ref; /* continue with the scoped variable */
|
||
+ }
|
||
+
|
||
+ const Bundle *last_bundle = EvalContextStackCurrentBundle(ctx);
|
||
+
|
||
+ /* If we are in a promise or a body, the variable might be coming from the
|
||
+ * last bundle. So try a last lookup with "this" or "body" special scopes
|
||
+ * replaced with the last bundle. */
|
||
+
|
||
+ if ((SpecialScopeFromString(ref->scope) == SPECIAL_SCOPE_THIS ||
|
||
+ SpecialScopeFromString(ref->scope) == SPECIAL_SCOPE_BODY)
|
||
+ && last_bundle != NULL)
|
||
+ {
|
||
+ VarRef *ref2 = VarRefCopy(ref);
|
||
+ VarRefQualify(ref2, last_bundle->ns, last_bundle->name);
|
||
+ Variable *var3 = VariableResolve2(ctx, ref2);
|
||
+
|
||
+ VarRefDestroy(scoped_ref);
|
||
+ VarRefDestroy(ref2);
|
||
+ return var3;
|
||
+ }
|
||
+
|
||
+ VarRefDestroy(scoped_ref);
|
||
+ return NULL;
|
||
+}
|
||
+
|
||
/**
|
||
*
|
||
* @NOTE NULL is a valid return value if #type_out is of list type and the
|
||
@@ -2176,27 +2213,6 @@ const void *EvalContextVariableGet(const EvalContext *ctx, const VarRef *ref, Da
|
||
return var->rval.item;
|
||
}
|
||
}
|
||
- else if (!VarRefIsQualified(ref))
|
||
- {
|
||
- /*
|
||
- * FALLBACK: Because VariableResolve currently does not walk the stack
|
||
- * (rather, it looks at only the top frame), we do an explicit retry
|
||
- * here to qualify an unqualified reference to the current bundle.
|
||
- *
|
||
- * This is overly complicated, and will go away as soon as
|
||
- * VariableResolve can walk the stack (which is pending rework in
|
||
- * variable iteration).
|
||
- */
|
||
- const Bundle *bp = EvalContextStackCurrentBundle(ctx);
|
||
- if (bp)
|
||
- {
|
||
- VarRef *qref = VarRefCopy(ref);
|
||
- VarRefQualify(qref, bp->ns, bp->name);
|
||
- const void *ret = EvalContextVariableGet(ctx, qref, type_out);
|
||
- VarRefDestroy(qref);
|
||
- return ret;
|
||
- }
|
||
- }
|
||
|
||
if (type_out)
|
||
{
|
||
diff --git a/libpromises/expand.c b/libpromises/expand.c
|
||
index 91426420eb..eaa7d0de8d 100644
|
||
--- a/libpromises/expand.c
|
||
+++ b/libpromises/expand.c
|
||
@@ -256,7 +256,8 @@ PromiseResult ExpandPromise(EvalContext *ctx, const Promise *pp,
|
||
* (including body inheritance). */
|
||
Promise *pcopy = DeRefCopyPromise(ctx, pp);
|
||
|
||
- EvalContextStackPushPromiseFrame(ctx, pcopy, true);
|
||
+ // TODO: Remove last parameter?
|
||
+ EvalContextStackPushPromiseFrame(ctx, pcopy, false);
|
||
PromiseIterator *iterctx = PromiseIteratorNew(pcopy);
|
||
|
||
/* 2. Parse all strings (promiser-promisee-constraints), find all
|
||
diff --git a/libpromises/var_expressions.c b/libpromises/var_expressions.c
|
||
index 6250e3b067..94d478cffb 100644
|
||
--- a/libpromises/var_expressions.c
|
||
+++ b/libpromises/var_expressions.c
|
||
@@ -317,6 +317,16 @@ VarRef *VarRefParseFromNamespaceAndScope(const char *qualified_name,
|
||
{
|
||
_ns = NULL;
|
||
}
|
||
+
|
||
+ /*
|
||
+ * Force considering non-special "this." variables as unqualified.
|
||
+ * This allows qualifying bundle parameters passed as reference with a "this" scope
|
||
+ * in the calling bundle.
|
||
+ */
|
||
+ if (is_this_not_special(scope, lval)) {
|
||
+ free(scope);
|
||
+ scope = NULL;
|
||
+ }
|
||
}
|
||
|
||
VarRef *ref = xmalloc(sizeof(VarRef));
|
||
@@ -330,6 +340,29 @@ VarRef *VarRefParseFromNamespaceAndScope(const char *qualified_name,
|
||
return ref;
|
||
}
|
||
|
||
+/*
|
||
+ * This function will return true if the given variable is
|
||
+ * a this.something variable that is an alias to a non-special local variable.
|
||
+ */
|
||
+bool is_this_not_special(const char *scope, const char *lval) {
|
||
+ // TODO: better way to get this list?
|
||
+ const char *special_this_variables[] = {"v","k","this","service_policy","promiser","promiser_uid","promiser_gid","promiser_pid","promiser_ppid","bundle","handle","namespace","promise_filename","promise_dirname","promise_linenumber", NULL};
|
||
+
|
||
+ if (!scope) {
|
||
+ return false;
|
||
+ }
|
||
+
|
||
+ if (SpecialScopeFromString(scope) != SPECIAL_SCOPE_THIS) {
|
||
+ return false;
|
||
+ }
|
||
+
|
||
+ if (IsStrIn(lval, special_this_variables)) {
|
||
+ return false;
|
||
+ }
|
||
+
|
||
+ return true;
|
||
+}
|
||
+
|
||
VarRef *VarRefParse(const char *var_ref_string)
|
||
{
|
||
return VarRefParseFromNamespaceAndScope(var_ref_string, NULL, NULL, CF_NS, '.');
|
||
diff --git a/libpromises/var_expressions.h b/libpromises/var_expressions.h
|
||
index 716b4faa0f..92f5b89f4d 100644
|
||
--- a/libpromises/var_expressions.h
|
||
+++ b/libpromises/var_expressions.h
|
||
@@ -57,6 +57,8 @@ VarRef *VarRefCopy(const VarRef *ref);
|
||
VarRef *VarRefCopyLocalized(const VarRef *ref);
|
||
VarRef *VarRefCopyIndexless(const VarRef *ref);
|
||
|
||
+bool is_this_not_special(const char *scope, const char *lval);
|
||
+
|
||
VarRef *VarRefParse(const char *var_ref_string);
|
||
|
||
VarRef *VarRefParseFromBundle(const char *var_ref_string, const Bundle *bundle);
|
||
diff --git a/libpromises/variable.c b/libpromises/variable.c
|
||
index deabd5c933..ccc13a9c96 100644
|
||
--- a/libpromises/variable.c
|
||
+++ b/libpromises/variable.c
|
||
@@ -341,23 +341,3 @@ void VariableTableIteratorDestroy(VariableTableIterator *iter)
|
||
free(iter);
|
||
}
|
||
}
|
||
-
|
||
-VariableTable *VariableTableCopyLocalized(const VariableTable *table, const char *ns, const char *scope)
|
||
-{
|
||
- VariableTable *localized_copy = VariableTableNew();
|
||
-
|
||
- VariableTableIterator *iter = VariableTableIteratorNew(table, ns, scope, NULL);
|
||
- Variable *foreign_var = NULL;
|
||
- while ((foreign_var = VariableTableIteratorNext(iter)))
|
||
- {
|
||
- /* TODO why is tags NULL here? Shouldn't it be an exact copy of
|
||
- * foreign_var->tags? */
|
||
- Variable *localized_var = VariableNew(VarRefCopyLocalized(foreign_var->ref),
|
||
- RvalCopy(foreign_var->rval), foreign_var->type,
|
||
- NULL, foreign_var->promise);
|
||
- VarMapInsert(localized_copy->vars, localized_var->ref, localized_var);
|
||
- }
|
||
- VariableTableIteratorDestroy(iter);
|
||
-
|
||
- return localized_copy;
|
||
-}
|
||
diff --git a/tests/acceptance/01_vars/01_basic/this_variables.cf b/tests/acceptance/01_vars/01_basic/this_variables.cf
|
||
new file mode 100644
|
||
index 0000000000..560573c3bd
|
||
--- /dev/null
|
||
+++ b/tests/acceptance/01_vars/01_basic/this_variables.cf
|
||
@@ -0,0 +1,63 @@
|
||
+# This was created because of a bug while working on purging variable table
|
||
+# copying for CFE-2524. The issue was that variables were not found when
|
||
+# looked up deep inside the INI_SECTION body.
|
||
+
|
||
+body common control
|
||
+{
|
||
+ inputs => { "../../default.cf.sub" };
|
||
+ bundlesequence => { default("$(this.promise_filename)") };
|
||
+ version => "1.0";
|
||
+}
|
||
+
|
||
+bundle agent init
|
||
+{
|
||
+ files:
|
||
+ "$(G.testfile).expected"
|
||
+ copy_from => local_cp("$(this.promise_filename).finish");
|
||
+}
|
||
+
|
||
+bundle agent test
|
||
+{
|
||
+ vars:
|
||
+ "var" slist => { "var in test" };
|
||
+ "var_test" slist => { "var_test in test" };
|
||
+
|
||
+ files:
|
||
+ "$(G.testfile).actual"
|
||
+ create => "true",
|
||
+ edit_line => test_edit_line("testline");
|
||
+}
|
||
+
|
||
+bundle edit_line test_edit_line(line)
|
||
+{
|
||
+
|
||
+ vars:
|
||
+ "sectionName" string => "test";
|
||
+
|
||
+ insert_lines:
|
||
+ "[$(sectionName)]
|
||
+[end]"
|
||
+ location => start;
|
||
+
|
||
+ "${line}"
|
||
+ select_region => INI_section(escape("$(sectionName)"));
|
||
+
|
||
+}
|
||
+
|
||
+bundle agent check
|
||
+{
|
||
+ methods:
|
||
+ "check"
|
||
+ usebundle => dcs_if_diff( "$(G.testfile).actual",
|
||
+ "$(G.testfile).expected",
|
||
+ "pass", "_fail");
|
||
+
|
||
+ # Fail the test if any of the files fail.
|
||
+ "fail"
|
||
+ usebundle => dcs_fail( $(this.promise_filename) ),
|
||
+ ifvarclass => "_fail";
|
||
+
|
||
+ pass::
|
||
+ "pass"
|
||
+ usebundle => dcs_pass( $(this.promise_filename) );
|
||
+}
|
||
diff --git a/tests/acceptance/01_vars/01_basic/this_variables.cf.finish b/tests/acceptance/01_vars/01_basic/this_variables.cf.finish
|
||
new file mode 100644
|
||
index 0000000000..904bb6cc72
|
||
--- /dev/null
|
||
+++ b/tests/acceptance/01_vars/01_basic/this_variables.cf.finish
|
||
@@ -0,0 +1,3 @@
|
||
+[test]
|
||
+testline
|
||
+[end]
|
||
diff --git a/tests/acceptance/01_vars/02_functions/nth_datacontainer.cf.expected b/tests/acceptance/01_vars/02_functions/nth_datacontainer.cf.expected
|
||
index fc702b7038..9375b9d119 100644
|
||
--- a/tests/acceptance/01_vars/02_functions/nth_datacontainer.cf.expected
|
||
+++ b/tests/acceptance/01_vars/02_functions/nth_datacontainer.cf.expected
|
||
@@ -1,20 +1,20 @@
|
||
jsonstring = {"boolean":true,"boolean_2":false,"integer":20130111,"integer_2":987654321,"list":["chris","dituri","was","here"],"null":null,"object":{"a":true,"b":[1,2,3],"c":"cat","d":108},"string":"Figaro. Figaro. Figaro, Figaro, Figaro... Figaro!","string_2":"Othello? Where art thou now?"}
|
||
keys:json = boolean
|
||
-keys:json = string
|
||
+keys:json = boolean_2
|
||
keys:json = integer
|
||
+keys:json = integer_2
|
||
keys:json = list
|
||
+keys:json = null
|
||
keys:json = object
|
||
-keys:json = integer_2
|
||
+keys:json = string
|
||
keys:json = string_2
|
||
-keys:json = boolean_2
|
||
-keys:json = null
|
||
primitive:json[boolean] = true
|
||
-primitive:json[string] = Figaro. Figaro. Figaro, Figaro, Figaro... Figaro!
|
||
+primitive:json[boolean_2] = false
|
||
primitive:json[integer] = 20130111
|
||
primitive:json[integer_2] = 987654321
|
||
-primitive:json[string_2] = Othello? Where art thou now?
|
||
-primitive:json[boolean_2] = false
|
||
primitive:json[null] = null
|
||
+primitive:json[string] = Figaro. Figaro. Figaro, Figaro, Figaro... Figaro!
|
||
+primitive:json[string_2] = Othello? Where art thou now?
|
||
list:json[0] = chris
|
||
list:json[1] = dituri
|
||
list:json[2] = was
|
||
|
||
From e4d90658f3ff71d4593c00da970655dd7b2c3552 Mon Sep 17 00:00:00 2001
|
||
From: Dimitrios Apostolou <jimis@northern.tech>
|
||
Date: Thu, 15 Feb 2018 20:29:42 +0100
|
||
Subject: [PATCH 5/5] Purge unused variable.
|
||
|
||
---
|
||
cf-agent/verify_packages.c | 12 ++++++------
|
||
libpromises/eval_context.c | 4 ++--
|
||
libpromises/eval_context.h | 2 +-
|
||
libpromises/expand.c | 3 +--
|
||
4 files changed, 10 insertions(+), 11 deletions(-)
|
||
|
||
diff --git a/cf-agent/verify_packages.c b/cf-agent/verify_packages.c
|
||
index e8644f931c..54b8564bca 100644
|
||
--- a/cf-agent/verify_packages.c
|
||
+++ b/cf-agent/verify_packages.c
|
||
@@ -2639,7 +2639,7 @@ static bool ExecuteSchedule(EvalContext *ctx, const PackageManager *schedule, Pa
|
||
Log(LOG_LEVEL_VERBOSE, "Command does not allow arguments");
|
||
PromiseResult result = PROMISE_RESULT_NOOP;
|
||
|
||
- EvalContextStackPushPromiseFrame(ctx, pp, false);
|
||
+ EvalContextStackPushPromiseFrame(ctx, pp);
|
||
if (EvalContextStackPushPromiseIterationFrame(ctx, NULL))
|
||
{
|
||
if (ExecPackageCommand(ctx, command_string, verify, true, a, pp, &result))
|
||
@@ -2692,7 +2692,7 @@ static bool ExecuteSchedule(EvalContext *ctx, const PackageManager *schedule, Pa
|
||
}
|
||
|
||
PromiseResult result = PROMISE_RESULT_NOOP;
|
||
- EvalContextStackPushPromiseFrame(ctx, ppi, false);
|
||
+ EvalContextStackPushPromiseFrame(ctx, ppi);
|
||
if (EvalContextStackPushPromiseIterationFrame(ctx, NULL))
|
||
{
|
||
if (ExecPackageCommand(ctx, command_string, verify, true, a, ppi, &result))
|
||
@@ -2753,7 +2753,7 @@ static bool ExecuteSchedule(EvalContext *ctx, const PackageManager *schedule, Pa
|
||
}
|
||
|
||
PromiseResult result = PROMISE_RESULT_NOOP;
|
||
- EvalContextStackPushPromiseFrame(ctx, pp, false);
|
||
+ EvalContextStackPushPromiseFrame(ctx, pp);
|
||
if (EvalContextStackPushPromiseIterationFrame(ctx, NULL))
|
||
{
|
||
bool ok = ExecPackageCommand(ctx, command_string, verify, true, a, pp, &result);
|
||
@@ -2910,7 +2910,7 @@ static bool ExecutePatch(EvalContext *ctx, const PackageManager *schedule, Packa
|
||
Log(LOG_LEVEL_VERBOSE, "Command does not allow arguments");
|
||
|
||
PromiseResult result = PROMISE_RESULT_NOOP;
|
||
- EvalContextStackPushPromiseFrame(ctx, pp, false);
|
||
+ EvalContextStackPushPromiseFrame(ctx, pp);
|
||
if (EvalContextStackPushPromiseIterationFrame(ctx, NULL))
|
||
{
|
||
if (ExecPackageCommand(ctx, command_string, false, true, a, pp, &result))
|
||
@@ -2943,7 +2943,7 @@ static bool ExecutePatch(EvalContext *ctx, const PackageManager *schedule, Packa
|
||
strcat(offset, pi->name);
|
||
|
||
PromiseResult result = PROMISE_RESULT_NOOP;
|
||
- EvalContextStackPushPromiseFrame(ctx, pp, false);
|
||
+ EvalContextStackPushPromiseFrame(ctx, pp);
|
||
if (EvalContextStackPushPromiseIterationFrame(ctx, NULL))
|
||
{
|
||
if (ExecPackageCommand(ctx, command_string, false, true, a, pp, &result))
|
||
@@ -2982,7 +2982,7 @@ static bool ExecutePatch(EvalContext *ctx, const PackageManager *schedule, Packa
|
||
}
|
||
|
||
PromiseResult result = PROMISE_RESULT_NOOP;
|
||
- EvalContextStackPushPromiseFrame(ctx, pp, false);
|
||
+ EvalContextStackPushPromiseFrame(ctx, pp);
|
||
if (EvalContextStackPushPromiseIterationFrame(ctx, NULL))
|
||
{
|
||
bool ok = ExecPackageCommand(ctx, command_string, false, true, a, pp, &result);
|
||
diff --git a/libpromises/eval_context.c b/libpromises/eval_context.c
|
||
index 115bbf5f56..e97fb22026 100644
|
||
--- a/libpromises/eval_context.c
|
||
+++ b/libpromises/eval_context.c
|
||
@@ -1339,7 +1339,7 @@ void EvalContextStackPushPromiseTypeFrame(EvalContext *ctx, const PromiseType *o
|
||
EvalContextStackPushFrame(ctx, frame);
|
||
}
|
||
|
||
-void EvalContextStackPushPromiseFrame(EvalContext *ctx, const Promise *owner, bool copy_bundle_context)
|
||
+void EvalContextStackPushPromiseFrame(EvalContext *ctx, const Promise *owner)
|
||
{
|
||
assert(LastStackFrame(ctx, 0));
|
||
assert(LastStackFrame(ctx, 0)->type == STACK_FRAME_TYPE_PROMISE_TYPE);
|
||
@@ -1350,7 +1350,7 @@ void EvalContextStackPushPromiseFrame(EvalContext *ctx, const Promise *owner, bo
|
||
|
||
EvalContextStackPushFrame(ctx, frame);
|
||
|
||
- // Ignore copy_bundle_context and create an empty table
|
||
+ // create an empty table
|
||
frame->data.promise.vars = VariableTableNew();
|
||
|
||
if (PromiseGetBundle(owner)->source_path)
|
||
diff --git a/libpromises/eval_context.h b/libpromises/eval_context.h
|
||
index adbc11f2d5..a87618d3ce 100644
|
||
--- a/libpromises/eval_context.h
|
||
+++ b/libpromises/eval_context.h
|
||
@@ -141,7 +141,7 @@ Rlist *EvalContextGetPromiseCallerMethods(EvalContext *ctx);
|
||
void EvalContextStackPushBundleFrame(EvalContext *ctx, const Bundle *owner, const Rlist *args, bool inherits_previous);
|
||
void EvalContextStackPushBodyFrame(EvalContext *ctx, const Promise *caller, const Body *body, const Rlist *args);
|
||
void EvalContextStackPushPromiseTypeFrame(EvalContext *ctx, const PromiseType *owner);
|
||
-void EvalContextStackPushPromiseFrame(EvalContext *ctx, const Promise *owner, bool copy_bundle_context);
|
||
+void EvalContextStackPushPromiseFrame(EvalContext *ctx, const Promise *owner);
|
||
Promise *EvalContextStackPushPromiseIterationFrame(EvalContext *ctx, const PromiseIterator *iter_ctx);
|
||
void EvalContextStackPopFrame(EvalContext *ctx);
|
||
const char *EvalContextStackToString(EvalContext *ctx);
|
||
diff --git a/libpromises/expand.c b/libpromises/expand.c
|
||
index eaa7d0de8d..e334ffef94 100644
|
||
--- a/libpromises/expand.c
|
||
+++ b/libpromises/expand.c
|
||
@@ -256,8 +256,7 @@ PromiseResult ExpandPromise(EvalContext *ctx, const Promise *pp,
|
||
* (including body inheritance). */
|
||
Promise *pcopy = DeRefCopyPromise(ctx, pp);
|
||
|
||
- // TODO: Remove last parameter?
|
||
- EvalContextStackPushPromiseFrame(ctx, pcopy, false);
|
||
+ EvalContextStackPushPromiseFrame(ctx, pcopy);
|
||
PromiseIterator *iterctx = PromiseIteratorNew(pcopy);
|
||
|
||
/* 2. Parse all strings (promiser-promisee-constraints), find all
|
Also available in: Unified diff
Fixes #12130: Backport complete variable table performance patch