Drop shared PLTs support and change PLT name to <OTP-VSN>.plt

[fishcakez]

I am worried about supporting the shared PLT location option because it will be very easy to use a PLT which is not suitable or has slightly different files than a project's dependencies. If there are two branches of an application with different versions of dependencies the same PLT probably should not be used.

I think it would be better to require a user to be specific about how they are sharing PLTs using the explicit directory location. Otherwise someone unfamiliar with dialyzer might think shared is an optimisation and end up using dialyzer incorrectly or in a sub-optimal fashion. That is, we should definitely make the optimisation available for those who need it (with the directory name option) but we should not make it too easy to do the wrong thing.

I also took this opportunity to change to the default PLT name to match that used (currently) by rebar3. It is very awkward to include the application name in the PLT's name because there can be multiple applications in a rebar3 release project. Without the shared option the PLT does not need to include the application name.

[ghost]

Where were you when I asked whether we should remove shared :)? Jokes aside, as I said on the list, I'm okay with removing shared, but I'm yet unconvinced about the filename change. If a user decides to have a ~/.cache/plts/ and put all PLTs there, it makes sense to include the app-name. If it's stored project-local, what's the problem with the old filename?

We can also remove all plt location options and add it later, if there's a feature request. Alternatively we could turn it into a full path option, but that would complicate if and how we embed the OTP release string in the filename.

If we support only project-local PLTs and no option to copy it around (embedded beam paths), I'm okay with dropping the app-name.

[fishcakez]

Where were you when I asked whether we should remove shared :)?

Hehe, I didn't notice this until porting rebar3 to be compatible, sorry.

If it's stored project-local, what's the problem with the old filename?

In rebar3 there is direct support for an apps/ structure to include multiple top level applications for a release. This means the application name can not be used because their could be multiple applications.

Could we add an option plt_name or plt_prefix, which defaults to rebar, so that PLTs are named <plt_name>_<otp_version>_pltor similar? Rebar3 already has several users of a custom directory for the PLT.

[ghost]

What's included from the local projects in the rebar3 PLT? Do you have multiple PLTs for the top-level project? Where does rebar3 store the PLT by default?

[ghost]

The rebar2 PLT model is that you have a single project with rebar deps and what's configured in the .app file. These are included in the PLT. Your apps/ may also be used like deps. You need to properly maintain your .app file.

[fishcakez]

What's included from the local projects in the rebar3 PLT?

Once erlang/rebar3#502 is merged it will include the same files as a rebar2 PLT with the caveat that erts, kernel``stdlib and crypto are always included (by default)

Do you have multiple PLTs for the top-level project? Where does rebar3 store the PLT by default?

The rebar3 PLT is stored in ./_build/<profiles>/ so the default location is ./_build/<profiles>/.

The rebar2 PLT model is that you have a single project with rebar deps and what's configured in the .app file. These are included in the PLT. Your apps/ may also be used like deps. You need to properly maintain your .app file.

rebar3 handles this situation perfectly. Lets stop worrying about what rebar3 does, it actually has tests to prove all these situations work correctly.

[ghost]

Once erlang/rebar3#502 is merged it will include the same files as a rebar2 PLT with the caveat that erts, kernelstdlib and crypto are always included (by default)

Adding kernel, stdlib, and crypto by default may work most of the time, but it's not a good idea, as you can build erlang apps with only erts. Somebody will request a feature to exclude apps from the hard-coded set.

The rebar3 PLT is stored in ./_build// so the default location is ./_build//.

Is it one PLT for each profile, or one for each app in each profile?

rebar3 handles this situation perfectly. Lets stop worrying about what rebar3 does, it actually has tests to prove all these situations work correctly.

I've tried to explain what rebar2's build-plt does, given how rebar2 projects work, trying to figure out whether the models and supported projects are different enough that unifying the naming schemes is pointless.

Also, from what I understand, with rebar3 custom PLT locations, you have to use extra sub-dirs to differentiate between app1's 18.0.plt and app2's 18.0.plt.

[fishcakez]

Adding kernel, stdlib, and crypto by default may work most of the time, but it's not a good idea, as you can build erlang apps with only erts. Somebody will request a feature to exclude apps from the hard-coded set.

Quite the opposite, it means rebar3 can do a lot less work (saving a couple of minutes on build time) and has always been fully configurable.

Is it one PLT for each profile, or one for each app in each profile?

It's one per profile per project. If a project contains multiple applications it is still one per profile.

I've tried to explain what rebar2's build-plt does, given how rebar2 projects work, trying to figure out whether the models and supported projects are different enough that unifying the naming schemes is pointless.

Rebar3 only builds the PLT in a different way, if it does not create the same PLT and analyse the same files it is a bug.

From what I understand, with rebar3 custom PLT locations, you have to use extra sub-dirs to differentiate between app1's 18.0.plt and app2's 18.0.plt.

With a simple rebar3 project (structured the same as rebar projects):

src/module.erl
_build/default/PLT

With a release structured project:

apps/app/src/module.erl
_build/default/PLT

The application name can't be used in the release structured project because there could be multiple applications.

The reason for a unifying naming scheme is so that {plt_location, Dir}, where Dir is a directory, points to the same file.

[fishcakez]

Once erlang/rebar3#502 is merged it will include the same files as a rebar2 PLT with the caveat that erts, kernelstdlib and crypto are always included (by default)

It is possible to fix this caveat if required for compatibility.

[ghost]

and has always been fully configurable

You didn't say so, and I could have assumed it, but it's fine if the default set can be overridden.

The application name can't be used in the release structured project because there could be multiple applications.

That's my point. rebar3 works without a master (aka top) project, and treats all apps equally. This is not the case in rebar2, and hence the possibility of including the app name for simple custom location dirs.

The reason for a unifying naming scheme is so that {plt_location, Dir}, where Dir is a directory, points to the same file.

Same reason the filename is generated in rebar2 and why I called it "location" (not "dir").

I've read your comments twice, but I cannot see an explanation for how plt1 and plt2 are stored in a shared location, without extra sub-dirs.

If not app name (which is the top project name in rebar2), you surely have a project name, don't you?

[ghost]

To not block 2.6.0, perhaps we should reduce the patch to a simple shared removal and discuss naming schemes independently.

[ferd]

Rebar 2.x can totally be made to ignore a top-project by specifying additional directories or running commands with -r. In a way it's like having no top app, and it's a common usage pattern even for releases, though it's unfriendly and usually an uphill battle doing so.

[fishcakez]

I've read your comments twice, but I cannot see an explanation for how plt1 and plt2 are stored in a shared location, without extra sub-dirs.

Ah, I was confused, I thought we were talking about when used in an /apps project! It has not come up as an issue and just use sub-dirs myself.

@ferd can you think of naming scheme that works for both that allows a global directory to contain PLTs from multiple projects?

[ferd]

{ok, Pwd} = file:get_cwd(), %% or use project root dir
integer_to_list(erlang:phash2(Pwd)) ++ "-" ++ OTPVsn ++ ".plt"

Then it becomes invalid if you move directories around I guess. Rebar3 would need to add the profile (or a different directory) to that seed. Not sure I like it.

[fishcakez]

So requiring sub-dirs for global PLTs is not an option for rebar2? If so I will remove the naming scheme change and accept the incompatibly.

[fishcakez]

I have force pushed to only remove shared PLTs. Given that removing the application name from the PLT in rebar could detract from the user experience (have to use sub-dirs to share a PLT) I think its best to leave the name as is.

Rebar3 can switch the PLT name to <prefix>_<otp_version>_plt, where the prefix defaults to a constant value such as rebar3 or otp and add an option plt_prefix, which can be set to the application name if someone wants to share PLTs between rebar and rebar3. Thoughts?

[ghost]

Thanks, can you append your name in THANKS and update rebar.config.sample, all as part the original commit?

[fishcakez]

Amended.

[ghost]

Thanks. Do we actually still need {plt_location, local}, or should we transform it into an optional {plt_dir, "dir"}? I'm okay with 643f045, just putting the question up for final discussion before we merge.

[fishcakez]

local is useful in rebar3 because it allows a profile to "undo" the plt_location set by another (or the default) profile. A commit adding configuration compatibility is available here: fishcakez/rebar3@74f62a6#diff-10e271abea3a3fa058f7fb38aa685fdeR115. The BaseDir on that line includes the profile combination and assuming the default profile is ./_build/default/ but could be ./_build/test+dev/.

[fishcakez]

Oh dear, I've just realised I used dialyzer_opts instead of dialyzer.

[ghost]

Okay, let's keep local. That means, this is ready to merge.

[ghost]

Can we please merge this?