Allow users to supply options to cvsupd without modifying the startup script. For example, my cvsup server hosts both the FreeBSD collection and my own by adding this entry to /etc/rc.conf: cvsupd_flags="-c sup:/home/repositories/sup"
Class Changed From-To: maintainer-update->change-request Fix category (submitter is not maintainer)
Responsible Changed From-To: freebsd-ports-bugs->jdp Over to maintainer
drnez suggested that the default be set to the following "-e -C ${maxclients:-8} -l @${facility:-daemon} -b ${base:- /home/ncvs} -s sup.client" so that users could override any options they wanted...
Responsible Changed From-To: jdp->freebsd-ports-bugs jdp has turned in his commit bit.
Responsible Changed From-To: freebsd-ports-bugs->wxs I'll take it.
wxs 2008-03-13 21:31:02 UTC FreeBSD ports repository Modified files: net/cvsup-mirror Makefile net/cvsup-mirror/files cvsupd.sh.in Log: RC script now supports _flags variable. Comments available in the RC script. PR: ports/108847 Submitted by: Dan Langille <dan@langille.org> Approved by: garga (mentor) Revision Changes Path 1.28 +2 -2 ports/net/cvsup-mirror/Makefile 1.3 +18 -3 ports/net/cvsup-mirror/files/cvsupd.sh.in _______________________________________________ cvs-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/cvs-all To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
State Changed From-To: open->closed Committed, with minor changes. Thanks!
Hmm, cvsupd_flags worked perfectly well before this patch to add additional flags, such as -A to bind to a specific IP on a jail host... This flag breaks everything and makes one use special magic in rc.conf to fix it, without any benefit, since the only thing it allows you to configure is the collection, which is hard coded in the rest of the port. Please consider backing this out and adding a 'cvsupd_coll' variable which defaults to 'sup.client', or better still allowing one to specify the collection in the port setup? Regards, -Jeremy -- FreeBSD - Because the best things in life are free... http://www.freebsd.org/
Jeremy Lea wrote: > Hmm, cvsupd_flags worked perfectly well before this patch to add > additional flags, such as -A to bind to a specific IP on a jail host... It did? I saw no reference at all to cvsupd_flags in the startup script. What have I missed? > This flag breaks everything and makes one use special magic in rc.conf > to fix it, without any benefit, since the only thing it allows you to > configure is the collection, which is hard coded in the rest of the > port. Breaks everything? What do you mean? Special magic? What are you referring to? Isn't rc.conf the standard place for specifying flags? > Please consider backing this out and adding a 'cvsupd_coll' variable > which defaults to 'sup.client', or better still allowing one to specify > the collection in the port setup? How are collections related to this PR?
On Mon, Mar 31, 2008 at 10:10:26PM -0800, Jeremy Lea wrote: > Hmm, cvsupd_flags worked perfectly well before this patch to add > additional flags, such as -A to bind to a specific IP on a jail host... And it continues to work perfectly well now, just cleaner and documented. Previously it would append the undocumented and hardcoded flags to the end of the command line. Now, it does not do that and the defaults are documented in the script. > This flag breaks everything and makes one use special magic in rc.conf > to fix it, without any benefit, since the only thing it allows you to > configure is the collection, which is hard coded in the rest of the > port. You can configure a lot more than the collection. For example, I tested configuration with the number of simultaneous connections, among others. > Please consider backing this out and adding a 'cvsupd_coll' variable > which defaults to 'sup.client', or better still allowing one to specify > the collection in the port setup? I don't see what's broken with this update. Would you be willing to provide more details as to what your configuration looks like and how you are using cvsupd and cvsupd_flags in both the new and old versions? -- WXS
Hi, Replying to both in one email... On Tue, 01 Apr 2008 08:19:41 -0400, Dan Langille wrote: > Jeremy Lea wrote: > > Hmm, cvsupd_flags worked perfectly well before this patch to add > > additional flags, such as -A to bind to a specific IP on a jail host... > > It did? I saw no reference at all to cvsupd_flags in the startup > script. What have I missed? foobar_flags is always added by rc.subr. It must be otherwise this patch wouldn't work! It is added in addition to command_args. Although looking more closely at the patch, I see this was not your idea... > > This flag breaks everything and makes one use special magic in rc.conf > > to fix it, without any benefit, since the only thing it allows you to > > configure is the collection, which is hard coded in the rest of the > > port. > > Breaks everything? What do you mean? With this you now have to include /usr/local/etc/cvsup/config.sh into rc.conf to get the values to put into the cvsupd_flags. If, you, like me, had just '-A xx.xx.xx.xx' in cvsupd_flags, then when you rebooted, cvsupd didn't go into the background on startup, and the startup scripts hung up and you had to go and find your remotely located box and fix it in person, and, then find that it still didn't work, because the defaults are wrong, and, and, and, and then you write angry over-reactionary responses to PRs... > Special magic? What are you referring to? Isn't rc.conf the standard > place for specifying flags? Yes, and it worked great. > > Please consider backing this out and adding a 'cvsupd_coll' variable > > which defaults to 'sup.client', or better still allowing one to specify > > the collection in the port setup? > > How are collections related to this PR? Sorry, I misunderstood your original PR. I thought you wanted to overload the -s flag not the -c flag. For what you want, just adding: cvsupd_flags="-c sup:/home/repositories/sup" to rc.conf should have worked fine without any changes to the port. On Wed, Apr 02, 2008 at 01:59:02PM -0400, Wesley Shields wrote: > On Mon, Mar 31, 2008 at 10:10:26PM -0800, Jeremy Lea wrote: > > Hmm, cvsupd_flags worked perfectly well before this patch to add > > additional flags, such as -A to bind to a specific IP on a jail host... > > And it continues to work perfectly well now, just cleaner and documented. > Previously it would append the undocumented and hardcoded flags to the > end of the command line. Now, it does not do that and the defaults are > documented in the script. No, it broke existing installations. The only hardcoded flags were '-e' which you always want if you're starting in rc.d and '-s sup.client' which is hardcoded by the port. The rest of the flags are coded in config.sh, and can be set just fine. BTW, the hard coded fallback for -b should be '/usr/local/etc/cvsup' not '/home/ncvs'. > > This flag breaks everything and makes one use special magic in rc.conf > > to fix it, without any benefit, since the only thing it allows you to > > configure is the collection, which is hard coded in the rest of the > > port. > > You can configure a lot more than the collection. For example, I tested > configuration with the number of simultaneous connections, among others. And you can do that just fine by editting config.sh. > > Please consider backing this out and adding a 'cvsupd_coll' variable > > which defaults to 'sup.client', or better still allowing one to specify > > the collection in the port setup? > > I don't see what's broken with this update. Would you be willing to > provide more details as to what your configuration looks like and how > you are using cvsupd and cvsupd_flags in both the new and old versions? I have a machine which hosts some jails with different worlds to the kernel. I have a cvsup-mirror on the host, so that I only pull /home/ncvs once. Then I can use it for csup updates of different src trees, and also for CVS development (not that I'm doing any of that these days). With jail hosts you have to limit the host services to the hosts IP, not the aliases. You could do this quickly for cvsupd by adding the '-A' flag to cvsupd_flags in rc.conf. Now you have to copy the include of config.sh to get the right values, then copy cvsupd_flags out of the script and then add -A to it. Please consider using the patch below. Regards, -Jeremy Index: cvsupd.sh.in =================================================================== RCS file: /home/ncvs/ports/net/cvsup-mirror/files/cvsupd.sh.in,v retrieving revision 1.3 diff -u -r1.3 cvsupd.sh.in --- cvsupd.sh.in 13 Mar 2008 21:31:02 -0000 1.3 +++ cvsupd.sh.in 2 Apr 2008 19:43:25 -0000 @@ -12,21 +12,6 @@ # #cvsupd_enable="YES" -# -# Flag settings are now able to be overridden in /etc/rc.conf. The order -# of priority is: -# -# Settings found in /etc/rc.conf are used (cvsupd_flags) -# If nothing is found in /etc/rc.conf then settings found in -# %%PREFIX%%/etc/cvsup/config.sh are used. -# If nothing is found in %%PREFIX%%/etc/cvsup/config.sh -# then the hardcoded values below are used. -# -#cvsupd_flags="-e -C 8 -l @daemon -b /home/ncvs -s sup.client" -# -# Note that answering the questions during the install will setup -# %%PREFIX%%/etc/cvsup/config.sh for you. - . %%RC_SUBR%% name="cvsupd" @@ -45,10 +30,19 @@ : ${cvsupd_enable:=NO} : ${cvsupd_outfile=/var/run/${name}.out} : ${cvsupd_user:=${user:-cvsup}} -: ${cvsupd_flags:="-e -C ${maxclients:-8} -l @${facility:-daemon} \ - -b ${base:-/home/ncvs} -s sup.client"} +# +# Additional command line arguments can be added by setting cvsupd_flags in +# /etc/rc.conf. The other values here are obtained from +# %%PREFIX%%/etc/cvsup/config.sh, and can be set by there. +# +# Some useful additional flags are '-A xx.xx.xx.xx' to bind cvsupd to only +# one address, and '-c sup:/somewhere/sup' to serve local collections in +# addition to the mirrored FreeBSD collections. +# command="%%PREFIX%%/sbin/cvsupd" +command_args="-e -C ${maxclients:-8} -l @${facility:-daemon} \ + -b ${base} -s sup.client" stop_cmd="cvsupd_stop" cvsupd_stop() { -- FreeBSD - Because the best things in life are free... http://www.freebsd.org/