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

consolidate_protocols seems to be gone from 0.5 #27

Open
filmor opened this issue Jul 12, 2021 · 20 comments
Open

consolidate_protocols seems to be gone from 0.5 #27

filmor opened this issue Jul 12, 2021 · 20 comments

Comments

@filmor
Copy link
Contributor

filmor commented Jul 12, 2021

In 9122f07 you have removed rebar_mix_hook which provides the documented consolidate_protocols hook. Is that hook not needed anymore or was the removal accidental?

@tsloughter
Copy link
Collaborator

I don't know how it wouldn't be still required.

@Supersonido ?

@icefoxen
Copy link

I think I've run into the same problem, trying to use the plugin results in rebar complaining Unable to run post hooks for 'compile', command 'consolidate_protocols' in namespace 'mix' not found.

@tsloughter
Copy link
Collaborator

If anyone has the time to revert this change and create a PR I can merge it and make a release.

@Supersonido
Copy link
Owner

I removed them because in my test projects they work for me without them, but if it does not work for you I recover them.

@tsloughter
Copy link
Collaborator

@Supersonido the protocols are consolidated when building a release in your test projects? Or the project can run in a shell? I don't see how you get consolidated protocols without this hook.

@Supersonido
Copy link
Owner

It works for me on both. But I repeat, if you want I put them back. I have no problem.

@tsloughter
Copy link
Collaborator

Maybe there is miscommunication where I assume the failure was not getting consolidated protocols when it was instead simply build failure because the hook doesn't exist. If it works then there is no need to add the hook back and it should instead be removed from the readme.

I think I have a test project I can go check with in a minute. But if anyone else on this thread can confirm if they get consolidated protocols then we can simply get the readme updated and let people know to remove the hook.

@icefoxen
Copy link

With just the {plugins, [rebar_mix]}. line added to my rebar.config, if I add {circuits_uart, "1.4.2"} to my deps then rebar3 compile fails with Dependency failure: source for circuits_uart does not contain a recognizable project and can not be built, and if I do {circuits_uart, {elixir, "circuits_uart", "1.4.2"}} it fails with Failed to fetch and copy dep: {elixir,"circuits_uart","1.4.2"}.

Is there an example project anywhere that shows how to do it right?

@ferd
Copy link
Contributor

ferd commented Jul 20, 2021

The first error you get is likely to be bypassed if you use the master version of rebar3. We should cut a release soon.

@tsloughter
Copy link
Collaborator

Argh, yes, we must cut a new release.

@tsloughter
Copy link
Collaborator

@Supersonido can you clarify that protocols are consolidated? I'll open a PR to remove the hook from the readme if this is the case.

@joaohf
Copy link

joaohf commented Aug 5, 2021

Hello,

Using rebar3 from master branch and rebar_mix 0.5.0 I got this:

~/bin/rebar3 compile
===> Verifying dependencies...
===> Analyzing applications...
===> Compiling benchee_erlang
===> Unable to run post hooks for 'compile', command 'consolidate_protocols' in namespace 'mix' not found.

I'm just trying to update the `benchee_erlang_try: https://github.com/joaohf/benchee_erlang_try/tree/rebar_mix

Full output as follows:

~/bin/rebar3 compile
===> Fetching rebar_mix v0.5.0
===> Analyzing applications...
===> Compiling rebar_mix
===> Verifying dependencies...
===> Fetching benchee (from {git,"https://github.com/bencheeorg/benchee",
                   {ref,"07b844e2c61c79a756a94dbdf5fef379c72942f7"}})
===> App elixir is no longer needed and can be deleted.
===> App logger is no longer needed and can be deleted.
===> App mix is no longer needed and can be deleted.
===> Compiling benchee
Resolving Hex dependencies...
Dependency resolution completed:
Unchanged:
  bunt 0.2.0
  certifi 2.5.2
  credo 1.4.0
  deep_merge 1.0.0
  dialyxir 1.0.0
  earmark 1.4.9
  earmark_parser 1.4.9
  erlex 0.2.6
  ex_doc 0.22.1
  ex_guard 1.3.3
  excoveralls 0.13.0
  fs 0.9.2
  hackney 1.16.0
  idna 6.0.1
  inch_ex 2.0.0
  jason 1.2.1
  makeup 1.0.3
  makeup_elixir 0.14.1
  metrics 1.0.1
  mimerl 1.2.0
  nimble_parsec 0.6.0
  parse_trans 3.3.0
  ssl_verify_fun 1.1.6
  statistex 1.0.0
  unicode_util_compat 0.5.0
* Getting deep_merge (Hex package)
* Getting statistex (Hex package)
* Getting ex_guard (Hex package)
* Getting credo (Hex package)
* Getting ex_doc (Hex package)
* Getting earmark (Hex package)
* Getting excoveralls (Hex package)
* Getting inch_ex (Hex package)
* Getting dialyxir (Hex package)
* Getting erlex (Hex package)
* Getting bunt (Hex package)
* Getting jason (Hex package)
* Getting hackney (Hex package)
* Getting certifi (Hex package)
* Getting idna (Hex package)
* Getting metrics (Hex package)
* Getting mimerl (Hex package)
* Getting parse_trans (Hex package)
* Getting ssl_verify_fun (Hex package)
* Getting unicode_util_compat (Hex package)
* Getting earmark_parser (Hex package)
* Getting makeup_elixir (Hex package)
* Getting makeup (Hex package)
* Getting nimble_parsec (Hex package)
* Getting fs (Hex package)
==> deep_merge
Compiling 2 files (.ex)
Generated deep_merge app
==> statistex
Compiling 3 files (.ex)
Generated statistex app
==> benchee
Compiling 42 files (.ex)
Generated benchee app
===> Analyzing applications...
===> Compiling benchee_erlang
===> Unable to run post hooks for 'compile', command 'consolidate_protocols' in namespace 'mix' not found.

@tsloughter
Copy link
Collaborator

Either the hook must be removed from the config or use an older version of rebar_mix until a new release is made with the hook added back.

@tsloughter
Copy link
Collaborator

I brought this issue up at the Build and Packaging WG and Jose says that while it is possible you'll get the right full consolidation without the hook it is a safer bet if we have it.

So it does need to be added back.

@Supersonido
Copy link
Owner

ok, i'll added back after lunch (UTC+2 time)

@tsloughter
Copy link
Collaborator

@Supersonido any update on this?

@Supersonido
Copy link
Owner

It's on develop branch. If someone a part of me could test it would be great

@tsloughter
Copy link
Collaborator

@Supersonido it at least builds a project fine now even with the hook defined (when using rebar3 from git or the nightly release). So it would be good if you could publish a new version and we will get a new release of rebar3 out.

I still need to do more checking on the actual protocol stuff, I thought there used to be a directory in a release generated that had all the consolidated protocol in it and I'm not seeing that, but I may be misremembering.

@Supersonido
Copy link
Owner

Awesome, let's create a new release.

@joaohf
Copy link

joaohf commented Aug 8, 2021

Hello,

Not sure if I need to setup the compile hooks or not. But here is my output when I tried
to run https://github.com/joaohf/benchee_erlang_try/tree/rebar_mix

With the hook script

$ ~/bin/rebar3 clean -a
===> Verifying dependencies...
===> Cleaning out benchee...
===> Cleaning out benchee_erlang...
===> Cleaning out deep_merge...
===> Cleaning out elixir...
===> Cleaning out logger...
===> Cleaning out mix...
===> Cleaning out statistex...
$ ~/bin/rebar3 compile
===> Verifying dependencies...
===> Analyzing applications...
===> Compiling benchee_erlang
Consolidating 5 implementations of protocol Elixir.DeepMerge.Resolver
** (UndefinedFunctionError) function DeepMerge.Resolver.Benchee.Configuration.__impl__/1 is undefined (module DeepMerge.Resolver.Benchee.Configuration is not available)
    DeepMerge.Resolver.Benchee.Configuration.__impl__(:target)
    (elixir 1.12.1) lib/protocol.ex:661: Protocol.each_struct_clause_for/3
    (elixir 1.12.1) lib/protocol.ex:639: anonymous fn/4 in Protocol.change_struct_impl_for/4
    (elixir 1.12.1) lib/enum.ex:2356: Enum."-reduce/3-lists^foldl/2-0-"/3
    (elixir 1.12.1) lib/protocol.ex:639: Protocol.change_struct_impl_for/4
    (elixir 1.12.1) lib/protocol.ex:601: Protocol.change_debug_info/3
    (elixir 1.12.1) lib/protocol.ex:552: Protocol.consolidate/2
    rebar_mix_protocol_consolidation.exs:16: anonymous fn/4 in :elixir_compiler_0.__FILE__/1
===> Unable to run post hooks for 'compile', command 'elixir_script_failed' in namespace 'rebar_mix_hook' not found.

Without hook

$ ~/bin/rebar3 compile                                                                                   
===> Verifying dependencies...                                                                                                                           
===> Analyzing applications...                               
===> Compiling benchee_erlang                                             

$ ~/bin/rebar3 shell                                                                                     
===> Verifying dependencies...                                                                                                                           
===> Analyzing applications...                                                                                                                           
===> Compiling benchee_erlang                                                                                                                            
Erlang/OTP 24 [erts-12.0.2] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [jit]
                                                                            
Eshell V12.0.2  (abort with ^G)                                  
1> benchee:run(#{myFunc => fun() -> lists:sort([8, 2, 3, 4, 2, 1, 3, 4, 9, 10, 11, 12, 13, 20, 1000, -4, -5]) end}, [{warmup, 0}, {time, 2}]).
Not all of your protocols have been consolidated. In order to achieve the
best possible accuracy for benchmarks, please ensure protocol          
consolidation is enabled in your benchmarking environment.                                                                                               
                                                                            
Operating System: Linux                                                  
CPU Information: Intel(R) Core(TM) i7-4600M CPU @ 2.90GHz
Number of Available Cores: 4                              
Available memory: 7.66 GB
Elixir 1.12.1
Erlang 24.0.2                             

...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants