Summary: | new port: sysutils/swapmon: monitor swapusage, add swapspace as needed | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | Ports & Packages | Reporter: | freebsd | ||||||||||||
Component: | Individual Port(s) | Assignee: | Joseph S. Atkinson <jsa> | ||||||||||||
Status: | Closed FIXED | ||||||||||||||
Severity: | Affects Only Me | CC: | freebsd | ||||||||||||
Priority: | Normal | ||||||||||||||
Version: | Latest | ||||||||||||||
Hardware: | Any | ||||||||||||||
OS: | Any | ||||||||||||||
Attachments: |
|
Description
freebsd
2010-07-17 18:20:06 UTC
Responsible Changed From-To: freebsd-ports-bugs->jsa I'll take it. Really great idea. I have a couple of suggestions though. You use post-patch install target to generate a Makefile to do the install. Instead, consider just using a do-install target. Take a look at ports-mgmt/portmaster for a really good example of a shell script installed from ports. Technically, you don't even need a DISTFILE unless you absolutely desire. You could drop the script and manpage under files/. FYI the $FreeBSD: ports/sysutils/swapmon/Makefile,v 1.0 Exp $ line should always be $FreeBSD$ at start. FreeBSD's CVS will automatically expand this to track the version, i.e. it will get converted to show v1.0 on commit. I have already fixed this locally, as well as the extra 'e' in the COMMENT line. A couple of comments about port itself. Since it can only really be run as root, you might consider putting a check in the script to make sure it has the proper permissions to write to /usr/.swap, mdconfig, and swapon then complain out if it cannot ("Must be run as root"). Also, your copyright line in the sbin/swapmon script should have the year of your script's creation as well as your name, i.e. Copyright (C) 2010 Your Name Here, not 1992-2010 The FreeBSD Project. Thanks a bunch for your invaluable feedback! Attached an updated version. I believe I addressed all things you mentioned except that I do not explicitly check if I can execute mdconfig and swapon. Instead I verify that `id -u` == 0. I think I'll implement a testrun during startup in the next version. ;) Let me know if you see anything else. Alex. ----- Message from jsa@FreeBSD.org --------- Date: Thu, 05 Aug 2010 10:51:22 -0400 From: "Joseph S. Atkinson" <jsa@FreeBSD.org> Subject: Re: ports/148711: new port: sysutils/swapmon: monitor swapusage, add swapspace as needed To: bug-followup@FreeBSD.org, freebsd@nagilum.org > Really great idea. I have a couple of suggestions though. > > You use post-patch install target to generate a Makefile to do the install. > Instead, consider just using a do-install target. Take a look at > ports-mgmt/portmaster for a really good example of a shell script > installed from > ports. Technically, you don't even need a DISTFILE unless you > absolutely desire. > You could drop the script and manpage under files/. > > FYI the $FreeBSD: ports/sysutils/swapmon/Makefile,v 1.0 Exp $ line > should always > be $FreeBSD$ at start. FreeBSD's CVS will automatically expand this > to track the > version, i.e. it will get converted to show v1.0 on commit. I have > already fixed > this locally, as well as the extra 'e' in the COMMENT line. > > A couple of comments about port itself. > > Since it can only really be run as root, you might consider putting > a check in > the script to make sure it has the proper permissions to write to /usr/.swap, > mdconfig, and swapon then complain out if it cannot ("Must be run as root"). > > Also, your copyright line in the sbin/swapmon script should have the year of > your script's creation as well as your name, i.e. Copyright (C) 2010 > Your Name > Here, not 1992-2010 The FreeBSD Project. > ----- End message from jsa@FreeBSD.org ----- ======================================================================== # _ __ _ __ http://www.nagilum.org/ \n icq://69646724 # # / |/ /__ ____ _(_) /_ ____ _ nagilum@nagilum.org \n +491776461165 # # / / _ `/ _ `/ / / // / ' \ Amiga (68k/PPC): AOS/NetBSD/Linux # # /_/|_/\_,_/\_, /_/_/\_,_/_/_/_/ Mac (PPC): MacOS-X / Linux / MacOS-X # # /___/ x86: FreeBSD/Linux/Solaris/Win2k ARM9: EPOC EV6 # ======================================================================== ---------------------------------------------------------------- cakebox.homeunix.net - all the machine one needs.. On 08/05/2010 16:14, Alexander wrote: > Thanks a bunch for your invaluable feedback! > Attached an updated version. > I believe I addressed all things you mentioned except that I do not explicitly > check if I can execute mdconfig and swapon. Instead I verify that `id -u` == 0. > I think I'll implement a testrun during startup in the next version. ;) > Let me know if you see anything else. > Alex. This one is important. In order to function as an rc.d script proper, you should install a wrapper in etc/rc.d instead and have it look for swapmon_enable="YES" (will be NO by default) in rc.conf. These are actually small, specially formatted shell scripts themselves. These go under files/ as well in for from of files/${PORTNAME}.in typically and installed via a simple USE_RC_SUBR= ${PORTNAME} directive. A good simple one can be found in multimedia/ffmpeg/files/ffserver.in as a reference. Address this and I think this will be good to good to go. > > ----- Message from jsa@FreeBSD.org --------- > Date: Thu, 05 Aug 2010 10:51:22 -0400 > From: "Joseph S. Atkinson" <jsa@FreeBSD.org> > Subject: Re: ports/148711: new port: sysutils/swapmon: monitor swapusage, add > swapspace as needed > To: bug-followup@FreeBSD.org, freebsd@nagilum.org > > >> Really great idea. I have a couple of suggestions though. >> >> You use post-patch install target to generate a Makefile to do the install. >> Instead, consider just using a do-install target. Take a look at >> ports-mgmt/portmaster for a really good example of a shell script installed from >> ports. Technically, you don't even need a DISTFILE unless you absolutely desire. >> You could drop the script and manpage under files/. >> >> FYI the $FreeBSD: ports/sysutils/swapmon/Makefile,v 1.0 Exp $ line should always >> be $FreeBSD$ at start. FreeBSD's CVS will automatically expand this to track the >> version, i.e. it will get converted to show v1.0 on commit. I have already fixed >> this locally, as well as the extra 'e' in the COMMENT line. >> >> A couple of comments about port itself. >> >> Since it can only really be run as root, you might consider putting a check in >> the script to make sure it has the proper permissions to write to /usr/.swap, >> mdconfig, and swapon then complain out if it cannot ("Must be run as root"). >> >> Also, your copyright line in the sbin/swapmon script should have the year of >> your script's creation as well as your name, i.e. Copyright (C) 2010 Your Name >> Here, not 1992-2010 The FreeBSD Project. >> > > > ----- End message from jsa@FreeBSD.org ----- > > > ======================================================================== > # _ __ _ __ http://www.nagilum.org/ \n icq://69646724 # > # / |/ /__ ____ _(_) /_ ____ _ nagilum@nagilum.org \n +491776461165 # > # / / _ `/ _ `/ / / // / ' \ Amiga (68k/PPC): AOS/NetBSD/Linux # > # /_/|_/\_,_/\_, /_/_/\_,_/_/_/_/ Mac (PPC): MacOS-X / Linux / MacOS-X # > # /___/ x86: FreeBSD/Linux/Solaris/Win2k ARM9: EPOC EV6 # > ======================================================================== > > > ---------------------------------------------------------------- > cakebox.homeunix.net - all the machine one needs.. > Thanks again for your advice and hints! I created the wrapper as suggested. There is a bit of homework left but it works and I gotta leave some room for future improvements. ;) Kind regards, Alex. ----- Message from jsa@FreeBSD.org --------- Date: Thu, 05 Aug 2010 20:47:13 -0400 From: "Joseph S. Atkinson" <jsa@FreeBSD.org> Subject: Re: ports/148711: new port: sysutils/swapmon: monitor swapusage, add swapspace as needed To: Alexander <freebsd@nagilum.org> Cc: bug-followup@FreeBSD.org > On 08/05/2010 16:14, Alexander wrote: >> Thanks a bunch for your invaluable feedback! >> Attached an updated version. >> I believe I addressed all things you mentioned except that I do not >> explicitly >> check if I can execute mdconfig and swapon. Instead I verify that >> `id -u` == 0. >> I think I'll implement a testrun during startup in the next version. ;) >> Let me know if you see anything else. >> Alex. > > This one is important. > > In order to function as an rc.d script proper, you should install a > wrapper in > etc/rc.d instead and have it look for swapmon_enable="YES" (will be NO by > default) in rc.conf. > > These are actually small, specially formatted shell scripts > themselves. These > go under files/ as well in for from of files/${PORTNAME}.in typically and > installed via a simple USE_RC_SUBR= ${PORTNAME} directive. A good > simple one can > be found in multimedia/ffmpeg/files/ffserver.in as a reference. > > Address this and I think this will be good to good to go. > >> >> ----- Message from jsa@FreeBSD.org --------- >> Date: Thu, 05 Aug 2010 10:51:22 -0400 >> From: "Joseph S. Atkinson" <jsa@FreeBSD.org> >> Subject: Re: ports/148711: new port: sysutils/swapmon: monitor >> swapusage, add >> swapspace as needed >> To: bug-followup@FreeBSD.org, freebsd@nagilum.org >> >> >>> Really great idea. I have a couple of suggestions though. >>> >>> You use post-patch install target to generate a Makefile to do the install. >>> Instead, consider just using a do-install target. Take a look at >>> ports-mgmt/portmaster for a really good example of a shell script >>> installed from >>> ports. Technically, you don't even need a DISTFILE unless you >>> absolutely desire. >>> You could drop the script and manpage under files/. >>> >>> FYI the $FreeBSD: ports/sysutils/swapmon/Makefile,v 1.0 Exp $ line >>> should always >>> be $FreeBSD$ at start. FreeBSD's CVS will automatically expand >>> this to track the >>> version, i.e. it will get converted to show v1.0 on commit. I have >>> already fixed >>> this locally, as well as the extra 'e' in the COMMENT line. >>> >>> A couple of comments about port itself. >>> >>> Since it can only really be run as root, you might consider >>> putting a check in >>> the script to make sure it has the proper permissions to write to >>> /usr/.swap, >>> mdconfig, and swapon then complain out if it cannot ("Must be run >>> as root"). >>> >>> Also, your copyright line in the sbin/swapmon script should have >>> the year of >>> your script's creation as well as your name, i.e. Copyright (C) >>> 2010 Your Name >>> Here, not 1992-2010 The FreeBSD Project. >>> >> >> >> ----- End message from jsa@FreeBSD.org ----- >> >> >> ======================================================================== >> # _ __ _ __ http://www.nagilum.org/ \n icq://69646724 # >> # / |/ /__ ____ _(_) /_ ____ _ nagilum@nagilum.org \n +491776461165 # >> # / / _ `/ _ `/ / / // / ' \ Amiga (68k/PPC): AOS/NetBSD/Linux # >> # /_/|_/\_,_/\_, /_/_/\_,_/_/_/_/ Mac (PPC): MacOS-X / Linux / MacOS-X # >> # /___/ x86: FreeBSD/Linux/Solaris/Win2k ARM9: EPOC EV6 # >> ======================================================================== >> >> >> ---------------------------------------------------------------- >> cakebox.homeunix.net - all the machine one needs.. >> > ----- End message from jsa@FreeBSD.org ----- ======================================================================== # _ __ _ __ http://www.nagilum.org/ \n icq://69646724 # # / |/ /__ ____ _(_) /_ ____ _ nagilum@nagilum.org \n +491776461165 # # / / _ `/ _ `/ / / // / ' \ Amiga (68k/PPC): AOS/NetBSD/Linux # # /_/|_/\_,_/\_, /_/_/\_,_/_/_/_/ Mac (PPC): MacOS-X / Linux / MacOS-X # # /___/ x86: FreeBSD/Linux/Solaris/Win2k ARM9: EPOC EV6 # ======================================================================== ---------------------------------------------------------------- cakebox.homeunix.net - all the machine one needs.. Ok, another update. This time with proper /etc/rc.subr integration (all the usual commands work). I no longer need a wrapper instead the script itself has become the rc script and $PREFIX/sbin/swapmon is just a symlink to it. Thanks & kind regards, Alex. ----- Message from jsa@FreeBSD.org --------- Date: Thu, 05 Aug 2010 20:47:13 -0400 From: "Joseph S. Atkinson" <jsa@FreeBSD.org> Subject: Re: ports/148711: new port: sysutils/swapmon: monitor swapusage, add swapspace as needed To: Alexander <freebsd@nagilum.org> Cc: bug-followup@FreeBSD.org > On 08/05/2010 16:14, Alexander wrote: >> Thanks a bunch for your invaluable feedback! >> Attached an updated version. >> I believe I addressed all things you mentioned except that I do not >> explicitly >> check if I can execute mdconfig and swapon. Instead I verify that >> `id -u` == 0. >> I think I'll implement a testrun during startup in the next version. ;) >> Let me know if you see anything else. >> Alex. > > This one is important. > > In order to function as an rc.d script proper, you should install a > wrapper in > etc/rc.d instead and have it look for swapmon_enable="YES" (will be NO by > default) in rc.conf. > > These are actually small, specially formatted shell scripts > themselves. These > go under files/ as well in for from of files/${PORTNAME}.in typically and > installed via a simple USE_RC_SUBR= ${PORTNAME} directive. A good > simple one can > be found in multimedia/ffmpeg/files/ffserver.in as a reference. > > Address this and I think this will be good to good to go. > >> >> ----- Message from jsa@FreeBSD.org --------- >> Date: Thu, 05 Aug 2010 10:51:22 -0400 >> From: "Joseph S. Atkinson" <jsa@FreeBSD.org> >> Subject: Re: ports/148711: new port: sysutils/swapmon: monitor >> swapusage, add >> swapspace as needed >> To: bug-followup@FreeBSD.org, freebsd@nagilum.org >> >> >>> Really great idea. I have a couple of suggestions though. >>> >>> You use post-patch install target to generate a Makefile to do the install. >>> Instead, consider just using a do-install target. Take a look at >>> ports-mgmt/portmaster for a really good example of a shell script >>> installed from >>> ports. Technically, you don't even need a DISTFILE unless you >>> absolutely desire. >>> You could drop the script and manpage under files/. >>> >>> FYI the $FreeBSD: ports/sysutils/swapmon/Makefile,v 1.0 Exp $ line >>> should always >>> be $FreeBSD$ at start. FreeBSD's CVS will automatically expand >>> this to track the >>> version, i.e. it will get converted to show v1.0 on commit. I have >>> already fixed >>> this locally, as well as the extra 'e' in the COMMENT line. >>> >>> A couple of comments about port itself. >>> >>> Since it can only really be run as root, you might consider >>> putting a check in >>> the script to make sure it has the proper permissions to write to >>> /usr/.swap, >>> mdconfig, and swapon then complain out if it cannot ("Must be run >>> as root"). >>> >>> Also, your copyright line in the sbin/swapmon script should have >>> the year of >>> your script's creation as well as your name, i.e. Copyright (C) >>> 2010 Your Name >>> Here, not 1992-2010 The FreeBSD Project. >>> >> >> >> ----- End message from jsa@FreeBSD.org ----- >> >> >> ======================================================================== >> # _ __ _ __ http://www.nagilum.org/ \n icq://69646724 # >> # / |/ /__ ____ _(_) /_ ____ _ nagilum@nagilum.org \n +491776461165 # >> # / / _ `/ _ `/ / / // / ' \ Amiga (68k/PPC): AOS/NetBSD/Linux # >> # /_/|_/\_,_/\_, /_/_/\_,_/_/_/_/ Mac (PPC): MacOS-X / Linux / MacOS-X # >> # /___/ x86: FreeBSD/Linux/Solaris/Win2k ARM9: EPOC EV6 # >> ======================================================================== >> >> >> ---------------------------------------------------------------- >> cakebox.homeunix.net - all the machine one needs.. >> > ----- End message from jsa@FreeBSD.org ----- ======================================================================== # _ __ _ __ http://www.nagilum.org/ \n icq://69646724 # # / |/ /__ ____ _(_) /_ ____ _ nagilum@nagilum.org \n +491776461165 # # / / _ `/ _ `/ / / // / ' \ Amiga (68k/PPC): AOS/NetBSD/Linux # # /_/|_/\_,_/\_, /_/_/\_,_/_/_/_/ Mac (PPC): MacOS-X / Linux / MacOS-X # # /___/ x86: FreeBSD/Linux/Solaris/Win2k ARM9: EPOC EV6 # ======================================================================== ---------------------------------------------------------------- cakebox.homeunix.net - all the machine one needs.. Sorry for the delay. Same result as the 3rd version, so let me explain what is at issue. http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/rc-scripts.html The way an rc.d script is supposed to work is to be as minimal as needed to control another process. Primarily starting and stopping it. These functions your port does correctly. However, the issue is central to the sbin/ script. As with the second version (that was about ready for commit when you sent me the third), the script itself should reside in sbin/. It seems that in order to simplify this, it's become more complicated than it needs be, i.e. the unnecessary subr. You are currently putting in the rc.d script, including swapmon.subr, and then symlinking the rc.d script to to sbin/. It would silence the critics if you were to revert the current method and to place the swapmon script back in sbin, then allow the rc.d script to do simple start/stop toggling. This was the technically correct approach, and I apologize for not being more explicit in explaining this prior to this attempt. Sorry that this seems so complicated, and I really like this port. I am eager to get this into the ports tree so I can use it on a few machines where ram+swap isn't enough for some intense process (like building OOo). Oh, and note, USE_RC_SUBR should be above PLIST_FILES. I've been moving that line for you. ;) Thanks for the explanation! How about this version then? ----- Message from jsa@FreeBSD.org --------- Date: Sat, 14 Aug 2010 21:49:35 -0400 From: "Joseph S. Atkinson" <jsa@FreeBSD.org> Subject: Re: ports/148711: new port: sysutils/swapmon: monitor swapusage, add swapspace as needed To: Alexander <freebsd@nagilum.org> Cc: bug-followup@FreeBSD.org > Sorry for the delay. > > Same result as the 3rd version, so let me explain what is at issue. > > http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/rc-scripts.html > > The way an rc.d script is supposed to work is to be as minimal as needed to > control another process. Primarily starting and stopping it. These functions > your port does correctly. However, the issue is central to the sbin/ > script. As > with the second version (that was about ready for commit when you sent me the > third), the script itself should reside in sbin/. It seems that in order to > simplify this, it's become more complicated than it needs be, i.e. the > unnecessary subr. You are currently putting in the rc.d script, including > swapmon.subr, and then symlinking the rc.d script to to sbin/. > > It would silence the critics if you were to revert the current method and to > place the swapmon script back in sbin, then allow the rc.d script to > do simple > start/stop toggling. This was the technically correct approach, and > I apologize > for not being more explicit in explaining this prior to this attempt. > > Sorry that this seems so complicated, and I really like this port. I > am eager to > get this into the ports tree so I can use it on a few machines where ram+swap > isn't enough for some intense process (like building OOo). > > Oh, and note, USE_RC_SUBR should be above PLIST_FILES. I've been moving that > line for you. ;) > ----- End message from jsa@FreeBSD.org ----- ======================================================================== # _ __ _ __ http://www.nagilum.org/ \n icq://69646724 # # / |/ /__ ____ _(_) /_ ____ _ nagilum@nagilum.org \n +491776461165 # # / / _ `/ _ `/ / / // / ' \ Amiga (68k/PPC): AOS/NetBSD/Linux # # /_/|_/\_,_/\_, /_/_/\_,_/_/_/_/ Mac (PPC): MacOS-X / Linux / MacOS-X # # /___/ x86: FreeBSD/Linux/Solaris/Win2k ARM9: EPOC EV6 # ======================================================================== ---------------------------------------------------------------- cakebox.homeunix.net - all the machine one needs.. Version1.4, changes: - do a testrun instead of simply demanding UID==0, do warn however - use variables for repeated commands (ie. RM=/bin/rm ; $RM foo) ======================================================================== # _ __ _ __ http://www.nagilum.org/ \n icq://69646724 # # / |/ /__ ____ _(_) /_ ____ _ nagilum@nagilum.org \n +491776461165 # # / / _ `/ _ `/ / / // / ' \ Amiga (68k/PPC): AOS/NetBSD/Linux # # /_/|_/\_,_/\_, /_/_/\_,_/_/_/_/ Mac (PPC): MacOS-X / NetBSD /Linux # # /___/ x86: FreeBSD/Linux/Solaris/Win2k ARM9: EPOC EV6 # ======================================================================== ---------------------------------------------------------------- cakebox.homeunix.net - all the machine one needs.. jsa 2010-08-23 02:40:15 UTC FreeBSD ports repository Modified files: sysutils Makefile Added files: sysutils/swapmon Makefile pkg-descr sysutils/swapmon/files swapmon.1 swapmon.in swapmon.sh.in Log: Introducing sysutils/swapmon. This port attempts to monitor swap usage and dynamically add a swapfile as neccessary. PR: ports/148711 Submitted by: Alexander Kuehn <freebsd@nagilum.org> Approved by: wxs (mentor) Revision Changes Path 1.1221 +1 -0 ports/sysutils/Makefile 1.1 +40 -0 ports/sysutils/swapmon/Makefile (new) 1.1 +88 -0 ports/sysutils/swapmon/files/swapmon.1 (new) 1.1 +33 -0 ports/sysutils/swapmon/files/swapmon.in (new) 1.1 +187 -0 ports/sysutils/swapmon/files/swapmon.sh.in (new) 1.1 +9 -0 ports/sysutils/swapmon/pkg-descr (new) _______________________________________________ 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! |