Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ros_environment #2836

Merged
merged 1 commit into from
May 14, 2024
Merged

Add ros_environment #2836

merged 1 commit into from
May 14, 2024

Conversation

kjeremy
Copy link
Contributor

@kjeremy kjeremy commented May 9, 2024

So that we can get to ROS_DISTRO at build time.

See: lopsided98/nix-ros-overlay#406

@kjeremy kjeremy marked this pull request as ready for review May 9, 2024 19:05
@kjeremy
Copy link
Contributor Author

kjeremy commented May 10, 2024

The build failures seem unrelated.

@sktometometo
Copy link
Contributor

Let me check why this package is necessary with more details so that the maintainer (@k-okada ) can easily understand the situation since he is so busy.

ROS_DISTRO is already set when building the package with catkin on ubuntu. So I think that's why we haven't needed ros_environment package before.
I don't know much about your project. What is the aim of the project and why there is such difference when building this package?

@kjeremy
Copy link
Contributor Author

kjeremy commented May 13, 2024

@sktometometo we are building using https://github.com/lopsided98/nix-ros-overlay on NixOS. The wiki page for ros_environment states that it provides ROS_DISTRO, ROS_VERSION etc.

REP 149 states:

A new ROS package named ros_environment which has minimal dependencies will be available in both ROS versions and providing the new environment variables as well as some of the existing environment variables.

Also see this https://answers.ros.org/question/286969/use-identical-packagexml-for-ros1-and-ros2-pkgs/

@k-okada k-okada merged commit a5f828e into jsk-ros-pkg:master May 14, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants