koji

Clone

#1689 cli: Print a warning in latest-build if the tag is not a buildroot
Opened 4 years ago by fweimer. Modified 4 years ago
fweimer/koji master  into  master

Merge
file modified
+16 
@@ -2310,6 +2310,9 @@ 
 

                  help=_("Do not print the header information"))
 

      parser.add_option("--paths", action="store_true", help=_("Show the file paths"))
 

      parser.add_option("--type", help=_("Show builds of the given type only.  Currently supported types: maven"))
 

+     parser.add_option("--disable-buildroot-check", action="store_true",
 

+                       default=False,
 

+                       help=_("Do not check that the tag has a buildroot"))
 

      (options, args) = parser.parse_args(args)
 

      if len(args) == 0:
 

          parser.error(_("A tag name must be specified"))
 
@@ -2327,6 +2330,19 @@ 
 

              assert False  # pragma: no cover
 

      pathinfo = koji.PathInfo()
 

  

+     if not options.disable_buildroot_check:
 

+         targets = session.getBuildTargets(args[0])

Is there any reason to use getBuildTargets instead of getBuildTarget? You are querying against strict name, so you can never get more than one (would also simplify rest of code)

 
+         build_tag_names = [target["build_tag_name"] for target in targets]
 

+         if targets and args[0] not in build_tag_names:
 

+             warn(_("warning: %r is not a buildroot tag.") % args[0])
 

+             if len(build_tag_names) == 1:
 

+                 warn(_("warning: Did you mean %r instead?")
 

+                      % build_tag_names[0])
 

+             else:
 

+                 warn(_("warning: Did you mean any of %s instead?")
 

+                      % ", ".join([repr(suggestion)
 

+                                   for suggestion in build_tag_names]))
 

+ 

      for pkg in args[1:]:
 

          if options.arch:
 

              rpms, builds = session.getLatestRPMS(args[0], package=pkg, arch=options.arch)

Warning: I'm not familiar with the Koji codebase. But I think a warning like this would be very useful to casual Koji users.

If you accidentally use latest-build with a tag argument that is
not a buildroot, latest-build gives very confusing results. This
mistake is easy to make if the tag builds into a buildroot of a
different name. With this patch, we now a warning and suggest to
use the corresponding buildroot tag instead:

$ koji latest-build f32 glibc
warning: 'f32' is not a buildroot tag
warning: Did you mean 'f32-build' instead?
Build Tag Built by

glibc-2.30.9000-11.fc32 f32 submachine

Created https://pagure.io/koji/issue/1808 to track this.
@tkopecek please take a look

Is there any reason to use getBuildTargets instead of getBuildTarget? You are querying against strict name, so you can never get more than one (would also simplify rest of code)

I don't think this is the correct way to get a "BuildRoot" tag, since there is no strict rule to specify buildtarget's name as the same as a tag.
Usually, a BuildTag is treated as a BuildRoot tag, so I think the situation to warn such kind of msg is there is not any BuildTarget with args[0] as its BuildTag.

So I think it is something like

targets = session.getBuildTargets(buildTagID=session.getTag(args[0], strict=True)['id'])
...

I'm not sure I understand what is confusing about the current behavior. Would you please say more about what you expected to happen?

The reason I ask is that I have some code that does interrogate the latest build in a tag that is not a buildroot tag. I guess I can add --disable-buildroot-check to my code now, but this is a little surprising.

I wonder if we should make the "--help" text more verbose for "latest-build" instead?

@ktdreyer The confusing part is that you use koji build f32 to submit builds, but koji latest-build f32 shows package versions which are not actually in the buildroot and are not used by the build.

@julian8628 I do not have any opinion on Koji API use, I just put together something that appeared to be working for me. Sorry.

@julian8628 I do not have any opinion on Koji API use, I just put together something that appeared to be working for me. Sorry.

We should enrich the doc about tag and target :blush:

I have a concern about adding the warning as a default behavior. While cli output is not intended to be an API, there likely are scripts in the wild besides Ken's that are expecting the current output.

I would suggest that we change --disable-buildroot-check to --enable-buildroot-check

I think a warning controlled by a non-default flag defeats its purpose. If you use it, you'll likely be aware of the issue and don't need the warning.

Maybe we can add a different command that requires a buildroot and errors out for other tags? Then we can teach users to use this comment to check buildroot contents.

That's an option too. @tkopecek @mikem what's your opinion here?

It is reasonable and normal to use latest-build on any sort of tag, not just build tags. It seems wrong to print a warning about it. Seems like we need to improve the docs instead.

We could add some text to koji build --help and koji latest-build --help to make it clearer why each one operates on a tag versus a target.

Enriched help #2071
Changes Summary 1
+16 -0
file changed
cli/koji_cli/commands.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.