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

Add :tar option for releases to create a tarball #9290

Merged
merged 4 commits into from
Aug 19, 2019

Conversation

Gazler
Copy link
Contributor

@Gazler Gazler commented Aug 13, 2019

Closes #9289

There are a couple of points worth discussing.

@ericmj mentioned using :systools, even if we want to change that, the tests, docs and validations in this PR should still be valid.

Currently the step is simply called :tar. Not sure if we want to use a different name, or allow users to build the tar in a different location (something like {:tar, "some/path/for/myapp.tar.gz"}

out_path = Path.join(release.path, tar_filename)

dirs =
["bin", "lib", Path.join("releases", release.version), "erts-#{release.erts_version}"] ++
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to use systools:make_tar or duplicate its functionality because simply including lib in the tarball will include the libs used by other versions of the release that may have been built.

@josevalim
Copy link
Member

Not sure if we want to use a different name, or allow users to build the tar in a different location (something like {:tar, "some/path/for/myapp.tar.gz"}

i wouldn't worry about it for now, we can always add it later.

@Gazler can you look into using sys_tools? If that works, we don't need the :clean step.

@tsloughter
Copy link
Contributor

You probably will need to copy the start.boot and such back to relname.boot before calling make_tar. It expects the release name to be the name of those files and mix currently renames them.

@josevalim
Copy link
Member

Does it copy only the relname.boot file though? Because we have more than one... then it may be best to not use sys_tools and add a clean up task.

@Gazler
Copy link
Contributor Author

Gazler commented Aug 13, 2019

Can we walk the runtime dependencies of each application in the struct and only include those directories from within the lib dir?

@tsloughter
Copy link
Contributor

@josevalim yea, it copies that file into the archive as start.boot https://github.com/erlang/otp/blob/master/lib/sasl/src/systools_make.erl#L1770

@Gazler you can, that is what systools does.

@Gazler
Copy link
Contributor Author

Gazler commented Aug 13, 2019

Thanks @tsloughter.

@josevalim What do you suggest? Should we go the systools route? Or are Elixir releases different enough that it'd be valuable to do it outside of systools?

@tsloughter
Copy link
Contributor

Patching systools to allow copying the start.boot would be nice too :). You'd still need the hack to copy back to relname.boot when run on earlier OTPs but it would be a nice change.

@josevalim
Copy link
Member

I am not confident systools would copy everything we need:

  1. We have two .boot files
  2. We have a releases/COOKIE file

So I am afraid we may need to perform a bunch of workarounds and rolling our own should be less lines of code (as we already have all applications and their versions listed in the release struct anyway.

@Gazler Gazler force-pushed the feat/release-tar branch 2 times, most recently from 2c4b4ec to 92e393a Compare August 14, 2019 08:53
Some of the directories inside of lib may be from previous builds. This commit
walks the included apps and ensures that only they are include in the build. The
test explicitly creates an old application and ensures that it is not included
in the build.

All of the files (not directories) inside of the releases dir are also included
in the tar, the specific directory for the version of erts is explicitly added.
@Gazler
Copy link
Contributor Author

Gazler commented Aug 14, 2019

I've pushed the code that limits the directories added for lib and releases as a separate commit so you can see the diff more clearly.

Enum.map(dirs, &{String.to_charlist(&1), String.to_charlist(Path.join(release.path, &1))})

:ok = :erl_tar.create(String.to_charlist(out_path), files, [:dereference, :compressed])
:ok = File.rename(out_path, Path.join(release.version_path, tar_filename))
Copy link
Member

Choose a reason for hiding this comment

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

By using the path I proposed above, we don't need the rename anymore, but we will need to call File.rm before :erl_tar.create.

@tsloughter
Copy link
Contributor

@josevalim relx uses make_tarto make the initial tarball, unpacks it and then copies in stuff it also needs (like overlays that allow you to include any file in a release).

Another thing that would be nice to have added to systools itself :)

@tsloughter
Copy link
Contributor

Something I expect will come up is it doesn't look like you can include other files in the release. It is often useful for the user to be able to bundle any files they want along with the release.

@josevalim
Copy link
Member

@tsloughter agreed. Any suggestions? Should we always include everything else at the root? Or should we allow them to list extra directories?

@josevalim
Copy link
Member

In any case, I would merge this as is and add extra directories support next.

@ericmj ericmj merged commit d44da48 into elixir-lang:master Aug 19, 2019
@ericmj
Copy link
Member

ericmj commented Aug 19, 2019

Thank you @Gazler! 💜

@tsloughter
Copy link
Contributor

@josevalim I suggest requiring an explicit list of additional files/directories to include. Otherwise you risk including shit like logs if they ran the release from the _build dir at some point before tar'ing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add :tar release step
6 participants