Created attachment 171068 [details] shar of the port First time contribution, sorry if I messed up, I am happy to learn. I wasn't sure how to submit the patch that is necessary for the UIDs/GIDs files. A diff on those files gives the following result: 237a238 > prometheus:*:789:789::0:0:Prometheus Daemon:/nonexistent:/usr/sbin/nologin 227a228 > prometheus:*:789:
Am I missing here something? Anything I got to do?
Thanks for your contribution Thomas, and welcome to porting :) First things first, QA/testing is really important and works wonders to get your changes into the tree quicker, particularly for large/complex ones. For ports, we want to ensure the changes pass: - portlint (syntactic/semantic) - poudriere (building and packaging) These will usually identify the vast majority of major quality issues/problems in a port. For more information and instructions: https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/testing.html Since this is a new port, feel free to replace the shar with a unified diff (svn diff preferred) that includes all the other changes like: - Adding the port category/Makefile - UID/GID There's additional information about adding a new port here: https://www.freebsd.org/doc/en/articles/committers-guide/ports.html#ports-qa-adding If you have questions or need more help, #freebsd-ports on freenode IRC :) Once you're satisfied with the QA results from above, I'll review it in detail
Created attachment 175671 [details] SVN diff
Added a SVN diff and updated to the current prometheus version. Portlint had issues, but I have resolved them. There is one warning, because it picks up the "install" from "$(GO) install", but that should be ok. poudriere has also passed with the current state.
I'm attaching an alternative patch based on Thomas's that passes portlint and poudriere testing (amd64: 11.0p1, 10.3p10; i386: 11.0p1). The main differences are: - Makefile "USES=go" for building - sample config from the project source is used - sample config is declared as such in pkg-plist - collected data is kept in /var/db/prometheus [portlint warning] - rc.d file adds extra_commands="reload" because prometheus supports SIGHUP - rc.d file fixed to run prometheus as the self-named user and group - rc.d file requires config file existance - rc.d file allows rc.conf override of config file and data dir location - includes SUBDIR change for sysutils
Created attachment 175808 [details] Patch for prometheus 1.2.0 port
Comment on attachment 175808 [details] Patch for prometheus 1.2.0 port @Thomas, please review this attachment 175808 [details] and set maintainer-approval to + if you are happy for it to be the version that is used for commit. Alternatively, set maintainer-approval on the attachment to '-' to specify that it shouldn't be used, with comment if necessary.
(In reply to Andy Carrel from comment #6) @Andy Thank you for the contribution
Hmm, it seems this port has been committed via bug 212468. I have no idea how I didn't recognize the duplicate (given I commented on both).
Whow, I totally missed this PR for prometheus -- sorry! Now, what's the difference between the two submissions ? Is there any here which is missing in the committed port from PR#212468 ?
(In reply to Kurt Jaeger from comment #10) 'any feature'
Hi There, I created the other port. I'm very sorry I didn't see that this ticket already existed. I should have helped with this one, instead of making a new ticket. I have no problem if my port is replaced with @Thomas' & @Andy's one. Whatever makes the situation right. :)
A quick summary of the differences... - The committed port includes promtool, which is a good idea. - The committed port doesn't declare it's use of /var/db/prometheus in pkg-plist. - The committed port logs to a file using shell redirection (vs. logging with syslog or using the -log.format command line flag to choose). - The committed port leaves the .yml config file which should probably end in .sample and be declared @sample in pkg-plist to avoid user sadness on de/reinstall. - The committed port has a comment about the default for prometheus_data_dir which is not quite correct (/var/tmp vs. /var/db) I don't feel too strongly either way about the logging, but /usr/sbin/daemon -c -f always makes me feel better for some reason. I'll attach a patch with the quick fixes for the last two items.
Created attachment 175831 [details] Quick fixes for net-mgmt/prometheus Here are quick fixes for the comment documented default for the data directory and making sure user configs don't get clobbered on de/reinstall.
Created attachment 175872 [details] improvement patch for net-mgmt/prometheus @Andy; Thank you, Your changes are good. I had to add a `post-stage` target to create the /var/db/prometheus` directory in staging to allow it to build. I think this is the "right way", but please do tell me if there's a better way. I also added PORTREVISION = 1. Can someone kindly commit the latest attached patch? Thank you all,
testbuild for the new patch @work
A commit references this bug: Author: pi Date: Tue Oct 18 18:08:33 UTC 2016 New revision: 424194 URL: https://svnweb.freebsd.org/changeset/ports/424194 Log: sysutils/prometheus: Fix some issues discussed in PRs - the config file is now a @sample - pkg-plist added - /var/db/prometheus is the new place to be PR: 210059 Submitted by: Andy Carrel <wac@google.com> Approved by: jev@ecadlabs.com (maintainer) Changes: head/net-mgmt/prometheus/Makefile head/net-mgmt/prometheus/files/prometheus.in head/net-mgmt/prometheus/pkg-plist
Committed, thanks!