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

systemd: fix error thrown by 'sed' in scw-set-hostname #224

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

systemd: fix error thrown by 'sed' in scw-set-hostname #224

wants to merge 1 commit into from

Conversation

mark-kubacki
Copy link

hostnamectl can return more than one hostname, the pair "transient"
and "static" hostname is not uncommon.

The script then fails because $old_hostname is set to two lines (!)
with the first being "n/a". Both get passed to sed and result in an
invalid script both ways, because grep finds it erroneously due to
its special meaning.

Just assume it's not in /etc/hosts

@superseed
Copy link
Member

superseed commented Jun 27, 2019

Whoops, that's what I get for not paying enough attention to manpage contents...

However, now that I looked, I see that we could just set --static on the hostnamectl status and hostnamectl set-hostname commands. This would avoid this situation you're describing and also simplify the grep | cut | tr at the beginning. Does that seem decent to you?

@mark-kubacki
Copy link
Author

Let's do both, because I don't quite trust the output altogether. The check I've originally introduced is cheap enough, I believe.

I'll push a change to this PR later today.

`hostnamectl` can return more than one hostname, the pair "transient"
and "static" hostname is not uncommon.

The script then fails because `$old_hostname` is set to two lines (!)
with the first being "n/a". Both get passed to `sed` and result in an
invalid script both ways, because `grep` finds it erroneously due to
its special meaning.
      In that case a `hostnamectl --static` won't return 'localhost',
but is indeed empty.

Just assume it's not in /etc/hosts in those cases and stick to writing
the new hostname.
@mark-kubacki
Copy link
Author

I am here for you if you have any questions!

@mark-kubacki
Copy link
Author

Any way to move this forward?

@mark-kubacki
Copy link
Author

How can we land this, any changes you'd like to see?

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