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

On 2017/06/01 23:58, Helen Koike wrote:
> > 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

Patch filenames shouldn't have "." either. I suggest this to normalize
them all:

make clean
make patch
mv patches patches.old (or just rm patches/*)
make update-patches

If you patch any other files, "cp somefile somefile.orig", edit somefile,
"make update-patches" to pick it up.

> > 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

/etc/rc.d/ntpd is for openntpd, which is in the base OS.
net/ntpd in ports is the ntp.org software and only used for special
cases on OpenBSD, it's not a good choice for general-purpose ntp here.

> > 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.

I haven't looked but hopefully there will be a list in setup.py
that will help.

> > 7. Any reason to not to enable tests?
>
> no (I just didn't know I had this option), I'll check that

You can do this as a do-test target in the port's Makefile.

> > 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?

The usual method is to patch to use ${LOCALBASE} and use something like
"${SUBST_CMD} ${WRKSRC}/dir/file" in the port's Makefile. Ones with %%
etc are often old or there's some special reason.

This used to be a bit annoying and undid the ${..} changes if you ran
"update-patches" after a build, workaround is to clean/patch/edit/update
without building, but this has been improved after the 6.1 release.

> > 10. LOCALBASE wouldn't be better than PREFIX?
>
> maybe, I didn't know about it, I'll check, thanks

PREFIX is correct for that one. (PREFIX is "file from this port being
installed", LOCALBASE is "file from another port that we're referencing")

> > 11. Don't you need a RDEP on net/py-netifaces?
>
> Yes, It is there already:
>
> RUN_DEPENDS = shells/bash \
> security/sudo \

Prefer to use doas (in the base OS) rather than sudo if possible.

Also prefer to use ksh from base rather than bash if the scripts work
with it without excessive changes.

For the rc scripts, I suggest a "meta" script (google_compute_engine.rc?)
which calls the others and refer to that in the README, so users only have
to make one entry in pkg_scripts and don't need to worry about ordering.
sysutils/bacula/pkg/bacula.rc would be a good model.

nit: the === underlining in pkg/README is too short, make sure you edit
it with a fixed-width font.