-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
hex: handle support files with macro in umbrellas #4213
Conversation
path = Pathname.new(File.join(*support_file_args.compact.reverse)). | ||
cleanpath.to_path | ||
fetch_file_from_host(path).tap { |f| f.support_file = true } | ||
mixfiles = [mixfile] + subapp_mixfiles |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scans mix.exs
files in umbrellas for support files too
mixfiles.flat_map do |mixfile| | ||
mixfile_dir = mixfile.path.sub("/mix.exs", "").delete_prefix("/") | ||
|
||
mixfile.content.gsub(/__DIR__/, "\"#{mixfile_dir}\"").scan(SUPPORT_FILE).map do |support_file_args| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaces the __DIR__
macro with the file dirname, so that in /apps/my_app/mix.exs
Code.eval_file("../../script.exs", __DIR__)
becomes
Code.eval_file("../../script.exs", "/apps/my_app")
I haven't got any comments here, so wondering if this contribution is something that the maintainers think it's worth adding. I had an issue with a client's repo that was using something like this and I've changed it not eval the file when running on dependabot, but I think it would be cleaner if dependabot handled it. |
Yep it definitely looks useful to me, thanks @nirev In some ecosystems we've started just performing a (shallow) clone vs this custom file fetching logic, and I contemplated suggesting that, but I think it might end up being a larger change that's a bit more involved to roll out, so this seems like a good fix to me 👍 |
Thank you, @jurre! |
This PR adds support for handling required files when a
__DIR__
macro is used.It also handles required files in umbrellas
The test case added extends the umbrella context with the following change:
apps/bank_web/mix.exs
docs for
__DIR__
: https://hexdocs.pm/elixir/Kernel.SpecialForms.html#__DIR__/0