Does this look right? zxfer port

I'm trying to port my script zxfer. If it's ok, I'd like to get some more expert opinion if I'm right to submit it. I've been through the porting handbook. If you want me to post the pkg-descr, I will.

So far I seem to be getting good output for ports-mgmt/portlint and ports-mgmt/porttools .i.e.
# port test

The only real question I have is the do-build line, as if I remove it I get an error, though as I said portlint etc. say that it "looks fine". Other than that, am I right to submit it?

Note I used the ports-mgmt/portmaster Makefile as an example.

Code:
# New ports collection makefile for:    zxfer
# Date created:                         31 March 2011
# Whom:                                 Ivan Dreckman
#                                       <ivannashdreckman@fastmail.fm>
#
# $FreeBSD$
#

PORTNAME=       zxfer
PORTVERSION=    0.9.5
CATEGORIES=     sysutils
MASTER_SITES=   ${MASTER_SITE_GOOGLE_CODE}

MAINTAINER=     ivannashdreckman@fastmail.fm
COMMENT=        Easily and reliably transfer ZFS filesystems

USE_BZIP2=      yes

LICENSE=        BSD

PLIST_FILES=    sbin/zxfer

MAN8=           zxfer.8
MANCOMPRESSED=  yes

.include <bsd.port.pre.mk>

do-build:

do-install:
        ${INSTALL_SCRIPT} ${WRKDIR}/zxfer-${PORTVERSION}/zxfer \
${PREFIX}/sbin/zxfer
        ${INSTALL_MAN} ${WRKDIR}/zxfer-${PORTVERSION}/zxfer.8.gz \
${MAN8PREFIX}/man/man8

.include <bsd.port.post.mk>
 
Thank you. So now the end looks like (and it is tested to work):
Code:
USE_BZIP2=      yes
NO_BUILD=       yes

LICENSE=        BSD

PLIST_FILES=    sbin/zxfer

MAN8=           zxfer.8
MANCOMPRESSED=  yes

.include <bsd.port.pre.mk>

do-install:
        ${INSTALL_SCRIPT} ${WRKDIR}/zxfer-${PORTVERSION}/zxfer \
${PREFIX}/sbin/zxfer
        ${INSTALL_MAN} ${WRKDIR}/zxfer-${PORTVERSION}/zxfer.8.gz \
${MAN8PREFIX}/man/man8

.include <bsd.port.post.mk>
 
A few cosmetic nits:
  • Only use bsd.port.(pre|post).mk if you have to, otherwise it'd complicate port needlessly.
  • Use WRKSRC instead of inventing your own.
  • Be careful with absolute paths. Consider a case when PREFIX != LOCALBASE != /usr/local.
  • Don't forget to check the source. zxfer needs gawk and rsync for rsync mode, lang/gawk pieces can sometimes be replaced with/rewritten as awk(1).
Code:
@@ -9,7 +9,7 @@
 PORTNAME=	zxfer
 PORTVERSION=	0.9.5
 CATEGORIES=	sysutils
-MASTER_SITES=	${MASTER_SITE_GOOGLE_CODE}
+MASTER_SITES=	GOOGLE_CODE
 
 MAINTAINER=	ivannashdreckman@fastmail.fm
 COMMENT=	Easily and reliably transfer ZFS filesystems
@@ -24,12 +24,13 @@ PLIST_FILES=	sbin/zxfer
 MAN8=		zxfer.8
 MANCOMPRESSED=	yes
 
-.include <bsd.port.pre.mk>
+post-patch:
+	@${REINPLACE_CMD} -e 's/gawk/awk/' \
+		-e 's|/usr/local|${LOCALBASE}|g' \
+		${WRKSRC}/zxfer
 
 do-install:
-	${INSTALL_SCRIPT} ${WRKDIR}/zxfer-${PORTVERSION}/zxfer \
-${PREFIX}/sbin/zxfer
-	${INSTALL_MAN} ${WRKDIR}/zxfer-${PORTVERSION}/zxfer.8.gz \
-${MAN8PREFIX}/man/man8
+	${INSTALL_SCRIPT} ${WRKSRC}/zxfer ${PREFIX}/sbin
+	${INSTALL_MAN} ${WRKSRC}/zxfer.8.gz ${MAN8PREFIX}/man/man8
 
-.include <bsd.port.post.mk>
+.include <bsd.port.mk>
 
dandelion said:
A few cosmetic nits:
  • Only use bsd.port.(pre|post).mk if you have to, otherwise it'd complicate port needlessly.
  • Use WRKSRC instead of inventing your own.
  • Be careful with absolute paths. Consider a case when PREFIX != LOCALBASE != /usr/local.
  • Don't forget to check the source. zxfer needs gawk and rsync for rsync mode, lang/gawk pieces can sometimes be replaced with/rewritten as awk(1).

Thanks for that, dandelion! I'll patch it and see how I go.

Yeah, you are right about rsync, I need to include that as a dependency, I'll have to learn how to do that. And how did you figure out that I needed gawk? Something else probably installed that on my system, and I just figured that I could use it. I'll see if I can rewrite it as awk.
 
I just checked my notes as I developed zxfer, and I realized that I actually had to change awk to gawk in order to get it to work. Since I think it's important for the script to be able to run (at least, without the rsync functionality) during a restore before you might have or want any internet connectivity, I'll have to figure out why it didn't work under awk and figure out some way of doing the same thing without creating any additional dependencies.

Edit: I figured out that gawk is only necessary under Solaris, so I just tested for it via uname so that it can be awk in FreeBSD and gawk in Solaris.
 
Ok, I've fixed it so that only rsync is a runtime dependency in the port. I've also made a new version so that gawk is only used with Solaris, so it is no longer a dependency.

Here is what the (hopefully) end result is. Very simple.
Code:
# New ports collection makefile for:    zxfer
# Date created:                         02 April 2011
# Whom:                                 Ivan Dreckman
#                                       <ivannashdreckman@fastmail.fm>
#
# $FreeBSD$
#

PORTNAME=       zxfer
PORTVERSION=    0.9.6
CATEGORIES=     sysutils
MASTER_SITES=   GOOGLE_CODE

MAINTAINER=     ivannashdreckman@fastmail.fm
COMMENT=        Easily and reliably transfer ZFS filesystems

RUN_DEPENDS=    ${LOCALBASE}/bin/rsync:${PORTSDIR}/net/rsync

USE_BZIP2=      yes
NO_BUILD=       yes

LICENSE=        BSD

PLIST_FILES=    sbin/zxfer

MAN8=           zxfer.8
MANCOMPRESSED=  yes

do-install:
        ${INSTALL_SCRIPT} ${WRKSRC}/zxfer ${PREFIX}/sbin
        ${INSTALL_MAN} ${WRKSRC}/zxfer.8.gz ${MAN8PREFIX}/man/man8

.include <bsd.port.mk>
Now to submit it. I wonder if I can use this?

Edit: I used the web form, only trick is to rename the .shar file to .shar.txt.
 
RUN_DEPENDS doesn't require a path for the binary if it's in the search path.
Code:
RUN_DEPENDS=    rsync:${PORTSDIR}/net/rsync
rsync dependencies are done both ways in ports, but the majority are without the path.
% find /usr/ports -depth 3 -name Makefile -exec grep -H rsync: {} \+ | less
 
wblock said:
RUN_DEPENDS doesn't require a path for the binary if it's in the search path.
Code:
RUN_DEPENDS=    rsync:${PORTSDIR}/net/rsync
rsync dependencies are done both ways in ports, but the majority are without the path.
% find /usr/ports -depth 3 -name Makefile -exec grep -H rsync: {} \+ | less
Thanks. You are right, only 12.5% of ports are done the way I did it. Thanks for the tip! It's now looking very streamlined, thanks everyone!

Hopefully it gets committed before I release my howtos on installing, backing up and restoring a very reliable FreeBSD system with ZFS root mirror and ZFS storage mirror, since I wrote it to perform the backup and restore function in that system. But if it doesn't, it's still very easy to install, consisting only of a script and a man page to copy to the correct locations.
 
My point about /usr/local still stands. For example, on my box LOCALBASE=/usr/pkg, i.e. it's intentionally same as default for pkgsrc. And so zxfer can't find rsync even though it's installed. You either need to use REINPLACE_CMD as I've showed you above or rewrite the app to depend on proper PATH more.
 
dandelion said:
My point about /usr/local still stands. For example, on my box LOCALBASE=/usr/pkg, i.e. it's intentionally same as default for pkgsrc. And so zxfer can't find rsync even though it's installed. You either need to use REINPLACE_CMD as I've showed you above or rewrite the app to depend on proper PATH more.
Thanks for your help, dandelion.

I'm a bit confused here. Are you talking about the port, as in "zxfer can't find rsync..."? As it stands, the script goes through /usr/local/bin,/usr/local/sbin,/usr,/usr/sfw,/opt,/usr/bin and goes with the first location that it can find it in. I've amended the port to be like so:
Code:
# New ports collection makefile for:    zxfer
# Date created:                         02 April 2011
# Whom:                                 Ivan Dreckman
#                                       <ivannashdreckman@fastmail.fm>
#
# $FreeBSD$
#

PORTNAME=       zxfer
PORTVERSION=    0.9.6
CATEGORIES=     sysutils
MASTER_SITES=   GOOGLE_CODE

MAINTAINER=     ivannashdreckman@fastmail.fm
COMMENT=        Easily and reliably transfer ZFS filesystems

RUN_DEPENDS=    rsync:${PORTSDIR}/net/rsync

USE_BZIP2=      yes
NO_BUILD=       yes

LICENSE=        BSD

PLIST_FILES=    sbin/zxfer

MAN8=           zxfer.8
MANCOMPRESSED=  yes

do-install:
        ${INSTALL_SCRIPT} ${WRKSRC}/zxfer ${PREFIX}/sbin
        ${INSTALL_MAN} ${WRKSRC}/zxfer.8.gz ${MAN8PREFIX}/man/man8

.include <bsd.port.mk>
I can't remember whether I submitted that version, or the previous. I thought rather than confusing the list with two submissions, I'd wait until it either got included in ports or I got a response back.

So, is there a problem still? I'm not sure.
 
dandelion said:
RUN_DEPENDS doesn't silently patch the app.
Code:
$ zxfer -S foo
Error: Can't find rsync. Zxfer expects to find rsync in one of the following locations:
/usr/local/bin /usr/local/sbin /usr /usr/sfw /opt /usr/bin
Exit 1
$ rsync --version | head -1
rsync  version 3.0.7  protocol version 30
Ok... so what you are saying is that you can configure your system so that it is other than default, and when you install the port of rsync, it will install the rsync binary to wherever you specify, which is not in one of those possible places I listed, I take it. And since you can define whatever that place should be, then it makes no sense to list a bunch of directories to scan through in the hope of finding rsync, yes?

Hmmm. How to solve. Note that since the script should work out of the box with Solaris, I could not simply subsitute gawk with awk as is done with your suggestion. Well, I could I suppose, but then if another operating system comes along to use zxfer, then using awk as a default would be better.

But I think I see how you have solved it. You are going through zxfer to look for /usr/local, and replacing that with ${LOCALBASE}, correct? That should work.

+post-patch:
+ @${REINPLACE_CMD} -e 's/gawk/awk/' \
+ -e 's|/usr/local|${LOCALBASE}|g' \
+ ${WRKSRC}/zxfer
So, a couple questions:

1. if I put in a lines like:
+post-patch:
+ @${REINPLACE_CMD} \
+ -e 's|/usr/local|${LOCALBASE}|g' \
+ ${WRKSRC}/zxfer
Will that be effective? And it should be just after the MANCOMPRESSED line?

2. Should I wait until the port is accepted, or should I submit this port and say something to the effect of (use this instead of submitted)? Or what exactly?

Thanks for alerting me to these issues, dandelion, and apologies for skimming instead of reading, though not sure I would have understood the first time around even if I had read, tbh.
 
carlton_draught said:
How to solve. Note that since the script should work out of the box with Solaris, I could not simply subsitute gawk with awk as is done with your suggestion. Well, I could I suppose, but then if another operating system comes along to use zxfer, then using awk as a default would be better.
Why not use alias or function?
Code:
@@ -138,7 +138,6 @@ LZFS="/sbin/zfs"
 RZFS="/sbin/zfs"
 
 LCAT=""
-HAWK="/usr/bin/awk"   # location of awk or gawk on home OS
 
 rsync_locations="/usr/local/bin,/usr/local/sbin,/usr,/usr/sfw,/opt,/usr/bin"
 cat_locations="/bin,/usr/bin"
@@ -267,7 +266,7 @@ init_variables() {
   fi
 
   if [ "$home_os" = "SunOS" ]; then
-    HAWK="/usr/bin/gawk"
+    alias awk=gawk || awk() { command gawk "$@"; }
   fi
  }
 
@@ -679,7 +678,7 @@ get_zfs_list_rsync_mode() {
     if [ "$option_L" != "" ];then
       for fs in $temp_fs_list_comma; do
         # count the "/" in the line, should be equal or greater to option_L
-	slash_no=$(echo "$fs" | $HAWK '$0= NF-1' FS=/)
+	slash_no=$(echo "$fs" | awk '$0= NF-1' FS=/)
 	if [ "$slash_no" -lt "$option_L" ]; then
 	  echo "Error: If using option L, ensure that all source files and\
  directories are contained in filesystems with as many \"/\" as L."
@@ -687,7 +686,7 @@ get_zfs_list_rsync_mode() {
 	  exit 1
 	fi
 	old_root_fs=$root_fs
-	root_fs=$(echo "$fs" | $HAWK '{for (i=1; i <= '$inc_option_L'; i++) printf("%s%c", $i, (i=='$inc_option_L')?ORS:OFS)}' FS=/ OFS=/ ORS=)
+	root_fs=$(echo "$fs" | awk '{for (i=1; i <= '$inc_option_L'; i++) printf("%s%c", $i, (i=='$inc_option_L')?ORS:OFS)}' FS=/ OFS=/ ORS=)
         if [ "$root_fs" != "$old_root_fs" -a "$old_root_fs" != "" ]; then
 	  echo "Error: No common root filesystem. If using option L, ensure \
 that each source file/directory comes from a common filesystem, and that the
And if it's not enough you can modify PATH inside the wrapper function. So, unless some platforms have wrong PATH set by default you can eliminate get_path().
carlton_draught said:
But I think I see how you have solved it. You are going through zxfer to look for /usr/local, and replacing that with ${LOCALBASE}, correct? That should work.
Yep, but it's ports-specific. A portable way is to use PATH and/or command wrappers.
carlton_draught said:
1. if I put in a lines like:

Will that be effective? And it should be just after the MANCOMPRESSED line?
Only if LOCALBASE/sbin is in the PATH but this is required for using ports, except for framework itself.
carlton_draught said:
2. Should I wait until the port is accepted, or should I submit this port and say something to the effect of (use this instead of submitted)? Or what exactly?
As this is a low profile fix I'd just submit a small diff.
 
Thanks dandelion for your input. You are very terse. I'm trying to understand what you say, and I think I'm learning. My shell scripting is basically self taught. I'm sure there are quite a few niceties that I need to pick up in order to be a better shell scripter.
dandelion said:
Why not use alias or function?
Why not alias? As a positive, it's quick and terse. As a negative, when something is overridden like that, it's easy to forget what is happening, which may be hard to debug and have unintended consequences. At least when you use a variable, it is explicit what you are doing. I would prefer to use a function. I might copy phoenix's style, and his use of which. e.g.
Code:
awk=$( ${which} awk )
Can you see any problems with doing that?
 
Ok, I've gotten rid of the get_path() function, and now use which to do the same job, which is much more elegant. So am I correct that there should now be no use for the following in the port? Now /usr/local does not even exist in the script.

Code:
+post-patch:
+ @${REINPLACE_CMD} \
+ -e 's|/usr/local|${LOCALBASE}|g' \
+ ${WRKSRC}/zxfer
 
Back
Top