Re: [NEW] net/google-compute-engine

Hi Daniel,

Thanks a lot for your review. I'll send a new version with the suggested
changes (and with the latest version of the port).

On 2017-06-01 11:38 PM, Daniel Jakots wrote:
> On Thu, 1 Jun 2017 22:03:22 -0300, Helen Koike
> <helen.koike@collabora.com> wrote:
>
>> I would appreciate if someone could review it and make any comments
>> or tell me that the package is OK.
>
> 1. $ portcheck
> patches/patch-google__compute__engine_accounts_accounts__daemon.py does not have $OpenBSD$ RCS tag at the top
> [...]
>
> I guess you don't know about make update-patches' do you? :)

no, I don't :(
(yet) I am in the process of "s/don't/do" in this phrase

>
> 2. I'm a bit puzzled by the number of rcscripts.

There is a bunch of them in the mainline project, you can see them here
https://github.com/GoogleCloudPlatform/compute-image-packages/tree/master/google_compute_engine_init/

>
> 3. I think you lack share/doc/pkg-readmes/${FULLPKGNAME} in the PLIST

ok

>
> 4. HOMEPAGE is already set by the GH_* things so you can remove it

good to know, thanks

>
> 5. There are a bunch of bash script with
> google_compute_engine_init/build_packages.sh:#!/bin/bash
> which I guess should be changed

build_packages.sh is just a script to build the google-compute-engine
packages for different linux distros using ruby fpm to call setup.py
with different arguments, it is not used in OpenBSD at all as the
Makefile uses setuptools directly

>
> 6. does it really need net/ntp as a dependency?
> google_compute_engine/clock_skew/clock_skew_daemon.py: ntpd_inactive = subprocess.call(['/etc/rc.d/ntpd', 'check'])
> That rather smells like openntpd should be enough :)

ok, I'll check openntpd

>
> 6bis. How did you choose the actual list of dependencies?

I checked which programs and python libs the code was using and I added
in the list basically. But I think I missed some libs, I'll check/test
again.

>
> 7. Any reason to not to enable tests?

no (I just didn't know I had this option), I'll check that

>
> 8. patch-google__compute__engine_boto_boto__config.py is empty

Sorry, this is a left over, in the git link I sent previously there is
no such file

>
> 9. What's the point of patching stuff like
> - '/usr/bin/google_instance_setup.')
> + '%%PREFIX%%/bin/google_instance_setup.')
> and then sed'ding it in pre-configure?

Because I saw other ports doing this, and this allows different
installation PREFIX to be used no?

>
> 10. LOCALBASE wouldn't be better than PREFIX?

maybe, I didn't know about it, I'll check, thanks

>
> 11. Don't you need a RDEP on net/py-netifaces?

Yes, It is there already:

RUN_DEPENDS = shells/bash \
security/sudo \
net/ntp \
net/py-boto \
net/py-netifaces \
net/py-netaddr


>
> There are probably other nits but that's all for now :)
>

LN