cleanup / faster builds

[fenollp]

No description provided.

[kostis]

I see a lot of effort being put in this PR which, on the surface, seems (to me, at least) completely pointless. Is it possible to provide some motivation and/or description in what you are trying to achieve? (i.e., what is currently wrong/sub-optimal and why it needs fixing.) Thanks.

[fenollp]

The idea is:

• to pin dependencies to ensure some don't appear by surprise
• to replace compile_flags mechanism with normal rebar/3 options to save superfluous forking/writing to disk
• to allow overriding the RNG at least at compile time and to a value not specially pre-known by proper

At first it was only about the first 2 but progress in #165 brought the 3rd point.

[kostis]

Let me start with two pieces of advice:

1. Add some description to PRs when you open them so that you get early feedback and do not waste your time unnecessarily.
2. Submit PRs that do only one thing at a time. They maximize the chances that they will be accepted.

Regarding the motivation of this PR, let's take the first bullet in your list: "dependencies". The only thing that can be considered a "dependency" of PropEr is sfmt-erlang which

1. is only enabled via a configure flag (which, quite likely, nobody uses)
2. will be dropped anyway once we support only Erlang/OTP releases with rand instead of random, and
3. AFAIK has been stable for a couple of years now (and has never been a problem).

Have you ever experienced any issues with dependencies?

Regarding the second bullet, what exactly is the problem with write_compile_flags? Have you experienced disk problems or slowness in building? AFAIK, PropEr builds in about 2 secs on all machines I have access to (with make -j it is even less). Also, as mentioned in another thread, we will NOT switch to rebar3, at least not for the time being. PropEr comes with its own (patched) rebar which works OK as far as PropEr is concerned. Have you experienced any issues with it?

Regarding the third bullet: Please do that as part of that (#165) or a separate PR, if it's needed.

Please do not misunderstand me. We appreciate contributions. But, like the name of the tool suggests, they have to be PropEr (™).

[fenollp]

Why would sfmt be dropped? Is it a bad RNG?

Pushing an empty lockfile may not be super useful, agreed. My point is that maybe it helps to show/ensure that a prod build of PropEr depends on nothing. And it doesn't hurt.

I don't think you understand that compile_flags things are actually useless: rebar2 and 3 both support platform_define. For sure one could run a lot of code and generate a lot of files to radiate heat and demagnetize SSDs but in the end that's just wasted resources.

I understand you maintain and package your own version of a fork of rebar2.
There are people today that use PropEr (as a dep) with build tools that read rebar.config (e.g. rebar3, probably mix too. Maybe even rebar2!). In these cases your own rebar2 is not used and if assumptions are made there things may go wrong.
This PR does not drop your rebar2, it just allows a sane way to build PropEr with both rebars.

Any idea why Travis has stopped building this PR?
I'll split the ?RNG stuff into another PR.
#165 passes CI. Any comments there?

[TheGeorge]

Consider adding this patch to the PR to make the tests work again (RNG_SEED needs to be defined correctly, otherwise PropEr does not clean it up afterwards and state_is_clean returns false)

diff --git a/include/proper_internal.hrl b/include/proper_internal.hrl
index 9af3a34..bdc41c2 100644
--- a/include/proper_internal.hrl
+++ b/include/proper_internal.hrl
@@ -51,18 +51,20 @@
 
 -ifdef(USE_EXSPLUS).
 -define(RNG, rand).
+-define(SEED_NAME, rand_seed).
 -define(RNG_SET_SEED(Seed), ?RNG:seed(exsplus,Seed)).
 -endif.
 
 -ifdef(USE_SFMT).
 -define(RNG, sfmt).
+-defing(SEED_NAME, sfmt_seed).
 -endif.
 
 -ifdef(USE_RANDOM).
 -define(RNG, random).
+-define(SEED_NAME, random_seed).
 -endif.
 
--define(SEED_NAME, rng_seed).
 -ifndef(USE_EXSPLUS).
 -define(RNG_SET_SEED(Seed), ?RNG:seed(Seed)).
 -endif.

(I'm not sure if this belongs in this PR or #165 but only this one contains the relevant code for setting the RNG)

Also, I think that at some point (very soon) we can drop random altogether.

[TheGeorge]

I think it is worthwhile exploring if updating the build system to, e.g. rebar3 is advantageous. Currently, we use a patched version of rebar2 because this way:

1. It works nicely with the travis build and produces the coverage reports we want
2. It lets us suppress the compile warnings when compiling the tests.
   The reason for patching rebar2 is because it does not let us configure the two previous points in an unpatched version.

Of course that comes with the disadvantage of needing to maintain our own rebar version and packing a compiled rebar executable with PropEr and asking the users to trust that version. I think for travis builds thats fine but it would be nice to be able to use proper with an unpatched build system. If that is possible with rebar3 than I am for switching.

But all the things that work right now (no warnings, coverage for only the relevant modules) must also work with the new build system.

[TheGeorge]

This should probably be ?SEED_NAME

[kostis]

My guess is that this PR does not build anymore due to the Makefile conflicts.

In any case, I suggest we close this one and possibly continue the discussion(s) on new pull requests that address just one issue at a time.

[kostis]

Is there something that (still) can be salvaged from this PR, or should I close it?

[fenollp]

Well, now that you've yourself created & merged #207, #208 and #211 there's indeed nothing to salvage in this PR.
Thanks for finally making these changes.