Squash bug that removes /usr/local/bin from PATH when eicrecon-this.sh is sourced

[faustus123]

Briefly, what does this PR introduce?

This unsets the ROOTSYS envar just before sourcing jana-this.sh and thisroot.sh in the eicrecon-this.sh script.

This addresses issue #407 and replaces PR #425

NOTE: A side effect of this fix is that ROOTSYS (and friends) will be set to the long, spack-generated directory name instead of /usr/local. This should not adversely affect things in eic-shell since it is the same ROOT build. It may, however, confuse users who see/expect ROOTSYS to be unaffected by sourcing eicrecon-this.sh

What kind of change does this PR introduce?

• Bug fix (issue #__)
• New feature (issue #__)
• Documentation update
• Other: __

Please check if this PR fulfills the following:

• Tests for the changes have been added
• Documentation has been added / updated
• Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No

Does this PR change default behavior?

No

[faustus123]

This is failing on the clang-tidy test. There is a message:

fatal: bad object bd77dcca1b00ba6363f852a5aad96e5b05977868

This appears to be the hash being passed into git diff in the command:

git diff bd77dcca1b00ba6363f852a5aad96e5b05977868 | clang-tidy-diff -p build -export-fixes clang_tidy_fixes.yml -extra-arg='-std=c++17'

@wdconinc can you give any insight on this?

[wdconinc]

Yeah, clang-tidy needs a fetch-depth of commits in the checkout. I'll file a PR against this branch with the fix

[wdconinc]

The clang-tidy fix included in #437. I'll update this branch when that's merged.

[rahmans1]

@faustus123 Still doesn't quite do what i expect it to do. I followed the steps as listed below

git clone https://github.com/eic/EICrecon
cd EICrecon
cmake -DCMAKE_INSTALL_PREFIX=install -S . -B build
cmake --build build -j 8
cmake --install build
cd install/bin
source eicrecon-this.sh

At this point, I expect to use the custom install instead of the eicrecon from the container when I run:

python run_eicrecon_reco_flags.py .......

However, this still picks up the executible and plugins from the container as shown by the logs.

What i expect to happen is it runs eicrecon from install/bin/eicrecon and loads the plugins from install/lib/EICrecon/plugins under my custom EICrecon directory.

At this point, I found that I need to do:

cd install/bin
export JANA_PLUGIN_PATH=../lib/EICrecon/plugins
python ./run_eicrecon_reco_flags.py .....

However, this still picks up the executible from the container as shown by the logs but at least the plugins and flag values are picked up from the right custom location. My non-elegant patch has been to use an additional environment setting script under install/bin that does

export JANA_PLUGIN_PATH=../lib/EICrecon/plugins
sed -i 's%f"eicrecon"%f"./eicrecon"%g' run_eicrecon_reco_flags.py

before running

 python ./run_eicrecon_reco_flags.py .....

This picks up the plugins, reco flags and executible all from the custom location without touching anything else in the container. May be it's ok to leave this as it is now and make it explicit in documentation that to run custom you need to use ./ prior to the executibles.

[rahmans1]

Approving this for now. Please see the last comment.

[faustus123]

Thank Sakib. I did not get back to trying to fix the run_eicrecon_reco_flags.py script. It is considered deprecated at this point so I'm cautious on how much time I would spend on fixes to that code.

The expectation is that after all of the recent work done to align the C++ defaults with what was put into the reco_flags scripts, one would just run eicrecon without a wrapper script. e.g.

eicrecon inputfile.root

This should give the behavior you're expecting.