[libvirt PATCH 7/7] ci: integration.sh: Define the SCRATCH_DIR variable for local execution

Martin Kletzander mkletzan at redhat.com
Mon Jan 23 10:42:52 UTC 2023


On Mon, Jan 23, 2023 at 10:50:31AM +0100, Erik Skultety wrote:
>On Mon, Jan 23, 2023 at 09:41:10AM +0100, Martin Kletzander wrote:
>> On Fri, Jan 20, 2023 at 04:06:03PM +0100, Erik Skultety wrote:
>> > Running outside of GitLab will likely not have the variable set and
>> > hence the execution would fail.
>> >
>> > Signed-off-by: Erik Skultety <eskultet at redhat.com>
>> > ---
>> > ci/integration.sh | 8 ++++++++
>> > 1 file changed, 8 insertions(+)
>> >
>> > diff --git a/ci/integration.sh b/ci/integration.sh
>> > index 41326d6e40..ac04c46d8e 100644
>> > --- a/ci/integration.sh
>> > +++ b/ci/integration.sh
>> > @@ -37,6 +37,14 @@ virsh net-start default &>/dev/null || true
>> > # END AS ROOT
>> > exit
>> >
>> > +# If we're running outside of GitLab, this variable will likely not exist, so
>> > +# we need to define it and create the scratch directory
>> > +if [ -z "$SCRATCH_DIR" ]
>> > +then
>> > +    SCRATCH_DIR="/tmp/scratch"
>> > +    mkdir "$SCRATCH_DIR" 2>/dev/null
>>
>> This could fail if someone has this directory already.  Which is a good
>> thing as otherwise it could override some of it.  But wouldn't it be
>> nicer to use mktemp -d and print the result?
>
>Although an option, the main motivation here to remain consistent with how it
>works in GitLab. Since we define SCRATCH_DIR under the 'vars' section, IIRC you
>can only use a scalar value, not a command (if we can, I retract my argument)
>and hence we'd have to export and define the variable under each script,
>before_script, after_script sections.
>

I don't really understand how that affects a change from:

SCRATCH_DIR="/tmp/scratch"
mkdir "$SCRATCH_DIR"

to something like

SCRATCH_DIR=$(mktemp -d)

or possibly

SCRATCH_DIR=$(mktemp -d "/tmp/scratch.XXX")

especially when only used without the SCRATCH_DIR variable.

>So, I preferred consistency here, since I wouldn't  realistically expect an
>engineer to have /tmp/scratch prior to executing script given the main
>motivation here is to quickly get a throwaway test machine to run the script
>and THEN debug if the tests fail. So, while I agree you're right here, I don't
>think it's a likely scenario.
>
>Erik
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230123/cb7be0d4/attachment.sig>


More information about the libvir-list mailing list