Created attachment 146169 [details] shar archive
Created attachment 146170 [details] Shar archive
Created attachment 146171 [details] Shar archive
Thanks! Do you have any verification available? In order of preference, examples of what I'm looking for are: 1) "poudriere testport" or "poudriere bulk -t" logs 2) Redports or tinderbox logs 3) "make check-plist" followed by "make stage-qa" output (https://www.freebsd.org/doc/en/books/porters-handbook/porting-testing.html) Also, please run "portlint" and paste the output.
Couple things. Please change the formatting by putting tabs after variables, like all the other ports do. Second, the COMMENT string is insufficient ("Consul") and the pkg-descr is just a marketing blurb. Third, you are forcing compilation with clang. FreeBSD supports GCC as well, and ccache and distcc. Pass CC="${CC}" if you have to. Fourth, install with ${INSTALL_PROGRAM} or ${INSTALL_SCRIPT}, not ${CP}.
also wrong: .include <bsd.port.pre.mk> .include "${PORTSDIR}/lang/go/files/bsd.go.mk" .include <bsd.port.post.mk> Should be: .include "${.CURDIR}/../../lang/go/files/bsd.go.mk" .include <bsd.port.mk> If you see other ports with ${PORTSDIR} then tell me and I'll fix them.
Hi, sorry for those mistakes this is my first submission to the ports tree. I have one follow up question. When the port has a dependency that is not in the ports tree, should the dependency be a separate submission or combined with the existing. (In reply to John Marino from comment #4) > Thanks! Do you have any verification available? In order of preference, > examples of what I'm looking for are: > > 1) "poudriere testport" or "poudriere bulk -t" logs > 2) Redports or tinderbox logs > 3) "make check-plist" followed by "make stage-qa" output > (https://www.freebsd.org/doc/en/books/porters-handbook/porting-testing.html) > > Also, please run "portlint" and paste the output.
I think you are better off with separate PRs and marking depends on XXX / blocks XXX. The oldest ready PR has 4 ports attached and people are skipping it for others, probably because they prefer to test one port at a time, not 4 at a time.
(In reply to John Marino from comment #6) > also wrong: > > .include <bsd.port.pre.mk> > .include "${PORTSDIR}/lang/go/files/bsd.go.mk" > .include <bsd.port.post.mk> > > Should be: > > .include "${.CURDIR}/../../lang/go/files/bsd.go.mk" > .include <bsd.port.mk> For others viewing this PR, the <pre> and <post> are still needed even if ${PORTSDIR} is removed because bsd.go.mk uses the ${ARCH} variable (as discussed in a related PR)
(In reply to Adam Weinberger from comment #5) > Couple things. Please change the formatting by putting tabs after variables, > like all the other ports do. > > Second, the COMMENT string is insufficient ("Consul") and the pkg-descr is > just a marketing blurb. > > Third, you are forcing compilation with clang. FreeBSD supports GCC as well, > and ccache and distcc. Pass CC="${CC}" if you have to. > > Fourth, install with ${INSTALL_PROGRAM} or ${INSTALL_SCRIPT}, not ${CP}. Thomas, to be perfectly clear, this PR is on-hold waiting for a new submission to fix at least the problems Adam mentioned.
Created attachment 149492 [details] updated patch Here's an updated version that passes poudriere, includes the UI bits and an init script. This version also doesn't fetch during build, which is a requirement for ports, hence the huge list of source files. Seems to work, please let me know if you're still willing to be maintainer.
Steve's cleaned up patch works nicely for me. I would be interested in a consul-template ports as well but I don't believe that a patch was submitted for it yet. Willing to contribute as needed, if there is demand!
I am not sure if the Web-UI should be part of the port or just be a separate port.
(In reply to Robert Barabas from comment #12) > Steve's cleaned up patch works nicely for me. I would be interested in a > consul-template ports as well but I don't believe that a patch was submitted > for it yet. Willing to contribute as needed, if there is demand! I don't think anyone started on a consul-template port.
(In reply to Thomas Bartelmess from comment #13) > I am not sure if the Web-UI should be part of the port or just be a separate > port. Seems like it should be in the same port to me. It's tiny. Do you want to be maintainer or should I? Also, I have a port of consul-template ready to commit, just waiting on committing this. I have made a few minor changes since this patch, I'll post an updated copy. It is ready to go.
Created attachment 150495 [details] Second updated patch Here's the new version of the patch.
Created attachment 150496 [details] Third updated patch Oops, that version lacked the category update, here's the whole thing.
Created attachment 150497 [details] consul-template patch And here's the consul-template port if anyone wants to try it out.
(In reply to Steve Wills from comment #18) > Created attachment 150497 [details] > consul-template patch > > And here's the consul-template port if anyone wants to try it out. Thanks, I have a look tonight
(In reply to Steve Wills from comment #18) > Created attachment 150497 [details] > consul-template patch > > And here's the consul-template port if anyone wants to try it out. Looks good to me.
I'm not sure I'm a big fan of rewriting what the embedded go builder script would do into the ports Makefile. Maintaining the multisites could become a pain in the neck relatively soon. I'd much rather use the upstream installer and call that. Anyone interested in an alternative solution?
(In reply to Robert Barabas from comment #21) > I'm not sure I'm a big fan of rewriting what the embedded go builder script > would do into the ports Makefile. Maintaining the multisites could become a > pain in the neck relatively soon. I'd much rather use the upstream installer > and call that. Anyone interested in an alternative solution? The go build for this software fetches during the build stage, which ports does not allow, so it is not usable. Further, it does not always fetch the same versions of it's deps, so it can produce different results at different times depending on the git repos of the deps. Using the upstream installer won't work.
You mean it won't work in a way that ports is designed to deal with, right? Like checksumming issues due to the nature of a git repo and the varying versions of submodules pulled by go deps, etc. I imagine the issue with the go dependencies is analog to the issues with CPAN modules, ruby gems, python modules. What's the blessed upon way to handle those currently?
(In reply to Robert Barabas from comment #23) > You mean it won't work in a way that ports is designed to deal with, right? > Like checksumming issues due to the nature of a git repo and the varying > versions of submodules pulled by go deps, etc. I mean it will not be able to fetch during the build phase, so the build will fail. > I imagine the issue with the go dependencies is analog to the issues with > CPAN modules, ruby gems, python modules. What's the blessed upon way to > handle those currently? I believe you imagine incorrectly. Each those works differently from each other and from how go builds work. For CPAN, for example, there is fetch during build, so that's not an issue. For ruby gems, the gem command gives you separate fetch and build commands, which we use. Python, I'm not as familiar with, but I know it doesn't fetch during build. See the Porters Handbook for more details on how to use those currently.
I see now what you are trying to get to regarding the build. If one were to fetch the deps via make deps (consul has a Makefile if I recall it correctly) / go deps during do-fetch, the modules wouldn't need to be fetched during the build phase and the build could conclude as if the modules were prepackaged (as in syncthing with Godeps). The difference would be that fetch would grab these modules and they wouldn't be embedded in the parent project or tarball.
(In reply to Robert Barabas from comment #25) > I see now what you are trying to get to regarding the build. If one were to > fetch the deps via make deps (consul has a Makefile if I recall it > correctly) / go deps during do-fetch, the modules wouldn't need to be > fetched during the build phase and the build could conclude as if the > modules were prepackaged (as in syncthing with Godeps). The difference would > be that fetch would grab these modules and they wouldn't be embedded in the > parent project or tarball. Right, but the trouble there is that with the way consul's build grab it's deps, they are not the same versions of the deps every time. There is a risk that someone could add something malicious or simply change the deps in a way that would cause a build failure or other problem. Syncthing bundles it's deps so this is not an issue. I do wish consul bundled it's deps the way syncthing does, but it doesn't, so the way I've done it here is required, IMHO.
(In reply to Steve Wills from comment #26) > (In reply to Robert Barabas from comment #25) > > I see now what you are trying to get to regarding the build. If one were to > > fetch the deps via make deps (consul has a Makefile if I recall it > > correctly) / go deps during do-fetch, the modules wouldn't need to be > > fetched during the build phase and the build could conclude as if the > > modules were prepackaged (as in syncthing with Godeps). The difference would > > be that fetch would grab these modules and they wouldn't be embedded in the > > parent project or tarball. > > Right, but the trouble there is that with the way consul's build grab it's > deps, they are not the same versions of the deps every time. There is a risk > that someone could add something malicious or simply change the deps in a > way that would cause a build failure or other problem. Syncthing bundles > it's deps so this is not an issue. I do wish consul bundled it's deps the > way syncthing does, but it doesn't, so the way I've done it here is > required, IMHO. Actually, I take that back, my memory was failing me there. After looking, I see that consul does specify git commits for it's deps, so that might be workable. Right now I prefer the way I've done it, but a patch might change my mind.
(In reply to Steve Wills from comment #27) > (In reply to Steve Wills from comment #26) > > (In reply to Robert Barabas from comment #25) > > > I see now what you are trying to get to regarding the build. If one were to > > > fetch the deps via make deps (consul has a Makefile if I recall it > > > correctly) / go deps during do-fetch, the modules wouldn't need to be > > > fetched during the build phase and the build could conclude as if the > > > modules were prepackaged (as in syncthing with Godeps). The difference would > > > be that fetch would grab these modules and they wouldn't be embedded in the > > > parent project or tarball. > > > > Right, but the trouble there is that with the way consul's build grab it's > > deps, they are not the same versions of the deps every time. There is a risk > > that someone could add something malicious or simply change the deps in a > > way that would cause a build failure or other problem. Syncthing bundles > > it's deps so this is not an issue. I do wish consul bundled it's deps the > > way syncthing does, but it doesn't, so the way I've done it here is > > required, IMHO. > > Actually, I take that back, my memory was failing me there. After looking, I > see that consul does specify git commits for it's deps, so that might be > workable. Right now I prefer the way I've done it, but a patch might change > my mind. Just wanted to add, the reason I'm preferring having ports fetch the deps is that if the deps are fetched using the consul Makefile at fetch time (post-fetch, with a EXTRACT_DEPENDS on golang), there's still the issue that the deps aren't cached anywhere like the main port src file, so there's still a larger possibility of issue, though I don't realistically expect an issue with being able to reach github. Still, that way is somewhat messier and inconsistent with how ports typically work. Then again, so is not using shared libs where possible.
IMHO a PR to upstream would be better than maintaining GH commit hash pointers 2x but this is just my $0.02. If you prefer to do it that way that's perfectly fine with me. I could try a PR to upstream to see if they would be willing to package the deps, if you are interested. As far as the updated consul patch goes, please note that there appears to be a regression. The Makefile still specifies a consul user and group but the patch doesn't contain the entries for UIDs/GIDs anymore. Would you mind to fix?
Created attachment 151241 [details] fourth patch Here's an updated version of the patch including the UIDs and GIDs file updates.
(In reply to Robert Barabas from comment #29) You're welcome to contact upstream. Regression is a bit strong, just forgot to include the UIDs and GIDs files in the patch, an updated patch has been added.
A commit references this bug: Author: swills Date: Thu Mar 26 02:21:27 UTC 2015 New revision: 382288 URL: https://svnweb.freebsd.org/changeset/ports/382288 Log: sysutils/consul: add port Consule is a distributed, highly available and data center aware tool for discovering and configuring services. Set maintainer to myself for now since I made so many changes I'm not sure submitter wants to maintain any more. PR: 192937 Submitted by: Thomas Bartelmess <thomas@bartelmess.io> (based on) Changes: head/sysutils/Makefile head/sysutils/consul/ head/sysutils/consul/Makefile head/sysutils/consul/distinfo head/sysutils/consul/files/ head/sysutils/consul/files/consul.in head/sysutils/consul/pkg-descr head/sysutils/consul/pkg-plist
Finally committed this and consul-template, though they weren't the latest version. Will try to update them soon, unless someone submits a patch first. Also, let me know if you're interested in taking maintainership, I only set myself as maintainer because I'd made so many changes from what you did I wasn't sure you would still want to maintain it.
On FreeBSD-10.1 ===> Building for consul-0.4.1 # github.com/armon/gomdb ../../armon/gomdb/mdb.c:8513:46: warning: data argument not used by format string [-Wformat-extra-args] ===> Staging for consul-0.4.1 ===> Generating temporary packing list ===> Creating users and/or groups. ** Cannot find any information about group `consul' in /usr/ports/GIDs. *** Error code 1 Stop. make: stopped in /usr/ports/sysutils/consul
A commit references this bug: Author: swills Date: Thu Mar 26 04:48:21 UTC 2015 New revision: 382291 URL: https://svnweb.freebsd.org/changeset/ports/382291 Log: Add UIDs/GIDs for sysutils/consul PR: 192937 Pointyhat to: swills Reported by: w.schwarzenfeld@utanet.at Changes: head/GIDs head/UIDs
(In reply to w.schwarzenfeld from comment #34) Oops, sorry about that, fix committed, thanks for reporting.