Re: go-module(5): problematic ALL_TARGET and cmd directory handling
On Thu, Mar 18, 2021 at 09:30:44AM +, Mikolaj Kucharski wrote:
> Not tested, but does below help?
>
> | grep -qe '^main$$' && \
Yes, this works perfectly. Thanks!
Updated diff, and also updated the go-module(5) manpage to reflect
this change.
diff 6eaf30816f4def2f40cb2b765a3aa33ae11ae4a8 /home/jrick/ports
blob - 3c137447b5e134aea6cbb3f9ad5d8dccfd27e234
file + lang/go/go.port.mk
--- lang/go/go.port.mk
+++ lang/go/go.port.mk
@@ -52,11 +52,13 @@ MAKE_ENV += GOCACHE="${MODGO_GOCACHE}"
MODGO_CMD ?= ${SETENV} ${MAKE_ENV} go
MODGO_BUILD_CMD = ${MODGO_CMD} install ${MODGO_FLAGS}
+MODGO_LIST_CMD = ${MODGO_CMD} list ${MODGO_FLAGS}
MODGO_TEST_CMD = ${MODGO_CMD} test ${MODGO_FLAGS} ${MODGO_TEST_FLAGS}
MODGO_BINDIR ?=bin
.if ! empty(MODGO_LDFLAGS)
MODGO_BUILD_CMD += -ldflags="${MODGO_LDFLAGS}"
+MODGO_LIST_CMD += -ldflags="${MODGO_LDFLAGS}"
MODGO_TEST_CMD += -ldflags="${MODGO_LDFLAGS}"
.endif
@@ -153,12 +155,13 @@ do-build:
cd ${WRKSRC} && \
${MODGO_BUILD_TARGET}
.else
+ cd ${WRKSRC} && \
+ ${MODGO_LIST_CMD} -f '{{.Name}}' ${ALL_TARGET} 2>/dev/null \
+ | grep -qe '^main$$' && \
+ ${MODGO_BUILD_CMD} ${ALL_TARGET} ; \
if [ -d ${WRKSRC}/cmd ]; then \
cd ${WRKSRC} && \
- ${MODGO_BUILD_CMD} ${ALL_TARGET}/cmd/... ; \
- else \
- cd ${WRKSRC} && \
- ${MODGO_BUILD_CMD} ${ALL_TARGET} ; \
+ ${MODGO_BUILD_CMD} ./cmd/... ; \
fi;
.endif
. endif
diff 5dfe4c7116fefdfa88d0ee0df89758e96e280278 /usr/src
blob - fa9ad732b02deea54e376413bd61ebec3051898b
file + share/man/man5/go-module.5
--- share/man/man5/go-module.5
+++ share/man/man5/go-module.5
@@ -135,7 +135,7 @@ are appended to port's
Defaults to
.Ar Yes .
.It Ev MODGO_MODNAME
-is the module name as defined in the
+Name of Go module as defined in the
.Pa go.mod
file contained in a project.
If this is set,
@@ -150,9 +150,7 @@ When
.Ev MODGO_MODFILES
is set, and a "cmd" directory is found in
.Ev WRKSRC ,
-"cmd/..." is appended to
-.Ev ALL_TARGET
-in
+"./cmd/..." is also installed by
.Cm do-build
automatically.
.It Ev MODGO_VERSION
Re: go-module(5): problematic ALL_TARGET and cmd directory handling
On Wed, Mar 17, 2021 at 11:56:12PM -0400, Josh Rickmar wrote:
> Here is an ugly hack, but it solves an issue I hit while testing
> portgen(1).
>
> A project I was testing a port for uses a repo structure where the
> project's primary executable package is contained in the root of the
> repo, with some optional and auxiliary executables found under a "cmd"
> directory.
>
> go.port.mk does not work well with this project layout, because the
> do-build target notices the cmd directory exists and will only install
> "${ALL_TARGET}/cmd/...". The port builds, but is missing the
> executable that should be installed by the port. Even if I modify my
> ALL_TARGET, it will still never build the right thing.
>
> The only workaround is to redefine do-build, and I think we can do
> better than that.
>
> The 'go list' command can be used to tell if any packages defined in
> ALL_TARGET are a main package. The patch below will run an additional
> 'go install' command to build ALL_TARGET if this is the case.
>
> The patch also tries to keep the existing "cmd" behavior. In addition
> to building ALL_TARGET if it has main packages, if the cmd directory
> exists, ./cmd/... will also be built. Note that this is no longer
> concatenating ALL_TARGET to 'cmd/...', as this will obviously break if
> ALL_TARGET is multiple words (many packages can be listed together,
> separated by spaces), or ALL_TARGET itself ends in '...'.
>
> Personally, I am not a fan of the "cmd" behavior being done here, and
> would prefer each port Makefile to set ALL_TARGET to ./cmd/... if that
> is what should be built. If this were to change though, then all
> ports relying on this behavior would need updates, and it's not
> immediately obvious which ports would be broken by this change.
>
> On to the patch itself, you will see that I'm grepping the output of
> 'go list' to find if there are any main packages. The search pattern
> used here should be '^main$', but this was causing a variable
> expansion and an unquoted string, so the patch is only using ^main for
> now. I'm definitely open to suggestions on how to better deal with
> this.
>
> diff 6eaf30816f4def2f40cb2b765a3aa33ae11ae4a8 /usr/ports
> blob - 3c137447b5e134aea6cbb3f9ad5d8dccfd27e234
> file + lang/go/go.port.mk
> --- lang/go/go.port.mk
> +++ lang/go/go.port.mk
> @@ -52,11 +52,13 @@ MAKE_ENV += GOCACHE="${MODGO_GOCACHE}"
>
> MODGO_CMD ?= ${SETENV} ${MAKE_ENV} go
> MODGO_BUILD_CMD =${MODGO_CMD} install ${MODGO_FLAGS}
> +MODGO_LIST_CMD = ${MODGO_CMD} list ${MODGO_FLAGS}
> MODGO_TEST_CMD = ${MODGO_CMD} test ${MODGO_FLAGS} ${MODGO_TEST_FLAGS}
> MODGO_BINDIR ?= bin
>
> .if ! empty(MODGO_LDFLAGS)
> MODGO_BUILD_CMD += -ldflags="${MODGO_LDFLAGS}"
> +MODGO_LIST_CMD +=-ldflags="${MODGO_LDFLAGS}"
> MODGO_TEST_CMD +=-ldflags="${MODGO_LDFLAGS}"
> .endif
>
> @@ -153,12 +155,13 @@ do-build:
> cd ${WRKSRC} && \
> ${MODGO_BUILD_TARGET}
> .else
> + cd ${WRKSRC} && \
> + ${MODGO_LIST_CMD} -f '{{.Name}}' ${ALL_TARGET} 2>/dev/null \
> + | grep ^main >/dev/null && \
Not tested, but does below help?
| grep -qe '^main$$' && \
> + ${MODGO_BUILD_CMD} ${ALL_TARGET} ; \
> if [ -d ${WRKSRC}/cmd ]; then \
> cd ${WRKSRC} && \
> - ${MODGO_BUILD_CMD} ${ALL_TARGET}/cmd/... ; \
> - else \
> - cd ${WRKSRC} && \
> - ${MODGO_BUILD_CMD} ${ALL_TARGET} ; \
> + ${MODGO_BUILD_CMD} ./cmd/... ; \
> fi;
> .endif
> . endif
>
--
Regards,
Mikolaj
go-module(5): problematic ALL_TARGET and cmd directory handling
Here is an ugly hack, but it solves an issue I hit while testing
portgen(1).
A project I was testing a port for uses a repo structure where the
project's primary executable package is contained in the root of the
repo, with some optional and auxiliary executables found under a "cmd"
directory.
go.port.mk does not work well with this project layout, because the
do-build target notices the cmd directory exists and will only install
"${ALL_TARGET}/cmd/...". The port builds, but is missing the
executable that should be installed by the port. Even if I modify my
ALL_TARGET, it will still never build the right thing.
The only workaround is to redefine do-build, and I think we can do
better than that.
The 'go list' command can be used to tell if any packages defined in
ALL_TARGET are a main package. The patch below will run an additional
'go install' command to build ALL_TARGET if this is the case.
The patch also tries to keep the existing "cmd" behavior. In addition
to building ALL_TARGET if it has main packages, if the cmd directory
exists, ./cmd/... will also be built. Note that this is no longer
concatenating ALL_TARGET to 'cmd/...', as this will obviously break if
ALL_TARGET is multiple words (many packages can be listed together,
separated by spaces), or ALL_TARGET itself ends in '...'.
Personally, I am not a fan of the "cmd" behavior being done here, and
would prefer each port Makefile to set ALL_TARGET to ./cmd/... if that
is what should be built. If this were to change though, then all
ports relying on this behavior would need updates, and it's not
immediately obvious which ports would be broken by this change.
On to the patch itself, you will see that I'm grepping the output of
'go list' to find if there are any main packages. The search pattern
used here should be '^main$', but this was causing a variable
expansion and an unquoted string, so the patch is only using ^main for
now. I'm definitely open to suggestions on how to better deal with
this.
diff 6eaf30816f4def2f40cb2b765a3aa33ae11ae4a8 /usr/ports
blob - 3c137447b5e134aea6cbb3f9ad5d8dccfd27e234
file + lang/go/go.port.mk
--- lang/go/go.port.mk
+++ lang/go/go.port.mk
@@ -52,11 +52,13 @@ MAKE_ENV += GOCACHE="${MODGO_GOCACHE}"
MODGO_CMD ?= ${SETENV} ${MAKE_ENV} go
MODGO_BUILD_CMD = ${MODGO_CMD} install ${MODGO_FLAGS}
+MODGO_LIST_CMD = ${MODGO_CMD} list ${MODGO_FLAGS}
MODGO_TEST_CMD = ${MODGO_CMD} test ${MODGO_FLAGS} ${MODGO_TEST_FLAGS}
MODGO_BINDIR ?=bin
.if ! empty(MODGO_LDFLAGS)
MODGO_BUILD_CMD += -ldflags="${MODGO_LDFLAGS}"
+MODGO_LIST_CMD += -ldflags="${MODGO_LDFLAGS}"
MODGO_TEST_CMD += -ldflags="${MODGO_LDFLAGS}"
.endif
@@ -153,12 +155,13 @@ do-build:
cd ${WRKSRC} && \
${MODGO_BUILD_TARGET}
.else
+ cd ${WRKSRC} && \
+ ${MODGO_LIST_CMD} -f '{{.Name}}' ${ALL_TARGET} 2>/dev/null \
+ | grep ^main >/dev/null && \
+ ${MODGO_BUILD_CMD} ${ALL_TARGET} ; \
if [ -d ${WRKSRC}/cmd ]; then \
cd ${WRKSRC} && \
- ${MODGO_BUILD_CMD} ${ALL_TARGET}/cmd/... ; \
- else \
- cd ${WRKSRC} && \
- ${MODGO_BUILD_CMD} ${ALL_TARGET} ; \
+ ${MODGO_BUILD_CMD} ./cmd/... ; \
fi;
.endif
. endif
