On Mon, Jan 23, 2023 at 12:02:44PM +0000, Daniel P. Berrangé wrote:
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@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.
I think I may have found the answer to ^this - variables can only be static strings, so $PWD/vroot would not be an option in such case. The obvious benefit is that the variables are inherited to all jobs which extend a template, so even if we currently use SCRATCH_DIR in a single YAML file only, I'd like to keep it that way.
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.
Like I mentioned in my previous response, seems like we'd only rename the variable and even had to re-define it in all (.*_)?script sections along with all inherited templates. So, to keep being consistent with other libvirt projects where I believe we still use SCRATCH_DIR in this form. Let's just fix the ramdisk mount issue and move it to /var/tmp - it's a static path, everyone has access there (EUID depends on how the runner was configured), and it's persistent storage Regards, Erik