Issue #2620: "--write" option for koji import-sig has no effect - koji

koji

#2620 "--write" option for koji import-sig has no effect

Closed: Fixed 3 years ago by tkopecek. Opened 3 years ago by ktdreyer.

The import-sig sub-command always calls the writeSignedRPM RPC (if the user does not specify --test). Nothing checks whether the user has specified the --write option, and it's effectively always true. There's no way to run import-sig without writing the signed copies.

This behavior is almost certainly what users want anyway, so we might as well deprecate the --write flag and then remove it from the CLI.

Steps to reproduce:

Do a build in Koji
Download the build
GPG-sign it with a key
Use the import-sig command, eg. koji -p kojidev import-sig bash-5.0.17-2.fc33.src.rpm . Note the output.
Review the command options withkoji import-sig --help

Actual results:
koji import-sig unconditionally writes the signed copies. The koji CLI shows the following output:

Importing signature [key 782096ac] from bash-5.0.17-2.fc33.src.rpm...
Writing signed copy

The --help text incorrectly indicates that Koji will only write the signed copies if the user specifies the --write flag.

Expected results
koji import-sig should unconditionally write the signed copies, and the --help text should mention that --write has no effect and is deprecated. Running import-sig with --write should print a DeprecationWarning.

ktdreyer commented 3 years ago

Looks like it's been this way since we introduced this flag in f570704

The unit tests actually do verify that session.writeSignedRPM.assert_has_calls(write_sig_calls)

ktdreyer commented 3 years ago

We could update the help text to indicate this as well.

 def handle_import_sig(goptions, session, args):
-    "[admin] Import signatures into the database"
+    "[admin] Import signatures into the database and write signed RPMs"

Metadata Update from @tkopecek:
- Custom field Size adjusted to None
- Issue set to the milestone: 1.24

3 years ago

mikem commented 3 years ago

I would prefer to not break working command lines. I would suggest making the option hidden in the help, but still accept it for compatibility. I don't know if we need to actually deprecate it. Many Unix commands accept such historical options for compatibility.

ktdreyer commented 3 years ago

Ok. Looks like the way to do this in optparse is help=SUPPRESS_HELP.

tkopecek commented 3 years ago

PR #2654