Page MenuHomeFreeBSD

databases/automysqlbackup: add initial version of port (3.0_rc6)
ClosedPublic

Authored by me_cschwarz.com on Mar 17 2019, 1:48 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Oct 7, 1:00 PM
Unknown Object (File)
Sat, Oct 5, 9:44 AM
Unknown Object (File)
Fri, Oct 4, 4:40 PM
Unknown Object (File)
Fri, Oct 4, 3:38 AM
Unknown Object (File)
Thu, Oct 3, 5:33 AM
Unknown Object (File)
Tue, Oct 1, 12:34 PM
Unknown Object (File)
Mon, Sep 30, 5:13 PM
Unknown Object (File)
Tue, Sep 24, 5:00 PM
Subscribers

Details

Summary

port databases/autobackupmysql exists, but does not have the same featureset

Test Plan

Works in my MySQL jail (mariadb-103-{server,client})

Diff Detail

Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 23159
Build 22208: arc lint + arc unit

Event Timeline

matthew added inline comments.
databases/automysqlbackup/Makefile
29

${MKDIR} already includes the '-p' flag: no need to repeat it.

databases/automysqlbackup/pkg-plist
4

You aren't recreating the 0700 directory permissions that you set in the makefile -- this will end up wit 0755 permissionns if you install from pkg.

address first review round remarks
(where are the docs for @dir params?)

databases/automysqlbackup/pkg-plist
4

where is the documentation for @dir?

mmokhi added inline comments.
databases/automysqlbackup/Makefile
29

I'm not sure if I understood @matthew 's feedback on this line...
But I would personally let the chmod stuff being done in pkg-plist instead of Makefile... (Or at least either all here or all there)...

databases/automysqlbackup/files/patch-automysqlbackup
9

Maybe it's too much what I'm pointing, but I often use %LOCALBASE% (or in this case %ETCDIR%) and the ${REINPLACE_CMD} in post-patch πŸ˜…
Not saying it's better or worse to do so, but might help other people (who have diff place that /usr/local to use it as well 😊

databases/automysqlbackup/pkg-plist
4

This is probably what you are looking for. πŸ‘

databases/automysqlbackup/Makefile
29

When you are building a port, all the Makefile installation commands are doing is copying files into ${STAGEDIR}. The contents of ${STAGEDIR} are then turned into a pkg -- either gathered into a pkg tarball, or in it's installed form with files spread across your active filesytem. This happens whether or not you install directly from your ports tree, or whether you create a pkg tarball, stick it into a repository and download and install from there.

You're meant to be able to install to ${STAGEDIR} and thence build a pkg as an unprivileged user, which means that file ownership and permissions in ${STAGEDIR} can't just be copied into the pkg generated from ${STAGEDIR}, and thence into the main filesystem when the pkg is installed -- in fact, they are ignored, other than copying over any execute permissions. In particular anything like setuid or setgid bits are stripped; regular files default to root:wheel and mode 0644 or 0755 if executable, and directories to root:wheel and mode 0755. If you need any other permissions or ownership, this needs to be set in the pkg-plist.

In summary: permissions and ownership in ${STAGEDIR} are (almost) irrelevant: it's what's set in pkg-plist that counts.

databases/automysqlbackup/Makefile
29

Aha, yes.
Makes sense, I believe that's why we basically have the permission and ownership stuff in pkg-plist (as I suggested; more explanation available here).

Thanks for complete and reasonable explanation πŸ™ πŸ‘

databases/automysqlbackup/Makefile
29

Any action required on my part? Otherwise please mark this as done.

databases/automysqlbackup/files/patch-automysqlbackup
9

When is %ETCDIR% replaced then? This is a patch file... Does the ports tree use some magic version of patch(1)?

databases/automysqlbackup/Makefile
29

I guess, It makes sense to update your patch deleting the chmod stuff from Makefile and handle all the rest in pkg-plist 😊
Then the rest is fine IMHO πŸ‘

databases/automysqlbackup/files/patch-automysqlbackup
9

${REINPLACE_CMD} in post-patch target 😊
letting the patch file put the %ETCDIR% then you sed the patched file

me_cschwarz.com marked an inline comment as done.
  • remove chmod from post-install (covered by pkg-plist)

The %ETCDIR% thingie I commented about might be as well good to take care of
(If people with paths different that /usr/local are using your port for example).
But I don't think it's a necessity at the moment πŸ™‚
You can take care of it later πŸ‘

Macro lgtm:

Macro shipit:

This revision is now accepted and ready to land.Mar 27 2019, 10:32 AM

Just replacing /usr/local/etc with %ETCDIR% doesn't work on my machine.
Is it because I override do-install?

Just replacing /usr/local/etc with %ETCDIR% doesn't work on my machine.
Is it because I override do-install?

Sorry, maybe my comment was a bit misleading.
After replacing stuff with %ETCDIR% or anything in the patch-files, of course you need a post-patch: target with
a ${REINPLACE_CMD} in it to replace the %ETCDIR% value with ${ETCDIR}.

This revision now requires review to proceed.Mar 27 2019, 11:13 AM

Thanks.
To me it's perfect now 😊

Macro lgtm:

Macro everything-is-fine:

This revision is now accepted and ready to land.Mar 27 2019, 11:16 AM
This revision was automatically updated to reflect the committed changes.