Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

khepri_fun: Work around Rebar's use of cth_readable parse_transform #176

Merged

Conversation

dumbbell
Copy link
Member

@dumbbell dumbbell commented Dec 9, 2022

To improve the output of common_test in the terminal, Rebar uses cth_readable. This library comes with a parse_transform module to convert all calls to ct:pal().

The application modules are thus recompiled on-the-fly (i.e. the result is not written to disk) with this parse_transform. The result is that the module loaded in memory doesn't match the one on disk (different checksums).

This causes the following exception while trying to extract a transaction function. Here is an example:

{khepri_ex, failed_to_prepare_tx_fun,
 #{error =>
   {mismatching_module_checksum,
    #{checksum_from_fun =>
      <<194,208,71,94,61,101,247,31,220,109,126,242,101,120,40,129>>,
      checksum_on_disk =>
      <<152,39,250,105,94,89,145,46,251,4,237,114,45,132,63,33>>,
      compile_info_from_fun =>
      [{version, "8.2.1"},
       {options,
        [{d, 'COMMON_TEST'},
         {d, 'TEST'},
         {i, ".../_build/test/lib/khepri_mnesia_migration/src"},
         {i, ".../_build/test/lib/khepri_mnesia_migration/test"},
         {i, ".../_build/test/lib/khepri_mnesia_migration/include"},
         {i, ".../_build/test/lib/khepri_mnesia_migration"}]},
       {source, ".../src/m2k_table_copy.erl"}],
      compile_info_on_disk =>
      [{version, "8.2.1"},
       {options,
        [debug_info,no_spawn_compiler_process,
         {d, 'COMMON_TEST'},
         warn_export_vars,
         {d, 'TEST'},
         {parse_transform,cth_readable_transform},
         {i, ".../_build/test/lib/khepri_mnesia_migration/src"},
         {i, ".../_build/test/lib/khepri_mnesia_migration/test"},
         {i, ".../_build/test/lib/khepri_mnesia_migration/include"},
         {i, ".../_build/test/lib/khepri_mnesia_migration"}]},
       {source, ".../src/m2k_table_copy.erl"}],
      module => m2k_table_copy}}}}

The solutions consists in comparing the source filename and compiler options. If both copies are compiled for tests and one of them uses cth_readable_transform, we accept the inconsistency and continue.

While here, the error returned when modules mismatch was updated to include more details.

@dumbbell dumbbell added the enhancement New feature or request label Dec 9, 2022
@dumbbell dumbbell added this to the v0.7.0 milestone Dec 9, 2022
@dumbbell dumbbell self-assigned this Dec 9, 2022
@codecov
Copy link

codecov bot commented Dec 9, 2022

Codecov Report

Base: 90.73% // Head: 90.74% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (e0af4b3) compared to base (f1c5730).
Patch coverage: 92.68% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #176      +/-   ##
==========================================
+ Coverage   90.73%   90.74%   +0.01%     
==========================================
  Files          19       19              
  Lines        3388     3427      +39     
==========================================
+ Hits         3074     3110      +36     
- Misses        314      317       +3     
Flag Coverage Δ
erlang-24 89.58% <92.68%> (+0.14%) ⬆️
erlang-25 89.67% <92.68%> (+0.05%) ⬆️
os-ubuntu-latest 90.74% <92.68%> (+0.01%) ⬆️
os-windows-latest 89.61% <92.68%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/khepri_fun.erl 93.62% <92.68%> (+0.20%) ⬆️
src/khepri_machine.erl 94.14% <0.00%> (-0.27%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dumbbell
Copy link
Member Author

dumbbell commented Dec 9, 2022

Thank you @the-mikedavis for the quick review!

I can't reproduce the problem outside of khepri_mnesia_migration. I would prefer to come up with a testcase for this patch. That's why I'm not ready to commit it yet.

@dumbbell dumbbell force-pushed the workaround-rebar-cth_readable-parse-transform-in-khepri_fun branch from 31f5f81 to 97e5208 Compare December 13, 2022 10:02
To improve the output of common_test in the terminal, Rebar uses
`cth_readable`. This library comes with a parse_transform module to
convert all calls to `ct:pal()`.

The application modules are thus recompiled on-the-fly (i.e. the result
is not written to disk) with this parse_transform. The result is that
the module loaded in memory doesn't match the one on disk (different
checksums).

This causes the following exception while trying to extract a
transaction function. Here is an example:

    {khepri_ex, failed_to_prepare_tx_fun,
     #{error =>
       {mismatching_module_checksum,
        #{checksum_from_fun =>
          <<194,208,71,94,61,101,247,31,220,109,126,242,101,120,40,129>>,
          checksum_on_disk =>
          <<152,39,250,105,94,89,145,46,251,4,237,114,45,132,63,33>>,
          compile_info_from_fun =>
          [{version, "8.2.1"},
           {options,
            [{d, 'COMMON_TEST'},
             {d, 'TEST'},
             {i, ".../_build/test/lib/khepri_mnesia_migration/src"},
             {i, ".../_build/test/lib/khepri_mnesia_migration/test"},
             {i, ".../_build/test/lib/khepri_mnesia_migration/include"},
             {i, ".../_build/test/lib/khepri_mnesia_migration"}]},
           {source, ".../src/m2k_table_copy.erl"}],
          compile_info_on_disk =>
          [{version, "8.2.1"},
           {options,
            [debug_info,no_spawn_compiler_process,
             {d, 'COMMON_TEST'},
             warn_export_vars,
             {d, 'TEST'},
             {parse_transform,cth_readable_transform},
             {i, ".../_build/test/lib/khepri_mnesia_migration/src"},
             {i, ".../_build/test/lib/khepri_mnesia_migration/test"},
             {i, ".../_build/test/lib/khepri_mnesia_migration/include"},
             {i, ".../_build/test/lib/khepri_mnesia_migration"}]},
           {source, ".../src/m2k_table_copy.erl"}],
          module => m2k_table_copy}}}}

The solutions consists in comparing the source filename and compiler
options. If both copies are compiled for tests and one of them uses
`cth_readable_transform`, we accept the inconsistency and continue.

While here, the error returned when modules mismatch was updated to
include more details.
…d functions

If the processed function calls another function in the same module but
we don't extract it, we need to transform this instruction into an
external call.

Before this fix, the compiler would fail with the following error:

    {error,
     [{[],
       [{none,compile,
         {crash,beam_asm,function_clause,
          [{beam_asm,encode_arg,
            [{SameModule, Function, Arity},
	    ...
@dumbbell dumbbell force-pushed the workaround-rebar-cth_readable-parse-transform-in-khepri_fun branch from 97e5208 to e0af4b3 Compare December 13, 2022 12:37
@dumbbell dumbbell marked this pull request as ready for review December 13, 2022 12:47
@dumbbell dumbbell merged commit 30c0c66 into main Dec 13, 2022
@dumbbell dumbbell deleted the workaround-rebar-cth_readable-parse-transform-in-khepri_fun branch December 13, 2022 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants