Support ERL_ROOTDIR passed as an environment variable in erl and start scripts

[JeromeDeBretagne]

Here is a first step to address ERL-1323 by allowing ROOTDIR passed as an environment variable in both the erl and start commands. I've followed exactly the same approach and shell syntax as already used for RELDIR in start.src

In the context of ERL-1323, this small commit fixes a concrete issue on Android, as it finally allows to run Erlang/OTP on recent OS versions for which it's not possible to know in advance the absolute path of the Erlang runtime anymore. Being able to pass the install location via the ROOTDIR environment variable at runtime solves the issue in this case.

I've tested this approach on the following https://github.com/JeromeDeBretagne/erlanglauncher/ project to finally implement the support for multiple users and I can confirm it works perfectly.

Thanks, Jérôme

EDIT: This proposition has been updated in the end with a follow-up commit to use the ERL_ROOTDIR environment variable, instead of the intial ROOTDIR plan, to address a test case regression. You can find a reference to the follow-up commit at the end of the discussion. I've updated this PR title to mention ERL_ROOTDIR to match the final picture.

[kjellwinblad]

Thanks for the pull-request. I don't see any problem in merging this but I will discuss it with other members of the Ericsson Erlang/OTP team fitst.

[JeromeDeBretagne]

Thanks @kjellwinblad for your feedback

[garazdawi]

The testcase sasl/release_handler_SUITE/target_system has started to fail because of this PR.

[JeromeDeBretagne]

Thanks Lukas for the warning, I'll investigate if this has broken a specific test implementation or if this is the sign of a deeper script expectation that I wasn't aware of.

[garazdawi]

The problem as far as I can figure out is that the variable ROOTDIR is exported to the slave node. So that when the test target system is started which is not supposed to use ROOTDIR it uses the ROOTDIR set for the test system.

The solution I think would be to change the interval variable name within the sh script to not be ROOTDIR.

However, this shows a deeper problem that slave nodes inherit the ROOTDIR variable so that any node created will use the same rootdir as the parent. I'm not sure how to solve that, or if indeed it should be solved.

[JeromeDeBretagne]

Just to be sure, you meant "internal", not "interval", right?

[garazdawi]

Yes.

[JeromeDeBretagne]

Lukas, I've spent some time investigating tonight but I'm a bit stuck.

I've run the mentioned tests with Erlang built at commit cd4ccd6 (one commit from today, so including the discussed change) and another clean one built at f4a8975 (the commit just before 28f2cc2 that is the actual change) and I get exactly the same number of failed tests in the 2 test runs:

Testing tests.sasl_test.release_handler_SUITE: TEST COMPLETE, 19 ok, 5 failed of 24 test cases

In terms of steps, here is what I've done in both scenarios :

$ ./otp_build autoconf
$ ./otp_build configure
$ make -j4
$ export ERL_TOP=`pwd`
$ export PATH=`pwd`/bin:$PATH
$ ./otp_build setup -a
$ ./otp_build tests
$ cd release/tests/test_server/
$ erl
Erlang/OTP 24 [DEVELOPMENT] [erts-11.1.2] [source-f4a89752fc] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [jit]
1> ts:install(). 
[...]
2> ts:run(sasl, release_handler_SUITE, [batch]).

Am I doing something wrong? Or do you suggest to run the tests differently?
Are you getting a different test run outcome on your side when testing Erlang built at f4a8975?

I've double-checked, I'm running the correct erl script though as it doesn't contain the change:

$ grep ROOTDIR `which erl`
ROOTDIR="/REDACTED/Erlang/src/otp_master"
BINDIR=$ROOTDIR/bin/x86_64-unknown-linux-gnu
export ROOTDIR

I can't get a clean test run, and I can't detect (yet) that the failed test was introduced by 28f2cc2 so I'm stuck at the moment in my investigation to try to improve the situation.

Any suggestions would be welcome, thank you.

[garazdawi]

You cannot use the source tree version when testing, you need to use a released Erlang/OTP.

See: https://github.com/erlang/otp/blob/master/HOWTO/TESTING.md#short-version

[JeromeDeBretagne]

You cannot use the source tree version when testing, you need to use a released Erlang/OTP.

Thank you Lukas, putting the corresponding release in the path indeed allows me to run the 2 test scenarios properly.

I don't know if it's me but I've found the testing instructions you've linked (and that I had tried to follow yesterday in fact) confusing, or maybe I'm misreading them:

Short version
-------------
Move to the top directory of the Erlang release you want to test, i.e.
cd /ldisk/work/otp

    export ERL_TOP=`pwd`
	./otp_build setup -a
	export PATH=`pwd`/bin:$PATH
	./otp_build tests
	cd release/tests/test_server
	erl -s ts install -s ts run all_tests -s init stop

Indeed, the above instructions state to move to the top directory of the Erlang sources used for a given release, that's clear. Then, the next instruction is export PATH=pwd/bin:$PATH which is going to put the source tree version of erl at the beginning of the path, which seems to go against your comment to use the released version.

On my side, I've simply fixed the issue with a different export command, the following one:

	export PATH=/path/to/the/corresponding/Erlang/release/matching/the/source/to/test:$PATH

but I am certainly misunderstanding / misinterpreting something in the TESTING.md instructions. Anyway, I believe I should be able to continue my investigation now.

[garazdawi]

Indeed, the above instructions state to move to the top directory of the Erlang sources used for a given release, that's clear. Then, the next instruction is export PATH=pwd/bin:$PATH which is going to put the source tree version of erl at the beginning of the path, which seems to go against your comment to use the released version.

Sorry, you are correct. This is what happens when you assume things. This is what happens when you have the same information in multiple places. The wiki seems to have a better description.

Thank you Lukas, putting the corresponding release in the path indeed allows me to run the 2 test scenarios properly.

Great that you got it working! I think that @kjellwinblad is also doing some work to fix this testcase, you should co-ordinate your efforts.

[kjellwinblad]

Sorry, I have been away so I did not see your discussion until now. I suggest the following fix:

kjellwinblad@8d78247

This will allow the user to change the root dir with the environment variable CUSTOM_ERL_ROOTDIR instead of using ROOTDIR directly.

@JeromeDeBretagne btw, I run into the same issue as you with not running the test in the correct way so this is something we definitely should document better.

[JeromeDeBretagne]

Sorry, you are correct.

btw, I run into the same issue as you with not running the test in the correct way so this is something we definitely should document better.

Thank you both for your confirmation!

I suggest the following fix [...] This will allow the user to change the root dir with the environment variable CUSTOM_ERL_ROOTDIR instead of using ROOTDIR directly.

Your proposed commit looks good to me, I was thinking about exactly the same fix.

Maybe you could switch to the shorter ERL_ROOTDIR instead, it's not used at all in the Erlang/OTP sources currently and it better matches other supported environment variables with the ERL_ prefix. What do you think, @kjellwinblad ?

[kjellwinblad]

Yes, I agree that ERL_ROOTDIR is a better name. I will change.

[JeromeDeBretagne]

Thank you @kjellwinblad for the test case fix that landed yesterday, switching to support the ERL_ROOTDIR environment variable (instead of the initial ROOTDIR proposition).

For reference, the fix was merged in the master branch with commit: 6cb6cd2 and with the actual change in commit: 5475a9e