Do not remove groups by default. Issue rpm-software-management#265

Revert the logic introduced in commit 6a19ac. Now, groups are kept untouched by default. Introduce a new option '--remove-groups' that removes Group tags from the specfile. Remove '--preserve_groups' option.
kstreitova · Nov 8, 2019 · fcbbbb3 · fcbbbb3
1 parent 1a397b8
commit fcbbbb3
Show file tree

Hide file tree

Showing 18 changed files with 46 additions and 21 deletions.
diff --git a/spec_cleaner/__init__.py b/spec_cleaner/__init__.py
@@ -60,7 +60,7 @@ def process_args(argv: List[str]) -> Dict[str, Any]:
     )
     parser.add_argument('--no-copyright', action='store_true', help='do not include official SUSE copyright hear and just keep what is present')
     parser.add_argument('--libexecdir', action='store_true', help='convert /usr/lib to %%{_libexecdir}')
-    parser.add_argument('--preserve-groups', action='store_true', help='validate group tags instead of removing them')
+    parser.add_argument('--remove-groups', action='store_true', help='remove groups from the specfile.')
     parser.add_argument(
         '--copyright-year',
         metavar='YYYY',

diff --git a/spec_cleaner/rpmcleaner.py b/spec_cleaner/rpmcleaner.py
@@ -92,10 +92,10 @@ def __init__(self, options: Dict[str, Any]) -> None:
         if self.options['perl']:
             self.options['perl_conversions'] = read_perl_changes()
         self.options['license_conversions'] = read_licenses_changes()
-        if self.options['preserve_groups']:
-            self.options['allowed_groups'] = read_group_changes()
-        else:
+        if self.options['remove_groups']:
             self.options['allowed_groups'] = None
+        else:
+            self.options['allowed_groups'] = read_group_changes()
         self.options['reg'] = Regexp(self.options['unbrace_keywords'])
 
         # If gvim is used for the diff then run it in foreground mode

diff --git a/spec_cleaner/rpmpreamble.py b/spec_cleaner/rpmpreamble.py
@@ -60,6 +60,8 @@ def __init__(self, options):
         self.tex = options['tex']
         # are we supposed to keep empty lines intact?
         self.keep_space = options['keep_space']
+        # do we want to remove groups from the specfile?
+        self.remove_groups = options['remove_groups']
         # dict of license replacement options
         self.license_conversions = options['license_conversions']
         # dict of pkgconfig and other conversions
@@ -557,18 +559,19 @@ def add(self, line):
             return
 
         elif self.reg.re_group.match(line):
-            # if we do not have list of groups we are removing them in non
-            # minimal mode
-            if not self.minimal and not self.allowed_groups:
+            # remove groups if requested
+            if not self.minimal and self.remove_groups:
                 return
 
-            # otherwise we scan up and warn if the group does not match allowed
-            # list of groups
+            # validate (if we have a list of groups)
             match = self.reg.re_group.match(line)
             value = match.group(1)
-            if not self.minimal:
-                if self.previous_line and not self.previous_line.startswith('# FIXME') and value not in self.allowed_groups:
-                    self.paragraph.current_group.append('# FIXME: use correct group, see "https://en.opensuse.org/openSUSE:Package_group_guidelines"')
+            if not self.minimal and self.allowed_groups:
+                if self.previous_line and not self.previous_line.startswith(
+                        '# FIXME') and value not in self.allowed_groups:
+                    self.paragraph.current_group.append(
+                        '# FIXME: use correct group or remove it,'
+                        ' see "https://en.opensuse.org/openSUSE:Package_group_guidelines"')
             self._add_line_value_to('group', value)
             return
 

diff --git a/tests/acceptance-tests.py b/tests/acceptance-tests.py
@@ -51,7 +51,7 @@ class TestCompare(object):
         'no_copyright': True,
         'libexecdir': True,
         'copyright_year': 2013,
-        'preserve_groups': False,
+        'remove_groups': False,
         'tex': False,
         'perl': False,
         'cmake': False,
@@ -151,7 +151,7 @@ def test_unicode(self, tmpfile):
             ('tex.spec', 'tex', {'tex': True}),
             ('perl.spec', 'perl', {'perl': True}),
             ('cmake.spec', 'cmake', {'cmake': True}),
-            ('langpackage.spec', 'group', {'preserve_groups': True}),
+            ('langpackage.spec', 'group', {'remove_groups': True}),
         ],
     )
     def test_single_output(self, tmpfile, test, compare_dir, options):

diff --git a/tests/group/langpackage.spec b/tests/group/langpackage.spec
@@ -1,24 +1,17 @@
 %package -n something-lang
 # FIXME: consider using %%lang_package macro
 Summary:        Something
-# FIXME: use correct group, see "https://en.opensuse.org/openSUSE:Package_group_guidelines"
-Group:          Whatever
 
 %package lang
 # FIXME: consider using %%lang_package macro
 Summary:        Something
-# FIXME: use correct group, see "https://en.opensuse.org/openSUSE:Package_group_guidelines"
-Group:          Whatever
 
 %package lang
 # I have reason not to convert this to lang macro
 Summary:        Something
-# FIXME: use correct group, see "https://en.opensuse.org/openSUSE:Package_group_guidelines"
-Group:          Whatever
 
 %package -n %{_name}
 Summary:        Evolution Plugin for RSS Feeds Support
-Group:          Productivity/Networking/Email/Clients
 Recommends:     %{_name}-lang
 Provides:       %{name} = %{version}
 

diff --git a/tests/out/conditionalpkgs.spec b/tests/out/conditionalpkgs.spec
@@ -33,13 +33,15 @@ http://httpd.apache.org/docs-2.2/upgrading.html.
 %if %{worker}
 %package worker
 Summary:        Apache 2 worker MPM (Multi-Processing Module)
+Group:          Productivity/Networking/Web/Servers
 Requires:       %{name} = %{version}
 Provides:       %{pname}-MPM
 %endif
 
 %if %{prefork}
 %package prefork
 Summary:        Apache 2 "prefork" MPM (Multi-Processing Module)
+Group:          Productivity/Networking/Web/Servers
 Requires:       %{name} = %{version}
 Provides:       %{pname}-MPM
 %if 0%{?suse_version} >= 901 && 0%{?sles_version} != 9
@@ -50,13 +52,15 @@ Provides:       apache:%{_sbindir}/httpd
 %if %{event}
 %package event
 Summary:        Apache 2 event MPM (Multi-Processing Module)
+Group:          Productivity/Networking/Web/Servers
 Requires:       %{name} = %{version}
 Provides:       %{pname}-MPM
 %endif
 
 %if %{itk}
 %package itk
 Summary:        Apache 2 "ITK" MPM (Multi-Processing Module)
+Group:          Productivity/Networking/Web/Servers
 Requires:       %{name} = %{version}
 Provides:       %{pname}-MPM
 %endif

diff --git a/tests/out/conditions.spec b/tests/out/conditions.spec
@@ -6,6 +6,7 @@
 Version:        2.8.2
 Summary:        AppArmor userlevel parser utility
 License:        GPL-2.0-or-later
+Group:          Productivity/Networking/Security
 Source0:        apparmor-%{version}.tar.gz
 Source1:        apparmor-%{version}.tar.gz.asc
 Source2:        %{name}.keyring

diff --git a/tests/out/enhances.spec b/tests/out/enhances.spec
@@ -3,6 +3,7 @@ Version:        1.9.1
 Release:        0
 Summary:        Feed entropy into random pool
 License:        GPL-2.0-or-later
+Group:          System/Daemons
 Enhances:       smtp_daemon
 Supplements:    packageand(%{name}:nautilus}
 

diff --git a/tests/out/interestingheader.spec b/tests/out/interestingheader.spec
@@ -8,6 +8,7 @@ Release:        0
 Summary:        .spec file cleaner
 Summary(de):    Ein Synthesizer der dritten Art
 License:        GPL-2.0-or-later
+Group:          Productivity/Multimedia/Sound/Midi
 URL:            http://www.dinisnoise.org/
 Source:         %{name}-%{version}.tar.bz2
 Source1:        %{name}.png

diff --git a/tests/out/langpackage.spec b/tests/out/langpackage.spec
@@ -1,17 +1,24 @@
 %package -n something-lang
 # FIXME: consider using %%lang_package macro
 Summary:        Something
+# FIXME: use correct group or remove it, see "https://en.opensuse.org/openSUSE:Package_group_guidelines"
+Group:          Whatever
 
 %package lang
 # FIXME: consider using %%lang_package macro
 Summary:        Something
+# FIXME: use correct group or remove it, see "https://en.opensuse.org/openSUSE:Package_group_guidelines"
+Group:          Whatever
 
 %package lang
 # I have reason not to convert this to lang macro
 Summary:        Something
+# FIXME: use correct group or remove it, see "https://en.opensuse.org/openSUSE:Package_group_guidelines"
+Group:          Whatever
 
 %package -n %{_name}
 Summary:        Evolution Plugin for RSS Feeds Support
+Group:          Productivity/Networking/Email/Clients
 Recommends:     %{_name}-lang
 Provides:       %{name} = %{version}
 

diff --git a/tests/out/mingw32-clutter.spec b/tests/out/mingw32-clutter.spec
@@ -22,6 +22,8 @@ Version:        1.6.20
 Release:        0
 Summary:        The clutter library
 License:        LGPL-2.1-or-later
+# FIXME: use correct group or remove it, see "https://en.opensuse.org/openSUSE:Package_group_guidelines"
+Group:          Development/Libraries
 URL:            http://clutter-project.org/
 Source:         http://www.clutter-project.org/sources/clutter/1.5/clutter-%{version}.tar.bz2
 Patch0:         clutter-1.6.14-windows.patch
@@ -63,6 +65,7 @@ shared library.
 
 %package -n mingw32-libclutter-win32-1_0-0
 Summary:        MinGW Windows port of the Clutter library
+Group:          System/Libraries
 Obsoletes:      mingw32-clutter
 Provides:       mingw32-clutter
 
@@ -73,6 +76,8 @@ shared library.
 
 %package devel
 Summary:        The clutter library (Development)
+# FIXME: use correct group or remove it, see "https://en.opensuse.org/openSUSE:Package_group_guidelines"
+Group:          Development/Libraries
 Requires:       mingw32-glee-devel
 
 %description devel

diff --git a/tests/out/patterns.spec b/tests/out/patterns.spec
@@ -1,6 +1,7 @@
 %package dhcp_dns_server
 %pattern_serverfunctions
 Summary:        DHCP and DNS Server
+Group:          Metapackages
 Provides:       pattern() = dhcp_dns_server
 Provides:       pattern-icon() = yast-dns-server
 Provides:       pattern-order() = 3040
@@ -15,6 +16,7 @@ Recommends:     wget
 %package devel_gnome
 %pattern_development
 Summary:        GNOME Development
+Group:          Metapackages
 Provides:       pattern() = devel_gnome
 Provides:       pattern-icon() = pattern-gnome-devel
 # Don't freely assign order values, contact release managers

diff --git a/tests/out/providesobsoletes.spec b/tests/out/providesobsoletes.spec
@@ -1,5 +1,6 @@
 %package -n %{libname}
 Summary:        Library for Importing WordPerfect (tm) Documents
+Group:          System/Libraries
 Provides:       NetworkManager-lang = %(rpm -q --queryformat '%{VERSION}' NetworkManager-lang)
 Obsoletes:      NetworkManager-lang < %(rpm -q --queryformat '%{VERSION}' NetworkManager-lang)
 # remove the old non-versioned package (built in the bs for instance)

diff --git a/tests/out/replace_pwdutils.spec b/tests/out/replace_pwdutils.spec
@@ -3,6 +3,7 @@ Version:        3.1.23
 Release:        0
 Summary:        A Job Manager
 License:        GPL-2.0-or-later
+Group:          System/Daemons
 URL:            http://ftp.debian.org/debian/pool/main/a/at
 Source:         http://ftp.debian.org/debian/pool/main/a/at/%{name}_%{version}.orig.tar.gz
 BuildRequires:  autoconf >= 2.69

diff --git a/tests/out/rpmcallpkg.spec b/tests/out/rpmcallpkg.spec
@@ -1,6 +1,7 @@
 %if %{with kde4}
 %package -n %{name}-client-kde4
 Summary:        KDE 4 Backend for sflphone
+Group:          Productivity/Telephony/SIP/Clients
 Requires:       %{name} = %{version}-%{release}
 # For building with KDE 4.12
 # % kde4_akonadi_requires == "Requires: akonadi-runtime  >= 1.10.2 akonadi-runtime < 1.10.40" (on openSUSE 13.1)

diff --git a/tests/out/rpmpreamble.spec b/tests/out/rpmpreamble.spec
@@ -23,6 +23,7 @@ Version:        0.9.49
 Release:        0
 Summary:        Small Embeddable HTTP Server Library
 License:        LGPL-2.1-or-later
+Group:          Productivity/Networking/Web/Servers
 URL:            https://www.gnu.org/software/libmicrohttpd/
 Source0:        http://ftp.gnu.org/gnu/libmicrohttpd/%{name}-%{version}.tar.gz
 Source1:        http://ftp.gnu.org/gnu/libmicrohttpd/%{name}-%{version}.tar.gz.sig
@@ -70,12 +71,14 @@ application, check it out.
 
 %package -n %{libmicrohttpd}
 Summary:        Small embeddable http server library
+Group:          System/Libraries
 
 %description -n %{libmicrohttpd}
 Shared library for %{name} (%{summary}).
 
 %package devel
 Summary:        Small Embeddable HTTP Server Library
+Group:          Development/Libraries/C and C++
 Requires:       %{libmicrohttpd} = %{version}
 Requires:       pkgconfig
 Requires:       pkgconfig(gnutls)

diff --git a/tests/out/spec-cleaner.spec b/tests/out/spec-cleaner.spec
@@ -22,6 +22,7 @@ Version:        0.4.1
 Release:        0
 Summary:        .spec file cleaner
 License:        BSD-3-Clause
+Group:          Development/Tools/Other
 URL:            https://github.com/openSUSE/spec-cleaner
 Source0:        https://github.com/openSUSE/%{name}/archive/%{name}-%{version}.tar.gz
 BuildRequires:  python

diff --git a/tests/out/susekmp.spec b/tests/out/susekmp.spec
@@ -1,5 +1,6 @@
 %package guest-KMP
 Summary:        Guest kernel modules for VirtualBox
+Group:          System/Emulators/PC
 Supplements:    modalias(dmi:*:[bs]vnD[Ee][Ll][Ll]*:*)
 Supplements:    modalias(x86cpu:vendor%%3A0002%%3Afamily%%3A*%%3Amodel%%3A*%%3Afeature%%3A*)
 #SUSE specify macro to define guest kmp package