[IMP] adapt code to new simplified safe_eval API

[KangOl]

safe_eval loose the nocopy option. As we rely on it when evaluating expression, we need to reimplement safe_eval to do the evaluation using the SelfPrintEvalContext.

See odoo/odoo#206846
See odoo/enterprise#83818

[robodoo]

[KangOl]

upgradeci retry with always only hr in all versions

[aj-fuentes]

LGTM. Minimal comment.

[aj-fuentes]

Why don't we assert in both cases that expr is str ot bytes?

If we can assert it in both then I'd suggest we move the shared part to a toplevel safe_eval that then calls _safe_eval_impl with bot expr and context mandatory. Sort of:

if version_gte("saas~18.4"):
    import odoo.tools.safe_eval as _safe_eval_mod

    def _safe_eval_impl(expr, context):
        c = _safe_eval_mod.test_expr(expr, _safe_eval_mod._SAFE_OPCODES, mode="eval", filename=None)
        context["__builtins__"] = dict(_safe_eval_mod._BUILTINS)
        try:
            return _safe_eval_mod.unsafe_eval(c, context, None)
        except _safe_eval_mod._BUBBLEUP_EXCEPTIONS:
            raise
        except Exception as e:
            raise ValueError("{!r} while evaluating\n{!r}".format(e, expr))
        finally:
            del context["__builtins__"]
else:
    try:
        from odoo.tools.safe_eval import safe_eval as _safe_eval_func
    except ImportError:
        from openerp.tools.safe_eval import safe_eval as _safe_eval_func

    def _safe_eval_impl(expr, context):
        return _safe_eval_func(expr, context, nocopy=True)

def safe_eval(expr, context=None):
    if context is None:
        context = SelfPrintEvalContext()

    assert isinstance(expr, (str, bytes))
    assert isinstance(context, SelfPrintEvalContext)
    return _safe_eval_impl(expr, context)

[antonrom1]

Actually compile already checks the type of the source here. It's can either be a string, bytesarray or AST. We already check that the source object is not a codeobject, so I think the assert isinstance(expr, (str, bytes)) is not really needed here

[aj-fuentes]

Potential change of behaviour, LGTM just flagging it it new errors start appearing after the merge.

[aj-fuentes]

I think this is OK to merge. Optional comment for extra checks (isinstance) grouped in one place, the rest looks good to me.

[KangOl]

@robodoo r+