Bug 192937 - [New Port] sysutils/consul
Summary: [New Port] sysutils/consul
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Steve Wills
URL:
Keywords:
Depends on: 192964
Blocks:
  Show dependency treegraph
 
Reported: 2014-08-23 05:38 UTC by Thomas Bartelmess
Modified: 2015-03-26 04:49 UTC (History)
5 users (show)

See Also:


Attachments
shar archive (1.85 KB, application/x-shar)
2014-08-23 05:39 UTC, Thomas Bartelmess
no flags Details
Shar archive (1.81 KB, application/x-shar)
2014-08-23 05:47 UTC, Thomas Bartelmess
no flags Details
Shar archive (1.81 KB, text/plain)
2014-08-23 05:53 UTC, Thomas Bartelmess
no flags Details
updated patch (19.73 KB, patch)
2014-11-16 20:41 UTC, Steve Wills
no flags Details | Diff
Second updated patch (18.58 KB, patch)
2014-12-12 02:29 UTC, Steve Wills
no flags Details | Diff
Third updated patch (18.94 KB, patch)
2014-12-12 02:31 UTC, Steve Wills
no flags Details | Diff
consul-template patch (5.36 KB, patch)
2014-12-12 02:33 UTC, Steve Wills
no flags Details | Diff
fourth patch (19.49 KB, patch)
2015-01-03 05:12 UTC, Steve Wills
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Bartelmess 2014-08-23 05:38:30 UTC

    
Comment 1 Thomas Bartelmess 2014-08-23 05:39:09 UTC
Created attachment 146169 [details]
shar archive
Comment 2 Thomas Bartelmess 2014-08-23 05:47:31 UTC
Created attachment 146170 [details]
Shar archive
Comment 3 Thomas Bartelmess 2014-08-23 05:53:25 UTC
Created attachment 146171 [details]
Shar archive
Comment 4 John Marino freebsd_committer freebsd_triage 2014-08-23 11:26:13 UTC
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.
Comment 5 Adam Weinberger freebsd_committer freebsd_triage 2014-08-23 13:20:58 UTC
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}.
Comment 6 John Marino freebsd_committer freebsd_triage 2014-08-23 13:27:51 UTC
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.
Comment 7 Thomas Bartelmess 2014-08-24 01:22:40 UTC
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.
Comment 8 John Marino freebsd_committer freebsd_triage 2014-08-24 08:48:51 UTC
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.
Comment 9 John Marino freebsd_committer freebsd_triage 2014-08-25 06:46:36 UTC
(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)
Comment 10 John Marino freebsd_committer freebsd_triage 2014-08-25 08:21:57 UTC
(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.
Comment 11 Steve Wills freebsd_committer freebsd_triage 2014-11-16 20:41:32 UTC
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.
Comment 12 Robert Barabas 2014-11-24 14:25:11 UTC
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!
Comment 13 Thomas Bartelmess 2014-12-12 02:14:59 UTC
I am not sure if the Web-UI should be part of the port or just be a separate port.
Comment 14 Thomas Bartelmess 2014-12-12 02:15:47 UTC
(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.
Comment 15 Steve Wills freebsd_committer freebsd_triage 2014-12-12 02:28:04 UTC
(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.
Comment 16 Steve Wills freebsd_committer freebsd_triage 2014-12-12 02:29:57 UTC
Created attachment 150495 [details]
Second updated patch

Here's the new version of the patch.
Comment 17 Steve Wills freebsd_committer freebsd_triage 2014-12-12 02:31:36 UTC
Created attachment 150496 [details]
Third updated patch

Oops, that version lacked the category update, here's the whole thing.
Comment 18 Steve Wills freebsd_committer freebsd_triage 2014-12-12 02:33:02 UTC
Created attachment 150497 [details]
consul-template patch

And here's the consul-template port if anyone wants to try it out.
Comment 19 Thomas Bartelmess 2014-12-12 18:15:10 UTC
(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
Comment 20 Thomas Bartelmess 2014-12-15 13:13:59 UTC
(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.
Comment 21 Robert Barabas 2014-12-31 17:08:46 UTC
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?
Comment 22 Steve Wills freebsd_committer freebsd_triage 2014-12-31 17:26:45 UTC
(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.
Comment 23 Robert Barabas 2014-12-31 20:03:16 UTC
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?
Comment 24 Steve Wills freebsd_committer freebsd_triage 2014-12-31 20:52:36 UTC
(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.
Comment 25 Robert Barabas 2014-12-31 21:54:49 UTC
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.
Comment 26 Steve Wills freebsd_committer freebsd_triage 2015-01-01 02:16:17 UTC
(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.
Comment 27 Steve Wills freebsd_committer freebsd_triage 2015-01-01 02:27:23 UTC
(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.
Comment 28 Steve Wills freebsd_committer freebsd_triage 2015-01-01 03:52:55 UTC
(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.
Comment 29 Robert Barabas 2015-01-02 22:35:15 UTC
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?
Comment 30 Steve Wills freebsd_committer freebsd_triage 2015-01-03 05:12:23 UTC
Created attachment 151241 [details]
fourth patch

Here's an updated version of the patch including the UIDs and GIDs file updates.
Comment 31 Steve Wills freebsd_committer freebsd_triage 2015-01-03 05:14:07 UTC
(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.
Comment 32 commit-hook freebsd_committer freebsd_triage 2015-03-26 02:21:35 UTC
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
Comment 33 Steve Wills freebsd_committer freebsd_triage 2015-03-26 02:48:53 UTC
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.
Comment 34 Walter Schwarzenfeld 2015-03-26 03:49:23 UTC
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
Comment 35 commit-hook freebsd_committer freebsd_triage 2015-03-26 04:48:48 UTC
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
Comment 36 Steve Wills freebsd_committer freebsd_triage 2015-03-26 04:49:55 UTC
(In reply to w.schwarzenfeld from comment #34)

Oops, sorry about that, fix committed, thanks for reporting.