Merge ~mirespace/ubuntu/+source/augeas:lp-1828074-bionic-augeas-memorybomb into ubuntu/+source/augeas:ubuntu/bionic-devel

[Miriam España Acebal]

Hi Team,

PPA for these SRUs is : ppa:mirespace/sru-lp-1828074-bionic-augeas-large-http-files

Two issues were fixed here:

- The use of memory grows as a site conf file enlarges when using augeas:
    * d/p/0005-Memory-usage-issues-with-large-Apache-httpd-configuration-files.patch:
      Replace pure function invocations in
      path expressions with their result. Thanks to
      David Lutterkort <email address hidden>
      (LP: #1828074).

    Steps to reproduce the issue are described in the SRU template of the bug.

- Package augeas was FTBFS in Bionic:
    * d/rules: Disable gnulib tests, thanks to Matthias
    Klose <email address hidden>
    (Closes: #919662)(LP: #1813566).

  Without this, the package can't be built on Bionic (2 tests fail: test-rwlock1 , test-thread_create). This tests are not checking augeas itself as commented on https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=919662

Both fixes have been cherry picked from upstream (the patch) and Debian.

Built tests passed ok (No DEP8 tests present):

============================================================================
Testsuite summary for augeas 1.10.1
============================================================================
# TOTAL: 247
# PASS: 243
# SKIP: 4
# XFAIL: 0
# FAIL: 0
# XPASS: 0
# ERROR: 0
============================================================================

Thanks in advance!

[Utkarsh Gupta (utkarsh)]

Hello,

Thank you for the MP, please find the comments inline. TIA! \o/

review: Needs Fixing

[Miriam España Acebal (mirespace)]

Hello... some changes are done, some answers are inline with the no-changes.

Let me what you think and thanks for reviewing.

[Miriam España Acebal (mirespace)]

(For the inline comments)

[Utkarsh Gupta (utkarsh)]

Looks good, thanks, Miriam. I'll make minor edits in the d/ch entry and the patch whilst sponsoring.

review: Approve

[Miriam España Acebal (mirespace)]

SRUs templates updated!

[Utkarsh Gupta (utkarsh)]

$ dput ubuntu ../augeas_1.10.1-2ubuntu1_source.changes
Checking signature on .changes
gpg: ../augeas_1.10.1-2ubuntu1_source.changes: Valid signature from 823E967606C34B96
Checking signature on .dsc
gpg: ../augeas_1.10.1-2ubuntu1.dsc: Valid signature from 823E967606C34B96
Package includes an .orig.tar.gz file although the debian revision suggests
that it might not be required. Multiple uploads of the .orig.tar.gz may be
rejected by the upload queue management software.
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading augeas_1.10.1-2ubuntu1.dsc: done.
  Uploading augeas_1.10.1.orig.tar.gz: done.
  Uploading augeas_1.10.1-2ubuntu1.debian.tar.xz: done.
  Uploading augeas_1.10.1-2ubuntu1_source.buildinfo: done.
  Uploading augeas_1.10.1-2ubuntu1_source.changes: done.
Successfully uploaded packages.

Unmerged commits

b0c6fa2... by Miriam España Acebal on 2021-09-20

changelog: 1.10.1-2ubuntu1

1e324e6... by Miriam España Acebal on 2021-09-20

update-maintainers

f4e6a83... by Miriam España Acebal on 2021-09-21

* d/rules: Disable gnulib tests due to that tests are not testing augeas
  itself. Thanks to Matthias Klose (Closes: #919662)(LP: #1813566).

c243422... by Miriam España Acebal on 2021-09-20

  * d/p/0005-Replace-pure-funcs-calls-in-path-expr-w-result.patch:
    Replace pure function invocations in path
    expressions with their result. Thanks to
    David Lutterkort <email address hidden>.
    (LP: #1828074)

diff --git a/debian/changelog b/debian/changelog
index a3fef72..7690cfa 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,16 @@
+augeas (1.10.1-2ubuntu1) bionic; urgency=medium
+
+  * d/p/0005-Replace-pure-funcs-calls-in-path-expr-w-result.patch:
+    Replace pure function invocations in path
+    expressions with their result. Thanks to
+    David Lutterkort <[e-mail]>.
+    (LP: #1828074)
+  * d/rules: Disable gnulib tests due to that tests are not
+    testing augeas itself. Thanks to Matthias Klose
+    (Closes: #919662)(LP: #1813566).
+
+ -- Miriam España Acebal <[e-mail]>  Mon, 20 Sep 2021 11:18:53 +0200
+
 augeas (1.10.1-2) unstable; urgency=medium
 
   * Add correct file paths to clamav.aug (Closes: #892913)
diff --git a/debian/control b/debian/control
index 523f25d..20711d5 100644
--- a/debian/control
+++ b/debian/control
@@ -1,6 +1,7 @@
 Source: augeas
 Priority: optional
-Maintainer: Hilko Bengen <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Hilko Bengen <[e-mail]>
 Uploaders:  Micah Anderson <[e-mail]>
 Build-Depends: debhelper (>= 10~),
  libreadline-dev,
diff --git a/debian/patches/0005-Replace-pure-funcs-calls-in-path-expr-w-result.patch b/debian/patches/0005-Replace-pure-funcs-calls-in-path-expr-w-result.patch
 new file mode 100644
index 0000000..57513cd
--- /dev/null
+++ b/debian/patches/0005-Replace-pure-funcs-calls-in-path-expr-w-result.patch
@@ -0,0 +1,143 @@
+Description: Replace pure function invocations in path expressions with their result
+ .
+ In path expressions, we generally need to evaluate functions against every
+ node that we consider for the result set. For example, in the path
+ expression /files/etc/hosts/*[ipaddr =~ regexp('127\\.')], the regexp
+ function was evaluated against every entry in /etc/hosts. Evaluating that
+ function requires the construction and compilation of a new regexp. Because
+ of how memory is managed during evaluation of path expressions, the memory
+ used by all these copies of the same regexp is only freed after we are done
+ evaluating the path expression. This causes unacceptable memory usage in
+ large files (see https://github.com/hercules-team/augeas/issues/569)
+ .
+ To avoid these issues, we now distinguish between pure and impure functions
+ in the path expression interpreter. When we encounter a pure function, we
+ change the AST for the path expression so that the function invocation is
+ replaced with the result of invoking the function. With the example above,
+ that means we only construct and compile the regexp '127\\.' once,
+ regardless of how many nodes it gets checked against. That leads to a
+ dramatic reduction in the memory required to evaluate path expressions with
+ such constructs against large files.
+Author: David Lutterkort <[e-mail]>
+Origin: upstream
+Bug: https://github.com/hercules-team/augeas/issues/569
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/augeas/+bug/1828074
+Forwarded: not-needed
+Applied-Upstream: https://github.com/hercules-team/augeas/pull/578/commits/bbf31f719db54916993be9042254f6d77b61cb13 
+Last-Update: 2021-09-20
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+diff --git a/src/pathx.c b/src/pathx.c
+index 48c8b0b1..4c2aa3a7 100644
+--- a/src/pathx.c
++++ b/src/pathx.c
+@@ -208,6 +208,10 @@ struct expr {
+         struct {                       /* E_APP */
+             const struct func *func;
+             struct expr       **args;
++            /* If fold is true, replace this function invocation
++             * with its value after the first time we evaluate this
++             * expression */
++            bool              fold;
+         };
+     };
+ };
+@@ -282,6 +286,7 @@ struct func {
+     const char      *name;
+     unsigned int     arity;
+     enum type        type;
++    bool             pure;      /* Result only depends on args */
+     const enum type *arg_types;
+     func_impl_t      impl;
+ };
+@@ -304,40 +309,40 @@ static const enum type arg_types_nodeset_string[] = { T_NODESET, T_STRING };
+ 
+ static const struct func builtin_funcs[] = {
+     { .name = "last", .arity = 0, .type = T_NUMBER, .arg_types = NULL,
+-      .impl = func_last },
++      .impl = func_last, .pure = false },
+     { .name = "position", .arity = 0, .type = T_NUMBER, .arg_types = NULL,
+-      .impl = func_position },
++      .impl = func_position, .pure = false },
+     { .name = "label", .arity = 0, .type = T_STRING, .arg_types = NULL,
+-      .impl = func_label },
++      .impl = func_label, .pure = false },
+     { .name = "count", .arity = 1, .type = T_NUMBER,
+       .arg_types = arg_types_nodeset,
+-      .impl = func_count },
++      .impl = func_count, .pure = false },
+     { .name = "regexp", .arity = 1, .type = T_REGEXP,
+       .arg_types = arg_types_string,
+-      .impl = func_regexp },
++      .impl = func_regexp, .pure = true },
+     { .name = "regexp", .arity = 1, .type = T_REGEXP,
+       .arg_types = arg_types_nodeset,
+-      .impl = func_regexp },
++      .impl = func_regexp, .pure = true },
+     { .name = "regexp", .arity = 2, .type = T_REGEXP,
+       .arg_types = arg_types_string_string,
+-      .impl = func_regexp_flag },
++      .impl = func_regexp_flag, .pure = true },
+     { .name = "regexp", .arity = 2, .type = T_REGEXP,
+       .arg_types = arg_types_nodeset_string,
+-      .impl = func_regexp_flag },
++      .impl = func_regexp_flag, .pure = true },
+     { .name = "glob", .arity = 1, .type = T_REGEXP,
+       .arg_types = arg_types_string,
+-      .impl = func_glob },
++      .impl = func_glob, .pure = true },
+     { .name = "glob", .arity = 1, .type = T_REGEXP,
+       .arg_types = arg_types_nodeset,
+-      .impl = func_glob },
++      .impl = func_glob, .pure = true },
+     { .name = "int", .arity = 1, .type = T_NUMBER,
+-      .arg_types = arg_types_string, .impl = func_int },
++      .arg_types = arg_types_string, .impl = func_int, .pure = false },
+     { .name = "int", .arity = 1, .type = T_NUMBER,
+-      .arg_types = arg_types_nodeset, .impl = func_int },
++      .arg_types = arg_types_nodeset, .impl = func_int, .pure = false },
+     { .name = "int", .arity = 1, .type = T_NUMBER,
+-      .arg_types = arg_types_bool, .impl = func_int },
++      .arg_types = arg_types_bool, .impl = func_int, .pure = false },
+     { .name = "not", .arity = 1, .type = T_BOOLEAN,
+-      .arg_types = arg_types_bool, .impl = func_not }
++      .arg_types = arg_types_bool, .impl = func_not, .pure = true }
+ };
+ 
+ #define RET_ON_ERROR                                                    \
+@@ -1409,6 +1414,16 @@ static void eval_expr(struct expr *expr, struct state *state) {
+         break;
+     case E_APP:
+         eval_app(expr, state);
++        if (expr->fold) {
++            /* Do constant folding: replace the function application with
++             * a reference to the value that resulted from evaluating it */
++            for (int i=0; i < expr->func->arity; i++)
++                free_expr(expr->args[i]);
++            free(expr->args);
++            value_ind_t vind = state->values_used - 1;
++            expr->tag = E_VALUE;
++            expr->value_ind = state->values[vind];
++        }
+         break;
+     default:
+         assert(0);
+@@ -1493,6 +1508,18 @@ static void check_app(struct expr *expr, struct state *state) {
+     if (f < ARRAY_CARDINALITY(builtin_funcs)) {
+         expr->func = builtin_funcs + f;
+         expr->type = expr->func->type;
++        expr->fold = expr->func->pure;
++        if (expr->fold) {
++            /* We only do constant folding for invocations of pure functions
++             * whose arguments are literal values. That misses opportunities
++             * for constant folding, e.g., "regexp('foo' + 'bar')" but is
++             * a bit simpler than doing full tracking of constants
++             */
++            for (int i=0; i < expr->func->arity; i++) {
++                if (expr->args[i]->tag != E_VALUE)
++                    expr->fold = false;
++            }
++        }
+     } else {
+         STATE_ERROR(state, PATHX_ETYPE);
+     }
diff --git a/debian/patches/series b/debian/patches/series
index 03f97f2..c9d719e 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -2,3 +2,4 @@
 0002-Skip-tests-that-need-root-privileges-when-fakeroot-h.patch
 0003-Make-NRPE-lens-less-strict.patch
 0004-Add-Debian-specific-paths-for-ClamAV-configuration-f.patch
+0005-Replace-pure-funcs-calls-in-path-expr-w-result.patch
diff --git a/debian/rules b/debian/rules
index f077249..524d61e 100755
--- a/debian/rules
+++ b/debian/rules
@@ -10,6 +10,9 @@ include /usr/share/dpkg/buildflags.mk
 override_dh_autoreconf:
 	dh_autoreconf autoreconf -- -i
 
+override_dh_auto_configure:
+	dh_auto_configure -- --disable-gnulib-tests
+
 override_dh_auto_build:
 	rm -f tetsts/lens-*.sh
 	dh_auto_build