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

Overwrite /etc/resolv.conf.host if it exists #3103

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

rotu
Copy link
Contributor

@rotu rotu commented Jan 27, 2025

This prevents the install script from crashing if run on an existing blueos machine.

This prevents the install script from crashing if run on an existing blueos machine.
@@ -253,7 +253,7 @@ echo "Restarting random-seeds."
rm -rf /var/lib/systemd/random-seed /loader/random-seed

echo "creating dns link"
sudo ln /etc/resolv.conf /etc/resolv.conf.host
sudo ln --force /etc/resolv.conf /etc/resolv.conf.host
Copy link
Contributor

@dougsland dougsland Jan 28, 2025

Choose a reason for hiding this comment

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

How about create a function to execute such operation and also check if the file exists or not and use --force?

# Function to create a symbolic link with optional --force
create_symlink() {
    local src=$1
    local dst=$2
    local options=""

    # Check if the destination file exists
    if [ -e "$dst" ]; then
        echo "Destination $dst exists. Adding --force option."
        options="--force"
    else
        echo "Destination $dst does not exist. Creating link without --force."
    fi

    # Create the symbolic link
    ln $options "$src" "$dst"

    # Check if the operation was successful
    if [ $? -eq 0 ]; then
        echo "Successfully linked $src to $dst"
    else
        echo "Failed to link $src to $dst"
        return 1
    fi
}

create_symlink "/etc/resolv.conf" "/etc/resolv.conf.host"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand. What benefit does that have over my suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

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

logging and only use force if required but it's just a suggestion.

Copy link
Member

Choose a reason for hiding this comment

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

@dougsland, thanks for the suggestion, but it appears to add unnecessary logic/code for such simple change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. The --force flag here only serves to overwrite the file if it exists, so it’s not like using it only if the file exists is making it at all safer.

some other thoughts:

  1. I’m not totally sure why we create this file on the host system; it seems in the docker image we only need to read the file, and there’s no need to pollute the host filesystem with an extra file. So it’s possible that we shouldn’t be doing this step at all.
  2. If we wanted to change how we move/link files, I would look to the standard install utility which also provides options to set permissions, rename files instead of clobber them, and not modify files if they match. https://linux.die.net/man/1/install

@patrickelectric patrickelectric merged commit 2e8abc5 into bluerobotics:master Jan 28, 2025
6 checks passed
@rotu rotu deleted the patch-10 branch January 28, 2025 17:16
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