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

runcommand - allow the runcommand log directory to be changed. #3556

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joolswills
Copy link
Member

If an entry log_dir is made in /opt/retropie/configs/all/runcommand.cfg it will be used by runcommand for saving runcommand.log/runcommand.info files instead of the default /dev/shm

#3554

@twojstaryzdomu
Copy link

twojstaryzdomu commented Jun 16, 2022

Very good but if you want to do it right, you need to:

  1. Get rid of the hard coded /dev/shm. Have a single log/scratch area.
  2. Add a menu item that configures this, similar to the other variables, configurable via the menu.

I completed what you missed on #3557.

If an entry log_dir is made in /opt/retropie/configs/all/runcommand.cfg it will be used by runcommand for saving runcommand.log/runcommand.info files instead of the default /dev/shm

It will also be used for generation of retropie_xinitrc and the generated/appended retroarch.cfg
@joolswills joolswills force-pushed the runcommand_log_dir branch from bc71978 to 4514ad0 Compare June 16, 2022 11:08
@@ -119,6 +118,10 @@ function get_config() {
iniGet "image_delay"
IMAGE_DELAY="$ini_value"
[[ -z "$IMAGE_DELAY" ]] && IMAGE_DELAY=2
iniGet "log_dir"
LOGDIR="$ini_value"
[[ -z "$LOGDIR" ]] && LOGDIR="/dev/shm"

Choose a reason for hiding this comment

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

Please modify [[ -z "$LOGDIR" ]] && LOGDIR="/dev/shm" to [[ -d "$LOGDIR" ]] || LOGDIR="/dev/shm".

If the $LOGDIR is unset or not a directory, default to /dev/shm. Though mind /dev/shm isn't guaranteed to be mounted. /tmp would be the safest bet.

@twojstaryzdomu
Copy link

twojstaryzdomu commented Jul 14, 2022

@joolswills your commit is inconsistent with the other options being controlled via the GUI but not this one.
The commit #3557 added the GUI configuration part for the parameter but regrettably you have chosen to close it without merging.

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.

2 participants