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

Fixed clean hooks mechanism #20

Merged
merged 1 commit into from
Feb 20, 2024
Merged

Fixed clean hooks mechanism #20

merged 1 commit into from
Feb 20, 2024

Conversation

ferd
Copy link
Collaborator

@ferd ferd commented Feb 20, 2024

As mentioned in erlang/rebar3#2863, fixing the clean hooks appears to have fixed their count by properly assigning them in the top-level app in a non-umbrella project. See how the multi app file has a single CLEAN! hook but the single app one had two for clean while having a single one for compile.

This is because the discovery step we weren't doing right before the PR meant that the hook was assigned both to the project and the individual app. By fixing the detection and overrides, the hook is properly assigned to the app in a single-app project, and to the project root when in an umbrella.

This PR prepares tests to pass once the rebar3 one is merged.

As mentioned in erlang/rebar3#2863, fixing the `clean` hooks appears to have fixed their count by properly assigning them in the top-level app in a non-umbrella project. See how [the multi app file](https:/tsloughter/rebar3_tests/blob/cba67afa851747bcdb5294c9d88c697f0d814b3c/multi_app_hooks/multi_app_hooks.test#L5) has a single `CLEAN!` hook but [the single app one had two for clean](https:/tsloughter/rebar3_tests/blob/cba67afa851747bcdb5294c9d88c697f0d814b3c/single_app_hooks/single_app_hooks.test#L5-L6) while [having a single one for compile](https:/tsloughter/rebar3_tests/blob/cba67afa851747bcdb5294c9d88c697f0d814b3c/single_app_hooks/single_app_hooks.test#L14).

This is because the discovery step we weren't doing right before the PR meant that the hook was assigned both to the project and the individual app. By fixing the detection and overrides, the hook is properly assigned to the app in a single-app project, and to the project root when in an umbrella.

This PR prepares tests to pass once the rebar3 one is merged.
@ferd
Copy link
Collaborator Author

ferd commented Feb 20, 2024

I'm going to merge this based on our docs at https://rebar3.org/docs/configuration/configuration/#provider-hooks:

These hooks are, by default, running for every application, because dependencies may specify their own hook in their own context. The distinction is that in some cases (umbrella apps), hooks can be defined on many levels (omitting overrides):

  • the rebar.config file at the application root
  • each top-level app’s (in apps/ or libs/) rebar.config
  • each dependency’s rebar.config

By default, when there is no umbrella app, the hook defined in the top-level rebar.config is attributed to be part of the top-level app. This allows the hook to keep working for a dependency when the library is later published.

If however the hook is defined in rebar.config at the root of a project with umbrella applications, the hooks will be run before/after the task runs for all of the top-level applications.

To preserve the per-app behaviour in an umbrella project, hooks must instead be defined within each application’s rebar.config.

The hook should never have run twice.

@ferd ferd merged commit c249196 into master Feb 20, 2024
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 this pull request may close these issues.

1 participant