This is of course devel/ocaml-opam and not devel/opam-ocaml.

Comment 2 John Marino 2014-11-01 09:24:17 UTC

We need maintainer approval, notifying (auto-assigner failed because port origin was wrong)

I remember discussing this in a private mail thread, don't remember if it was with you. Like I said, i don't think adding a hard dependency to rsync is a good idea, since you can use opam without it, like you can use opam with only rsync and no git. Maybe adding a note ? Or is there a way to specify optionnal dependencies ?

Comment 4 John Marino 2014-11-01 16:10:11 UTC

using the options framework is a good idea.

you have to decide if the RSYNC option is on by default or not (I would probably saw it should be for the purposes of having more useful binary packages)

You can perhaps split all the protocols into options.

Created attachment 148892 [details]
upgrade to 1.2.0 and add optionnal dependencies

Ok, here is a patch to add rsync, git, and wget as optionnal dependencies, and i took the opportunity to upgrade the port to the latest 1.2.0 version. I can open a new PR if you prefer.

Oops sorry i was too eager, i'll post an update when i can. The patch is not good.

Created attachment 148896 [details]
upgrade to 1.2.0 and add optionnal dependencies

Comment 8 John Marino 2014-11-01 22:37:07 UTC

At first glance, I'm fine with everything except the MANPAGES option.  Since MANPAGES doesn't pull in any dependencies, they should be installed unconditionally without possibility of disabling them (note man pages are considered separately from "documentation").

Basically, unless you need doxygen or sphinx or something that that to generate the manpages, they are always installed.

So maybe you can update the patch accordingly.  I'll commit it then.

Created attachment 148929 [details]
upgrade to 1.2.0 and add optionnal dependencies

Ok, i've removed the MANPAGES option.

Created attachment 148930 [details]
upgrade to 1.2.0 and add optionnal dependencies

Sorry, i just realized pkg-descr was still pointing to the ocamlpro website while opam.ocaml.org is probably a better place now. I thought it had been fixed a while ago.

Comment 11 John Marino 2014-11-02 15:37:51 UTC

I had to reorder some lines to make portlint happy.  That was easy, but this doesn't build in poudriere because ocaml-opam tries to download something during the build phase which is completely illegal.  fetching has to happen during fetch phase only.


======================<phase: build          >============================
===>  Building for ocaml-opam-1.2.0
gmake[1]: Entering directory '/wrkdirs/usr/ports/devel/ocaml-opam/work/ocaml-opam-cbe460b'
gmake -C src_ext lib-ext
gmake[2]: Entering directory '/wrkdirs/usr/ports/devel/ocaml-opam/work/ocaml-opam-cbe460b/src_ext'
[ -e  extlib-1.5.3.tar.gz ] || curl  -OL http://ocaml-extlib.googlecode.com/files/extlib-1.5.3.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0curl: (6) Could not resolve host: ocaml-extlib.googlecode.com
Makefile:51: recipe for target 'extlib.download' failed
gmake[2]: *** [extlib.download] Error 6
gmake[2]: Leaving directory '/wrkdirs/usr/ports/devel/ocaml-opam/work/ocaml-opam-cbe460b/src_ext'
Makefile:29: recipe for target 'lib-ext' failed
gmake[1]: *** [lib-ext] Error 2
gmake[1]: Leaving directory '/wrkdirs/usr/ports/devel/ocaml-opam/work/ocaml-opam-cbe460b'
*** Error code 1



So what is the fix?

Comment 12 John Marino 2014-11-02 15:39:22 UTC

should  not extlib-1.5.3.tar.gz be in the distinfo file and fetched at the same time?  perhaps with EXTRACT_ONLY set to the main distfile?

I see. That is tricky, because i guess it means i have to create a bunch of ocaml ports to package all the ocaml dependencies opam needs. I'll need more time for this.

Comment 14 John Marino 2014-11-02 15:43:43 UTC

how many are we talking about, and can the pre-fetching idea work?

E.g. just fetch everything needed in the fetch phase and put in a place that ocaml-opam build will look for it and not attempt to fetch it.

It'll probably work, but while here, better create real ports for these libs i guess.

Comment 16 John Marino 2014-11-02 15:54:42 UTC

if this proposed ports are useful outside of opam, sure.
If they are *only* used for opam, then I'd prefer my approach.

Comment 17 John Marino 2014-11-14 08:40:10 UTC

Hi Joris, can we get a status update on this PR?

Created attachment 154263 [details]
upgrade to 1.2.0 and add optionnal dependencies

Hi, after a long time, here is a new patch

Created attachment 154264 [details]
upgrade to 1.2.0 and add optionnal dependencies

The patch in the right direction is better :>

Comment on attachment 154264 [details]
upgrade to 1.2.0 and add optionnal dependencies

Joris, it is great that you prepared an upgrade for opam!

Could you please produce a clean patch, rather than a git specific patch?
I think, you just need to remember to use the --no-headers options, when
using git diff:

http://stackoverflow.com/questions/4610744/can-i-get-a-patch-compatible-output-from-git-diff

Comment on attachment 154264 [details]
upgrade to 1.2.0 and add optionnal dependencies

Joris, it is great that you prepared an upgrade for opam!

Could you please produce a clean patch, rather than a git specific patch?
I think, you just need to remember to use the --no-headers options, when
using git diff:

http://stackoverflow.com/questions/4610744/can-i-get-a-patch-compatible-output-from-git-diff

Comment on attachment 154264 [details]
upgrade to 1.2.0 and add optionnal dependencies

Joris, it is great that you prepared an upgrade for opam!

Could you please produce a clean patch, rather than a git specific patch?
I think, you just need to remember to use the --no-headers options, when
using git diff:

http://stackoverflow.com/questions/4610744/can-i-get-a-patch-compatible-output-from-git-diff

Comment 23 John Marino 2015-03-14 14:57:24 UTC

the patch format doesn't bother me, extra paths can be stripped.

Applying with -p1 did not work well for me, because some paths are of the form
{a,b}/… and some others are /dev/null.  How did you manage to apply the patch.

(Let's hope I do not hit three times the Save Changes button this time.)

Comment 25 commit-hook 2015-03-15 13:05:55 UTC

A commit references this bug:

Author: marino
Date: Sun Mar 15 13:05:43 UTC 2015
New revision: 381326
URL: https://svnweb.freebsd.org/changeset/ports/381326

Log:
  devel/ocaml-opam: Upgrade version 1.1.1 => 1.2.0

  Add several options for protocols such as rsync, git, wget, Hg, etc,
  where the first three options are on by default.

  PR:		194529
  Suggested by:	Michael Gruenewald
  Final patch:	maintainer (Joris Giovannangeli)

Changes:
  head/devel/ocaml-opam/Makefile
  head/devel/ocaml-opam/distinfo
  head/devel/ocaml-opam/files/
  head/devel/ocaml-opam/pkg-descr
  head/devel/ocaml-opam/pkg-plist

Comment 26 John Marino 2015-03-15 13:06:52 UTC

Thanks, all.