Skip to content

Support resources of arbitrary size in rebar3 #2956

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.

Sign up for GitHub

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

Merged
ferd merged 1 commit into from
Jun 27, 2025
+4 −3
Merged

Support resources of arbitrary size in rebar3 #2956

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions apps/rebar/src/rebar_resource_v2.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,16 @@ format_error({no_resource, Source}) ->
is_resource_type(Type, Resources) ->
lists:any(fun(#resource{type=T}) -> T =:= Type end, Resources).

-spec get_resource_type(term(), [resource()]) -> {ok, resource()}.
-spec get_resource_type(tuple(), [resource()]) -> {ok, resource()}.
get_resource_type({Type, Location}, Resources) ->
get_resource(Type, Location, Resources);
get_resource_type({Type, Location, _}, Resources) ->
get_resource(Type, Location, Resources);
get_resource_type({Type, _, _, Location}, Resources) ->
get_resource(Type, Location, Resources);
get_resource_type(Location={Type, _, _, _, _, _}, Resources) ->
get_resource(Type, Location, Resources);
get_resource_type(Source, Resources) when tuple_size(Source) > 3 ->
Type = element(1, Source),
get_resource(Type, Source, Resources);
Copy link
Copy Markdown
Collaborator

@tsloughter tsloughter Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Location and Source aren't the same. In some cases the Location is pulled out of the tuple.

Copy link
Copy Markdown
Contributor Author

@aronisstav aronisstav Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! The reason I originally flattened everything is that a custom resource of size 2-4 can have whatever it likes in its tuple elements, so I thought that being picky about Source and Location is maybe too specific, especially if all this generates is an error message (which may not be the case).

I restored the existing clauses, and merely extended the size 6 case to cover all sizes > 4. The main motivation for the PR is to properly add support a custom resource I am using for some private code, which needs 5 args and previously raised a mere Failed to fetch and copy dep:.

Copy link
Copy Markdown
Collaborator

@ferd ferd Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth still calling the Source variable be Location to keep the old meaning.

Copy link
Copy Markdown
Contributor Author

@aronisstav aronisstav Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought Location is supposed to be just an element of Source and the whole tuple I am matching is therefore a Source. Have I got this wrong?

Copy link
Copy Markdown
Collaborator

@ferd ferd Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually unsure, I was just going from having replaced Location={Type, ...} and argued for keeping naming consistent, but if the prior naming was wrong then the new one is probably fine.

I'm unsure what the original naming meant and which part of the tuple it referred to, and I haven't recently done the context loading again.

Copy link
Copy Markdown
Contributor Author

@aronisstav aronisstav Jun 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understandable. Is there anything more I should do for this?

get_resource_type(Source, _) ->
throw(?PRV_ERROR({no_resource, Source})).

Expand Down
Loading