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.

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

Jump to bottom

Include source of applications from current project #685

Merged
merged 1 commit into from Jul 1, 2019
Merged

Include source of applications from current project #685

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 31 additions & 6 deletions lib/distillery/releases/assembler.ex
View file
Open in desktop
Expand Up @@ -136,7 +136,7 @@ defmodule Distillery.Releases.Assembler do

case include_erts? do
true ->
copy_app(app_dir, target_dir, dev_mode?, include_src?)
copy_app(app_name, app_dir, target_dir, dev_mode?, include_src?)

p when is_binary(p) ->
app_dir =
Expand All @@ -146,20 +146,20 @@ defmodule Distillery.Releases.Assembler do
app_dir
end

copy_app(app_dir, target_dir, dev_mode?, include_src?)
copy_app(app_name, app_dir, target_dir, dev_mode?, include_src?)

_ ->
case Utils.is_erts_lib?(app_dir) do
true ->
:ok

false ->
copy_app(app_dir, target_dir, dev_mode?, include_src?)
copy_app(app_name, app_dir, target_dir, dev_mode?, include_src?)
end
end
end

defp copy_app(app_dir, target_dir, true, _include_src?) do
defp copy_app(_app_name, app_dir, target_dir, true, _include_src?) do
case File.ln_s(app_dir, target_dir) do
:ok ->
:ok
Expand All @@ -169,7 +169,7 @@ defmodule Distillery.Releases.Assembler do
end
end

defp copy_app(app_dir, target_dir, false, include_src?) do
defp copy_app(app_name, app_dir, target_dir, false, include_src?) do
case File.mkdir_p(target_dir) do
{:error, reason} ->
{:error, {:assembler, :file, {:copy_app, target_dir, reason}}}
Expand All @@ -184,7 +184,9 @@ defmodule Distillery.Releases.Assembler do
["ebin", "include", "priv"]
end

Path.wildcard(Path.join(app_dir, "*"))
extra_source_dirs = project_app_src_dirs(app_name)

(Path.wildcard(Path.join(app_dir, "*")) ++ extra_source_dirs)
|> Enum.filter(fn p -> Path.basename(p) in valid_dirs end)
|> Enum.each(fn p ->
t = Path.join(target_dir, Path.basename(p))
Expand Down Expand Up @@ -224,6 +226,29 @@ defmodule Distillery.Releases.Assembler do
{:error, err}
end

# Finds sources for `app_name`. This is used for applications built from
# this project, where the source code lies in a different location than the
# compiled files.
defp project_app_src_dirs(app_name) do
case project_app_src_root(app_name) do
nil -> []
path -> Path.wildcard(Path.join(path, "{lib,src}"))
end
end

# Returns a path of the source code directory for an application built from
# this project.
defp project_app_src_root(app_name) do
if not Mix.Project.umbrella?() && Mix.Project.get().project[:app] == app_name do
# This is a top-level application.
File.cwd!()
Copy link
Owner

Choose a reason for hiding this comment

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

This will result in including not only source files, but all of the generated build artifacts, including the contents of the release being copied by the current in-progress assembly. It is really only intended that source code be included (so that one can use that source to modify modules and hot load them in production if needed, or at the very least, be able to read the code of the current deployed version if you don't have another means of finding that information).

If you want to bundle the entire project with the release, then I'd recommend doing that as a plugin, rather than as part of Distillery itself (since Distillery can't anticipate all the possible variations of things that may or may not want to be included).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The source code subdirectories get filtered in project_app_scr_dirs: https://github.com/bitwalker/distillery/pull/685/files#diff-f54ed4fbf4049c971597b8a1e8d04ca1R235.

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, that's fine then. The other problem though is in the other branch: It misses the case where a project is an umbrella. We should find the app directory in the umbrella (using the Mix.Project config option as the base path and joining the app name) and use that as the source path. Unless I'm missing why that case was skipped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, this is not obvious from the code, I should've added a comment. Mix.Project.deps_paths/0 returns a map with all the deps and also all the umbrella apps. I've tried this (showing only the relevant output):

$ mix new myubrella --umbrella
$ cd myubrella/apps
$ mix new myapp1
$ mix new myapp2
$ cd ..
$ mix run --eval 'IO.inspect(Mix.Project.deps_paths())'
%{                                                                                                                    
  myapp1: "/home/hrubi/projects/elixir/distillery-include_src/tmp2/myubrella/apps/myapp1",
  myapp2: "/home/hrubi/projects/elixir/distillery-include_src/tmp2/myubrella/apps/myapp2"
}

Next I've verified that with distillery with this patch and include_src: true, the release indeed includes sources of the umbrella apps configured for the release:

$ tar tf _build/prod/rel/myubrella/releases/0.1.0/myubrella.tar.gz | grep 'myapp.*/lib'
lib/myapp2-0.1.0/lib/myapp2.ex
lib/myapp1-0.1.0/lib/myapp1.ex

Is this what you thought? Should I add the comment at least? Or do you suggest some other approach?

Copy link
Owner

Choose a reason for hiding this comment

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

I would add a comment to the change that explains that - I recall the reasoning behind why that occurs, but it is definitely something I or others may miss again later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added, please check if it's fine.

else
# This returns a directory if the app is either a dependency or an
# application in umbrella. It returns `nil` otherwise.
Map.get(Mix.Project.deps_paths(), app_name)
end
end

# Creates release metadata files
defp write_release_metadata(%Release{name: name} = release) do
resource_path =
Expand Down