On 08/26/22 14:39, Eric Blake wrote:
> On Fri, Aug 26, 2022 at 01:39:05PM +0200, Laszlo Ersek wrote:
>> Extract and somewhat generalize the recipe for the $(PHYSICAL_MACHINE)
>> target to a separate shell script. In preparation for the multiple steps
>> we're going to introduce later, redirect virt-builder to a temp file at
>> first (placed in the same directory as the finally expected disk image),
>> and rename that file upon success.
>>
>> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
>> ---
>
>> +++ b/make-physical-machine.sh
>> @@ -0,0 +1,37 @@
>> +#!/bin/bash -
>
> See the response in the other thread about not needing the - here.
> Are we sure that /bin/bash is on all systems where this script will be
> run, or is it better as '#!/bin/env bash'?
I had grepped the virt-p2v and virt-v2v shell scripts for shebangs:
1 #!/bin/sh
2 #!/bin/bash
4 #!/usr/bin/env perl
17 #!/bin/bash -
1 #!/bin/bash
1 #!/usr/bin/env python3
1 #!/usr/bin/sh
5 #!/usr/bin/env perl
67 #!/bin/bash -
The thinking seems to have been that
- python3 and perl may "move",
- bash is considered always available.
So I wouldn't break consistency with "#!/bin/bash". I did feel like
breaking consistency with the hyphen "#!/bin/bash -" -- in that,
minimally, I would write "#!/bin/bash --".
Anyway, as a temporary approach, I just stuck with the tradition (the
hyphen), and ended up posting the patch like that. It's not "wrong"
technically, just strange to my taste. I no longer feel strongly about
it (but I'm happy to remove the hyphen if that's desired).
>
>> +
>> +set -e -u -C
>
> My personal opinion is that 'set -e' is a crutch that should be
> avoided because of its unintended interaction with functions;
Can you please elaborate?
POSIX writes, "When a function is executed, it shall have the
syntax-error and variable-assignment properties described for special
built-in utilities in the enumerated list at the beginning of Special
Built-In Utilities", and I've checked that list -- I don't know what you
mean.
> but I'm
> not adamant enough about it to tell people to rip it out of scripts.
> For short scripts, like this one, it's easy enough to check that we
> aren't tripping over any of -e's surprises.
>
>> +
>> +disk=
>> +
>> +cleanup()
>> +{
>> + set +e
>> + if test -n "$disk"; then
>> + rm -f -- "$disk"
>> + disk=
>> + fi
>> +}
>> +
>> +trap cleanup EXIT
>
> Is it intentional that you are not also cleaning up on signals like
> INT and HUP?
Yes, as EXIT covers those.
Easy to test with:
------
#!/bin/bash
cleanup()
{
echo done >zzz
}
trap cleanup EXIT
sleep 100
------
rm -f zzz
./hello.sh
and then hitting either ^C or Alt-F4 in/on the terminal window.
It's for a similar reason I didn't specify ERR -- ERR applies roughly
under the same conditions where "set -e" causes the shell to exit, so
"set -e" causes (in effect) the EXIT trap handler to cover ERR too.
Laszlo
>
>> +
>> +output=$1
>> +outdir=$(dirname -- "$output")
>> +disk=$(mktemp -p "$outdir" physical-machine.tmp.XXXXXXXXXX)
>> +virt-builder --format raw -o "$disk" fedora-35
>> +mv -- "$disk" "$output"
>> +disk=
>
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
