Re: [LTP] [PATCH] Add simple Containerfile

[Petr Vorel <pvorel@suse.cz>]

Hi Richie,

> >> RFC comments:
> >>     * Add git clean -fdX which should remove any build artifacts
> >>       this is different from the suggestion of just doing a check. I just
> >>       found it easier to remove the build files.
> > FYI what we do in release scripts, is to do a local clone to a different
> > directory [2]:
> > git clone ltp ltp-full-YYYYMMDD

> > Not sure what is faster.

> I guess that is something a script could also do then 'git clean'
> becomes a null op. git clean -X will only remove untracked files so
> pending changes should get picked up. Which is probably what people want
"remove untracked files" - if you develop a new test, forget to add it with 'git
add' and run the container, you will get disappointed :).

> during development. Doing a fresh checkout is probably more like a hard
> reset and clean.

The benefit is that you have not only a clean git repo for the container,
but also not touching your working copy directory. But unless nobody else
raise any concern, I'm ok with your current proposal.

> AFAICT git clean is very quick, far faster than 'make distclean'.


> >>     * Added seperate alpine and tumbleweed runtime scripts. Again it's
> >>       different from the suggestion just because it's easier to add
> >>       seperate scripts than adding a switch
> > +1

> > But maybe put it into container directory, because it's not used in
> > GitHub CI?

> I was thinking it could be used in CI. All we need is a CI that runs VMs
> and we can do some testing. (e.g. srchut).

Makes sense. Also, having scripts on two directories can lead to confusion,
let's keep it in ci directory.

...
> >> +#!/bin/sh -eux
> > nit: out of curiosity, why -u (fail unset variables and parameters)?

> I find it finds errors in shell scripts or when using them. E.g. typo's
> in env variable names. I just include it wherever possible.

+1, maybe we should add it to the current ci scripts as well (+ use params
instead of setting it via set command, it should work in dash and busybox shell
as well).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp