Re: [PATCH 5/5] travis-ci: don't fail if user already exists on 32 bit Linux build job
On Tue, Jan 23, 2018 at 5:46 PM, Jeff King wrote: > On Mon, Jan 22, 2018 at 02:32:20PM +0100, SZEDER Gábor wrote: >> diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh >> index e37e1d2d5f..13047adde3 100755 >> --- a/ci/run-linux32-build.sh >> +++ b/ci/run-linux32-build.sh >> @@ -33,7 +33,13 @@ then >> CI_USER=root >> else >> CI_USER=ci >> - useradd -u $HOST_UID $CI_USER >> + if test "$(id -u $CI_USER)" = $HOST_UID >> + then >> + : # user already exists with the right ID >> + else >> + useradd -u $HOST_UID $CI_USER >> + fi > > Is it worth redirecting the stderr of "id" to avoid noise when $CI_USER > does not yet exist at all? Or is that a useful log message? :) I think it's worth silencing that error. I'm not even sure it was useful while working on this patch series, but an error message in the middle of the log of a successful build job is surely distracting and might raise some eyebrows (though I suspect that barely anyone looks at logs of successful build jobs). OTOH, I might turn that comment into an echo msg...
Re: [PATCH 5/5] travis-ci: don't fail if user already exists on 32 bit Linux build job
On Tue, Jan 23, 2018 at 11:46 PM, Jeff King wrote: >> The build job on Travis CI always starts with a fresh Docker >> container, so this change doesn't make a difference there. > > I wonder if that is fixable. Installing dependencies into the container > takes quite a lot of time, and is just wasted effort. I agree (because apt-get install phase took forever on my laptop). I've seen people just include a Dockerfile that builds everything in (except things like creating user matching host). First try to fetch it from docker hub (or a local registry but I don't think it applies to us). If not found, rebuild the image with Dockerfile and run a new container with it. Every time somebody changes dockerfile, they are supposed to step the image version up. Of course this is still wasted effort unless somebody pushes images to docker registry and the script has to rebuild the image every single time [1]. Somebody could do that manually. I see docker hub has some automated rebuild feature, perhaps that'll work for us. [1] It still benefits users who run ci scripts manually because the created image will stay in their local machine. -- Duy
Re: [PATCH 5/5] travis-ci: don't fail if user already exists on 32 bit Linux build job
On Mon, Jan 22, 2018 at 02:32:20PM +0100, SZEDER Gábor wrote: > The 32 bit Linux build job runs in a Docker container, which lends > itself to running and debugging locally, too. Especially during > debugging one usually doesn't want to start with a fresh container > every time, to save time spent on installing a bunch of dependencies. > However, that doesn't work quite smootly, because the script running > in the container always creates a new user, which then must be removed > every time before subsequent executions, or the build script fails. > > Make this process more convenient and don't try to create that user if > it already exists and has the right user ID in the container, so > developers don't have to bother with running a 'userdel' each time > before they run the build script. Makes sense. > The build job on Travis CI always starts with a fresh Docker > container, so this change doesn't make a difference there. I wonder if that is fixable. Installing dependencies into the container takes quite a lot of time, and is just wasted effort. > diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh > index e37e1d2d5f..13047adde3 100755 > --- a/ci/run-linux32-build.sh > +++ b/ci/run-linux32-build.sh > @@ -33,7 +33,13 @@ then > CI_USER=root > else > CI_USER=ci > - useradd -u $HOST_UID $CI_USER > + if test "$(id -u $CI_USER)" = $HOST_UID > + then > + : # user already exists with the right ID > + else > + useradd -u $HOST_UID $CI_USER > + fi Is it worth redirecting the stderr of "id" to avoid noise when $CI_USER does not yet exist at all? Or is that a useful log message? :) -Peff
[PATCH 5/5] travis-ci: don't fail if user already exists on 32 bit Linux build job
The 32 bit Linux build job runs in a Docker container, which lends itself to running and debugging locally, too. Especially during debugging one usually doesn't want to start with a fresh container every time, to save time spent on installing a bunch of dependencies. However, that doesn't work quite smootly, because the script running in the container always creates a new user, which then must be removed every time before subsequent executions, or the build script fails. Make this process more convenient and don't try to create that user if it already exists and has the right user ID in the container, so developers don't have to bother with running a 'userdel' each time before they run the build script. The build job on Travis CI always starts with a fresh Docker container, so this change doesn't make a difference there. Signed-off-by: SZEDER Gábor --- ci/run-linux32-build.sh | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh index e37e1d2d5f..13047adde3 100755 --- a/ci/run-linux32-build.sh +++ b/ci/run-linux32-build.sh @@ -33,7 +33,13 @@ then CI_USER=root else CI_USER=ci - useradd -u $HOST_UID $CI_USER + if test "$(id -u $CI_USER)" = $HOST_UID + then + : # user already exists with the right ID + else + useradd -u $HOST_UID $CI_USER + fi + # Due to a bug the test suite was run as root in the past, so # a prove state file created back then is only accessible by # root. Now that bug is fixed, the test suite is run as a -- 2.16.1.80.gc0eec9753d
