PR#3417: save source for wrapperRPM - koji

koji

#3417 save source for wrapperRPM

Merged a year ago by tkopecek. Opened 2 years ago by tkopecek.

tkopecek/koji issue3409 into master

drop custom_user_metadata propagation

Tomas Kopecek • 2 years ago

eb65bfc

save source for wrapperRPM

Tomas Kopecek • 2 years ago

19205d8

builder/kojid

file modified

+6 -1

		`@@ -2069,6 +2069,8 @@`
		`srcdir=specdir,`
		`buildroot=buildroot)`

		`+ # get the source before chown, git > 2.35.2 would refuse to that later`
		`+ source = scm.get_source()`
		`spec_template = None`
		`for path, dir, files in os.walk(specdir):`
		`files.sort()`
		`@@ -2136,6 +2138,8 @@`
		`h = koji.get_rpm_header(srpm)`
		`data = koji.get_header_fields(h, ['name', 'version', 'release', 'epoch'])`
		`data['task_id'] = self.id`
		`+ data['source'] = source['source']`
		`+ data['extra'] = {'source': {'original_url': source['url']}}`
		`self.logger.info("Reading package config for %(name)s" % data)`
		`pkg_cfg = self.session.getPackageConfig(build_target['dest_tag'], data['name'])`
		`if not opts.get('skip_tag'):`
		`@@ -2205,7 +2209,8 @@`
		`results = {'buildroot_id': buildroot.id,`
		`'srpm': srpm,`
		`'rpms': rpms,`
		`- 'logs': logs}`
		`+ 'logs': logs,`
		`+ 'source': source}`

		`if not task:`
		`# Called as a standalone top-level task, so handle the rpms now.`

tkopecek commented 2 years ago

rebased onto 26bf70598466083a2c5dae7f4c87c67bf471d760

2 years ago

pretty please pagure-ci rebuild

2 years ago

mikem commented 2 years ago

Overall notion of enhancing the saved data for WrapperRPM builds is find and the code looks good overall.

This PR does more than it says. The title only mentions saving source, but it also:

saves logs
saves custom_user_metadata

Saving the logs isn't mentioned here or in the original issue, though it is certainly a valid feature to add. We should make sure the title/description of the PR is accurate.

The bit with custom_user_metadata probably needs some adjustment. This change propagates the field from the task opts to extra, as the build task does. However, we don't provide a way to set that opt, short of calling the api directly. We should probably add the --custom-user-metadata option to the wrapper-rpm command.

It's also worth noting that many wrapperRPM tasks are triggered as subtasks of other tasks, and this makes me wonder if we need to propagate that option (granted, none of those tasks have a --custom-user-metadata option either. This sort of propagation might be out of scope for this issue.

mikem commented 2 years ago

I'm a little uncertain how we want custom-user-metadata to work outside the fairly narrow scope where it was first introduced. It might be best to leave this out and revisit it later.

tkopecek commented 2 years ago

I've removed custom_user_metadata part. Logs are not part of the PR (it was there, just added comma in the end).

rebased onto 19205d8

2 years ago

mikem commented 2 years ago

OK, looks fine, though I note that #3409 has now been edited to also mention custom_user_metadata. I still think that part is worth a more consideration before we change it.

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

2 years ago