dnf5 icon indicating copy to clipboard operation
dnf5 copied to clipboard

`dnf5 -x=dnf install dnf` does not work as expected

Open j-mracek opened this issue 10 months ago • 15 comments

-x=dnf adds exclude = =dnf to configuration. Try --dump-main-config to see it.

-x dnf works as expected

Proposing the bug as a bug with high priority, because the option is important and other options might be affected as well.

j-mracek avatar Mar 28 '24 09:03 j-mracek

I think we do not support syntax with = for short options. Only for the long ones.

m-blaha avatar Mar 28 '24 10:03 m-blaha

We don't. dnf5(8) manual agrees it's an invalid invocation. I would rather fix the option parser to reject "-x=dnf" as an invalid option.

ppisar avatar Mar 28 '24 12:03 ppisar

Also this behavior should be documented in the dnf4 vs dnf5 changes doc.

jan-kolarik avatar Mar 28 '24 12:03 jan-kolarik

DNF5 command line options format:

Prefix of long option --. The prefix of a list of short options -. The last option in the list may have a value.

Examples with the same meaning:

  • --assumeyes --exclude=dnf (preferred long format, unambiguous format)
  • --assumeyes --exclude dnf (alternative long format, For the parser to correctly decode the string dnf - is it a value of the --exclude option or the next argument - it must know whether or not the --exclude option expects a value.)
  • -yxdnf
  • -yx dnf
  • -y -xdnf
  • -y -x dnf
  • --assumeyes -xdnf
  • --assumeyes -x dnf
  • -y --exclude=dnf
  • -y --exclude dnf

In DNF5, there is already a lot of freedom when entering options on the command line. Introducing another possibility, a short option with a value separated by = (as requested -x=dnf), would be really confusing.

jrohel avatar Mar 28 '24 19:03 jrohel

In DNF5, there is already a lot of freedom when entering options on the command line. Introducing another possibility, a short option with a value separated by = (as requested -x=dnf), would be really confusing.

I agree. I'd prefer to not implement -x=package format in dnf5 and just document the difference (as dnf4 supports it). The format without = is also inline with POSIX Utility conventions (https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html)

m-blaha avatar Apr 02 '24 09:04 m-blaha

Be honest -yxdnf looks confusing. Anyway it might be a compatibility issue. It looks like that RPM supports short options with -EARCH, -E ARCH and -E=ARCH

j-mracek avatar Apr 02 '24 09:04 j-mracek

I see as a problem that DNF5 is not unified in using option separators.

--exlude=dnf works but -x=dnf doesn't.

And with excludes you will not get any error, but only a different behavior that might be difficult to discover.

j-mracek avatar Apr 02 '24 09:04 j-mracek

We don't. dnf5(8) manual agrees it's an invalid invocation. I would rather fix the option parser to reject "-x=dnf" as an invalid option.

This would be another option how to handle the issue.

j-mracek avatar Apr 02 '24 09:04 j-mracek

As a console user, I like applications to behave consistently. And when I wrote the argument parser, I wanted it to be consistent with the rest of the system. The = is not a value delimiter in the short options list in the system.

Is the fact that rpm parses the short option list differently than the other standard utilities on the system enough reason for us to do it differently too? I don't want to be surprised when one day I want to set a value with = at the beginning and it omits it as a delimiter.

I understand that it may be a surprise to a beginner that = is not a delimiter value in a short list of options, but to a user who normally works in the Linux console and uses the standard utilities, it's a fact.

Examples:

$ cut -f=2 test.txt 
cut: invalid field value '=2'
$ grep -A=4 needle
grep: =4: invalid context length argument
$ head -n=1 test.txt 
head: invalid number of lines: '=1'

$ tar -cf=something.tar something creates the file =something.tar. The = will be part of the name and not a delimiter.

jrohel avatar Apr 23 '24 08:04 jrohel

The patch (https://github.com/rpm-software-management/dnf5/pull/1434) unifies the behavior with DNF4, RPM and other project that uses Python3 argparse (including CI_DNF_STACK). So far it looks like that it doesn't break anything. Anyway I am going to provide an alternative solution that raises an exception, because the silently skipping this situation looks like the worst option.

j-mracek avatar Apr 24 '24 05:04 j-mracek

Here is an alternative solution mentioned by Petr Pisar above - Reject = with short options - https://github.com/rpm-software-management/dnf5/pull/1442.

j-mracek avatar Apr 24 '24 06:04 j-mracek

I tested DNF4. And I found that it doesn't support the = delimiter in the short options list. The = is only taken as a delimiter if just one short option is listed. The DNF4 parser has another strange rule. So -vx=dnf causes the exclude =dnf to be set. Same behaviour as in DNF5. In other words, DNF5 is incompatible with DNF4 only if the short options list contains just one short option.

# dnf -vx=dnf install dnf -> Excludes in dnf.conf: =dnf # dnf -vxdnf install dnf -> Excludes in dnf.conf: dnf # dnf -v -x=dnf install dnf -> Excludes in dnf.conf: dnf # dnf -v -xdnf install dnf -> Excludes in dnf.conf: dnf

jrohel avatar Apr 24 '24 13:04 jrohel

@jrohel I understand your point and I am not pushing any solution. I have one very strong concern - the change of behavior is silent therefore very difficult to discover.

So far we have 4 options

  1. Keep the code like it is and only document the change I consider this solution a risky because it would be easy to overlook the change because nothing will fail

  2. Do nothing (no comment)

  3. Raise an exception when = is the first character in an argument and document the change This will make the change transparent, but it will create a noise.

  4. Provide a compatible solution with DNF4

DNF and DNF5 does not validate configuration - string =dnf is not a valid name of RPM

j-mracek avatar Apr 25 '24 09:04 j-mracek

I was searching for some existing standard related to this topic and found the following link: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html. According to 2. a., conforming applications should support both -xdnf and -x dnf variants. If we want to comply, we should drop support for the = delimiter for short options. I didn't find an explicit standard for long options mentioning the use of the = delimiter, but it seems generally accepted with other standard commands and is also supported by UNIX getopt.

Given the existing feedback, I'm personally in favor of not supporting = as a delimiter for short options. However, we should document this change clearly and include it in the dnf4 vs. dnf5 changes documentation. I'm not sure about always throwing an error when the = delimiter is used with short options, as it might be a valid value for some future option, as Jarek mentioned. Instead, I'd prefer a more consistent approach: if a specific character isn't valid for a particular option and needs validation, then let's throw an error, and it should apply consistently wherever the validation fails.

EDIT: Oh, Marek had already posted the IEEE link. Although I read his post, I somehow missed this. :)

jan-kolarik avatar Apr 30 '24 08:04 jan-kolarik

Since there has been no feedback for some time, let's finalize our implementation approach.

It seems we agree on not supporting the = delimiter for short options. If so, we should document this properly in the changes docs, etc.

The last thing to decide is whether to throw an exception when the = character is used after a short option (see my previous comment).

jan-kolarik avatar Jun 05 '24 07:06 jan-kolarik