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

Dependency post_hooks clean override not executed. #2862

Closed
cannadayr opened this issue Feb 17, 2024 · 3 comments · Fixed by #2863
Closed

Dependency post_hooks clean override not executed. #2862

cannadayr opened this issue Feb 17, 2024 · 3 comments · Fixed by #2863

Comments

@cannadayr
Copy link

Pre-Check

I could potentially help fix if somebody points me in the right direction.

Environment

Rebar3 report
 version 3.18.0
 generated at 2024-02-17T00:16:53+00:00
=================
Please submit this along with your issue at https://github.com/erlang/rebar3/issues (and feel free to edit out private information, if any)
-----------------
Task:
Entered as:

-----------------
Operating System: x86_64-unknown-linux-gnu
ERTS: Erlang/OTP 25 [erts-13.2.2] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [jit:ns]
Root Directory: /gnu/store/9nv2p9vi9s0h5yv97mzf1cvwqgcm9nb6-erlang-25.3.2/lib/erlang
Library directory: /gnu/store/9nv2p9vi9s0h5yv97mzf1cvwqgcm9nb6-erlang-25.3.2/lib/erlang/lib
-----------------
Loaded Applications:
bbmustache: 1.12.2
certifi: 2.9.0
cf: 0.3.1
common_test: 1.24
compiler: 8.2.6
crypto: 5.1.4
cth_readable: 1.5.1
dialyzer: 5.0.5
edoc: 1.2
erlware_commons: 1.6.0
eunit: 2.8.2
eunit_formatters: 0.5.0
getopt: 1.0.2
inets: 8.3.1
kernel: 8.5.4
providers: 1.9.0
public_key: 1.13.3
relx: 4.6.0
sasl: 4.2
snmp: 5.13.5
ssl_verify_fun: 1.1.6
stdlib: 4.3.1
syntax_tools: 3.0.1
tools: 3.5.3

-----------------
Escript path: ~/.guix-profile/bin/rebar3
Providers:
  app_discovery as clean compile compile cover ct deps dialyzer do edoc escriptize eunit get-deps help install install_deps list lock new path pkgs release relup report repos shell state tar tree unlock update upgrade upgrade upgrade version xref

Current behaviour

I created two repositories to demonstrate the issue.
https://github.com/cannadayr/rebar_bug_demo and https://github.com/cannadayr/rebar_bug_demo_dep
If you clone rebar_bug_demo, and compile it, the dependency override will create the foo file, however the clean override is not executed, and the artifact remains. For example:

$ DEBUG=1 rebar3 clean -a
===> Expanded command sequence to be run: [app_discovery,install_deps,clean]
===> Running provider: app_discovery
===> Found top-level apps: [rebar_bug_demo]
        using config: [{src_dirs,["src"]},{lib_dirs,["apps/*","lib/*","."]}]
===> Running provider: install_deps
===> Verifying dependencies...
===> Comparing git ref d5e5fb1 with d5e5fb1
===> Running provider: clean
===> Running hooks for clean with configuration:
===>    {pre_hooks, []}.
===> Cleaning out rebar_bug_demo...
===> Running hooks for clean in app rebar_bug_demo (/tmp/bar/rebar_bug_demo/_build/default/lib/rebar_bug_demo) with configuration:
===>    {pre_hooks, []}.
===> Running hooks for clean in app rebar_bug_demo (/tmp/bar/rebar_bug_demo/_build/default/lib/rebar_bug_demo) with configuration:
===>    {post_hooks, []}.
===> Cleaning out rebar_bug_demo_dep...
===> Running hooks for clean in app rebar_bug_demo_dep (/tmp/bar/rebar_bug_demo/_build/default/lib/rebar_bug_demo_dep) with configuration:
===>    {pre_hooks, []}.
===> Running hooks for clean in app rebar_bug_demo_dep (/tmp/bar/rebar_bug_demo/_build/default/lib/rebar_bug_demo_dep) with configuration:
===>    {post_hooks, []}.
===> Running hooks for clean with configuration:
===>    {post_hooks, []}.

Expected behaviour

I would expect the rebar3 clean -a to run the clean override, and potentially remove the artifact on clean. If for some reason the configuration is invalid, then I would expect an error to be thrown when invoking rebar3.

@ferd
Copy link
Collaborator

ferd commented Feb 19, 2024

I've started looking into this, though I have not yet found how this is or is not being triggered.

ferd added a commit to ferd/rebar3 that referenced this issue Feb 19, 2024
The issue appears to be that because the 'clean' operation might be run
while dependencies have not yet been compiled, we applied a partial app
detection mechanism with `rebar_app_disover:find_apps(..., ..., all,
...)`, which worked to parse "invalid" (unbuilt) apps, but also did not
apply overrides.

Instead, we trust the `install_deps` provider dependency by reusing the
apps as they were fully parsed _if_ they were valid, and falling back to
the `rebar_app_discover:find_apps/4` call only to cover the unreadable
ones.

This, it turns out, has the side effect of properly applying hooks when
apps are fully parsed, and fixes erlang#2862
ferd added a commit to ferd/rebar3 that referenced this issue Feb 19, 2024
The issue appears to be that because the 'clean' operation might be run
while dependencies have not yet been compiled, we applied a partial app
detection mechanism with `rebar_app_disover:find_apps(..., ..., all,
...)`, which worked to parse "invalid" (unbuilt) apps, but also did not
apply overrides.

Instead, we trust the `install_deps` provider dependency by reusing the
apps as they were fully parsed _if_ they were valid, and falling back to
the `rebar_app_discover:find_apps/4` call only to cover the unreadable
ones.

This, it turns out, has the side effect of properly applying hooks when
apps are fully parsed, and fixes erlang#2862
@ferd
Copy link
Collaborator

ferd commented Feb 19, 2024

See if #2863 works, if it does for you, I'll merge the fix.

@cannadayr
Copy link
Author

Yup! That fixes it. Thanks so much!!

ferd added a commit to ferd/rebar3 that referenced this issue Feb 20, 2024
The issue appears to be that because the 'clean' operation might be run
while dependencies have not yet been compiled, we applied a partial app
detection mechanism with `rebar_app_disover:find_apps(..., ..., all,
...)`, which worked to parse "invalid" (unbuilt) apps, but also did not
apply overrides.

Instead, we trust the `install_deps` provider dependency by reusing the
apps as they were fully parsed _if_ they were valid, and falling back to
the `rebar_app_discover:find_apps/4` call only to cover the unreadable
ones.

This, it turns out, has the side effect of properly applying hooks when
apps are fully parsed, and fixes erlang#2862

Note that we can only clean paths safely if the discovery steps for the
apps is done with the right profile and options, which may also impact
configurations and hooks.

So rather than duplicating that, we invoke the 'as' provider. This also
opens the door on choosing a different provider (such as 'app_discover'
only) down the road if the -a option isn't given.
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

Successfully merging a pull request may close this issue.

2 participants