Skip to content

Allow configuring logging directory through environment variables#460

Merged
ivanpauno merged 10 commits intoros2:masterfrom
christophebedard:allow-configuring-logging-dir-through-env-vars
Oct 13, 2020
Merged

Allow configuring logging directory through environment variables#460
ivanpauno merged 10 commits intoros2:masterfrom
christophebedard:allow-configuring-logging-dir-through-env-vars

Conversation

@christophebedard
Copy link
Copy Markdown
Member

@christophebedard christophebedard commented Sep 22, 2020

This allows configuring the logging directory through environment variables using the following logic:

  • Use $ROS_LOG_DIR if ROS_LOG_DIR is set and not empty.
  • Otherwise, use $ROS_HOME/log, using ~/.ros for ROS_HOME if not set or if empty.

Includes a test.

Perhaps this logic should be moved to launch_ros in the future.

Relates to ros2/rcl_logging#50

See also ros2/rcl_logging#53

Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
@christophebedard christophebedard force-pushed the allow-configuring-logging-dir-through-env-vars branch from 3eae739 to bfdcc71 Compare September 22, 2020 17:16
@ivanpauno ivanpauno self-requested a review September 29, 2020 14:40
Copy link
Copy Markdown
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
@christophebedard
Copy link
Copy Markdown
Member Author

Switched to ROS_{LOG_DIR,HOME} as discussed here ros2/rcl_logging#53 (comment)

Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
@christophebedard
Copy link
Copy Markdown
Member Author

CI from ros2/rcl_logging#53 including that PR and this one:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@hidmic
Copy link
Copy Markdown

hidmic commented Oct 13, 2020

This one looks ready to go :)

@ivanpauno
Copy link
Copy Markdown
Member

Thanks for the contribution @christophebedard !

@ivanpauno ivanpauno merged commit d648e19 into ros2:master Oct 13, 2020
@christophebedard christophebedard deleted the allow-configuring-logging-dir-through-env-vars branch October 13, 2020 21:30
@tgreier
Copy link
Copy Markdown

tgreier commented Nov 13, 2020

Will this PR be merged into Foxy?

@hidmic
Copy link
Copy Markdown

hidmic commented Nov 13, 2020

Looks like a safe backport. @ivanpauno would you mind?

@ivanpauno
Copy link
Copy Markdown
Member

If we backport this one, we should also backport ros2/rcl_logging#53 for consistency.
I think that both are "ok" to backport, but we don't really need to do so.

@tgreier feel free to open PRs proposing the addition to Foxy.

@jacobperron
Copy link
Copy Markdown
Member

Bumping to Foxy patch release 5, since no one has gotten to this yet.

tgreier added a commit to tgreier/launch that referenced this pull request Dec 21, 2020
Signed-off-by: tgreier <tgreier@moog.com>
tgreier pushed a commit to tgreier/launch that referenced this pull request Dec 22, 2020
…s2#460)

Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
hidmic pushed a commit that referenced this pull request Dec 22, 2020
…) (#477)

Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
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.

5 participants