Mk/bsd.options.desc.mk: Add version control systems OPTION descriptions

[loader]

Summary

Proposed commit log message:

Mk/bsd.options.desc.mk: Add shared version control OPTIONS & descriptions

- Add CVS description
- Add DARCS description
- Add GIT description
- Add MERCURIAL description
- Add PERFORCE description
- Rename SVN to SUBVERSION

devel/anjuta:
    - Rename OPTION SVN to SUBVERSION
    - Sort OPTIONS_DEFINE and OPTIONS_DEFAULT
    - Change SUBVERSION_DESC to use the default description

devel/git:
    - Rename OPTION P4 to PERFORCE
    - Rename OPTION SVN to SUBVERSION
    - Change PERFORCE_DESC to use the default description

devel/ocaml-opam:
    - Rename OPTION HG to MERCURIAL
    - Sort OPTIONS_DEFINE and OPTIONS_DEFAULT
    - Change {DARCS,GIT,MERCURIAL}_DESC to use the default description

devel/thunar-vcs-plugin:
    - Rename OPTION SVN to SUBVERSION
    - Sort OPTIONS_DEFINE
    - Change GIT_DESC to use the default description

devel/viewvc:
    - Rename OPTION SVN to SUBVERSION
    - Sort OPTIONS_DEFINE, OPTIONS_DEFAULT and OPTIONS_SINGLE

devel/viewvc-devel:
    - Rename OPTION SVN to SUBVERSION
    - Sort OPTIONS_DEFINE, OPTIONS_DEFAULT and OPTIONS_SINGLE

net-mgmt/observium:
    - Rename OPTION SVN to SUBVERSION

net-mgmt/rancid3:
    - Rename OPTION SVN to SUBVERSION
    - Sort OPTIONS_SINGLE_SCM
    - Change {CVS,GIT,SUBVERSION}_DESC to use the default description

ports-mgmt/portshaker:
    - Rename OPTION SVN to SUBVERSION
    - Rename HG to MERCURIAL
    - Sort OPTIONS_DEFINE
    - Change {GIT,MERCURIAL}_DESC to use the default description

security/hydra:
    - Rename OPTION SVN to SUBVERSION

security/medusa:
    - Rename OPTION SVN to SUBVERSION
    - Sort OPTIONS_DEFINE

shells/scponly:
    - Rename OPTION SVN to SUBVERSION
    - Sort OPTIONS_DEFINE
    - Typo fix in SVNSERVE_DESC

www/trac-devel:
    - Rename OPTION SVN to SUBVERSION
    - Sort OPTIONS_DEFINE, OPTIONS_DEFAULT and OPTIONS_MULTI_DATABASE
    - Change {GIT,SUBVERSION}_DESC to use the default description

Reviewed_by: koobs, adamw
Approved by: koobs (ports, mentor)
Differential_Revision: D17459
MFH:    No (OPTION description updates)

[koobs]

In D17459#456234, @loader wrote:

In D17459#456127, @koobs wrote:

For all options added in Mk/bsd.options.desc.mk I think we should have 'version control` in the description, like

MERCURIAL_DESC= Mercurial version control support

After changing the names in Mk/bsd.options.desc.mk to their full names, update ports that use those names too

Should I also change the SVN_DESC?= Subversion support to SUBVERSION_DESC?= Subversion version control support in Mk/bsdoptions.desc.mk?

Yep, all options should be full names, with '<name> version control support' as the description

We should also add to this review any changes to existing ports to have them use the new names consistently. This will be your "sweeping commit" mentee item

[koobs]

In D17459#456222, @adamw wrote:

Perhaps these could say Git repository support or, even better, Git code repository support.

I'm sick of LibXYZ support descriptions.

My previous comments/request for changes included this

[loader]

Change version control OPTIONs to their full name and update ports with these OPTIONs

[loader]

I'm not sure if it's okay to update the OPTION descriptioins in some ports or just keep them as is,
for example, GIT_DESC in www/moinmoincl/Makefile:

GIT_DESC=        Install git to use git-diff(1) instead of diff(1) for colored output

How do I get approval from all these port maintainers? Should I add all of them to the reviewer list?

[koobs]

In D17459#456514, @loader wrote:

I'm not sure if it's okay to update the OPTION descriptioins in some ports or just keep them as is,
for example, GIT_DESC in www/moinmoincl/Makefile:

GIT_DESC=        Install git to use git-diff(1) instead of diff(1) for colored output

We dont need to change any <OPTIONS_DESC> used in any ports who's descriptions dont match the default description, that is <name> version control support. That is to say, we leave port defined descriptions in place is they already better/more correctly describe the option

If you've removed any <OPTIONS>_DESC from ports that don't match the default description, re-add them (remove them from this diff)

How do I get approval from all these port maintainers? Should I add all of them to the reviewer list?

For any ports where we *remove* the <OPTIONS>_DESC because it matches the default description, approval is not required, as the change is a `framework / sweeping change' that doesn't require approval.

Said another way, it doesnt require approval because were just switching from *ports* defined descriptions, to *equivalent* (or better described) *shared* descriptions.

[koobs]

Mk/bsd.options.desc.mk

431

Is this not intended to be 'Rsync version control support` ?

devel/anjuta/Makefile

82 ↗

(On Diff #60046)

For this (anjuta) port: Is this option for explicitly and only for 'version control' support, or something else/more?

If so, this change is fine.

If not, change the option name (SVN -> SUBVERSION) but leave the defined (overriding) description

devel/bzrtools/Makefile

19 ↗

(On Diff #60046)

For this (bzrtools) port: Is this option for *explicitly and only* for 'version control' support, or something else/more?

If so, this change is fine.

If not, or its valuable/meaningful to keep the 'for rspush' description in place, change the option name (SVN -> SUBVERSION) but leave the defined (overriding) description

devel/ocaml-opam/Makefile

30–33 ↗

(On Diff #60046)

For this (ocaml-opam) port: Is this option for explicitly and only for 'version control' support, or something else/more?

If so, this change is fine.

If not, or if the existing port-defined description is *more* correct/meaningful, change the option names but leave the defined (overriding) descriptions

devel/p5-SVN-Notify-Mirror/Makefile

29 ↗

(On Diff #60046)

For this port: Is this option for explicitly and only for 'version control' support, or something else/more?

If so, this change is fine.

If not, or the port-defined description is *more* meaningful/correct, leave the defined (overriding) description

devel/tig/Makefile

31 ↗

(On Diff #60046)

For this port: Is this option for explicitly and only for 'version control' support, or something else/more?

If so, this change is fine.

If not, leave the port-defined (overriding) description

net-mgmt/rancid3/Makefile

41 ↗

(On Diff #60046)

For this port: Is this option for explicitly and only for 'version control' support, or something else/more? The current description reads like it *swaps* version control support from CVS to SVN (does this mean CVS is no longer supported?)

If so, this change is fine.

If not, change the option name (SVN -> SUBVERSION) but leave the defined (overriding) description

net/prosearch/Makefile

31 ↗

(On Diff #60046)

This ports description doesn't like it only means 'version control support'. Is this option for explicitly and only for 'version control' support, or something else/more?

If so, this change is fine.

If not, or if the port-defined description is *more* meaningful/correct, leave the defined (overriding) description

security/hydra/Makefile

7 ↗

(On Diff #60046)

Why 0 not 1 ?

sysutils/password-store/Makefile

29 ↗

(On Diff #60046)

For this port: Is this option for explicitly and only for 'version control' support, or something else/more? This description reads like it ("storage") is not explicitly (or only) about version control.

If so, this change is fine.

If not, or the port-defined description is *more* meaningful/correct, leave the defined (overriding) description

sysutils/zxfer/Makefile

21 ↗

(On Diff #60046)

For this port: Is this option for explicitly and only for 'version control' support, or something else/more? It doesn't read like its a 'version control support' function, just a transfer/sync function.

If so, this change is fine.

If not, or the port-defined description is *more* correct/accurate, leave the defined (overriding) description

textproc/docproj/Makefile

52 ↗

(On Diff #60046)

Does "from ports" here mean that if the option is not defined, it uses svnlite from base?

If so, the shared 'Subversion version control support' is probably not appropriate, and the port-defined description should remain.

If not, and this options explicitly and only means 'version control support', this change is fine.

www/moinmoincli/Makefile

24 ↗

(On Diff #60046)

This option doesn't read like 'git version control support', but more like 'Use git-diff instead of bundled diff`

If the existing port-defined description is *more* accurate/meaningful, leave it

[adamw]

Mk/bsd.options.desc.mk

431

Rsync doesn't do version control AFAIK.

This should be something like Remote syncing support, but it doesn't seem related to the rest of this patch.

[loader]

Remove RSYNC_DESC and keep some of the port defined descritions.

[loader]

Mk/bsd.options.desc.mk

431

RSYNC_DESC was added because of the port devel/ocaml-opam
https://reviews.freebsd.org/D17459#389750
I have deleted RSYNC_DESC from this review.

security/hydra/Makefile

7 ↗

(On Diff #60046)

Sorry, typo.

[koobs]

@loader Let's get this sorted/complete so we can concentrate on graduating you (See D25395)

See also: https://wiki.freebsd.org/KubilayKocak/MentoringChecklist

[loader]

Update patch on a recent ports tree (r539330)

[adamw]

Mk/bsd.options.desc.mk

157

Just a head's up, MERCURIAL is no longer sorted here after the rename.

[loader]

sort MERCURIAL_DESC and SUBVERSION_DESC in bsd.options.desc.mk

[loader]

Mk/bsd.options.desc.mk

157

Thanks @adamw.

[koobs]

@kevans is assisting me with my mentorship duties, to otherwise unblock this review/change, because I am super occupied for at least a week

@kevans please add yourself to co-mentors for @loader

[koobs]

bsd.options.desc.mk changes are not blocked by anyone, the file is explicitly available for anyone to commit to. update reviewers accordingly

[loader]

Update patch for the latest ports tree

[loader]

Deleted `BAZAAR_DESC?= Bazaar version control support```
the devel/bzr port had been removed:

devel/bzr||2021-01-02|Has expired: Uses Python 2.7 which is EOLed upstream

[koobs]

All looks good, except:

• The purpose of this diff is to make consistent and remove the need to use in-port options descriptions where a shared one suffices. So:

All ports using any of CVS, DARCS, GIT, MERCURIAL, PERFORCE or SUBVERSION options, should have their *_DESC options 'removed' if the default descriptions cover them.

I've provided inline examples of a 3-5, there might be more (I cannot see full context diffs here)

devel/anjuta/Makefile

85 ↗

(On Diff #91267)

This can be dropped, since this is an IDE and it is for 'subversion version control support' (the default description covers it, and is more descriptive)

devel/ocaml-opam/Makefile

28–29 ↗

(On Diff #91267)

Alpha sort these

32–35 ↗

(On Diff #91267)

I would drop these. From upstream [1]:

src, archive, http, local, git, hg, darcs are the location where the package upstream sources can be downloaded

... 

a version-controlled repository under git, darcs or hg, or a specific commit, tag or branch in that repository if the string ends by #<SHA1> or #<tag-name> or #<branch-name>. Use git, hg or darcs.

All of the above are 'source repositories', so the default descriptions cover them.

[1] https://opam.ocaml.org/doc/manual/dev-manual.html#sec10

devel/thunar-vcs-plugin/Makefile

29 ↗

(On Diff #91267)

This can go

net-mgmt/rancid3/Makefile

46–48 ↗

(On Diff #91267)

These 3 seem to be covered by the default descriptions

54 ↗

(On Diff #91267)

Were the following cases tested:

• FOO_CONFIGURE_WITH instead of _ON, which sets --without-foo in the OFF case. This is preferred if it works, because *explicit* disabling is better than not, because autoconf setups often do 'autodetection' (see test case below)
• The build explicitly doesn't find or build with svn support, when the option is OFF but subversion is *installed*

ports-mgmt/portshaker/Makefile

23–25 ↗

(On Diff #91267)

It would seem these can go because the default option descriptions added in this diff cover them

www/trac-devel/Makefile

43–44 ↗

(On Diff #91267)

It would seem these can go because the default option descriptions added in this diff cover them

[loader]

Remove *_DESC that covered by the default descriptions, sort OPTIONS_DEFINE and OPTIONS_DEFAULT

[loader]

net-mgmt/rancid3/Makefile

54 ↗

(On Diff #91267)

The configure script does not find and build with svn or git automatically
unless --with-svn or --with-git is specified in CONFIGURE_ARGS.

${WRKSRC}/configure.ac:

133	# Configure for subversion revision control system instead of CVS.
134	SVN_FSTYPE="--fs-type fsfs"
135	AC_MSG_CHECKING([whether subversion])
136	AC_ARG_WITH(svn,
137		AS_HELP_STRING([--with-svn=fstype],
138				[use subversion instead of cvs, with optional svn fstype (fsfs|bdb)]),
139	[ case "$withval" in
140	  yes)
141		AC_MSG_RESULT(yes)
142		RCSSYS="svn"
143		;;
144	  fsfs)
145		AC_MSG_RESULT([yes fstype fsfs])
146		RCSSYS="svn"
147		SVN_FSTYPE="--fs-type fsfs"
148		;;
149	  bdb)
150		AC_MSG_RESULT([yes fstype bdb])
151		RCSSYS="svn"
152		SVN_FSTYPE="--fs-type bdb"
153		;;
154	  no)
155		AC_MSG_RESULT(no)
156		RCSSYS="cvs"
157		;;
158	  *)
159		AC_MSG_ERROR([unknown svn fs-type $withval])
160	  esac
161	], [AC_MSG_RESULT(no)
162	    AC_ARG_WITH(git,
163		AS_HELP_STRING([--with-git], [use git instead of cvs]),
164	    [ case "$withval" in
165	      yes)
166		AC_MSG_RESULT(yes)
167		RCSSYS="git"
168		;;
169	      no)
170		AC_MSG_RESULT(no)
171		RCSSYS="cvs"
172	      esac
173	    ], [AC_MSG_RESULT(no)
174	        RCSSYS="cvs"
175	    ])
176	])
177	AC_SUBST(RCSSYS)
178	AC_SUBST(SVN_FSTYPE)
179	rd_cv_RCSSYS=$RCSSYS
180

The value of RCSSYS only affects rancid.conf.sample in the final package:

${STAGEDIR}/usr/local/share/rancid/rancid.conf.sample:

63	# Select which RCS system to use, "cvs" (default), "svn" or "git".  Do not
64	# change this after CVSROOT has been created with rancid-cvs.  Changing between
65	# these requires manual conversions.
66	RCSSYS=cvs; export RCSSYS
67	#

[koobs]

I'm happy if you're happy. Ship it when ready :)

[koobs]

If you'd like, feel free to email maintainers privately as a heads up (in case they don't notice).

[loader]

added ports maintainers to reviewers, except devel/viewvc

devel/anjuta: [e-mail]
devel/git: [e-mail]
devel/ocaml-opam: [e-mail]
devel/thunar-vcs-plugin: [e-mail]
devel/viewvc-devel: [e-mail]
devel/viewvc: [e-mail]
net-mgmt/observium: [e-mail]
net-mgmt/rancid3: [e-mail]
ports-mgmt/portshaker: [e-mail]
security/hydra: [e-mail]
security/medusa: [e-mail]
shells/scponly: [e-mail]
www/trac-devel: [e-mail]

[dvl]

devel/viewvc/Makefile

1 ↗

(On Diff #91489)

devel/viewvc was removed on 26 Jun 2021

[hannes_mehnert.org]

For devel/ocaml-opam -- the version control systems are solely used for downloading remote repositories. This change looks good to me, thanks.

[koobs]

@loader Thanks for adding everyone to review. Commit does not block on maintainer approval.

[loader]

Many thanks to everyone who helped review this patch.