rpmbuild updates

[marcosfrm]

While trying to create v8 fah-client RPM (see https://github.com/FoldingAtHome/fah-client-bastet/pull/155), I came up with these changes:

diff --git a/config/rpm/__init__.py b/config/rpm/__init__.py
index ea373240..38e2e398 100644
--- a/config/rpm/__init__.py
+++ b/config/rpm/__init__.py
@@ -42,18 +42,23 @@ def install_files(f, env, key, build_dir, path, prefix = None, perms = None,
 def build_function(target, source, env):
     name = env.get('package_name_lower')

-    # Create package build dir
-    build_dir = 'build/%s-RPM' % name
-    if os.path.exists(build_dir): shutil.rmtree(build_dir)
-    os.makedirs(build_dir)
+    # Create directories needed by rpmbuild
+    for dir in ['BUILD', 'BUILDROOT', 'RPMS', 'SOURCES', 'SPECS', 'SRPMS']:
+        rpmdir = 'build/' + dir
+        if os.path.exists(rpmdir): shutil.rmtree(rpmdir)
+        os.makedirs(rpmdir)
+
+    # SPEC's %install section is responsible for populating BUILDROOT
+    # (aka %{buildroot} or $RPM_BUILD_ROOT)
+    build_dir = 'build/BUILD'

     # Create the SPEC file
-    spec_file = 'build/%s.spec' % name
+    spec_file = 'build/SPECS/%s.spec' % name
     with open(spec_file, 'w') as f:
         # Create the preamble
         write_var = env.WriteVariable
         write_var(env, f, 'Summary', 'summary')
-        write_var(env, f, 'Name', 'package_name_lower', None, replace_dash)
+        write_var(env, f, 'Name', 'package_name_lower')
         write_var(env, f, 'Version', 'version', None, replace_dash)
         write_var(env, f, 'Release', 'package_build', '1', replace_dash)
         write_var(env, f, 'License', 'rpm_license')
@@ -68,16 +73,16 @@ def build_function(target, source, env):
         write_var(env, f, 'Conflicts', 'rpm_conflicts', multi = True)
         write_var(env, f, 'Obsoletes', 'rpm_obsoletes', multi = True)
         write_var(env, f, 'BuildRequires', 'rpm_build_requires', multi = True)
-        write_var(env, f, 'Requires(pre)', 'rpm_pre_requires', multi = True)
         write_var(env, f, 'Requires', 'rpm_requires', multi = True)
+        write_var(env, f, 'Requires(pre)', 'rpm_pre_requires', multi = True)
+        write_var(env, f, 'Requires(preun)', 'rpm_preun_requires', multi = True)
+        write_var(env, f, 'Requires(post)', 'rpm_post_requires', multi = True)
         write_var(env, f, 'Requires(postun)', 'rpm_postun_requires',
                   multi = True)

         # Description
         write_spec_text_section(f, env, 'description', 'description')

-        f.write('%define strip ' + env.get('STRIP', 'strip') + '\n\n')
-
         # Scripts
         for script in ['prep', 'build', 'install', 'clean', 'pre', 'post',
                        'preun', 'postun', 'verifyscript']:
@@ -87,18 +92,18 @@ def build_function(target, source, env):
         if 'rpm_filelist' in env:
             f.write('%%files -f %s\n' % env.get('rpm_filelist'))
         else: f.write('%files\n')
-        f.write('%defattr(- root root)\n')

         for files in [
             ['documents', '/usr/share/doc/' + name, '%doc', None],
-            ['programs', '/usr/bin', '%attr(0775 root root)', 0o755],
-            ['scripts', '/usr/bin', '%attr(0775 root root)', 0o755],
+            ['programs', '/usr/bin', '%attr(0775,-,-)', 0o755],
+            ['scripts', '/usr/bin', '%attr(0775,-,-)', 0o755],
             ['desktop_menu', '/usr/share/applications', None, None],
-            ['init_d', '/etc/init.d', '%config %attr(0775 root root)', None],
+            ['init_d', '/etc/init.d', '%config %attr(0775,-,-)', None],
             ['config', '/etc/' + name, '%config', None],
             ['icons', '/usr/share/pixmaps', None, None],
             ['mime', '/usr/share/mime/packages', None, None],
             ['platform_independent', '/usr/share/' + name, None, None],
+            ['misc', '/', None, None],
             ]:
             install_files(f, env, files[0], build_dir, files[1], files[2],
                           files[3])
@@ -106,22 +111,21 @@ def build_function(target, source, env):
         # ChangeLog
         write_spec_text_section(f, env, 'changelog', 'rpm_changelog')

-    # Create directories needed by rpmbuild
-    for dir in ['BUILD', 'BUILDROOT', 'RPMS', 'SOURCES', 'SPECS', 'SRPMS']:
-        dir = 'build/' + dir
-        if not os.path.exists(dir): os.makedirs(dir)
-
+    # rpmbuild strips debug information from binaries by default if %build and
+    # %install sections are present (may be empty)
+    if int(env.get('debug')):
+        cmddebug = ' --define "__strip /usr/bin/true"'
+    else: cmddebug = ''

     # Build the package
-    build_dir = os.path.realpath(build_dir)
-    cmd = 'rpmbuild -bb --buildroot %s --define "_topdir %s/build" ' \
-        '--target %s %s' % (
-        build_dir, os.getcwd(), env.GetPackageArch(), spec_file)
-    CommandAction(cmd).execute(target, [build_dir], env)
+    target = str(target[0])
+    cmd = 'rpmbuild -bb --define "_topdir %s/build" --define "_rpmfilename %s"' \
+          '%s --target %s %s' % (
+          os.getcwd(), target, cmddebug, env.GetPackageArch(), spec_file)
+    CommandAction(cmd).execute(target, [], env)

     # Move the package
-    target = str(target[0])
-    path = 'build/RPMS/' + env.GetPackageArch() + '/' + target
+    path = 'build/RPMS/' + target
     shutil.move(path, target)

Do you think acceptable?

There are missing pieces:

• RPM filename still has underscores ("fah_client-8.2.3-1.x86_64.rpm")
• Debian's systemd "system" path is /lib/systemd/system, while Fedora/RHEL is /usr/lib/systemd/system (did not test https://github.com/FoldingAtHome/fah-client-bastet/pull/155#issuecomment-1758881226 yet)
• Need a way to create directories in the build dir, without copying from anywhere, with %attr(-,fah-client,fah-client) %dir in the SPEC's %files section

Tried with this hack in install_files() (config/rpm/__init__.py):

import re

# Copy or create directory
if re.search(r'%dir', str(prefix)):
    for emptydir in env.get(key):
        os.makedirs(build_dir + path + emptydir)
else:
    env.CopyToPackage(env.get(key), target, perms, dperms)
...

And:

['rpm_client_dirs', '/', '%attr(-,fah-client,fah-client) %dir', None, None],

in build_function().

And:

rpm_client_dirs = ['var/lib/fah-client', 'var/log/fah-client', 'etc/fah-client'],

in fah-client's SConstruct.

(also suggested by @kbernhagen)

But then ResolvePackageFileMap breaks resulting in incomplete %files...

[marcosfrm]

About permissions/ownership in %files, let's drop all of them. Only set a different owner (%attr(-,foo,foo)) when required. From RPM manual:

By default, packaged files are owned by root:root and permission bits
are taken directly from the on-disk files.

[kbernhagen]

I think it's risky to trust that all file permissions are correct in the repo. There have been incorrect perms in the past.

If you introduce a systemd variable that can handle the different service file paths. In SConstruct add

        systemd = ['build/install/lin/fah-client.service'],

Would need to modify cbang deb packager to also use it, and not use var misc in SConstruct for this. Just thinking out loud.

[kbernhagen]

I believe target passed to the scons build function is always an array(list) of str or bytes.

Which is why you may see

str(target[0])

[kbernhagen]

Why are underscores in the package filename a problem?

See GetPackageName in cbang/config/packager. ResolvePackageFileMap is also in there.

[kbernhagen]

To keep the resolve happy, you might need

rpm_client_dirs = [('var/lib/fah-client', '.'), ('var/log/fah-client', '.'), ('etc/fah-client', '.')],

Which makes each element a tuple. I might be off my rocker.

Edit: never mind this. An install_dirs function is a better idea.

[kbernhagen]

Maybe just not use install_files for rpm_client_dirs. Make an install_dirs function.

    install_dirs(f, env, 'rpm_client_dirs', build_dir, '/', '%attr(-,fah-client,fah-client) %dir')

[kbernhagen]

Untested:

def install_dirs(f, env, key, build_dir, path, prefix = None, dperms = 0o755):
    # Create dirs and write dirs list
    for dir in env.get(key):
        os.makedirs(os.path.join(build_dir, path, dir), dperms, True)
        if prefix: f.write(prefix + ' ')
        else: f.write('%dir ')
        f.write(os.path.join(path, dir) + '\n')

Roughly equivalent to your hack, I think. But no resolving excludes. Dirs are expected to be strings, not tuples.

[kbernhagen]

In case rpm_client_dirs has any absolute paths in it, install_dirs maybe should .replace('//', '/'). Or use whatever python has to normalize paths.

[jcoffland]

PR #125 merged.