Page MenuHomeFreeBSD

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

Authored by loader on Oct 7 2018, 3:43 PM.

Details

Reviewers
koobs
Group Reviewers
portmgr
O5: Ports Framework(Owns No Changed Paths)
Summary

Proposed commit log message:

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

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


Reviewed_by: koobs
Approved by: koobs (mentor)
Differential_Revision: D17459

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 25519
Build 24137: arc lint + arc unit

Event Timeline

loader created this revision.Oct 7 2018, 3:43 PM
loader edited the summary of this revision. (Show Details)Oct 7 2018, 3:45 PM
mat removed a reviewer: portmgr.Oct 8 2018, 3:40 PM
koobs edited the summary of this revision. (Show Details)Nov 23 2018, 3:40 AM
koobs requested changes to this revision.Nov 23 2018, 3:44 AM

Please also include the updates to any ports that use these OPTIONS in their Makefile's already (thereby leveraging the new shared descriptions)

Apologies for the delayed review

This revision now requires changes to proceed.Nov 23 2018, 3:44 AM
koobs removed 1 blocking reviewer(s): portmgr.Nov 23 2018, 3:44 AM
koobs added a comment.Nov 23 2018, 3:45 AM

Changes to Mk/bsd.options.desc.mk don't explicitly require (block on) portmgr approval

# - This file is maintained by ports@FreeBSD.org so that entries can be added
#   to it easily.  Any sweeping changes should be approved by portmgr@.

Options_Desc_MAINTAINER=        ports@FreeBSD.org

Please also include the updates to any ports that use these OPTIONS in their Makefile's already (thereby leveraging the new shared descriptions)

Do I also need to add RSYNC_DESC, DARCS_DESC and BAZAAR_DESC into bsd.options.desc.mk?

https://svnweb.freebsd.org/ports/head/devel/ocaml-opam/Makefile?revision=483093&view=markup#l30

30      RSYNC_DESC=             Remote repository synchronized with Rsync
31      GIT_DESC=               Remote repository synchronized with git
32      HG_DESC=                Remote repository synchronized with mercurial
33      DARCS_DESC=             Remote repository synchronized with darcs

https://svnweb.freebsd.org/ports/head/devel/cvs2svn/Makefile?revision=479407&view=markup#l30

30      SUBVERSION_DESC=        Build with subversion support
31      GIT_DESC=               Build with git support
32      BAZAAR_DESC=            Build with Bazaar support

If we are going to change all these descriptions, I should open a Bugzilla report to get an approval from the maintainer for the ports like devel/ocaml-opam, right?

mat added a comment.Nov 25 2018, 11:57 AM

You can add whatever *default* description, but those descriptions are really bad most of the time, they only help if you already know what they do because they just say "FOO_DESC= foo support". If you don't know what foo is, or whatever it is used for, you can't know what it is going to change for the port. Most port should define their own _DESC to something really descriptive, that explains what it does.

koobs added a comment.Nov 27 2018, 9:21 AM

Please also include the updates to any ports that use these OPTIONS in their Makefile's already (thereby leveraging the new shared descriptions)

Do I also need to add RSYNC_DESC, DARCS_DESC and BAZAAR_DESC into bsd.options.desc.mk?
https://svnweb.freebsd.org/ports/head/devel/ocaml-opam/Makefile?revision=483093&view=markup#l30

30      RSYNC_DESC=             Remote repository synchronized with Rsync
31      GIT_DESC=               Remote repository synchronized with git
32      HG_DESC=                Remote repository synchronized with mercurial
33      DARCS_DESC=             Remote repository synchronized with darcs

https://svnweb.freebsd.org/ports/head/devel/cvs2svn/Makefile?revision=479407&view=markup#l30

30      SUBVERSION_DESC=        Build with subversion support
31      GIT_DESC=               Build with git support
32      BAZAAR_DESC=            Build with Bazaar support

If we are going to change all these descriptions, I should open a Bugzilla report to get an approval from the maintainer for the ports like devel/ocaml-opam, right?

Yes, I suppose it makes (logical) sense to include any/all other "revision control system" options that exist in ports that, as @mat said, use very generic default descriptions, of the form "{Build with|Include|Enable} $repository support".

The ocaml-opam options are bit too specific (in my opinion) to include here.

Please include the individual port changes in this diff too (the ports that will no longer have OPTIONS_DESC set after adding them to bsd.options.desc.mk

loader edited the summary of this revision. (Show Details)Nov 28 2018, 5:12 PM
loader updated this revision to Diff 51295.Nov 28 2018, 5:14 PM
loader edited the summary of this revision. (Show Details)

Add {BZR,CVS,RSYNC}_DESC and a few ports changes

loader edited the summary of this revision. (Show Details)Nov 28 2018, 5:14 PM

Here's a list of the ports that have the same description in bsd.options.desc.mk (changes are included in this diff):

GIT_DESC:
ports-mgmt/portest|ultima@FreeBSD.org|Git support
ports-mgmt/portshaker|romain@FreeBSD.org|git support
www/py-rhodecode|wg@FreeBSD.org|GIT support

HG_DESC:
ports-mgmt/portshaker|romain@FreeBSD.org|mercurial support

RSYNC_DESC:
ports-mgmt/portshaker|romain@FreeBSD.org|rsync support
shells/scponly|garga@FreeBSD.org|rsync support

loader added a comment.EditedNov 28 2018, 5:21 PM

These ports remain unchanged:

BAZAAR_DESC:
devel/cvs2svn|ohauer@FreeBSD.org|Build with Bazaar support
BZR_DESC:
deskutils/zim|rm@FreeBSD.org|Version control for notebooks using bzr

CVS_DESC:
devel/git|garga@FreeBSD.org|Enable CVS support
devel/git-gui|garga@FreeBSD.org|Enable CVS support
devel/git-lite|garga@FreeBSD.org|Enable CVS support
devel/git-subversion|garga@FreeBSD.org|Enable CVS support

DARCS_DESC:
devel/ocaml-opam|hannes@mehnert.org|Remote repository synchronized with darcs

GIT_DESC:
deskutils/zim|rm@FreeBSD.org|Version control for notebooks using git
devel/cvs2svn|ohauer@FreeBSD.org|Build with git support
devel/ocaml-opam|hannes@mehnert.org|Remote repository synchronized with git
devel/thunar-vcs-plugin|sergey.dyatko@gmail.com|Git support
devel/tig|tobik@FreeBSD.org|Install devel/git as runtime dependency
sysutils/password-store|rene@FreeBSD.org|Enable git storage
www/moinmoincli|0mp@FreeBSD.org|Install git to use git-diff(1) instead of diff(1) for colored output
www/py-kallithea|freebsd@skinc.ru|Git repositories support

HG_DESC:
devel/ocaml-opam|hannes@mehnert.org|Remote repository synchronized with mercurial

MERCURIAL_DESC:
deskutils/zim|rm@FreeBSD.org|Version control for notebooks using hg

RSYNC_DESC:
devel/bzrtools|fullermd@over-yonder.net|rsync for rspush
devel/ocaml-opam|hannes@mehnert.org|Remote repository synchronized with Rsync
devel/p5-SVN-Notify-Mirror|bofh@FreeBSD.org|Enable rsync for remote working copy update
net/prosearch|ports@FreeBSD.org|crawler with RSYNC
sysutils/zxfer|ports@scaleengine.com|Enable RSYNC Support

Should I add these ports maintainers to reviewers?
garga,romain,ultima,wg

koobs requested changes to this revision.Jul 22 2019, 3:37 AM

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

Mk/bsd.options.desc.mk
41

Rename to full name (BAZAAR)

158

Rename to MERCURIAL

This revision now requires changes to proceed.Jul 22 2019, 3:37 AM
koobs edited the summary of this revision. (Show Details)Jul 22 2019, 3:38 AM

Mk/bsd.options.desc.mk MAINTAINER is:

Options_Desc_MAINTAINER= ports@FreeBSD.org

No need for protmgr blocking review

koobs retitled this revision from Mk/bsd.options.desc.mk: Add GIT and MERCURIAL descriptions to Mk/bsd.options.desc.mk: Add version control systems OPTION descriptions.Jul 22 2019, 3:39 AM
koobs removed a reviewer: portmgr.
adamw added a subscriber: adamw.Jul 22 2019, 12:07 PM

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

I'm sick of LibXYZ support descriptions.

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?

koobs added a comment.Jul 23 2019, 4:53 AM

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 added a comment.Jul 23 2019, 4:54 AM

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 updated this revision to Diff 60046.Jul 23 2019, 2:45 PM

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

loader edited the summary of this revision. (Show Details)Jul 23 2019, 2:50 PM
loader marked 2 inline comments as done.Jul 23 2019, 2:53 PM

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 added a comment.EditedJul 26 2019, 4:51 AM

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 requested changes to this revision.Jul 26 2019, 5:10 AM
koobs added inline comments.
Mk/bsd.options.desc.mk
432

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

devel/anjuta/Makefile
82

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

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

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

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

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

This revision now requires changes to proceed.Jul 26 2019, 5:10 AM
adamw added inline comments.Jul 26 2019, 12:00 PM
Mk/bsd.options.desc.mk
432

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 updated this revision to Diff 60169.Jul 26 2019, 3:58 PM

Remove RSYNC_DESC and keep some of the port defined descritions.

loader edited the summary of this revision. (Show Details)Jul 26 2019, 4:01 PM
loader marked 7 inline comments as done.Jul 26 2019, 4:36 PM
loader added inline comments.
Mk/bsd.options.desc.mk
432

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

Sorry, typo.