[Devel] [PATCH] Backup network config file if it lacks of warning

[Kir Kolyshkin]

On 04/30/2013 05:50 PM, Igor M Podlesny wrote:
> On 1 May 2013 08:28, Kir Kolyshkin <kir at openvz.org> wrote:
>> On 04/30/2013 04:44 PM, Igor Podlesny wrote:
> […]
>>>      setup_network()
>>>    {
>>> +       local dont_edit='WARNING: Do not edit this file'
>>
>> 1 Add an empty line between variables declaration and code
>     ok
>> 2 I suggest using the very first line of comment we are leaving, i.e.
>>
>>
>> # This configuration file is auto-generated.
>>
>> and using it as a whole, so we can grep for the whole line (less probable
>> false positive).
>     Well, I thought about it and now I can suggest using
> "openvz-vzctl:WARNING_DONT_EDIT" pattern.
> This one surely wouldn't make false positives.

I thought about it as well.

Unfortunately we must reuse existing line, so new version of vzctl will
not lead to those .bak files in every debian container.

>>
>>> +       [ -r "$CFGFILE" ] &&
>>> +               head -n20 "$CFGFILE" | fgrep -q -- "$dont_edit" || {
>>
>> 1 Lots of && and || and { makes this code a bit cryptic to read.
>     Well, there're only 2 of them )
>> 2 If we are using the first line we can do head -n1 (again less probable
>> false positive)
>     Hm, I can argue saying that some other don't edit participants
> could also add a line or two, couldn't they?
>> 3 Also, we can remove check for file and do head $CFGFILE 2>/dev/null
>> instead
>     That's not cosy — why muting utility if there's no file at all? I
> disagree on that.


>
>>> +                       # -- It has no our warning inside, so we'd better
>>> +                       # back it up:
>>
>> Moving this comment out of this block can make it one-liner, easier to read
>>
>>
>>> +                       cp "$CFGFILE" \
>>> +                               "${CFGFILE}-$(date
>>> --rfc-3339=seconds).bak"
>>
>> 1 Use cp -a to retain file attributes such as times
>> 2 Do you really want spaces in file name? IMHO "interfaces.2013-04-30
>> 17:20:57-07:00.bak" is an ugly name for a file.
>     Yep, I see no problems with that, but I surely can convert them to
> '_' if you'd like to.

I dislike spaces in such file names because they make it harder to do 
copy-paste with mouse/trackball clicks (word selection)

Converting spaces to undercores will make it yet more complicated. I'd 
rather not to, it's already complicated enough.
>
>> 3 Please make sure this date syntax works in ancient date versions (I
>> suggest checking centos4 or debian 3.1, or both).
>     It was an issue I thought of, yep. Alas, I don't have anything
> ancient enough ATM, couldn't you try it please?

Me neither. But I can point you out to debian 3.1 and centos 4 ostemplates:

http://download.openvz.org/template/precreated/unsupported/

>
>> 4 Maybe it's easier to just use $$ (current shell pid).
>     May be, but precise enough date-time is better.

Why exactly do you need date? I thought you just wanted the file to not 
be lost, and using $$.bak is a simple enough way.

I can extrapolate and say something like renaming this to

"$CFGFILE -- a backup copy generated by a component of openvz vzctl, 
script $0 at $(date --give-me-nanosecond-precision)"

is better.

But sometimes being perfect is being simple.

>>
>>> +               }
>>> +
>>>          echo "# This configuration file is auto-generated.
>>>    #
>>> -# WARNING: Do not edit this file, your changes will be lost.
>>> +# $dont_edit, your changes will be lost.
>>>    # Please create/edit $CFGFILE.head and
>>>    # $CFGFILE.tail instead, their contents will be
>>>    # inserted at the beginning and at the end of this file, respectively.
>>
>> So, taking into account all the above comments,
>> what about something like this (untested):
>>
>> local warn='# This configuration file is auto-generated.'
>>
>> # Make a backup if the file does not look like generated by us
>> if ! head -n1 "$CFGFILE" 2>/dev/null | grep -q -- "^${warn}$"; then
>>      cp -a "$CFGFILE" "$CFGFILE-$$.bak"
>> fi
>>
>> echo "$warn
>> #
>>
>> # WARNING: Do not edit this file, your changes will be lost.
>> ....