Skip to content
Permalink
Branch: master
Commits on Mar 27, 2020
  1. require passing Python3 CI

    q3aiml authored and tbm committed Mar 25, 2020
Commits on Mar 26, 2020
  1. py3: ensure python output is not lost

    q3aiml authored and tbm committed Mar 26, 2020
    Unbuffer python's stdio to avoid output getting stuck in buffer when
    stdout is not a TTY. Normally buffers are flushed by `Py_Finalize` but
    Boost has a long-standing issue preventing the proper shutdown of the
    interpreter with `Py_Finalize` when embedded [1].
    
    This applies the same fix as 139beba but to any ledger usage rather than
    only the test suite. I removed `PYTHONUNBUFFERED=1` from tests as there
    is no expectation that users should need to have this set for ledger to
    function.
    
    For example without this fix piping ledger into cat usually loses any
    output (unless the output is large enough to cause the buffer to flush):
    
        $ ./ledger -f "test/baseline/feat-value_py3.test" reg
        <class 'bool'> True
        [...]
        $ ./ledger -f "test/baseline/feat-value_py3.test" reg | cat
        $
    
    Interestingly `--verify` causes `python_interpreter_t` to be destroyed
    -- it doesn't appear to be otherwise -- which does call `Py_Finalize`.
    As expected this fixes the issue but can also crash due to the boost
    issue mentioned above:
    
        $ ./ledger -f "test/baseline/feat-value_py3.test" --verify reg
        <class 'bool'> True
        [...]
        Segmentation fault (core dumped)
        $ ./ledger -f "test/baseline/feat-value_py3.test" --verify reg | cat
        <class 'bool'> True
        [...]
        $
    
    1. https://www.boost.org/doc/libs/1_62_0/libs/python/doc/html/tutorial/tutorial/embedding.html
Commits on Mar 25, 2020
  1. py3: fix builtin ledger module for python command

    q3aiml authored and tbm committed Mar 25, 2020
    With python3 the `python` ledger command wound up loading the ledger
    module shared library rather than using the builtin module as intended.
    This resulted in duplicated initialization and crashing on cleanup. It
    was also visible as duplicate converter warnings when importing ledger:
    
        $ ./ledger --no-pager python
        Python 3.7.5 (default, Nov 20 2019, 09:21:52)
        [...]
        >>> import ledger
        /usr/lib/python3.7/importlib/_bootstrap.py:219: RuntimeWarning: to-Python converter for boost::posix_time::ptime already registered; second conversion method ignored.
        [...]
        /usr/lib/python3.7/importlib/_bootstrap.py:219: RuntimeWarning: to-Python converter for ledger::xact_t already registered; second conversion method ignored.
        [...]
        >>> ledger
        <module 'ledger' from '/home/q/src/ledger/ledger.so'>
        >>> exit()
        Segmentation fault (core dumped)
    
    After this change:
    
        Python 3.7.5 (default, Nov 20 2019, 09:21:52)
        [...]
        >>> import ledger
        >>> ledger
        <module 'ledger' (built-in)>
        >>> exit()
        $
    
    Switches to PyImport_AppendInittab from python::detail::init_module
    because 1) that is what the boost docs and examples show and 2)
    init_module appears to be undocumented and not intended for outside use.
    
    Fixes #1867
  2. fix python3 command (argv) wchar_t conversion

    q3aiml authored and tbm committed Mar 25, 2020
    Ensure strings passed to Py_Main have a terminating null character by
    including the extra character allocated for terminating null in the size
    passed to mbstowcs.
    
    Fix argv index so all arguments are not copied to argv[0]. Fixes
    potential buffer overflow due to passing argv[0] as destination with
    argv[i + 1] src and size to mbstowcs.
  3. Revert expected test time value to correct value

    q3aiml authored and tbm committed Mar 25, 2020
    This was changed to be specific to the author's local TZ in 139beba.
    With the test TZ override fixed, revert this back to the value that is
    correct across all environments (including travis).
  4. fix setting TZ in tests

    q3aiml authored and tbm committed Mar 25, 2020
    This was regressed in 139beba which set `PYTHONUNBUFFERED` to fix other
    test issues. When setting multiple environment variables in this way
    they need to be delimited with semicolons rather than spaces. As it is
    `PYTHONUNBUFFERED` is being set to `1 TZ=America/Chicago`. The CMake docs
    are not as clear about this as they probably should be.
    
    This can be verified by throwing together a CTestTestfile.cmake:
    
        add_test(incorrect_env "printenv" "PYTHONUNBUFFERED" "TZ")
        set_tests_properties(incorrect_env PROPERTIES  ENVIRONMENT "PYTHONUNBUFFERED=1 TZ=America/Chicago")
        add_test(correct_env "printenv" "PYTHONUNBUFFERED" "TZ")
        set_tests_properties(correct_env PROPERTIES  ENVIRONMENT "PYTHONUNBUFFERED=1;TZ=America/Chicago")
    
    When run with `ctest -V`:
    
        1: Test command: /usr/bin/printenv "PYTHONUNBUFFERED" "TZ"
        1: Environment variables:
        1:  PYTHONUNBUFFERED=1 TZ=America/Chicago
        1: Test timeout computed to be: 10000000
        1: 1 TZ=America/Chicago
        1/2 Test #1: incorrect_env ....................***Failed    0.00 sec
        test 2
            Start 2: correct_env
    
        2: Test command: /usr/bin/printenv "PYTHONUNBUFFERED" "TZ"
        2: Environment variables:
        2:  PYTHONUNBUFFERED=1
        2:  TZ=America/Chicago
        2: Test timeout computed to be: 10000000
        2: 1
        2: America/Chicago
        2/2 Test #2: correct_env ......................   Passed    0.00 sec
You can’t perform that action at this time.