consolidate_protocols seems to be gone from 0.5

filmor commented 3 years ago

In 9122f07239a4344cc3fb0b03769003a2b1ea962a 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 commented 3 years ago

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

@Supersonido ?

icefoxen commented 3 years ago

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 commented 3 years ago

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

Supersonido commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

tsloughter commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Argh, yes, we must cut a new release.

tsloughter commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

tsloughter commented 3 years ago

@Supersonido any update on this?

Supersonido commented 3 years ago

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

tsloughter commented 3 years ago

@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 commented 3 years ago

Awesome, let's create a new release.

joaohf commented 3 years ago

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                             

...

Supersonido / rebar_mix

consolidate_protocols seems to be gone from 0.5 #27