[gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Damo Brisbane]

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

I've run up a couple of golang based ebuilds - for the fabio load balancer.
My first run at it, not completely sure of any follow up process, mentor?
other posting, overlap with existing work? Anyway, would appreciate the
feedback.

FYI, custom overlay at * https://github.com/damobrisbane/damo-overlay *.

Tried to follow ?prior art? - but first run at it, probably dumb stuff in
there, of note:

* Followed "consul" ebuild for template, specifically adds users/openrc
init and confd files and logging. Not a systemd fan so please dont ask
unless you want to do it yourself..

* I think installs ok?:

>>> emerge -aq fabio

[ebuild  N    ] dev-go/govendor-1.0.9
[ebuild  N    ] dev-go/vendorfmt-9999
[ebuild  N    ] net-proxy/fabio-1.5.3

Would you like to merge these packages? [Yes/No] y

>>> Verifying ebuild manifests
>>> Emerging (1 of 3) dev-go/govendor-1.0.9::damo-overlay
>>> Installing (1 of 3) dev-go/govendor-1.0.9::damo-overlay
>>> Emerging (2 of 3) dev-go/vendorfmt-9999::damo-overlay
>>> Installing (2 of 3) dev-go/vendorfmt-9999::damo-overlay
>>> Emerging (3 of 3) net-proxy/fabio-1.5.3::damo-overlay
>>> Installing (3 of 3) net-proxy/fabio-1.5.3::damo-overlay
>>> Recording net-proxy/fabio in "world" favorites file...

>>>

Thanks in advance
Damon

[-- Attachment #2: Type: text/html, Size: 1863 bytes --]

Re: [gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Michael Orlitzky]

On 11/09/2017 11:08 PM, Damo Brisbane wrote:
> I've run up a couple of golang based ebuilds - for the fabio load
> balancer. My first run at it, not completely sure of any follow up
> process, mentor? other posting, overlap with existing work? Anyway,
> would appreciate the feedback.

Your $VERSION variable can probably be replaced with "${PV}" to save a line.

Your init script takes care of the permissions on /var/lib/fabio and
/var/log/fabio/fabio.log...

  start_pre() {
    checkpath -q -d -o ${FABIO_USER}:${FABIO_GROUP} ${FABIO_HOMEDIR}
    checkpath -q -f -o ${FABIO_USER}:${FABIO_GROUP} ${FABIO_LOGFILE}
  }

so the following in the ebuild might be redundant?

  for x in /var/{lib,log}/${PN}; do
    keepdir "${x}"
    fowners fabio:fabio "${x}"
  done

(warning: I have never understood what keepdir is supposed to
accomplish, so maybe I'm wrong here).

On the other hand, if you've created a dedicated user and group for the
daemon, I don't think there's much benefit to letting the end user
switch them via FABIO_USER and FABIO_GROUP (it just makes the
permissions harder to get right). That's a judgment call though.

Finally, if the stanza above *does* turn out to be redundant, then
there's another small improvement that can be made. Since the "fabio"
user and group are used nowhere else in the ebuild, you could create
them in pkg_preinst() instead of pkg_setup(). Doing that has one main
benefit -- namely that if the installation fails, the user and group
won't be created.

Overall, looks good.

For testing help, you'll probably have the best luck in #gentoo-user on
IRC. For ebuild reviews, we have a dedicated mailing list,
gentoo-devhelp@lists.gentoo.org and an associated IRC channel,
#gentoo-dev-help (yes, they're hyphenated differently...)

Re: [gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Damo Brisbane]

[-- Attachment #1: Type: text/plain, Size: 2799 bytes --]

Cheers Michael,

I've made most those changes, including USER/GROUP*, that confuses me too;
copied pattern from another ebuild. Also removed creating separate folder
/etc/fabio.d; re this, I noticed the fabio configuration accepts either
folder path or *URL* for configuration file; not sure how this works,
though it does seem to underlie some type of arbitrariness in including
such a /etc/fabio.d folder within this ebuild.

Re for...keepdir, I found removing it then the /var/log/fabio folders were
not getting created, so keeping it in there.

http://www.calculate-linux.org/main/en/using_ebuild, says this of *keepdir*:

*Creates (if necessary) a .keep file in the given directory so that it
isn't auto-cleaned. Never create a .keep file yourself. If Portage changes
how keepdir works, then creating the file yourself will break the package.*

regards,
Damon

On Fri, Nov 10, 2017 at 10:59 PM, Michael Orlitzky <mjo@gentoo.org> wrote:

> On 11/09/2017 11:08 PM, Damo Brisbane wrote:
> > I've run up a couple of golang based ebuilds - for the fabio load
> > balancer. My first run at it, not completely sure of any follow up
> > process, mentor? other posting, overlap with existing work? Anyway,
> > would appreciate the feedback.
>
> Your $VERSION variable can probably be replaced with "${PV}" to save a
> line.
>
> Your init script takes care of the permissions on /var/lib/fabio and
> /var/log/fabio/fabio.log...
>
>   start_pre() {
>     checkpath -q -d -o ${FABIO_USER}:${FABIO_GROUP} ${FABIO_HOMEDIR}
>     checkpath -q -f -o ${FABIO_USER}:${FABIO_GROUP} ${FABIO_LOGFILE}
>   }
>
> so the following in the ebuild might be redundant?
>
>   for x in /var/{lib,log}/${PN}; do
>     keepdir "${x}"
>     fowners fabio:fabio "${x}"
>   done
>
> (warning: I have never understood what keepdir is supposed to
> accomplish, so maybe I'm wrong here).
>
> On the other hand, if you've created a dedicated user and group for the
> daemon, I don't think there's much benefit to letting the end user
> switch them via FABIO_USER and FABIO_GROUP (it just makes the
> permissions harder to get right). That's a judgment call though.
>
> Finally, if the stanza above *does* turn out to be redundant, then
> there's another small improvement that can be made. Since the "fabio"
> user and group are used nowhere else in the ebuild, you could create
> them in pkg_preinst() instead of pkg_setup(). Doing that has one main
> benefit -- namely that if the installation fails, the user and group
> won't be created.
>
> Overall, looks good.
>
> For testing help, you'll probably have the best luck in #gentoo-user on
> IRC. For ebuild reviews, we have a dedicated mailing list,
> gentoo-devhelp@lists.gentoo.org and an associated IRC channel,
> #gentoo-dev-help (yes, they're hyphenated differently...)
>
>

[-- Attachment #2: Type: text/html, Size: 4427 bytes --]

Re: [gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Michael Orlitzky]

On 11/10/2017 04:36 PM, Damo Brisbane wrote:
> 
> Re for...keepdir, I found removing it then the /var/log/fabio folders
> were not getting created, so keeping it in there.

You need to tell the ebuild to create that directory one way or another.
The "dodir" function will create the directory, but without the ".keep"
file inside of it. However that may be "illegal" in this case; see below.


> http://www.calculate-linux.org/main/en/using_ebuild, says this of *keepdir*:
> 
> *Creates (if necessary) a |.keep| file in the given directory so that it
> isn't auto-cleaned. Never create a |.keep| file yourself. If Portage
> changes how |keepdir| works, then creating the file yourself will break
> the package.*

To my knowledge, no package manager will remove a non-empty directory,
nor will it remove anything that the package manager did not itself
create. To me that raises a question: why would I ever want to keep
around an (otherwise empty) directory that was created by the package
manager?

I found this,

  https://dev.gentoo.org/~ulm/pms/head/pms.html#x1-15100013.2.2

which states

  Behaviour upon encountering an empty directory is undefined. Ebuilds
  must not attempt to install an empty directory.

Certainly "keepdir" will make the directory non-empty, but with the
additional (unwanted) side-effect that the directory won't be removed
when the package is uninstalled. Thus "keepdir" doesn't seem like it was
intended to address that technicality. So, I have two questions now...

  a) When would you want to use keepdir?

  b) What's the right way to prevent a directory from being empty? Touch
     a dummy file?

and a meta-question,

  c) Seriously, empty directories are undefined behavior?

Re: [gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Michał Górny]

W dniu pią, 10.11.2017 o godzinie 19∶21 -0500, użytkownik Michael
Orlitzky napisał:
> On 11/10/2017 04:36 PM, Damo Brisbane wrote:
> > 
> > Re for...keepdir, I found removing it then the /var/log/fabio folders
> > were not getting created, so keeping it in there.
> 
> You need to tell the ebuild to create that directory one way or another.
> The "dodir" function will create the directory, but without the ".keep"
> file inside of it. However that may be "illegal" in this case; see below.
> 
> 
> > http://www.calculate-linux.org/main/en/using_ebuild, says this of *keepdir*:
> > 
> > *Creates (if necessary) a |.keep| file in the given directory so that it
> > isn't auto-cleaned. Never create a |.keep| file yourself. If Portage
> > changes how |keepdir| works, then creating the file yourself will break
> > the package.*
> 
> To my knowledge, no package manager will remove a non-empty directory,
> nor will it remove anything that the package manager did not itself
> create. To me that raises a question: why would I ever want to keep
> around an (otherwise empty) directory that was created by the package
> manager?
> 
> I found this,
> 
>   https://dev.gentoo.org/~ulm/pms/head/pms.html#x1-15100013.2.2
> 
> which states
> 
>   Behaviour upon encountering an empty directory is undefined. Ebuilds
>   must not attempt to install an empty directory.
> 
> Certainly "keepdir" will make the directory non-empty, but with the
> additional (unwanted) side-effect that the directory won't be removed
> when the package is uninstalled. Thus "keepdir" doesn't seem like it was
> intended to address that technicality. So, I have two questions now...

Wrong. It creates a dotfile inside it, and removes it along with it.

> 
>   a) When would you want to use keepdir?

Because it works.

> 
>   b) What's the right way to prevent a directory from being empty? Touch
>      a dummy file?

Use keepdir.

> 
> and a meta-question,
> 
>   c) Seriously, empty directories are undefined behavior?

...and how could they be defined if a directory can be installed by
multiple packages and has no explicit ownership?

-- 
Best regards,
Michał Górny

Re: [gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Michael Orlitzky]

On 11/11/2017 02:58 AM, Michał Górny wrote:
>>
>> Certainly "keepdir" will make the directory non-empty, but with the
>> additional (unwanted) side-effect that the directory won't be removed
>> when the package is uninstalled.
> 
> Wrong. It creates a dotfile inside it, and removes it along with it.

Gotcha, I never tested my assumption that keepdir would keep the dir =P

FWIW, `man 5 ebuild` says keepdir "Tells portage to leave directories
behind..." It's not at all apparent that the "left behind" refers to the
unmerge of some other, unrelated, package.


>>   a) When would you want to use keepdir?
> 
> Because it works.

Makes sense now, thanks.



>> and a meta-question,
>>
>>   c) Seriously, empty directories are undefined behavior?
> 
> ...and how could they be defined if a directory can be installed by
> multiple packages and has no explicit ownership?

I see the problem, but the package manager knows which packages are
using a given directory. (Portage does, and it is at least easy to
record that information.)

Empty directories could be installed normally, and then during an
unmerge, the package manager could check to see if the empty directory
is used by any package. If it is, leave it -- if not, remove it. You
might object that this would slow down the unmerge process, but a clever
lookup scheme would let you map directory names to packages quickly.

In fact, such a lookup scheme is already implemented in the filesystem
itself, leading me full circle to the following idea: if the package
manager is about to install an empty directory, it could create the
".keep" file itself. Then "ls -a $dir" is your lookup function that
determines whether or not a directory is in use by a package.

Having the package manager handle empty directories solves two problems,

  1) Use of "keepdir" is inconsistent, because portage is happy to let
     you create an empty directory without it (even though that
     operation is illegal).

  2) The build systems of many packages will create empty directories
     during "make install", and it's unreasonable to expect developers
     to "keepdir" them all.

Essentially,we have two commands to create a directory, "dodir" and
"do-empty-dir" (which we call "keepdir"). The latter is only necessary
due to an implementation detail, so it doesn't belong in the user
interface -- the PM should figure out what to do.

As far as the actual implementation goes, I'm not sure that
automatically-generated ".keep" files are better than having the package
manager maintain its own database. The latter would be more complex, but
would avoid littering everyone's filesystems with ".keep" files.

Re: [gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Michał Górny]

W dniu sob, 11.11.2017 o godzinie 12∶31 -0500, użytkownik Michael
Orlitzky napisał:
> > > and a meta-question,
> > > 
> > >   c) Seriously, empty directories are undefined behavior?
> > 
> > ...and how could they be defined if a directory can be installed by
> > multiple packages and has no explicit ownership?
> 
> I see the problem, but the package manager knows which packages are
> using a given directory. (Portage does, and it is at least easy to
> record that information.)
> 
> Empty directories could be installed normally, and then during an
> unmerge, the package manager could check to see if the empty directory
> is used by any package. If it is, leave it -- if not, remove it. You
> might object that this would slow down the unmerge process, but a clever
> lookup scheme would let you map directory names to packages quickly.
> 
> In fact, such a lookup scheme is already implemented in the filesystem
> itself, leading me full circle to the following idea: if the package
> manager is about to install an empty directory, it could create the
> ".keep" file itself. Then "ls -a $dir" is your lookup function that
> determines whether or not a directory is in use by a package.

What about a directory that is installed empty by multiple different
packages, and non-empty by some other packages?

> Having the package manager handle empty directories solves two problems,
> 
>   1) Use of "keepdir" is inconsistent, because portage is happy to let
>      you create an empty directory without it (even though that
>      operation is illegal).

It is not. It is just not guaranteed to be meaningful.

> 
>   2) The build systems of many packages will create empty directories
>      during "make install", and it's unreasonable to expect developers
>      to "keepdir" them all.

Not all of those directories are really meaningful.

> Essentially,we have two commands to create a directory, "dodir" and
> "do-empty-dir" (which we call "keepdir"). The latter is only necessary
> due to an implementation detail, so it doesn't belong in the user
> interface -- the PM should figure out what to do.
> 
> As far as the actual implementation goes, I'm not sure that
> automatically-generated ".keep" files are better than having the package
> manager maintain its own database. The latter would be more complex, but
> would avoid littering everyone's filesystems with ".keep" files.

Do you care enough to spec this properly, introduce EAPI-conditional
behavior for it and prepare patches for the package managers?

-- 
Best regards,
Michał Górny

Re: [gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Michael Orlitzky]

On 11/11/2017 02:26 PM, Michał Górny wrote:
>>
>> As far as the actual implementation goes, I'm not sure that
>> automatically-generated ".keep" files are better than having the package
>> manager maintain its own database. The latter would be more complex, but
>> would avoid littering everyone's filesystems with ".keep" files.
> 
> Do you care enough to spec this properly, introduce EAPI-conditional
> behavior for it and prepare patches for the package managers?
> 

Some day -- I'll add it to my list. For now I'll update the docs to
explain why you should use keepdir, and do a QA warning for empty
directories. Then how does this sound for EAPI=next?

  * Ban keepdir.

  * Have portage call its keepdir code on any empty directories in $D
    between src_install and pkg_preinst.

  * Update the devmanual and portage documentation to suggest dodir
    instead of keepdir in the new EAPI.

  * Change the PMS to remove "undefined behavior" and replace it with
    "empty directories must be tracked, and may only be removed once no
    installed package is using them," or something along those lines.
    That leaves the implementation up to the PM.

Re: [gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Michał Górny]

W dniu nie, 12.11.2017 o godzinie 07∶53 -0500, użytkownik Michael
Orlitzky napisał:
> On 11/11/2017 02:26 PM, Michał Górny wrote:
> > > 
> > > As far as the actual implementation goes, I'm not sure that
> > > automatically-generated ".keep" files are better than having the package
> > > manager maintain its own database. The latter would be more complex, but
> > > would avoid littering everyone's filesystems with ".keep" files.
> > 
> > Do you care enough to spec this properly, introduce EAPI-conditional
> > behavior for it and prepare patches for the package managers?
> > 
> 
> Some day -- I'll add it to my list. For now I'll update the docs to
> explain why you should use keepdir, and do a QA warning for empty
> directories.

I'm not convinced a QA warning is valid, given that not every empty
directory is meaningful. You're going to either cause people to create
unnecessary 'keepdir's, or to be swamped by false positives.

>  Then how does this sound for EAPI=next?
> 
>   * Ban keepdir.
> 
>   * Have portage call its keepdir code on any empty directories in $D
>     between src_install and pkg_preinst.

How does this account for /run and other non-persistent locations?

>   * Update the devmanual and portage documentation to suggest dodir
>     instead of keepdir in the new EAPI.
> 
>   * Change the PMS to remove "undefined behavior" and replace it with
>     "empty directories must be tracked, and may only be removed once no
>     installed package is using them," or something along those lines.
>     That leaves the implementation up to the PM.

...and makes interoperability between different package managers
impossible, defeating the purpose of PMS in the first place.

-- 
Best regards,
Michał Górny

Re: [gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Michael Orlitzky]

On 11/12/2017 08:43 AM, Michał Górny wrote:
> 
> I'm not convinced a QA warning is valid, given that not every empty
> directory is meaningful. You're going to either cause people to create
> unnecessary 'keepdir's, or to be swamped by false positives.

The warning would essentially be saying, "you installed this empty
directory, do you need it?" Granted, it would be annoying if the answer
was "no" and if my package triggered thirty of them, but then that's a
bug (possibly upstream) that should be reported and fixed. On the other
hand, if you need the directory, then you should be calling "keepdir" on
it -- so in either case I don't think the warning is a false positive.


>>   * Have portage call its keepdir code on any empty directories in $D
>>     between src_install and pkg_preinst.
> 
> How does this account for /run and other non-persistent locations?

What specific problem do you foresee?

The implementation above should be no worse than what we have. Right
now, there are a lot of ebuilds that are installing empty directories.
Those ebuilds are buggy in one way or another: either relying on
undefined behavior, or creating unused directories for no reason. (Many
of the first type are not easily fixed, since the upstream build system
creates them.) The "automatic keepdir" would still create unused
directories, but it would fix the directories that should have been
keepdir'd but weren't.

This presents the usual problems with /run, but none that we don't
already have. Ebuilds that create directories under /run already receive
a QA warning not to do that, and those directories are already clobbered
on a reboot; that doesn't change. If a package like baselayout wants to
create an empty /run, it can do so in pkg_postinst to avoid the ".keep"
file. What else am I missing?


>>   * Change the PMS to remove "undefined behavior" and replace it with
>>     "empty directories must be tracked, and may only be removed once no
>>     installed package is using them," or something along those lines.
>>     That leaves the implementation up to the PM.
> 
> ...and makes interoperability between different package managers
> impossible, defeating the purpose of PMS in the first place.

Well, that's why I'm asking.. I don't know WTF I'm doing, but I'd still
like the proposal to be decent when I give it to the people who do.

Re: [gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

>>>>> On Sun, 12 Nov 2017, Michael Orlitzky wrote:

> Some day -- I'll add it to my list. For now I'll update the docs to
> explain why you should use keepdir, and do a QA warning for empty
> directories. Then how does this sound for EAPI=next?

>   * Ban keepdir.

>   * Have portage call its keepdir code on any empty directories in $D
>     between src_install and pkg_preinst.

>   * Update the devmanual and portage documentation to suggest dodir
>     instead of keepdir in the new EAPI.

>   * Change the PMS to remove "undefined behavior" and replace it with
>     "empty directories must be tracked, and may only be removed once no
>     installed package is using them," or something along those lines.
>     That leaves the implementation up to the PM.

How? Look up VDB entries of all installed packages? Note that this
would have to be done for every dir that becomes empty, not just the
ones currently containing a .keep_* file.

What problem are you trying to solve? I see typically around 100
.keep_* files on my systems. These are empty files, so they don't use
any blocks. And 100 inodes system wide looks like negligible usage
of resources to me.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Michael Orlitzky]

On 11/12/2017 10:21 AM, Ulrich Mueller wrote:
> 
>>   * Change the PMS to remove "undefined behavior" and replace it with
>>     "empty directories must be tracked, and may only be removed once no
>>     installed package is using them," or something along those lines.
>>     That leaves the implementation up to the PM.
> 
> How? Look up VDB entries of all installed packages? Note that this
> would have to be done for every dir that becomes empty, not just the
> ones currently containing a .keep_* file.

Not necessarily. I chose the "empty directories must be tracked" wording
to avoid that. If the PM were about to remove a directory that wasn't in
the database of empty directories, then it could proceed normally.


> What problem are you trying to solve? I see typically around 100
> .keep_* files on my systems. These are empty files, so they don't use
> any blocks. And 100 inodes system wide looks like negligible usage
> of resources to me.

If you're asking what problem I was trying to solve by leaving the
implementation up to the PM, then I was only trying not to be pushy. If
the "automatic keepdir" idea...

>   * Have portage call its keepdir code on any empty directories in $D
>     between src_install and pkg_preinst.

is workable and if the PM authors are fine with it, then we could spec that.

Re: [gentoo-dev] Help testing ebuilds? golang/Fabio load balancer

[Brian Dolbec]

On Sat, 11 Nov 2017 12:31:15 -0500
Michael Orlitzky <mjo@gentoo.org> wrote:


> Essentially,we have two commands to create a directory, "dodir" and
> "do-empty-dir" (which we call "keepdir"). The latter is only necessary
> due to an implementation detail, so it doesn't belong in the user
> interface -- the PM should figure out what to do.
> 
> As far as the actual implementation goes, I'm not sure that
> automatically-generated ".keep" files are better than having the
> package manager maintain its own database. The latter would be more
> complex, but would avoid littering everyone's filesystems with
> ".keep" files.
> 

Well, for:

1) using .keepdir files makes this package manager non-specific, ie:
using a different PM would mean it too sees that it is suppose to be
kept.

2) Most ebuilds don't need/or use .keepdir, so I doubt there are that
   many littering up your filesystem.

3) There already is a database of the files installed by a package.
   But, how does it know which other packages might need to install to
   that directory.  So, what does it do on unmerge of the package.
   Typically, the package manager will refuse to remove a non-empty
   directory after the files it is removing are removed.

4) Creating a single database for these will make the system more
   vulnerable to corruption and data loss.  It would save on disk usage
   and number of inodes used.  But there is a tradeoff between space and
   robustness.  If one .keepdir is lost, it may only affect a few
   packages.  If the database becomes corrupted.  It could potentially
   mean the loss of much more of your installed pkg data. Spanning a
   great deal of the installed pkgs.  In this case simplicity
   of .keepdir is in my opinion, much better than a single db.

-- 
Brian Dolbec <dolsen>