The changes to the biggroumscript.sh breaks the tests in python/fixrgraph/musedev/test.
We were testing that the bash script was calling the python script correctly, but now the bash script became "path dependent" (e.g., biggroumsetup/biggroum and /root/biggroumsetup) and without a way to skip the setup process (we cannot test the script on mac anymore for example, because path are hardcoded and the update-alternative command that is always called).
The comment Note: Environment variable to determine run was difficult. docker does not run .bashrc on shell start, sourcing .bashrc in script also failed does not really explain why we switched from environment variables (which are quite easy to set when invoking the script, for local testing) to files, it just motivates a workaround.
Why does sourcing .bashrc file does not work? That seems to break the behavior of bash, while it should not be the case.
Here my main complaints:
The script uses the absolute path /root, while you should use ${HOME} and save everything there (e.g., what does it happen if tomorrow the musedev image change and you run as another user?).
Using the absolute path (e.g. /root/biggroumsetup) further makes the local testing (outside the container) impossible. It would be ok to replace the absolute path with a relative path (e.g. ${HOME}/biggroumsetup_completed).
At this point you could really just save a file with the additional environment variables you set during the setup.
using the relative path biggroumsetup to invoke the python script is another issue for running the test locally (e.g., on a machine where we already have a setup). The lines in the script are:
They were inside the setup first, under the assumption that the container was created fresh at every run.
I would just export those environment variables in the setup and export them in the bashrc (and execute the bashrc every time), so we do not lose them and we can test the script locally.
I would also not change directory before invoking the biggroumscript.py, and I would use an environment variable telling where is the biggroum repository:
The changes to the biggroumscript.sh breaks the tests in python/fixrgraph/musedev/test.
We were testing that the bash script was calling the python script correctly, but now the bash script became "path dependent" (e.g.,
biggroumsetup/biggroumand /root/biggroumsetup) and without a way to skip the setup process (we cannot test the script on mac anymore for example, because path are hardcoded and theupdate-alternativecommand that is always called).The comment
Note: Environment variable to determine run was difficult. docker does not run .bashrc on shell start, sourcing .bashrc in script also faileddoes not really explain why we switched from environment variables (which are quite easy to set when invoking the script, for local testing) to files, it just motivates a workaround.Why does sourcing
.bashrcfile does not work? That seems to break the behavior of bash, while it should not be the case.Here my main complaints:
The script uses the absolute path
/root, while you should use ${HOME} and save everything there (e.g., what does it happen if tomorrow the musedev image change and you run as another user?).Using the absolute path (e.g.
/root/biggroumsetup) further makes the local testing (outside the container) impossible. It would be ok to replace the absolute path with a relative path (e.g. ${HOME}/biggroumsetup_completed). At this point you could really just save a file with the additional environment variables you set during the setup.using the relative path biggroumsetup to invoke the python script is another issue for running the test locally (e.g., on a machine where we already have a setup). The lines in the script are:
FIrst,
cd "$(dirname "${BASH_SOURCE[0]}")"may not work on the musedev deployment unless thebiggroumcheck.shscript is always in the home directory.You have the same issues for the other environment variables you keep setting:
They were inside the setup first, under the assumption that the container was created fresh at every run. I would just export those environment variables in the setup and export them in the bashrc (and execute the bashrc every time), so we do not lose them and we can test the script locally.
I would also not change directory before invoking the
biggroumscript.py, and I would use an environment variable telling where is the biggroum repository:update_alternativecommand in the setup steps (the change is persistent, I think)Originally posted by @smover in https://github.com/cuplv/biggroum/pull/60#issuecomment-558210250