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

[Daniel P. Berrangé]

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.
> 
> 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.

I'm wondering why I put it at /tmp/scratch in the first place when we
started using gitlab, and struggling to come up with a justification.

In fact I'm not entirely convinced we really need a SCRATCH_DIR env
variable at all, since AFAICT, we only use it one place for creating
$VROOT.

In terms of standalone reproduction of build env, it would be saner
to use VOORT=$CWD/vroot, which I think would probably work under
GitLab ok too, and do away with SCRATCH_DIR.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|