On Thu, May 05, 2016 at 12:07:01AM +0200, Dmitrij D. Czarkoff wrote:
> Dmitrij D. Czarkoff said:
> > Stuart Henderson said:
> > > On 2016/04/21 19:35, Christian Weisgerber wrote:
> > >> Dmitrij D. Czarkoff:
> > >>
> > >>>> I think you should just set
> > >>>> INSTALL_STRIP =
> > >>>> in the ports (or modules) that require it and be done with it.
> > >>>> No point in introducing yet another variable.
> > >>>
> > >>> INSTALL_STRIP is mentioned in mk.conf(5), so it is a user setting.
> > >>> /etc/mk.conf may contain INSTALL_STRIP=-s, and that would override
> > >>> INSTALL_STRIP= from port.
> > >>
> > >> In practice, INSTALL_STRIP has two possible values: "-s" and "".
> > >> "-s" is already the default, so the only user setting that makes
> > >> any sense is "". Having some ports already set it to "" is just
> > >> fine.
> > >>
> > >> A few ports already do this.
> > >
> > > Makes sense to me. go-bootstrap needs it too.
> >
> > Well, then asking for OKs for the following patch.
>
> I totally forgot about this. Updated version of the patch follows. It
> touchs directly:
>
> * lang/go
> * lang/go-bootstrap
> * net/syncthing
> * sysutils/terraform
>
> and indirectly every port with MODULES=lang/go. That said, only lang/go
> and lang/go-bootstrap suffer changes in binaries - other ports either
> include own workarounds I remove or use MODGO_INSTALL_TARGET from
> lang/go MODULE, which never had this issue in the first place.
I am OK with this but why don't you use INSTALL_PROGRAM in the
MODGO_INSTALL_TARGET then, instead of cp?
I'd give us more control about the owner/mode no?
> --
> Dmitrij D. Czarkoff
>
> Index: lang/go/Makefile
> ===================================================================
> RCS file: /cvs/ports/lang/go/Makefile,v
> retrieving revision 1.29
> diff -u -p -r1.29 Makefile
> --- lang/go/Makefile 14 Apr 2016 17:43:26 -0000 1.29
> +++ lang/go/Makefile 4 May 2016 21:46:35 -0000
> @@ -8,6 +8,7 @@ VERSION = 1.5.4
> EXTRACT_SUFX = .src.tar.gz
> DISTNAME = go${VERSION}
> PKGNAME = go-${VERSION}
> +REVISION = 0
> CATEGORIES = lang
>
> HOMEPAGE = https://golang.org/
> @@ -29,6 +30,8 @@ SUBST_VARS = GOCFG
>
> WRKDIST = ${WRKDIR}/go
> WRKSRC = ${WRKDIST}/src
> +
> +INSTALL_STRIP =
>
> GOOS = openbsd
> GOARCH = unknown
> Index: lang/go/go.port.mk
> ===================================================================
> RCS file: /cvs/ports/lang/go/go.port.mk,v
> retrieving revision 1.6
> diff -u -p -r1.6 go.port.mk
> --- lang/go/go.port.mk 28 Feb 2016 13:24:16 -0000 1.6
> +++ lang/go/go.port.mk 4 May 2016 21:47:45 -0000
> @@ -53,6 +53,10 @@ MODGO_INSTALL_TARGET = ${INSTALL_DATA_DI
> ${MODGO_PACKAGE_PATH};
> .endif
>
> +# Although this module does not use INSTALL_PROGRAM, unset INSTALL_STRIP to
> +# prevent stripping go programs in ports with custom do-install targets.
> +INSTALL_STRIP =
> +
> MODGO_TEST_TARGET = ${MODGO_TEST_CMD} ${TEST_TARGET}
>
> .if empty(CONFIGURE_STYLE)
> Index: lang/go-bootstrap/Makefile
> ===================================================================
> RCS file: /cvs/ports/lang/go-bootstrap/Makefile,v
> retrieving revision 1.1.1.1
> diff -u -p -r1.1.1.1 Makefile
> --- lang/go-bootstrap/Makefile 4 Dec 2015 17:19:18 -0000 1.1.1.1
> +++ lang/go-bootstrap/Makefile 4 May 2016 21:48:19 -0000
> @@ -8,6 +8,7 @@ VERSION = 1.4.3
> EXTRACT_SUFX = .src.tar.gz
> DISTNAME = go${VERSION}
> PKGNAME = go-bootstrap-${VERSION}
> +REVISION = 0
> CATEGORIES = lang
>
> HOMEPAGE = https://golang.org/
> @@ -31,6 +32,8 @@ SUBST_VARS = GOEXE GOCFG
>
> WRKDIST = ${WRKDIR}/go
> WRKSRC = ${WRKDIST}/src
> +
> +INSTALL_STRIP =
>
> GOOS = openbsd
> GOARCH = unknown
> Index: net/syncthing/Makefile
> ===================================================================
> RCS file: /cvs/ports/net/syncthing/Makefile,v
> retrieving revision 1.2
> diff -u -p -r1.2 Makefile
> --- net/syncthing/Makefile 23 Apr 2016 08:02:05 -0000 1.2
> +++ net/syncthing/Makefile 4 May 2016 21:51:13 -0000
> @@ -34,8 +34,7 @@ do-test:
> cd ${WRKSRC} && ${MODGO_CMD} run build.go test
>
> do-install:
> - # Note: Don't use INSTALL_PROGRAM. It strips, and go hates this.
> - ${INSTALL_SCRIPT} ${WRKSRC}/bin/syncthing ${PREFIX}/bin/
> + ${INSTALL_PROGRAM} ${WRKSRC}/bin/syncthing ${PREFIX}/bin/
> .for sec in 1 5 7
> ${INSTALL_MAN} ${WRKSRC}/man/*.${sec} ${PREFIX}/man/man${sec}/
> .endfor
> Index: sysutils/terraform/Makefile
> ===================================================================
> RCS file: /cvs/ports/sysutils/terraform/Makefile,v
> retrieving revision 1.4
> diff -u -p -r1.4 Makefile
> --- sysutils/terraform/Makefile 3 May 2016 16:16:47 -0000 1.4
> +++ sysutils/terraform/Makefile 4 May 2016 21:53:15 -0000
> @@ -35,9 +35,6 @@ PROVIDERS= atlas aws azure azurerm chef
>
> PROVISIONERS= chef file local-exec remote-exec
>
> -# prevent stripping go binaries
> -INSTALL_STRIP=
> -
> post-build:
> .for provider in ${PROVIDERS}
> cd ${WRKSRC} && \
--
Antoine