1 |
On Sat, 2019-09-21 at 17:10 -0500, William Hubbs wrote: |
2 |
> This eclass adds a src_unpack function that supports the EGO_VENDOR |
3 |
> variable for vendoring modules. |
4 |
> --- |
5 |
> eclass/go-module-vendor.eclass | 133 +++++++++++++++++++++++++++++++++ |
6 |
> 1 file changed, 133 insertions(+) |
7 |
> create mode 100644 eclass/go-module-vendor.eclass |
8 |
> |
9 |
> diff --git a/eclass/go-module-vendor.eclass b/eclass/go-module-vendor.eclass |
10 |
> new file mode 100644 |
11 |
> index 00000000000..af9007df411 |
12 |
> --- /dev/null |
13 |
> +++ b/eclass/go-module-vendor.eclass |
14 |
> @@ -0,0 +1,133 @@ |
15 |
> +# Copyright 2019 gentoo authors |
16 |
> +# Distributed under the terms of the GNU General Public License v2 |
17 |
> + |
18 |
> +# @ECLASS: go-module-vendor.eclass |
19 |
> +# @MAINTAINER: |
20 |
> +# William Hubbs <williamh@g.o> |
21 |
> +# @SUPPORTED_EAPIS: 7 |
22 |
> +# @BLURB: Eclass for building software written in the go |
23 |
> +# programming language that uses go modules and does not vendor. |
24 |
> +# @DESCRIPTION: |
25 |
> +# This eclass provides a src_unpack function which supports vendoring |
26 |
> +# dependencies for software written in the go programming language that |
27 |
> +# uses go modules. |
28 |
> +# |
29 |
> +# You will know the software you are packaging uses modules because |
30 |
> +# it will have files named go.sum and go.mod in its top-level source |
31 |
> +# directory. If it does not have these files, use the golang-* eclasses. |
32 |
> +# |
33 |
> +# If there is also a directory named vendor in the top level source directory |
34 |
> +# of your package, use the golang-module eclass instead of this one. |
35 |
> +# |
36 |
> +# This eclass provides a src_unpack function which unpacks the |
37 |
> +# first tarball mentioned in SRC_URI to the usual location and unpacks |
38 |
> +# any modules mentioned in EGO_VENDOR to ${S}/vendor. |
39 |
> +# |
40 |
> +# Please note that this eclass currently handles only tarballs |
41 |
> +# (.tar.gz), but support for more formats may be added in the future. |
42 |
> +# |
43 |
> +# Since Go programs are statically linked, it is important that your ebuild's |
44 |
> +# LICENSE= setting includes the licenses of all statically linked |
45 |
> +# dependencies. So please make sure it is accurate. |
46 |
> +# |
47 |
> +# @EXAMPLE: |
48 |
> +# |
49 |
> +# @CODE |
50 |
> +# EGO_VENDOR=( |
51 |
> +# "github.com/xenolf/lego 6cac0ea7d8b28c889f709ec7fa92e92b82f490dd" |
52 |
> +# "golang.org/x/crypto 453249f01cfeb54c3d549ddb75ff152ca243f9d8 github.com/golang/crypto" |
53 |
> +# ) |
54 |
> +# |
55 |
> +# inherit go-module-vendor |
56 |
> +# |
57 |
> +# SRC_URI="https://github.com/example/${PN}/archive/v${PV}.tar.gz -> ${P}.tar.gz |
58 |
> +# ${EGO_VENDOR_URI}" |
59 |
> +# @CODE |
60 |
> +# |
61 |
> +# The above example will extract the tarball to ${S} and |
62 |
> +# extract the contents of EGO_VENDOR to ${S}/vendor. |
63 |
> + |
64 |
> +inherit go-module |
65 |
> + |
66 |
> +case ${EAPI:-0} in |
67 |
> + 7) ;; |
68 |
> + *) die "${ECLASS} API in EAPI ${EAPI} not yet established." |
69 |
> +esac |
70 |
> + |
71 |
> +if [[ -z ${_GO_MODULE_VENDOR} ]]; then |
72 |
> + |
73 |
> +_GO_MODULE_VENDOR=1 |
74 |
> + |
75 |
> +EXPORT_FUNCTIONS src_unpack |
76 |
> + |
77 |
> +# @ECLASS-VARIABLE: EGO_VENDOR |
78 |
|
79 |
You want @REQUIRED here. |
80 |
|
81 |
> +# @DESCRIPTION: |
82 |
> +# This variable contains a list of vendored packages. |
83 |
> +# The items of this array are strings that contain the |
84 |
> +# import path and the git commit hash for a vendored package. |
85 |
> +# If the import path does not start with github.com, the third argument |
86 |
> +# can be used to point to a github repository. |
87 |
> + |
88 |
> +declare -arg EGO_VENDOR |
89 |
> + |
90 |
> +_go-module-vendor_set_vendor_uri() { |
91 |
|
92 |
I think you ought to check whether EGO_VENDOR_URI is non-empty here, |
93 |
and error out if it's empty. This will be important in catching people |
94 |
accidentally defining it after 'inherit'. |
95 |
|
96 |
Another thing worth considering is to actually permit defining it |
97 |
anywhere. If you replaced EGO_VENDOR_URI with a function, user would |
98 |
only need to define it prior to SRC_URI, e.g.: |
99 |
|
100 |
inherit go-module-vendor |
101 |
# ... |
102 |
|
103 |
EGO_VENDOR_URI=(...) |
104 |
SRC_URI="... |
105 |
$(ego_vendor_uri)" |
106 |
|
107 |
But it's up to you. I personally feel like very long lists prior to |
108 |
inherit are inconvenient for the reader. |
109 |
|
110 |
> + EGO_VENDOR_URI= |
111 |
> + local lib |
112 |
> + for lib in "${EGO_VENDOR[@]}"; do |
113 |
> + lib=(${lib}) |
114 |
> + if [[ -n ${lib[2]} ]]; then |
115 |
> + EGO_VENDOR_URI+=" https://${lib[2]}/archive/${lib[1]}.tar.gz -> ${lib[2]//\//-}-${lib[1]}.tar.gz" |
116 |
> + else |
117 |
> + EGO_VENDOR_URI+=" https://${lib[0]}/archive/${lib[1]}.tar.gz -> ${lib[0]//\//-}-${lib[1]}.tar.gz" |
118 |
> + fi |
119 |
> + done |
120 |
> + readonly EGO_VENDOR_URI |
121 |
> +} |
122 |
> + |
123 |
> +_go-module-vendor_set_vendor_uri |
124 |
> +unset -f _go-module-vendor_set_vendor_uri |
125 |
> + |
126 |
> +_go-module-vendor_dovendor() { |
127 |
> + local VENDOR_PATH=$1 VENDORPN=$2 TARBALL=$3 |
128 |
|
129 |
Common convention is to use lowercase names for local variables. |
130 |
|
131 |
> + rm -fr "${VENDOR_PATH}/${VENDORPN}" || die |
132 |
> + mkdir -p "${VENDOR_PATH}/${VENDORPN}" || die |
133 |
> + tar -C "${VENDOR_PATH}/${VENDORPN}" -x --strip-components 1\ |
134 |
|
135 |
Add space before `\`. |
136 |
|
137 |
> + -f "${DISTDIR}/${TARBALL}" || die |
138 |
> +} |
139 |
> + |
140 |
> +# @FUNCTION: go-module-vendor_src_unpack |
141 |
> +# @DESCRIPTION: |
142 |
> +# Extract the first archive from ${A} to ${S}, then extract |
143 |
> +# any sources mentioned in ${EGO_VENDOR} to ${S}/vendor. |
144 |
> +go-module-vendor_src_unpack() { |
145 |
> + local lib vendor_path x |
146 |
> + set -- ${A} |
147 |
> + x="$1" |
148 |
> + mkdir -p "${S}" || die |
149 |
> + tar -C "${S}/" -x --strip-components 1 \ |
150 |
> + -f "${DISTDIR}/${x}" || die |
151 |
|
152 |
It's probably a bad idea to hardcode the 'first argument' logic. My |
153 |
suggestion would be to match all ${A} against ${EGO_VENDOR}, and unpack |
154 |
those that aren't present in the latter. |
155 |
|
156 |
> + |
157 |
> + if [[ -d "${S}/vendor" ]]; then |
158 |
> + eerror "Upstream for ${P}.ebuild vendors dependencies." |
159 |
> + die "This ebuild should inherit go-module.eclass" |
160 |
> + fi |
161 |
|
162 |
I'm sorry but all things said, there might be a valid use case to permit |
163 |
this after all -- if packager wants to upgrade vendored dependencies |
164 |
e.g. to fix a security problem. |
165 |
|
166 |
> + |
167 |
> + if [[ -n "${EGO_VENDOR}" ]]; then |
168 |
|
169 |
Empty EGO_VENDOR should probably be an error here. |
170 |
|
171 |
> + vendor_path="${S}/vendor" |
172 |
> + mkdir -p "${vendor_path}" || die |
173 |
> + for lib in "${EGO_VENDOR[@]}"; do |
174 |
> + lib=(${lib}) |
175 |
> + if [[ -n ${lib[2]} ]]; then |
176 |
|
177 |
Sounds like you could simplify that by: |
178 |
|
179 |
[[ -z ${lib[2]} ]] && set -- "${lib[@]}" "${lib[0]}" |
180 |
|
181 |
and then leaving only one branch of the 'if'. You could also use |
182 |
readable variable names instead of [0], [1], [2]. And then, you could |
183 |
inline _go-module_dovendor since it has only one call site now and has |
184 |
all nice variable names already. |
185 |
|
186 |
> + einfo "Vendoring ${lib[0]} ${lib[2]//\//-}-${lib[1]}.tar.gz" |
187 |
> + _go-module_dovendor "${vendor_path}" ${lib[0]} \ |
188 |
> + ${lib[2]//\//-}-${lib[1]}.tar.gz |
189 |
> + else |
190 |
> + einfo "Vendoring ${lib[0]} ${lib[0]//\//-}-${lib[1]}.tar.gz" |
191 |
> + _go-module_dovendor "${vendor_path}" ${lib[0]} \ |
192 |
> + ${lib[0]//\//-}-${lib[1]}.tar.gz |
193 |
> + fi |
194 |
> + done |
195 |
> + fi |
196 |
> +} |
197 |
> + |
198 |
> +fi |
199 |
|
200 |
-- |
201 |
Best regards, |
202 |
Michał Górny |