PR#4274: Don't prepopulate log list for mavenBuild - koji

koji

#4274 Don't prepopulate log list for mavenBuild

Merged 4 months ago by tkopecek. Opened 4 months ago by tkopecek.

tkopecek/koji issue4273 into master

include the checkout logs in task result

Mike McLean • 4 months ago

9a6b666

Don't prepopulate log list for mavenBuild

Tomas Kopecek • 4 months ago

51f5a4e

builder/kojid

file modified

+7 -4

		`@@ -2033,9 +2033,7 @@`
		`os.path.join(outputdir, maven_info['artifact_id'] + '-' +`
		`maven_info['version'] + '-patches.zip'))`

		`- logs = ['checkout.log']`
		`- if self.opts.get('patches'):`
		`- logs.append('patches.log')`
		`+ logs = []`
		`output_files = {}`

		`for path, dirs, files in os.walk(outputdir):`
		`@@ -2058,9 +2056,14 @@`
		`self.uploadFile(os.path.join(outputdir, relpath, filename),`
		`relPath=relpath)`

		`- # Also include the logs already upload by BuildRoot`
		`+ # Also include the logs already uploaded by BuildRoot`
		`logs.extend(buildroot.logs)`

		`+ # Also include the checkout logs (uploaded during checkout)`
		`+ logs.append('checkout.log')`
		`+ if self.opts.get('patches'):`
		`+ logs.append('patches.log')`
		`+`
		`buildroot.expire()`

		`return {'maven_info': maven_info,`

tkopecek commented 4 months ago

I'm not completely sure that it is ok, but I believe that this is superceded by update in https://pagure.io/koji/c/c51ae5d6b34d67bc903a818241720e0b15c95de4

please doublecheck that I'm not breaking it. (real task in brew 66091839 - see that checkout.log is already there)

mikem commented 4 months ago

The problem is a longstanding bug in the maven code that was revealed by #4093.

The maven code is adding checkout.log to the logs list, where the entries are expected to be relative to outputdir. This file is under the task workdir. It's created by the call to scm.checkout, not by a BuildRoot method, so it won't be listed included in buildroot.logs. It is already uploaded though, because scm.checkout uses log_output.

The maven code is calling uploadFile with a path that never existed.

I suspect that the current version of this PR will prevent the inclusion of these files in the build entry, even though they are uploaded to the task directory.

Edited 4 months ago by mikem

mikem commented 4 months ago

Perhaps this?
https://pagure.io/fork/mikem/koji/commits/pr4274updates