Page MenuHomeFreeBSD
Differential D17459

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

Authored by loader on Oct 7 2018, 3:43 PM.
Tags
None
Referenced Files
F105798599: D17459.diff
Fri, Dec 20, 8:44 PM
F105771063: D17459.id91534.diff
Fri, Dec 20, 11:28 AM
Unknown Object (File)
Sun, Dec 15, 4:31 PM
Unknown Object (File)
Sat, Dec 14, 3:32 PM
Unknown Object (File)
Thu, Dec 12, 12:05 PM
Unknown Object (File)
Thu, Dec 12, 6:06 AM
Unknown Object (File)
Wed, Dec 11, 9:22 PM
Unknown Object (File)
Mon, Dec 9, 3:59 AM
View All Files
Subscribers
adamw
mat

Details

Reviewers
koobs
kevans
garga
dvl
feld
marcus
romain
rm
dbaio
samm
hannes_mehnert.org
sergey.dyatko_gmail.com
Group Reviewers
portmgr
gnome
Commits
R11:f76aa00290b6: Mk/bsd.options.desc.mk: Add shared version control OPTIONS & descriptions
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)

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 31917
Build 29468: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
koobs added a comment.Jul 23 2019, 4:53 AM
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 added a comment.Jul 23 2019, 4:54 AM
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 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

Owners added a reviewer: O5: Ports Framework.Jul 23 2019, 2:45 PM
Harbormaster completed remote builds in B25468: Diff 60046.Jul 23 2019, 2:45 PM
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
loader added a comment.Jul 23 2019, 3:11 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
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 requested changes to this revision.Jul 26 2019, 5:10 AM
koobs added inline comments.
Mk/bsd.options.desc.mk
434

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
47–49

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
41

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
434

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.

Harbormaster completed remote builds in B25519: Diff 60169.Jul 26 2019, 3:58 PM
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
434

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.

koobs added a comment.Jun 22 2020, 3:17 AM

@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 updated this revision to Diff 73560.Jun 24 2020, 5:57 AM
loader marked an inline comment as done.

Update patch on a recent ports tree (r539330)

Harbormaster completed remote builds in B31917: Diff 73560.Jun 24 2020, 5:57 AM
adamw added inline comments.Jun 24 2020, 6:58 AM
Mk/bsd.options.desc.mk
159

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

loader updated this revision to Diff 73566.Jun 24 2020, 7:47 AM

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

Harbormaster completed remote builds in B31920: Diff 73566.Jun 24 2020, 7:47 AM
loader edited the summary of this revision. (Show Details)Jun 24 2020, 7:51 AM
loader marked an inline comment as done.Jun 24 2020, 7:53 AM
loader added inline comments.
Mk/bsd.options.desc.mk
159

Thanks @adamw.

loader edited the summary of this revision. (Show Details)Jun 24 2020, 8:02 AM
koobs added a reviewer: kevans.Jul 22 2020, 3:28 AM

@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 removed reviewers: portmgr, O5: Ports Framework.Jul 22 2020, 3:30 AM
Herald added a reviewer: portmgr. · View Herald TranscriptJul 22 2020, 3:30 AM
koobs added a comment.Jul 22 2020, 3:30 AM

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

loader updated this revision to Diff 91267.Jun 23 2021, 3:27 PM
loader marked an inline comment as done.

Update patch for the latest ports tree

Harbormaster completed remote builds in B40060: Diff 91267.Jun 23 2021, 3:27 PM
loader edited the summary of this revision. (Show Details)Jun 23 2021, 3:34 PM
loader added a comment.Jun 23 2021, 3:42 PM

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
loader edited the summary of this revision. (Show Details)Jun 23 2021, 4:19 PM
koobs edited the summary of this revision. (Show Details)Jun 29 2021, 2:35 AM
koobs requested changes to this revision.Jun 29 2021, 2:53 AM

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
82

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

Alpha sort these

31–34

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–30

This can go

net-mgmt/rancid3/Makefile
47–49

These 3 seem to be covered by the default descriptions

55

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
24–25

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

www/trac-devel/Makefile
47–48

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

This revision now requires changes to proceed.Jun 29 2021, 2:53 AM
loader updated this revision to Diff 91489.Jun 29 2021, 8:53 AM
loader marked 7 inline comments as done.

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

Harbormaster completed remote builds in B40158: Diff 91489.Jun 29 2021, 8:54 AM
loader added inline comments.Jun 29 2021, 8:55 AM
net-mgmt/rancid3/Makefile
55

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	#
loader edited the summary of this revision. (Show Details)Jun 29 2021, 8:57 AM
koobs accepted this revision.Jun 29 2021, 12:03 PM

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

koobs added a comment.Jun 29 2021, 12:03 PM

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

loader edited the summary of this revision. (Show Details)Jun 29 2021, 1:43 PM
loader added reviewers: gnome, garga, dvl, feld, marcus, romain, rm, dbaio, samm, hannes_mehnert.org, sergey.dyatko_gmail.com.
loader added a comment.Jun 29 2021, 1:46 PM

added ports maintainers to reviewers, except devel/viewvc

devel/anjuta: gnome@FreeBSD.org
devel/git: garga@FreeBSD.org
devel/ocaml-opam: hannes@mehnert.org
devel/thunar-vcs-plugin: sergey.dyatko@gmail.com
devel/viewvc-devel: dvl@FreeBSD.org
devel/viewvc: ports@FreeBSD.org
net-mgmt/observium: feld@FreeBSD.org
net-mgmt/rancid3: marcus@FreeBSD.org
ports-mgmt/portshaker: romain@FreeBSD.org
security/hydra: rm@FreeBSD.org
security/medusa: dbaio@FreeBSD.org
shells/scponly: garga@FreeBSD.org
www/trac-devel: samm@FreeBSD.org

dvl accepted this revision.Jun 29 2021, 2:05 PM
dvl added inline comments.
devel/viewvc/Makefile
1–2

devel/viewvc was removed on 26 Jun 2021

garga accepted this revision.Jun 29 2021, 3:02 PM
hannes_mehnert.org accepted this revision.Jun 29 2021, 3:49 PM

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

rm accepted this revision.Jun 29 2021, 4:11 PM
dbaio accepted this revision.Jun 29 2021, 4:51 PM
romain accepted this revision.Jun 30 2021, 12:14 AM
koobs added a comment.Jun 30 2021, 12:20 AM

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

loader added a comment.Jun 30 2021, 5:24 AM

Many thanks to everyone who helped review this patch.

This revision was not accepted when it landed; it landed in state Needs Review.Jun 30 2021, 5:26 AM
Closed by commit R11:f76aa00290b6: Mk/bsd.options.desc.mk: Add shared version control OPTIONS & descriptions (authored by loader). · Explain Why
This revision was automatically updated to reflect the committed changes.
loader added a commit: R11:f76aa00290b6: Mk/bsd.options.desc.mk: Add shared version control OPTIONS & descriptions.

Revision Contents
Changeset List

PathSize
Mk/
8 lines
deskutils/
zim/
7 lines
devel/
anjuta/
12 lines
12 lines
git/
23 lines
8 lines
ocaml-opam/
7 lines
thunar-vcs-plugin/
11 lines
2 lines
viewvc/
8 lines
net-mgmt/
observium/
8 lines
rancid3/
10 lines
ports-mgmt/
portest/
2 lines
portshaker/
12 lines
security/
hydra/
5 lines
medusa/
9 lines
2 lines
shells/
scponly/
12 lines
textproc/
docproj/
10 lines
www/
py-kallithea/
2 lines
trac-devel/
7 lines
trac/
7 lines

Diff 73560

View Options

Mk/bsd.options.desc.mk

Loading...
View Options

deskutils/zim/Makefile

Loading...
View Options

devel/anjuta/Makefile

Loading...
View Options

devel/anjuta/pkg-plist

Loading...
View Options

devel/git/Makefile

Loading...
View Options

devel/git/pkg-plist

Loading...
View Options

devel/ocaml-opam/Makefile

Loading...
View Options

devel/thunar-vcs-plugin/Makefile

Loading...
View Options

devel/thunar-vcs-plugin/pkg-plist

Loading...
View Options

devel/viewvc/Makefile

Loading...
View Options

net-mgmt/observium/Makefile

Loading...
View Options

net-mgmt/rancid3/Makefile

Loading...
View Options

ports-mgmt/portest/Makefile

Loading...
View Options

ports-mgmt/portshaker/Makefile

Loading...
View Options

security/hydra/Makefile

Loading...
View Options

security/medusa/Makefile

Loading...
View Options

security/medusa/pkg-plist

Loading...
View Options

shells/scponly/Makefile

Loading...
View Options

textproc/docproj/Makefile

Loading...
View Options

www/py-kallithea/Makefile

Loading...
View Options

www/trac-devel/Makefile

Loading...
View Options

www/trac/Makefile

Loading...