Run post_hooks (sh script) on *nixes only

[big-r81]

Running .\rebar.cmd clean inside the proper dir on Windows will result in the following error:

WARN:  Missing plugins: [covertool]
==> proper (clean)
'.' is not recognized as an internal or external command,
ERROR: Command [clean] failed!
operable program or batch file.

Solution: Run this post hook only on *nixes.

[kostis]

Thanks for your interest in PropEr and for your pull request.

I have zero experience in the Windows workflow, but I want to understand the necessity of this change.

For starters, where/why do need to run .\rebar.cmd clean inside the proper dir? Note that PropEr is (supposed to be) built using make (or make all) and cleaned using make clean which do not call (with a rebar3 clean command) the rebar3 script which is (supposed to be used and) downloaded by the make call.

To help me understand the issue that is being addressed by this PR, perhaps it would help to show the sequence of commands one needs to execute in order to build (and possibly re-build) PropEr on Windows.

[big-r81]

Hi,

this was only an example (we don't call rebar.cmd in proper directly).

We have a Makefile target clean . This will run through all dirs an call "rebar clean". Under Windows, it will fail with the above error message.

[kostis]

We have a Makefile target clean. This will run through all dirs an call "rebar clean". Under Windows, it will fail with the above error message.

Perhaps then the problem is your Makefile making (wrong) assumptions about how its sub-directories are supposed to be cleaned? And you should appropriately modify your Makefile(s) instead to call make clean instead of rebar clean for all sub-directories that provide a Makefile with a clean target?

[big-r81]

Perhaps then the problem is your Makefile making (wrong) assumptions about how its sub-directories are supposed to be cleaned?

The rebar clean is only one step. In your post hook, you call your script clean_doc.sh which is a shell script which is only valid for linux/unixes.

[kostis]

Perhaps then the problem is your Makefile making (wrong) assumptions about how its sub-directories are supposed to be cleaned?

The rebar clean is only one step. In your post hook, you call your script clean_doc.sh which is a shell script which is only valid for linux/unixes.

I will repeat myself: The problem is your Makefile making (wrong) assumptions about how its sub-directories are supposed to be cleaned.

IMO, you should not do that. PropEr is cleaned with make clean, which does indeed call a shell script (which, as you write, is not valid on Windows -- but then again, PropEr never claimed to be supporting Windows!). However, note that it calls a different script (./scripts/clean_temp.sh), not the one you quoted.

The point I am trying to make is that PropEr's clean target does a different thing from what rebar clean does, both nowadays and possibly even tomorrow (for example, it might be extended arbitrarily). It is wrong to assume you can run a rebar clean call inside its directory. If you do that, it's your repository's problem, not PropEr's.

[big-r81]

The problem is your Makefile making (wrong) assumptions about how its sub-directories are supposed to be cleaned.

It's not wrong to call rebar clean, you do it in your (Makefile) target distclean too!

distclean: clean
    $(REBAR3) clean
    $(RM) .plt/proper_plt
    $(RM) -r _build ebin rebar3 rebar.lock

It's not about your Makefile-target clean it's about your post hook in rebar.config. If you call make distclean then rebar clean gets invoked, which will execute your post hook {post_hooks, [{clean, "./scripts/clean_doc.sh"}]}!

It's totally okay that

PropEr never claimed to be supporting Windows!

this PR prevents only, that your post hook is called on Windows, nothing more!

[kostis]

It seems that have we have some trouble communicating... Let me try one last time.

First of all, I wrote:

The problem is your Makefile making (wrong) assumptions about how its sub-directories are supposed to be cleaned.

and you replied:

It's not wrong to call rebar clean, you do it in your (Makefile) target distclean too!

Note that I did not write "It is wrong to call rebar clean", I wrote "it is wrong to assume how sub-directories are supposed to be cleaned" (e.g. using rebar clean). The PropEr(TM) way to clean them (if you really want to - otherwise, what is the point in doing this in the first place?) is to issue a make clean call there, not rebar clean, because theclean Makefile target may be doing something completely different (which in PropEr's case it actually does).

Second, I never wrote "call make distclean", did I? I only wrote "Do not (assume you can) call rebar clean in PropEr's dir". This is not functionality that PropEr supports.

Third, and perhaps most important, what you claim this PR does is inaccurate.

this PR prevents only, that your post hook is called on Windows, nothing more!

Well, it does not do that. In fact, it nowhere mentions Windows. Instead, what it does is it makes the post hook work on a specific, restricted set of OSes, right?

How do we know that this set is the set of "All OSes except Windows" and that with this PR the e.g. make distclean target is not broken on e.g. some other OS that PropEr may be used today? (Or new one that may exist tomorrow.)

Is there a complete set of OSes that rebar3 supports anywhere? (This is a sincere question - a quick Google search but did not find me any answer.)

[big-r81]

Erlang/OTP 25 [erts-13.2] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [jit:ns] [dtrace]

Eshell V13.2  (abort with ^G)
1> f().
ok

😉

Is there a complete set of OSes that rebar3 supports anywhere?

How do we know that this set is the set of "All OSes except Windows" and that with this PR the e.g. make distclean target is not broken on e.g. some other OS that PropEr may be used today? (Or new one that may exist tomorrow.)

I don't find an official list too, I derived it from here.

Let me ask some questions:

1. Why do you have make distclean?
2. Why and when do you call it?

Let's assume we have a dir (/src) with subdirs (mostly erlang apps) like:

.
├── mango
├── meck
├── mem3
├── mochiweb
├── proper
├── rebar
├── rebar3
├── recon

You said:

Perhaps then the problem is your Makefile making (wrong) assumptions about how its sub-directories are supposed to be cleaned? And you should appropriately modify your Makefile(s) instead to call make clean instead of rebar clean for all sub-directories that provide a Makefile with a clean target?

Fine. We would recursively call make clean and all subdirs will be cleaned. Finished. :tada:

Now, I want also to get rid off all Erlang stuff, like beam files, plt's, ebin-dirs, etc. etc. and I will call a make distclean target for all subdirs (Assumption: they all have a distclean target). We are entering all subdirs, all doing their distclean target, all is working. PropER is doing their distclean target, calling rebar3 clean doing it's work and finish with success. :tada::tada::tada:

Now we are making the same steps on Windows and calling make distclean. It works for all directories, except for propER, because propER has a post hook in rebar.config which is calling a shell script, which does not work on Windows - logically.

Now we have multiple possibilities:

1. We can port this script to Windows, and have different post-hooks for different OSes.
2. We could have a rebar.config.script which conditionally creates a rebar.config like

   case os:type() of
   {win32, _} -> 
     {post_hooks, []};
   _ -> 
     {post_hooks, [{clean, "./scripts/clean_doc.sh"}]}
   end.

3. We run the post-hook only on known OSes, which have (POSIX) shells (excluding Windows). (<-- my preference, this PR!)
4. We do nothing and let the error persist, had a pleasant discussion (I really mean that!) - but without agreement and solution.
5. Maybe we’re talking past each other, or I’m not making myself clear... 🤷‍♂️ :wink:

/Ronny

[kostis]

Apologies for the silence and the inactivity for more than a week, I wanted to give the issue some thought and then was swamped with other stuff,

I've submitted #309 which should provide an alternative and removes calling a script in post_hooks. Let me know whether this works for your use-case and also any other comments/thoughts you may have.

PS. I also found our discussion pleasant.

[big-r81]

Hi,

thanks for looking at it again.

I have tested PR #309 on Windows. Now it doesn't stops while running the cleancommand of rebar. 🎉

...
==> mochiweb (clean)
==> recon (clean)
==> proper (clean)
==> couch_epi (clean)
...

PR #309 is definitely a more generic solution and has the same functionality on *nixes and doesn't break the cleanup on Windows. 👍

[kostis]

Thanks for confirming! I will then merge #309 and close this one.