Page MenuHomeFreeBSD

New port: www/go-appengine-sdk: App Engine SDK for Go

Authored by yuri on Feb 11 2018, 3:12 PM.

Diff Detail

rP FreeBSD ports repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline created this revision.Feb 11 2018, 3:12 PM
mat added inline comments.Feb 12 2018, 2:09 PM
44–50 ↗(On Diff #39181)
GOARCH=  ${ARCH:S/i386/386}
52–54 ↗(On Diff #39181)

GO387_VARS= GO386=387 added inline comments.Feb 12 2018, 2:14 PM
44–50 ↗(On Diff #39181)

Thanks. I just realised that there's no need for IGNORE because of ONLY_FOR_ARCHS above.

Simplify GOARCH and GO386 options. marked 2 inline comments as done.Feb 12 2018, 3:34 PM
sunpoet added inline comments.
7 ↗(On Diff #39207)

Use DISTNAME and USES=zip.

12–13 ↗(On Diff #39207)

Please update LICENSE and add LICENSE_FILE.

Since you did not install the whole zip file, you do not need to list all licenses mentioned in ${WRKSRC}/LICENSE.
Please set LICENSE to the licenses of installed items.

mat added inline comments.Feb 12 2018, 6:57 PM
73 ↗(On Diff #39207)

I am not sure what is being CP'ed here, but never use cp to install files, use COPYTREE_SHARE/BIN or one of the INSTALL_* macros. marked 2 inline comments as done.

Address review feedback. added inline comments.Feb 12 2018, 9:55 PM
12–13 ↗(On Diff #39207)

We do install the while zip file (apart from a few Linux binary artifacts and PHP stuff that doesn't have a separate license) so all licenses listed in LICENSE_FILE apply.

73 ↗(On Diff #39207)

This ports rebuilds two Go distributions (1.6 and 1.8) included in the upstream zip to get native FreeBSD binaries and installs resulting GOROOTs with ${CP}, I stole this CP line from lang/go. While it probably would be possible to get the same result with a combination of COPYTREE_SHARE/BIN, it would make do-install much more complex because there's a bunch of executables and .pl/.sh scripts scattered through GOROOT subfolders. added inline comments.Feb 12 2018, 10:18 PM
20 ↗(On Diff #39223)

Maybe we need something like LICENSE_FILE_combo for cases when all licenses are lumped together in a single file.

yuri commandeered this revision.Feb 17 2018, 3:41 AM
yuri added a reviewer:
yuri retitled this revision from www/go-appengine-sdk: App Engine SDK for Go to New port: www/go-appengine-sdk: App Engine SDK for Go.Feb 17 2018, 3:42 AM
yuri edited reviewers, added: tcberner, adamw; removed: added inline comments.
82 ↗(On Diff #39406)

That would remove executable bit from all binaries, no?

tcberner accepted this revision.Feb 17 2018, 9:54 PM

From my side it looks good.

This revision is now accepted and ready to land.Feb 17 2018, 9:54 PM
This revision was automatically updated to reflect the committed changes.
sunpoet added inline comments.Feb 17 2018, 10:43 PM

For me, I would not set LICENSE_FILE for a license without full text in the file.

Though 7 licenses are mentioned in ${WRKSRC}/LICENSE, it only has full text of 2 licenses (BSD3CLAUSE and MIT).

That means I would only set LICNESE_FILE_BSD3CLAUSE and LICENSE_FILE_MIT.

If you think ${WRKSRC}/LICENSE contains 7 licenses, then LICENSE_FILE=${WRKSRC}/LICENSE is fine (support added in r451432).

Regarding GPLv2, LGPL21 and MPL11, I guess you add them because of cacerts.
GPLv2 and LGPL21 should be change to GPLv2+ and LGPL21+.
The correct license for this port is "APACHE20 and BSD3CLAUSE and MIT and PSFL and (GPLv2+ or LGPL21+ or MPL11)".
Since our LICENSE framework does not support the case of "A and B and C and D and (E or F or G)", it will be better to add a comment.



Version: MPL 1.1/GPL 2.0/LGPL 2.1
Alternatively, the contents of this file may be used under the terms of
either the GNU General Public License Version 2 or later (the "GPL"), or
the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),


Use ${PYTHON_CMD} instead of "{LOCALBASE}/bin/python2.7".

yuri added inline comments.Feb 17 2018, 11:20 PM


Thank you for this information.
I will update the port.