Thursday, September 26, 2019

Re: lang/go: make test vs PORTS_PRIVSEP=Yes

On 19-09-26 16:14:44, Theo Buehler wrote:
> On Thu, Sep 26, 2019 at 11:09:16PM +1000, Joel Sing wrote:
> > On 19-09-26 13:06:51, Stuart Henderson wrote:
> > > On 2019/09/26 09:44, Theo Buehler wrote:
> > > > I noticed two problems with go's 'make test' when PORTS_PRIVSEP=Yes:
> > > > both having to do with _pbuild's home being /nonexistent:
> > > >
> > > > failed to initialize build cache at /nonexistent/.cache/go-build: mkdir /nonexistent: permission denied
> > > >
> > > > and
> > > >
> > > > --- FAIL: TestUserHomeDir (0.00s)
> > > > os_test.go:2350: stat /nonexistent: no such file or directory
> > > > FAIL
> > > >
> > > > both problems can be worked around by setting the GOCACHE and HOME
> > > > environment variables to something different than /nonexistent.
> > > >
> > > > Since do-build alreday sets GOCACHE=${WRKDIST}/go-cache, it seems
> > > > reasonable to do the same for do-test independently of PORTS_PRIVSEP.
> > >
> > > FWIW that one's OK with me.
> > >
> > > > I'm less sure about the proper solution for TestHomeDir and would like
> > > > to have some advice on that. Setting HOME to ${WRKDIST} or a temporary
> > > > directory works and is in line with the custom PATH, but seems very
> > > > kludgy and might have undesirable side effects for other tests in the
> > > > future. One could patch the TestHomeDir test to skip the Stat and IsDir
> > > > part if the user is _pbuild. I'm not too worried about the TestHomeDir
> > > > per se but rather the fact that its failure takes out a sizable number
> > > > of later tests.
> > >
> > > Patching the TestHomeDir test might also have side effects in the future..
> > > Setting PORTHOME=${WRKDIR} is fairly common in ports in general, so that
> > > would probably be my first choice (I think it's better in WRKDIR rather
> > > than WRKDIST).
> >
> > Agreed. It is going to be preferable to set HOME to a sensible
> > location, rather than patch tests - ok jsing@.
>
> Ok, thanks.
>
> Setting PORTHOME=${WRKDIST} works directly. Using WRKDIR does work in
> the sense that all tests pass, but it leads to an ugly complaint about
> a missing GOPATH:
>
> $ make test
> ===> Regression tests for go-1.13
> missing $GOPATH
>
> To avoid this complaint, I added GOPATH=WRKDIR to the environment. Also,
> the reason why the poisoned PORTHOME didn't work is the custom do-test
> target that bypasses the default test environment. After pulling in
> ALL_TEST_ENV, one could now make use of the TEST_ENV mechanism, but I
> don't want to interfere too much with jsing's choices.
>
> Index: Makefile
> ===================================================================
> RCS file: /var/cvs/ports/lang/go/Makefile,v
> retrieving revision 1.71
> diff -u -p -r1.71 Makefile
> --- Makefile 15 Sep 2019 02:00:34 -0000 1.71
> +++ Makefile 26 Sep 2019 14:10:47 -0000
> @@ -34,6 +34,7 @@ SEPARATE_BUILD = simple
> CONFIGURE_STYLE = None
> SUBST_VARS = GOCFG
>
> +PORTHOME = ${WRKDIR}
> WRKDIST = ${WRKDIR}/go
> WRKSRC = ${WRKDIST}/src
>
> @@ -94,8 +95,11 @@ do-build:
> do-test:
> @cd ${WRKSRC} && \
> ulimit -d $$(ulimit -H -d) -n 256 && \
> + ${SETENV} ${ALL_TEST_ENV} \
> CC="${CC}" \
> CXX="${CXX}" \
> + GOCACHE=${WRKDIST}/go-cache \
> + GOPATH=${WRKDIR} \
> PATH=${WRKDIST}/bin:${PATH} \
> /bin/ksh -c "exec 3>&-; exec 4>&-; ./run.bash"

Given this is already a special snowflake, it might be simpler to
just do:

Index: Makefile
===================================================================
RCS file: /cvs/ports/lang/go/Makefile,v
retrieving revision 1.71
diff -u -p -u -p -r1.71 Makefile
--- Makefile 15 Sep 2019 02:00:34 -0000 1.71
+++ Makefile 26 Sep 2019 15:31:29 -0000
@@ -96,6 +96,8 @@ do-test:
ulimit -d $$(ulimit -H -d) -n 256 && \
CC="${CC}" \
CXX="${CXX}" \
+ GOCACHE=${WRKDIST}/go-cache \
+ HOME=${WRKDIR} \
PATH=${WRKDIST}/bin:${PATH} \
/bin/ksh -c "exec 3>&-; exec 4>&-; ./run.bash"

No comments:

Post a Comment