- Stage - Add CONFLICTS (prepare to new port sysutils/backuppc-devel) - Add license - BackupPC.pod: add =encoding declaration (fix build with perl5.18) Fix: Patch attached with submission follows:
I'll take this.
Hi Alexander, I see you are adding config.pl.sample and hosts.sample. The porters handbook as been updated with a new @sample that handles these explicitly in the pkg-plist: http://www.freebsd.org/doc/en/books/porters-handbook/book.html#plist-config I'm a bit confused on the source of config.pl and hosts because they exist in the distfile and you also copy directly from the host system in do-install. Can you update the patch using @sample?
(In reply to milki from comment #2) Hi, I think I can't use @sample. config.pl and hosts that exists in the distfile are templates. configure.pl creates actual config files during install time. Using @sample is ok for fresh installed port. But if port was builded not on a clean system, config files contents depends on previous config files found. In case of upgrade or reinstallation config files will be lost. Also users may treat .sample files as default configs provided by the source. It may be confusing. To avoide this, I had place config files to ${STAGEDIR}${PREFIX}/libexec/${PORTNAME}. They intended to be conditionally copied as .sample during installation. In other words, .sample conditionally placed in ${ETCDIR} during port or package installation only if config files not exists in ${ETCDIR}. Ability to upgrade configuration files to new version is the reason to copy them from the host system in do-install to ${STAGEDIR}${ETCDIR}. BackupPC has no separate upgrade script. All actions (makes substitutions in sources, makes backup and updates config files during upgrade, database conversion, etc.) performes by all-in-one complex script configure.pl . To perform upgrade properly it requires BackupPC configuration files (config.pl, hosts) to be in ${ETCDIR} during installation. Otherwise user will need to make changes in config.pl and in some cases other actions manually. But for major upgrade it may be somewhat complex and, since it not intendend to be maked by hand, not well documented. Before staging configure.pl was able to upgrade config files in place, but the staging broke this. The only way I found to solve this problem is to copy config files to ${STAGEDIR}${ETCDIR} before running configure.pl Of course, automatic upgrade is possible only when installing from ports or sources. I know all this is tricky. Would you suggest more elegant way?
(In reply to Alexander Moisseev from comment #3) > (In reply to milki from comment #2) > > I think I can't use @sample. config.pl and hosts that exists in the distfile > are templates. configure.pl creates actual config files during install time. Ah, so they actually overwrite the user's files on install and only behave as sample files if this is a fresh install. That does indeed make it too complicated for @sample itself. If this is the expected upgrade path - using configure.pl to upgrade an existing installating, then not using @sample seems to be fine.
(In reply to Alexander Moisseev from comment #3) > I know all this is tricky. Would you suggest more elegant way? My reviewers have concerns about config.pl and hosts. If the users modify the files after install, they may not be overwritten properly by the port because they would be considered to be dirty files. The suggestion then is to leave the converted config.pl and hosts file with the .sample suffix and then add a pkg-message that tells the user to manually copy the files over. On another note, BPC_CGIDIR seems to align well with the port native WWWDIR. Would it make more sense to use WWWDIR?
(In reply to milki from comment #5) > > My reviewers have concerns about config.pl and hosts. If the users modify > the files after install, they may not be overwritten properly by the port > because they would be considered to be dirty files. > > The suggestion then is to leave the converted config.pl and hosts file with > the .sample suffix and then add a pkg-message that tells the user to > manually copy the files over. > As far as I understand files installed/removed using @exec/@unexec would not be considered to be dirty files because they not known to the package's table of contents. If I wrong, I would rework the port as suggested, but @sample keyword can not be used anyway because of reasons I mentioned before. I was taking a look at the Porter's Handbook today and found that @exec and @unexec keywords are deprecated. What should be used instead? > On another note, BPC_CGIDIR seems to align well with the port native WWWDIR. > Would it make more sense to use WWWDIR? OK. I don't remember why BPC_CGIDIR and BPC_DATADIR was introduced. Now they don't needed any more. I'll take care of them.
(In reply to Alexander Moisseev from comment #6) > As far as I understand files installed/removed using @exec/@unexec would not > be considered to be dirty files because they not known to the package's > table of contents. If I wrong, I would rework the port as suggested, but > @sample keyword can not be used anyway because of reasons I mentioned > before. I was taking a look at the Porter's Handbook today and found that > @exec and @unexec keywords are deprecated. What should be used instead? @exec and @unexec run at install. Since this is subsumed in the stage step, you can do them in the install target directly. I'm not aware of the @exec/@unexec and dirty files handling that you mention. I'll need to read and understand that bit and see how this works.
(In reply to milki from comment #7) > (In reply to Alexander Moisseev from comment #6) > > > @exec and @unexec keywords are deprecated. What should be used instead? > > @exec and @unexec run at install. Since this is subsumed in the stage step, > you can do them in the install target directly. > I didn't get the point. Would you clarify for example how to create /var/log/BackupPC directory owned by backuppc:backuppc in install target? It can't be just added to pkg-plist since user backuppc doesn't exist during stage. I.e. what to do instead of pkg-plist lines: @exec mkdir -p -m 0750 /var/log/BackupPC @exec chown -R backuppc:backuppc /var/log/BackupPC @unexec rmdir >/dev/null 2>&1 /var/log/BackupPC || :
(In reply to Alexander Moisseev from comment #8) > I didn't get the point. > > Would you clarify for example how to create /var/log/BackupPC directory > owned by backuppc:backuppc in install target? It can't be just added to > pkg-plist since user backuppc doesn't exist during stage. For this example, if you take a look at the logs, you'll notice the user is created during stage: http://poudriere.ircmylife.com:13780/data/latest-per-pkg/backuppc/3.3.0_1/92x86-svn.log =======================<phase: stage >============================ ===> Staging for backuppc-3.3.0_1 ===> Generating temporary packing list ===> Creating users and/or groups. And in the handbook, section 6.25.1 (http://www.freebsd.org/doc/en/books/porters-handbook/book.html#rc-scripts), bullet 17 says to use install -d.
(In reply to milki from comment #9) > > For this example, if you take a look at the logs, you'll notice the user is > created during stage: > > http://poudriere.ircmylife.com:13780/data/latest-per-pkg/backuppc/3.3.0_1/ > 92x86-svn.log > > =======================<phase: stage >============================ > ===> Staging for backuppc-3.3.0_1 > ===> Generating temporary packing list > ===> Creating users and/or groups. I am not agree. The port framework lies about it. Actually it creates the users-groups.sh during stage. The user is created during install: =======================<phase: install >============================ ===> Creating users and/or groups. Creating group 'backuppc' with gid '300'. Creating user 'backuppc' with uid '300'.
(In reply to milki from comment #5) > > On another note, BPC_CGIDIR seems to align well with the port native WWWDIR. > Would it make more sense to use WWWDIR? Would you to be more exact about "align well"? Did you mean just to define it like this: CGIDIR?=${WWWDIR:S,${PORTNAME}$$,cgi-bin,} Or _relocate_ CGI directory from ${PREFIX}/www/cgi-bin to ${WWWDIR}/cgi-bin ?
(In reply to Alexander Moisseev from comment #11) > Or _relocate_ CGI directory from ${PREFIX}/www/cgi-bin to ${WWWDIR}/cgi-bin ? I was thinking of relocating to ${WWWDIR}/cgi-bin. However, I did a cursory search of the ports tree and found other ports that define their own CGIDIR that also use ${PREFIX}/www/cgi-bin. The porter's handbook doesn't cover this specific situation. I'll clarify with my reviewers on the CGIDIR variable and the user id creation time with my reviewers.
(In reply to Alexander Moisseev from comment #8) > I.e. what to do instead of pkg-plist lines: > @exec mkdir -p -m 0750 /var/log/BackupPC > @exec chown -R backuppc:backuppc /var/log/BackupPC > @unexec rmdir >/dev/null 2>&1 /var/log/BackupPC || : For now, you can leave it as such. Other ports like puppet [0] seem to keep this syntax still. [0] http://svnweb.freebsd.org/ports/head/sysutils/puppet/pkg-plist?revision=357971&view=markup
Created attachment 144057 [details] Reworked patch - Remove obsolete BPC_DATADIR - Replace BPC_CGIDIR with CGIDIR - Replace mkdir and chown with install -d in pkg-pklist
(In reply to milki from comment #5) > > My reviewers have concerns about config.pl and hosts. If the users modify > the files after install, they may not be overwritten properly by the port > because they would be considered to be dirty files. > I have checked possible scenarios and found no problem with the files. > The suggestion then is to leave the converted config.pl and hosts file with > the .sample suffix and then add a pkg-message that tells the user to > manually copy the files over. > Is it really necessary?
(In reply to Alexander Moisseev from comment #15) > (In reply to milki from comment #5) > > > > My reviewers have concerns about config.pl and hosts. If the users modify > > the files after install, they may not be overwritten properly by the port > > because they would be considered to be dirty files. > > > I have checked possible scenarios and found no problem with the files. Since this seems to work, then let's go with this. I'll run through your newest patch.
Submitted new patch to reviewers. Sorry for the delay.
A commit references this bug: Author: milki Date: Mon Jul 14 01:29:27 UTC 2014 New revision: 361729 URL: http://svnweb.freebsd.org/changeset/ports/361729 Log: Bump sysutils/backuppc to 3.3.0_1 - Support StageDir - Add CONFLICTS (prepare to new port sysutils/backuppc-devel) - Add LICENSE - BackupPC.pod: add =encoding declaration (fix build with perl5.18) - Add DOCS options PR: 189799 Submitted by: Alexander Moisseev <moiseev@mezonplus.ru> (maintainer) Reviewed by: swills (mentor) Changes: head/sysutils/backuppc/Makefile head/sysutils/backuppc/files/patch-configure.pl head/sysutils/backuppc/files/patch-doc-BackupPC.pod head/sysutils/backuppc/files/pkg-deinstall.in head/sysutils/backuppc/files/pkg-message.in head/sysutils/backuppc/pkg-plist
Thanks! Committed with minor patches. Now we can look at bug #183241 as well.
I fixed some typos in the latest commit. Please take a look at the bug #191860. After it will be reviewed, I'll modify accordingly the patch for the bug #183241.