On Mon, Jan 23, 2023 at 01:27:12PM +0000, Daniel P. Berrangé wrote: > On Mon, Jan 23, 2023 at 02:19:23PM +0100, Erik Skultety wrote: > > ... > > > > > > 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. > > > > That would be just a replacement of one var for another and we'd still have to > > keep clearing/removing vroot anyway - one thing I didn't mention, because it > > was irrelevant up until this point, is that in the future we should improve the > > local experience even more by following the logic we have for local container > > envs where we mount the git directory inside the container as a volume. Using > > the same mantra, I can imagine us using e.g. virtiofs to mount the git dir to the > > VM so that the developer can run ninja build immediately without having to wait > > for gitlab and test directly with their own development branch in a safe > > environment. > > > > The difference is that while /tmp would be cleared on VM destroy, vroot's > > content as you propose would remain as a side-effect after destroying the VM > > unless we clear it in which case it's no different to the current SCRATCH_DIR > > solution, so with this in mind I think we should keep it the way we have. > > The /tmp directory is often a ram disk though, hence my aversion to > using /tmp, as you can't be confident it has sufficient space for > without impacting other aspects of the system. With $CWD/vroot you > can expect access to the full extent of the user's storage. Fair enough. > > If we're passing through a host git checkout though, that's sketchy, > unless we did a git clone --reflink, to get a pristine checkout for > each job in an efficient way. --reflink is not at option, did you mean --local which creates hardlinks for the git objects or --reference <repo> that pulls objects from the reference repo? For local containers we're doing the former before mounting them as volumes. Regards, Erik
