[Bug 219687] [NEW PORT] net/google-compute-engine: User daemon for Google Compute Engine

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219687

Helen Koike <helen.koike at collabora.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #183898|0                           |1
        is obsolete|                            |
 Attachment #183899|0                           |1
        is obsolete|                            |

--- Comment #10 from Helen Koike <helen.koike at collabora.com> ---
Created attachment 183934
  --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=183934&action=edit
Patch which adds google-compute-engine v2.4.0 in freebsd-ports

(In reply to Kubilay Kocak from comment #8)

> Thank you for your new port contribution Helen.
> 
> Pending further review, initial review items (needing updated patch) are:

Thanks for your review
I applied most of the items, please see new attached patch. But I still have
some questions below.

> 
> - Add missing PKGNAMEPREFIX=${PYTHON_PKGNAMEPREFIX}

Done

> - Missing secondary 'python' virtual category in CATEGORIES

Done

> - Add USE_PYTHON=concurrent if the port is multiple python version concurrent
> installation safe (all files are uniquely named and dont conflict)

Done

> - Use 'CHEESESHOP' (PyPI) as MASTER_SITES unless there is a compelling reason
> not to (for instance, test or other important files missing from source
> distribution on PyPI)

I added CHEESESHOP, but do I need to define MASTER_SITES when USE_GITHUB=yes ?

> - Add test support (using do-test: target), adding TEST_DEPENDS (py.test, etc
> as per setup.py/tox.ini) and executing ${PYTHON_CMD} -m pytest or ${PYTHON_CMD}
> setup.py test if supported. It's OK if not all test dependencies have been
> ported yet, just add the ones that have, adding comments for the ones that
> havent.

setup.py test is not supported, as explained in the previous comment, only mock
tests are supported and I am not sure if it is worthy to port them all.

> - setuptools appears (in setup.py) as an install_requires (RUN_DEPENDS) not a
> setup_requires (BUILD_DEPENDS) as in the patch. I don't know why its explicitly
> in install_requires though.

hm, I'll check with upstream maintainers, it centenly doesn't use setuptools at
run time

> - Add LICENSE_FILE=${WRKSRC}/path/to/license when one exists in the
> distribution file

Done

> - netifaces/netaddr don't appear as setup.py (compulsory) or tox.ini
> (potentially optional) dependencies. Can you elaborate on their source and
> function?

The upstream project doesn't use netifaces/netaddr.
For example, it uses /sys/class/net/<iface-name>/address to retrieve the mac
address. As FreeBSD doesn't seem to have this patch in sysfs, I changed the
script to use netifaces/netaddr instead and I added them in the dependencies
list

> 
> If possible (highly desirable/recommended), please provide as an attachment the
> results of the package test suite as well.

As the mock tests are not ported, I am testing by hand so I unfortunatelly
doesn't have a test log to attach.

> 
> For more information on creating ports for Python packages, see:
> 
> https://wiki.freebsd.org/Python/PortsPolicy

Thanks for this link, I also add the following changes:

* I set the PORTVERSION back to 2.4.0 but I had to define GH_TAGNAME and
DISTNAME so it downloads the package correctly, is this ok?
-PORTVERSION=   20170609
...
+PORTVERSION=   2.4.0
+DISTNAME=      ${GH_ACCOUNT}-${GH_PROJECT}-${GH_TAGNAME}
...
+GH_TAGNAME=    20170609

* About the patches, I know they are not ready to be upstreamable yet (it is
not that straight forward to make it compatible to Linux), but I'll work on
that. Should I add comments on the patches anyway?

* Portlint result:
# portlint -AC
WARN: Makefile: using hyphen in PORTNAME. consider using PKGNAMEPREFIX and/or
PKGNAMESUFFIX.
WARN: /usr/ports/net/google-compute-engine/pkg-message: possible use of
absolute pathname "/etc/rc.conf".
0 fatal errors and 2 warnings found.

It seems two false positives, the first one wasn't firing before, not sure why
it is appearing now. About the second one, I checked how other packages mention
/etc/rc.conf and all of them do like this, so I suppose this is the right way
to do.

Thanks

-- 
You are receiving this mail because:
You are on the CC list for the bug.