Add new API to set envar while specifying overwrite

[Yadunund]

Fix #472

[cottsay]

This was discussed in the ROS 2 ticket triage meeting today.

There are concerns about using the POSIX-specific overwrite parameter when we'd need to essentially implement the behavior from scratch on Windows. To avoid the diverging code paths while still implementing the desired API, it was suggested that the function could be written using the mechanisms we're already using elsewhere to reproduce similar behavior to the POSIX overwrite semantics but reliably behave the same on all supported platforms.

[Yadunund]

Thanks @cottsay for the valuable feedback!

it was suggested that the function could be written using the mechanisms we're already using elsewhere to reproduce similar behavior to the POSIX overwrite semantics but reliably behave the same on all supported platforms.

Could someone share a link to such code for my reference?

[clalancette]

Could someone share a link to such code for my reference?

Looking around the ROS 2 codebase, it actually turns out that I don't think we do this anywhere. That is at least partially because setenv itself is very infrequently used.

Nevertheless, to make this operate under the same semantics as the Unix setenv, I imagine we'd do something like:

bool
rcutils_set_env_overwrite(const char * env_name, const char * env_value, bool overwrite)
{
  RCUTILS_CAN_RETURN_WITH_ERROR_OF(false);

  RCUTILS_CHECK_FOR_NULL_WITH_MSG(
    env_name, "env_name is null", return false);

  if (getenv(env_name) != NULL) {
    if (overwrite == 0) {
      return true;
    }
  }

  int set_ret;
#ifdef _WIN32
  if (NULL == env_value) {
    env_value = "";
  }
  set_ret = _putenv_s(env_name, env_value);
#else
  set_ret = setenv(env_name, env_value, (int)overwrite);
#endif

  if (set_ret != 0) {
    RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("setting environment variable failed: %d", errno);
    return false;
  }

  return true;
}

[ahcorde]

@Yadunund any updates here ? what do you think about @clalancette suggestions ?

[Yadunund]

I've applied @clalancette's suggestion to the implementation and @Barry-Xu-2018's recommendation to rename this function to rcutils_set_env2 and deprecate rcutils_set_env.

[Yadunund]

I've updated this PR such that it adds another API called rcutils_set_env_overwrite without deprecating rcutils_set_env. Also included a test.

[ahcorde]

Pulls: ros2/rcutils#473 Gist: https://gist.githubusercontent.com/ahcorde/fc88a94c71e0976f560ad9caed82a442/raw/c532c395841074fe3420ac842f60553b83ec9cbe/ros2.repos BUILD args: --packages-up-to rcutils --packages-above-and-dependencies rcutils TEST args: --packages-select rcutils --packages-above rcutils ROS Distro: rolling Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14565

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[ahcorde]

https://github.com/Mergifyio backport jazzy iron

[mergify[bot]]

backport jazzy iron

✅ Backports have been created

* [#480 Add new API to set envar while specifying overwrite (backport #473)](https://github.com/ros2/rcutils/pull/480) has been created for branch `jazzy` * [#481 Add new API to set envar while specifying overwrite (backport #473)](https://github.com/ros2/rcutils/pull/481) has been created for branch `iron`