[gentoo-portage-dev] [PATCH 0/2] Use consistent list of booleans & validate verify-commit-signature

[gentoo-portage-dev] [PATCH 0/2] Use consistent list of booleans & validate verify-commit-signature

[Wynn Wolf Arbor]

Hi,

Whilst configuring a bunch of overlay repositories via repos.conf, I
discovered that not all boolean options take the same values. Some only
take 'true' and 'false', others are documented only as 'yes' or 'no',
but take 'true', 'false', 'yes', and 'no'. This is inconsistent and can
lead to very confusing outcomes, so I decided to write a patch.

I came across https://bugs.gentoo.org/703698 whilst working on this, and
since my work was tangentially related, decided to fix that as well.

Wynn Wolf Arbor (2):
      repos.conf: Use consistent list of values for boolean options
      git: Verify boolean values passed to sync-git-verify-commit-signature

 lib/portage/repository/config.py         |  4 +--
 lib/portage/sync/modules/git/__init__.py | 11 ++++++++
 lib/portage/sync/modules/git/git.py      |  2 +-
 lib/portage/sync/modules/rsync/rsync.py  |  2 +-
 man/portage.5                            | 45 +++++++++++++++-----------------
 5 files changed, 36 insertions(+), 28 deletions(-)

[gentoo-portage-dev] [PATCH 1/2] repos.conf: Use consistent list of values for boolean options

[Wynn Wolf Arbor]

Valid values for boolean options in repos.conf are currently not managed
in a consistent manner. Some options only support 'true' and 'false',
whilst others additionally support 'yes' and 'no'. Using the latter
forms on options that do not support them will lead to unexpected
behaviour. For example, an option checking for 'true' will be disabled
when 'yes' is used. This is counter-intuitive and adds additional
burden: the user has to look up in the manual which form is accepted by
which option.

Have all boolean options consistently accept 'yes', 'no', 'true', and
'false' and make sure to document this in the portage(5) manual.
Additionally, document the default value for each.

Signed-off-by: Wynn Wolf Arbor <wolf@oriole.systems>
---
 lib/portage/repository/config.py        |  4 +--
 lib/portage/sync/modules/git/git.py     |  2 +-
 lib/portage/sync/modules/rsync/rsync.py |  2 +-
 man/portage.5                           | 45 ++++++++++++-------------
 4 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/lib/portage/repository/config.py b/lib/portage/repository/config.py
index 6155c130a..3a5425be7 100644
--- a/lib/portage/repository/config.py
+++ b/lib/portage/repository/config.py
@@ -220,10 +220,10 @@ class RepoConfig(object):
 		self.sync_depth = repo_opts.get('sync-depth')
 
 		self.sync_hooks_only_on_change = repo_opts.get(
-			'sync-hooks-only-on-change', 'false').lower() == 'true'
+			'sync-hooks-only-on-change', 'false').lower() in ('true', 'yes')
 
 		self.strict_misc_digests = repo_opts.get(
-			'strict-misc-digests', 'true').lower() == 'true'
+			'strict-misc-digests', 'true').lower() in ('true', 'yes')
 
 		self.sync_allow_hardlinks = repo_opts.get(
 			'sync-allow-hardlinks', 'true').lower() in ('true', 'yes')
diff --git a/lib/portage/sync/modules/git/git.py b/lib/portage/sync/modules/git/git.py
index 7df4b6d61..1447dca0e 100644
--- a/lib/portage/sync/modules/git/git.py
+++ b/lib/portage/sync/modules/git/git.py
@@ -204,7 +204,7 @@ class GitSync(NewBase):
 
 	def verify_head(self, revision='-1'):
 		if (self.repo.module_specific_options.get(
-				'sync-git-verify-commit-signature', 'false') != 'true'):
+				'sync-git-verify-commit-signature', 'false') not in ('true', 'yes')):
 			return True
 
 		if self.repo.sync_openpgp_key_path is not None:
diff --git a/lib/portage/sync/modules/rsync/rsync.py b/lib/portage/sync/modules/rsync/rsync.py
index a40e1c854..aed8a8dc7 100644
--- a/lib/portage/sync/modules/rsync/rsync.py
+++ b/lib/portage/sync/modules/rsync/rsync.py
@@ -68,7 +68,7 @@ class RsyncSync(NewBase):
 		out = portage.output.EOutput(quiet=quiet)
 		syncuri = self.repo.sync_uri
 		if self.repo.module_specific_options.get(
-			'sync-rsync-vcs-ignore', 'false').lower() == 'true':
+			'sync-rsync-vcs-ignore', 'false').lower() in ('true', 'yes'):
 			vcs_dirs = ()
 		else:
 			vcs_dirs = frozenset(VCS_DIRS)
diff --git a/man/portage.5 b/man/portage.5
index 136ebaafe..7a5817134 100644
--- a/man/portage.5
+++ b/man/portage.5
@@ -916,13 +916,11 @@ When 'force = aliases' attribute is not set, \fBegencache\fR(1),
 since operations performed by these tools are inherently
 \fBnot\fR \fIsite\-specific\fR.
 .TP
-.B auto\-sync
+.B auto\-sync = yes|no|true|false
 This setting determines if the repo will be synced during "\fBemerge \-\-sync\fR" or
 "\fBemaint sync \-\-auto\fR" runs.  This allows for repositories to be synced only when
 desired via "\fBemaint sync \-\-repo foo\fR".
 .br
-Valid values: yes, no, true, false.
-.br
 If unset, the repo will be treated as set
 yes, true.
 .TP
@@ -966,20 +964,18 @@ since operations performed by these tools are inherently
 .B priority
 Specifies priority of given repository.
 .TP
-.B strict\-misc\-digests
+.B strict\-misc\-digests = yes|no|true|false
 This setting determines whether digests are checked for files declared
 in the Manifest with MISC type (includes ChangeLog and metadata.xml
-files). Defaults to true.
-.br
-Valid values: true, false.
+files). Defaults to yes, true.
 .TP
-.B sync\-allow\-hardlinks = yes|no
+.B sync\-allow\-hardlinks = yes|no|true|false
 Allow sync plugins to use hardlinks in order to ensure that a repository
 remains in a valid state if something goes wrong during the sync operation.
 For example, if signature verification fails during a sync operation,
 the previous state of the repository will be preserved. This option may
 conflict with configurations that restrict the use of hardlinks, such as
-overlay filesystems.
+overlay filesystems. Defaults to yes, true.
 .TP
 .B sync\-cvs\-repo
 Specifies CVS repository.
@@ -1016,16 +1012,17 @@ See also example for sync-git-clone-env.
 .B sync\-git\-pull\-extra\-opts
 Extra options to give to git when updating repository (git pull).
 .TP
-.B sync\-git\-verify\-commit\-signature = true|false
+.B sync\-git\-verify\-commit\-signature = yes|no|true|false
 Require the top commit in the repository to contain a good OpenPGP
-signature. Defaults to false.
+signature. Defaults to no, false.
 .TP
-.B sync\-hooks\-only\-on\-change
+.B sync\-hooks\-only\-on\-change = yes|no|true|false
 If set to true, then sync of a given repository will not trigger postsync
 hooks unless hooks would have executed for a master repository or the
-repository has changed since the previous sync operation.
+repository has changed since the previous sync operation. Defaults to
+no, false.
 .TP
-.B sync\-rcu = yes|no
+.B sync\-rcu = yes|no|true|false
 Enable read\-copy\-update (RCU) behavior for sync operations. The current
 latest immutable version of a repository will be referenced by a symlink
 found where the repository would normally be located (see the \fBlocation\fR
@@ -1156,10 +1153,10 @@ Pass \fIname\fR as the `gpg \-\-keyserver` argument. Refer to the
 \fBgpg\fR(1) man page for information about the `gpg \-\-keyserver`
 \fIname\fR format.
 .TP
-.B sync-rsync-vcs-ignore = true|false
+.B sync\-rsync\-vcs\-ignore = yes|no|true|false
 Ignore vcs directories that may be present in the repository. It is the
 user's responsibility to set sync-rsync-extra-opts to protect vcs
-directories if appropriate.
+directories if appropriate. Defaults to no, false.
 .TP
 .B sync\-rsync\-verify\-jobs = 1
 Number of parallel jobs to use when verifying nested Manifests. When
@@ -1171,21 +1168,21 @@ Defaults to 1.
 Warn if repository is older than the specified number of days. Disabled
 when 0. Defaults to disabled.
 .TP
-.B sync\-rsync\-verify\-metamanifest = yes|no
+.B sync\-rsync\-verify\-metamanifest = yes|no|true|false
 Require the repository to contain a signed MetaManifest and verify
-it using \fBapp\-portage/gemato\fR. Defaults to no.
+it using \fBapp\-portage/gemato\fR. Defaults to no, false.
 .TP
-.B sync\-webrsync\-delta = true|false
+.B sync\-webrsync\-delta = yes|no|true|false
 Use \fBapp\-portage/emerge\-delta\-webrsync\fR to minimize bandwidth.
-Defaults to false.
+Defaults to no, false.
 .TP
-.B sync\-webrsync\-keep\-snapshots = true|false
-Keep snapshots in \fBDISTDIR\fR (do not delete). Defaults to false.
+.B sync\-webrsync\-keep\-snapshots = yes|no|true|false
+Keep snapshots in \fBDISTDIR\fR (do not delete). Defaults to no, false.
 .TP
-.B sync\-webrsync\-verify\-signature = true|false
+.B sync\-webrsync\-verify\-signature = yes|no|true|false
 Require the detached tarball signature to contain a good OpenPGP
 signature. This uses the OpenPGP key(ring) specified by the
-sync\-openpgp\-key\-path setting. Defaults to false.
+sync\-openpgp\-key\-path setting. Defaults to no, false.
 
 .RE
 

base-commit: c36e8dfaf808f68586e7519232e072490ccc51db
-- 
2.27.0

[gentoo-portage-dev] [PATCH 2/2] git: Verify boolean values passed to sync-git-verify-commit-signature

[Wynn Wolf Arbor]

Currently, if 'sync-git-verify-commit-signature' is set to anything
other than 'yes', 'no', 'true', or 'false', its value is ignored
silently and nothing is verified because the option defaults to 'false'.

Introduce a check to CheckGitConfig that warns the user if their input
is invalid.

Closes: https://bugs.gentoo.org/703698
Signed-off-by: Wynn Wolf Arbor <wolf@oriole.systems>
---
 lib/portage/sync/modules/git/__init__.py | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/portage/sync/modules/git/__init__.py b/lib/portage/sync/modules/git/__init__.py
index 270d97186..913e391cb 100644
--- a/lib/portage/sync/modules/git/__init__.py
+++ b/lib/portage/sync/modules/git/__init__.py
@@ -14,6 +14,7 @@ class CheckGitConfig(CheckSyncConfig):
 	def __init__(self, repo, logger):
 		CheckSyncConfig.__init__(self, repo, logger)
 		self.checks.append('check_depth')
+		self.checks.append('check_verify_commit_signature')
 
 	def check_depth(self):
 		for attr in ('clone_depth', 'sync_depth'):
@@ -33,6 +34,16 @@ class CheckGitConfig(CheckSyncConfig):
 			else:
 				setattr(self.repo, attr, d)
 
+	def check_verify_commit_signature(self):
+		v = self.repo.module_specific_options.get(
+			'sync-git-verify-commit-signature', 'false')
+
+		if v not in ['yes', 'no', 'true', 'false']:
+			writemsg_level("!!! %s\n" %
+				_("sync-git-verify-commit-signature not one of: %s")
+				% ('{yes, no, true, false}'),
+				level=self.logger.ERROR, noiselevel=-1)
+
 
 module_spec = {
 	'name': 'git',
-- 
2.27.0

Re: [gentoo-portage-dev] [PATCH 0/2] Use consistent list of booleans & validate verify-commit-signature

[Zac Medico]

[-- Attachment #1.1: Type: text/plain, Size: 1432 bytes --]

On 7/2/20 8:50 AM, Wynn Wolf Arbor wrote:
> Hi,
> 
> Whilst configuring a bunch of overlay repositories via repos.conf, I
> discovered that not all boolean options take the same values. Some only
> take 'true' and 'false', others are documented only as 'yes' or 'no',
> but take 'true', 'false', 'yes', and 'no'. This is inconsistent and can
> lead to very confusing outcomes, so I decided to write a patch.
> 
> I came across https://bugs.gentoo.org/703698 whilst working on this, and
> since my work was tangentially related, decided to fix that as well.
> 
> Wynn Wolf Arbor (2):
>       repos.conf: Use consistent list of values for boolean options
>       git: Verify boolean values passed to sync-git-verify-commit-signature
> 
>  lib/portage/repository/config.py         |  4 +--
>  lib/portage/sync/modules/git/__init__.py | 11 ++++++++
>  lib/portage/sync/modules/git/git.py      |  2 +-
>  lib/portage/sync/modules/rsync/rsync.py  |  2 +-
>  man/portage.5                            | 45 +++++++++++++++-----------------
>  5 files changed, 36 insertions(+), 28 deletions(-)


Thanks! I've merged both patches, with a couple of additional lower()
calls for case insensitivity:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=141ef203661248a2c29945f8c6770ce0c242eaf0
https://gitweb.gentoo.org/proj/portage.git/commit/?id=33b08baff4825bf84f639cf213de92ed36f76771
-- 
Thanks,
Zac




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 981 bytes --]