Bug 191562

Created attachment 144343 [details]
rundeck.shar

This one also needs correct GID and UID in /usr/ports/UIDs and /usr/ports/GIDs. I don't know where I can about them.

Comment 3 John Marino 2014-07-31 16:09:03 UTC

(In reply to arcade from comment #2)
> This one also needs correct GID and UID in /usr/ports/UIDs and
> /usr/ports/GIDs. I don't know where I can about them.

So those need a patch too.

Comment 4 John Marino 2014-07-31 16:10:15 UTC

After providing a GIDs and UIDs patch which reserve the actual IDs, can you provide test logs from poudriere or redports or at least the output from "make check-plist" followed by "make stage-qa" ?

Comment 5 John Marino 2014-07-31 16:35:09 UTC

If you choose poudriere, make sure you use either "poudriere testport" or "poudriere bulk -t" in order to enable stage-qa checks, thanks!

Comment 6 John Marino 2014-08-24 19:43:21 UTC

quick look - port is staged, seems fine except the USERS stuff is really weird.  I don't know why testlogs didn't come yet, it's the thing holding this PR back.

Created attachment 146780 [details]
rundeck.patch

New patch version, now with UIDs/GIDs. Redports result: https://redports.org/buildarchive/20140904103002-56710/

(In reply to John Marino from comment #6)
> quick look - port is staged, seems fine except the USERS stuff is really
> weird.  I don't know why testlogs didn't come yet, it's the thing holding
> this PR back.

Mmm, I just ripped this one off jenkins port. What's bad with USERS?

Comment 9 John Marino 2014-09-04 11:53:27 UTC

Quick review:

I guess nothing is wrong with users, I see now it's a substition.

However:

Issue 1) Please respect 80-column limits.  Use "\" to wrap lines.  I see a couple of places this is needed.

Issue 2) regarding this code:

+.include <bsd.port.pre.mk>
+
+do-install:
+	${MKDIR} ${STAGEDIR}${DATADIR} ${STAGEDIR}${RUNDECK_HOME}/server/lib
+	${INSTALL_DATA} ${DISTDIR}/${DIST_SUBDIR}/${DISTNAME}${EXTRACT_SUFX} ${STAGEDIR}${DATADIR}/${PORTNAME}-launcher${EXTRACT_SUFX}
+
+.include <bsd.port.post.mk>


Question 2A) Why is <pre> and <post> needed here?
Request 2B)  Please replace ${PORTNAME} with "rundeck".  If I changed the name the port, the do-install would break.  I know that *TONS* of people do exactly this and I think it's outright wrong.


If you use jenkins, that's fine, but attach the log that proves this builds cleanly.

(In reply to John Marino from comment #9)
> Issue 1) Please respect 80-column limits.  Use "\" to wrap lines.  I see a
> couple of places this is needed.

fixed

> Issue 2) regarding this code:
> 
> +.include <bsd.port.pre.mk>
> +
> +do-install:
> +	${MKDIR} ${STAGEDIR}${DATADIR} ${STAGEDIR}${RUNDECK_HOME}/server/lib
> +	${INSTALL_DATA} ${DISTDIR}/${DIST_SUBDIR}/${DISTNAME}${EXTRACT_SUFX}
> ${STAGEDIR}${DATADIR}/${PORTNAME}-launcher${EXTRACT_SUFX}
> +
> +.include <bsd.port.post.mk>
> 
> 
> Question 2A) Why is <pre> and <post> needed here?

How do I know? :) I just ripped that part from jenkins port. I suppose there were some variables used that were defined at <pre> step.

fixed

> Request 2B)  Please replace ${PORTNAME} with "rundeck".  If I changed the
> name the port, the do-install would break.  I know that *TONS* of people do
> exactly this and I think it's outright wrong.

fixed

I also updated port to 2.2.1.

Created attachment 146787 [details]
rundeck.patch

(In reply to John Marino from comment #9)
> If you use jenkins, that's fine, but attach the log that proves this builds
> cleanly.

Oh, and I already posted a redports link for the previous version: https://redports.org/buildarchive/20140904103002-56710/

Comment 13 John Marino 2014-09-04 14:35:28 UTC

First the good news:
It visually looks ok to me

The average news:
You aren't supposed to provide patches for new ports, but shar files.  See instructions here: https://www.freebsd.org/doc/en/books/porters-handbook/porting-submitting.html
That should be simple to rectify

The bad news:
You're not acknowledging my requests for verification (e.g. testlogs, make check-plist, make stage-qa, etc).  If you are thinking it's okay because it's based on another port, it's really not.  Can you provide at least minimal verification (check-plist + stage-qa)?

Comment 14 John Marino 2014-09-04 14:37:07 UTC

(In reply to arcade from comment #12)
> (In reply to John Marino from comment #9)
> > If you use jenkins, that's fine, but attach the log that proves this builds
> > cleanly.
> 
> Oh, and I already posted a redports link for the previous version:
> https://redports.org/buildarchive/20140904103002-56710/


I've found Redports fails miserably for QA verification.
But if you get me "make check-plist" output followed by "make stage-qa" output I'll accept those together with redports.

It should be pretty simple to get.

Comment 15 John Marino 2014-09-04 14:39:01 UTC

(In reply to John Marino from comment #13):
> You're not acknowledging my requests for verification (e.g. testlogs, make


obviously we cross-posted.

(In reply to John Marino from comment #13)
> The average news:
> You aren't supposed to provide patches for new ports, but shar files.  See
> instructions here:
> https://www.freebsd.org/doc/en/books/porters-handbook/porting-submitting.html
> That should be simple to rectify

I started with shar but... how can I add patches to UIDS and GIDs to it? Should they be added as a separate patch instead?

> The bad news:
> You're not acknowledging my requests for verification (e.g. testlogs, make
> check-plist, make stage-qa, etc).  If you are thinking it's okay because
> it's based on another port, it's really not.  Can you provide at least
> minimal verification (check-plist + stage-qa)?

Sorry, I thought build logs were enough...

# make check-plist
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
===> Checking for directories owned by MTREEs
===> Checking for directories handled by dependencies
===> Checking for items in pkg-plist which are not in STAGEDIR
===> No pkg-plist issues found (check-plist)

# make stage-qa
====> Running Q/A tests (stage-qa)

Comment 17 John Marino 2014-09-04 14:47:28 UTC

(In reply to arcade from comment #16)
> I started with shar but... how can I add patches to UIDS and GIDs to it?
> Should they be added as a separate patch instead?

A patch for UIDs/GIDS in one attachment, a shar in a second attachment.


> Sorry, I thought build logs were enough...
> 
> # make check-plist
> ====> Checking for pkg-plist issues (check-plist)
> ===> Parsing plist
> ===> Checking for items in STAGEDIR missing from pkg-plist
> ===> Checking for directories owned by MTREEs
> ===> Checking for directories handled by dependencies
> ===> Checking for items in pkg-plist which are not in STAGEDIR
> ===> No pkg-plist issues found (check-plist)
> 
> # make stage-qa
> ====> Running Q/A tests (stage-qa)


Okay, this is good.  Just update the attachments for the patch + shar and we're done!

Created attachment 146788 [details]
rundeck.patch

Created attachment 146789 [details]
rundeck.shar

(In reply to John Marino from comment #17)
> Okay, this is good.  Just update the attachments for the patch + shar and
> we're done!

Updated. Also updated pkg-descr to describe rundeck not jenkins...

Comment 21 John Marino 2014-09-04 15:04:38 UTC

Thanks!

I'm moving this to the pool of "patch-ready" PRs as promised.

Comment 22 John Marino 2014-09-20 17:23:53 UTC

There's something screwy with the pkg-plist (which shouldn't even exist since it only has one file in it).

It's trying to remove 3 directories that were never installed.  The do-install target uselessly creates empty directories in the stagedir, but this will never happen during pkg-installation.  Even if it did, the pkg-plist is hardcoded to "rundeck" when it's supposed to be a user-defined ${RUNDECK_HOME} which might not be rundeck.  So it's wrong in more ways than one.

Comment 23 John Marino 2014-09-20 17:26:32 UTC

I don't even see how ${RUNDECK_HOME} can be used.  The default is 1 level deep, but what if ${RUNDECK_HOME} was set to "alpha/beta/gamma/delta" ?  the @dirrmtry would have to be variable depending on the depth.  This is not supported until @dirrm /@dirrmtry are obsolete.  (which is not yet the case)

Comment 24 commit-hook 2014-09-20 17:42:42 UTC

A commit references this bug:

Author: marino
Date: Sat Sep 20 17:42:17 UTC 2014
New revision: 368669
URL: http://svnweb.freebsd.org/changeset/ports/368669

Log:
  Add new port sysutils/rundeck

  PR:		191562
  Submitted by:	arcade (b1t.name)

  Welcome to Rundeck, the human interface to your operations. Rundeck
  features fine-grain access controls, a built-in job scheduler, and the
  ability to define workflows that dispatch commands and scripts to your
  nodes.

Changes:
  head/GIDs
  head/UIDs
  head/sysutils/Makefile
  head/sysutils/rundeck/
  head/sysutils/rundeck/Makefile
  head/sysutils/rundeck/distinfo
  head/sysutils/rundeck/files/
  head/sysutils/rundeck/files/rundeck.in
  head/sysutils/rundeck/pkg-descr

Comment 25 John Marino 2014-09-20 17:45:59 UTC

Okay, I moved pkg-plist to Makefile.
To do that, I had to change RUNDECK_HOME?= to RUNDECK_HOME= so this is no longer user definable.

I also added a zero-length ".keep" file to avoid having to use a bunch of "@exec" "@noexec" lines to create empty directories.  At least this works.

Anyway, it's in the ports tree now.