#2208 frontend: show source package version without %dist
Merged 2 years ago by praiskup. Opened 2 years ago by frostyx.
copr/ frostyx/copr srpm-version-without-dist  into  main

file modified
+2 -2
@@ -240,7 +240,7 @@ 

  install -m 644 main.ini %{buildroot}%{_sysconfdir}/copr-rpmbuild/main.ini

  install -m 644 mock.cfg.j2 %{buildroot}%{_sysconfdir}/copr-rpmbuild/mock.cfg.j2

  install -m 644 rpkg.conf.j2 %{buildroot}%{_sysconfdir}/copr-rpmbuild/rpkg.conf.j2

- install -m 644 mock-source-build.cfg %{buildroot}%{_sysconfdir}/copr-rpmbuild/

+ install -m 644 mock-source-build.cfg.j2 %{buildroot}%{_sysconfdir}/copr-rpmbuild/

  

  cat <<EOF > %buildroot%mock_config_overrides/README

  Contents of this directory is used by %_bindir/copr-update-builder script.
@@ -304,7 +304,7 @@ 

  %config(noreplace) %{_sysconfdir}/copr-rpmbuild/main.ini

  %config(noreplace) %{_sysconfdir}/copr-rpmbuild/mock.cfg.j2

  %config(noreplace) %{_sysconfdir}/copr-rpmbuild/rpkg.conf.j2

- %config(noreplace) %{_sysconfdir}/copr-rpmbuild/mock-source-build.cfg

+ %config(noreplace) %{_sysconfdir}/copr-rpmbuild/mock-source-build.cfg.j2

  

  %files -n copr-builder

  %license LICENSE

@@ -334,7 +334,8 @@ 

      if task_id:

          macros["%buildtag"] = ".copr" + re.sub("-.*", "", task_id)

  

-     if not task["chroot"]:

+     if is_srpm_build(task):

+         macros["%dist"] = "%nil"

          macros["%_disable_source_fetch"] = "0"

  

      protocols_str = config.get("main", "enabled_source_protocols", fallback=None)
@@ -344,3 +345,10 @@ 

          macros["%__urlhelper_localopts"] = "--proto -all,{0}".format(protocols)

  

      return macros

+ 

+ 

+ def is_srpm_build(task):

+     """

+     Return `True` if the `self.source_dict` belongs to a SRPM build task

+     """

+     return "source_type" in task

@@ -4,8 +4,10 @@ 

  import shutil

  import stat

  import tempfile

+ from jinja2 import Environment, FileSystemLoader

  

  from copr_common.request import SafeRequest

+ from copr_rpmbuild.helpers import CONF_DIRS

  

  

  log = logging.getLogger("__main__")
@@ -109,6 +111,25 @@ 

              for key, value in self.macros.items():

                  rpmmacros.write("{0} {1}\n".format(key, value))

  

+     def generate_mock_config(self, config_name=None):

+         """

+         Generate a mock config file for a specific task

+         """

+         config_name = config_name or "mock-source-build.cfg"

+         template_name = config_name + ".j2"

+         mock_config_file = os.path.join(self.resultdir, config_name)

+         with open(mock_config_file, "w") as fd:

+             fd.write(self.render_mock_config_template(template_name))

+         return mock_config_file

+ 

+     def render_mock_config_template(self, template_name):

+         """

+         Return a mock config (as a string) for a specific task

+         """

+         jinja_env = Environment(loader=FileSystemLoader(CONF_DIRS))

+         template = jinja_env.get_template(template_name)

+         return template.render(macros=self.macros)

+ 

      def produce_srpm(self):

          """

          Using the TASK dict and the CONFIG, generate a source RPM in the

@@ -36,19 +36,22 @@ 

          with open(self.file_script, 'w') as script:

              script.write(source_json['script'])

  

+     def render_mock_config_template(self, *_args):

+         template = "include('/etc/mock/{0}.cfg')\n".format(self.chroot)

+         template += "config_opts['rpmbuild_networking'] = True\n"

+         template += "config_opts['use_host_resolv'] = True\n"

  

-     def produce_srpm(self):

-         mock_config_file = os.path.join(self.resultdir, 'mock-config.cfg')

+         # Important e.g. to keep '/script' file available across several

+         # /bin/mock calls (when tmpfs_enable is on).

+         template += "config_opts['plugin_conf']['tmpfs_opts']['keep_mounted'] = True\n"

  

-         with open(mock_config_file, 'w') as f:

-             # Enable network.

-             f.write("include('/etc/mock/{0}.cfg')\n".format(self.chroot))

-             f.write("config_opts['rpmbuild_networking'] = True\n")

-             f.write("config_opts['use_host_resolv'] = True\n")

-             # Important e.g. to keep '/script' file available across several

-             # /bin/mock calls (when tmpfs_enable is on).

-             f.write("config_opts['plugin_conf']['tmpfs_opts']['keep_mounted'] = True\n")

+         for key, value in self.macros.items():

+             template += ("config_opts['macros']['{0}'] = '{1}'\n"

+                          .format(key, value))

+         return template

  

+     def produce_srpm(self):

+         mock_config_file = self.generate_mock_config("mock-config.cfg")

          cmd = [

              'unbuffer',

              'copr-sources-custom',

@@ -120,8 +120,9 @@ 

          make_srpm_cmd_part = MAKE_SRPM_TEPMLATE.format(mock_cwd, makefile_path,

                  mock_resultdir, mock_spec_path)

  

+         mock_config_file = self.generate_mock_config()

          return ['mock', '--uniqueext', get_mock_uniqueext(),

-                 '-r', '/etc/copr-rpmbuild/mock-source-build.cfg',

+                 '-r', mock_config_file,

                  mock_bind_mount_cmd_part, '--chroot', make_srpm_cmd_part]

  

      def produce_srpm(self):

@@ -22,8 +22,9 @@ 

          return path

  

      def build_srpm_from_spec(self):

+         mock_config_file = self.generate_mock_config()

          spec_path = self.save_spec()

-         cmd = ["mock", "-r", "/etc/copr-rpmbuild/mock-source-build.cfg",

+         cmd = ["mock", "-r", mock_config_file,

                 "--buildsrpm", "--spec", spec_path,

                 "--resultdir", self.resultdir]

  

rpmbuild/mock-source-build.cfg.j2 rpmbuild/mock-source-build.cfg
file renamed
+4
@@ -7,3 +7,7 @@ 

  config_opts['plugin_conf']['bind_mount_enable'] = True

  config_opts['use_bootstrap'] = False

  config_opts['nspawn_args'] = ['--drop-capability=CAP_SYS_ADMIN,CAP_IPC_OWNER,CAP_KILL,CAP_LEASE,CAP_LINUX_IMMUTABLE,CAP_NET_BIND_SERVICE,CAP_NET_BROADCAST,CAP_NET_RAW,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT,CAP_SYS_NICE,CAP_SYS_PTRACE,CAP_SYS_TTY_CONFIG,CAP_SYS_RESOURCE,CAP_SYS_BOOT,CAP_AUDIT_WRITE,CAP_AUDIT_CONTROL']

+ 

+ {% for key, value in macros.items() %}

+ config_opts['macros']['{{ key }}'] = '{{ value }}'

+ {% endfor %}

file modified
+7 -6
@@ -161,6 +161,8 @@ 

  config_opts['macros']['%copr_projectname'] = 'copr-dev'

  config_opts['macros']['%vendor'] = 'Copr Testsuite - group @copr'

  config_opts['macros']['%buildtag'] = '.copr10'

+ config_opts['macros']['%dist'] = '%nil'

+ config_opts['macros']['%_disable_source_fetch'] = '0'

  

  """  # TODO: make the output nicer

  
@@ -235,14 +237,13 @@ 

          call = f_mock_calls[1]

          assert call[0][0] == self.mock_rpm_call

  

-         part_of_expected_output = (

-             "config_opts['macros']['%buildtag'] = '.copr10'\n"

-             "config_opts['module_setup_commands'] = {0}\n\n"

-              .format(str([('enable', module) for module in modules]))

-         )

+         expected1 = "config_opts['macros']['%buildtag'] = '.copr10'\n"

+         expected2 = ("config_opts['module_setup_commands'] = {0}\n\n"

+                      .format(str([('enable', module) for module in modules])))

  

          config = ''.join(open(self.child_config, 'r').readlines())

-         assert part_of_expected_output in config

+         assert expected1 in config

+         assert expected2 in config

  

      @pytest.mark.parametrize('modules', [

          [], # dict expected

file modified
+6 -4
@@ -119,8 +119,7 @@ 

          self.assertEqual(provider.get_tito_test_command(), assert_cmd)

  

      @mock.patch("copr_rpmbuild.providers.scm.get_mock_uniqueext")

-     @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open())

-     def test_get_make_srpm_command(self, mock_open, get_mock_uniqueext_mock):

+     def test_get_make_srpm_command(self, get_mock_uniqueext_mock):

          tmpdir = tempfile.mkdtemp(prefix="copr-rpmbuild-test-")

          ws = os.path.join(tmpdir, "workspace")

          rd = os.path.join(tmpdir, "resultdir")
@@ -149,8 +148,11 @@ 

              "/mnt/{0}".format(basename),

              "/mnt/{0}/somerepo/subpkg/pkg.spec".format(workdir_base),

          )

-         assert_cmd = ['mock', '--uniqueext', '2', '-r', '/etc/copr-rpmbuild/mock-source-build.cfg',

-                       bind_mount_cmd_part, '--chroot', make_srpm_cmd_part]

+         assert_cmd = [

+             'mock', '--uniqueext', '2',

+             '-r', os.path.join(provider.resultdir, "mock-source-build.cfg"),

+             bind_mount_cmd_part, '--chroot', make_srpm_cmd_part

+         ]

  

          self.assertEqual(provider.get_make_srpm_command(), assert_cmd)

          shutil.rmtree(tmpdir)

file modified
+6 -2
@@ -40,13 +40,17 @@ 

      @mock.patch('copr_common.request.SafeRequest.get')

      @mock.patch("copr_rpmbuild.providers.spec.run_cmd")

      @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open())

+     @mock.patch("copr_rpmbuild.providers.spec.UrlProvider.create_rpmmacros")

+     @mock.patch("copr_rpmbuild.providers.spec.UrlProvider.generate_mock_config")

      @mock.patch('copr_rpmbuild.providers.base.os.mkdir')

-     def test_produce_srpm(self, mock_mkdir, mock_open, run_cmd, mock_get):

+     def test_produce_srpm(self, mock_mkdir, mock_generate_mock_config,

+                           _mock_create_rpmmacros, mock_open, run_cmd, mock_get):

+         mock_generate_mock_config.return_value = "/path/to/mock-source-build.cfg"

          macros = {"_disable_source_fetch": 0}

          provider = UrlProvider(self.source_json, self.config, macros)

          provider.produce_srpm()

          args = [

-             'mock', '-r', '/etc/copr-rpmbuild/mock-source-build.cfg',

+             'mock', '-r', '/path/to/mock-source-build.cfg',

              '--buildsrpm',

              '--spec', '{0}/somepackage.spec'.format(provider.workdir),

              '--resultdir', self.config.get("main", "resultdir"),

It was reported several times, that the %dist part of the version is
confusing because it is evaluated to the Fedora versions that is
currently running on our builders, instead of the target chroot.

We could either add some tooltip explaining this, or we can simply
hide the %dist part from users. I think it is not useful anyway.

Metadata Update from @frostyx:
- Pull-request tagged with: wip

2 years ago

My question is, do we want this?
If yes, I will update also other places where we print the source package version (API, and probably somewhere else)

Build succeeded.

We could either add some tooltip explaining this, or we can simply
hide the %dist part from users. I think it is not useful anyway.

There's also an option to run rpmbuild -bs --define "dist %nil" to produce
empty string... not sure what you think about it? For uploaded SRPMs we could
e.g. re-package.

My question is, do we want this?

This can have consequences.
The thing is that replacing .* can remove part of the version (if there's
%nil-ed %dist requested by the user), or it can remove only part of %dist (if there's
some complicated %dist variant).

rebased onto fe3ce3cab9097b0c64f692c5dcb2f247fbb7dc2b

2 years ago

Metadata Update from @frostyx:
- Pull-request untagged with: wip

2 years ago

Build succeeded.

3 new commits added

  • rpmbuild: define copr-specific macros for make_srpm method
  • rpmbuild: define copr-specific macros for custom builds
  • rpmbuild: determine SRPM builds by having source_type
2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

There's also an option to run rpmbuild -bs --define "dist %nil"

Re-implemented using this method

Also, when working on this feature, I noticed that we don't define Copr-specific macros like %copr_username for the custom method and for make_srpm method, so I added them. That's why this PR got bloated.

5 new commits added

  • rpmbuild: a step towards unifying mock template generation
  • rpmbuild: define copr-specific macros for make_srpm method
  • rpmbuild: define copr-specific macros for custom builds
  • rpmbuild: determine SRPM builds by having source_type
  • rpmbuild: undefine %dist for SRPM builds
2 years ago

Build succeeded.

Please in-line comment this, it is not obvious what we are trying to guess by this if-condition.

Perhaps we could have something like task.is_source_build() wrapper?

Very nice change. Would you mind adding a test case for make srpm and custom methods, checking that the macros are actually generated?

Tests are there, no need to add new.

5 new commits added

  • rpmbuild: a step towards unifying mock template generation
  • rpmbuild: define copr-specific macros for make_srpm method
  • rpmbuild: define copr-specific macros for custom builds
  • rpmbuild: determine SRPM builds by having source_type
  • rpmbuild: undefine %dist for SRPM builds
2 years ago

Please in-line comment this, it is not obvious what we are trying to guess by this if-condition.
Perhaps we could have something like task.is_source_build() wrapper?

Good idea, fixed

Build succeeded.

rebased onto 01d8e0f

2 years ago

Commit 70f21e0 fixes this pull-request

Pull-Request has been merged by praiskup

2 years ago

Commit ba2c52c fixes this pull-request

Pull-Request has been merged by praiskup

2 years ago

Commit 26c95b8 fixes this pull-request

Pull-Request has been merged by praiskup

2 years ago

Commit 2e79690 fixes this pull-request

Pull-Request has been merged by praiskup

2 years ago
Commit [2344ea31](https://pagure.io/copr/copr/c/2344ea31) fixes this pull-request

Pull-Request has been merged by praiskup

2 years ago

Build succeeded.