zopencommunity / gitport

The Git version control system
Apache License 2.0
10 stars 5 forks source link

Avoid collision with Rocket environment variables by unsetting GIT_ envars in .appenv #97

Closed IgorTodorovskiIBM closed 1 year ago

IgorTodorovskiIBM commented 1 year ago

GIT_TEMPLATE_DIR and GIT_EXEC_PATH environment variables are currently used because the default location, /usr/share/git-core/templates, does not reflect the real install location of git when installed via zopen.

However, Rocket software tools also depend on this setting and therefore if git is not setup correctly, it will conflict with Rocket tools. As such, we add an unset command to .appenv (which is sourced via .zopen-config).

lbdyck commented 1 year ago

Is this really required or needed? When I run the Rocket Git I have one set of env variables defined and when I run zOpen I have another. I keep them separated as path/manpath/libpath and more are different. I can not fathom having both active at the same time or even how that would work.

IgorTodorovskiIBM commented 1 year ago

Is this really required or needed? When I run the Rocket Git I have one set of env variables defined and when I run zOpen I have another. I keep them separated as path/manpath/libpath and more are different. I can not fathom having both active at the same time or even how that would work.

It has come up in a number of occasions. Many users seem to have knowingly or unknowingly added the GIT_* environment variables to the .profile to get Rocket's git to work, but have forgotten (or are unaware) to unset it.

We were leveraging the same environment variable because it's an easy way to pass along the real path to git's template/exec paths.

IgorTodorovskiIBM commented 1 year ago

And I see that one new test has failed! I will investigate

lbdyck commented 1 year ago

But if they aren't using them anymore why care - why not just ignore them (the variables not the users)

MikeFultonDev commented 1 year ago

Is this really required or needed? When I run the Rocket Git I have one set of env variables defined and when I run zOpen I have another. I keep them separated as path/manpath/libpath and more are different. I can not fathom having both active at the same time or even how that would work.

We are in a tricky spot. We have eliminated the need to require that people set environment variables for git which makes us consistent with other platforms. However rocket git port has not done this (yet) and therefore they hear quite people set these environment variables. We -could- just overwrite the variables but some users may actually want their own templates and so forth (which is what the variables are for).

I am not keen to create more env bars because we won’t work correctly compared to off Z

While it is a bit messier I would say people should properly set up their environment to use the tool. Perhaps our config should Unset the env var and then doe people that actually want their own vars they could still do it but for people with old env vars they would get cleared ?

IgorTodorovskiIBM commented 1 year ago

While it is a bit messier I would say people should properly set up their environment to use the tool. Perhaps our config should Unset the env var and then doe people that actually want their own vars they could still do it but for people with old env vars they would get cleared ?

We can add the unsets to git's .appenv, it would help users who are sourcing the zopen-config, but not those who are calling git directly without sourcing the zopen-config.

MikeFultonDev commented 1 year ago

While it is a bit messier I would say people should properly set up their environment to use the tool. Perhaps our config should Unset the env var and then doe people that actually want their own vars they could still do it but for people with old env vars they would get cleared ?

We can add the unsets to git's .appenv, it would help users who are sourcing the zopen-config, but not those who are calling git directly without sourcing the zopen-config.

Correct. But people are on their own if they don’t use the supplies config. At a minimum they should read it

IgorTodorovskiIBM commented 1 year ago

Since GIT_TEMPLATE_DIR and GIT_EXECPATH have legitimate use cases, including within the tests, I have decided to undo the original commit. I have updated the PR to unset the GIT environment variables that can potentially conflict with Rocket's tools.

IgorTodorovskiIBM commented 1 year ago

/run tests

MikeFultonDev commented 1 year ago

To summarize, to make sure I understand:

Do I have this right? Can we add this to our high level docs if we agree with the ordering of mix/match - we should make this general and not git-specific

lbdyck commented 1 year ago

I'm still unclear on why this has to happen. If I run the Rocket tools then I know I need to configure with their environment and if I run the ZOT tools then use the ZOT environment. And if using the ZOT version of git it will not care or use the Rocket environment variables and vice versa.

What am I missing - seems extra work for not benefit.

IgorTodorovskiIBM commented 1 year ago

To summarize, to make sure I understand:

  • we are adding to the 'configuration' script for our git to 'unset' 2 GIT env vars. If people need their own version of these variables, then they should ensure they 'set' one or both of these GIT env vars after running our configuration script.
  • if someone is mixing and matching Rocket and z/OS Open Tools, we require people to run the Rocket configuration scripts first and subsequently the z/OS Open Tools scripts, otherwise the GIT_ env vars would be set incorrectly.

Do I have this right? Can we add this to our high level docs if we agree with the ordering of mix/match - we should make this general and not git-specific

That is correct. Ordering now matters. That is partly why I initially went with a solution that does not rely on these environment variables, but rather new environment variables. However, that introduces other complications. But this approach is still better than what we have today, where if Rocket's scripts are sourced first, it will break z/OS Open Tools no matter when you source the zopen-config.

I agree, since there is collision in the tools provided between Rocket and zopen, we should mention this in the meta docs. I'll open a PR for that - https://github.com/ZOSOpenTools/meta/pull/583

I'm still unclear on why this has to happen. If I run the Rocket tools then I know I need to configure with their environment and if I run the ZOT tools then use the ZOT environment. And if using the ZOT version of git it will not care or use the Rocket environment variables and vice versa.

What am I missing - seems extra work for not benefit.

From what I've seen, this typically occurs in shared environments containing Rocket and zopen tools and where a common script is sourced. That common script may be altered without the person knowing. Sometimes it's also not clear what needs to be unset if the system has Rocket tools configured, but you want to use a local zopen install.